Hello Harsiman
[...]
> tus_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()
Apologies, I am going to backtrack on my previous response. Looking at it
again, I prefer a different approach. What I proposed is just going to make
us define duplicate runtimes in efi_variable.c and efi_variable_tee.c
Can we instead do this. Don't change efi_var_common.c Those are functions
that are currenlty used across file, SPI and OP-TEE implementations.
Instead define the runtime version you want in efi_variable_tee.c and
assign it efi_variables_boot_exit_notify(). You can then leave the optee
case unchanged, calling the memory backend and your own FF-A versions if
compiled with that.
Cheers
/Ilias
>
> 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
> > >
> >
>
>
>
