[PATCH v4 3/3] efi_vars: Implement SPI Flash store

[Michal Simek]

On 1/23/26 08:40, Ilias Apalodimas wrote:
> On Thu, 22 Jan 2026 at 14:32, Michal Simek <michal.simek at amd.com> 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.

Correct June, 2024
https://github.com/rhboot/efivar/pull/267


>>
>>
>>> 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.

I don't think it is going to be problem too.

What about?
dd if=/sys/firmware/efi/efivars/VarToFile-b2ac5fc9-92b7-4acd-aeac-11e818c3130c \
    of=/tmp/vars skip=4 bs=1
flashcp /tmp/vars /dev/mtdX

> 
>>
>>>
>>> [...]
>>>
>>>> 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.

ok.

Thanks,
Michal