[PATCH] efi_loader: optional persistence of variables

[Tom Saeger]

On Tue, Mar 22, 2022 at 06:03:28PM -0500, Tom Saeger wrote:
> On Tue, Mar 22, 2022 at 11:33:20PM +0100, Heinrich Schuchardt wrote:
> > On 3/22/22 23:16, Tom Saeger wrote:
> > > On Tue, Mar 22, 2022 at 10:41:40PM +0100, Heinrich Schuchardt wrote:
> > > > On 3/22/22 22:21, Tom Saeger wrote:
> > > > > Since be66b89da306 ("efi_loader: configuration of variables store")
> > > > > the choice of EFI_VARIABLE_FILE_STORE or EFI_MM_COMM_TEE
> > > > > is mutually-exclusive, however efi_var_to_file also allows
> > > > > for "neither".  Set Kconfig choice optional.
> > > > > 
> > > > > Signed-off-by: Tom Saeger <tom.saeger at oracle.com>
> > > > > ---
> > > > >    lib/efi_loader/Kconfig | 1 +
> > > > >    1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > > > index e5e35fe51f65..9add2a286ff4 100644
> > > > > --- a/lib/efi_loader/Kconfig
> > > > > +++ b/lib/efi_loader/Kconfig
> > > > > @@ -44,6 +44,7 @@ config EFI_SETUP_EARLY
> > > > > 
> > > > >    choice
> > > > >    	prompt "Store for non-volatile UEFI variables"
> > > > > +	optional
> > > > 
> > > > Storing non-volatile variables is required by the UEFI specification.
> > > > 
> > > > How should a user understand that a boot option he just created vanishes
> > > > upon reboot?
> > > > 
> > > > Please, explain your use case.
> > > 
> > > bootefi ${kernel_addr}
> > > 
> > > in this case linux kernel for an Armv8 platform.
> > > This platform does not want or need any variables to persist.
> > > 
> > > bootefi eventually calls efi_var_from_file(), where *NOT* defining
> > > CONFIG_EFI_VARIABLE_FILE_STORE would allow this use case to work,
> > > otherwise it fails.  This was possible before
> > > be66b89da306 ("efi_loader: configuration of variables store").
> > 
> > Please, describe exactly what does not work. Is it building or is it
> > running the bootefi command?
> 
> after be66b89da306 CONFIG_EFI_VARIABLE_FILE_STORE is now selected due to
> Kconfig choice.  This is the difference.
> 
> It fails while running, but the difference is the build.
> I'd rather configure for the old behavior - that being return
> EFI_SUCCESS from efi_var_from_file() when CONFIG_EFI_VARIABLE_FILE_STORE
> isn't defined.
> 
> The call-stack is something like:
> 
> do_bootefi->efi_init_obj_list->efi_init_variables->efi_var_from_file

I think perhaps I stumbled upon an unsupported mode, which the prior
Kconfig unintentionally allowed.

If so - this patch doesn't make sense.

It was however convenient for our application.
(NO ESP, NO persistence)

> 
> > 
> > Does your system have an ESP?

I'll look into this - perhaps it should, despite strong desire to
disallow changes to it.


> > Where does bootefi fail?

When CONFIG_EFI_VARIABLE_FILE_STORE is defined, runtime complains (as it
should)

"Failed to persist EFI variables"

> > Best regards
> > 
> > Heinrich
> > 

Appreciate all your feedback,

--Tom

> > > 
> > > Thoughts?
> > > 
> > > --Tom
> > > 
> > > > 
> > > > Best regards
> > > > 
> > > > Heinrich
> > > > 
> > > > >    	default EFI_VARIABLE_FILE_STORE
> > > > >    	help
> > > > >    	  Select where non-volatile UEFI variables shall be stored.
> > > > > 
> > > > > base-commit: 5f68470d69f853b1652ebe93525b60064717fe2e
> > > > 
> >