Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Missing compiler memory barrier causes dead-store elimination and 
constant-folding of the RSI host call buffer.
--

--- Patch [2]: [PATCH v3 2/6] firmware: smccc: Detect hypervisor via RSI host 
call in CCA Realms ---
commit 5f8398cb1bc3327a58d70463ef37d994ffacca02
Author: Kameron Carr <[email protected]>

firmware: smccc: Detect hypervisor via RSI host call in CCA Realms

This commit modifies arm_smccc_hypervisor_has_uuid() to use rsi_host_call()
when operating in a Realm. It introduces arm_smccc_realm_get_hypervisor_uuid()
which uses a spinlock-protected file-static buffer to make the call.

> diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
> index bdee057db2fd3..a876b7aa2dc99 100644
> --- a/drivers/firmware/smccc/smccc.c
> +++ b/drivers/firmware/smccc/smccc.c
[ ... ]
> @@ -67,12 +73,45 @@ s32 arm_smccc_get_soc_id_revision(void)
>  }
>  EXPORT_SYMBOL_GPL(arm_smccc_get_soc_id_revision);
>  
> +#ifdef CONFIG_ARM64
> +static struct rsi_host_call uuid_hc;
> +static DEFINE_SPINLOCK(uuid_hc_lock);
> +
> +/*
> + * Helper function to get the hypervisor UUID via an RsiHostCall.
> + */
> +static void arm_smccc_realm_get_hypervisor_uuid(struct arm_smccc_res *res)
> +{
> +     guard(spinlock_irqsave)(&uuid_hc_lock);
> +
> +     memset(&uuid_hc, 0, sizeof(uuid_hc));
> +     uuid_hc.gprs[0] = ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID;
> +
> +     if (rsi_host_call(__pa_symbol(&uuid_hc)) != RSI_SUCCESS) {
> +             res->a0 = SMCCC_RET_NOT_SUPPORTED;
> +             return;
> +     }
> +
> +     res->a0 = uuid_hc.gprs[0];

[Severity: High]
Does this code need a compiler memory barrier before reading back the results?

Because __pa_symbol() uses RELOC_HIDE() to explicitly hide the pointer's
provenance from the compiler's alias analysis, the compiler may not realize
that rsi_host_call() modifies uuid_hc.

Without a barrier(), could the compiler constant-fold the reads to their
statically-known previous values? If so, this could result in res->a0 being
optimized to ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID instead of the value
returned by the hypervisor, and potentially allow the stores during
initialization to be dead-store eliminated.

> +     res->a1 = uuid_hc.gprs[1];
> +     res->a2 = uuid_hc.gprs[2];
> +     res->a3 = uuid_hc.gprs[3];
> +}
> +#endif

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to