[PATCH v3 2/3] efi_vars: Implement SPI Flash store

[Ilias Apalodimas]

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
>