[PATCH 04/12] efi_loader: enable EFI runtime SetVariable()/GetVariable() using FF-A transport
Simon Glass
sjg at chromium.org
Thu May 7 17:31:47 CEST 2026
Hi Harsimran,
On Tue, 5 May 2026 at 08:30, Harsimran Singh Tungal
<harsimransingh.tungal at arm.com> wrote:
>
> On 2026-04-28 12:16 -0600, Simon Glass wrote:
> > Hi Harsimran,
> >
> > On 2026-04-24T17:31:50, 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
> > >
> > > This patch implements full EFI Runtime Variable Services (GetVariable,
> > > SetVariable, GetNextVariableName, and QueryVariableInfo) using the
> > > FF-A/MM communication backend. Once ExitBootServices() has been invoked,
> > > all variable operations now use the runtime-safe FF-A transport instead
> > > of the boot-time communication paths.
> > >
> > > Key changes:
> > > ============
> > >
> > > - Add runtime-safe variable property handling via FF-A MM SP.
> > > - Add runtime versions of variable access and property operations.
> > > - Replace all standard memory operations with EFI-runtime-safe variants.
> > >
> > > Signed-off-by: Harsimran Singh Tungal <harsimransingh.tungal at arm.com>
> > >
> > > lib/charset.c | 2 +-
> > > lib/efi_loader/efi_variable_tee.c | 322 +++++++++++++++++++++++++++++++++++++-
> > > 2 files changed, 318 insertions(+), 6 deletions(-)
> >
> > > diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> > > @@ -991,6 +1066,92 @@ out:
> > > + /*
> > > + * UEFI > 2.7 needs the attributes set even if the buffer is
> > > + * smaller
> > > + */
> > > + if (attributes) {
> > > + tmp = get_property_int_runtime(variable_name, name_size, vendor,
> > > + &var_property);
> > > + if (tmp != EFI_SUCCESS) {
> > > + ret = tmp;
> > > + return ret;
> > > + }
> > > + *attributes = var_acc->attr;
> > > + if (var_property.property &
> > > + VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY)
> > > + *attributes |= EFI_VARIABLE_READ_ONLY;
> > > + }
> >
> > get_property_int_runtime() ends up calling setup_mm_hdr() ->
> > get_comm_buf(), and at runtime get_comm_buf() does
> > efi_memset_runtime(ffa_shared_buf, 0, CONFIG_FFA_SHARED_MM_BUF_SIZE)
> > on the same shared buffer that var_acc points into.
> >
> > By the time you read var_acc->attr it has been zeroed, so
> > '*attributes' will only ever come back as 0 (optionally OR'd with
> > EFI_VARIABLE_READ_ONLY). The boot-time path gets away with this
> > because get_property_int() calloc()s a fresh buffer; the runtime path
> > cannot. Please cache var_acc->attr in a local before the call, or move
> > the get_property_int_runtime() call ahead of the SP exchange.
> >
> Agreed, you're right.
>
> `get_property_int_runtime()` reuses the same runtime FF-A shared buffer, so
> the second call can clear the buffer backing `var_acc` before `var_acc->attr`
> is copied out.
>
> For v2, I will cache `var_acc->attr` in a local before calling
> `get_property_int_runtime()` and then used that cached value when updating
> `*attributes`.
>
> Thanks
>
> > > diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> > > @@ -1228,7 +1477,70 @@ efi_set_variable_runtime(u16 *variable_name, const efi_guid_t *guid,
> > > + ro = !!(attributes & EFI_VARIABLE_READ_ONLY);
> > > + attributes &= EFI_VARIABLE_MASK;
> > > +
> > > + ret = get_property_int_runtime(variable_name, name_size, guid,
> > > + &var_property);
> > > + if (ret != EFI_SUCCESS)
> > > + return ret;
> > > +
> > > + /*
> > > + * Allocate the buffer early, before switching to RW (if needed)
> > > + * so we won't need to account for any failures in reading/setting
> > > + * the properties, if the allocation fails
> > > + */
> > > + comm_buf = setup_mm_hdr((void **)&var_acc, payload_size,
> > > + SMM_VARIABLE_FUNCTION_SET_VARIABLE, &ret);
> > > + if (!comm_buf)
> > > + return ret;
> > > +
> > > + if (var_property.property & VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY)
> > > + return EFI_WRITE_PROTECTED;
> >
> > The comment is copied from efi_set_variable_int() and no longer
> > matches the code - there is no RW switch in the runtime path (no
> > ro_check argument), and the allocation happens after the property
> > read, the opposite of what the comment claims. Either drop the comment
> > or, better, do the RO check before calling setup_mm_hdr() so the
> > early-return is quicker.
> >
>
> Agreed.
>
> That comment was copied from the boot-time path.
> For v2, I will drop it and move the `VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY`
> check ahead of `setup_mm_hdr()`, so the runtime path returns
> `EFI_WRITE_PROTECTED` before allocating the MM communication buffer.
>
> > > diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> > > @@ -859,6 +859,38 @@ out:
> > > +static efi_status_t __efi_runtime set_property_int_runtime(const u16 *variable_name,
> > > + efi_uintn_t name_size,
> > > + const efi_guid_t *vendor,
> > > + struct var_check_property *var_property)
> >
> > As mentioned elsewhere, the four new functions
> > (set_property_int_runtime, get_property_int_runtime,
> > efi_get_variable_runtime, efi_get_next_variable_name_runtime) are
> > near-line-for-line copies of their boot-time counterparts with
> > memcpy/guidcpy/memset swapped for the efi_*_runtime helpers and a
> > different mm_communicate(). Since setup_mm_hdr() was made runtime-safe
> > in patch 3, can the boot-time helpers be reused directly (with
> > mm_communicate() picking the right transport based on
> > efi_is_runtime_enabled())? It would also avoid the special-case free()
> > path.
> >
>
> I kept the separate runtime wrappers intentionally so the
> post-ExitBootServices call graph stays explicit and auditable: only
> runtime-safe helpers, no driver-model assumptions, and no boot-time heap
> cleanup semantics.
>
> I also tried to match the existing EFI variable structure: `efi_variable.c`
> (Done by Heinrich Schuchardt) already has separate boot-time and runtime
> service entry points, so I kept the same split here instead of introducing a
> different pattern only for the TEE/FF-A backend.
>
> Apart from these runtime variants, I will collapse the other `_runtime` helpers
> into the common implementations in v2 patchset. `get_mm_comms()`, `mm_communicate()`,
> `ffa_mm_communicate()`, and `ffa_notify_mm_sp()` will handle both boot and runtime,
> with only a small `efi_at_runtime()` branch for the phase-specific parts.
>
> Does this explanation sound reasonable to you?
Yes that makes sense, thanks.
>
> Thanks
>
> > > diff --git a/lib/charset.c b/lib/charset.c
> > > @@ -407,7 +407,7 @@ size_t __efi_runtime u16_strnlen(const u16 *in, size_t count)
> > > -size_t u16_strsize(const void *in)
> > > +size_t __efi_runtime u16_strsize(const void *in)
> >
> > This change should really be in its own patch with a one-line note
> > that it's needed by the new runtime callers. It is unrelated to the
> > SetVariable/GetVariable wiring and is easy to lose in this 320-line
> > diff.
> >
>
> For v2, I'll split the `u16_strsize()` `__efi_runtime` annotation into a
> separate small patch with a short note that it is needed by
> the new runtime variable callers. That should keep this patch focused on
> the SetVariable/GetVariable runtime wiring.
>
> > > Key changes:
> > > ============
> >
> > >
> > > - Add runtime-safe variable property handling via FF-A MM SP.
> > > - Add runtime versions of variable access and property operations.
> > > - Replace all standard memory operations with EFI-runtime-safe variants.
> >
> > The'============' underline is more for rST documentation than commit
> > messages - please drop it. The third bullet is also misleading: only
> > the new runtime functions use the runtime-safe variants; the existing
> > boot-time helpers are unchanged. It is generally better to use prose
> > than bullet points in a commit message.
> >
>
> Sure, I will update this in v2.
Regards,
Simon
More information about the U-Boot
mailing list