Hi Heinrich [...]
> > > > +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 > > @@ -153,7 +171,7 @@ endchoice > > > > 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 > > Hello Michal, > > If the backend store is SPI flash, we should not publish the variable > "RTStorageVolatile" at runtime as currently defined. We had this discussion on a previous version. I asked Michal to either update the commit message with instructions on how to update the SPI flash, or remove the EFI_RT_VOLATILE_STORE from SPI flashes. Technically writing these from a bash script would work, although I agree that ideally we want this into efivar. I don't mind if we currently prohibit EFI_RT_VOLATILE_STORE from SPI > > For the background see this commit for the efivar library: > > https://github.com/rhboot/efivar/commit/68daa04654acbe1bbaa17ebfc23c371b39e69c6b > > The first three patches look correct to me and I will add them to a > merge request for efi-next. > > The usage of efi_var_mem_ins() and efi_var_mem_del() when SetVariable() > does not change anything is not flash-friendly. Michal, do you want to > have a look at this area? > > @Ilias, @Vincent: > We currently lack documentation for the 3 variables. > Would it make sense to add the definition of the variables to chapter > source/chapter5-variable-storage.rst. I'll send a patch to update the documentation and we can discuss it. Regards /Ilias > > Would an efivar helper for SPI flash make sense? > How should RTStorageVolatile describe a SPI flash location? > > Best regards > > Heinrich > > > 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 > > @@ -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); > > + > > + if (ret != EFI_SUCCESS) > > + goto error; > > + > > + ret = uclass_get_device(UCLASS_SPI_FLASH, > > CONFIG_EFI_VARIABLE_SF_DEVICE_INDEX, &sfdev); > > + if (ret) > > + 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); > > + if (ret) > > + 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); > > + > > + if (ret) > > + 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__); > > + return EFI_OUT_OF_RESOURCES; > > + } > > + > > + ret = uclass_get_device(UCLASS_SPI_FLASH, 0, &sfdev); > > + if (ret) > > + 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 (ret || buf->length < sizeof(struct efi_var_file)) { > > + log_err("%s - buffer read from SPI Flash isn't valid\n", > > __func__); > > + goto error; > > + } > > + > > + 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; > > +} >
