[PATCH v9] efi_vars: Implement SPI Flash store
Michal Simek
michal.simek at amd.com
Wed Mar 11 17:26:50 CET 2026
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.
>
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> +
>> + ret = uclass_get_device(UCLASS_SPI_FLASH, 0, &sfdev);
>
> efi_var_restore() returns a signed int and not an unsigned efi_status_t. Please,
> define a separate variable.
>
>> + if (ret)
>
> Please, return EFI_DEVICE_ERROR.
>
>> + goto error;
>> +
>> + ret = spi_flash_read_dm(sfdev, CONFIG_EFI_VARIABLE_SF_OFFSET,
>> + EFI_VAR_BUF_SIZE, buf);
>> +
>> + log_debug("%s - read buffer buf->length: %x\n", __func__, buf->length);
>
> If spi_flash_read_dm() failed, you will just print some random value for buf-
> >length. Please, print the return value instead.
>
>> +
>> + if (ret || buf->length < sizeof(struct efi_var_file)) {
>> + log_err("%s - buffer read from SPI Flash isn't valid\n", __func__);
>
> If spi_flash_read_dm() fails we should return EFI_DEVICE_ERROR.
>
>> + goto error;
>
> After erasing the SPI flash buf->length will be zero here. This should not stop
> us from setting EFI variables.
>
> It is the task of efi_var_restore to check the validity of the buffer. So the
> length check here should be removed.
>
> The missing check in efi_var_restore() is
>
> buf->length <= EFI_VAR_BUF_SIZE
>
> to avoid a buffer overrun when calculating the CRC32.
I am not sure I understand this part.
Where exactly should be this check?
Thanks,
Michal
More information about the U-Boot
mailing list