[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 at 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;
> +}
>