[PATCH v2 04/11] efi_loader: enable EFI runtime SetVariable()/GetVariable() using FF-A transport

[Harsimran Singh Tungal]

On 2026-05-15 12:26 -0600, Simon Glass wrote:
> Hi Harsimran,
> 
> On 2026-05-14T12:49:13, Harsimran Singh Tungal
> <harsimransingh.tungal at arm.com> wrote:
> > efi_loader: enable EFI runtime SetVariable()/GetVariable() using FF-A transport
> >
> > Route EFI runtime variable APIs through FF-A MM communication
> >
> > Route EFI runtime variable services through the FF-A/MM backend in
> > lib/efi_loader/efi_variable_tee.c. After ExitBootServices(),
> > GetVariable(), SetVariable(), GetNextVariableName(), and
> > QueryVariableInfo() use the runtime entry points and continue to reach
> > the MM secure partition.
> >
> > Keep the existing boot-time helpers unchanged and add runtime service
> > wrappers for variable access and property handling. Reuse the
> > runtime-safe setup_mm_hdr() and common mm_communicate() path, which
> > selects the FF-A transport appropriate for the current phase, and use
> > the EFI runtime-safe memory helpers in the runtime-only code.
> >
> > Signed-off-by: Harsimran Singh Tungal <harsimransingh.tungal at arm.com>
> >
> > lib/efi_loader/efi_variable_tee.c | 320 +++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 315 insertions(+), 5 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> > diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> > @@ -830,6 +905,95 @@ out:
> > +efi_status_t __efi_runtime efi_get_variable_runtime(u16 *variable_name,
> > +                                                 const efi_guid_t *vendor,
> > +                                                 u32 *attributes,
> > +                                                 efi_uintn_t *data_size,
> > +                                                 void *data)
> 
> EFIAPI is missing so this will break on x86_64 where there is a
> different ABI. Same for efi_get_next_variable_name_runtime() below -
> please match efi_set_variable_runtime() /
> efi_query_variable_info_runtime() which already carry EFIAPI.
> 
> > diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> > @@ -743,6 +775,49 @@ out:
> > +     ret = mm_communicate(comm_buf, payload_size);
> > +     /*
> > +      * Currently only R/O property is supported in StMM.
> > +      * Variables that are not set to R/O will not set the property in StMM
> > +      * and the call will return EFI_NOT_FOUND. We are setting the
> > +      * properties to 0x0 so checking against that is enough for the
> > +      * EFI_NOT_FOUND case.
> > +      */
> > +     if (ret == EFI_NOT_FOUND)
> > +             ret = EFI_SUCCESS;
> > +     if (ret != EFI_SUCCESS)
> > +             return ret;
> > +     efi_memcpy_runtime(var_property, &smm_property->property, sizeof(*var_property));
> 
> When EFI_NOT_FOUND is mapped to EFI_SUCCESS we still fall through and
> memcpy smm_property->property over the zeroed var_property - at
> runtime smm_property aliases the FF-A shared buffer, so whatever the
> SP left in those bytes ends up in var_property and undoes the memset
> above. Either move the memcpy under an explicit found branch, or skip
> it on the NOT_FOUND-mapped-to-SUCCESS path. The boot-time copy doesn't
> matter because the SP fills the response into the local buffer; here
> the shared buffer is the response.
> 
> > diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> > @@ -1067,7 +1319,65 @@ efi_set_variable_runtime(u16 *variable_name, const efi_guid_t *guid,
> > +     if (ro && !(var_property.property & VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY)) {
> > +             var_property.revision = VAR_CHECK_VARIABLE_PROPERTY_REVISION;
> > +             var_property.property |= VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY;
> > +             var_property.attributes = attributes;
> > +             var_property.minsize = 1;
> > +             var_property.maxsize = var_acc->data_size;
> > +             ret = set_property_int_runtime(variable_name, name_size, guid, &var_property);
> > +     }
> 
> Just to check - at runtime var_acc points into the FF-A shared buffer,
> and the preceding mm_communicate() has already let the SP write its
> response there, so var_acc->data_size is whatever the SP reported (and
> may be 0 on failure). Is that what we want as maxsize, or should this
> be the original data_size argument? Please confirm the SP guarantees
> data_size on the way out for SET_VARIABLE before relying on it.
> 
> Regards,
> Simon
>

Hi Simon,

Thanks for pointing this out.

It make sense to allow the boot-time and runtime TEE SetVariable() paths to use the
original data_size argument for var_property.maxsize, instead of relying
on var_acc->data_size after the MM exchange.
I will update this in v3.

Regards,
Harsimran Singh Tungal