On Thu, 22 Jan 2026 at 14:32, Michal Simek <[email protected]> wrote:
>
> Hi,
>
> On 1/22/26 13:18, Ilias Apalodimas wrote:
> > Hi Michal,
> >
> > [...]
> >
> >>
> >> config EFI_RT_VOLATILE_STORE
> >> bool "Allow variable runtime services in volatile storage (e.g
> >> RAM)"
> >> - depends on EFI_VARIABLE_FILE_STORE
> >> + depends on EFI_VARIABLE_FILE_STORE || EFI_VARIABLE_SF_STORE
> >
> > Will this work on nands as well? They got a much shrter lifetime that
> > spi flashes.
>
> I don't think this is valid argument here. Users have options to decide where
> to
> put them. The same argument can be used for u-boot variables saved in NAND.
> Obviously I can update Kconfig option with saying that you should be careful
> about it but it is user choice.
Yes I think that's a good idea.
>
> >
> >> help
> >> When EFI variables are stored on file we don't allow
> >> SetVariableRT,
> >> since the OS doesn't know how to write that file. At the same
> >> time
> >
> > I am not sure we need to allow this for now. At least not until we've
> > talked to the efitool maintainers and make sure they will accept
> > another 'special' case.
>
> Not a problem for me to disable it for now.
>
> >
> > The problem with allowing this is that if people enable it, boot a
> > linux and do a setvariable, it will return a success. But none of the
> > variables will be preserved after a reboot unless someone manually
> > updates the serial flash contents. In theory, we can preserve the
> > driver model and the spi drivers in EFI runtime services and allow
> > 'proper' setvariable at runtime.
>
> Wasn't this the same case for file based one? I mean u-boot had it but efi
>
> Yours efivar patch was merged Jun 23, 2025
> But this symbol was introduce in U-Boot April 18, 2024.
Yes but the patches were sent a long time ago in efivar. If you look
at the original PR date it was a lot eariler [0]. It just took a long
time to discuss and merge.
Apart from that the case for file stored variables is by itself pretty
unique and convincing the maintainers to accept the patch had some
basis. I don't know if they want the same workaround for spi flashes.
>
>
> > However, I think this is not very useful. Having an unprotected to
> > store authenticated EFI variables, is dangerous. Someone can erase the
> > SPI flash and efectively disable secure boot. Due to that, I prefer
> > the current file based approach for EFI variables -- which doesn't
> > store/restore authenticated EFI variables (and which this patch
> > implements). The obvious downside is that enable setvariable at
> > runtime is tricky once again....
>
> Isn't the same happening with file store too?
>
> From implementation perspective it should be the same as file based.
> Obviously
> efivar part is missing.
Yes. However, that commit included instructions on how to set runtime
variables with dd, so people could use it until the efivar changes
were merged.
>
> >
> > [...]
> >
> >> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> >> index be670a8e7c25..feab212b245b 100644
> >> --- a/lib/efi_loader/efi_variable.c
> >> +++ b/lib/efi_loader/efi_variable.c
> >> @@ -397,11 +397,11 @@ 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 (attributes & EFI_VARIABLE_NON_VOLATILE) {
> >> -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> >> +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) ||
> >> CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
> >
> > Do we need the ifdefery here? efi_variable.o is basically compiled
> > whenever we have the variables managed by the non-secure world and
> > this function will exist either with a file back storage or a SPI
> > flash
>
> It can be reverted and return EFI_NOT_READY for EFI_VARIABLE_NO_STORE.
Yes I *thinkk* that will look cleaner.
>
[0] https://github.com/rhboot/efivar/pull/267
Cheers
/Ilias
> Thanks,
> Michal
