[U-Boot] [PATCH 0/4] efi_loader: non-volatile variables support

[U-Boot] [PATCH 0/4] efi_loader: non-volatile variables support

[AKASHI Takahiro]

As the subject suggested, this patch set allows any efi variable to be
volatile or non-volatile as UEFI specification describes.

With my efishell patch[1] with patch #2, you can try as follows:
  => efi setvar PlatformLang en
  => efi setvar -nv BootNext =H0200
  => env save

BootNext will be preserved across reboot, while PlatformLang not.

Please note that, currently, setvar command does not automatically
append NON_VOLATILE attribute, while UEFI specification expects that
PlatformLang be non-volatile, you'd better also specify -nv for
this variable here.

Patch #2/#3 depend on my efishell patch[1].
Patch #4 depends on my BootNext patch[2].

Patch[1] and [2] have not been merged yet, so patch#1 can be applied 
on its own.

[1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
[2] https://lists.denx.de/pipermail/u-boot/2018-November/349281.html

AKASHI Takahiro (4):
  efi_loader: support non-volatile variable behavior
  cmd: efishell: support -nv option to setvar sub-command
  cmd: efishell: make Boot####/BootOrder variable non-volatile
  efi_loader: bootmgr: make BootNext non-volatile

 cmd/efishell.c                | 20 ++++++++---
 env/env.c                     |  4 +++
 include/efi_loader.h          |  1 +
 lib/efi_loader/efi_bootmgr.c  |  3 +-
 lib/efi_loader/efi_variable.c | 64 +++++++++++++++++++++++++++++++++--
 5 files changed, 84 insertions(+), 8 deletions(-)

-- 
2.19.1

[U-Boot] [PATCH 1/4] efi_loader: support non-volatile variable behavior

[AKASHI Takahiro]

An EFI variable is nothing but a wrapper of a corresponding u-boot
environment variable (See efi_variable.c), but under the current
implementation, NON_VOLATILE attribute is not honored while u-boot
environment variables can be saved/restored in storage.

With this patch, the expected semantics will be mimicked by deleting
all the EFI variables *without* NON_VOLATILE attribute when loading
them from storage at boot time.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 env/env.c                     |  4 +++
 include/efi_loader.h          |  1 +
 lib/efi_loader/efi_variable.c | 64 +++++++++++++++++++++++++++++++++--
 3 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/env/env.c b/env/env.c
index afed0f3c95c3..c507a4ac5f78 100644
--- a/env/env.c
+++ b/env/env.c
@@ -5,6 +5,7 @@
  */
 
 #include <common.h>
+#include <efi_loader.h>
 #include <environment.h>
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -195,6 +196,9 @@ int env_load(void)
 		if (ret) {
 			debug("Failed (%d)\n", ret);
 		} else {
+#ifdef CONFIG_EFI_LOADER
+			efi_purge_volatile_variables();
+#endif
 			printf("OK\n");
 			return 0;
 		}
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 9f7a4068efa6..9cad1dcd62bb 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -533,6 +533,7 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
 efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_guid_t *vendor,
 				     u32 attributes, efi_uintn_t data_size,
 				     void *data);
+int efi_purge_volatile_variables(void);
 
 /*
  * See section 3.1.3 in the v2.7 UEFI spec for more details on
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 19d9cb865f25..ad8cd36fa1e1 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -8,6 +8,8 @@
 #include <malloc.h>
 #include <charset.h>
 #include <efi_loader.h>
+#include <environment.h>
+#include <search.h>
 
 #define READ_ONLY BIT(31)
 
@@ -142,6 +144,8 @@ static const char *parse_attr(const char *str, u32 *attrp)
 
 		if ((s = prefix(str, "ro"))) {
 			attr |= 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"))) {
@@ -293,7 +297,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_guid_t *vendor,
 		}
 	}
 
-	val = malloc(2 * data_size + strlen("{ro,run,boot}(blob)") + 1);
+	val = malloc(2 * data_size + strlen("{ro,run,boot,nv}(blob)") + 1);
 	if (!val) {
 		ret = EFI_OUT_OF_RESOURCES;
 		goto out;
@@ -302,12 +306,16 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_guid_t *vendor,
 	s = val;
 
 	/* store attributes: */
-	attributes &= (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS);
+	attributes &= (EFI_VARIABLE_NON_VOLATILE |
+		       EFI_VARIABLE_BOOTSERVICE_ACCESS |
+		       EFI_VARIABLE_RUNTIME_ACCESS);
 	s += sprintf(s, "{");
 	while (attributes) {
 		u32 attr = 1 << (ffs(attributes) - 1);
 
-		if (attr == EFI_VARIABLE_BOOTSERVICE_ACCESS)
+		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");
@@ -334,3 +342,53 @@ out:
 
 	return EFI_EXIT(ret);
 }
+
+/*
+ * Purge all the variables which are not marked non volatile.
+ * This function is assumed to be called only once@boot time.
+ */
+int efi_purge_volatile_variables(void)
+{
+	char regex[256];
+	char * const regexlist[] = {regex};
+	char *list = NULL, *name, *value;
+	int len, ret = 0;
+	u32 attr;
+
+	snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
+
+	len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
+			&list, 0, 1, regexlist);
+
+	if (len < 0)
+		return -1;
+	else if (!len)
+		return 0;
+
+	name = list;
+	while (*name) {
+		/* variable name */
+		value = strchr(name, '=');
+		if (!value)
+			break;
+		*value = '\0';
+		value++;
+
+		parse_attr(value, &attr);
+		if (!(attr & EFI_VARIABLE_NON_VOLATILE)) {
+			if (env_set(name, NULL)) {
+				printf("cannot purge efi variable: %s\n", name);
+				ret = -1;
+			}
+		}
+
+		name = strchr(value, '\n');
+		if (!name)
+			break;
+		name++;
+	}
+
+	free(list);
+
+	return ret;
+}
-- 
2.19.1

[U-Boot] [PATCH 1/4] efi_loader: support non-volatile variable behavior

[Heinrich Schuchardt]

On 11/28/18 7:00 AM, AKASHI Takahiro wrote:
> An EFI variable is nothing but a wrapper of a corresponding u-boot
> environment variable (See efi_variable.c), but under the current
> implementation, NON_VOLATILE attribute is not honored while u-boot
> environment variables can be saved/restored in storage.
> 
> With this patch, the expected semantics will be mimicked by deleting
> all the EFI variables *without* NON_VOLATILE attribute when loading
> them from storage at boot time.

This patch would only be needed if we would store non-volatile
variables. So here you try to fix an error after it has happened instead
of stopping it from happening.

EFI variables are not really useful unless we can change them from the
operating system. Saving them via a driver that is not available at
operating system runtime does not lead anywhere.

So I would recommend not to follow this route anymore.

Best regards

Heinrich


> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  env/env.c                     |  4 +++
>  include/efi_loader.h          |  1 +
>  lib/efi_loader/efi_variable.c | 64 +++++++++++++++++++++++++++++++++--
>  3 files changed, 66 insertions(+), 3 deletions(-)
> 
> diff --git a/env/env.c b/env/env.c
> index afed0f3c95c3..c507a4ac5f78 100644
> --- a/env/env.c
> +++ b/env/env.c
> @@ -5,6 +5,7 @@
>   */
>  
>  #include <common.h>
> +#include <efi_loader.h>
>  #include <environment.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
> @@ -195,6 +196,9 @@ int env_load(void)
>  		if (ret) {
>  			debug("Failed (%d)\n", ret);
>  		} else {
> +#ifdef CONFIG_EFI_LOADER
> +			efi_purge_volatile_variables();
> +#endif
>  			printf("OK\n");
>  			return 0;
>  		}
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 9f7a4068efa6..9cad1dcd62bb 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -533,6 +533,7 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
>  efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_guid_t *vendor,
>  				     u32 attributes, efi_uintn_t data_size,
>  				     void *data);
> +int efi_purge_volatile_variables(void);
>  
>  /*
>   * See section 3.1.3 in the v2.7 UEFI spec for more details on
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index 19d9cb865f25..ad8cd36fa1e1 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -8,6 +8,8 @@
>  #include <malloc.h>
>  #include <charset.h>
>  #include <efi_loader.h>
> +#include <environment.h>
> +#include <search.h>
>  
>  #define READ_ONLY BIT(31)
>  
> @@ -142,6 +144,8 @@ static const char *parse_attr(const char *str, u32 *attrp)
>  
>  		if ((s = prefix(str, "ro"))) {
>  			attr |= 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"))) {
> @@ -293,7 +297,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_guid_t *vendor,
>  		}
>  	}
>  
> -	val = malloc(2 * data_size + strlen("{ro,run,boot}(blob)") + 1);
> +	val = malloc(2 * data_size + strlen("{ro,run,boot,nv}(blob)") + 1);
>  	if (!val) {
>  		ret = EFI_OUT_OF_RESOURCES;
>  		goto out;
> @@ -302,12 +306,16 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_guid_t *vendor,
>  	s = val;
>  
>  	/* store attributes: */
> -	attributes &= (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS);
> +	attributes &= (EFI_VARIABLE_NON_VOLATILE |
> +		       EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +		       EFI_VARIABLE_RUNTIME_ACCESS);
>  	s += sprintf(s, "{");
>  	while (attributes) {
>  		u32 attr = 1 << (ffs(attributes) - 1);
>  
> -		if (attr == EFI_VARIABLE_BOOTSERVICE_ACCESS)
> +		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");
> @@ -334,3 +342,53 @@ out:
>  
>  	return EFI_EXIT(ret);
>  }
> +
> +/*
> + * Purge all the variables which are not marked non volatile.
> + * This function is assumed to be called only once at boot time.
> + */
> +int efi_purge_volatile_variables(void)
> +{
> +	char regex[256];
> +	char * const regexlist[] = {regex};
> +	char *list = NULL, *name, *value;
> +	int len, ret = 0;
> +	u32 attr;
> +
> +	snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
> +
> +	len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
> +			&list, 0, 1, regexlist);
> +
> +	if (len < 0)
> +		return -1;
> +	else if (!len)
> +		return 0;
> +
> +	name = list;
> +	while (*name) {
> +		/* variable name */
> +		value = strchr(name, '=');
> +		if (!value)
> +			break;
> +		*value = '\0';
> +		value++;
> +
> +		parse_attr(value, &attr);
> +		if (!(attr & EFI_VARIABLE_NON_VOLATILE)) {
> +			if (env_set(name, NULL)) {
> +				printf("cannot purge efi variable: %s\n", name);
> +				ret = -1;
> +			}
> +		}
> +
> +		name = strchr(value, '\n');
> +		if (!name)
> +			break;
> +		name++;
> +	}
> +
> +	free(list);
> +
> +	return ret;
> +}
> 

[U-Boot] [PATCH 2/4] cmd: efishell: support -nv option to setvar sub-command

[AKASHI Takahiro]

With -nv specified, a variable to be created will have NON_VOLATILE
attribute.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/efishell.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/cmd/efishell.c b/cmd/efishell.c
index 7cd3ca489559..7cdff757b06c 100644
--- a/cmd/efishell.c
+++ b/cmd/efishell.c
@@ -249,11 +249,22 @@ static int _do_efi_set_var(int argc, char * const argv[])
 	unsigned long size = 0;
 	u16 *var_name16, *p;
 	efi_guid_t guid;
+	u32 attributes;
 	efi_status_t ret;
 
 	if (argc == 1)
 		return CMD_RET_SUCCESS;
 
+	attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
+		     EFI_VARIABLE_RUNTIME_ACCESS;
+	if (!strcmp(argv[1], "-nv")) {
+		attributes |= EFI_VARIABLE_NON_VOLATILE;
+		argc--;
+		argv++;
+		if (argc == 1)
+			return CMD_RET_SUCCESS;
+	}
+
 	var_name = argv[1];
 	if (argc == 2) {
 		/* remove */
@@ -275,9 +286,7 @@ static int _do_efi_set_var(int argc, char * const argv[])
 	utf8_utf16_strncpy(&p, var_name, strlen(var_name) + 1);
 
 	guid = efi_global_variable_guid;
-	ret = efi_set_variable(var_name16, &guid,
-			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
-			       EFI_VARIABLE_RUNTIME_ACCESS, size, value);
+	ret = efi_set_variable(var_name16, &guid, attributes, size, value);
 	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
 out:
 	return ret;
@@ -978,7 +987,7 @@ static char efishell_help_text[] =
 	"\n"
 	"efishell dumpvar [<name>]\n"
 	"  - get uefi variable's value\n"
-	"efishell setvar <name> [<value>]\n"
+	"efishell setvar [-nv] <name> [<value>]\n"
 	"  - set/delete uefi variable's value\n"
 	"    <value> may be \"=\"...\"\", \"=0x...\" (set) or \"=\" (delete)\n"
 	"efishell devices\n"
-- 
2.19.1

[U-Boot] [PATCH 2/4] cmd: efishell: support -nv option to setvar sub-command

[Heinrich Schuchardt]

On 11/28/18 7:00 AM, AKASHI Takahiro wrote:
> With -nv specified, a variable to be created will have NON_VOLATILE
> attribute.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

Why would we only make the NON_VOLATILE attribute available and not all
the other attributes like EFI_VARIABLE_BOOTSERVICE_ACCESS?

Where is the patch that ensures that non-volatile variables are not stored?

Currently we do not make EFI variables available to the operating system
so they are not really of much use.

Best regards

Heinrich

> ---
>  cmd/efishell.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/cmd/efishell.c b/cmd/efishell.c
> index 7cd3ca489559..7cdff757b06c 100644
> --- a/cmd/efishell.c
> +++ b/cmd/efishell.c
> @@ -249,11 +249,22 @@ static int _do_efi_set_var(int argc, char * const argv[])
>  	unsigned long size = 0;
>  	u16 *var_name16, *p;
>  	efi_guid_t guid;
> +	u32 attributes;
>  	efi_status_t ret;
>  
>  	if (argc == 1)
>  		return CMD_RET_SUCCESS;
>  
> +	attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +		     EFI_VARIABLE_RUNTIME_ACCESS;
> +	if (!strcmp(argv[1], "-nv")) {
> +		attributes |= EFI_VARIABLE_NON_VOLATILE;
> +		argc--;
> +		argv++;
> +		if (argc == 1)
> +			return CMD_RET_SUCCESS;
> +	}
> +
>  	var_name = argv[1];
>  	if (argc == 2) {
>  		/* remove */
> @@ -275,9 +286,7 @@ static int _do_efi_set_var(int argc, char * const argv[])
>  	utf8_utf16_strncpy(&p, var_name, strlen(var_name) + 1);
>  
>  	guid = efi_global_variable_guid;
> -	ret = efi_set_variable(var_name16, &guid,
> -			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
> -			       EFI_VARIABLE_RUNTIME_ACCESS, size, value);
> +	ret = efi_set_variable(var_name16, &guid, attributes, size, value);
>  	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
>  out:
>  	return ret;
> @@ -978,7 +987,7 @@ static char efishell_help_text[] =
>  	"\n"
>  	"efishell dumpvar [<name>]\n"
>  	"  - get uefi variable's value\n"
> -	"efishell setvar <name> [<value>]\n"
> +	"efishell setvar [-nv] <name> [<value>]\n"
>  	"  - set/delete uefi variable's value\n"
>  	"    <value> may be \"=\"...\"\", \"=0x...\" (set) or \"=\" (delete)\n"
>  	"efishell devices\n"
> 

[U-Boot] [PATCH 3/4] cmd: efishell: make Boot####/BootOrder variable non-volatile

[AKASHI Takahiro]

See UEFI specification v2.7a, section 3.3 for details attributes.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/efishell.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/cmd/efishell.c b/cmd/efishell.c
index 7cdff757b06c..a594278a04fe 100644
--- a/cmd/efishell.c
+++ b/cmd/efishell.c
@@ -641,6 +641,7 @@ static int do_efi_boot_add(int argc, char * const argv[])
 	}
 
 	ret = efi_set_variable(var_name16, &guid,
+			       EFI_VARIABLE_NON_VOLATILE |
 			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
 			       EFI_VARIABLE_RUNTIME_ACCESS, size, data);
 	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
@@ -676,6 +677,7 @@ static int do_efi_boot_rm(int argc, char * const argv[])
 		utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);
 
 		ret = efi_set_variable(var_name16, &guid,
+				       EFI_VARIABLE_NON_VOLATILE |
 				       EFI_VARIABLE_BOOTSERVICE_ACCESS |
 				       EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL);
 		if (ret) {
@@ -898,6 +900,7 @@ static int do_efi_boot_order(int argc, char * const argv[])
 
 	guid = efi_global_variable_guid;
 	ret = efi_set_variable(L"BootOrder", &guid,
+			       EFI_VARIABLE_NON_VOLATILE |
 			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
 			       EFI_VARIABLE_RUNTIME_ACCESS, size, bootorder);
 	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
-- 
2.19.1

[U-Boot] [PATCH 3/4] cmd: efishell: make Boot####/BootOrder variable non-volatile

[Heinrich Schuchardt]

On 11/28/18 7:00 AM, AKASHI Takahiro wrote:
> See UEFI specification v2.7a, section 3.3 for details attributes.

I cannot find file cmd/efishell.c in U-Boot. And I cannot find it in
https://patchwork.ozlabs.org/project/uboot/list/?submitter=61166

So I have no clue what this patch is based on.

Best regards

Heinrich

> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  cmd/efishell.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/cmd/efishell.c b/cmd/efishell.c
> index 7cdff757b06c..a594278a04fe 100644
> --- a/cmd/efishell.c
> +++ b/cmd/efishell.c
> @@ -641,6 +641,7 @@ static int do_efi_boot_add(int argc, char * const argv[])
>  	}
>  
>  	ret = efi_set_variable(var_name16, &guid,
> +			       EFI_VARIABLE_NON_VOLATILE |
>  			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
>  			       EFI_VARIABLE_RUNTIME_ACCESS, size, data);
>  	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> @@ -676,6 +677,7 @@ static int do_efi_boot_rm(int argc, char * const argv[])
>  		utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);
>  
>  		ret = efi_set_variable(var_name16, &guid,
> +				       EFI_VARIABLE_NON_VOLATILE |
>  				       EFI_VARIABLE_BOOTSERVICE_ACCESS |
>  				       EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL);
>  		if (ret) {
> @@ -898,6 +900,7 @@ static int do_efi_boot_order(int argc, char * const argv[])
>  
>  	guid = efi_global_variable_guid;
>  	ret = efi_set_variable(L"BootOrder", &guid,
> +			       EFI_VARIABLE_NON_VOLATILE |
>  			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
>  			       EFI_VARIABLE_RUNTIME_ACCESS, size, bootorder);
>  	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> 

[U-Boot] [PATCH 4/4] efi_loader: bootmgr: make BootNext non-volatile

[AKASHI Takahiro]

See UEFI specification v2.7a, section 3.3 for details attributes.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 lib/efi_loader/efi_bootmgr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index db391147fb2d..128d1e887cb4 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -210,7 +210,8 @@ void *efi_bootmgr_load(int boot_id,
 	if (!bootnext)
 		goto run_list;
 
-	attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
+	attributes = EFI_VARIABLE_NON_VOLATILE |
+		     EFI_VARIABLE_BOOTSERVICE_ACCESS |
 		     EFI_VARIABLE_RUNTIME_ACCESS;
 	size = 0;
 	ret = rs->set_variable(L"BootNext",
-- 
2.19.1

[U-Boot] [PATCH 4/4] efi_loader: bootmgr: make BootNext non-volatile

[Heinrich Schuchardt]

On 11/28/18 7:00 AM, AKASHI Takahiro wrote:
> See UEFI specification v2.7a, section 3.3 for details attributes.

This patch is not applicable to U-Boot master not to efi-next. There is
no preceding patch in
https://patchwork.ozlabs.org/project/uboot/list/?submitter=61166

I have no clue what this patch is based on.

Best regards

Heinrich

> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  lib/efi_loader/efi_bootmgr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index db391147fb2d..128d1e887cb4 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -210,7 +210,8 @@ void *efi_bootmgr_load(int boot_id,
>  	if (!bootnext)
>  		goto run_list;
>  
> -	attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +	attributes = EFI_VARIABLE_NON_VOLATILE |
> +		     EFI_VARIABLE_BOOTSERVICE_ACCESS |
>  		     EFI_VARIABLE_RUNTIME_ACCESS;
>  	size = 0;
>  	ret = rs->set_variable(L"BootNext",
> 

[U-Boot] [PATCH 0/4] efi_loader: non-volatile variables support

[Sumit Garg]

Hi Akashi,

On Wed, 28 Nov 2018 at 11:28, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> As the subject suggested, this patch set allows any efi variable to be
> volatile or non-volatile as UEFI specification describes.
>
> With my efishell patch[1] with patch #2, you can try as follows:
>   => efi setvar PlatformLang en
>   => efi setvar -nv BootNext =H0200
>   => env save
>

How do you expect usage of "EFI_VARIABLE_NON_VOLATILE" attribute to
work during boot-time services as we don't have "env save" command
option as a boot-time service?

Take example scenario with EDK2 shell launched from u-boot and tries
to set non-volatile efi variable.

IMHO, support should be added to "efi_set_variable" to save variable
with "EFI_VARIABLE_NON_VOLATILE" attribute to non volatile storage.

-Sumit

> BootNext will be preserved across reboot, while PlatformLang not.
>
> Please note that, currently, setvar command does not automatically
> append NON_VOLATILE attribute, while UEFI specification expects that
> PlatformLang be non-volatile, you'd better also specify -nv for
> this variable here.
>
> Patch #2/#3 depend on my efishell patch[1].
> Patch #4 depends on my BootNext patch[2].
>
> Patch[1] and [2] have not been merged yet, so patch#1 can be applied
> on its own.
>
> [1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
> [2] https://lists.denx.de/pipermail/u-boot/2018-November/349281.html
>
> AKASHI Takahiro (4):
>   efi_loader: support non-volatile variable behavior
>   cmd: efishell: support -nv option to setvar sub-command
>   cmd: efishell: make Boot####/BootOrder variable non-volatile
>   efi_loader: bootmgr: make BootNext non-volatile
>
>  cmd/efishell.c                | 20 ++++++++---
>  env/env.c                     |  4 +++
>  include/efi_loader.h          |  1 +
>  lib/efi_loader/efi_bootmgr.c  |  3 +-
>  lib/efi_loader/efi_variable.c | 64 +++++++++++++++++++++++++++++++++--
>  5 files changed, 84 insertions(+), 8 deletions(-)
>
> --
> 2.19.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 0/4] efi_loader: non-volatile variables support

[Takahiro Akashi]

# My patch is more or less a RFC to raise attention.

On Wed, Dec 05, 2018 at 11:53:42AM +0530, Sumit Garg wrote:
> Hi Akashi,
> 
> On Wed, 28 Nov 2018 at 11:28, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > As the subject suggested, this patch set allows any efi variable to be
> > volatile or non-volatile as UEFI specification describes.
> >
> > With my efishell patch[1] with patch #2, you can try as follows:
> >   => efi setvar PlatformLang en
> >   => efi setvar -nv BootNext =H0200
> >   => env save
> >
> 
> How do you expect usage of "EFI_VARIABLE_NON_VOLATILE" attribute to
> work during boot-time services as we don't have "env save" command
> option as a boot-time service?
> 
> Take example scenario with EDK2 shell launched from u-boot and tries
> to set non-volatile efi variable.

I know the issue, but

> IMHO, support should be added to "efi_set_variable" to save variable
> with "EFI_VARIABLE_NON_VOLATILE" attribute to non volatile storage.

I also hesitate to implement such a behavior to efi_set_variable
as it ends up writing to flash every time. Please note that data on
storage is a sorted list in plain text and so updating it can be painful.

-Takahiro Akashi


> -Sumit
> 
> > BootNext will be preserved across reboot, while PlatformLang not.
> >
> > Please note that, currently, setvar command does not automatically
> > append NON_VOLATILE attribute, while UEFI specification expects that
> > PlatformLang be non-volatile, you'd better also specify -nv for
> > this variable here.
> >
> > Patch #2/#3 depend on my efishell patch[1].
> > Patch #4 depends on my BootNext patch[2].
> >
> > Patch[1] and [2] have not been merged yet, so patch#1 can be applied
> > on its own.
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
> > [2] https://lists.denx.de/pipermail/u-boot/2018-November/349281.html
> >
> > AKASHI Takahiro (4):
> >   efi_loader: support non-volatile variable behavior
> >   cmd: efishell: support -nv option to setvar sub-command
> >   cmd: efishell: make Boot####/BootOrder variable non-volatile
> >   efi_loader: bootmgr: make BootNext non-volatile
> >
> >  cmd/efishell.c                | 20 ++++++++---
> >  env/env.c                     |  4 +++
> >  include/efi_loader.h          |  1 +
> >  lib/efi_loader/efi_bootmgr.c  |  3 +-
> >  lib/efi_loader/efi_variable.c | 64 +++++++++++++++++++++++++++++++++--
> >  5 files changed, 84 insertions(+), 8 deletions(-)
> >
> > --
> > 2.19.1
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 0/4] efi_loader: non-volatile variables support

[Sumit Garg]

On Fri, 7 Dec 2018 at 07:56, Takahiro Akashi <takahiro.akashi@linaro.org> wrote:
>
> # My patch is more or less a RFC to raise attention.
>
> On Wed, Dec 05, 2018 at 11:53:42AM +0530, Sumit Garg wrote:
> > Hi Akashi,
> >
> > On Wed, 28 Nov 2018 at 11:28, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> > >
> > > As the subject suggested, this patch set allows any efi variable to be
> > > volatile or non-volatile as UEFI specification describes.
> > >
> > > With my efishell patch[1] with patch #2, you can try as follows:
> > >   => efi setvar PlatformLang en
> > >   => efi setvar -nv BootNext =H0200
> > >   => env save
> > >
> >
> > How do you expect usage of "EFI_VARIABLE_NON_VOLATILE" attribute to
> > work during boot-time services as we don't have "env save" command
> > option as a boot-time service?
> >
> > Take example scenario with EDK2 shell launched from u-boot and tries
> > to set non-volatile efi variable.
>
> I know the issue, but
>
> > IMHO, support should be added to "efi_set_variable" to save variable
> > with "EFI_VARIABLE_NON_VOLATILE" attribute to non volatile storage.
>
> I also hesitate to implement such a behavior to efi_set_variable
> as it ends up writing to flash every time. Please note that data on
> storage is a sorted list in plain text and so updating it can be painful.
>

AFAIK, flash should be updated every time in case efi_set_variable is
called with non-volatile attribute. Do you foresee any issues with
call to "env_save" during this call? I think it would be similar to
what you are doing currently from prompt with your
purge_volatile_variables() api in place.

-Sumit

> -Takahiro Akashi
>
>
> > -Sumit
> >
> > > BootNext will be preserved across reboot, while PlatformLang not.
> > >
> > > Please note that, currently, setvar command does not automatically
> > > append NON_VOLATILE attribute, while UEFI specification expects that
> > > PlatformLang be non-volatile, you'd better also specify -nv for
> > > this variable here.
> > >
> > > Patch #2/#3 depend on my efishell patch[1].
> > > Patch #4 depends on my BootNext patch[2].
> > >
> > > Patch[1] and [2] have not been merged yet, so patch#1 can be applied
> > > on its own.
> > >
> > > [1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
> > > [2] https://lists.denx.de/pipermail/u-boot/2018-November/349281.html
> > >
> > > AKASHI Takahiro (4):
> > >   efi_loader: support non-volatile variable behavior
> > >   cmd: efishell: support -nv option to setvar sub-command
> > >   cmd: efishell: make Boot####/BootOrder variable non-volatile
> > >   efi_loader: bootmgr: make BootNext non-volatile
> > >
> > >  cmd/efishell.c                | 20 ++++++++---
> > >  env/env.c                     |  4 +++
> > >  include/efi_loader.h          |  1 +
> > >  lib/efi_loader/efi_bootmgr.c  |  3 +-
> > >  lib/efi_loader/efi_variable.c | 64 +++++++++++++++++++++++++++++++++--
> > >  5 files changed, 84 insertions(+), 8 deletions(-)
> > >
> > > --
> > > 2.19.1
> > >
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot at lists.denx.de
> > > https://lists.denx.de/listinfo/u-boot