Hi Ilias and Akashi, On Wed, Nov 22, 2023 at 6:58 AM Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > Hi Shahtur > > On Wed, 22 Nov 2023 at 01:58, Shantur Rathore <i...@shantur.com> wrote: > > > > 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...@shantur.com> > > --- > > > > include/efi_variable.h | 25 ++++++++++ > > lib/efi_loader/Kconfig | 18 +++++++ > > lib/efi_loader/Makefile | 1 + > > lib/efi_loader/efi_var_sf.c | 91 +++++++++++++++++++++++++++++++++++ > > lib/efi_loader/efi_variable.c | 4 ++ > > 5 files changed, 139 insertions(+) > > create mode 100644 lib/efi_loader/efi_var_sf.c > > > > diff --git a/include/efi_variable.h b/include/efi_variable.h > > index ca7e19d514..766dd109f5 100644 > > --- a/include/efi_variable.h > > +++ b/include/efi_variable.h > > @@ -188,6 +188,31 @@ efi_status_t efi_var_from_file(void); > > > > #endif // CONFIG_EFI_VARIABLE_FILE_STORE > > > > +#ifdef CONFIG_EFI_VARIABLE_SF_STORE > > + > > +/** > > + * efi_var_from_sf() - read variables from SPI Flash > > + * > > + * EFI variable buffer is read from SPI Flash at offset defined with > > + * CONFIG_EFI_VARIABLE_SF_OFFSET of length CONFIG_EFI_VAR_BUF_SIZE > > + * > > + * > > + * Return: status code > > + */ > > +efi_status_t efi_var_from_sf(void); > > + > > +/** > > + * efi_var_to_sf() - save non-volatile variables to SPI Flash > > + * > > + * EFI variable buffer is saved to SPI Flash at offset defined with > > + * CONFIG_EFI_VARIABLE_SF_OFFSET of length CONFIG_EFI_VAR_BUF_SIZE. > > + * > > + * Return: status code > > + */ > > +efi_status_t efi_var_to_sf(void); > > + > > +#endif // CONFIG_EFI_VARIABLE_SF_STORE > > + > > /** > > * efi_var_mem_init() - set-up variable list > > * > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index 4ccd26f94a..1fcc6fabb4 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -54,6 +54,17 @@ 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. > > + > > + > > config EFI_MM_COMM_TEE > > bool "UEFI variables storage service via the trusted world" > > depends on OPTEE > > @@ -108,6 +119,13 @@ config EFI_VARIABLE_NO_STORE > > > > endchoice > > > > +config EFI_VARIABLE_SF_OFFSET > > + hex "EFI variables in SPI flash offset" > > + depends on EFI_VARIABLE_SF_STORE > > + default 0x7D0000 > > + help > > + Offset from the start of the SPI Flash where EFI variables will > > be stored. > > + > > config EFI_VARIABLES_PRESEED > > bool "Initial values for UEFI variables" > > depends on !EFI_MM_COMM_TEE > > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile > > index 8d31fc61c6..b9b715b1ff 100644 > > --- a/lib/efi_loader/Makefile > > +++ b/lib/efi_loader/Makefile > > @@ -67,6 +67,7 @@ obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += > > efi_unicode_collation.o > > obj-y += efi_var_common.o > > obj-y += efi_var_mem.o > > obj-y += efi_var_file.o > > +obj-$(CONFIG_EFI_VARIABLE_SF_STORE) += efi_var_sf.o > > ifeq ($(CONFIG_EFI_MM_COMM_TEE),y) > > obj-y += efi_variable_tee.o > > else > > diff --git a/lib/efi_loader/efi_var_sf.c b/lib/efi_loader/efi_var_sf.c > > new file mode 100644 > > index 0000000000..e604a2225c > > --- /dev/null > > +++ b/lib/efi_loader/efi_var_sf.c > > @@ -0,0 +1,91 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * File interface for UEFI variables > > + * > > + * Copyright (c) 2023, Shantur Rtahore > > Name is misspelled >
Fixed, thanks. I shouldn't be sending patches 2 minutes before midnight > > + */ > > + > > +#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_sf(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 SF size"); > > + ret = EFI_BAD_BUFFER_SIZE; > > The UEFI spec doesn't define this return code for SetVariable. > Instead, EFI_OUT_OF_RESOURCES is used when the available storage isn't > enough to hold the variable and its data. > > > + goto error; > > + } > > + > > + 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, 0, &sfdev); > > This is a bit weird. Do we always want to use the first available SPI > on the system to store the variables? > I don't remember if the driver model has a mechanism to define one of > the SPI flashes, but if it doesn't perhaps we should use a Kconfig for > it? Taken care of in V2. Thanks. > > > + if (ret) > > + goto error; > > + > > + ret = spi_flash_erase_dm(sfdev, CONFIG_EFI_VARIABLE_SF_OFFSET, > > EFI_VAR_BUF_SIZE); > > + debug("%s - Erased SPI Flash offset %lx\n", __func__, > > CONFIG_EFI_VARIABLE_SF_OFFSET); > > + if (ret) > > + goto error; > > + > > + ret = spi_flash_write_dm(sfdev, CONFIG_EFI_VARIABLE_SF_OFFSET, len, > > buf); > > + 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_sf(void) > > +{ > > + struct efi_var_file *buf; > > + efi_status_t ret; > > + struct udevice *sfdev; > > + > > + buf = calloc(1, EFI_VAR_BUF_SIZE); > > + if (!buf) { > > + log_err("Out of memory\n"); > > + return EFI_OUT_OF_RESOURCES; > > + } > > You don't want normal memory for the variables. Instead, you want EFI > memory which is marked for EFI runtime services data. U-Boot already > has code to support Get/GetNextvariableRT, you just need to use > efi_var_mem_init(). > If you need hints have a look at lib/efi_loader/efi_variable_tee.c > which implements variables stored in an RPMB, but still uses the > memory backend for runtime services. > I believe EFI Runtime stuff is handled by efi_variable.c functions. This code is run while we are still in U-boot to initialise the memory backed. As of now, we don't have the SetVariableRT so I guess this won't be a problem. I took the code from efi_var_file.c and just replaced file storage with SPI Flash. I will have to look deeper once I get to implement SetVariableRT which will be a separate patch series once I get this in. I have submitted V2 of the patch, can you kindly review them. Kind regards, Shantur