[PATCH v2 00/17] efi_loader: non-volatile and runtime variables

[PATCH v2 00/17] efi_loader: non-volatile and runtime variables

[Heinrich Schuchardt]

Up to now UEFI variables where stored in U-Boot environment variables.
Saving UEFI variables was not possible without saving the U-Boot
environment variables.

With this patch series file ubootefi.var in the EFI system partition is
used for saving UEFI variables.

Furthermore the UEFI variables are exposed for reading at runtime.

v2:
	Rebase the series to consider OP-TEE based variables and
	authenticated variables.
	Do not enable SetVariable() at runtime as we cannot persist
	non-volatile variables at runtime.
	Display read-only attribute in printenv -e.

Heinrich Schuchardt (17):
  efi_loader: prepare for read only OP-TEE variables
  efi_loader: display RO attribute in printenv -e
  efi_loader: separate UEFI variable API from implemementation
  efi_loader: OsIndicationsSupported, PlatformLangCodes
  efi_loader: simplify boot manager
  efi_loader: keep attributes in efi_set_variable_int
  efi_loader: value of VendorKeys
  efi_loader: read-only AuditMode and DeployedMode
  efi_loader: secure boot flag
  efi_loader: UEFI variable persistence
  efi_loader: export efi_convert_pointer()
  efi_loader: optional pointer for ConvertPointer
  efi_loader: new function efi_memcpy_runtime()
  efi_loader: memory buffer for variables
  efi_loader: use memory based variable storage
  efi_loader: enable UEFI variables at runtime
  efi_selftest: adjust runtime test for variables

 cmd/nvedit_efi.c                              |  24 +-
 doc/api/efi.rst                               |   2 +
 include/efi_api.h                             |   2 +
 include/efi_loader.h                          |   6 +
 include/efi_variable.h                        | 198 +++++
 lib/efi_loader/Kconfig                        |   8 +
 lib/efi_loader/Makefile                       |   3 +
 lib/efi_loader/efi_bootmgr.c                  |  28 +-
 lib/efi_loader/efi_runtime.c                  |  35 +-
 lib/efi_loader/efi_setup.c                    |  59 +-
 lib/efi_loader/efi_var_common.c               | 140 +++
 lib/efi_loader/efi_var_file.c                 | 239 +++++
 lib/efi_loader/efi_var_mem.c                  | 266 ++++++
 lib/efi_loader/efi_variable.c                 | 823 ++++--------------
 lib/efi_loader/efi_variable_tee.c             | 130 +--
 .../efi_selftest_variables_runtime.c          |  13 +-
 16 files changed, 1154 insertions(+), 822 deletions(-)
 create mode 100644 include/efi_variable.h
 create mode 100644 lib/efi_loader/efi_var_common.c
 create mode 100644 lib/efi_loader/efi_var_file.c
 create mode 100644 lib/efi_loader/efi_var_mem.c

--
2.27.0

[PATCH v2 01/17] efi_loader: prepare for read only OP-TEE variables

[Heinrich Schuchardt]

We currently have two implementations of UEFI variables:

* variables provided via an OP-TEE module
* variables stored in the U-Boot environment

Read only variables are up to now only implemented in the U-Boot
environment implementation.

Provide a common interface for both implementations that allows handling
read-only variables.

As variable access is limited to very few source files put variable
related definitions into new include efi_variable.h instead of efi_loader.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 doc/api/efi.rst                   |   2 +
 include/efi_variable.h            |  43 ++++++++
 lib/efi_loader/Makefile           |   1 +
 lib/efi_loader/efi_var_common.c   |  78 +++++++++++++
 lib/efi_loader/efi_variable.c     | 175 ++++++++----------------------
 lib/efi_loader/efi_variable_tee.c |  75 ++++---------
 6 files changed, 193 insertions(+), 181 deletions(-)
 create mode 100644 include/efi_variable.h
 create mode 100644 lib/efi_loader/efi_var_common.c

diff --git a/doc/api/efi.rst b/doc/api/efi.rst
index d5114f05b3..cb2a1c897e 100644
--- a/doc/api/efi.rst
+++ b/doc/api/efi.rst
@@ -93,6 +93,8 @@ Runtime services
 Variable services
 ~~~~~~~~~~~~~~~~~

+.. kernel-doc:: include/efi_variable.h
+   :internal:
 .. kernel-doc:: lib/efi_loader/efi_variable.c
    :internal:

diff --git a/include/efi_variable.h b/include/efi_variable.h
new file mode 100644
index 0000000000..6789118eba
--- /dev/null
+++ b/include/efi_variable.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2020, Heinrich Schuchardt <xypron.glpk@gmx.de>
+ */
+
+#ifndef _EFI_VARIABLE_H
+#define _EFI_VARIABLE_H
+
+#include <linux/bitops.h>
+
+#define EFI_VARIABLE_READ_ONLY BIT(31)
+
+/**
+ * efi_get_variable() - retrieve value of a UEFI variable
+ *
+ * @variable_name:	name of the variable
+ * @vendor:		vendor GUID
+ * @attributes:		attributes of the variable
+ * @data_size:		size of the buffer to which the variable value is copied
+ * @data:		buffer to which the variable value is copied
+ * @timep:		authentication time (seconds since start of epoch)
+ * Return:		status code
+ */
+efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
+				  u32 *attributes, efi_uintn_t *data_size,
+				  void *data, u64 *timep);
+
+/**
+ * efi_set_variable() - set value of a UEFI variable
+ *
+ * @variable_name:	name of the variable
+ * @vendor:		vendor GUID
+ * @attributes:		attributes of the variable
+ * @data_size:		size of the buffer with the variable value
+ * @data:		buffer with the variable value
+ * @ro_check:		check the read only read only bit in attributes
+ * Return:		status code
+ */
+efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
+				  u32 attributes, efi_uintn_t data_size,
+				  const void *data, bool ro_check);
+
+#endif
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 57c7e66ea0..7eddd7ef37 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -35,6 +35,7 @@ obj-y += efi_root_node.o
 obj-y += efi_runtime.o
 obj-y += efi_setup.o
 obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += efi_unicode_collation.o
+obj-y += efi_var_common.o
 ifeq ($(CONFIG_EFI_MM_COMM_TEE),y)
 obj-y += efi_variable_tee.o
 else
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
new file mode 100644
index 0000000000..6a4efa3f27
--- /dev/null
+++ b/lib/efi_loader/efi_var_common.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * UEFI runtime variable services
+ *
+ * Copyright (c) 2020, Heinrich Schuchardt <xypron.glpk@gmx.de>
+ */
+
+#include <common.h>
+#include <efi_loader.h>
+#include <efi_variable.h>
+
+/**
+ * efi_efi_get_variable() - retrieve value of a UEFI variable
+ *
+ * This function implements the GetVariable runtime service.
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * @variable_name:	name of the variable
+ * @vendor:		vendor GUID
+ * @attributes:		attributes of the variable
+ * @data_size:		size of the buffer to which the variable value is copied
+ * @data:		buffer to which the variable value is copied
+ * Return:		status code
+ */
+efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
+				     const efi_guid_t *vendor, u32 *attributes,
+				     efi_uintn_t *data_size, void *data)
+{
+	efi_status_t ret;
+
+	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
+		  data_size, data);
+
+	ret = efi_get_variable_int(variable_name, vendor, attributes,
+				   data_size, data, NULL);
+
+	/* Remove EFI_VARIABLE_READ_ONLY flag */
+	if (attributes)
+		*attributes &= EFI_VARIABLE_MASK;
+
+	return EFI_EXIT(ret);
+}
+
+/**
+ * efi_set_variable() - set value of a UEFI variable
+ *
+ * This function implements the SetVariable runtime service.
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * @variable_name:	name of the variable
+ * @vendor:		vendor GUID
+ * @attributes:		attributes of the variable
+ * @data_size:		size of the buffer with the variable value
+ * @data:		buffer with the variable value
+ * Return:		status code
+ */
+efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
+				     const efi_guid_t *vendor, u32 attributes,
+				     efi_uintn_t data_size, const void *data)
+{
+	efi_status_t ret;
+
+	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
+		  data_size, data);
+
+	/* Make sure that the EFI_VARIABLE_READ_ONLY flag is not set */
+	if (attributes & ~(u32)EFI_VARIABLE_MASK)
+		ret = EFI_INVALID_PARAMETER;
+	else
+		ret = efi_set_variable_int(variable_name, vendor, attributes,
+					   data_size, data, true);
+
+	return EFI_EXIT(ret);
+}
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index efaba869ef..6ec1f97326 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -7,6 +7,7 @@

 #include <common.h>
 #include <efi_loader.h>
+#include <efi_variable.h>
 #include <env.h>
 #include <env_internal.h>
 #include <hexdump.h>
@@ -15,7 +16,6 @@
 #include <search.h>
 #include <uuid.h>
 #include <crypto/pkcs7_parser.h>
-#include <linux/bitops.h>
 #include <linux/compat.h>
 #include <u-boot/crc.h>

@@ -30,21 +30,6 @@ static bool efi_secure_boot;
 static enum efi_secure_mode efi_secure_mode;
 static u8 efi_vendor_keys;

-#define READ_ONLY BIT(31)
-
-static efi_status_t efi_get_variable_common(u16 *variable_name,
-					    const efi_guid_t *vendor,
-					    u32 *attributes,
-					    efi_uintn_t *data_size, void *data,
-					    u64 *timep);
-
-static efi_status_t efi_set_variable_common(u16 *variable_name,
-					    const efi_guid_t *vendor,
-					    u32 attributes,
-					    efi_uintn_t data_size,
-					    const void *data,
-					    bool ro_check);
-
 /*
  * Mapping between EFI variables and u-boot variables:
  *
@@ -155,7 +140,7 @@ static const char *parse_attr(const char *str, u32 *attrp, u64 *timep)
 		str++;

 		if ((s = prefix(str, "ro"))) {
-			attr |= READ_ONLY;
+			attr |= EFI_VARIABLE_READ_ONLY;
 		} else if ((s = prefix(str, "nv"))) {
 			attr |= EFI_VARIABLE_NON_VOLATILE;
 		} else if ((s = prefix(str, "boot"))) {
@@ -203,29 +188,29 @@ static efi_status_t efi_set_secure_state(u8 secure_boot, u8 setup_mode,

 	attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
 		     EFI_VARIABLE_RUNTIME_ACCESS |
-		     READ_ONLY;
-	ret = efi_set_variable_common(L"SecureBoot", &efi_global_variable_guid,
-				      attributes, sizeof(secure_boot),
-				      &secure_boot, false);
+		     EFI_VARIABLE_READ_ONLY;
+	ret = efi_set_variable_int(L"SecureBoot", &efi_global_variable_guid,
+				   attributes, sizeof(secure_boot),
+				   &secure_boot, false);
 	if (ret != EFI_SUCCESS)
 		goto err;

-	ret = efi_set_variable_common(L"SetupMode", &efi_global_variable_guid,
-				      attributes, sizeof(setup_mode),
-				      &setup_mode, false);
+	ret = efi_set_variable_int(L"SetupMode", &efi_global_variable_guid,
+				   attributes, sizeof(setup_mode),
+				   &setup_mode, false);
 	if (ret != EFI_SUCCESS)
 		goto err;

-	ret = efi_set_variable_common(L"AuditMode", &efi_global_variable_guid,
-				      attributes, sizeof(audit_mode),
-				      &audit_mode, false);
+	ret = efi_set_variable_int(L"AuditMode", &efi_global_variable_guid,
+				   attributes, sizeof(audit_mode),
+				   &audit_mode, false);
 	if (ret != EFI_SUCCESS)
 		goto err;

-	ret = efi_set_variable_common(L"DeployedMode",
-				      &efi_global_variable_guid, attributes,
-				      sizeof(deployed_mode), &deployed_mode,
-				      false);
+	ret = efi_set_variable_int(L"DeployedMode",
+				   &efi_global_variable_guid, attributes,
+				   sizeof(deployed_mode), &deployed_mode,
+				   false);
 err:
 	return ret;
 }
@@ -235,7 +220,7 @@ err:
  * @mode:	new state
  *
  * Depending on @mode, secure boot related variables are updated.
- * Those variables are *read-only* for users, efi_set_variable_common()
+ * Those variables are *read-only* for users, efi_set_variable_int()
  * is called here.
  *
  * Return:	status code
@@ -254,10 +239,10 @@ static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode)

 		efi_secure_boot = true;
 	} else if (mode == EFI_MODE_AUDIT) {
-		ret = efi_set_variable_common(L"PK", &efi_global_variable_guid,
-					      EFI_VARIABLE_BOOTSERVICE_ACCESS |
-					      EFI_VARIABLE_RUNTIME_ACCESS,
-					      0, NULL, false);
+		ret = efi_set_variable_int(L"PK", &efi_global_variable_guid,
+					   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+					   EFI_VARIABLE_RUNTIME_ACCESS,
+					   0, NULL, false);
 		if (ret != EFI_SUCCESS)
 			goto err;

@@ -309,8 +294,8 @@ static efi_status_t efi_init_secure_state(void)
 	 */

 	size = 0;
-	ret = efi_get_variable_common(L"PK", &efi_global_variable_guid,
-				      NULL, &size, NULL, NULL);
+	ret = efi_get_variable_int(L"PK", &efi_global_variable_guid,
+				   NULL, &size, NULL, NULL);
 	if (ret == EFI_BUFFER_TOO_SMALL) {
 		if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT))
 			mode = EFI_MODE_USER;
@@ -327,13 +312,13 @@ static efi_status_t efi_init_secure_state(void)

 	ret = efi_transfer_secure_state(mode);
 	if (ret == EFI_SUCCESS)
-		ret = efi_set_variable_common(L"VendorKeys",
-					      &efi_global_variable_guid,
-					      EFI_VARIABLE_BOOTSERVICE_ACCESS |
-					      EFI_VARIABLE_RUNTIME_ACCESS |
-					      READ_ONLY,
-					      sizeof(efi_vendor_keys),
-					      &efi_vendor_keys, false);
+		ret = efi_set_variable_int(L"VendorKeys",
+					   &efi_global_variable_guid,
+					   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+					   EFI_VARIABLE_RUNTIME_ACCESS |
+					   EFI_VARIABLE_READ_ONLY,
+					   sizeof(efi_vendor_keys),
+					   &efi_vendor_keys, false);

 err:
 	return ret;
@@ -599,11 +584,9 @@ static efi_status_t efi_variable_authenticate(u16 *variable,
 }
 #endif /* CONFIG_EFI_SECURE_BOOT */

-static efi_status_t efi_get_variable_common(u16 *variable_name,
-					    const efi_guid_t *vendor,
-					    u32 *attributes,
-					    efi_uintn_t *data_size, void *data,
-					    u64 *timep)
+efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
+				  u32 *attributes, efi_uintn_t *data_size,
+				  void *data, u64 *timep)
 {
 	char *native_name;
 	efi_status_t ret;
@@ -684,40 +667,11 @@ static efi_status_t efi_get_variable_common(u16 *variable_name,

 out:
 	if (attributes)
-		*attributes = attr & EFI_VARIABLE_MASK;
+		*attributes = attr;

 	return ret;
 }

-/**
- * efi_efi_get_variable() - retrieve value of a UEFI variable
- *
- * This function implements the GetVariable runtime service.
- *
- * See the Unified Extensible Firmware Interface (UEFI) specification for
- * details.
- *
- * @variable_name:	name of the variable
- * @vendor:		vendor GUID
- * @attributes:		attributes of the variable
- * @data_size:		size of the buffer to which the variable value is copied
- * @data:		buffer to which the variable value is copied
- * Return:		status code
- */
-efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
-				     const efi_guid_t *vendor, u32 *attributes,
-				     efi_uintn_t *data_size, void *data)
-{
-	efi_status_t ret;
-
-	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
-		  data_size, data);
-
-	ret = efi_get_variable_common(variable_name, vendor, attributes,
-				      data_size, data, NULL);
-	return EFI_EXIT(ret);
-}
-
 static char *efi_variables_list;
 static char *efi_cur_variable;

@@ -881,12 +835,9 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
 	return EFI_EXIT(ret);
 }

-static efi_status_t efi_set_variable_common(u16 *variable_name,
-					    const efi_guid_t *vendor,
-					    u32 attributes,
-					    efi_uintn_t data_size,
-					    const void *data,
-					    bool ro_check)
+efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
+				  u32 attributes, efi_uintn_t data_size,
+				  const void *data, bool ro_check)
 {
 	char *native_name = NULL, *old_data = NULL, *val = NULL, *s;
 	efi_uintn_t old_size;
@@ -909,15 +860,15 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
 	/* check if a variable exists */
 	old_size = 0;
 	attr = 0;
-	ret = efi_get_variable_common(variable_name, vendor, &attr,
-				      &old_size, NULL, &time);
+	ret = efi_get_variable_int(variable_name, vendor, &attr,
+				   &old_size, NULL, &time);
 	append = !!(attributes & EFI_VARIABLE_APPEND_WRITE);
 	attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE;
 	delete = !append && (!data_size || !attributes);

 	/* check attributes */
 	if (old_size) {
-		if (ro_check && (attr & READ_ONLY)) {
+		if (ro_check && (attr & EFI_VARIABLE_READ_ONLY)) {
 			ret = EFI_WRITE_PROTECTED;
 			goto err;
 		}
@@ -925,8 +876,8 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
 		/* attributes won't be changed */
 		if (!delete &&
 		    ((ro_check && attr != attributes) ||
-		     (!ro_check && ((attr & ~(u32)READ_ONLY)
-				    != (attributes & ~(u32)READ_ONLY))))) {
+		     (!ro_check && ((attr & ~(u32)EFI_VARIABLE_READ_ONLY)
+				    != (attributes & ~(u32)EFI_VARIABLE_READ_ONLY))))) {
 			ret = EFI_INVALID_PARAMETER;
 			goto err;
 		}
@@ -1000,8 +951,8 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
 			ret = EFI_OUT_OF_RESOURCES;
 			goto err;
 		}
-		ret = efi_get_variable_common(variable_name, vendor,
-					      &attr, &old_size, old_data, NULL);
+		ret = efi_get_variable_int(variable_name, vendor,
+					   &attr, &old_size, old_data, NULL);
 		if (ret != EFI_SUCCESS)
 			goto err;
 	} else {
@@ -1021,7 +972,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
 	/*
 	 * store attributes
 	 */
-	attributes &= (READ_ONLY |
+	attributes &= (EFI_VARIABLE_READ_ONLY |
 		       EFI_VARIABLE_NON_VOLATILE |
 		       EFI_VARIABLE_BOOTSERVICE_ACCESS |
 		       EFI_VARIABLE_RUNTIME_ACCESS |
@@ -1030,7 +981,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
 	while (attributes) {
 		attr = 1 << (ffs(attributes) - 1);

-		if (attr == READ_ONLY) {
+		if (attr == EFI_VARIABLE_READ_ONLY) {
 			s += sprintf(s, "ro");
 		} else if (attr == EFI_VARIABLE_NON_VOLATILE) {
 			s += sprintf(s, "nv");
@@ -1084,12 +1035,12 @@ out:
 		/* update VendorKeys */
 		if (vendor_keys_modified & efi_vendor_keys) {
 			efi_vendor_keys = 0;
-			ret = efi_set_variable_common(
+			ret = efi_set_variable_int(
 						L"VendorKeys",
 						&efi_global_variable_guid,
 						EFI_VARIABLE_BOOTSERVICE_ACCESS
 						 | EFI_VARIABLE_RUNTIME_ACCESS
-						 | READ_ONLY,
+						 | EFI_VARIABLE_READ_ONLY,
 						sizeof(efi_vendor_keys),
 						&efi_vendor_keys,
 						false);
@@ -1106,36 +1057,6 @@ err:
 	return ret;
 }

-/**
- * efi_set_variable() - set value of a UEFI variable
- *
- * This function implements the SetVariable runtime service.
- *
- * See the Unified Extensible Firmware Interface (UEFI) specification for
- * details.
- *
- * @variable_name:	name of the variable
- * @vendor:		vendor GUID
- * @attributes:		attributes of the variable
- * @data_size:		size of the buffer with the variable value
- * @data:		buffer with the variable value
- * Return:		status code
- */
-efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
-				     const efi_guid_t *vendor, u32 attributes,
-				     efi_uintn_t data_size, const void *data)
-{
-	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
-		  data_size, data);
-
-	/* READ_ONLY bit is not part of API */
-	attributes &= ~(u32)READ_ONLY;
-
-	return EFI_EXIT(efi_set_variable_common(variable_name, vendor,
-						attributes, data_size, data,
-						true));
-}
-
 /**
  * efi_query_variable_info() - get information about EFI variables
  *
diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
index cacc76e23d..d5a28163f5 100644
--- a/lib/efi_loader/efi_variable_tee.c
+++ b/lib/efi_loader/efi_variable_tee.c
@@ -10,6 +10,7 @@
 #include <efi.h>
 #include <efi_api.h>
 #include <efi_loader.h>
+#include <efi_variable.h>
 #include <tee.h>
 #include <malloc.h>
 #include <mm_communication.h>
@@ -243,24 +244,9 @@ out:
 	return ret;
 }

-/**
- * efi_get_variable() - retrieve value of a UEFI variable
- *
- * This function implements the GetVariable runtime service.
- *
- * See the Unified Extensible Firmware Interface (UEFI) specification for
- * details.
- *
- * @name:		name of the variable
- * @guid:		vendor GUID
- * @attr:		attributes of the variable
- * @data_size:		size of the buffer to which the variable value is copied
- * @data:		buffer to which the variable value is copied
- * Return:		status code
- */
-efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
-				     u32 *attr, efi_uintn_t *data_size,
-				     void *data)
+efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
+				  u32 *attributes, efi_uintn_t *data_size,
+				  void *data, u64 *timep)
 {
 	struct smm_variable_access *var_acc;
 	efi_uintn_t payload_size;
@@ -269,15 +255,13 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
 	u8 *comm_buf = NULL;
 	efi_status_t ret;

-	EFI_ENTRY("\"%ls\" %pUl %p %p %p", name, guid, attr, data_size, data);
-
-	if (!name || !guid || !data_size) {
+	if (!variable_name || !vendor || !data_size) {
 		ret = EFI_INVALID_PARAMETER;
 		goto out;
 	}

 	/* Check payload size */
-	name_size = u16_strsize(name);
+	name_size = u16_strsize(variable_name);
 	if (name_size > max_payload_size - MM_VARIABLE_ACCESS_HEADER_SIZE) {
 		ret = EFI_INVALID_PARAMETER;
 		goto out;
@@ -300,11 +284,11 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
 		goto out;

 	/* Fill in contents */
-	guidcpy(&var_acc->guid, guid);
+	guidcpy(&var_acc->guid, vendor);
 	var_acc->data_size = tmp_dsize;
 	var_acc->name_size = name_size;
-	var_acc->attr = attr ? *attr : 0;
-	memcpy(var_acc->name, name, name_size);
+	var_acc->attr = attributes ? *attributes : 0;
+	memcpy(var_acc->name, variable_name, name_size);

 	/* Communicate */
 	ret = mm_communicate(comm_buf, payload_size);
@@ -315,8 +299,8 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
 	if (ret != EFI_SUCCESS)
 		goto out;

-	if (attr)
-		*attr = var_acc->attr;
+	if (attributes)
+		*attributes = var_acc->attr;
 	if (data)
 		memcpy(data, (u8 *)var_acc->name + var_acc->name_size,
 		       var_acc->data_size);
@@ -325,7 +309,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,

 out:
 	free(comm_buf);
-	return EFI_EXIT(ret);
+	return ret;
 }

 /**
@@ -417,24 +401,9 @@ out:
 	return EFI_EXIT(ret);
 }

-/**
- * efi_set_variable() - set value of a UEFI variable
- *
- * This function implements the SetVariable runtime service.
- *
- * See the Unified Extensible Firmware Interface (UEFI) specification for
- * details.
- *
- * @name:		name of the variable
- * @guid:		vendor GUID
- * @attr:		attributes of the variable
- * @data_size:		size of the buffer with the variable value
- * @data:		buffer with the variable value
- * Return:		status code
- */
-efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
-				     u32 attr, efi_uintn_t data_size,
-				     const void *data)
+efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
+				  u32 attributes, efi_uintn_t data_size,
+				  const void *data, bool ro_check)
 {
 	struct smm_variable_access *var_acc;
 	efi_uintn_t payload_size;
@@ -442,9 +411,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
 	u8 *comm_buf = NULL;
 	efi_status_t ret;

-	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", name, guid, attr, data_size, data);
-
-	if (!name || name[0] == 0 || !guid) {
+	if (!variable_name || variable_name[0] == 0 || !vendor) {
 		ret = EFI_INVALID_PARAMETER;
 		goto out;
 	}
@@ -454,7 +421,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
 	}

 	/* Check payload size */
-	name_size = u16_strsize(name);
+	name_size = u16_strsize(variable_name);
 	payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + data_size;
 	if (payload_size > max_payload_size) {
 		ret = EFI_INVALID_PARAMETER;
@@ -468,11 +435,11 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
 		goto out;

 	/* Fill in contents */
-	guidcpy(&var_acc->guid, guid);
+	guidcpy(&var_acc->guid, vendor);
 	var_acc->data_size = data_size;
 	var_acc->name_size = name_size;
-	var_acc->attr = attr;
-	memcpy(var_acc->name, name, name_size);
+	var_acc->attr = attributes;
+	memcpy(var_acc->name, variable_name, name_size);
 	memcpy((u8 *)var_acc->name + name_size, data, data_size);

 	/* Communicate */
@@ -480,7 +447,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,

 out:
 	free(comm_buf);
-	return EFI_EXIT(ret);
+	return ret;
 }

 /**
--
2.27.0

[PATCH v2 02/17] efi_loader: display RO attribute in printenv -e

[Heinrich Schuchardt]

Let the 'printenv -e' command display the read only flag.
If the variable is time authenticated write the time stamp.

Avoid EFI_CALL() when calling SetVariable() and GetVariable().

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 cmd/nvedit_efi.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/cmd/nvedit_efi.c b/cmd/nvedit_efi.c
index 29cad38e19..3f61d5d6cc 100644
--- a/cmd/nvedit_efi.c
+++ b/cmd/nvedit_efi.c
@@ -9,11 +9,13 @@
 #include <common.h>
 #include <command.h>
 #include <efi_loader.h>
+#include <efi_variable.h>
 #include <env.h>
 #include <exports.h>
 #include <hexdump.h>
 #include <malloc.h>
 #include <mapmem.h>
+#include <rtc.h>
 #include <uuid.h>
 #include <linux/kernel.h>

@@ -34,6 +36,7 @@ static const struct {
 	{EFI_VARIABLE_RUNTIME_ACCESS, "RT"},
 	{EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS, "AW"},
 	{EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, "AT"},
+	{EFI_VARIABLE_READ_ONLY, "RO"},
 };

 static const struct {
@@ -87,20 +90,22 @@ static void efi_dump_single_var(u16 *name, const efi_guid_t *guid, bool verbose)
 {
 	u32 attributes;
 	u8 *data;
+	u64 time;
+	struct rtc_time tm;
 	efi_uintn_t size;
 	int count, i;
 	efi_status_t ret;

 	data = NULL;
 	size = 0;
-	ret = EFI_CALL(efi_get_variable(name, guid, &attributes, &size, data));
+	ret = efi_get_variable_int(name, guid, &attributes, &size, data, &time);
 	if (ret == EFI_BUFFER_TOO_SMALL) {
 		data = malloc(size);
 		if (!data)
 			goto out;

-		ret = EFI_CALL(efi_get_variable(name, guid, &attributes, &size,
-						data));
+		ret = efi_get_variable_int(name, guid, &attributes, &size,
+					   data, &time);
 	}
 	if (ret == EFI_NOT_FOUND) {
 		printf("Error: \"%ls\" not defined\n", name);
@@ -109,13 +114,16 @@ static void efi_dump_single_var(u16 *name, const efi_guid_t *guid, bool verbose)
 	if (ret != EFI_SUCCESS)
 		goto out;

-	printf("%ls:\n    %s:", name, efi_guid_to_str(guid));
+	rtc_to_tm(time, &tm);
+	printf("%ls:\n    %s:\n", name, efi_guid_to_str(guid));
+	if (attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)
+		printf("    %04d-%02d-%02d %02d:%02d:%02d\n", tm.tm_year,
+		       tm.tm_mon, tm.tm_mday, tm.tm_hour, tm.tm_min, tm.tm_sec);
+	printf("    ");
 	for (count = 0, i = 0; i < ARRAY_SIZE(efi_var_attrs); i++)
 		if (attributes & efi_var_attrs[i].mask) {
 			if (count)
 				putc('|');
-			else
-				putc(' ');
 			count++;
 			puts(efi_var_attrs[i].text);
 		}
@@ -592,8 +600,8 @@ int do_env_set_efi(struct cmd_tbl *cmdtp, int flag, int argc,
 	p = var_name16;
 	utf8_utf16_strncpy(&p, var_name, len + 1);

-	ret = EFI_CALL(efi_set_variable(var_name16, &guid, attributes,
-					size, value));
+	ret = efi_set_variable_int(var_name16, &guid, attributes, size, value,
+				   true);
 	unmap_sysmem(value);
 	if (ret == EFI_SUCCESS) {
 		ret = CMD_RET_SUCCESS;
--
2.27.0

[PATCH v2 03/17] efi_loader: separate UEFI variable API from implemementation

[Heinrich Schuchardt]

Separate the remaining UEFI variable API functions GetNextVariableName and
QueryVariableInfo() from internal functions implementing them.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_variable.h            | 39 +++++++++++++++++++
 lib/efi_loader/efi_var_common.c   | 62 +++++++++++++++++++++++++++++++
 lib/efi_loader/efi_variable.c     | 57 ++++++++++++----------------
 lib/efi_loader/efi_variable_tee.c | 55 +++++----------------------
 4 files changed, 134 insertions(+), 79 deletions(-)

diff --git a/include/efi_variable.h b/include/efi_variable.h
index 6789118eba..3ba274fce1 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -40,4 +40,43 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
 				  u32 attributes, efi_uintn_t data_size,
 				  const void *data, bool ro_check);

+/**
+ * efi_get_next_variable_name_int() - enumerate the current variable names
+ *
+ * @variable_name_size:	size of variable_name buffer in byte
+ * @variable_name:	name of uefi variable's name in u16
+ * @vendor:		vendor's guid
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * Return: status code
+ */
+efi_status_t efi_get_next_variable_name_int(efi_uintn_t *variable_name_size,
+					    u16 *variable_name,
+					    efi_guid_t *vendor);
+
+/**
+ * efi_query_variable_info_int() - get information about EFI variables
+ *
+ * This function implements the QueryVariableInfo() runtime service.
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * @attributes:				bitmask to select variables to be
+ *					queried
+ * @maximum_variable_storage_size:	maximum size of storage area for the
+ *					selected variable types
+ * @remaining_variable_storage_size:	remaining size of storage are for the
+ *					selected variable types
+ * @maximum_variable_size:		maximum size of a variable of the
+ *					selected type
+ * Returns:				status code
+ */
+efi_status_t efi_query_variable_info_int(u32 attributes,
+					 u64 *maximum_variable_storage_size,
+					 u64 *remaining_variable_storage_size,
+					 u64 *maximum_variable_size);
+
 #endif
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index 6a4efa3f27..1e2be1135b 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -76,3 +76,65 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,

 	return EFI_EXIT(ret);
 }
+
+/**
+ * efi_get_next_variable_name() - enumerate the current variable names
+ *
+ * @variable_name_size:	size of variable_name buffer in byte
+ * @variable_name:	name of uefi variable's name in u16
+ * @vendor:		vendor's guid
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * Return: status code
+ */
+efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
+					       u16 *variable_name,
+					       efi_guid_t *vendor)
+{
+	efi_status_t ret;
+
+	EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, vendor);
+
+	ret = efi_get_next_variable_name_int(variable_name_size, variable_name,
+					     vendor);
+
+	return EFI_EXIT(ret);
+}
+
+/**
+ * efi_query_variable_info() - get information about EFI variables
+ *
+ * This function implements the QueryVariableInfo() runtime service.
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * @attributes:				bitmask to select variables to be
+ *					queried
+ * @maximum_variable_storage_size:	maximum size of storage area for the
+ *					selected variable types
+ * @remaining_variable_storage_size:	remaining size of storage are for the
+ *					selected variable types
+ * @maximum_variable_size:		maximum size of a variable of the
+ *					selected type
+ * Returns:				status code
+ */
+efi_status_t EFIAPI efi_query_variable_info(
+			u32 attributes, u64 *maximum_variable_storage_size,
+			u64 *remaining_variable_storage_size,
+			u64 *maximum_variable_size)
+{
+	efi_status_t ret;
+
+	EFI_ENTRY("%x %p %p %p", attributes, maximum_variable_storage_size,
+		  remaining_variable_storage_size, maximum_variable_size);
+
+	ret = efi_query_variable_info_int(attributes,
+					  maximum_variable_storage_size,
+					  remaining_variable_storage_size,
+					  maximum_variable_size);
+
+	return EFI_EXIT(ret);
+}
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 6ec1f97326..6706438b26 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -745,23 +745,9 @@ static efi_status_t parse_uboot_variable(char *variable,
 	return EFI_SUCCESS;
 }

-/**
- * efi_get_next_variable_name() - enumerate the current variable names
- *
- * @variable_name_size:	size of variable_name buffer in byte
- * @variable_name:	name of uefi variable's name in u16
- * @vendor:		vendor's guid
- *
- * This function implements the GetNextVariableName service.
- *
- * See the Unified Extensible Firmware Interface (UEFI) specification for
- * details.
- *
- * Return: status code
- */
-efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
-					       u16 *variable_name,
-					       efi_guid_t *vendor)
+efi_status_t efi_get_next_variable_name_int(efi_uintn_t *variable_name_size,
+					    u16 *variable_name,
+					    efi_guid_t *vendor)
 {
 	char *native_name, *variable;
 	ssize_t name_len, list_len;
@@ -771,10 +757,8 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
 	int i;
 	efi_status_t ret;

-	EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, vendor);
-
 	if (!variable_name_size || !variable_name || !vendor)
-		return EFI_EXIT(EFI_INVALID_PARAMETER);
+		return EFI_INVALID_PARAMETER;

 	if (variable_name[0]) {
 		/* check null-terminated string */
@@ -782,12 +766,12 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
 			if (!variable_name[i])
 				break;
 		if (i >= *variable_name_size)
-			return EFI_EXIT(EFI_INVALID_PARAMETER);
+			return EFI_INVALID_PARAMETER;

 		/* search for the last-returned variable */
 		ret = efi_to_native(&native_name, variable_name, vendor);
 		if (ret)
-			return EFI_EXIT(ret);
+			return ret;

 		name_len = strlen(native_name);
 		for (variable = efi_variables_list; variable && *variable;) {
@@ -802,14 +786,14 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,

 		free(native_name);
 		if (!(variable && *variable))
-			return EFI_EXIT(EFI_INVALID_PARAMETER);
+			return EFI_INVALID_PARAMETER;

 		/* next variable */
 		variable = strchr(variable, '\n');
 		if (variable)
 			variable++;
 		if (!(variable && *variable))
-			return EFI_EXIT(EFI_NOT_FOUND);
+			return EFI_NOT_FOUND;
 	} else {
 		/*
 		 *new search: free a list used in the previous search
@@ -824,7 +808,7 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
 				     &efi_variables_list, 0, 1, regexlist);

 		if (list_len <= 1)
-			return EFI_EXIT(EFI_NOT_FOUND);
+			return EFI_NOT_FOUND;

 		variable = efi_variables_list;
 	}
@@ -832,7 +816,7 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
 	ret = parse_uboot_variable(variable, variable_name_size, variable_name,
 				   vendor, &attributes);

-	return EFI_EXIT(ret);
+	return ret;
 }

 efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
@@ -1057,13 +1041,17 @@ err:
 	return ret;
 }

+efi_status_t efi_query_variable_info_int(u32 attributes,
+					 u64 *maximum_variable_storage_size,
+					 u64 *remaining_variable_storage_size,
+					 u64 *maximum_variable_size)
+{
+	return EFI_UNSUPPORTED;
+}
+
 /**
- * efi_query_variable_info() - get information about EFI variables
- *
- * This function implements the QueryVariableInfo() runtime service.
- *
- * See the Unified Extensible Firmware Interface (UEFI) specification for
- * details.
+ * efi_query_variable_info_runtime() - runtime implementation of
+ *				       QueryVariableInfo()
  *
  * @attributes:				bitmask to select variables to be
  *					queried
@@ -1075,7 +1063,7 @@ err:
  *					selected type
  * Returns:				status code
  */
-efi_status_t __efi_runtime EFIAPI efi_query_variable_info(
+efi_status_t __efi_runtime EFIAPI efi_query_variable_info_runtime(
 			u32 attributes,
 			u64 *maximum_variable_storage_size,
 			u64 *remaining_variable_storage_size,
@@ -1144,6 +1132,9 @@ void efi_variables_boot_exit_notify(void)
 	efi_runtime_services.get_next_variable_name =
 				efi_get_next_variable_name_runtime;
 	efi_runtime_services.set_variable = efi_set_variable_runtime;
+	efi_runtime_services.set_variable = efi_set_variable_runtime;
+	efi_runtime_services.query_variable_info =
+				efi_query_variable_info_runtime;
 	efi_update_table_header_crc32(&efi_runtime_services.hdr);
 }

diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
index d5a28163f5..abe7941cbc 100644
--- a/lib/efi_loader/efi_variable_tee.c
+++ b/lib/efi_loader/efi_variable_tee.c
@@ -312,23 +312,9 @@ out:
 	return ret;
 }

-/**
- * efi_get_next_variable_name() - enumerate the current variable names
- *
- * @variable_name_size:	size of variable_name buffer in bytes
- * @variable_name:	name of uefi variable's name in u16
- * @guid:		vendor's guid
- *
- * This function implements the GetNextVariableName service.
- *
- * See the Unified Extensible Firmware Interface (UEFI) specification for
- * details.
- *
- * Return: status code
- */
-efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
-					       u16 *variable_name,
-					       efi_guid_t *guid)
+efi_status_t efi_get_next_variable_name_int(efi_uintn_t *variable_name_size,
+					    u16 *variable_name,
+					    efi_guid_t *guid)
 {
 	struct smm_variable_getnext *var_getnext;
 	efi_uintn_t payload_size;
@@ -339,8 +325,6 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
 	u8 *comm_buf = NULL;
 	efi_status_t ret;

-	EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, guid);
-
 	if (!variable_name_size || !variable_name || !guid) {
 		ret = EFI_INVALID_PARAMETER;
 		goto out;
@@ -398,7 +382,7 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,

 out:
 	free(comm_buf);
-	return EFI_EXIT(ret);
+	return ret;
 }

 efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
@@ -450,37 +434,16 @@ out:
 	return ret;
 }

-/**
- * efi_query_variable_info() - get information about EFI variables
- *
- * This function implements the QueryVariableInfo() runtime service.
- *
- * See the Unified Extensible Firmware Interface (UEFI) specification for
- * details.
- *
- * @attributes:				bitmask to select variables to be
- *					queried
- * @maximum_variable_storage_size:	maximum size of storage area for the
- *					selected variable types
- * @remaining_variable_storage_size:	remaining size of storage are for the
- *					selected variable types
- * @maximum_variable_size:		maximum size of a variable of the
- *					selected type
- * Returns:				status code
- */
-efi_status_t EFIAPI __efi_runtime
-efi_query_variable_info(u32 attributes, u64 *max_variable_storage_size,
-			u64 *remain_variable_storage_size,
-			u64 *max_variable_size)
+efi_status_t efi_query_variable_info_int(u32 attributes,
+					 u64 *max_variable_storage_size,
+					 u64 *remain_variable_storage_size,
+					 u64 *max_variable_size)
 {
 	struct smm_variable_query_info *mm_query_info;
 	efi_uintn_t payload_size;
 	efi_status_t ret;
 	u8 *comm_buf;

-	EFI_ENTRY("%x %p %p %p", attributes, max_variable_storage_size,
-		  remain_variable_storage_size, max_variable_size);
-
 	payload_size = sizeof(*mm_query_info);
 	comm_buf = setup_mm_hdr((void **)&mm_query_info, payload_size,
 				SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO,
@@ -499,7 +462,7 @@ efi_query_variable_info(u32 attributes, u64 *max_variable_storage_size,

 out:
 	free(comm_buf);
-	return EFI_EXIT(ret);
+	return ret;
 }

 /**
--
2.27.0

[PATCH v2 04/17] efi_loader: OsIndicationsSupported, PlatformLangCodes

[Heinrich Schuchardt]

UEFI variables OsIndicationsSupported, PlatformLangCodes should be read
only.

Avoid EFI_CALL() for SetVariable().

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_setup.c | 59 ++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 28 deletions(-)

diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index a3b05a4a9b..6196c0a06c 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -8,6 +8,7 @@
 #include <common.h>
 #include <bootm.h>
 #include <efi_loader.h>
+#include <efi_variable.h>

 #define OBJ_LIST_NOT_INITIALIZED 1

@@ -40,12 +41,13 @@ static efi_status_t efi_init_platform_lang(void)
 	 * Variable PlatformLangCodes defines the language codes that the
 	 * machine can support.
 	 */
-	ret = EFI_CALL(efi_set_variable(L"PlatformLangCodes",
-					&efi_global_variable_guid,
-					EFI_VARIABLE_BOOTSERVICE_ACCESS |
-					EFI_VARIABLE_RUNTIME_ACCESS,
-					sizeof(CONFIG_EFI_PLATFORM_LANG_CODES),
-					CONFIG_EFI_PLATFORM_LANG_CODES));
+	ret = efi_set_variable_int(L"PlatformLangCodes",
+				   &efi_global_variable_guid,
+				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				   EFI_VARIABLE_RUNTIME_ACCESS |
+				   EFI_VARIABLE_READ_ONLY,
+				   sizeof(CONFIG_EFI_PLATFORM_LANG_CODES),
+				   CONFIG_EFI_PLATFORM_LANG_CODES, false);
 	if (ret != EFI_SUCCESS)
 		goto out;

@@ -53,9 +55,9 @@ static efi_status_t efi_init_platform_lang(void)
 	 * Variable PlatformLang defines the language that the machine has been
 	 * configured for.
 	 */
-	ret = EFI_CALL(efi_get_variable(L"PlatformLang",
-					&efi_global_variable_guid,
-					NULL, &data_size, &pos));
+	ret = efi_get_variable_int(L"PlatformLang",
+				   &efi_global_variable_guid,
+				   NULL, &data_size, &pos, NULL);
 	if (ret == EFI_BUFFER_TOO_SMALL) {
 		/* The variable is already set. Do not change it. */
 		ret = EFI_SUCCESS;
@@ -70,12 +72,12 @@ static efi_status_t efi_init_platform_lang(void)
 	if (pos)
 		*pos = 0;

-	ret = EFI_CALL(efi_set_variable(L"PlatformLang",
-					&efi_global_variable_guid,
-					EFI_VARIABLE_NON_VOLATILE |
-					EFI_VARIABLE_BOOTSERVICE_ACCESS |
-					EFI_VARIABLE_RUNTIME_ACCESS,
-					1 + strlen(lang), lang));
+	ret = efi_set_variable_int(L"PlatformLang",
+				   &efi_global_variable_guid,
+				   EFI_VARIABLE_NON_VOLATILE |
+				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				   EFI_VARIABLE_RUNTIME_ACCESS,
+				   1 + strlen(lang), lang, false);
 out:
 	if (ret != EFI_SUCCESS)
 		printf("EFI: cannot initialize platform language settings\n");
@@ -96,13 +98,13 @@ static efi_status_t efi_init_secure_boot(void)
 	};
 	efi_status_t ret;

-	/* TODO: read-only */
-	ret = EFI_CALL(efi_set_variable(L"SignatureSupport",
-					&efi_global_variable_guid,
-					EFI_VARIABLE_BOOTSERVICE_ACCESS
-					 | EFI_VARIABLE_RUNTIME_ACCESS,
-					sizeof(signature_types),
-					&signature_types));
+	ret = efi_set_variable_int(L"SignatureSupport",
+				   &efi_global_variable_guid,
+				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				   EFI_VARIABLE_RUNTIME_ACCESS |
+				   EFI_VARIABLE_READ_ONLY,
+				   sizeof(signature_types),
+				   &signature_types, false);
 	if (ret != EFI_SUCCESS)
 		printf("EFI: cannot initialize SignatureSupport variable\n");

@@ -160,12 +162,13 @@ efi_status_t efi_init_obj_list(void)
 		goto out;

 	/* Indicate supported features */
-	ret = EFI_CALL(efi_set_variable(L"OsIndicationsSupported",
-					&efi_global_variable_guid,
-					EFI_VARIABLE_BOOTSERVICE_ACCESS |
-					EFI_VARIABLE_RUNTIME_ACCESS,
-					sizeof(os_indications_supported),
-					&os_indications_supported));
+	ret = efi_set_variable_int(L"OsIndicationsSupported",
+				   &efi_global_variable_guid,
+				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				   EFI_VARIABLE_RUNTIME_ACCESS |
+				   EFI_VARIABLE_READ_ONLY,
+				   sizeof(os_indications_supported),
+				   &os_indications_supported, false);
 	if (ret != EFI_SUCCESS)
 		goto out;

--
2.27.0

[PATCH v2 05/17] efi_loader: simplify boot manager

[Heinrich Schuchardt]

Simplify the implementation of the UEFI boot manager:

* avoid EFI_CALL for SetVariable() and GetVariable()
* remove unnecessary type conversions

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_bootmgr.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index e268e9c4b8..e03198b57a 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -12,6 +12,7 @@
 #include <log.h>
 #include <malloc.h>
 #include <efi_loader.h>
+#include <efi_variable.h>
 #include <asm/unaligned.h>

 static const struct efi_boot_services *bs;
@@ -147,15 +148,14 @@ unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data)
 static void *get_var(u16 *name, const efi_guid_t *vendor,
 		     efi_uintn_t *size)
 {
-	efi_guid_t *v = (efi_guid_t *)vendor;
 	efi_status_t ret;
 	void *buf = NULL;

 	*size = 0;
-	EFI_CALL(ret = rs->get_variable(name, v, NULL, size, buf));
+	ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
 	if (ret == EFI_BUFFER_TOO_SMALL) {
 		buf = malloc(*size);
-		EFI_CALL(ret = rs->get_variable(name, v, NULL, size, buf));
+		ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
 	}

 	if (ret != EFI_SUCCESS) {
@@ -219,10 +219,9 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle)
 		attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
 			     EFI_VARIABLE_RUNTIME_ACCESS;
 		size = sizeof(n);
-		ret = EFI_CALL(efi_set_variable(
-				L"BootCurrent",
-				(efi_guid_t *)&efi_global_variable_guid,
-				attributes, size, &n));
+		ret = efi_set_variable_int(L"BootCurrent",
+					   &efi_global_variable_guid,
+					   attributes, size, &n, false);
 		if (ret != EFI_SUCCESS) {
 			if (EFI_CALL(efi_unload_image(*handle))
 			    != EFI_SUCCESS)
@@ -262,22 +261,19 @@ efi_status_t efi_bootmgr_load(efi_handle_t *handle)
 	rs = systab.runtime;

 	/* BootNext */
-	bootnext = 0;
 	size = sizeof(bootnext);
-	ret = EFI_CALL(efi_get_variable(L"BootNext",
-					(efi_guid_t *)&efi_global_variable_guid,
-					NULL, &size, &bootnext));
+	ret = efi_get_variable_int(L"BootNext",
+				   &efi_global_variable_guid,
+				   NULL, &size, &bootnext, NULL);
 	if (ret == EFI_SUCCESS || ret == EFI_BUFFER_TOO_SMALL) {
 		/* BootNext does exist here */
 		if (ret == EFI_BUFFER_TOO_SMALL || size != sizeof(u16))
 			log_err("BootNext must be 16-bit integer\n");

 		/* delete BootNext */
-		ret = EFI_CALL(efi_set_variable(
-					L"BootNext",
-					(efi_guid_t *)&efi_global_variable_guid,
-					EFI_VARIABLE_NON_VOLATILE, 0,
-					&bootnext));
+		ret = efi_set_variable_int(L"BootNext",
+					   &efi_global_variable_guid,
+					   0, 0, NULL, false);

 		/* load BootNext */
 		if (ret == EFI_SUCCESS) {
--
2.27.0

[PATCH v2 06/17] efi_loader: keep attributes in efi_set_variable_int

[Heinrich Schuchardt]

Do not change the value of parameter attributes in function
efi_set_variable_int(). This allows to use it later.

Do not use variable attr for different purposes but declare separate
variables (attr and old_attr).

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_variable.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 6706438b26..a7de0b4022 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -827,7 +827,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
 	efi_uintn_t old_size;
 	bool append, delete;
 	u64 time = 0;
-	u32 attr;
+	u32 old_attr;
 	efi_status_t ret = EFI_SUCCESS;

 	if (!variable_name || !*variable_name || !vendor ||
@@ -843,8 +843,8 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,

 	/* check if a variable exists */
 	old_size = 0;
-	attr = 0;
-	ret = efi_get_variable_int(variable_name, vendor, &attr,
+	old_attr = 0;
+	ret = efi_get_variable_int(variable_name, vendor, &old_attr,
 				   &old_size, NULL, &time);
 	append = !!(attributes & EFI_VARIABLE_APPEND_WRITE);
 	attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE;
@@ -852,15 +852,15 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,

 	/* check attributes */
 	if (old_size) {
-		if (ro_check && (attr & EFI_VARIABLE_READ_ONLY)) {
+		if (ro_check && (old_attr & EFI_VARIABLE_READ_ONLY)) {
 			ret = EFI_WRITE_PROTECTED;
 			goto err;
 		}

 		/* attributes won't be changed */
 		if (!delete &&
-		    ((ro_check && attr != attributes) ||
-		     (!ro_check && ((attr & ~(u32)EFI_VARIABLE_READ_ONLY)
+		    ((ro_check && old_attr != attributes) ||
+		     (!ro_check && ((old_attr & ~(u32)EFI_VARIABLE_READ_ONLY)
 				    != (attributes & ~(u32)EFI_VARIABLE_READ_ONLY))))) {
 			ret = EFI_INVALID_PARAMETER;
 			goto err;
@@ -902,7 +902,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
 		    EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
 			ret = efi_variable_authenticate(variable_name, vendor,
 							&data_size, &data,
-							attributes, &attr,
+							attributes, &old_attr,
 							&time);
 			if (ret != EFI_SUCCESS)
 				goto err;
@@ -936,7 +936,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
 			goto err;
 		}
 		ret = efi_get_variable_int(variable_name, vendor,
-					   &attr, &old_size, old_data, NULL);
+					   &old_attr, &old_size, old_data, NULL);
 		if (ret != EFI_SUCCESS)
 			goto err;
 	} else {
@@ -962,8 +962,8 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
 		       EFI_VARIABLE_RUNTIME_ACCESS |
 		       EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS);
 	s += sprintf(s, "{");
-	while (attributes) {
-		attr = 1 << (ffs(attributes) - 1);
+	for (u32 attr_rem = attributes; attr_rem;) {
+		u32 attr = 1 << (ffs(attr_rem) - 1);

 		if (attr == EFI_VARIABLE_READ_ONLY) {
 			s += sprintf(s, "ro");
@@ -979,8 +979,8 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
 			s = bin2hex(s, (u8 *)&time, sizeof(time));
 		}

-		attributes &= ~attr;
-		if (attributes)
+		attr_rem &= ~attr;
+		if (attr_rem)
 			s += sprintf(s, ",");
 	}
 	s += sprintf(s, "}");
--
2.27.0

[PATCH v2 07/17] efi_loader: value of VendorKeys

[Heinrich Schuchardt]

According to the UEFI specification the variable VendorKeys is 1 if the
"system is configured to use only vendor-provided keys".

As we do not supply any vendor keys yet the variable VendorKeys must be
zero.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_variable.c | 75 +++++++----------------------------
 1 file changed, 15 insertions(+), 60 deletions(-)

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index a7de0b4022..e3b29663a0 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -282,45 +282,29 @@ err:
  */
 static efi_status_t efi_init_secure_state(void)
 {
-	enum efi_secure_mode mode;
-	efi_uintn_t size;
+	enum efi_secure_mode mode = EFI_MODE_SETUP;
+	efi_uintn_t size = 0;
 	efi_status_t ret;

-	/*
-	 * TODO:
-	 * Since there is currently no "platform-specific" installation
-	 * method of Platform Key, we can't say if VendorKeys is 0 or 1
-	 * precisely.
-	 */
-
-	size = 0;
 	ret = efi_get_variable_int(L"PK", &efi_global_variable_guid,
 				   NULL, &size, NULL, NULL);
 	if (ret == EFI_BUFFER_TOO_SMALL) {
 		if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT))
 			mode = EFI_MODE_USER;
-		else
-			mode = EFI_MODE_SETUP;
-
-		efi_vendor_keys = 0;
-	} else if (ret == EFI_NOT_FOUND) {
-		mode = EFI_MODE_SETUP;
-		efi_vendor_keys = 1;
-	} else {
-		goto err;
 	}

 	ret = efi_transfer_secure_state(mode);
-	if (ret == EFI_SUCCESS)
-		ret = efi_set_variable_int(L"VendorKeys",
-					   &efi_global_variable_guid,
-					   EFI_VARIABLE_BOOTSERVICE_ACCESS |
-					   EFI_VARIABLE_RUNTIME_ACCESS |
-					   EFI_VARIABLE_READ_ONLY,
-					   sizeof(efi_vendor_keys),
-					   &efi_vendor_keys, false);
+	if (ret != EFI_SUCCESS)
+		return ret;

-err:
+	/* As we do not provide vendor keys this variable is always 0. */
+	ret = efi_set_variable_int(L"VendorKeys",
+				   &efi_global_variable_guid,
+				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				   EFI_VARIABLE_RUNTIME_ACCESS |
+				   EFI_VARIABLE_READ_ONLY,
+				   sizeof(efi_vendor_keys),
+				   &efi_vendor_keys, false);
 	return ret;
 }

@@ -998,39 +982,10 @@ out:
 	if (env_set(native_name, val)) {
 		ret = EFI_DEVICE_ERROR;
 	} else {
-		bool vendor_keys_modified = false;
-
-		if ((u16_strcmp(variable_name, L"PK") == 0 &&
-		     guidcmp(vendor, &efi_global_variable_guid) == 0)) {
-			ret = efi_transfer_secure_state(
-					(delete ? EFI_MODE_SETUP :
-						  EFI_MODE_USER));
-			if (ret != EFI_SUCCESS)
-				goto err;
-
-			if (efi_secure_mode != EFI_MODE_SETUP)
-				vendor_keys_modified = true;
-		} else if ((u16_strcmp(variable_name, L"KEK") == 0 &&
-		     guidcmp(vendor, &efi_global_variable_guid) == 0)) {
-			if (efi_secure_mode != EFI_MODE_SETUP)
-				vendor_keys_modified = true;
-		}
-
-		/* update VendorKeys */
-		if (vendor_keys_modified & efi_vendor_keys) {
-			efi_vendor_keys = 0;
-			ret = efi_set_variable_int(
-						L"VendorKeys",
-						&efi_global_variable_guid,
-						EFI_VARIABLE_BOOTSERVICE_ACCESS
-						 | EFI_VARIABLE_RUNTIME_ACCESS
-						 | EFI_VARIABLE_READ_ONLY,
-						sizeof(efi_vendor_keys),
-						&efi_vendor_keys,
-						false);
-		} else {
+		if (!u16_strcmp(variable_name, L"PK"))
+			ret = efi_init_secure_state();
+		else
 			ret = EFI_SUCCESS;
-		}
 	}

 err:
--
2.27.0

[PATCH v2 08/17] efi_loader: read-only AuditMode and DeployedMode

[Heinrich Schuchardt]

Set the read only property of the UEFI variables AuditMode and DeployedMode
conforming to the UEFI specification.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_variable.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index e3b29663a0..b84b86672a 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -183,32 +183,36 @@ static const char *parse_attr(const char *str, u32 *attrp, u64 *timep)
 static efi_status_t efi_set_secure_state(u8 secure_boot, u8 setup_mode,
 					 u8 audit_mode, u8 deployed_mode)
 {
-	u32 attributes;
 	efi_status_t ret;
+	const u32 attributes_ro = EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				  EFI_VARIABLE_RUNTIME_ACCESS |
+				  EFI_VARIABLE_READ_ONLY;
+	const u32 attributes_rw = EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				  EFI_VARIABLE_RUNTIME_ACCESS;

-	attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
-		     EFI_VARIABLE_RUNTIME_ACCESS |
-		     EFI_VARIABLE_READ_ONLY;
 	ret = efi_set_variable_int(L"SecureBoot", &efi_global_variable_guid,
-				   attributes, sizeof(secure_boot),
+				   attributes_ro, sizeof(secure_boot),
 				   &secure_boot, false);
 	if (ret != EFI_SUCCESS)
 		goto err;

 	ret = efi_set_variable_int(L"SetupMode", &efi_global_variable_guid,
-				   attributes, sizeof(setup_mode),
+				   attributes_ro, sizeof(setup_mode),
 				   &setup_mode, false);
 	if (ret != EFI_SUCCESS)
 		goto err;

 	ret = efi_set_variable_int(L"AuditMode", &efi_global_variable_guid,
-				   attributes, sizeof(audit_mode),
-				   &audit_mode, false);
+				   audit_mode || setup_mode ?
+				   attributes_ro : attributes_rw,
+				   sizeof(audit_mode), &audit_mode, false);
 	if (ret != EFI_SUCCESS)
 		goto err;

 	ret = efi_set_variable_int(L"DeployedMode",
-				   &efi_global_variable_guid, attributes,
+				   &efi_global_variable_guid,
+				   audit_mode || deployed_mode || setup_mode ?
+				   attributes_ro : attributes_rw,
 				   sizeof(deployed_mode), &deployed_mode,
 				   false);
 err:
--
2.27.0

[PATCH v2 09/17] efi_loader: secure boot flag

[Heinrich Schuchardt]

In audit mode the UEFI variable SecureBoot is set to zero but the
efi_secure_boot flag is set to true.

The efi_secure_boot flag should match the UEFIvariable SecureBoot.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_variable.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index b84b86672a..f8a50bb1be 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -190,6 +190,8 @@ static efi_status_t efi_set_secure_state(u8 secure_boot, u8 setup_mode,
 	const u32 attributes_rw = EFI_VARIABLE_BOOTSERVICE_ACCESS |
 				  EFI_VARIABLE_RUNTIME_ACCESS;

+	efi_secure_boot = secure_boot;
+
 	ret = efi_set_variable_int(L"SecureBoot", &efi_global_variable_guid,
 				   attributes_ro, sizeof(secure_boot),
 				   &secure_boot, false);
@@ -240,8 +242,6 @@ static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode)
 		ret = efi_set_secure_state(1, 0, 0, 1);
 		if (ret != EFI_SUCCESS)
 			goto err;
-
-		efi_secure_boot = true;
 	} else if (mode == EFI_MODE_AUDIT) {
 		ret = efi_set_variable_int(L"PK", &efi_global_variable_guid,
 					   EFI_VARIABLE_BOOTSERVICE_ACCESS |
@@ -253,14 +253,10 @@ static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode)
 		ret = efi_set_secure_state(0, 1, 1, 0);
 		if (ret != EFI_SUCCESS)
 			goto err;
-
-		efi_secure_boot = true;
 	} else if (mode == EFI_MODE_USER) {
 		ret = efi_set_secure_state(1, 0, 0, 0);
 		if (ret != EFI_SUCCESS)
 			goto err;
-
-		efi_secure_boot = true;
 	} else if (mode == EFI_MODE_SETUP) {
 		ret = efi_set_secure_state(0, 1, 0, 0);
 		if (ret != EFI_SUCCESS)
--
2.27.0

[PATCH v2 10/17] efi_loader: UEFI variable persistence

[Heinrich Schuchardt]

Persist non-volatile UEFI variables in a file on the EFI system partition.

The file is written whenever a non-volatile UEFI variable is changed after
initialization of the UEFI sub-system.

The file is read during the UEFI sub-system initialization to restore
non-volatile UEFI variables.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_variable.h        |  62 +++++++++
 lib/efi_loader/Kconfig        |   8 ++
 lib/efi_loader/Makefile       |   1 +
 lib/efi_loader/efi_var_file.c | 239 ++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_variable.c |  10 +-
 5 files changed, 319 insertions(+), 1 deletion(-)
 create mode 100644 lib/efi_loader/efi_var_file.c

diff --git a/include/efi_variable.h b/include/efi_variable.h
index 3ba274fce1..01054209c4 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -79,4 +79,66 @@ efi_status_t efi_query_variable_info_int(u32 attributes,
 					 u64 *remaining_variable_storage_size,
 					 u64 *maximum_variable_size);

+#define EFI_VAR_FILE_NAME "ubootefi.var"
+
+#define EFI_VAR_BUF_SIZE 0x4000
+
+#define EFI_VAR_FILE_MAGIC 0x0161566966456255 /* UbEfiVa, version 1 */
+
+/**
+ * struct efi_var_entry - UEFI variable file entry
+ *
+ * @length:	length of enty, multiple of 8
+ * @attr:	variable attributes
+ * @time:	authentication time (seconds since start of epoch)
+ * @guid:	vendor GUID
+ * @name:	UTF16 variable name
+ */
+struct efi_var_entry {
+	u32 length;
+	u32 attr;
+	u64 time;
+	efi_guid_t guid;
+	u16 name[];
+};
+
+/**
+ * struct efi_var_file - file for storing UEFI variables
+ *
+ * @reserved:	unused, may be overwritten by memory probing
+ * @magic:	identifies file format
+ * @length:	length including header
+ * @crc32:	CRC32 without header
+ * @var:	variables
+ */
+struct efi_var_file {
+	u64 reserved;
+	u64 magic;
+	u32 length;
+	u32 crc32;
+	struct efi_var_entry var[];
+};
+
+/**
+ * efi_var_to_file() - 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_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.
+ *
+ * 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);
+
 #endif
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 6c9df3a767..4324694d48 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -27,6 +27,14 @@ config EFI_LOADER

 if EFI_LOADER

+config EFI_VARIABLE_FILE_STORE
+	bool "Store non-volatile UEFI variables as file"
+	depends on FAT_WRITE
+	default y
+	help
+	  Select tis option if you want non-volatile UEFI variables to be stored
+	  as file /ubootefi.var on the EFI system partition.
+
 config EFI_GET_TIME
 	bool "GetTime() runtime service"
 	depends on DM_RTC
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 7eddd7ef37..c87b82db32 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -40,6 +40,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
 endif
 obj-y += efi_watchdog.o
 obj-$(CONFIG_LCD) += efi_gop.o
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
new file mode 100644
index 0000000000..b1b7532495
--- /dev/null
+++ b/lib/efi_loader/efi_var_file.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * File interface for UEFI variables
+ *
+ * Copyright (c) 2020, Heinrich Schuchardt
+ */
+
+#define LOG_CATEGORY LOGC_EFI
+
+#include <common.h>
+#include <charset.h>
+#include <fs.h>
+#include <log.h>
+#include <malloc.h>
+#include <mapmem.h>
+#include <efi_loader.h>
+#include <efi_variable.h>
+#include <u-boot/crc.h>
+
+#define PART_STR_LEN 10
+
+/**
+ * efi_set_blk_dev_to_system_partition() - select EFI system partition
+ *
+ * Set the EFI system partition as current block device.
+ *
+ * Return:	status code
+ */
+static efi_status_t __maybe_unused efi_set_blk_dev_to_system_partition(void)
+{
+	char part_str[PART_STR_LEN];
+	int r;
+
+	if (!efi_system_partition.if_type) {
+		log_err("No EFI system partition\n");
+		return EFI_DEVICE_ERROR;
+	}
+	snprintf(part_str, PART_STR_LEN, "%u:%u",
+		 efi_system_partition.devnum, efi_system_partition.part);
+	r = fs_set_blk_dev(blk_get_if_type_name(efi_system_partition.if_type),
+			   part_str, FS_TYPE_ANY);
+	if (r) {
+		log_err("Cannot read EFI system partition\n");
+		return EFI_DEVICE_ERROR;
+	}
+	return EFI_SUCCESS;
+}
+
+/**
+ * efi_var_collect() - collect non-volatile variables in buffer
+ *
+ * A buffer is allocated and filled with all non-volatile variables in a
+ * format ready to be written to disk.
+ *
+ * @bufp:	pointer to pointer of buffer with collected variables
+ * @lenp:	pointer to length of buffer
+ * Return:	status code
+ */
+static efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp,
+						   loff_t *lenp)
+{
+	size_t len = EFI_VAR_BUF_SIZE;
+	struct efi_var_file *buf;
+	struct efi_var_entry *var, *old_var;
+	size_t old_var_name_length = 2;
+
+	*bufp = NULL; /* Avoid double free() */
+	buf = calloc(1, len);
+	if (!buf)
+		return EFI_OUT_OF_RESOURCES;
+	var = buf->var;
+	old_var = var;
+	for (;;) {
+		efi_uintn_t data_length, var_name_length;
+		u8 *data;
+		efi_status_t ret;
+
+		if ((uintptr_t)buf + len <=
+		    (uintptr_t)var->name + old_var_name_length)
+			return EFI_BUFFER_TOO_SMALL;
+
+		var_name_length = (uintptr_t)buf + len - (uintptr_t)var->name;
+		memcpy(var->name, old_var->name, old_var_name_length);
+		guidcpy(&var->guid, &old_var->guid);
+		ret = efi_get_next_variable_name_int(
+				&var_name_length, var->name, &var->guid);
+		if (ret == EFI_NOT_FOUND)
+			break;
+		if (ret != EFI_SUCCESS) {
+			free(buf);
+			return ret;
+		}
+		old_var_name_length = var_name_length;
+		old_var = var;
+
+		data = (u8 *)var->name + old_var_name_length;
+		data_length = (uintptr_t)buf + len - (uintptr_t)data;
+		ret = efi_get_variable_int(var->name, &var->guid,
+					   &var->attr, &data_length, data,
+					   &var->time);
+		if (ret != EFI_SUCCESS) {
+			free(buf);
+			return ret;
+		}
+		if (!(var->attr & EFI_VARIABLE_NON_VOLATILE))
+			continue;
+		var->length = data_length;
+		var = (struct efi_var_entry *)
+		      ALIGN((uintptr_t)data + data_length, 8);
+	}
+
+	buf->reserved = 0;
+	buf->magic = EFI_VAR_FILE_MAGIC;
+	len = (uintptr_t)var - (uintptr_t)buf;
+	buf->crc32 = crc32(0, (u8 *)buf->var,
+			   len - sizeof(struct efi_var_file));
+	buf->length = len;
+	*bufp = buf;
+	*lenp = len;
+
+	return EFI_SUCCESS;
+}
+
+/**
+ * efi_var_to_file() - 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)
+{
+#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
+	efi_status_t ret;
+	struct efi_var_file *buf;
+	loff_t len;
+	loff_t actlen;
+	int r;
+
+	ret = efi_var_collect(&buf, &len);
+	if (ret != EFI_SUCCESS)
+		goto error;
+
+	ret = efi_set_blk_dev_to_system_partition();
+	if (ret != EFI_SUCCESS)
+		goto error;
+
+	r = fs_write(EFI_VAR_FILE_NAME, map_to_sysmem(buf), 0, len, &actlen);
+	if (r || len != actlen)
+		ret = EFI_DEVICE_ERROR;
+
+error:
+	if (ret != EFI_SUCCESS)
+		log_err("Failed to persist EFI variables\n");
+	free(buf);
+	return ret;
+#else
+	return EFI_SUCCESS;
+#endif
+}
+
+/**
+ * efi_var_restore() - restore EFI variables from buffer
+ *
+ * @buf:	buffer
+ * Return:	status code
+ */
+static efi_status_t __maybe_unused efi_var_restore(struct efi_var_file *buf)
+{
+	struct efi_var_entry *var, *last_var;
+	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;
+	}
+
+	var = buf->var;
+	last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
+	while (var < last_var) {
+		u16 *data = var->name + u16_strlen(var->name) + 1;
+
+		if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) {
+			ret = efi_set_variable_int(var->name, &var->guid,
+						   var->attr, var->length,
+						   data, true);
+			if (ret != EFI_SUCCESS)
+				log_err("Failed to set EFI variable %ls\n",
+					var->name);
+		}
+		var = (struct efi_var_entry *)
+		      ALIGN((uintptr_t)data + var->length, 8);
+	}
+	return EFI_SUCCESS;
+}
+
+/**
+ * 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.
+ *
+ * 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)
+{
+#ifdef CONFIG_EFI_VARIABLE_FILE_STORE
+	struct efi_var_file *buf;
+	loff_t len;
+	efi_status_t ret;
+	int r;
+
+	buf = calloc(1, EFI_VAR_BUF_SIZE);
+	if (!buf) {
+		log_err("Out of memory\n");
+		return EFI_OUT_OF_RESOURCES;
+	}
+
+	ret = efi_set_blk_dev_to_system_partition();
+	if (ret != EFI_SUCCESS)
+		goto error;
+	r = fs_read(EFI_VAR_FILE_NAME, map_to_sysmem(buf), 0, EFI_VAR_BUF_SIZE,
+		    &len);
+	if (r || len < sizeof(struct efi_var_file)) {
+		log_err("Failed to load EFI variables\n");
+		goto error;
+	}
+	if (buf->length != len || efi_var_restore(buf) != EFI_SUCCESS)
+		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 f8a50bb1be..f2f787fc8d 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -988,6 +988,11 @@ out:
 			ret = EFI_SUCCESS;
 	}

+	/* Write non-volatile EFI variables to file */
+	if (attributes & EFI_VARIABLE_NON_VOLATILE &&
+	    ret == EFI_SUCCESS && efi_obj_list_initialized == EFI_SUCCESS)
+		efi_var_to_file();
+
 err:
 	free(native_name);
 	free(old_data);
@@ -1083,6 +1088,7 @@ efi_set_variable_runtime(u16 *variable_name, const efi_guid_t *vendor,
  */
 void efi_variables_boot_exit_notify(void)
 {
+	/* Switch variable services functions to runtime version */
 	efi_runtime_services.get_variable = efi_get_variable_runtime;
 	efi_runtime_services.get_next_variable_name =
 				efi_get_next_variable_name_runtime;
@@ -1103,6 +1109,8 @@ efi_status_t efi_init_variables(void)
 	efi_status_t ret;

 	ret = efi_init_secure_state();
+	if (ret != EFI_SUCCESS)
+		return ret;

-	return ret;
+	return efi_var_from_file();
 }
--
2.27.0

[PATCH v2 11/17] efi_loader: export efi_convert_pointer()

[Heinrich Schuchardt]

We need ConvertPointer() to adjust pointers when implementing  runtime
services within U-Boot.

After ExitBootServices() gd is not available anymore. So we should not use
EFI_ENTRY() and EFI_EXIT().

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_loader.h         | 3 +++
 lib/efi_loader/efi_runtime.c | 8 +++-----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index fc9344c742..fe8774f133 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -397,6 +397,9 @@ efi_status_t efi_root_node_register(void);
 efi_status_t efi_initialize_system_table(void);
 /* efi_runtime_detach() - detach unimplemented runtime functions */
 void efi_runtime_detach(void);
+/* efi_convert_pointer() - convert pointer to virtual address */
+efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition,
+					void **address);
 /* Called by bootefi to make console interface available */
 efi_status_t efi_console_register(void);
 /* Called by bootefi to make all disk storage accessible as EFI objects */
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 121e2f65c6..45baa2fd3e 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -496,15 +496,13 @@ static __efi_runtime efi_status_t EFIAPI efi_convert_pointer_runtime(
  * @address:		pointer to be converted
  * Return:		status code
  */
-static __efi_runtime efi_status_t EFIAPI efi_convert_pointer(
-			efi_uintn_t debug_disposition, void **address)
+__efi_runtime efi_status_t EFIAPI
+efi_convert_pointer(efi_uintn_t debug_disposition, void **address)
 {
 	efi_physical_addr_t addr;
 	efi_uintn_t i;
 	efi_status_t ret = EFI_NOT_FOUND;

-	EFI_ENTRY("%zu %p", debug_disposition, address);
-
 	if (!efi_virtmap) {
 		ret = EFI_UNSUPPORTED;
 		goto out;
@@ -533,7 +531,7 @@ static __efi_runtime efi_status_t EFIAPI efi_convert_pointer(
 	}

 out:
-	return EFI_EXIT(ret);
+	return ret;
 }

 static __efi_runtime void efi_relocate_runtime_table(ulong offset)
--
2.27.0

[PATCH v2 12/17] efi_loader: optional pointer for ConvertPointer

[Heinrich Schuchardt]

If the EFI_OPTIONAL_PTR is set in DebugDisposition, a NULL pointer does not
constitute an invalid parameter.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_api.h            | 2 ++
 lib/efi_loader/efi_runtime.c | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/include/efi_api.h b/include/efi_api.h
index 759d911875..5744f6aed8 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -251,6 +251,8 @@ struct efi_rt_properties_table {
 	u32 runtime_services_supported;
 };

+#define EFI_OPTIONAL_PTR	0x00000001
+
 struct efi_runtime_services {
 	struct efi_table_hdr hdr;
 	efi_status_t (EFIAPI *get_time)(struct efi_time *time,
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 45baa2fd3e..a4aa1d8b6c 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -512,6 +512,12 @@ efi_convert_pointer(efi_uintn_t debug_disposition, void **address)
 		ret = EFI_INVALID_PARAMETER;
 		goto out;
 	}
+	if (!*address) {
+		if (debug_disposition & EFI_OPTIONAL_PTR)
+			return EFI_SUCCESS;
+		else
+			return EFI_INVALID_PARAMETER;
+	}

 	addr = (uintptr_t)*address;
 	for (i = 0; i < efi_descriptor_count; i++) {
--
2.27.0

[PATCH v2 13/17] efi_loader: new function efi_memcpy_runtime()

[Heinrich Schuchardt]

Provide a memcpy() function that we can use at UEFI runtime.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_loader.h         |  3 +++
 lib/efi_loader/efi_runtime.c | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index fe8774f133..bf03a89ca5 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -789,6 +789,9 @@ bool efi_secure_boot_enabled(void);
 bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
 		     WIN_CERTIFICATE **auth, size_t *auth_len);

+/* runtime implementation of memcpy() */
+void efi_memcpy_runtime(void *dest, const void *src, size_t n);
+
 #else /* CONFIG_IS_ENABLED(EFI_LOADER) */

 /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index a4aa1d8b6c..5b6506fbdc 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -137,6 +137,25 @@ efi_status_t efi_init_runtime_supported(void)
 	return ret;
 }

+/**
+ * efi_memcpy_runtime() - copy memory area
+ *
+ * At runtime memcpy() is not available.
+ *
+ * @dest:	destination buffer
+ * @src:	source buffer
+ * @n:		number of bytes to copy
+ * Return:	pointer to destination buffer
+ */
+void __efi_runtime efi_memcpy_runtime(void *dest, const void *src, size_t n)
+{
+	u8 *d = dest;
+	const u8 *s = src;
+
+	for (; n; --n)
+		*d++ = *s++;
+}
+
 /**
  * efi_update_table_header_crc32() - Update crc32 in table header
  *
--
2.27.0

[PATCH v2 14/17] efi_loader: memory buffer for variables

[Heinrich Schuchardt]

Saving UEFI variable as encoded U-Boot environment variables does not allow
support at runtime.

Provide functions to manage a memory buffer with UEFI variables.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_variable.h       |  54 +++++++
 lib/efi_loader/Makefile      |   1 +
 lib/efi_loader/efi_var_mem.c | 266 +++++++++++++++++++++++++++++++++++
 3 files changed, 321 insertions(+)
 create mode 100644 lib/efi_loader/efi_var_mem.c

diff --git a/include/efi_variable.h b/include/efi_variable.h
index 01054209c4..bc5985cfdb 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -141,4 +141,58 @@ efi_status_t efi_var_to_file(void);
  */
 efi_status_t efi_var_from_file(void);

+/**
+ * efi_var_mem_init() - set-up variable list
+ *
+ * Return:	status code
+ */
+efi_status_t efi_var_mem_init(void);
+
+/**
+ * efi_var_mem_find() - find a variable in the list
+ *
+ * @guid:	GUID of the variable
+ * @name:	name of the variable
+ * @next:	on exit pointer to the next variable after the found one
+ * Return:	found variable
+ */
+struct efi_var_entry *efi_var_mem_find(const efi_guid_t *guid, const u16 *name,
+				       struct efi_var_entry **next);
+
+/**
+ * efi_var_mem_del() - delete a variable from the list of variables
+ *
+ * @var:	variable to delete
+ */
+void efi_var_mem_del(struct efi_var_entry *var);
+
+/**
+ * efi_var_mem_ins() - append a variable to the list of variables
+ *
+ * The variable is appended without checking if a variable of the same name
+ * already exists. The two data buffers are concatenated.
+ *
+ * @variable_name:	variable name
+ * @vendor:		GUID
+ * @attributes:		variable attributes
+ * @size1:		size of the first data buffer
+ * @data1:		first data buffer
+ * @size2:		size of the second data field
+ * @data2:		second data buffer
+ * @time:		time of authentication (as seconds since start of epoch)
+ * Result:		status code
+ */
+efi_status_t efi_var_mem_ins(u16 *variable_name,
+			     const efi_guid_t *vendor, u32 attributes,
+			     const efi_uintn_t size1, const void *data1,
+			     const efi_uintn_t size2, const void *data2,
+			     const u64 time);
+
+/**
+ * efi_var_mem_free() - determine free memory for variables
+ *
+ * Return:	maximum data size plus variable name size
+ */
+u64 efi_var_mem_free(void);
+
 #endif
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index c87b82db32..f81ec8d277 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -36,6 +36,7 @@ obj-y += efi_runtime.o
 obj-y += efi_setup.o
 obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += efi_unicode_collation.o
 obj-y += efi_var_common.o
+obj-y += efi_var_mem.o
 ifeq ($(CONFIG_EFI_MM_COMM_TEE),y)
 obj-y += efi_variable_tee.o
 else
diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c
new file mode 100644
index 0000000000..ac55d8f8dc
--- /dev/null
+++ b/lib/efi_loader/efi_var_mem.c
@@ -0,0 +1,266 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * File interface for UEFI variables
+ *
+ * Copyright (c) 2020, Heinrich Schuchardt
+ */
+
+#include <common.h>
+#include <efi_loader.h>
+#include <efi_variable.h>
+#include <u-boot/crc.h>
+
+static struct efi_var_file __efi_runtime_data *efi_var_buf;
+static struct efi_var_entry __efi_runtime_data *efi_current_var;
+
+/**
+ * efi_var_mem_compare() - compare GUID and name with a variable
+ *
+ * @var:	variable to compare
+ * @guid:	GUID to compare
+ * @name:	variable name to compare
+ * @next:	pointer to next variable
+ * Return:	true if match
+ */
+static bool __efi_runtime
+efi_var_mem_compare(struct efi_var_entry *var, const efi_guid_t *guid,
+		    const u16 *name, struct efi_var_entry **next)
+{
+	int i;
+	u8 *guid1, *guid2;
+	const u16 *data, *var_name;
+	bool match = true;
+
+	for (guid1 = (u8 *)&var->guid, guid2 = (u8 *)guid, i = 0;
+	     i < sizeof(efi_guid_t) && match; ++i)
+		match = (guid1[i] == guid2[i]);
+
+	for (data = var->name, var_name = name;; ++data, ++var_name) {
+		if (match)
+			match = (*data == *var_name);
+		if (!*data)
+			break;
+	}
+
+	++data;
+
+	if (next)
+		*next = (struct efi_var_entry *)
+			ALIGN((uintptr_t)data + var->length, 8);
+
+	return match;
+}
+
+struct efi_var_entry __efi_runtime
+*efi_var_mem_find(const efi_guid_t *guid, const u16 *name,
+		  struct efi_var_entry **next)
+{
+	struct efi_var_entry *var, *last;
+
+	last = (struct efi_var_entry *)
+	       ((uintptr_t)efi_var_buf + efi_var_buf->length);
+
+	if (!*name) {
+		if (next) {
+			*next = efi_var_buf->var;
+			if (*next >= last)
+				*next = NULL;
+		}
+		return NULL;
+	}
+	if (efi_current_var &&
+	    efi_var_mem_compare(efi_current_var, guid, name, next)) {
+		if (next && *next >= last)
+			*next = NULL;
+		return efi_current_var;
+	}
+
+	var = efi_var_buf->var;
+	if (var < last) {
+		for (; var;) {
+			struct efi_var_entry *pos;
+			bool match;
+
+			match = efi_var_mem_compare(var, guid, name, &pos);
+			if (pos >= last)
+				pos = NULL;
+			if (match) {
+				if (next)
+					*next = pos;
+				return var;
+			}
+			var = pos;
+		}
+	}
+	if (next)
+		*next = NULL;
+	return NULL;
+}
+
+void __efi_runtime efi_var_mem_del(struct efi_var_entry *var)
+{
+	u16 *data;
+	struct efi_var_entry *next, *last;
+	u64 *from, *to;
+
+	if (!var)
+		return;
+
+	last = (struct efi_var_entry *)
+	       ((uintptr_t)efi_var_buf + efi_var_buf->length);
+	if (var < efi_current_var)
+		efi_current_var = NULL;
+
+	for (data = var->name; *data; ++data)
+		;
+	++data;
+	next = (struct efi_var_entry *)
+	       ALIGN((uintptr_t)data + var->length, 8);
+	efi_var_buf->length -= (uintptr_t)next - (uintptr_t)var;
+
+	for (to = (u64 *)var, from = (u64 *)next; from < (u64 *)last;
+	     ++to, ++from)
+		*to = *from;
+	efi_var_buf->crc32 = crc32(0, (u8 *)efi_var_buf->var,
+				   efi_var_buf->length -
+				   sizeof(struct efi_var_file));
+}
+
+efi_status_t __efi_runtime efi_var_mem_ins(
+				u16 *variable_name,
+				const efi_guid_t *vendor, u32 attributes,
+				const efi_uintn_t size1, const void *data1,
+				const efi_uintn_t size2, const void *data2,
+				const u64 time)
+{
+	u16 *data;
+	struct efi_var_entry *var;
+	u32 var_name_len;
+
+	var = (struct efi_var_entry *)
+	      ((uintptr_t)efi_var_buf + efi_var_buf->length);
+	for (var_name_len = 0; variable_name[var_name_len]; ++var_name_len)
+		;
+	++var_name_len;
+	data = var->name + var_name_len;
+
+	if ((uintptr_t)data - (uintptr_t)efi_var_buf + size1 + size2 >
+	    EFI_VAR_BUF_SIZE)
+		return EFI_OUT_OF_RESOURCES;
+
+	var->attr = attributes;
+	var->length = size1 + size2;
+	var->time = time;
+
+	efi_memcpy_runtime(&var->guid, vendor, sizeof(efi_guid_t));
+	efi_memcpy_runtime(var->name, variable_name,
+			   sizeof(u16) * var_name_len);
+	efi_memcpy_runtime(data, data1, size1);
+	efi_memcpy_runtime((u8 *)data + size1, data2, size2);
+
+	var = (struct efi_var_entry *)
+	      ALIGN((uintptr_t)data + var->length, 8);
+	efi_var_buf->length = (uintptr_t)var - (uintptr_t)efi_var_buf;
+	efi_var_buf->crc32 = crc32(0, (u8 *)efi_var_buf->var,
+				   efi_var_buf->length -
+				   sizeof(struct efi_var_file));
+
+	return EFI_SUCCESS;
+}
+
+u64 __efi_runtime efi_var_mem_free(void)
+{
+	return EFI_VAR_BUF_SIZE - efi_var_buf->length -
+	       sizeof(struct efi_var_entry);
+}
+
+/**
+ * efi_var_mem_bs_del() - delete boot service only variables
+ */
+static void efi_var_mem_bs_del(void)
+{
+	struct efi_var_entry *var = efi_var_buf->var;
+
+	for (;;) {
+		struct efi_var_entry *last;
+
+		last = (struct efi_var_entry *)
+		       ((uintptr_t)efi_var_buf + efi_var_buf->length);
+		if (var >= last)
+			break;
+		if (var->attr & EFI_VARIABLE_RUNTIME_ACCESS) {
+			u16 *data;
+
+			/* skip variable */
+			for (data = var->name; *data; ++data)
+				;
+			++data;
+			var = (struct efi_var_entry *)
+			      ALIGN((uintptr_t)data + var->length, 8);
+		} else {
+			/* delete variable */
+			efi_var_mem_del(var);
+		}
+	}
+}
+
+/**
+ * efi_var_mem_notify_exit_boot_services() - ExitBootService callback
+ *
+ * @event:	callback event
+ * @context:	callback context
+ */
+static void EFIAPI __efi_runtime
+efi_var_mem_notify_exit_boot_services(struct efi_event *event, void *context)
+{
+	EFI_ENTRY("%p, %p", event, context);
+
+	/* Delete boot service only variables */
+	efi_var_mem_bs_del();
+
+	EFI_EXIT(EFI_SUCCESS);
+}
+
+/**
+ * efi_var_mem_notify_exit_boot_services() - SetVirtualMemoryMap callback
+ *
+ * @event:	callback event
+ * @context:	callback context
+ */
+static void EFIAPI __efi_runtime
+efi_var_mem_notify_virtual_address_map(struct efi_event *event, void *context)
+{
+	efi_convert_pointer(0, (void **)&efi_var_buf);
+}
+
+efi_status_t efi_var_mem_init(void)
+{
+	u64 memory;
+	efi_status_t ret;
+	struct efi_event *event;
+
+	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
+				 EFI_RUNTIME_SERVICES_DATA,
+				 efi_size_in_pages(EFI_VAR_BUF_SIZE),
+				 &memory);
+	if (ret != EFI_SUCCESS)
+		return ret;
+	efi_var_buf = (struct efi_var_file *)(uintptr_t)memory;
+	memset(efi_var_buf, 0, EFI_VAR_BUF_SIZE);
+	efi_var_buf->magic = EFI_VAR_FILE_MAGIC;
+	efi_var_buf->length = (uintptr_t)efi_var_buf->var -
+			      (uintptr_t)efi_var_buf;
+	/* crc32 for 0 bytes = 0 */
+
+	ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
+			       efi_var_mem_notify_exit_boot_services, NULL,
+			       NULL, &event);
+	if (ret != EFI_SUCCESS)
+		return ret;
+	ret = efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, TPL_CALLBACK,
+			       efi_var_mem_notify_virtual_address_map, NULL,
+			       NULL, &event);
+	if (ret != EFI_SUCCESS)
+		return ret;
+	return ret;
+}
--
2.27.0

[PATCH v2 15/17] efi_loader: use memory based variable storage

[Heinrich Schuchardt]

Saving UEFI variable as encoded U-Boot environment variables does not allow
implement run-time support.

Use a memory buffer for storing UEFI variables.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_variable.c | 556 ++++++----------------------------
 1 file changed, 93 insertions(+), 463 deletions(-)

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index f2f787fc8d..13123e7e41 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -30,145 +30,6 @@ static bool efi_secure_boot;
 static enum efi_secure_mode efi_secure_mode;
 static u8 efi_vendor_keys;

-/*
- * Mapping between EFI variables and u-boot variables:
- *
- *   efi_$guid_$varname = {attributes}(type)value
- *
- * For example:
- *
- *   efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported=
- *      "{ro,boot,run}(blob)0000000000000000"
- *   efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_BootOrder=
- *      "(blob)00010000"
- *
- * The attributes are a comma separated list of these possible
- * attributes:
- *
- *   + ro   - read-only
- *   + boot - boot-services access
- *   + run  - runtime access
- *
- * NOTE: with current implementation, no variables are available after
- * ExitBootServices, and all are persisted (if possible).
- *
- * If not specified, the attributes default to "{boot}".
- *
- * The required type is one of:
- *
- *   + utf8 - raw utf8 string
- *   + blob - arbitrary length hex string
- *
- * Maybe a utf16 type would be useful to for a string value to be auto
- * converted to utf16?
- */
-
-#define PREFIX_LEN (strlen("efi_xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx_"))
-
-/**
- * efi_to_native() - convert the UEFI variable name and vendor GUID to U-Boot
- *		     variable name
- *
- * The U-Boot variable name is a concatenation of prefix 'efi', the hexstring
- * encoded vendor GUID, and the UTF-8 encoded UEFI variable name separated by
- * underscores, e.g. 'efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_BootOrder'.
- *
- * @native:		pointer to pointer to U-Boot variable name
- * @variable_name:	UEFI variable name
- * @vendor:		vendor GUID
- * Return:		status code
- */
-static efi_status_t efi_to_native(char **native, const u16 *variable_name,
-				  const efi_guid_t *vendor)
-{
-	size_t len;
-	char *pos;
-
-	len = PREFIX_LEN + utf16_utf8_strlen(variable_name) + 1;
-	*native = malloc(len);
-	if (!*native)
-		return EFI_OUT_OF_RESOURCES;
-
-	pos = *native;
-	pos += sprintf(pos, "efi_%pUl_", vendor);
-	utf16_utf8_strcpy(&pos, variable_name);
-
-	return EFI_SUCCESS;
-}
-
-/**
- * prefix() - skip over prefix
- *
- * Skip over a prefix string.
- *
- * @str:	string with prefix
- * @prefix:	prefix string
- * Return:	string without prefix, or NULL if prefix not found
- */
-static const char *prefix(const char *str, const char *prefix)
-{
-	size_t n = strlen(prefix);
-	if (!strncmp(prefix, str, n))
-		return str + n;
-	return NULL;
-}
-
-/**
- * parse_attr() - decode attributes part of variable value
- *
- * Convert the string encoded attributes of a UEFI variable to a bit mask.
- * TODO: Several attributes are not supported.
- *
- * @str:	value of U-Boot variable
- * @attrp:	pointer to UEFI attributes
- * @timep:	pointer to time attribute
- * Return:	pointer to remainder of U-Boot variable value
- */
-static const char *parse_attr(const char *str, u32 *attrp, u64 *timep)
-{
-	u32 attr = 0;
-	char sep = '{';
-
-	if (*str != '{') {
-		*attrp = EFI_VARIABLE_BOOTSERVICE_ACCESS;
-		return str;
-	}
-
-	while (*str == sep) {
-		const char *s;
-
-		str++;
-
-		if ((s = prefix(str, "ro"))) {
-			attr |= EFI_VARIABLE_READ_ONLY;
-		} else if ((s = prefix(str, "nv"))) {
-			attr |= EFI_VARIABLE_NON_VOLATILE;
-		} else if ((s = prefix(str, "boot"))) {
-			attr |= EFI_VARIABLE_BOOTSERVICE_ACCESS;
-		} else if ((s = prefix(str, "run"))) {
-			attr |= EFI_VARIABLE_RUNTIME_ACCESS;
-		} else if ((s = prefix(str, "time="))) {
-			attr |= EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
-			hex2bin((u8 *)timep, s, sizeof(*timep));
-			s += sizeof(*timep) * 2;
-		} else if (*str == '}') {
-			break;
-		} else {
-			printf("invalid attribute: %s\n", str);
-			break;
-		}
-
-		str = s;
-		sep = ',';
-	}
-
-	str++;
-
-	*attrp = attr;
-
-	return str;
-}
-
 /**
  * efi_set_secure_state - modify secure boot state variables
  * @secure_boot:	value of SecureBoot
@@ -568,296 +429,115 @@ static efi_status_t efi_variable_authenticate(u16 *variable,
 }
 #endif /* CONFIG_EFI_SECURE_BOOT */

-efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
-				  u32 *attributes, efi_uintn_t *data_size,
-				  void *data, u64 *timep)
+efi_status_t __efi_runtime
+efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
+		     u32 *attributes, efi_uintn_t *data_size, void *data,
+		     u64 *timep)
 {
-	char *native_name;
-	efi_status_t ret;
-	unsigned long in_size;
-	const char *val = NULL, *s;
-	u64 time = 0;
-	u32 attr;
+	efi_uintn_t old_size;
+	struct efi_var_entry *var;
+	u16 *pdata;

 	if (!variable_name || !vendor || !data_size)
 		return EFI_INVALID_PARAMETER;
-
-	ret = efi_to_native(&native_name, variable_name, vendor);
-	if (ret)
-		return ret;
-
-	EFI_PRINT("get '%s'\n", native_name);
-
-	val = env_get(native_name);
-	free(native_name);
-	if (!val)
+	var = efi_var_mem_find(vendor, variable_name, NULL);
+	if (!var)
 		return EFI_NOT_FOUND;

-	val = parse_attr(val, &attr, &time);
-
-	if (timep)
-		*timep = time;
-
-	in_size = *data_size;
-
-	if ((s = prefix(val, "(blob)"))) {
-		size_t len = strlen(s);
-
-		/* number of hexadecimal digits must be even */
-		if (len & 1)
-			return EFI_DEVICE_ERROR;
-
-		/* two characters per byte: */
-		len /= 2;
-		*data_size = len;
-
-		if (in_size < len) {
-			ret = EFI_BUFFER_TOO_SMALL;
-			goto out;
-		}
-
-		if (!data) {
-			EFI_PRINT("Variable with no data shouldn't exist.\n");
-			return EFI_INVALID_PARAMETER;
-		}
-
-		if (hex2bin(data, s, len))
-			return EFI_DEVICE_ERROR;
-
-		EFI_PRINT("got value: \"%s\"\n", s);
-	} else if ((s = prefix(val, "(utf8)"))) {
-		unsigned len = strlen(s) + 1;
-
-		*data_size = len;
-
-		if (in_size < len) {
-			ret = EFI_BUFFER_TOO_SMALL;
-			goto out;
-		}
-
-		if (!data) {
-			EFI_PRINT("Variable with no data shouldn't exist.\n");
-			return EFI_INVALID_PARAMETER;
-		}
-
-		memcpy(data, s, len);
-		((char *)data)[len] = '\0';
-
-		EFI_PRINT("got value: \"%s\"\n", (char *)data);
-	} else {
-		EFI_PRINT("invalid value: '%s'\n", val);
-		return EFI_DEVICE_ERROR;
-	}
-
-out:
 	if (attributes)
-		*attributes = attr;
-
-	return ret;
-}
-
-static char *efi_variables_list;
-static char *efi_cur_variable;
-
-/**
- * parse_uboot_variable() - parse a u-boot variable and get uefi-related
- *			    information
- * @variable:		whole data of u-boot variable (ie. name=value)
- * @variable_name_size: size of variable_name buffer in byte
- * @variable_name:	name of uefi variable in u16, null-terminated
- * @vendor:		vendor's guid
- * @attributes:		attributes
- *
- * A uefi variable is encoded into a u-boot variable as described above.
- * This function parses such a u-boot variable and retrieve uefi-related
- * information into respective parameters. In return, variable_name_size
- * is the size of variable name including NULL.
- *
- * Return:		EFI_SUCCESS if parsing is OK, EFI_NOT_FOUND when
- *			the entire variable list has been returned,
- *			otherwise non-zero status code
- */
-static efi_status_t parse_uboot_variable(char *variable,
-					 efi_uintn_t *variable_name_size,
-					 u16 *variable_name,
-					 const efi_guid_t *vendor,
-					 u32 *attributes)
-{
-	char *guid, *name, *end, c;
-	size_t name_len;
-	efi_uintn_t old_variable_name_size;
-	u64 time;
-	u16 *p;
-
-	guid = strchr(variable, '_');
-	if (!guid)
-		return EFI_INVALID_PARAMETER;
-	guid++;
-	name = strchr(guid, '_');
-	if (!name)
-		return EFI_INVALID_PARAMETER;
-	name++;
-	end = strchr(name, '=');
-	if (!end)
-		return EFI_INVALID_PARAMETER;
+		*attributes = var->attr;
+	if (timep)
+		*timep = var->time;

-	name_len = end - name;
-	old_variable_name_size = *variable_name_size;
-	*variable_name_size = sizeof(u16) * (name_len + 1);
-	if (old_variable_name_size < *variable_name_size)
+	old_size = *data_size;
+	*data_size = var->length;
+	if (old_size < var->length)
 		return EFI_BUFFER_TOO_SMALL;

-	end++; /* point to value */
-
-	/* variable name */
-	p = variable_name;
-	utf8_utf16_strncpy(&p, name, name_len);
-	variable_name[name_len] = 0;
-
-	/* guid */
-	c = *(name - 1);
-	*(name - 1) = '\0'; /* guid need be null-terminated here */
-	if (uuid_str_to_bin(guid, (unsigned char *)vendor,
-			    UUID_STR_FORMAT_GUID))
-		/* The only error would be EINVAL. */
+	if (!data)
 		return EFI_INVALID_PARAMETER;
-	*(name - 1) = c;

-	/* attributes */
-	parse_attr(end, attributes, &time);
+	for (pdata = var->name; *pdata; ++pdata)
+		;
+	++pdata;
+
+	efi_memcpy_runtime(data, pdata, var->length);

 	return EFI_SUCCESS;
 }

-efi_status_t efi_get_next_variable_name_int(efi_uintn_t *variable_name_size,
-					    u16 *variable_name,
-					    efi_guid_t *vendor)
+efi_status_t __efi_runtime
+efi_get_next_variable_name_int(efi_uintn_t *variable_name_size,
+			       u16 *variable_name, efi_guid_t *vendor)
 {
-	char *native_name, *variable;
-	ssize_t name_len, list_len;
-	char regex[256];
-	char * const regexlist[] = {regex};
-	u32 attributes;
-	int i;
-	efi_status_t ret;
+	struct efi_var_entry *var;
+	efi_uintn_t old_size;
+	u16 *pdata;

 	if (!variable_name_size || !variable_name || !vendor)
 		return EFI_INVALID_PARAMETER;

-	if (variable_name[0]) {
-		/* check null-terminated string */
-		for (i = 0; i < *variable_name_size; i++)
-			if (!variable_name[i])
-				break;
-		if (i >= *variable_name_size)
-			return EFI_INVALID_PARAMETER;
-
-		/* search for the last-returned variable */
-		ret = efi_to_native(&native_name, variable_name, vendor);
-		if (ret)
-			return ret;
+	efi_var_mem_find(vendor, variable_name, &var);

-		name_len = strlen(native_name);
-		for (variable = efi_variables_list; variable && *variable;) {
-			if (!strncmp(variable, native_name, name_len) &&
-			    variable[name_len] == '=')
-				break;
+	if (!var)
+		return EFI_NOT_FOUND;

-			variable = strchr(variable, '\n');
-			if (variable)
-				variable++;
-		}
+	for (pdata = var->name; *pdata; ++pdata)
+		;
+	++pdata;

-		free(native_name);
-		if (!(variable && *variable))
-			return EFI_INVALID_PARAMETER;
+	old_size = *variable_name_size;
+	*variable_name_size = (uintptr_t)pdata - (uintptr_t)var->name;

-		/* next variable */
-		variable = strchr(variable, '\n');
-		if (variable)
-			variable++;
-		if (!(variable && *variable))
-			return EFI_NOT_FOUND;
-	} else {
-		/*
-		 *new search: free a list used in the previous search
-		 */
-		free(efi_variables_list);
-		efi_variables_list = NULL;
-		efi_cur_variable = NULL;
-
-		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
-		list_len = hexport_r(&env_htab, '\n',
-				     H_MATCH_REGEX | H_MATCH_KEY,
-				     &efi_variables_list, 0, 1, regexlist);
-
-		if (list_len <= 1)
-			return EFI_NOT_FOUND;
-
-		variable = efi_variables_list;
-	}
+	if (old_size < *variable_name_size)
+		return EFI_BUFFER_TOO_SMALL;

-	ret = parse_uboot_variable(variable, variable_name_size, variable_name,
-				   vendor, &attributes);
+	efi_memcpy_runtime(variable_name, var->name, *variable_name_size);
+	efi_memcpy_runtime(vendor, &var->guid, sizeof(efi_guid_t));

-	return ret;
+	return EFI_SUCCESS;
 }

 efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
 				  u32 attributes, efi_uintn_t data_size,
 				  const void *data, bool ro_check)
 {
-	char *native_name = NULL, *old_data = NULL, *val = NULL, *s;
-	efi_uintn_t old_size;
+	struct efi_var_entry *var;
+	efi_uintn_t ret;
 	bool append, delete;
 	u64 time = 0;
-	u32 old_attr;
-	efi_status_t ret = EFI_SUCCESS;

 	if (!variable_name || !*variable_name || !vendor ||
 	    ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) &&
-	     !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) {
-		ret = EFI_INVALID_PARAMETER;
-		goto err;
-	}
-
-	ret = efi_to_native(&native_name, variable_name, vendor);
-	if (ret)
-		goto err;
+	     !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)))
+		return EFI_INVALID_PARAMETER;

 	/* check if a variable exists */
-	old_size = 0;
-	old_attr = 0;
-	ret = efi_get_variable_int(variable_name, vendor, &old_attr,
-				   &old_size, NULL, &time);
+	var = efi_var_mem_find(vendor, variable_name, NULL);
 	append = !!(attributes & EFI_VARIABLE_APPEND_WRITE);
 	attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE;
 	delete = !append && (!data_size || !attributes);

 	/* check attributes */
-	if (old_size) {
-		if (ro_check && (old_attr & EFI_VARIABLE_READ_ONLY)) {
-			ret = EFI_WRITE_PROTECTED;
-			goto err;
-		}
+	if (var) {
+		if (ro_check && (var->attr & EFI_VARIABLE_READ_ONLY))
+			return EFI_WRITE_PROTECTED;

 		/* attributes won't be changed */
 		if (!delete &&
-		    ((ro_check && old_attr != attributes) ||
-		     (!ro_check && ((old_attr & ~(u32)EFI_VARIABLE_READ_ONLY)
+		    ((ro_check && var->attr != attributes) ||
+		     (!ro_check && ((var->attr & ~(u32)EFI_VARIABLE_READ_ONLY)
 				    != (attributes & ~(u32)EFI_VARIABLE_READ_ONLY))))) {
-			ret = EFI_INVALID_PARAMETER;
-			goto err;
+			return EFI_INVALID_PARAMETER;
 		}
+		time = var->time;
 	} else {
-		if (delete || append) {
+		if (delete || append)
 			/*
 			 * Trying to delete or to update a non-existent
 			 * variable.
 			 */
-			ret = EFI_NOT_FOUND;
-			goto err;
-		}
+			return EFI_NOT_FOUND;
 	}

 	if (((!u16_strcmp(variable_name, L"PK") ||
@@ -869,27 +549,26 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
 		/* authentication is mandatory */
 		if (!(attributes &
 		      EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) {
-			EFI_PRINT("%ls: AUTHENTICATED_WRITE_ACCESS required\n",
+			EFI_PRINT("%ls: TIME_BASED_AUTHENTICATED_WRITE_ACCESS required\n",
 				  variable_name);
-			ret = EFI_INVALID_PARAMETER;
-			goto err;
+			return EFI_INVALID_PARAMETER;
 		}
 	}

 	/* authenticate a variable */
 	if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) {
-		if (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) {
-			ret = EFI_INVALID_PARAMETER;
-			goto err;
-		}
+		if (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS)
+			return EFI_INVALID_PARAMETER;
 		if (attributes &
 		    EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
+			u32 env_attr;
+
 			ret = efi_variable_authenticate(variable_name, vendor,
 							&data_size, &data,
-							attributes, &old_attr,
+							attributes, &env_attr,
 							&time);
 			if (ret != EFI_SUCCESS)
-				goto err;
+				return ret;

 			/* last chance to check for delete */
 			if (!data_size)
@@ -900,105 +579,46 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
 		    (EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS |
 		     EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) {
 			EFI_PRINT("Secure boot is not configured\n");
-			ret = EFI_INVALID_PARAMETER;
-			goto err;
+			return EFI_INVALID_PARAMETER;
 		}
 	}

-	/* delete a variable */
+	efi_var_mem_del(var);
 	if (delete) {
-		/* !old_size case has been handled before */
-		val = NULL;
+		/* EFI_NOT_FOUND has been handled before */
 		ret = EFI_SUCCESS;
 		goto out;
 	}

 	if (append) {
-		old_data = malloc(old_size);
-		if (!old_data) {
-			ret = EFI_OUT_OF_RESOURCES;
-			goto err;
-		}
-		ret = efi_get_variable_int(variable_name, vendor,
-					   &old_attr, &old_size, old_data, NULL);
-		if (ret != EFI_SUCCESS)
-			goto err;
+		u16 *old_data = var->name;
+
+		for (; *old_data; ++old_data)
+			;
+		++old_data;
+		ret = efi_var_mem_ins(variable_name, vendor, attributes,
+				      var->length, old_data, data_size, data,
+				      time);
 	} else {
-		old_size = 0;
+		ret = efi_var_mem_ins(variable_name, vendor, attributes,
+				      data_size, data, 0, NULL, time);
 	}

-	val = malloc(2 * old_size + 2 * data_size
-		     + strlen("{ro,run,boot,nv,time=0123456701234567}(blob)")
-		     + 1);
-	if (!val) {
-		ret = EFI_OUT_OF_RESOURCES;
-		goto err;
-	}
-
-	s = val;
-
-	/*
-	 * store attributes
-	 */
-	attributes &= (EFI_VARIABLE_READ_ONLY |
-		       EFI_VARIABLE_NON_VOLATILE |
-		       EFI_VARIABLE_BOOTSERVICE_ACCESS |
-		       EFI_VARIABLE_RUNTIME_ACCESS |
-		       EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS);
-	s += sprintf(s, "{");
-	for (u32 attr_rem = attributes; attr_rem;) {
-		u32 attr = 1 << (ffs(attr_rem) - 1);
-
-		if (attr == EFI_VARIABLE_READ_ONLY) {
-			s += sprintf(s, "ro");
-		} else if (attr == EFI_VARIABLE_NON_VOLATILE) {
-			s += sprintf(s, "nv");
-		} else if (attr == EFI_VARIABLE_BOOTSERVICE_ACCESS) {
-			s += sprintf(s, "boot");
-		} else if (attr == EFI_VARIABLE_RUNTIME_ACCESS) {
-			s += sprintf(s, "run");
-		} else if (attr ==
-			   EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
-			s += sprintf(s, "time=");
-			s = bin2hex(s, (u8 *)&time, sizeof(time));
-		}
-
-		attr_rem &= ~attr;
-		if (attr_rem)
-			s += sprintf(s, ",");
-	}
-	s += sprintf(s, "}");
-	s += sprintf(s, "(blob)");
-
-	/* store payload: */
-	if (append)
-		s = bin2hex(s, old_data, old_size);
-	s = bin2hex(s, data, data_size);
-	*s = '\0';
-
-	EFI_PRINT("setting: %s=%s\n", native_name, val);
+	if (ret != EFI_SUCCESS)
+		return ret;

 out:
-	if (env_set(native_name, val)) {
-		ret = EFI_DEVICE_ERROR;
-	} else {
-		if (!u16_strcmp(variable_name, L"PK"))
-			ret = efi_init_secure_state();
-		else
-			ret = EFI_SUCCESS;
-	}
+	if (!u16_strcmp(variable_name, L"PK"))
+		ret = efi_init_secure_state();
+	else
+		ret = EFI_SUCCESS;

 	/* Write non-volatile EFI variables to file */
 	if (attributes & EFI_VARIABLE_NON_VOLATILE &&
 	    ret == EFI_SUCCESS && efi_obj_list_initialized == EFI_SUCCESS)
 		efi_var_to_file();

-err:
-	free(native_name);
-	free(old_data);
-	free(val);
-
-	return ret;
+	return EFI_SUCCESS;
 }

 efi_status_t efi_query_variable_info_int(u32 attributes,
@@ -1006,7 +626,13 @@ efi_status_t efi_query_variable_info_int(u32 attributes,
 					 u64 *remaining_variable_storage_size,
 					 u64 *maximum_variable_size)
 {
-	return EFI_UNSUPPORTED;
+	*maximum_variable_storage_size = EFI_VAR_BUF_SIZE -
+					 sizeof(struct efi_var_file);
+	*remaining_variable_storage_size = efi_var_mem_free();
+	*maximum_variable_size = EFI_VAR_BUF_SIZE -
+				 sizeof(struct efi_var_file) -
+				 sizeof(struct efi_var_entry);
+	return EFI_SUCCESS;
 }

 /**
@@ -1108,6 +734,10 @@ efi_status_t efi_init_variables(void)
 {
 	efi_status_t ret;

+	ret = efi_var_mem_init();
+	if (ret != EFI_SUCCESS)
+		return ret;
+
 	ret = efi_init_secure_state();
 	if (ret != EFI_SUCCESS)
 		return ret;
--
2.27.0

[PATCH v2 16/17] efi_loader: enable UEFI variables at runtime

[Heinrich Schuchardt]

Enable UEFI variables at runtime.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_runtime.c  | 2 ++
 lib/efi_loader/efi_variable.c | 6 ++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 5b6506fbdc..91a4551448 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -121,6 +121,8 @@ efi_status_t efi_init_runtime_supported(void)
 	rt_table->version = EFI_RT_PROPERTIES_TABLE_VERSION;
 	rt_table->length = sizeof(struct efi_rt_properties_table);
 	rt_table->runtime_services_supported =
+				EFI_RT_SUPPORTED_GET_VARIABLE |
+				EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME |
 				EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
 				EFI_RT_SUPPORTED_CONVERT_POINTER;

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 13123e7e41..c472a054d0 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -672,7 +672,8 @@ static efi_status_t __efi_runtime EFIAPI
 efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *vendor,
 			 u32 *attributes, efi_uintn_t *data_size, void *data)
 {
-	return EFI_UNSUPPORTED;
+	return efi_get_variable_int(variable_name, vendor, attributes,
+				    data_size, data, NULL);
 }

 /**
@@ -688,7 +689,8 @@ static efi_status_t __efi_runtime EFIAPI
 efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
 				   u16 *variable_name, efi_guid_t *vendor)
 {
-	return EFI_UNSUPPORTED;
+	return efi_get_next_variable_name_int(variable_name_size, variable_name,
+					      vendor);
 }

 /**
--
2.27.0

[PATCH v2 17/17] efi_selftest: adjust runtime test for variables

[Heinrich Schuchardt]

As variable services are available at runtime we have to expect EFI_SUCCESS
when calling the services.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_selftest/efi_selftest_variables_runtime.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c
index b3b40ad2cf..3226069c0b 100644
--- a/lib/efi_selftest/efi_selftest_variables_runtime.c
+++ b/lib/efi_selftest/efi_selftest_variables_runtime.c
@@ -16,9 +16,7 @@

 static struct efi_boot_services *boottime;
 static struct efi_runtime_services *runtime;
-static const efi_guid_t guid_vendor0 =
-	EFI_GUID(0x67029eb5, 0x0af2, 0xf6b1,
-		 0xda, 0x53, 0xfc, 0xb5, 0x66, 0xdd, 0x1c, 0xe6);
+static const efi_guid_t guid_vendor0 = EFI_GLOBAL_VARIABLE_GUID;

 /*
  * Setup unit test.
@@ -68,17 +66,18 @@ static int execute(void)
 		efi_st_error("SetVariable failed\n");
 		return EFI_ST_FAILURE;
 	}
-	len = 3;
-	ret = runtime->get_variable(L"efi_st_var0", &guid_vendor0,
+	len = EFI_ST_MAX_DATA_SIZE;
+	ret = runtime->get_variable(L"PlatformLangCodes", &guid_vendor0,
 				    &attr, &len, data);
-	if (ret != EFI_UNSUPPORTED) {
+	if (ret != EFI_SUCCESS) {
 		efi_st_error("GetVariable failed\n");
 		return EFI_ST_FAILURE;
 	}
 	memset(&guid, 0, 16);
 	*varname = 0;
+	len = 2 * EFI_ST_MAX_VARNAME_SIZE;
 	ret = runtime->get_next_variable_name(&len, varname, &guid);
-	if (ret != EFI_UNSUPPORTED) {
+	if (ret != EFI_SUCCESS) {
 		efi_st_error("GetNextVariableName failed\n");
 		return EFI_ST_FAILURE;
 	}
--
2.27.0

[PATCH v2 16/17] efi_loader: enable UEFI variables at runtime

[ilias.apalodimas at linaro.org]

On Tue, Jul 07, 2020 at 05:11:59AM +0200, Heinrich Schuchardt wrote:
> Enable UEFI variables at runtime.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_runtime.c  | 2 ++
>  lib/efi_loader/efi_variable.c | 6 ++++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index 5b6506fbdc..91a4551448 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -121,6 +121,8 @@ efi_status_t efi_init_runtime_supported(void)
>  	rt_table->version = EFI_RT_PROPERTIES_TABLE_VERSION;
>  	rt_table->length = sizeof(struct efi_rt_properties_table);
>  	rt_table->runtime_services_supported =
> +				EFI_RT_SUPPORTED_GET_VARIABLE |
> +				EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME |
>  				EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
>  				EFI_RT_SUPPORTED_CONVERT_POINTER;
> 
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index 13123e7e41..c472a054d0 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -672,7 +672,8 @@ static efi_status_t __efi_runtime EFIAPI
>  efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *vendor,
>  			 u32 *attributes, efi_uintn_t *data_size, void *data)
>  {
> -	return EFI_UNSUPPORTED;
> +	return efi_get_variable_int(variable_name, vendor, attributes,
> +				    data_size, data, NULL);
>  }
> 
>  /**
> @@ -688,7 +689,8 @@ static efi_status_t __efi_runtime EFIAPI
>  efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
>  				   u16 *variable_name, efi_guid_t *vendor)
>  {
> -	return EFI_UNSUPPORTED;
> +	return efi_get_next_variable_name_int(variable_name_size, variable_name,
> +					      vendor);

Since we use the internal functions for the variables, don't we include the 
EFI_VARIABLE_READ_ONLY bit as well (which is different on the tee based
version). Wouldn't it be better to just remove the runtime variant completely?

Thanks
/Ilias
>  }
> 
>  /**
> --
> 2.27.0
> 

[PATCH v2 01/17] efi_loader: prepare for read only OP-TEE variables

[ilias.apalodimas at linaro.org]

Hi Heinrich, 

the TEE side looks fine to me, but I can't apply this to current master


Thanks
/Ilias
On Tue, Jul 07, 2020 at 05:11:44AM +0200, Heinrich Schuchardt wrote:
> We currently have two implementations of UEFI variables:
> 
> * variables provided via an OP-TEE module
> * variables stored in the U-Boot environment
> 
> Read only variables are up to now only implemented in the U-Boot
> environment implementation.
> 
> Provide a common interface for both implementations that allows handling
> read-only variables.
> 
> As variable access is limited to very few source files put variable
> related definitions into new include efi_variable.h instead of efi_loader.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  doc/api/efi.rst                   |   2 +
>  include/efi_variable.h            |  43 ++++++++
>  lib/efi_loader/Makefile           |   1 +
>  lib/efi_loader/efi_var_common.c   |  78 +++++++++++++
>  lib/efi_loader/efi_variable.c     | 175 ++++++++----------------------
>  lib/efi_loader/efi_variable_tee.c |  75 ++++---------
>  6 files changed, 193 insertions(+), 181 deletions(-)
>  create mode 100644 include/efi_variable.h
>  create mode 100644 lib/efi_loader/efi_var_common.c
> 
> diff --git a/doc/api/efi.rst b/doc/api/efi.rst
> index d5114f05b3..cb2a1c897e 100644
> --- a/doc/api/efi.rst
> +++ b/doc/api/efi.rst
> @@ -93,6 +93,8 @@ Runtime services
>  Variable services
>  ~~~~~~~~~~~~~~~~~
> 
> +.. kernel-doc:: include/efi_variable.h
> +   :internal:
>  .. kernel-doc:: lib/efi_loader/efi_variable.c
>     :internal:
> 
> diff --git a/include/efi_variable.h b/include/efi_variable.h
> new file mode 100644
> index 0000000000..6789118eba
> --- /dev/null
> +++ b/include/efi_variable.h
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2020, Heinrich Schuchardt <xypron.glpk@gmx.de>
> + */
> +
> +#ifndef _EFI_VARIABLE_H
> +#define _EFI_VARIABLE_H
> +
> +#include <linux/bitops.h>
> +
> +#define EFI_VARIABLE_READ_ONLY BIT(31)
> +
> +/**
> + * efi_get_variable() - retrieve value of a UEFI variable
> + *
> + * @variable_name:	name of the variable
> + * @vendor:		vendor GUID
> + * @attributes:		attributes of the variable
> + * @data_size:		size of the buffer to which the variable value is copied
> + * @data:		buffer to which the variable value is copied
> + * @timep:		authentication time (seconds since start of epoch)
> + * Return:		status code
> + */
> +efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> +				  u32 *attributes, efi_uintn_t *data_size,
> +				  void *data, u64 *timep);
> +
> +/**
> + * efi_set_variable() - set value of a UEFI variable
> + *
> + * @variable_name:	name of the variable
> + * @vendor:		vendor GUID
> + * @attributes:		attributes of the variable
> + * @data_size:		size of the buffer with the variable value
> + * @data:		buffer with the variable value
> + * @ro_check:		check the read only read only bit in attributes
> + * Return:		status code
> + */
> +efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> +				  u32 attributes, efi_uintn_t data_size,
> +				  const void *data, bool ro_check);
> +
> +#endif
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 57c7e66ea0..7eddd7ef37 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -35,6 +35,7 @@ obj-y += efi_root_node.o
>  obj-y += efi_runtime.o
>  obj-y += efi_setup.o
>  obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += efi_unicode_collation.o
> +obj-y += efi_var_common.o
>  ifeq ($(CONFIG_EFI_MM_COMM_TEE),y)
>  obj-y += efi_variable_tee.o
>  else
> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> new file mode 100644
> index 0000000000..6a4efa3f27
> --- /dev/null
> +++ b/lib/efi_loader/efi_var_common.c
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * UEFI runtime variable services
> + *
> + * Copyright (c) 2020, Heinrich Schuchardt <xypron.glpk@gmx.de>
> + */
> +
> +#include <common.h>
> +#include <efi_loader.h>
> +#include <efi_variable.h>
> +
> +/**
> + * efi_efi_get_variable() - retrieve value of a UEFI variable
> + *
> + * This function implements the GetVariable runtime service.
> + *
> + * See the Unified Extensible Firmware Interface (UEFI) specification for
> + * details.
> + *
> + * @variable_name:	name of the variable
> + * @vendor:		vendor GUID
> + * @attributes:		attributes of the variable
> + * @data_size:		size of the buffer to which the variable value is copied
> + * @data:		buffer to which the variable value is copied
> + * Return:		status code
> + */
> +efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> +				     const efi_guid_t *vendor, u32 *attributes,
> +				     efi_uintn_t *data_size, void *data)
> +{
> +	efi_status_t ret;
> +
> +	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
> +		  data_size, data);
> +
> +	ret = efi_get_variable_int(variable_name, vendor, attributes,
> +				   data_size, data, NULL);
> +
> +	/* Remove EFI_VARIABLE_READ_ONLY flag */
> +	if (attributes)
> +		*attributes &= EFI_VARIABLE_MASK;
> +
> +	return EFI_EXIT(ret);
> +}
> +
> +/**
> + * efi_set_variable() - set value of a UEFI variable
> + *
> + * This function implements the SetVariable runtime service.
> + *
> + * See the Unified Extensible Firmware Interface (UEFI) specification for
> + * details.
> + *
> + * @variable_name:	name of the variable
> + * @vendor:		vendor GUID
> + * @attributes:		attributes of the variable
> + * @data_size:		size of the buffer with the variable value
> + * @data:		buffer with the variable value
> + * Return:		status code
> + */
> +efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
> +				     const efi_guid_t *vendor, u32 attributes,
> +				     efi_uintn_t data_size, const void *data)
> +{
> +	efi_status_t ret;
> +
> +	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
> +		  data_size, data);
> +
> +	/* Make sure that the EFI_VARIABLE_READ_ONLY flag is not set */
> +	if (attributes & ~(u32)EFI_VARIABLE_MASK)
> +		ret = EFI_INVALID_PARAMETER;
> +	else
> +		ret = efi_set_variable_int(variable_name, vendor, attributes,
> +					   data_size, data, true);
> +
> +	return EFI_EXIT(ret);
> +}
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index efaba869ef..6ec1f97326 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -7,6 +7,7 @@
> 
>  #include <common.h>
>  #include <efi_loader.h>
> +#include <efi_variable.h>
>  #include <env.h>
>  #include <env_internal.h>
>  #include <hexdump.h>
> @@ -15,7 +16,6 @@
>  #include <search.h>
>  #include <uuid.h>
>  #include <crypto/pkcs7_parser.h>
> -#include <linux/bitops.h>
>  #include <linux/compat.h>
>  #include <u-boot/crc.h>
> 
> @@ -30,21 +30,6 @@ static bool efi_secure_boot;
>  static enum efi_secure_mode efi_secure_mode;
>  static u8 efi_vendor_keys;
> 
> -#define READ_ONLY BIT(31)
> -
> -static efi_status_t efi_get_variable_common(u16 *variable_name,
> -					    const efi_guid_t *vendor,
> -					    u32 *attributes,
> -					    efi_uintn_t *data_size, void *data,
> -					    u64 *timep);
> -
> -static efi_status_t efi_set_variable_common(u16 *variable_name,
> -					    const efi_guid_t *vendor,
> -					    u32 attributes,
> -					    efi_uintn_t data_size,
> -					    const void *data,
> -					    bool ro_check);
> -
>  /*
>   * Mapping between EFI variables and u-boot variables:
>   *
> @@ -155,7 +140,7 @@ static const char *parse_attr(const char *str, u32 *attrp, u64 *timep)
>  		str++;
> 
>  		if ((s = prefix(str, "ro"))) {
> -			attr |= READ_ONLY;
> +			attr |= EFI_VARIABLE_READ_ONLY;
>  		} else if ((s = prefix(str, "nv"))) {
>  			attr |= EFI_VARIABLE_NON_VOLATILE;
>  		} else if ((s = prefix(str, "boot"))) {
> @@ -203,29 +188,29 @@ static efi_status_t efi_set_secure_state(u8 secure_boot, u8 setup_mode,
> 
>  	attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
>  		     EFI_VARIABLE_RUNTIME_ACCESS |
> -		     READ_ONLY;
> -	ret = efi_set_variable_common(L"SecureBoot", &efi_global_variable_guid,
> -				      attributes, sizeof(secure_boot),
> -				      &secure_boot, false);
> +		     EFI_VARIABLE_READ_ONLY;
> +	ret = efi_set_variable_int(L"SecureBoot", &efi_global_variable_guid,
> +				   attributes, sizeof(secure_boot),
> +				   &secure_boot, false);
>  	if (ret != EFI_SUCCESS)
>  		goto err;
> 
> -	ret = efi_set_variable_common(L"SetupMode", &efi_global_variable_guid,
> -				      attributes, sizeof(setup_mode),
> -				      &setup_mode, false);
> +	ret = efi_set_variable_int(L"SetupMode", &efi_global_variable_guid,
> +				   attributes, sizeof(setup_mode),
> +				   &setup_mode, false);
>  	if (ret != EFI_SUCCESS)
>  		goto err;
> 
> -	ret = efi_set_variable_common(L"AuditMode", &efi_global_variable_guid,
> -				      attributes, sizeof(audit_mode),
> -				      &audit_mode, false);
> +	ret = efi_set_variable_int(L"AuditMode", &efi_global_variable_guid,
> +				   attributes, sizeof(audit_mode),
> +				   &audit_mode, false);
>  	if (ret != EFI_SUCCESS)
>  		goto err;
> 
> -	ret = efi_set_variable_common(L"DeployedMode",
> -				      &efi_global_variable_guid, attributes,
> -				      sizeof(deployed_mode), &deployed_mode,
> -				      false);
> +	ret = efi_set_variable_int(L"DeployedMode",
> +				   &efi_global_variable_guid, attributes,
> +				   sizeof(deployed_mode), &deployed_mode,
> +				   false);
>  err:
>  	return ret;
>  }
> @@ -235,7 +220,7 @@ err:
>   * @mode:	new state
>   *
>   * Depending on @mode, secure boot related variables are updated.
> - * Those variables are *read-only* for users, efi_set_variable_common()
> + * Those variables are *read-only* for users, efi_set_variable_int()
>   * is called here.
>   *
>   * Return:	status code
> @@ -254,10 +239,10 @@ static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode)
> 
>  		efi_secure_boot = true;
>  	} else if (mode == EFI_MODE_AUDIT) {
> -		ret = efi_set_variable_common(L"PK", &efi_global_variable_guid,
> -					      EFI_VARIABLE_BOOTSERVICE_ACCESS |
> -					      EFI_VARIABLE_RUNTIME_ACCESS,
> -					      0, NULL, false);
> +		ret = efi_set_variable_int(L"PK", &efi_global_variable_guid,
> +					   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +					   EFI_VARIABLE_RUNTIME_ACCESS,
> +					   0, NULL, false);
>  		if (ret != EFI_SUCCESS)
>  			goto err;
> 
> @@ -309,8 +294,8 @@ static efi_status_t efi_init_secure_state(void)
>  	 */
> 
>  	size = 0;
> -	ret = efi_get_variable_common(L"PK", &efi_global_variable_guid,
> -				      NULL, &size, NULL, NULL);
> +	ret = efi_get_variable_int(L"PK", &efi_global_variable_guid,
> +				   NULL, &size, NULL, NULL);
>  	if (ret == EFI_BUFFER_TOO_SMALL) {
>  		if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT))
>  			mode = EFI_MODE_USER;
> @@ -327,13 +312,13 @@ static efi_status_t efi_init_secure_state(void)
> 
>  	ret = efi_transfer_secure_state(mode);
>  	if (ret == EFI_SUCCESS)
> -		ret = efi_set_variable_common(L"VendorKeys",
> -					      &efi_global_variable_guid,
> -					      EFI_VARIABLE_BOOTSERVICE_ACCESS |
> -					      EFI_VARIABLE_RUNTIME_ACCESS |
> -					      READ_ONLY,
> -					      sizeof(efi_vendor_keys),
> -					      &efi_vendor_keys, false);
> +		ret = efi_set_variable_int(L"VendorKeys",
> +					   &efi_global_variable_guid,
> +					   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +					   EFI_VARIABLE_RUNTIME_ACCESS |
> +					   EFI_VARIABLE_READ_ONLY,
> +					   sizeof(efi_vendor_keys),
> +					   &efi_vendor_keys, false);
> 
>  err:
>  	return ret;
> @@ -599,11 +584,9 @@ static efi_status_t efi_variable_authenticate(u16 *variable,
>  }
>  #endif /* CONFIG_EFI_SECURE_BOOT */
> 
> -static efi_status_t efi_get_variable_common(u16 *variable_name,
> -					    const efi_guid_t *vendor,
> -					    u32 *attributes,
> -					    efi_uintn_t *data_size, void *data,
> -					    u64 *timep)
> +efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> +				  u32 *attributes, efi_uintn_t *data_size,
> +				  void *data, u64 *timep)
>  {
>  	char *native_name;
>  	efi_status_t ret;
> @@ -684,40 +667,11 @@ static efi_status_t efi_get_variable_common(u16 *variable_name,
> 
>  out:
>  	if (attributes)
> -		*attributes = attr & EFI_VARIABLE_MASK;
> +		*attributes = attr;
> 
>  	return ret;
>  }
> 
> -/**
> - * efi_efi_get_variable() - retrieve value of a UEFI variable
> - *
> - * This function implements the GetVariable runtime service.
> - *
> - * See the Unified Extensible Firmware Interface (UEFI) specification for
> - * details.
> - *
> - * @variable_name:	name of the variable
> - * @vendor:		vendor GUID
> - * @attributes:		attributes of the variable
> - * @data_size:		size of the buffer to which the variable value is copied
> - * @data:		buffer to which the variable value is copied
> - * Return:		status code
> - */
> -efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> -				     const efi_guid_t *vendor, u32 *attributes,
> -				     efi_uintn_t *data_size, void *data)
> -{
> -	efi_status_t ret;
> -
> -	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
> -		  data_size, data);
> -
> -	ret = efi_get_variable_common(variable_name, vendor, attributes,
> -				      data_size, data, NULL);
> -	return EFI_EXIT(ret);
> -}
> -
>  static char *efi_variables_list;
>  static char *efi_cur_variable;
> 
> @@ -881,12 +835,9 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
>  	return EFI_EXIT(ret);
>  }
> 
> -static efi_status_t efi_set_variable_common(u16 *variable_name,
> -					    const efi_guid_t *vendor,
> -					    u32 attributes,
> -					    efi_uintn_t data_size,
> -					    const void *data,
> -					    bool ro_check)
> +efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> +				  u32 attributes, efi_uintn_t data_size,
> +				  const void *data, bool ro_check)
>  {
>  	char *native_name = NULL, *old_data = NULL, *val = NULL, *s;
>  	efi_uintn_t old_size;
> @@ -909,15 +860,15 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
>  	/* check if a variable exists */
>  	old_size = 0;
>  	attr = 0;
> -	ret = efi_get_variable_common(variable_name, vendor, &attr,
> -				      &old_size, NULL, &time);
> +	ret = efi_get_variable_int(variable_name, vendor, &attr,
> +				   &old_size, NULL, &time);
>  	append = !!(attributes & EFI_VARIABLE_APPEND_WRITE);
>  	attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE;
>  	delete = !append && (!data_size || !attributes);
> 
>  	/* check attributes */
>  	if (old_size) {
> -		if (ro_check && (attr & READ_ONLY)) {
> +		if (ro_check && (attr & EFI_VARIABLE_READ_ONLY)) {
>  			ret = EFI_WRITE_PROTECTED;
>  			goto err;
>  		}
> @@ -925,8 +876,8 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
>  		/* attributes won't be changed */
>  		if (!delete &&
>  		    ((ro_check && attr != attributes) ||
> -		     (!ro_check && ((attr & ~(u32)READ_ONLY)
> -				    != (attributes & ~(u32)READ_ONLY))))) {
> +		     (!ro_check && ((attr & ~(u32)EFI_VARIABLE_READ_ONLY)
> +				    != (attributes & ~(u32)EFI_VARIABLE_READ_ONLY))))) {
>  			ret = EFI_INVALID_PARAMETER;
>  			goto err;
>  		}
> @@ -1000,8 +951,8 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
>  			ret = EFI_OUT_OF_RESOURCES;
>  			goto err;
>  		}
> -		ret = efi_get_variable_common(variable_name, vendor,
> -					      &attr, &old_size, old_data, NULL);
> +		ret = efi_get_variable_int(variable_name, vendor,
> +					   &attr, &old_size, old_data, NULL);
>  		if (ret != EFI_SUCCESS)
>  			goto err;
>  	} else {
> @@ -1021,7 +972,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
>  	/*
>  	 * store attributes
>  	 */
> -	attributes &= (READ_ONLY |
> +	attributes &= (EFI_VARIABLE_READ_ONLY |
>  		       EFI_VARIABLE_NON_VOLATILE |
>  		       EFI_VARIABLE_BOOTSERVICE_ACCESS |
>  		       EFI_VARIABLE_RUNTIME_ACCESS |
> @@ -1030,7 +981,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name,
>  	while (attributes) {
>  		attr = 1 << (ffs(attributes) - 1);
> 
> -		if (attr == READ_ONLY) {
> +		if (attr == EFI_VARIABLE_READ_ONLY) {
>  			s += sprintf(s, "ro");
>  		} else if (attr == EFI_VARIABLE_NON_VOLATILE) {
>  			s += sprintf(s, "nv");
> @@ -1084,12 +1035,12 @@ out:
>  		/* update VendorKeys */
>  		if (vendor_keys_modified & efi_vendor_keys) {
>  			efi_vendor_keys = 0;
> -			ret = efi_set_variable_common(
> +			ret = efi_set_variable_int(
>  						L"VendorKeys",
>  						&efi_global_variable_guid,
>  						EFI_VARIABLE_BOOTSERVICE_ACCESS
>  						 | EFI_VARIABLE_RUNTIME_ACCESS
> -						 | READ_ONLY,
> +						 | EFI_VARIABLE_READ_ONLY,
>  						sizeof(efi_vendor_keys),
>  						&efi_vendor_keys,
>  						false);
> @@ -1106,36 +1057,6 @@ err:
>  	return ret;
>  }
> 
> -/**
> - * efi_set_variable() - set value of a UEFI variable
> - *
> - * This function implements the SetVariable runtime service.
> - *
> - * See the Unified Extensible Firmware Interface (UEFI) specification for
> - * details.
> - *
> - * @variable_name:	name of the variable
> - * @vendor:		vendor GUID
> - * @attributes:		attributes of the variable
> - * @data_size:		size of the buffer with the variable value
> - * @data:		buffer with the variable value
> - * Return:		status code
> - */
> -efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
> -				     const efi_guid_t *vendor, u32 attributes,
> -				     efi_uintn_t data_size, const void *data)
> -{
> -	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
> -		  data_size, data);
> -
> -	/* READ_ONLY bit is not part of API */
> -	attributes &= ~(u32)READ_ONLY;
> -
> -	return EFI_EXIT(efi_set_variable_common(variable_name, vendor,
> -						attributes, data_size, data,
> -						true));
> -}
> -
>  /**
>   * efi_query_variable_info() - get information about EFI variables
>   *
> diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> index cacc76e23d..d5a28163f5 100644
> --- a/lib/efi_loader/efi_variable_tee.c
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -10,6 +10,7 @@
>  #include <efi.h>
>  #include <efi_api.h>
>  #include <efi_loader.h>
> +#include <efi_variable.h>
>  #include <tee.h>
>  #include <malloc.h>
>  #include <mm_communication.h>
> @@ -243,24 +244,9 @@ out:
>  	return ret;
>  }
> 
> -/**
> - * efi_get_variable() - retrieve value of a UEFI variable
> - *
> - * This function implements the GetVariable runtime service.
> - *
> - * See the Unified Extensible Firmware Interface (UEFI) specification for
> - * details.
> - *
> - * @name:		name of the variable
> - * @guid:		vendor GUID
> - * @attr:		attributes of the variable
> - * @data_size:		size of the buffer to which the variable value is copied
> - * @data:		buffer to which the variable value is copied
> - * Return:		status code
> - */
> -efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
> -				     u32 *attr, efi_uintn_t *data_size,
> -				     void *data)
> +efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> +				  u32 *attributes, efi_uintn_t *data_size,
> +				  void *data, u64 *timep)
>  {
>  	struct smm_variable_access *var_acc;
>  	efi_uintn_t payload_size;
> @@ -269,15 +255,13 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
>  	u8 *comm_buf = NULL;
>  	efi_status_t ret;
> 
> -	EFI_ENTRY("\"%ls\" %pUl %p %p %p", name, guid, attr, data_size, data);
> -
> -	if (!name || !guid || !data_size) {
> +	if (!variable_name || !vendor || !data_size) {
>  		ret = EFI_INVALID_PARAMETER;
>  		goto out;
>  	}
> 
>  	/* Check payload size */
> -	name_size = u16_strsize(name);
> +	name_size = u16_strsize(variable_name);
>  	if (name_size > max_payload_size - MM_VARIABLE_ACCESS_HEADER_SIZE) {
>  		ret = EFI_INVALID_PARAMETER;
>  		goto out;
> @@ -300,11 +284,11 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
>  		goto out;
> 
>  	/* Fill in contents */
> -	guidcpy(&var_acc->guid, guid);
> +	guidcpy(&var_acc->guid, vendor);
>  	var_acc->data_size = tmp_dsize;
>  	var_acc->name_size = name_size;
> -	var_acc->attr = attr ? *attr : 0;
> -	memcpy(var_acc->name, name, name_size);
> +	var_acc->attr = attributes ? *attributes : 0;
> +	memcpy(var_acc->name, variable_name, name_size);
> 
>  	/* Communicate */
>  	ret = mm_communicate(comm_buf, payload_size);
> @@ -315,8 +299,8 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
>  	if (ret != EFI_SUCCESS)
>  		goto out;
> 
> -	if (attr)
> -		*attr = var_acc->attr;
> +	if (attributes)
> +		*attributes = var_acc->attr;
>  	if (data)
>  		memcpy(data, (u8 *)var_acc->name + var_acc->name_size,
>  		       var_acc->data_size);
> @@ -325,7 +309,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
> 
>  out:
>  	free(comm_buf);
> -	return EFI_EXIT(ret);
> +	return ret;
>  }
> 
>  /**
> @@ -417,24 +401,9 @@ out:
>  	return EFI_EXIT(ret);
>  }
> 
> -/**
> - * efi_set_variable() - set value of a UEFI variable
> - *
> - * This function implements the SetVariable runtime service.
> - *
> - * See the Unified Extensible Firmware Interface (UEFI) specification for
> - * details.
> - *
> - * @name:		name of the variable
> - * @guid:		vendor GUID
> - * @attr:		attributes of the variable
> - * @data_size:		size of the buffer with the variable value
> - * @data:		buffer with the variable value
> - * Return:		status code
> - */
> -efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
> -				     u32 attr, efi_uintn_t data_size,
> -				     const void *data)
> +efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
> +				  u32 attributes, efi_uintn_t data_size,
> +				  const void *data, bool ro_check)
>  {
>  	struct smm_variable_access *var_acc;
>  	efi_uintn_t payload_size;
> @@ -442,9 +411,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
>  	u8 *comm_buf = NULL;
>  	efi_status_t ret;
> 
> -	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", name, guid, attr, data_size, data);
> -
> -	if (!name || name[0] == 0 || !guid) {
> +	if (!variable_name || variable_name[0] == 0 || !vendor) {
>  		ret = EFI_INVALID_PARAMETER;
>  		goto out;
>  	}
> @@ -454,7 +421,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
>  	}
> 
>  	/* Check payload size */
> -	name_size = u16_strsize(name);
> +	name_size = u16_strsize(variable_name);
>  	payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + data_size;
>  	if (payload_size > max_payload_size) {
>  		ret = EFI_INVALID_PARAMETER;
> @@ -468,11 +435,11 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
>  		goto out;
> 
>  	/* Fill in contents */
> -	guidcpy(&var_acc->guid, guid);
> +	guidcpy(&var_acc->guid, vendor);
>  	var_acc->data_size = data_size;
>  	var_acc->name_size = name_size;
> -	var_acc->attr = attr;
> -	memcpy(var_acc->name, name, name_size);
> +	var_acc->attr = attributes;
> +	memcpy(var_acc->name, variable_name, name_size);
>  	memcpy((u8 *)var_acc->name + name_size, data, data_size);
> 
>  	/* Communicate */
> @@ -480,7 +447,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
> 
>  out:
>  	free(comm_buf);
> -	return EFI_EXIT(ret);
> +	return ret;
>  }
> 
>  /**
> --
> 2.27.0
> 

[PATCH v2 16/17] efi_loader: enable UEFI variables at runtime

[Ilias Apalodimas]

On Tue, 7 Jul 2020 at 16:10, <ilias.apalodimas@linaro.org> wrote:
>
> On Tue, Jul 07, 2020 at 05:11:59AM +0200, Heinrich Schuchardt wrote:
> > Enable UEFI variables at runtime.
> >
> > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > ---
> >  lib/efi_loader/efi_runtime.c  | 2 ++
> >  lib/efi_loader/efi_variable.c | 6 ++++--
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> > index 5b6506fbdc..91a4551448 100644
> > --- a/lib/efi_loader/efi_runtime.c
> > +++ b/lib/efi_loader/efi_runtime.c
> > @@ -121,6 +121,8 @@ efi_status_t efi_init_runtime_supported(void)
> >       rt_table->version = EFI_RT_PROPERTIES_TABLE_VERSION;
> >       rt_table->length = sizeof(struct efi_rt_properties_table);
> >       rt_table->runtime_services_supported =
> > +                             EFI_RT_SUPPORTED_GET_VARIABLE |
> > +                             EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME |
> >                               EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
> >                               EFI_RT_SUPPORTED_CONVERT_POINTER;
> >
> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > index 13123e7e41..c472a054d0 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -672,7 +672,8 @@ static efi_status_t __efi_runtime EFIAPI
> >  efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *vendor,
> >                        u32 *attributes, efi_uintn_t *data_size, void *data)
> >  {
> > -     return EFI_UNSUPPORTED;
> > +     return efi_get_variable_int(variable_name, vendor, attributes,
> > +                                 data_size, data, NULL);
> >  }
> >
> >  /**
> > @@ -688,7 +689,8 @@ static efi_status_t __efi_runtime EFIAPI
> >  efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
> >                                  u16 *variable_name, efi_guid_t *vendor)
> >  {
> > -     return EFI_UNSUPPORTED;
> > +     return efi_get_next_variable_name_int(variable_name_size, variable_name,
> > +                                           vendor);
>
> Since we use the internal functions for the variables, don't we include the
> EFI_VARIABLE_READ_ONLY bit as well (which is different on the tee based
> version). Wouldn't it be better to just remove the runtime variant completely?

Replying to myself on that. Removing is not possible since I just
noticed the EFI_ENTRY is still
present on the non-runtime variants. The RO flag needs to be handled though.

Thanks
/Ilias
>
> Thanks
> /Ilias
> >  }
> >
> >  /**
> > --
> > 2.27.0
> >

[PATCH v2 16/17] efi_loader: enable UEFI variables at runtime

[Heinrich Schuchardt]

On 07.07.20 15:30, Ilias Apalodimas wrote:
> On Tue, 7 Jul 2020 at 16:10, <ilias.apalodimas@linaro.org> wrote:
>>
>> On Tue, Jul 07, 2020 at 05:11:59AM +0200, Heinrich Schuchardt wrote:
>>> Enable UEFI variables at runtime.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> ---
>>>  lib/efi_loader/efi_runtime.c  | 2 ++
>>>  lib/efi_loader/efi_variable.c | 6 ++++--
>>>  2 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>>> index 5b6506fbdc..91a4551448 100644
>>> --- a/lib/efi_loader/efi_runtime.c
>>> +++ b/lib/efi_loader/efi_runtime.c
>>> @@ -121,6 +121,8 @@ efi_status_t efi_init_runtime_supported(void)
>>>       rt_table->version = EFI_RT_PROPERTIES_TABLE_VERSION;
>>>       rt_table->length = sizeof(struct efi_rt_properties_table);
>>>       rt_table->runtime_services_supported =
>>> +                             EFI_RT_SUPPORTED_GET_VARIABLE |
>>> +                             EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME |
>>>                               EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
>>>                               EFI_RT_SUPPORTED_CONVERT_POINTER;
>>>
>>> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
>>> index 13123e7e41..c472a054d0 100644
>>> --- a/lib/efi_loader/efi_variable.c
>>> +++ b/lib/efi_loader/efi_variable.c
>>> @@ -672,7 +672,8 @@ static efi_status_t __efi_runtime EFIAPI
>>>  efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *vendor,
>>>                        u32 *attributes, efi_uintn_t *data_size, void *data)
>>>  {
>>> -     return EFI_UNSUPPORTED;
>>> +     return efi_get_variable_int(variable_name, vendor, attributes,
>>> +                                 data_size, data, NULL);
>>>  }
>>>
>>>  /**
>>> @@ -688,7 +689,8 @@ static efi_status_t __efi_runtime EFIAPI
>>>  efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
>>>                                  u16 *variable_name, efi_guid_t *vendor)
>>>  {
>>> -     return EFI_UNSUPPORTED;
>>> +     return efi_get_next_variable_name_int(variable_name_size, variable_name,
>>> +                                           vendor);
>>
>> Since we use the internal functions for the variables, don't we include the
>> EFI_VARIABLE_READ_ONLY bit as well (which is different on the tee based
>> version). Wouldn't it be better to just remove the runtime variant completely?
>
> Replying to myself on that. Removing is not possible since I just
> noticed the EFI_ENTRY is still
> present on the non-runtime variants. The RO flag needs to be handled though.

Thanks for reviewing. Yes we have to mask the RO bit.

Best regards

Heinrich