[PATCH v9] efi_vars: Implement SPI Flash store
Heinrich Schuchardt
xypron.glpk at gmx.de
Thu Mar 12 09:00:37 CET 2026
On 3/11/26 17:26, Michal Simek 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.
>>
>>> + 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
See this patch:
https://lore.kernel.org/u-boot/20260311173033.10002-1-heinrich.schuchardt@canonical.com/T/#u
Best regards
Heinrich
More information about the U-Boot
mailing list