[PATCH v4 0/3] efi_vars: Implement SPI Flash storage for EFI

[PATCH v4 0/3] efi_vars: Implement SPI Flash storage for EFI

[Michal Simek]

This is updated series based on v3 version sent here
https://lore.kernel.org/all/20231126220836.374956-1-i@shantur.com/

that's why I am continuing on v4 instead of starting from scratch.

Tested on kv260 with saving variables to location where User MTD partition
is.
CONFIG_EFI_VARIABLE_SF_STORE=y
CONFIG_EFI_RT_VOLATILE_STORE=y
CONFIG_EFI_VARIABLE_SF_OFFSET=0x22a0000

Thanks,
Michal

Changes in v4:
- New patch based on review comments from v3
- 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

Michal Simek (1):
  efi_var: Unify read/write access helper function

Shantur Rathore (2):
  efi_var_file: refactor to move buffer functions
  efi_vars: Implement SPI Flash store

 include/efi_variable.h          | 18 +++----
 lib/efi_loader/Kconfig          | 26 +++++++++-
 lib/efi_loader/Makefile         |  3 +-
 lib/efi_loader/efi_var_common.c | 44 ++++++++++++++++
 lib/efi_loader/efi_var_file.c   | 65 ++---------------------
 lib/efi_loader/efi_var_sf.c     | 91 +++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_variable.c   | 16 ++++--
 7 files changed, 187 insertions(+), 76 deletions(-)
 create mode 100644 lib/efi_loader/efi_var_sf.c

-- 
2.43.0

base-commit: 4aa15db6ff8b2bbdcdb082e547967bf4a96fee52
branch: master-next-test

[PATCH v4 1/3] efi_var_file: refactor to move buffer functions

[Michal Simek]

From: Shantur Rathore <i@shantur.com>

Currently efi_var_file.c has functions to store/read
EFI variables to/from memory buffer. These functions
can be used with other EFI variable stores so move
them out to efi_var_common.c

Signed-off-by: Shantur Rathore <i@shantur.com>
Signed-off-by: Michal Simek <michal.simek@amd.com>
---

(no changes since v1)

 include/efi_variable.h          |  5 +++
 lib/efi_loader/Makefile         |  2 +-
 lib/efi_loader/efi_var_common.c | 44 +++++++++++++++++++++++++
 lib/efi_loader/efi_var_file.c   | 57 ---------------------------------
 lib/efi_loader/efi_variable.c   | 10 +++++-
 5 files changed, 59 insertions(+), 59 deletions(-)

diff --git a/include/efi_variable.h b/include/efi_variable.h
index 4065cf45ecaf..ee68fa4a885f 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -161,6 +161,11 @@ efi_status_t efi_var_to_file(void);
 efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *lenp,
 					    u32 check_attr_mask);
 
+/* GUID used by Shim to store the MOK database */
+#define SHIM_LOCK_GUID \
+	EFI_GUID(0x605dab50, 0xe046, 0x4300, \
+		 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
+
 /**
  * efi_var_restore() - restore EFI variables from buffer
  *
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index bfa607c88274..07ae048f2e0c 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -53,7 +53,7 @@ ifeq ($(CONFIG_EFI_MM_COMM_TEE),y)
 obj-y += efi_variable_tee.o
 else
 obj-y += efi_variable.o
-obj-y += efi_var_file.o
+obj-$(CONFIG_EFI_VARIABLE_FILE_STORE) += efi_var_file.o
 obj-$(CONFIG_EFI_VARIABLES_PRESEED) += efi_var_seed.o
 endif
 obj-y += efi_watchdog.o
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index 4b34a58b4cf7..41cefb94198d 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -41,6 +41,7 @@ static const struct efi_auth_var_name_type name_type[] = {
 
 static bool efi_secure_boot;
 static enum efi_secure_mode efi_secure_mode;
+static const efi_guid_t shim_lock_guid = SHIM_LOCK_GUID;
 
 /**
  * efi_efi_get_variable() - retrieve value of a UEFI variable
@@ -488,3 +489,46 @@ efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *
 
 	return EFI_SUCCESS;
 }
+
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
+{
+	struct efi_var_entry *var, *last_var;
+	u16 *data;
+	efi_status_t ret;
+
+	if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
+	    buf->crc32 != crc32(0, (u8 *)buf->var,
+				buf->length - sizeof(struct efi_var_file))) {
+		log_err("Invalid EFI variables file\n");
+		return EFI_INVALID_PARAMETER;
+	}
+
+	last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
+	for (var = buf->var; var < last_var;
+	     var = (struct efi_var_entry *)
+		   ALIGN((uintptr_t)data + var->length, 8)) {
+
+		data = var->name + u16_strlen(var->name) + 1;
+
+		/*
+		 * Secure boot related and volatile variables shall only be
+		 * restored from U-Boot's preseed.
+		 */
+		if (!safe &&
+		    (efi_auth_var_get_type(var->name, &var->guid) !=
+		     EFI_AUTH_VAR_NONE ||
+		     !guidcmp(&var->guid, &shim_lock_guid) ||
+		     !(var->attr & EFI_VARIABLE_NON_VOLATILE)))
+			continue;
+		if (!var->length)
+			continue;
+		if (efi_var_mem_find(&var->guid, var->name, NULL))
+			continue;
+		ret = efi_var_mem_ins(var->name, &var->guid, var->attr,
+				      var->length, data, 0, NULL,
+				      var->time);
+		if (ret != EFI_SUCCESS)
+			log_err("Failed to set EFI variable %ls\n", var->name);
+	}
+	return EFI_SUCCESS;
+}
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
index ba0bf33ffbd1..d32edaac277d 100644
--- a/lib/efi_loader/efi_var_file.c
+++ b/lib/efi_loader/efi_var_file.c
@@ -14,17 +14,9 @@
 #include <mapmem.h>
 #include <efi_loader.h>
 #include <efi_variable.h>
-#include <u-boot/crc.h>
 
 #define PART_STR_LEN 10
 
-/* GUID used by Shim to store the MOK database */
-#define SHIM_LOCK_GUID \
-	EFI_GUID(0x605dab50, 0xe046, 0x4300, \
-		 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
-
-static const efi_guid_t shim_lock_guid = SHIM_LOCK_GUID;
-
 /**
  * efi_set_blk_dev_to_system_partition() - select EFI system partition
  *
@@ -59,7 +51,6 @@ static efi_status_t __maybe_unused efi_set_blk_dev_to_system_partition(void)
  */
 efi_status_t efi_var_to_file(void)
 {
-#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
 	efi_status_t ret;
 	struct efi_var_file *buf;
 	loff_t len;
@@ -91,52 +82,6 @@ error:
 out:
 	free(buf);
 	return ret;
-#else
-	return EFI_SUCCESS;
-#endif
-}
-
-efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
-{
-	struct efi_var_entry *var, *last_var;
-	u16 *data;
-	efi_status_t ret;
-
-	if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
-	    buf->crc32 != crc32(0, (u8 *)buf->var,
-				buf->length - sizeof(struct efi_var_file))) {
-		log_err("Invalid EFI variables file\n");
-		return EFI_INVALID_PARAMETER;
-	}
-
-	last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
-	for (var = buf->var; var < last_var;
-	     var = (struct efi_var_entry *)
-		   ALIGN((uintptr_t)data + var->length, 8)) {
-
-		data = var->name + u16_strlen(var->name) + 1;
-
-		/*
-		 * Secure boot related and volatile variables shall only be
-		 * restored from U-Boot's preseed.
-		 */
-		if (!safe &&
-		    (efi_auth_var_get_type(var->name, &var->guid) !=
-		     EFI_AUTH_VAR_NONE ||
-		     !guidcmp(&var->guid, &shim_lock_guid) ||
-		     !(var->attr & EFI_VARIABLE_NON_VOLATILE)))
-			continue;
-		if (!var->length)
-			continue;
-		if (efi_var_mem_find(&var->guid, var->name, NULL))
-			continue;
-		ret = efi_var_mem_ins(var->name, &var->guid, var->attr,
-				      var->length, data, 0, NULL,
-				      var->time);
-		if (ret != EFI_SUCCESS)
-			log_err("Failed to set EFI variable %ls\n", var->name);
-	}
-	return EFI_SUCCESS;
 }
 
 /**
@@ -155,7 +100,6 @@ efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
  */
 efi_status_t efi_var_from_file(void)
 {
-#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
 	struct efi_var_file *buf;
 	loff_t len;
 	efi_status_t ret;
@@ -180,6 +124,5 @@ efi_status_t efi_var_from_file(void)
 		log_err("Invalid EFI variables file\n");
 error:
 	free(buf);
-#endif
 	return EFI_SUCCESS;
 }
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index f3533f4def3a..9394ecf6994c 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -400,8 +400,13 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
 	 * Write non-volatile EFI variables to file
 	 * TODO: check if a value change has occured to avoid superfluous writes
 	 */
-	if (attributes & EFI_VARIABLE_NON_VOLATILE)
+	if (attributes & EFI_VARIABLE_NON_VOLATILE) {
+#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
 		efi_var_to_file();
+#else
+		return EFI_NOT_READY;
+#endif
+	}
 
 	return EFI_SUCCESS;
 }
@@ -594,9 +599,12 @@ efi_status_t efi_init_variables(void)
 	if (ret != EFI_SUCCESS)
 		return ret;
 
+#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
 	ret = efi_var_from_file();
 	if (ret != EFI_SUCCESS)
 		return ret;
+#endif
+
 	if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
 		ret = efi_var_restore((struct efi_var_file *)
 				      __efi_var_file_begin, true);
-- 
2.43.0

[PATCH v4 2/3] efi_var: Unify read/write access helper function

[Michal Simek]

efi_var_to/from_file() suggest method where variables are placed. But there
is no reason for it and generic name can be used to wire also different
locations for variables.

Signed-off-by: Michal Simek <michal.simek@amd.com>
---

Changes in v4:
- New patch based on review comments from v3

 include/efi_variable.h        | 13 ++++---------
 lib/efi_loader/efi_var_file.c |  8 ++++----
 lib/efi_loader/efi_variable.c |  4 ++--
 3 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/include/efi_variable.h b/include/efi_variable.h
index ee68fa4a885f..ae542c597129 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -137,13 +137,11 @@ struct efi_var_file {
 };
 
 /**
- * efi_var_to_file() - save non-volatile variables as file
- *
- * File ubootefi.var is created on the EFI system partion.
+ * efi_var_write() - save non-volatile variables
  *
  * Return:	status code
  */
-efi_status_t efi_var_to_file(void);
+efi_status_t efi_var_write(void);
 
 /**
  * efi_var_collect() - collect variables in buffer
@@ -178,17 +176,14 @@ efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *
 efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
 
 /**
- * efi_var_from_file() - read variables from file
- *
- * File ubootefi.var is read from the EFI system partitions and the variables
- * stored in the file are created.
+ * efi_var_read() - read variables
  *
  * In case the file does not exist yet or a variable cannot be set EFI_SUCCESS
  * is returned.
  *
  * Return:	status code
  */
-efi_status_t efi_var_from_file(void);
+efi_status_t efi_var_read(void);
 
 /**
  * efi_var_mem_init() - set-up variable list
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
index d32edaac277d..01ac7cc20e1f 100644
--- a/lib/efi_loader/efi_var_file.c
+++ b/lib/efi_loader/efi_var_file.c
@@ -43,13 +43,13 @@ static efi_status_t __maybe_unused efi_set_blk_dev_to_system_partition(void)
 }
 
 /**
- * efi_var_to_file() - save non-volatile variables as file
+ * efi_var_write() - save non-volatile variables as file
  *
  * File ubootefi.var is created on the EFI system partion.
  *
  * Return:	status code
  */
-efi_status_t efi_var_to_file(void)
+efi_status_t efi_var_write(void)
 {
 	efi_status_t ret;
 	struct efi_var_file *buf;
@@ -85,7 +85,7 @@ out:
 }
 
 /**
- * efi_var_from_file() - read variables from file
+ * efi_var_read() - read variables from file
  *
  * File ubootefi.var is read from the EFI system partitions and the variables
  * stored in the file are created.
@@ -98,7 +98,7 @@ out:
  *
  * Return:	status code
  */
-efi_status_t efi_var_from_file(void)
+efi_status_t efi_var_read(void)
 {
 	struct efi_var_file *buf;
 	loff_t len;
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 9394ecf6994c..be670a8e7c25 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -402,7 +402,7 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
 	 */
 	if (attributes & EFI_VARIABLE_NON_VOLATILE) {
 #if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
-		efi_var_to_file();
+		efi_var_write();
 #else
 		return EFI_NOT_READY;
 #endif
@@ -600,7 +600,7 @@ efi_status_t efi_init_variables(void)
 		return ret;
 
 #if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
-	ret = efi_var_from_file();
+	ret = efi_var_read();
 	if (ret != EFI_SUCCESS)
 		return ret;
 #endif
-- 
2.43.0

[PATCH v4 3/3] efi_vars: Implement SPI Flash store

[Michal Simek]

From: Shantur Rathore <i@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@shantur.com>
Signed-off-by: Michal Simek <michal.simek@amd.com>
---

Changes in v4:
- 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        | 26 +++++++++-
 lib/efi_loader/Makefile       |  1 +
 lib/efi_loader/efi_var_sf.c   | 91 +++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_variable.c |  6 +--
 4 files changed, 120 insertions(+), 4 deletions(-)
 create mode 100644 lib/efi_loader/efi_var_sf.c

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 13e44be1d067..e753a3aef36d 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -123,6 +123,16 @@ 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
@@ -152,7 +162,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
 	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
@@ -193,6 +203,20 @@ 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"
+	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 07ae048f2e0c..0eb9826c7ed8 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..fd6b1ce22322
--- /dev/null
+++ b/lib/efi_loader/efi_var_sf.c
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * SPI Flash interface for UEFI variables
+ *
+ * Copyright (c) 2023, Shantur Rathore
+ */
+
+#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_write(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_read(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;
+}
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index be670a8e7c25..feab212b245b 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -397,11 +397,11 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
 		ret = EFI_SUCCESS;
 
 	/*
-	 * Write non-volatile EFI variables to file
+	 * Write non-volatile EFI variables to file or SPI Flash
 	 * TODO: check if a value change has occured to avoid superfluous writes
 	 */
 	if (attributes & EFI_VARIABLE_NON_VOLATILE) {
-#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
+#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
 		efi_var_write();
 #else
 		return EFI_NOT_READY;
@@ -599,7 +599,7 @@ efi_status_t efi_init_variables(void)
 	if (ret != EFI_SUCCESS)
 		return ret;
 
-#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
+#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
 	ret = efi_var_read();
 	if (ret != EFI_SUCCESS)
 		return ret;
-- 
2.43.0

Re: [PATCH v4 3/3] efi_vars: Implement SPI Flash store

[Ilias Apalodimas]

Hi Michal

Thanks for taking the time.

[...]

> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index be670a8e7c25..feab212b245b 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -397,11 +397,11 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
>                 ret = EFI_SUCCESS;
>
>         /*
> -        * Write non-volatile EFI variables to file
> +        * Write non-volatile EFI variables to file or SPI Flash
>          * TODO: check if a value change has occured to avoid superfluous writes
>          */
>         if (attributes & EFI_VARIABLE_NON_VOLATILE) {
> -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
>                 efi_var_write();
>  #else
>                 return EFI_NOT_READY;
> @@ -599,7 +599,7 @@ efi_status_t efi_init_variables(void)
>         if (ret != EFI_SUCCESS)
>                 return ret;
>
> -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)

So I think this is might to come back and bite us in the future.
The name of the file is a bit misleading, but efi_variable.c is
supposed to handle the variables with a file backed storage and
efi_variable_tee.c is handling the variables when the storage is
isolated in the secure world.
The problem is that each case defines the boottime and runtime
services. The functions that are common across cases live in
efi_var_common.c. That files defines the boottime and runtime calls,
by calling the *_int() variants.

The file backed and SPI flash storage seem to have a lot in common.
e.g they both use the memory backend to expose the vairables the the
OS, query the available storage etc. But I am not they will end up
being 100% identical. If they do this approach is ok. But ifthey don't
it's better to expand the efi_var_sf.c you added and add code for
efi_variables_boot_exit_notify(), and any *_int() functions that are
different.

Heinrich any opinions? I'll need to think through the SPI case
inclduing runtime support before taking this in

Thanks
/Ilias

>         ret = efi_var_read();
>         if (ret != EFI_SUCCESS)
>                 return ret;
> --
> 2.43.0
>

Re: [PATCH v4 3/3] efi_vars: Implement SPI Flash store

[Peter Robinson]

On Thu, 15 Jan 2026 at 08:05, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Michal
>
> Thanks for taking the time.
>
> [...]
>
> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > index be670a8e7c25..feab212b245b 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -397,11 +397,11 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
> >                 ret = EFI_SUCCESS;
> >
> >         /*
> > -        * Write non-volatile EFI variables to file
> > +        * Write non-volatile EFI variables to file or SPI Flash
> >          * TODO: check if a value change has occured to avoid superfluous writes
> >          */
> >         if (attributes & EFI_VARIABLE_NON_VOLATILE) {
> > -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> > +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
> >                 efi_var_write();
> >  #else
> >                 return EFI_NOT_READY;
> > @@ -599,7 +599,7 @@ efi_status_t efi_init_variables(void)
> >         if (ret != EFI_SUCCESS)
> >                 return ret;
> >
> > -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> > +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
>
> So I think this is might to come back and bite us in the future.
> The name of the file is a bit misleading, but efi_variable.c is
> supposed to handle the variables with a file backed storage and
> efi_variable_tee.c is handling the variables when the storage is
> isolated in the secure world.
> The problem is that each case defines the boottime and runtime
> services. The functions that are common across cases live in
> efi_var_common.c. That files defines the boottime and runtime calls,
> by calling the *_int() variants.
>
> The file backed and SPI flash storage seem to have a lot in common.
> e.g they both use the memory backend to expose the vairables the the
> OS, query the available storage etc. But I am not they will end up
> being 100% identical. If they do this approach is ok. But ifthey don't
> it's better to expand the efi_var_sf.c you added and add code for
> efi_variables_boot_exit_notify(), and any *_int() functions that are
> different.
>
> Heinrich any opinions? I'll need to think through the SPI case
> inclduing runtime support before taking this in

Do we also need to consider the case where the SPI flash remains owned
by the firmware/secure world, and the case where it's handed over to
the OS?

Re: [PATCH v4 3/3] efi_vars: Implement SPI Flash store

[Ilias Apalodimas]

On Thu, 15 Jan 2026 at 12:02, Peter Robinson <pbrobinson@gmail.com> wrote:
>
> On Thu, 15 Jan 2026 at 08:05, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Michal
> >
> > Thanks for taking the time.
> >
> > [...]
> >
> > > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > > index be670a8e7c25..feab212b245b 100644
> > > --- a/lib/efi_loader/efi_variable.c
> > > +++ b/lib/efi_loader/efi_variable.c
> > > @@ -397,11 +397,11 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
> > >                 ret = EFI_SUCCESS;
> > >
> > >         /*
> > > -        * Write non-volatile EFI variables to file
> > > +        * Write non-volatile EFI variables to file or SPI Flash
> > >          * TODO: check if a value change has occured to avoid superfluous writes
> > >          */
> > >         if (attributes & EFI_VARIABLE_NON_VOLATILE) {
> > > -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> > > +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
> > >                 efi_var_write();
> > >  #else
> > >                 return EFI_NOT_READY;
> > > @@ -599,7 +599,7 @@ efi_status_t efi_init_variables(void)
> > >         if (ret != EFI_SUCCESS)
> > >                 return ret;
> > >
> > > -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> > > +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
> >
> > So I think this is might to come back and bite us in the future.
> > The name of the file is a bit misleading, but efi_variable.c is
> > supposed to handle the variables with a file backed storage and
> > efi_variable_tee.c is handling the variables when the storage is
> > isolated in the secure world.
> > The problem is that each case defines the boottime and runtime
> > services. The functions that are common across cases live in
> > efi_var_common.c. That files defines the boottime and runtime calls,
> > by calling the *_int() variants.
> >
> > The file backed and SPI flash storage seem to have a lot in common.
> > e.g they both use the memory backend to expose the vairables the the
> > OS, query the available storage etc. But I am not they will end up
> > being 100% identical. If they do this approach is ok. But ifthey don't
> > it's better to expand the efi_var_sf.c you added and add code for
> > efi_variables_boot_exit_notify(), and any *_int() functions that are
> > different.
> >
> > Heinrich any opinions? I'll need to think through the SPI case
> > inclduing runtime support before taking this in
>
> Do we also need to consider the case where the SPI flash remains owned
> by the firmware/secure world, and the case where it's handed over to
> the OS?

No, we dont have to care about this. If the SPI moves to the secure
world, then efi_variable_tee.c will take care of it. This file is
basically a glue layer between U-Boot and StMM running in op-tee. So
the majority of the works to support this is in OP-TEE & StMM -- add a
spi bus driver, spi flash driver etc. I dont expect to change the
u-boot part substantially

Cheers
/Ilias

Re: [PATCH v4 3/3] efi_vars: Implement SPI Flash store

[Ilias Apalodimas]

Hi Michal,

[...]

>
>  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

Will this work on nands as well? They got a  much shrter lifetime that
spi flashes.

>         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

I am not sure we need to allow this for now. At least not until we've
talked to the efitool maintainers and make sure they will accept
another 'special' case.

The problem with allowing this is that if people enable it, boot a
linux and do a setvariable, it will return a success. But none of the
variables will be preserved after a reboot unless someone manually
updates the serial flash contents. In theory, we can preserve the
driver  model and the spi drivers in EFI runtime services and allow
'proper' setvariable at runtime.
However, I think this is not very useful. Having an unprotected to
store authenticated EFI variables, is dangerous. Someone can erase the
SPI flash and efectively disable secure boot. Due to that, I prefer
the current file based approach for EFI variables -- which doesn't
store/restore authenticated EFI variables (and which this patch
implements). The obvious downside is that enable setvariable at
runtime is tricky once again....

[...]

> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index be670a8e7c25..feab212b245b 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -397,11 +397,11 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
>                 ret = EFI_SUCCESS;
>
>         /*
> -        * Write non-volatile EFI variables to file
> +        * Write non-volatile EFI variables to file or SPI Flash
>          * TODO: check if a value change has occured to avoid superfluous writes
>          */
>         if (attributes & EFI_VARIABLE_NON_VOLATILE) {
> -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)

Do we need the ifdefery here? efi_variable.o is basically compiled
whenever we have the variables managed by the non-secure world and
this function will exist either with a file back storage or a SPI
flash
>                 efi_var_write();
>  #else
>                 return EFI_NOT_READY;
> @@ -599,7 +599,7 @@ efi_status_t efi_init_variables(void)
>         if (ret != EFI_SUCCESS)
>                 return ret;
>
> -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)

ditto

>         ret = efi_var_read();
>         if (ret != EFI_SUCCESS)
>                 return ret;
> --
> 2.43.0
>

Cheers
/Ilias

Re: [PATCH v4 3/3] efi_vars: Implement SPI Flash store

[Michal Simek]

Hi,

On 1/22/26 13:18, Ilias Apalodimas wrote:
> Hi Michal,
> 
> [...]
> 
>>
>>   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
> 
> Will this work on nands as well? They got a  much shrter lifetime that
> spi flashes.

I don't think this is valid argument here. Users have options to decide where to 
put them. The same argument can be used for u-boot variables saved in NAND.
Obviously I can update Kconfig option with saying that you should be careful 
about it but it is user choice.

> 
>>          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
> 
> I am not sure we need to allow this for now. At least not until we've
> talked to the efitool maintainers and make sure they will accept
> another 'special' case.

Not a problem for me to disable it for now.

> 
> The problem with allowing this is that if people enable it, boot a
> linux and do a setvariable, it will return a success. But none of the
> variables will be preserved after a reboot unless someone manually
> updates the serial flash contents. In theory, we can preserve the
> driver  model and the spi drivers in EFI runtime services and allow
> 'proper' setvariable at runtime.

Wasn't this the same case for file based one? I mean u-boot had it but efi

Yours efivar patch was merged Jun 23, 2025
But this symbol was introduce in U-Boot April 18, 2024.


> However, I think this is not very useful. Having an unprotected to
> store authenticated EFI variables, is dangerous. Someone can erase the
> SPI flash and efectively disable secure boot. Due to that, I prefer
> the current file based approach for EFI variables -- which doesn't
> store/restore authenticated EFI variables (and which this patch
> implements). The obvious downside is that enable setvariable at
> runtime is tricky once again....

Isn't the same happening with file store too?

 From implementation perspective it should be the same as file based. Obviously 
efivar part is missing.

> 
> [...]
> 
>> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
>> index be670a8e7c25..feab212b245b 100644
>> --- a/lib/efi_loader/efi_variable.c
>> +++ b/lib/efi_loader/efi_variable.c
>> @@ -397,11 +397,11 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
>>                  ret = EFI_SUCCESS;
>>
>>          /*
>> -        * Write non-volatile EFI variables to file
>> +        * Write non-volatile EFI variables to file or SPI Flash
>>           * TODO: check if a value change has occured to avoid superfluous writes
>>           */
>>          if (attributes & EFI_VARIABLE_NON_VOLATILE) {
>> -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
>> +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
> 
> Do we need the ifdefery here? efi_variable.o is basically compiled
> whenever we have the variables managed by the non-secure world and
> this function will exist either with a file back storage or a SPI
> flash

It can be reverted and return EFI_NOT_READY for EFI_VARIABLE_NO_STORE.

Thanks,
Michal

Re: [PATCH v4 3/3] efi_vars: Implement SPI Flash store

[Ilias Apalodimas]

On Thu, 22 Jan 2026 at 14:32, Michal Simek <michal.simek@amd.com> wrote:
>
> Hi,
>
> On 1/22/26 13:18, Ilias Apalodimas wrote:
> > Hi Michal,
> >
> > [...]
> >
> >>
> >>   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
> >
> > Will this work on nands as well? They got a  much shrter lifetime that
> > spi flashes.
>
> I don't think this is valid argument here. Users have options to decide where to
> put them. The same argument can be used for u-boot variables saved in NAND.
> Obviously I can update Kconfig option with saying that you should be careful
> about it but it is user choice.

Yes I think that's a good idea.

>
> >
> >>          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
> >
> > I am not sure we need to allow this for now. At least not until we've
> > talked to the efitool maintainers and make sure they will accept
> > another 'special' case.
>
> Not a problem for me to disable it for now.
>
> >
> > The problem with allowing this is that if people enable it, boot a
> > linux and do a setvariable, it will return a success. But none of the
> > variables will be preserved after a reboot unless someone manually
> > updates the serial flash contents. In theory, we can preserve the
> > driver  model and the spi drivers in EFI runtime services and allow
> > 'proper' setvariable at runtime.
>
> Wasn't this the same case for file based one? I mean u-boot had it but efi
>
> Yours efivar patch was merged Jun 23, 2025
> But this symbol was introduce in U-Boot April 18, 2024.

Yes but the patches were sent a long time ago in efivar. If you look
at the original PR date it was a lot eariler [0]. It just took a long
time to discuss and merge.
Apart from that the case for file stored variables is by itself pretty
unique and convincing the maintainers to accept the patch had some
basis. I don't know if they want the same workaround for spi flashes.

>
>
> > However, I think this is not very useful. Having an unprotected to
> > store authenticated EFI variables, is dangerous. Someone can erase the
> > SPI flash and efectively disable secure boot. Due to that, I prefer
> > the current file based approach for EFI variables -- which doesn't
> > store/restore authenticated EFI variables (and which this patch
> > implements). The obvious downside is that enable setvariable at
> > runtime is tricky once again....
>
> Isn't the same happening with file store too?
>
>  From implementation perspective it should be the same as file based. Obviously
> efivar part is missing.

Yes. However, that commit included instructions on how to set runtime
variables with dd, so people could use it until the efivar changes
were merged.

>
> >
> > [...]
> >
> >> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> >> index be670a8e7c25..feab212b245b 100644
> >> --- a/lib/efi_loader/efi_variable.c
> >> +++ b/lib/efi_loader/efi_variable.c
> >> @@ -397,11 +397,11 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
> >>                  ret = EFI_SUCCESS;
> >>
> >>          /*
> >> -        * Write non-volatile EFI variables to file
> >> +        * Write non-volatile EFI variables to file or SPI Flash
> >>           * TODO: check if a value change has occured to avoid superfluous writes
> >>           */
> >>          if (attributes & EFI_VARIABLE_NON_VOLATILE) {
> >> -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> >> +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
> >
> > Do we need the ifdefery here? efi_variable.o is basically compiled
> > whenever we have the variables managed by the non-secure world and
> > this function will exist either with a file back storage or a SPI
> > flash
>
> It can be reverted and return EFI_NOT_READY for EFI_VARIABLE_NO_STORE.

Yes I *thinkk* that will look cleaner.

>

[0] https://github.com/rhboot/efivar/pull/267

Cheers
/Ilias
> Thanks,
> Michal

Re: [PATCH v4 3/3] efi_vars: Implement SPI Flash store

[Michal Simek]

On 1/23/26 08:40, Ilias Apalodimas wrote:
> On Thu, 22 Jan 2026 at 14:32, Michal Simek <michal.simek@amd.com> wrote:
>>
>> Hi,
>>
>> On 1/22/26 13:18, Ilias Apalodimas wrote:
>>> Hi Michal,
>>>
>>> [...]
>>>
>>>>
>>>>    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
>>>
>>> Will this work on nands as well? They got a  much shrter lifetime that
>>> spi flashes.
>>
>> I don't think this is valid argument here. Users have options to decide where to
>> put them. The same argument can be used for u-boot variables saved in NAND.
>> Obviously I can update Kconfig option with saying that you should be careful
>> about it but it is user choice.
> 
> Yes I think that's a good idea.
> 
>>
>>>
>>>>           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
>>>
>>> I am not sure we need to allow this for now. At least not until we've
>>> talked to the efitool maintainers and make sure they will accept
>>> another 'special' case.
>>
>> Not a problem for me to disable it for now.
>>
>>>
>>> The problem with allowing this is that if people enable it, boot a
>>> linux and do a setvariable, it will return a success. But none of the
>>> variables will be preserved after a reboot unless someone manually
>>> updates the serial flash contents. In theory, we can preserve the
>>> driver  model and the spi drivers in EFI runtime services and allow
>>> 'proper' setvariable at runtime.
>>
>> Wasn't this the same case for file based one? I mean u-boot had it but efi
>>
>> Yours efivar patch was merged Jun 23, 2025
>> But this symbol was introduce in U-Boot April 18, 2024.
> 
> Yes but the patches were sent a long time ago in efivar. If you look
> at the original PR date it was a lot eariler [0]. It just took a long
> time to discuss and merge.
> Apart from that the case for file stored variables is by itself pretty
> unique and convincing the maintainers to accept the patch had some
> basis. I don't know if they want the same workaround for spi flashes.

Correct June, 2024
https://github.com/rhboot/efivar/pull/267


>>
>>
>>> However, I think this is not very useful. Having an unprotected to
>>> store authenticated EFI variables, is dangerous. Someone can erase the
>>> SPI flash and efectively disable secure boot. Due to that, I prefer
>>> the current file based approach for EFI variables -- which doesn't
>>> store/restore authenticated EFI variables (and which this patch
>>> implements). The obvious downside is that enable setvariable at
>>> runtime is tricky once again....
>>
>> Isn't the same happening with file store too?
>>
>>   From implementation perspective it should be the same as file based. Obviously
>> efivar part is missing.
> 
> Yes. However, that commit included instructions on how to set runtime
> variables with dd, so people could use it until the efivar changes
> were merged.

I don't think it is going to be problem too.

What about?
dd if=/sys/firmware/efi/efivars/VarToFile-b2ac5fc9-92b7-4acd-aeac-11e818c3130c \
    of=/tmp/vars skip=4 bs=1
flashcp /tmp/vars /dev/mtdX

> 
>>
>>>
>>> [...]
>>>
>>>> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
>>>> index be670a8e7c25..feab212b245b 100644
>>>> --- a/lib/efi_loader/efi_variable.c
>>>> +++ b/lib/efi_loader/efi_variable.c
>>>> @@ -397,11 +397,11 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
>>>>                   ret = EFI_SUCCESS;
>>>>
>>>>           /*
>>>> -        * Write non-volatile EFI variables to file
>>>> +        * Write non-volatile EFI variables to file or SPI Flash
>>>>            * TODO: check if a value change has occured to avoid superfluous writes
>>>>            */
>>>>           if (attributes & EFI_VARIABLE_NON_VOLATILE) {
>>>> -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
>>>> +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
>>>
>>> Do we need the ifdefery here? efi_variable.o is basically compiled
>>> whenever we have the variables managed by the non-secure world and
>>> this function will exist either with a file back storage or a SPI
>>> flash
>>
>> It can be reverted and return EFI_NOT_READY for EFI_VARIABLE_NO_STORE.
> 
> Yes I *thinkk* that will look cleaner.

ok.

Thanks,
Michal

Re: [PATCH v4 3/3] efi_vars: Implement SPI Flash store

[Ilias Apalodimas]

On Fri, 23 Jan 2026 at 10:22, Michal Simek <michal.simek@amd.com> wrote:
>
>
>
> On 1/23/26 08:40, Ilias Apalodimas wrote:
> > On Thu, 22 Jan 2026 at 14:32, Michal Simek <michal.simek@amd.com> wrote:
> >>
> >> Hi,
> >>
> >> On 1/22/26 13:18, Ilias Apalodimas wrote:
> >>> Hi Michal,
> >>>
> >>> [...]
> >>>
> >>>>
> >>>>    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
> >>>
> >>> Will this work on nands as well? They got a  much shrter lifetime that
> >>> spi flashes.
> >>
> >> I don't think this is valid argument here. Users have options to decide where to
> >> put them. The same argument can be used for u-boot variables saved in NAND.
> >> Obviously I can update Kconfig option with saying that you should be careful
> >> about it but it is user choice.
> >
> > Yes I think that's a good idea.
> >
> >>
> >>>
> >>>>           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
> >>>
> >>> I am not sure we need to allow this for now. At least not until we've
> >>> talked to the efitool maintainers and make sure they will accept
> >>> another 'special' case.
> >>
> >> Not a problem for me to disable it for now.
> >>
> >>>
> >>> The problem with allowing this is that if people enable it, boot a
> >>> linux and do a setvariable, it will return a success. But none of the
> >>> variables will be preserved after a reboot unless someone manually
> >>> updates the serial flash contents. In theory, we can preserve the
> >>> driver  model and the spi drivers in EFI runtime services and allow
> >>> 'proper' setvariable at runtime.
> >>
> >> Wasn't this the same case for file based one? I mean u-boot had it but efi
> >>
> >> Yours efivar patch was merged Jun 23, 2025
> >> But this symbol was introduce in U-Boot April 18, 2024.
> >
> > Yes but the patches were sent a long time ago in efivar. If you look
> > at the original PR date it was a lot eariler [0]. It just took a long
> > time to discuss and merge.
> > Apart from that the case for file stored variables is by itself pretty
> > unique and convincing the maintainers to accept the patch had some
> > basis. I don't know if they want the same workaround for spi flashes.
>
> Correct June, 2024
> https://github.com/rhboot/efivar/pull/267
>
>
> >>
> >>
> >>> However, I think this is not very useful. Having an unprotected to
> >>> store authenticated EFI variables, is dangerous. Someone can erase the
> >>> SPI flash and efectively disable secure boot. Due to that, I prefer
> >>> the current file based approach for EFI variables -- which doesn't
> >>> store/restore authenticated EFI variables (and which this patch
> >>> implements). The obvious downside is that enable setvariable at
> >>> runtime is tricky once again....
> >>
> >> Isn't the same happening with file store too?
> >>
> >>   From implementation perspective it should be the same as file based. Obviously
> >> efivar part is missing.
> >
> > Yes. However, that commit included instructions on how to set runtime
> > variables with dd, so people could use it until the efivar changes
> > were merged.
>
> I don't think it is going to be problem too.
>
> What about?
> dd if=/sys/firmware/efi/efivars/VarToFile-b2ac5fc9-92b7-4acd-aeac-11e818c3130c \
>     of=/tmp/vars skip=4 bs=1
> flashcp /tmp/vars /dev/mtdX

If you manage to test this then yes, please include it on the commit
message and in the future we can add it to the documentation

>
> >
> >>
> >>>
> >>> [...]
> >>>
> >>>> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> >>>> index be670a8e7c25..feab212b245b 100644
> >>>> --- a/lib/efi_loader/efi_variable.c
> >>>> +++ b/lib/efi_loader/efi_variable.c
> >>>> @@ -397,11 +397,11 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
> >>>>                   ret = EFI_SUCCESS;
> >>>>
> >>>>           /*
> >>>> -        * Write non-volatile EFI variables to file
> >>>> +        * Write non-volatile EFI variables to file or SPI Flash
> >>>>            * TODO: check if a value change has occured to avoid superfluous writes
> >>>>            */
> >>>>           if (attributes & EFI_VARIABLE_NON_VOLATILE) {
> >>>> -#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE)
> >>>> +#if CONFIG_IS_ENABLED(EFI_VARIABLE_FILE_STORE) || CONFIG_IS_ENABLED(EFI_VARIABLE_SF_STORE)
> >>>
> >>> Do we need the ifdefery here? efi_variable.o is basically compiled
> >>> whenever we have the variables managed by the non-secure world and
> >>> this function will exist either with a file back storage or a SPI
> >>> flash
> >>
> >> It can be reverted and return EFI_NOT_READY for EFI_VARIABLE_NO_STORE.
> >
> > Yes I *thinkk* that will look cleaner.
>
> ok.

Thanks
/Ilias
>
> Thanks,
> Michal

Re: [PATCH v4 3/3] efi_vars: Implement SPI Flash store

[Michal Simek]

On 1/23/26 14:01, Ilias Apalodimas wrote:
> On Fri, 23 Jan 2026 at 10:22, Michal Simek <michal.simek@amd.com> wrote:
>>
>>
>>
>> On 1/23/26 08:40, Ilias Apalodimas wrote:
>>> On Thu, 22 Jan 2026 at 14:32, Michal Simek <michal.simek@amd.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 1/22/26 13:18, Ilias Apalodimas wrote:
>>>>> Hi Michal,
>>>>>
>>>>> [...]
>>>>>
>>>>>>
>>>>>>     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
>>>>>
>>>>> Will this work on nands as well? They got a  much shrter lifetime that
>>>>> spi flashes.
>>>>
>>>> I don't think this is valid argument here. Users have options to decide where to
>>>> put them. The same argument can be used for u-boot variables saved in NAND.
>>>> Obviously I can update Kconfig option with saying that you should be careful
>>>> about it but it is user choice.
>>>
>>> Yes I think that's a good idea.
>>>
>>>>
>>>>>
>>>>>>            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
>>>>>
>>>>> I am not sure we need to allow this for now. At least not until we've
>>>>> talked to the efitool maintainers and make sure they will accept
>>>>> another 'special' case.
>>>>
>>>> Not a problem for me to disable it for now.
>>>>
>>>>>
>>>>> The problem with allowing this is that if people enable it, boot a
>>>>> linux and do a setvariable, it will return a success. But none of the
>>>>> variables will be preserved after a reboot unless someone manually
>>>>> updates the serial flash contents. In theory, we can preserve the
>>>>> driver  model and the spi drivers in EFI runtime services and allow
>>>>> 'proper' setvariable at runtime.
>>>>
>>>> Wasn't this the same case for file based one? I mean u-boot had it but efi
>>>>
>>>> Yours efivar patch was merged Jun 23, 2025
>>>> But this symbol was introduce in U-Boot April 18, 2024.
>>>
>>> Yes but the patches were sent a long time ago in efivar. If you look
>>> at the original PR date it was a lot eariler [0]. It just took a long
>>> time to discuss and merge.
>>> Apart from that the case for file stored variables is by itself pretty
>>> unique and convincing the maintainers to accept the patch had some
>>> basis. I don't know if they want the same workaround for spi flashes.
>>
>> Correct June, 2024
>> https://github.com/rhboot/efivar/pull/267
>>
>>
>>>>
>>>>
>>>>> However, I think this is not very useful. Having an unprotected to
>>>>> store authenticated EFI variables, is dangerous. Someone can erase the
>>>>> SPI flash and efectively disable secure boot. Due to that, I prefer
>>>>> the current file based approach for EFI variables -- which doesn't
>>>>> store/restore authenticated EFI variables (and which this patch
>>>>> implements). The obvious downside is that enable setvariable at
>>>>> runtime is tricky once again....
>>>>
>>>> Isn't the same happening with file store too?
>>>>
>>>>    From implementation perspective it should be the same as file based. Obviously
>>>> efivar part is missing.
>>>
>>> Yes. However, that commit included instructions on how to set runtime
>>> variables with dd, so people could use it until the efivar changes
>>> were merged.
>>
>> I don't think it is going to be problem too.
>>
>> What about?
>> dd if=/sys/firmware/efi/efivars/VarToFile-b2ac5fc9-92b7-4acd-aeac-11e818c3130c \
>>      of=/tmp/vars skip=4 bs=1
>> flashcp /tmp/vars /dev/mtdX
> 
> If you manage to test this then yes, please include it on the commit
> message and in the future we can add it to the documentation

yes, I have also tested it on SOM.

Let me send v5 with discussed changes and also with logs.

Thanks,
Michal

Re: [PATCH v4 0/3] efi_vars: Implement SPI Flash storage for EFI

[E Shattow]

Hi Michal I have some user-facing questions about this concept.

On 1/14/26 07:15, Michal Simek wrote:
> 
> This is updated series based on v3 version sent here
> https://lore.kernel.org/all/20231126220836.374956-1-i@shantur.com/
> 
> that's why I am continuing on v4 instead of starting from scratch.
> 
> Tested on kv260 with saving variables to location where User MTD partition
> is.
> CONFIG_EFI_VARIABLE_SF_STORE=y
> CONFIG_EFI_RT_VOLATILE_STORE=y
> CONFIG_EFI_VARIABLE_SF_OFFSET=0x22a0000
> 
> Thanks,
> Michal
> 
> Changes in v4:
> - New patch based on review comments from v3
> - 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
> 
> Michal Simek (1):
>   efi_var: Unify read/write access helper function
> 
> Shantur Rathore (2):
>   efi_var_file: refactor to move buffer functions
>   efi_vars: Implement SPI Flash store
> 
>  include/efi_variable.h          | 18 +++----
>  lib/efi_loader/Kconfig          | 26 +++++++++-
>  lib/efi_loader/Makefile         |  3 +-
>  lib/efi_loader/efi_var_common.c | 44 ++++++++++++++++
>  lib/efi_loader/efi_var_file.c   | 65 ++---------------------
>  lib/efi_loader/efi_var_sf.c     | 91 +++++++++++++++++++++++++++++++++
>  lib/efi_loader/efi_variable.c   | 16 ++++--
>  7 files changed, 187 insertions(+), 76 deletions(-)
>  create mode 100644 lib/efi_loader/efi_var_sf.c
> 

So far as I'm aware support for EFI variable storage is always integral
to a filesystem as the EFI System Partition, even in the case of the
specially-handled MMC boot partition facility. With presence of many
possible ESP is there then only one active ESP, or do all ESP become
part of a pool of available locations?  I'm not understanding this
clearly what is the purpose of selecting an ESP if the variable storage
is not integral to the ESP.

Does having EFI variable storage in SPI Flash always take priority over
other ESP presence that might have additional variable storage as a file?

Can (or should?) there be a runtime configurable setting to decide two
preferences; does one depend on the other or are these independent?:

1. Preferred ESP
2. Preferred EFI variable storage strategy

Consider not wanting to have a perceived ABI breakage as the SPI Flash
layout is previously codified by user expectations of what the vendor
initially shipped with the product and having been codified in upstream
Linux kernel devicetree definition. For the example where this would be
useful for boards where there is a well-known SPI Flash layout, I am
thinking of StarFive VisionFive 2 where there is {U-Boot SPL, U-Boot env
storage, U-Boot Main at 1MiB offset}; does your implementation of EFI
variable storage in SPI Flash have a suggested co-existence with the
"gap" in the area that is currently U-Boot env storage?

The use situations I am thinking of:

A. Installing a new OS or booting purpose-built image, where it is
intended that having (for example) a USB mass storage class or SD Card
of the all-in-one image depends on U-Boot in SPI Flash deciding both
that the EFI System Partition on the removable media is a priority over
other similar media (other different USB boot devices that may be
present, existing eMMC storage module possibly with ESP in addition to
the inserted removable SD Card).

B. Preventing the ambiguous selection of any additional removable
storage containing an EFI System Partition from breaking the desired
boot strategy. In many situations a user wants assurance that the board
will boot exactly the same every time regardless of what additional
removable media containing EFI System Partition might be connected.

C. Corrupt or unusable selected EFI System Partition prevents any
recovery and there is no runtime mechanism to attempt an alternative

Is there a possible mechanism for adding a runtime check (GPIO state,
key press, network event) to make the decision between some immutable
boot preference, or a user-configured boot preference?

Overall I want to get an understanding of how it might be recommended to
make use of this for StarFive VisionFive 2 board target(s). Many hours
of troubleshooting headache have been the result of EFI System Partition
confusion on board that have a wide variety of storage subsystems (MMC,
USB, NVMe, ...) which result in some really strange outcomes with
installers of GNU/Linux OS distros;  particularly as the devicetree dtb
search path currently points to storage subsystems that are typically
removable media, or the failure mode where an OS installer is using the
selected EFI System Partition that "leads on a path to nowhere" and is
not the intended installation target.

-E

Re: [PATCH v4 0/3] efi_vars: Implement SPI Flash storage for EFI

[Peter Robinson]

On Wed, 14 Jan 2026 at 18:35, E Shattow <e@freeshell.de> wrote:
>
> Hi Michal I have some user-facing questions about this concept.
>
> On 1/14/26 07:15, Michal Simek wrote:
> >
> > This is updated series based on v3 version sent here
> > https://lore.kernel.org/all/20231126220836.374956-1-i@shantur.com/
> >
> > that's why I am continuing on v4 instead of starting from scratch.
> >
> > Tested on kv260 with saving variables to location where User MTD partition
> > is.
> > CONFIG_EFI_VARIABLE_SF_STORE=y
> > CONFIG_EFI_RT_VOLATILE_STORE=y
> > CONFIG_EFI_VARIABLE_SF_OFFSET=0x22a0000
> >
> > Thanks,
> > Michal
> >
> > Changes in v4:
> > - New patch based on review comments from v3
> > - 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
> >
> > Michal Simek (1):
> >   efi_var: Unify read/write access helper function
> >
> > Shantur Rathore (2):
> >   efi_var_file: refactor to move buffer functions
> >   efi_vars: Implement SPI Flash store
> >
> >  include/efi_variable.h          | 18 +++----
> >  lib/efi_loader/Kconfig          | 26 +++++++++-
> >  lib/efi_loader/Makefile         |  3 +-
> >  lib/efi_loader/efi_var_common.c | 44 ++++++++++++++++
> >  lib/efi_loader/efi_var_file.c   | 65 ++---------------------
> >  lib/efi_loader/efi_var_sf.c     | 91 +++++++++++++++++++++++++++++++++
> >  lib/efi_loader/efi_variable.c   | 16 ++++--
> >  7 files changed, 187 insertions(+), 76 deletions(-)
> >  create mode 100644 lib/efi_loader/efi_var_sf.c
> >
>
> So far as I'm aware support for EFI variable storage is always integral
> to a filesystem as the EFI System Partition, even in the case of the
> specially-handled MMC boot partition facility. With presence of many
> possible ESP is there then only one active ESP, or do all ESP become
> part of a pool of available locations?  I'm not understanding this
> clearly what is the purpose of selecting an ESP if the variable storage
> is not integral to the ESP.

EFI variables have nothing to do with ESP filesystems. Most
implementations of EFI variables don't use ESP partitions to write
variables, U-Boot did that basically as a work around. If you look at
the average UEFI implementation on a Intel device the EFI vars are
written to the flash storage where they can be signed and validated to
ensure they're not changed without the firmware knowing.

> Does having EFI variable storage in SPI Flash always take priority over
> other ESP presence that might have additional variable storage as a file?

That would presumably depend on the U-Boot build options.

> Can (or should?) there be a runtime configurable setting to decide two
> preferences; does one depend on the other or are these independent?:

No. It would be a device decision.

> 1. Preferred ESP
> 2. Preferred EFI variable storage strategy
>
> Consider not wanting to have a perceived ABI breakage as the SPI Flash
> layout is previously codified by user expectations of what the vendor
> initially shipped with the product and having been codified in upstream
> Linux kernel devicetree definition. For the example where this would be
> useful for boards where there is a well-known SPI Flash layout, I am
> thinking of StarFive VisionFive 2 where there is {U-Boot SPL, U-Boot env
> storage, U-Boot Main at 1MiB offset}; does your implementation of EFI
> variable storage in SPI Flash have a suggested co-existence with the
> "gap" in the area that is currently U-Boot env storage?

The board maintainer would decide that, or the vendor, or if the user
wants to building their own.

> The use situations I am thinking of:
>
> A. Installing a new OS or booting purpose-built image, where it is
> intended that having (for example) a USB mass storage class or SD Card
> of the all-in-one image depends on U-Boot in SPI Flash deciding both
> that the EFI System Partition on the removable media is a priority over
> other similar media (other different USB boot devices that may be
> present, existing eMMC storage module possibly with ESP in addition to
> the inserted removable SD Card).

Well by storing the EFI vars on the SPI flash the user can erase one
of the storage options without necessarily impacting the EFI vars
without knowing.

The storage of EFI vars on the flash of the firmware is what you get
on x86 devices.

> B. Preventing the ambiguous selection of any additional removable
> storage containing an EFI System Partition from breaking the desired
> boot strategy. In many situations a user wants assurance that the board
> will boot exactly the same every time regardless of what additional
> removable media containing EFI System Partition might be connected.
>
> C. Corrupt or unusable selected EFI System Partition prevents any
> recovery and there is no runtime mechanism to attempt an alternative

No it doesn't there's EFI fall back options or you could erase the
vars from the u-boot cmd line etc.

> Is there a possible mechanism for adding a runtime check (GPIO state,
> key press, network event) to make the decision between some immutable
> boot preference, or a user-configured boot preference?
>
> Overall I want to get an understanding of how it might be recommended to
> make use of this for StarFive VisionFive 2 board target(s). Many hours
> of troubleshooting headache have been the result of EFI System Partition
> confusion on board that have a wide variety of storage subsystems (MMC,
> USB, NVMe, ...) which result in some really strange outcomes with
> installers of GNU/Linux OS distros;  particularly as the devicetree dtb
> search path currently points to storage subsystems that are typically
> removable media, or the failure mode where an OS installer is using the
> selected EFI System Partition that "leads on a path to nowhere" and is
> not the intended installation target.
>
> -E