[PATCH v8 4/4] efi_vars: Implement SPI Flash store

[Ilias Apalodimas]

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;
> > +}
>