[PATCH v9] efi_vars: Implement SPI Flash store

[Michal Simek]

On 3/12/26 08:50, Heinrich Schuchardt wrote:
> On 3/11/26 16:08, Heinrich Schuchardt wrote:
>> On 2/16/26 14:17, Michal Simek wrote:
>>> From: Shantur Rathore <i at shantur.com>
>>>
>>> Currently U-Boot uses ESP as storage for EFI variables.
>>> Devices with SPI Flash are used for storing environment with this
>>> commit we allow EFI variables to be stored on SPI Flash.
>>>
>>> Signed-off-by: Shantur Rathore <i at shantur.com>
>>> Signed-off-by: Michal Simek <michal.simek at amd.com>
>>> Tested-by: Neil Armstrong <neil.armstrong at linaro.org> # on AML-S905D3-CC
>>> Acked-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>>>
>>> ---
>>>
>>> Changes in v9:
>>> - Remove EFI_RT_VOLATILE_STORE enabling
>>> - Cleanup commit message
>>>
>>> Changes in v8:
>>> - Add missing EFI_VARIABLE_SF_STORE dependency to EFI_VARIABLE_SF_DEVICE_INDEX
>>>
>>> Changes in v7:
>>> - sed -i 's/efi_var_from/efi_var_from_storage/g'
>>>
>>> Changes in v6:
>>> - sed -i 's/efi_var_read/efi_var_from/g'
>>> - sed -i 's/efi_var_write/efi_var_to_storage/g'
>>>
>>> Changes in v4:
>>> - Extend Kconfig description
>>> - Extend commit message and describe efivar missing part
>>> - use unify methods for reading/writing variable
>>>
>>> Changes in v3:
>>> - Fixed compiler warnings.
>>>
>>> Changes in v2:
>>> - Refactored efi_var_file to move common parts out as requested
>>> - Changed ifdefs to use CONFIG_IS_DEFINED
>>> - Fixed typos
>>>
>>>   lib/efi_loader/Kconfig      | 33 +++++++++++++
>>>   lib/efi_loader/Makefile     |  1 +
>>>   lib/efi_loader/efi_var_sf.c | 92 +++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 126 insertions(+)
>>>   create mode 100644 lib/efi_loader/efi_var_sf.c
>>>
>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>>> index 579eed658801..315dd4f3e794 100644
>>> --- a/lib/efi_loader/Kconfig
>>> +++ b/lib/efi_loader/Kconfig
>>> @@ -124,6 +124,24 @@ config EFI_VARIABLE_FILE_STORE
>>>         Select this option if you want non-volatile UEFI variables to be
>>>         stored as file /ubootefi.var on the EFI system partition.
>>> +config EFI_VARIABLE_SF_STORE
>>> +    bool "Store non-volatile UEFI variables in SPI Flash"
>>> +    depends on SPI_FLASH
>>> +    help
>>> +      Select this option if you want non-volatile UEFI variables to be
>>> +      stored in SPI Flash.
>>> +
>>> +      Define CONFIG_EFI_VARIABLE_SF_OFFSET as offset in SPI Flash to use as
>>> +      the storage for variables. CONFIG_EFI_VAR_BUF_SIZE defines the space
>>> +      needed.
>>> +
>>> +      Note that SPI Flash devices have a limited number of program/erase
>>> +      cycles. Frequent updates to UEFI variables may cause excessive wear
>>> +      and can permanently damage the flash device, particularly on SPI NAND
>>> +      or low-end SPI NOR parts without wear leveling. This option should be
>>> +      used with care on such systems, and is not recommended for platforms
>>> +      where UEFI variables are updated frequently.
>>> +
>>>   config EFI_MM_COMM_TEE
>>>       bool "UEFI variables storage service via the trusted world"
>>>       depends on OPTEE
>>> @@ -194,6 +212,21 @@ config FFA_SHARED_MM_BUF_ADDR
>>>         the MM SP in secure world.
>>>         It is assumed that the MM SP knows the address of the shared MM 
>>> communication buffer.
>>> +config EFI_VARIABLE_SF_OFFSET
>>> +    hex "EFI variables in SPI flash offset"
>>> +    depends on EFI_VARIABLE_SF_STORE
>>> +    help
>>> +      Offset from the start of the SPI Flash where EFI variables will be 
>>> stored.
>>> +      This should be aligned to the sector size of SPI Flash.
>>> +
>>> +config EFI_VARIABLE_SF_DEVICE_INDEX
>>> +    int "Device Index for target SPI Flash"
>>> +    depends on EFI_VARIABLE_SF_STORE
>>> +    default 0
>>> +    help
>>> +      The index of SPI Flash device used for storing EFI variables. This 
>>> would be
>>> +      needed if there are more than 1 SPI Flash devices available to use.
>>> +
>>>   config EFI_VARIABLES_PRESEED
>>>       bool "Initial values for UEFI variables"
>>>       depends on !COMPILE_TEST
>>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>>> index ca1775eb03be..d73ad43951b1 100644
>>> --- a/lib/efi_loader/Makefile
>>> +++ b/lib/efi_loader/Makefile
>>> @@ -54,6 +54,7 @@ obj-y += efi_variable_tee.o
>>>   else
>>>   obj-y += efi_variable.o
>>>   obj-$(CONFIG_EFI_VARIABLE_FILE_STORE) += efi_var_file.o
>>> +obj-$(CONFIG_EFI_VARIABLE_SF_STORE) += efi_var_sf.o
>>>   obj-$(CONFIG_EFI_VARIABLES_PRESEED) += efi_var_seed.o
>>>   endif
>>>   obj-y += efi_watchdog.o
>>> diff --git a/lib/efi_loader/efi_var_sf.c b/lib/efi_loader/efi_var_sf.c
>>> new file mode 100644
>>> index 000000000000..61d68f7c5c94
>>> --- /dev/null
>>> +++ b/lib/efi_loader/efi_var_sf.c
>>> @@ -0,0 +1,92 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * SPI Flash interface for UEFI variables
>>> + *
>>> + * Copyright (c) 2023, Shantur Rathore
>>> + * Copyright (C) 2026, Advanced Micro Devices, Inc.
>>> + */
>>> +
>>> +#define LOG_CATEGORY LOGC_EFI
>>> +
>>> +#include <efi_loader.h>
>>> +#include <efi_variable.h>
>>> +#include <spi_flash.h>
>>> +#include <dm.h>
>>> +
>>> +efi_status_t efi_var_to_storage(void)
>>> +{
>>> +    efi_status_t ret;
>>> +    struct efi_var_file *buf;
>>> +    loff_t len;
>>> +    struct udevice *sfdev;
>>> +
>>> +    ret = efi_var_collect(&buf, &len, EFI_VARIABLE_NON_VOLATILE);
>>> +    if (len > EFI_VAR_BUF_SIZE) {
>>> +        log_err("EFI var buffer length more than target SPI Flash size");
>>> +        ret = EFI_OUT_OF_RESOURCES;
>>> +        goto error;
>>> +    }
>>> +
>>> +    log_debug("%s - Got buffer to write buf->len : %d\n", __func__, buf- 
>>> >length);
>>
>> Hello Michal,
>>
>> Thank you for driving this.
>>
>> The log system is configurable to print file, line, function name if wanted. 
>> We should not duplicate this here. Please, remove __func__ throughout the patch.
>>
>>> +
>>> +    if (ret != EFI_SUCCESS)
>>> +        goto error;
>>> +
>>> +    ret = uclass_get_device(UCLASS_SPI_FLASH, 
>>> CONFIG_EFI_VARIABLE_SF_DEVICE_INDEX, &sfdev);
>>> +    if (ret)
>>
>> This is not an EFI return code. EFI_DEVICE_ERROR should be used.
>>
>>> +        goto error;
>>> +
>>> +    ret = spi_flash_erase_dm(sfdev, CONFIG_EFI_VARIABLE_SF_OFFSET, 
>>> EFI_VAR_BUF_SIZE);
>>> +    log_debug("%s - Erased SPI Flash offset %x\n", __func__, 
>>> CONFIG_EFI_VARIABLE_SF_OFFSET);
>>
>> A debug output if erasing failed would be more interesting.
>>
>>> +    if (ret)
>>
>> This is not an EFI return code. EFI_DEVICE_ERROR should be used.
>>
>>> +        goto error;
>>> +
>>> +    ret = spi_flash_write_dm(sfdev, CONFIG_EFI_VARIABLE_SF_OFFSET, len, buf);
>>> +    log_debug("%s - Wrote buffer to SPI Flash : %ld\n", __func__, ret);
>>
>> The uclass_get_device() return code should be saved in an int variable to 
>> avoid confusion.
>>
>>> +
>>> +    if (ret)
>>
>> This is not an EFI return code. EFI_DEVICE_ERROR should be used.
>>
>>> +        goto error;
>>> +
>>> +    ret = EFI_SUCCESS;
>>> +error:
>>> +    if (ret)
>>> +        log_err("Failed to persist EFI variables in SF\n");
>>> +    free(buf);
>>> +    return ret;
>>> +}
>>> +
>>> +efi_status_t efi_var_from_storage(void)
>>> +{
>>> +    struct efi_var_file *buf;
>>> +    efi_status_t ret;
>>> +    struct udevice *sfdev;
>>> +
>>> +    buf = calloc(1, EFI_VAR_BUF_SIZE);
>>> +    if (!buf) {
>>> +        log_err("%s - Unable to allocate buffer\n", __func__);
>>
>> This function may be called from an EFI GUI application. We should not output 
>> any strings to the console from API functions. It is task of the caller to 
>> take appropriate actions. E.g. show a pop-up if a failure occurs. log_debug() 
>> is ok.
> 
> I got this wrong. This function is called when initializing the EFI sub-system. 
> So writing errors to the console is ok. It is efi_var_to_storage() which is 
> invoked by EFI applications and where we should avoid console output.

I don't think it is a problem to keep it under debug. Error value will be 
propagated properly.

M