Hi Shantur
I have a few remarks on the architecture.
Up to now, we are supporting
1. Variables on a file
2. Variables on an RPMB
The reason those two are in different files is that we generally
expect to use different bootime services and few differences in
efi_variables_boot_exit_notify() and efi_init_variables().
Whatever is common, for example the runtime functions are common
across those two since they both implement variable runtime service
via the memory backend, goes into efi_var_common.c, the memory backend
operations should go in efi_var_mem.c.
Since the SPI and file storage are similar -- and probably any storage
medium controlled by the non-secure world, I am fine treating treat
efi_variable.c as the variable management for the non-secure world and
see if that needs changing in the future.
As Heinrich pointed out the functions you are moving are better off
moved into efi_var_mem.c.
[...]
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index adc5ac6a80..f73fb40061 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -354,14 +354,16 @@ efi_status_t efi_set_variable_int(const u16
> *variable_name,
> ret = EFI_SUCCESS;
>
> /*
> - * Write non-volatile EFI variables to file
> + * Write non-volatile EFI variables to file or SPI Flash
> * TODO: check if a value change has occured to avoid superfluous
> writes
> */
> -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> if (attributes & EFI_VARIABLE_NON_VOLATILE) {
> +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> efi_var_to_file();
> - }
> +#elif CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
> + efi_var_to_sf();
> #endif
> + }
>
> return EFI_SUCCESS;
> }
> @@ -471,9 +473,14 @@ efi_status_t efi_init_variables(void)
>
> #if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> ret = efi_var_from_file();
> +#elif CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
> + ret = efi_var_from_sf();
> +#else
> + ret = EFI_SUCCESS;
> +#endif
Instead of those ifdefs can we do something different?
Define a structure with two function callbacks for read/write(). Add
the same function in both efi_var_file.c and efi_var_sp.c which fills
in those callbacks and have an efi_init_variable() call to that
function (obviously the SPI and file will be mutually exclusive)
Thanks
/Ilias
> +
> if (ret != EFI_SUCCESS)
> return ret;
> -#endif
>
> if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
> ret = efi_var_restore((struct efi_var_file *)
> --
> 2.40.1
>
