Re: [PATCH v1 2/3] efi_vars: Implement SPI Flash store
On Mon, 27 Nov 2023 at 11:31, Shantur Rathore wrote:
>
> Hi Ilias,
>
> On Mon, Nov 27, 2023 at 7:09 AM Ilias Apalodimas
> wrote:
> >
> > Hi Shantur
> >
> > Please don't send a v2 unless the v1 discussion has settled. It just
> > makes life harder. I'll ignore v2 for now and respond here.
> >
>
> Sure, I'm still learning the ways around here.
No worries -- we've all been there.
>
> > [...]
> >
> > >
> > > >
> > > > > + 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.
> >
> > Yes, they are, but there is a subtle difference here.
> > efi_variable.c and efi_variable_tee.c have their own version of
> > runtime service implementation. The functions that can be shared
> > across the two are in efi_var_common.c.
> > What your patch does is shoehorn an SPI backend storage to the
> > existing implementation for storing variables to a file. Depending on
> > what we decide for runtime calls that can end up being messy.
> >
> > efi_variable.c -> deals with variables on a file
> > efi_variable_tee.c -> deals with variables hidden behind the secure world
> >
> > I think it would be cleaner to add efi_variable_sf.c and move the
> > common functions you need from efi_variable.c -> efi_var_common.c
> > Heinrich, do you have a preference for that?
>
> I agree, the v1 patch was not implementing it cleanly.
> Based on Akashi's comments I made these changes for v2 ( v3 with some
> compiler warning fixes )
>
> - Moved common stuff out of efi_var_file.c to efi_var_common.c
> - Added efi_var_sf.c to handle writing vars to SPI Flash as
> efi_var_file.c does for File
>
> Happy to change things if needed.
Great, I'll have a look at v3 then
Thanks
/Ilias
>
> Kind regards,
> Shantur
Re: [PATCH v1 2/3] efi_vars: Implement SPI Flash store
Hi Ilias,
On Mon, Nov 27, 2023 at 7:09 AM Ilias Apalodimas
wrote:
>
> Hi Shantur
>
> Please don't send a v2 unless the v1 discussion has settled. It just
> makes life harder. I'll ignore v2 for now and respond here.
>
Sure, I'm still learning the ways around here.
> [...]
>
> >
> > >
> > > > + 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.
>
> Yes, they are, but there is a subtle difference here.
> efi_variable.c and efi_variable_tee.c have their own version of
> runtime service implementation. The functions that can be shared
> across the two are in efi_var_common.c.
> What your patch does is shoehorn an SPI backend storage to the
> existing implementation for storing variables to a file. Depending on
> what we decide for runtime calls that can end up being messy.
>
> efi_variable.c -> deals with variables on a file
> efi_variable_tee.c -> deals with variables hidden behind the secure world
>
> I think it would be cleaner to add efi_variable_sf.c and move the
> common functions you need from efi_variable.c -> efi_var_common.c
> Heinrich, do you have a preference for that?
I agree, the v1 patch was not implementing it cleanly.
Based on Akashi's comments I made these changes for v2 ( v3 with some
compiler warning fixes )
- Moved common stuff out of efi_var_file.c to efi_var_common.c
- Added efi_var_sf.c to handle writing vars to SPI Flash as
efi_var_file.c does for File
Happy to change things if needed.
Kind regards,
Shantur
Re: [PATCH v1 2/3] efi_vars: Implement SPI Flash store
Hi Shantur
Please don't send a v2 unless the v1 discussion has settled. It just
makes life harder. I'll ignore v2 for now and respond here.
[...]
>
> >
> > > + 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.
Yes, they are, but there is a subtle difference here.
efi_variable.c and efi_variable_tee.c have their own version of
runtime service implementation. The functions that can be shared
across the two are in efi_var_common.c.
What your patch does is shoehorn an SPI backend storage to the
existing implementation for storing variables to a file. Depending on
what we decide for runtime calls that can end up being messy.
efi_variable.c -> deals with variables on a file
efi_variable_tee.c -> deals with variables hidden behind the secure world
I think it would be cleaner to add efi_variable_sf.c and move the
common functions you need from efi_variable.c -> efi_var_common.c
Heinrich, do you have a preference for that?
Thanks
/Ilias
> 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
Re: [PATCH v1 2/3] efi_vars: Implement SPI Flash store
Hi Ilias and Akashi,
On Wed, Nov 22, 2023 at 6:58 AM Ilias Apalodimas
wrote:
>
> Hi Shahtur
>
> On Wed, 22 Nov 2023 at 01:58, Shantur Rathore 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
> > ---
> >
> > 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 0x7D
> > + 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 00..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
> > +#include
> > +#include
> > +#include
> > +
> > +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(, , 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.
>
Re: [PATCH v1 2/3] efi_vars: Implement SPI Flash store
Hi Shahtur
On Wed, 22 Nov 2023 at 01:58, Shantur Rathore 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
> ---
>
> 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 0x7D
> + 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 00..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
> + */
> +
> +#define LOG_CATEGORY LOGC_EFI
> +
> +#include
> +#include
> +#include
> +#include
> +
> +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(, , 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, );
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
Re: [PATCH v1 2/3] efi_vars: Implement SPI Flash store
On Tue, Nov 21, 2023 at 11:57:12PM +, Shantur Rathore 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
> ---
>
> 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
Not needed as I said.
> +
> +/**
> + * 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 0x7D
The default value is definitely board-specific.
Please add "if TARGET_ROCKPRO64_RK3399(?)".
> + 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 00..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
For Spi Flash?
> + *
> + * Copyright (c) 2023, Shantur Rtahore
> + */
> +
> +#define LOG_CATEGORY LOGC_EFI
> +
> +#include
> +#include
> +#include
> +#include
> +
> +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(, , 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;
> + goto error;
> + }
> +
> + debug("%s - Got buffer to write buf->len : %d\n", __func__,
> buf->length);
log_debug()?
> +
> + if (ret != EFI_SUCCESS)
> + goto error;
> +
> + ret = uclass_get_device(UCLASS_SPI_FLASH, 0, );
> + 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;
> +
> +
[PATCH v1 2/3] efi_vars: Implement SPI Flash store
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
---
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 0x7D
+ 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 00..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
+ */
+
+#define LOG_CATEGORY LOGC_EFI
+
+#include
+#include
+#include
+#include
+
+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(, , 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;
+ 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, );
+ 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;
+
