[PATCH v9] efi_vars: Implement SPI Flash store

[Heinrich Schuchardt]

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.

> +	}
> +
> +	ret = efi_var_restore(buf, false);
> +	if (ret != EFI_SUCCESS)
> +		log_err("%s - Unable to restore EFI variables from buffer\n", __func__);
> +
> +	ret = EFI_SUCCESS;
> +error:
> +	free(buf);
> +	return ret;

We should not return a non-EFI return code here if uclass_get_device() 
or spi_flash_read_dm() failed.

> +}

I configured starfive_visionfive2_defconfig with

CONFIG_EFI_VARIABLE_SF_STORE=y
CONFIG_EFI_VARIABLE_SF_OFFSET=0xc0000
CONFIG_EFI_VARIABLE_SF_DEVICE_INDEX=0
CONFIG_EFI_VAR_BUF_SIZE=131072

and it works.

Best regards

Heinrich