On 2026-06-05 14:04 +0300, Ilias Apalodimas wrote:
> Hi Harsiman
>
> On Thu, 14 May 2026 at 15:50, Harsimran Singh Tungal
> <[email protected]> wrote:
> >
> > Relocate the generic runtime variable read helpers from
> > efi_var_common.c to efi_variable.c.
> >
> > efi_var_common.c is always built, while efi_variable.c is only built
> > when CONFIG_EFI_MM_COMM_TEE is unset. Earlier patches in this series
> > add TEE/FF-A-specific runtime read helpers in efi_variable_tee.c.
> > Leaving the generic definitions in efi_var_common.c would therefore
> > cause a symbol clash in EFI_MM_COMM_TEE builds.
> >
> > Move efi_get_variable_runtime() and
> > efi_get_next_variable_name_runtime() into efi_variable.c so the
> > non-TEE backend keeps the generic memory-backed implementations,
> > while the TEE/FF-A backend continues to provide its own runtime read
> > helpers in efi_variable_tee.c.
> >
> > Signed-off-by: Harsimran Singh Tungal <[email protected]>
> >
> > ---
> >
> > Changelog:
> > ===============
> >
> > v2:
> >
> > Ilias, Simon:
> >
> > - Reword commit message to include relocation of generic non-TEE
> > helpers and broaden the wording to cover both runtime read helpers
> >
> > lib/efi_loader/efi_var_common.c | 24 ------------------------
> > lib/efi_loader/efi_variable.c | 24 ++++++++++++++++++++++++
> > 2 files changed, 24 insertions(+), 24 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_var_common.c
> > b/lib/efi_loader/efi_var_common.c
> > index d63c2d1b1cd..7cbf098c64a 100644
> > --- a/lib/efi_loader/efi_var_common.c
> > +++ b/lib/efi_loader/efi_var_common.c
> > @@ -173,30 +173,6 @@ efi_status_t EFIAPI efi_query_variable_info(
> > return EFI_EXIT(ret);
> > }
> >
> > -efi_status_t __efi_runtime EFIAPI
> > -efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *guid,
> > - u32 *attributes, efi_uintn_t *data_size, void
> > *data)
> > -{
> > - efi_status_t ret;
> > -
> > - ret = efi_get_variable_mem(variable_name, guid, attributes,
> > data_size,
> > - data, NULL, EFI_VARIABLE_RUNTIME_ACCESS);
> > -
> > - /* Remove EFI_VARIABLE_READ_ONLY flag */
> > - if (attributes)
> > - *attributes &= EFI_VARIABLE_MASK;
> > -
> > - return ret;
> > -}
> > -
>
> I think I mentioned this on the previous version as well.
> The reason that we represent variables in memory at runtime, is that
> it works for both secure and non secure world storage. In the secure
> world case, Linux scans that OP-TEE can support variables at runtime
> and swtiches get/setVariable to point in op-tee functions.
> Due to the above we were less strict wth the API, since evenryone just
> called the _mem variant for runtime calls and it saved us a function
> call.
>
> If we want to change this, the right fix would be to define _int
> variants in the efi_variable.c and efi_variable_tee.c and call them
> from efi_var_common.c. This basically boils down to calling the _int
> version in efi_get_variable_runtime and efi_get_next_variable_runtime
> on the common code.
>
> Thanks
> /Ilias
>
>
Hi Ilias,
Thanks, that makes sense.
Just to confirm the intended scope: should I apply this pattern only to
efi_get_variable_runtime() and efi_get_next_variable_name_runtime(), since
those are the helpers moved by this patch, or would you prefer the same
common-wrapper plus backend-specific _int pattern for all four runtime
variable service functions:
efi_get_variable_runtime()
efi_get_next_variable_name_runtime()
efi_set_variable_runtime()
efi_query_variable_info_runtime()
I will make relevant changes as per your feedback in v3.
Thanks
Regards
Harsimran Singh Tungal
> > -efi_status_t __efi_runtime EFIAPI
> > -efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
> > - u16 *variable_name, efi_guid_t *guid)
> > -{
> > - return efi_get_next_variable_name_mem(variable_name_size,
> > variable_name,
> > - guid,
> > EFI_VARIABLE_RUNTIME_ACCESS);
> > -}
> > -
> > /**
> > * efi_set_secure_state - modify secure boot state variables
> > * @secure_boot: value of SecureBoot
> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > index 9923936c1b5..f2e0e1ad4e2 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -579,6 +579,30 @@ efi_set_variable_runtime(u16 *variable_name, const
> > efi_guid_t *vendor,
> > return EFI_SUCCESS;
> > }
> >
> > +efi_status_t __efi_runtime EFIAPI
> > +efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *guid,
> > + u32 *attributes, efi_uintn_t *data_size, void
> > *data)
> > +{
> > + efi_status_t ret;
> > +
> > + ret = efi_get_variable_mem(variable_name, guid, attributes,
> > data_size,
> > + data, NULL, EFI_VARIABLE_RUNTIME_ACCESS);
> > +
> > + /* Remove EFI_VARIABLE_READ_ONLY flag */
> > + if (attributes)
> > + *attributes &= EFI_VARIABLE_MASK;
> > +
> > + return ret;
> > +}
> > +
> > +efi_status_t __efi_runtime EFIAPI
> > +efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
> > + u16 *variable_name, efi_guid_t *guid)
> > +{
> > + return efi_get_next_variable_name_mem(variable_name_size,
> > variable_name,
> > + guid,
> > EFI_VARIABLE_RUNTIME_ACCESS);
> > +}
> > +
> > /**
> > * efi_variables_boot_exit_notify() - notify ExitBootServices() is called
> > */
> > --
> > 2.34.1
> >
>
