[U-Boot,v1,14/15] efi_loader: efi variable support

Submitted by Rob Clark on Aug. 10, 2017, 6:29 p.m.

Details

Message ID 20170810182948.27653-15-robdclark@gmail.com
State New
Delegated to: Alexander Graf
Headers show

Commit Message

Rob Clark Aug. 10, 2017, 6:29 p.m.
Add EFI variable support, mapping to u-boot environment variables.
Variables are pretty important for setting up boot order, among other
things.  If the board supports saveenv, then it will be called in
ExitBootServices() to persist variables set by the efi payload.  (For
example, fallback.efi configuring BootOrder and BootXXXX load-option
variables.)

Variables are *not* currently exposed at runtime, post ExitBootServices.
On boards without a dedicated device for storage, which the loaded OS
is not trying to also use, this is rather tricky.  One idea, at least
for boards that can persist RAM across reboot, is to keep a "journal"
of modified variables in RAM, and then turn halt into a reboot into
u-boot, plus store variables, plus halt.  Whatever the solution, it
likely involves some per-board support.

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

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 cmd/bootefi.c                 |   4 +
 include/efi.h                 |  19 +++
 include/efi_loader.h          |  10 ++
 lib/efi_loader/Makefile       |   2 +-
 lib/efi_loader/efi_boottime.c |   5 +
 lib/efi_loader/efi_runtime.c  |  17 ++-
 lib/efi_loader/efi_variable.c | 342 ++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 394 insertions(+), 5 deletions(-)
 create mode 100644 lib/efi_loader/efi_variable.c

Comments

Alexander Graf Aug. 12, 2017, 1:01 p.m.
On 10.08.17 20:29, Rob Clark wrote:
> Add EFI variable support, mapping to u-boot environment variables.
> Variables are pretty important for setting up boot order, among other
> things.  If the board supports saveenv, then it will be called in
> ExitBootServices() to persist variables set by the efi payload.  (For
> example, fallback.efi configuring BootOrder and BootXXXX load-option
> variables.)
> 
> Variables are *not* currently exposed at runtime, post ExitBootServices.
> On boards without a dedicated device for storage, which the loaded OS
> is not trying to also use, this is rather tricky.  One idea, at least
> for boards that can persist RAM across reboot, is to keep a "journal"
> of modified variables in RAM, and then turn halt into a reboot into
> u-boot, plus store variables, plus halt.  Whatever the solution, it
> likely involves some per-board support.
> 
> 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
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>   cmd/bootefi.c                 |   4 +
>   include/efi.h                 |  19 +++
>   include/efi_loader.h          |  10 ++
>   lib/efi_loader/Makefile       |   2 +-
>   lib/efi_loader/efi_boottime.c |   5 +
>   lib/efi_loader/efi_runtime.c  |  17 ++-
>   lib/efi_loader/efi_variable.c | 342 ++++++++++++++++++++++++++++++++++++++++++
>   7 files changed, 394 insertions(+), 5 deletions(-)
>   create mode 100644 lib/efi_loader/efi_variable.c
> 
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index b9e1e5e131..80f52e9e35 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -181,6 +181,10 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt,
>   		goto exit;
>   	}
>   
> +	/* we don't support much: */
> +	setenv("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported",
> +			"{ro,boot}(blob)0000000000000000");
> +
>   	/* Call our payload! */
>   	debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry);
>   
> diff --git a/include/efi.h b/include/efi.h
> index ddd2b96417..dbd482a5db 100644
> --- a/include/efi.h
> +++ b/include/efi.h
> @@ -324,6 +324,25 @@ extern char image_base[];
>   /* Start and end of U-Boot image (for payload) */
>   extern char _binary_u_boot_bin_start[], _binary_u_boot_bin_end[];
>   
> +/*
> + * Variable Attributes
> + */
> +#define EFI_VARIABLE_NON_VOLATILE       0x0000000000000001

Shouldn't we honor this one too? A UEFI application may set runtime 
variables that should not get persisted across boots (think the UEFI 
shell setting some internal state thing, then booting Linux).

> +#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0000000000000002
> +#define EFI_VARIABLE_RUNTIME_ACCESS     0x0000000000000004
> +#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x0000000000000008
> +#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x0000000000000010
> +#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x0000000000000020
> +#define EFI_VARIABLE_APPEND_WRITE	0x0000000000000040
> +
> +#define EFI_VARIABLE_MASK 	(EFI_VARIABLE_NON_VOLATILE | \
> +				EFI_VARIABLE_BOOTSERVICE_ACCESS | \
> +				EFI_VARIABLE_RUNTIME_ACCESS | \
> +				EFI_VARIABLE_HARDWARE_ERROR_RECORD | \
> +				EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \
> +				EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS | \
> +				EFI_VARIABLE_APPEND_WRITE)
> +
>   /**
>    * efi_get_sys_table() - Get access to the main EFI system table
>    *
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index efb93f31f7..9cb568e615 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -271,6 +271,16 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
>   			struct efi_time_cap *capabilities);
>   void efi_get_time_init(void);
>   
> +efi_status_t EFIAPI efi_get_variable(s16 *variable_name,
> +		efi_guid_t *vendor, u32 *attributes,
> +		unsigned long *data_size, void *data);
> +efi_status_t EFIAPI efi_get_next_variable(
> +		unsigned long *variable_name_size,
> +		s16 *variable_name, efi_guid_t *vendor);
> +efi_status_t EFIAPI efi_set_variable(s16 *variable_name,
> +		efi_guid_t *vendor, u32 attributes,
> +		unsigned long data_size, void *data);
> +
>   #else /* defined(EFI_LOADER) && !defined(CONFIG_SPL_BUILD) */
>   
>   /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index cce92cfeb5..f58cb13337 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -16,7 +16,7 @@ always := $(efiprogs-y)
>   obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>   obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>   obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
> -obj-y += efi_file.o
> +obj-y += efi_file.o efi_variable.o
>   obj-$(CONFIG_LCD) += efi_gop.o
>   obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>   obj-$(CONFIG_PARTITIONS) += efi_disk.o
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 7e44ba56e2..0bb64f0351 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -927,6 +927,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,
>   {
>   	EFI_ENTRY("%p, %ld", image_handle, map_key);
>   
> +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
> +	/* save any EFI variables that have been written: */
> +	saveenv();
> +#endif
> +
>   	board_quiesce_devices();
>   
>   	/* Fix up caches for EFI payloads if necessary */
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index dd52755d1d..7615090ba3 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -184,7 +184,16 @@ static const struct efi_runtime_detach_list_struct efi_runtime_detach_list[] = {
>   		/* Clean up system table */
>   		.ptr = &systab.boottime,
>   		.patchto = NULL,
> -	},
> +	}, {
> +		.ptr = &efi_runtime_services.get_variable,
> +		.patchto = &efi_device_error,
> +	}, {
> +		.ptr = &efi_runtime_services.get_next_variable,
> +		.patchto = &efi_device_error,
> +	}, {
> +		.ptr = &efi_runtime_services.set_variable,
> +		.patchto = &efi_device_error,
> +	}
>   };
>   
>   static bool efi_runtime_tobedetached(void *p)
> @@ -382,9 +391,9 @@ struct efi_runtime_services __efi_runtime_data efi_runtime_services = {
>   	.set_wakeup_time = (void *)&efi_unimplemented,
>   	.set_virtual_address_map = &efi_set_virtual_address_map,
>   	.convert_pointer = (void *)&efi_invalid_parameter,
> -	.get_variable = (void *)&efi_device_error,
> -	.get_next_variable = (void *)&efi_device_error,
> -	.set_variable = (void *)&efi_device_error,
> +	.get_variable = efi_get_variable,
> +	.get_next_variable = efi_get_next_variable,
> +	.set_variable = efi_set_variable,
>   	.get_next_high_mono_count = (void *)&efi_device_error,
>   	.reset_system = &efi_reset_system_boottime,
>   };
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> new file mode 100644
> index 0000000000..745514e4fa
> --- /dev/null
> +++ b/lib/efi_loader/efi_variable.c
> @@ -0,0 +1,342 @@
> +/*
> + *  EFI utils
> + *
> + *  Copyright (c) 2017 Rob Clark
> + *
> + *  SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#include <malloc.h>
> +#include <charset.h>
> +#include <efi_loader.h>
> +
> +#define READ_ONLY BIT(31)
> +
> +/*
> + * 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 MAX_VAR_NAME 31
> +#define MAX_NATIVE_VAR_NAME \
> +	(strlen("efi_xxxxxxxx-xxxx-xxxx-xxxxxxxxxxxxxxxx_") + \
> +		(MAX_VAR_NAME * MAX_UTF8_PER_UTF16))
> +
> +static int hex(unsigned char ch)
> +{
> +	if (ch >= 'a' && ch <= 'f')
> +		return ch-'a'+10;
> +	if (ch >= '0' && ch <= '9')
> +		return ch-'0';
> +	if (ch >= 'A' && ch <= 'F')
> +		return ch-'A'+10;
> +	return -1;
> +}
> +
> +static const char * hex2mem(u8 *mem, const char *hexstr, int count)
> +{
> +	memset(mem, 0, count/2);
> +
> +	do {
> +		int nibble;
> +
> +		*mem = 0;
> +
> +		if (!count || !*hexstr)
> +			break;
> +
> +		nibble = hex(*hexstr);
> +		if (nibble < 0)
> +			break;
> +
> +		*mem = nibble;
> +		count--;
> +		hexstr++;
> +
> +		if (!count || !*hexstr)
> +			break;
> +
> +		nibble = hex(*hexstr);
> +		if (nibble < 0)
> +			break;
> +
> +		*mem = (*mem << 4) | nibble;
> +		count--;
> +		hexstr++;
> +		mem++;
> +
> +	} while (1);
> +
> +	if (*hexstr)
> +		return hexstr;
> +
> +	return NULL;
> +}
> +
> +static char * mem2hex(char *hexstr, const u8 *mem, int count)
> +{
> +	static const char hexchars[] = "0123456789abcdef";
> +
> +	while (count-- > 0) {
> +		u8 ch = *mem++;
> +		*hexstr++ = hexchars[ch >> 4];
> +		*hexstr++ = hexchars[ch & 0xf];
> +	}
> +
> +	return hexstr;
> +}
> +
> +static efi_status_t efi_to_native(char *native, s16 *variable_name,
> +		efi_guid_t *vendor)
> +{
> +	size_t len;
> +
> +	len = utf16_strlen((u16 *)variable_name);
> +	if (len >= MAX_VAR_NAME)
> +		return EFI_DEVICE_ERROR;
> +
> +	native += sprintf(native, "efi_%pUl_", vendor);
> +	native  = (char *)utf16_to_utf8((u8 *)native, (u16 *)variable_name, len);
> +	*native = '\0';
> +
> +	return EFI_SUCCESS;
> +}
> +
> +static const char * prefix(const char *str, const char *prefix)

Isn't this just strncmp(prefix, str, strlen(prefix));?


Alex
Rob Clark Aug. 12, 2017, 2:39 p.m.
On Sat, Aug 12, 2017 at 9:01 AM, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 10.08.17 20:29, Rob Clark wrote:
>>
>> Add EFI variable support, mapping to u-boot environment variables.
>> Variables are pretty important for setting up boot order, among other
>> things.  If the board supports saveenv, then it will be called in
>> ExitBootServices() to persist variables set by the efi payload.  (For
>> example, fallback.efi configuring BootOrder and BootXXXX load-option
>> variables.)
>>
>> Variables are *not* currently exposed at runtime, post ExitBootServices.
>> On boards without a dedicated device for storage, which the loaded OS
>> is not trying to also use, this is rather tricky.  One idea, at least
>> for boards that can persist RAM across reboot, is to keep a "journal"
>> of modified variables in RAM, and then turn halt into a reboot into
>> u-boot, plus store variables, plus halt.  Whatever the solution, it
>> likely involves some per-board support.
>>
>> 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
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>   cmd/bootefi.c                 |   4 +
>>   include/efi.h                 |  19 +++
>>   include/efi_loader.h          |  10 ++
>>   lib/efi_loader/Makefile       |   2 +-
>>   lib/efi_loader/efi_boottime.c |   5 +
>>   lib/efi_loader/efi_runtime.c  |  17 ++-
>>   lib/efi_loader/efi_variable.c | 342
>> ++++++++++++++++++++++++++++++++++++++++++
>>   7 files changed, 394 insertions(+), 5 deletions(-)
>>   create mode 100644 lib/efi_loader/efi_variable.c
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index b9e1e5e131..80f52e9e35 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -181,6 +181,10 @@ static unsigned long do_bootefi_exec(void *efi, void
>> *fdt,
>>                 goto exit;
>>         }
>>   +     /* we don't support much: */
>> +
>> setenv("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported",
>> +                       "{ro,boot}(blob)0000000000000000");
>> +
>>         /* Call our payload! */
>>         debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__,
>> (long)entry);
>>   diff --git a/include/efi.h b/include/efi.h
>> index ddd2b96417..dbd482a5db 100644
>> --- a/include/efi.h
>> +++ b/include/efi.h
>> @@ -324,6 +324,25 @@ extern char image_base[];
>>   /* Start and end of U-Boot image (for payload) */
>>   extern char _binary_u_boot_bin_start[], _binary_u_boot_bin_end[];
>>   +/*
>> + * Variable Attributes
>> + */
>> +#define EFI_VARIABLE_NON_VOLATILE       0x0000000000000001
>
>
> Shouldn't we honor this one too? A UEFI application may set runtime
> variables that should not get persisted across boots (think the UEFI shell
> setting some internal state thing, then booting Linux).

So the thing is, we honor non-volatile (at least to the extent the
board can do saveenv()).  What we don't do is make volatile vars
disappear on reboot... which isn't terribly easy to do since we don't
have any way to mark u-boot env vars as volatile.

But my reading of the spec doesn't preclude volatile variables from
being persisted.  It only says that non-volatile variables should be
persisted.

>
>> +#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0000000000000002
>> +#define EFI_VARIABLE_RUNTIME_ACCESS     0x0000000000000004
>> +#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x0000000000000008
>> +#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x0000000000000010
>> +#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
>> 0x0000000000000020
>> +#define EFI_VARIABLE_APPEND_WRITE      0x0000000000000040
>> +
>> +#define EFI_VARIABLE_MASK      (EFI_VARIABLE_NON_VOLATILE | \
>> +                               EFI_VARIABLE_BOOTSERVICE_ACCESS | \
>> +                               EFI_VARIABLE_RUNTIME_ACCESS | \
>> +                               EFI_VARIABLE_HARDWARE_ERROR_RECORD | \
>> +                               EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS |
>> \
>> +
>> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS | \
>> +                               EFI_VARIABLE_APPEND_WRITE)
>> +
>>   /**
>>    * efi_get_sys_table() - Get access to the main EFI system table
>>    *
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index efb93f31f7..9cb568e615 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -271,6 +271,16 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
>>                         struct efi_time_cap *capabilities);
>>   void efi_get_time_init(void);
>>   +efi_status_t EFIAPI efi_get_variable(s16 *variable_name,
>> +               efi_guid_t *vendor, u32 *attributes,
>> +               unsigned long *data_size, void *data);
>> +efi_status_t EFIAPI efi_get_next_variable(
>> +               unsigned long *variable_name_size,
>> +               s16 *variable_name, efi_guid_t *vendor);
>> +efi_status_t EFIAPI efi_set_variable(s16 *variable_name,
>> +               efi_guid_t *vendor, u32 attributes,
>> +               unsigned long data_size, void *data);
>> +
>>   #else /* defined(EFI_LOADER) && !defined(CONFIG_SPL_BUILD) */
>>     /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it
>> out */
>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>> index cce92cfeb5..f58cb13337 100644
>> --- a/lib/efi_loader/Makefile
>> +++ b/lib/efi_loader/Makefile
>> @@ -16,7 +16,7 @@ always := $(efiprogs-y)
>>   obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>>   obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>>   obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
>> -obj-y += efi_file.o
>> +obj-y += efi_file.o efi_variable.o
>>   obj-$(CONFIG_LCD) += efi_gop.o
>>   obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>>   obj-$(CONFIG_PARTITIONS) += efi_disk.o
>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> index 7e44ba56e2..0bb64f0351 100644
>> --- a/lib/efi_loader/efi_boottime.c
>> +++ b/lib/efi_loader/efi_boottime.c
>> @@ -927,6 +927,11 @@ static efi_status_t EFIAPI
>> efi_exit_boot_services(void *image_handle,
>>   {
>>         EFI_ENTRY("%p, %ld", image_handle, map_key);
>>   +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
>> +       /* save any EFI variables that have been written: */
>> +       saveenv();
>> +#endif
>> +
>>         board_quiesce_devices();
>>         /* Fix up caches for EFI payloads if necessary */
>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>> index dd52755d1d..7615090ba3 100644
>> --- a/lib/efi_loader/efi_runtime.c
>> +++ b/lib/efi_loader/efi_runtime.c
>> @@ -184,7 +184,16 @@ static const struct efi_runtime_detach_list_struct
>> efi_runtime_detach_list[] = {
>>                 /* Clean up system table */
>>                 .ptr = &systab.boottime,
>>                 .patchto = NULL,
>> -       },
>> +       }, {
>> +               .ptr = &efi_runtime_services.get_variable,
>> +               .patchto = &efi_device_error,
>> +       }, {
>> +               .ptr = &efi_runtime_services.get_next_variable,
>> +               .patchto = &efi_device_error,
>> +       }, {
>> +               .ptr = &efi_runtime_services.set_variable,
>> +               .patchto = &efi_device_error,
>> +       }
>>   };
>>     static bool efi_runtime_tobedetached(void *p)
>> @@ -382,9 +391,9 @@ struct efi_runtime_services __efi_runtime_data
>> efi_runtime_services = {
>>         .set_wakeup_time = (void *)&efi_unimplemented,
>>         .set_virtual_address_map = &efi_set_virtual_address_map,
>>         .convert_pointer = (void *)&efi_invalid_parameter,
>> -       .get_variable = (void *)&efi_device_error,
>> -       .get_next_variable = (void *)&efi_device_error,
>> -       .set_variable = (void *)&efi_device_error,
>> +       .get_variable = efi_get_variable,
>> +       .get_next_variable = efi_get_next_variable,
>> +       .set_variable = efi_set_variable,
>>         .get_next_high_mono_count = (void *)&efi_device_error,
>>         .reset_system = &efi_reset_system_boottime,
>>   };
>> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
>> new file mode 100644
>> index 0000000000..745514e4fa
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_variable.c
>> @@ -0,0 +1,342 @@
>> +/*
>> + *  EFI utils
>> + *
>> + *  Copyright (c) 2017 Rob Clark
>> + *
>> + *  SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#include <malloc.h>
>> +#include <charset.h>
>> +#include <efi_loader.h>
>> +
>> +#define READ_ONLY BIT(31)
>> +
>> +/*
>> + * 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 MAX_VAR_NAME 31
>> +#define MAX_NATIVE_VAR_NAME \
>> +       (strlen("efi_xxxxxxxx-xxxx-xxxx-xxxxxxxxxxxxxxxx_") + \
>> +               (MAX_VAR_NAME * MAX_UTF8_PER_UTF16))
>> +
>> +static int hex(unsigned char ch)
>> +{
>> +       if (ch >= 'a' && ch <= 'f')
>> +               return ch-'a'+10;
>> +       if (ch >= '0' && ch <= '9')
>> +               return ch-'0';
>> +       if (ch >= 'A' && ch <= 'F')
>> +               return ch-'A'+10;
>> +       return -1;
>> +}
>> +
>> +static const char * hex2mem(u8 *mem, const char *hexstr, int count)
>> +{
>> +       memset(mem, 0, count/2);
>> +
>> +       do {
>> +               int nibble;
>> +
>> +               *mem = 0;
>> +
>> +               if (!count || !*hexstr)
>> +                       break;
>> +
>> +               nibble = hex(*hexstr);
>> +               if (nibble < 0)
>> +                       break;
>> +
>> +               *mem = nibble;
>> +               count--;
>> +               hexstr++;
>> +
>> +               if (!count || !*hexstr)
>> +                       break;
>> +
>> +               nibble = hex(*hexstr);
>> +               if (nibble < 0)
>> +                       break;
>> +
>> +               *mem = (*mem << 4) | nibble;
>> +               count--;
>> +               hexstr++;
>> +               mem++;
>> +
>> +       } while (1);
>> +
>> +       if (*hexstr)
>> +               return hexstr;
>> +
>> +       return NULL;
>> +}
>> +
>> +static char * mem2hex(char *hexstr, const u8 *mem, int count)
>> +{
>> +       static const char hexchars[] = "0123456789abcdef";
>> +
>> +       while (count-- > 0) {
>> +               u8 ch = *mem++;
>> +               *hexstr++ = hexchars[ch >> 4];
>> +               *hexstr++ = hexchars[ch & 0xf];
>> +       }
>> +
>> +       return hexstr;
>> +}
>> +
>> +static efi_status_t efi_to_native(char *native, s16 *variable_name,
>> +               efi_guid_t *vendor)
>> +{
>> +       size_t len;
>> +
>> +       len = utf16_strlen((u16 *)variable_name);
>> +       if (len >= MAX_VAR_NAME)
>> +               return EFI_DEVICE_ERROR;
>> +
>> +       native += sprintf(native, "efi_%pUl_", vendor);
>> +       native  = (char *)utf16_to_utf8((u8 *)native, (u16
>> *)variable_name, len);
>> +       *native = '\0';
>> +
>> +       return EFI_SUCCESS;
>> +}
>> +
>> +static const char * prefix(const char *str, const char *prefix)
>
>
> Isn't this just strncmp(prefix, str, strlen(prefix));?
>

I suppose it could be implemented that way, at the expense of an extra
strlen(prefix).

BR,
-R
Alexander Graf Aug. 12, 2017, 5:28 p.m.
> Am 12.08.2017 um 16:39 schrieb Rob Clark <robdclark@gmail.com>:
> 
>> On Sat, Aug 12, 2017 at 9:01 AM, Alexander Graf <agraf@suse.de> wrote:
>> 
>> 
>>> On 10.08.17 20:29, Rob Clark wrote:
>>> 
>>> Add EFI variable support, mapping to u-boot environment variables.
>>> Variables are pretty important for setting up boot order, among other
>>> things.  If the board supports saveenv, then it will be called in
>>> ExitBootServices() to persist variables set by the efi payload.  (For
>>> example, fallback.efi configuring BootOrder and BootXXXX load-option
>>> variables.)
>>> 
>>> Variables are *not* currently exposed at runtime, post ExitBootServices.
>>> On boards without a dedicated device for storage, which the loaded OS
>>> is not trying to also use, this is rather tricky.  One idea, at least
>>> for boards that can persist RAM across reboot, is to keep a "journal"
>>> of modified variables in RAM, and then turn halt into a reboot into
>>> u-boot, plus store variables, plus halt.  Whatever the solution, it
>>> likely involves some per-board support.
>>> 
>>> 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
>>> 
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> ---
>>>  cmd/bootefi.c                 |   4 +
>>>  include/efi.h                 |  19 +++
>>>  include/efi_loader.h          |  10 ++
>>>  lib/efi_loader/Makefile       |   2 +-
>>>  lib/efi_loader/efi_boottime.c |   5 +
>>>  lib/efi_loader/efi_runtime.c  |  17 ++-
>>>  lib/efi_loader/efi_variable.c | 342
>>> ++++++++++++++++++++++++++++++++++++++++++
>>>  7 files changed, 394 insertions(+), 5 deletions(-)
>>>  create mode 100644 lib/efi_loader/efi_variable.c
>>> 
>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>> index b9e1e5e131..80f52e9e35 100644
>>> --- a/cmd/bootefi.c
>>> +++ b/cmd/bootefi.c
>>> @@ -181,6 +181,10 @@ static unsigned long do_bootefi_exec(void *efi, void
>>> *fdt,
>>>                goto exit;
>>>        }
>>>  +     /* we don't support much: */
>>> +
>>> setenv("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported",
>>> +                       "{ro,boot}(blob)0000000000000000");
>>> +
>>>        /* Call our payload! */
>>>        debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__,
>>> (long)entry);
>>>  diff --git a/include/efi.h b/include/efi.h
>>> index ddd2b96417..dbd482a5db 100644
>>> --- a/include/efi.h
>>> +++ b/include/efi.h
>>> @@ -324,6 +324,25 @@ extern char image_base[];
>>>  /* Start and end of U-Boot image (for payload) */
>>>  extern char _binary_u_boot_bin_start[], _binary_u_boot_bin_end[];
>>>  +/*
>>> + * Variable Attributes
>>> + */
>>> +#define EFI_VARIABLE_NON_VOLATILE       0x0000000000000001
>> 
>> 
>> Shouldn't we honor this one too? A UEFI application may set runtime
>> variables that should not get persisted across boots (think the UEFI shell
>> setting some internal state thing, then booting Linux).
> 
> So the thing is, we honor non-volatile (at least to the extent the
> board can do saveenv()).  What we don't do is make volatile vars
> disappear on reboot... which isn't terribly easy to do since we don't
> have any way to mark u-boot env vars as volatile.
> 
> But my reading of the spec doesn't preclude volatile variables from
> being persisted.  It only says that non-volatile variables should be
> persisted.

The spec actually says in the non_volatile definition that volatile vars get stored in ram and will disappear after reboot.

Volatile could just be an attribute like all the others and before saveenv we just search for volatile and remove them?



> 
>> 
>>> +#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0000000000000002
>>> +#define EFI_VARIABLE_RUNTIME_ACCESS     0x0000000000000004
>>> +#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x0000000000000008
>>> +#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x0000000000000010
>>> +#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
>>> 0x0000000000000020
>>> +#define EFI_VARIABLE_APPEND_WRITE      0x0000000000000040
>>> +
>>> +#define EFI_VARIABLE_MASK      (EFI_VARIABLE_NON_VOLATILE | \
>>> +                               EFI_VARIABLE_BOOTSERVICE_ACCESS | \
>>> +                               EFI_VARIABLE_RUNTIME_ACCESS | \
>>> +                               EFI_VARIABLE_HARDWARE_ERROR_RECORD | \
>>> +                               EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS |
>>> \
>>> +
>>> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS | \
>>> +                               EFI_VARIABLE_APPEND_WRITE)
>>> +
>>>  /**
>>>   * efi_get_sys_table() - Get access to the main EFI system table
>>>   *
>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>> index efb93f31f7..9cb568e615 100644
>>> --- a/include/efi_loader.h
>>> +++ b/include/efi_loader.h
>>> @@ -271,6 +271,16 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
>>>                        struct efi_time_cap *capabilities);
>>>  void efi_get_time_init(void);
>>>  +efi_status_t EFIAPI efi_get_variable(s16 *variable_name,
>>> +               efi_guid_t *vendor, u32 *attributes,
>>> +               unsigned long *data_size, void *data);
>>> +efi_status_t EFIAPI efi_get_next_variable(
>>> +               unsigned long *variable_name_size,
>>> +               s16 *variable_name, efi_guid_t *vendor);
>>> +efi_status_t EFIAPI efi_set_variable(s16 *variable_name,
>>> +               efi_guid_t *vendor, u32 attributes,
>>> +               unsigned long data_size, void *data);
>>> +
>>>  #else /* defined(EFI_LOADER) && !defined(CONFIG_SPL_BUILD) */
>>>    /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it
>>> out */
>>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>>> index cce92cfeb5..f58cb13337 100644
>>> --- a/lib/efi_loader/Makefile
>>> +++ b/lib/efi_loader/Makefile
>>> @@ -16,7 +16,7 @@ always := $(efiprogs-y)
>>>  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>>>  obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>>>  obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
>>> -obj-y += efi_file.o
>>> +obj-y += efi_file.o efi_variable.o
>>>  obj-$(CONFIG_LCD) += efi_gop.o
>>>  obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>>>  obj-$(CONFIG_PARTITIONS) += efi_disk.o
>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>>> index 7e44ba56e2..0bb64f0351 100644
>>> --- a/lib/efi_loader/efi_boottime.c
>>> +++ b/lib/efi_loader/efi_boottime.c
>>> @@ -927,6 +927,11 @@ static efi_status_t EFIAPI
>>> efi_exit_boot_services(void *image_handle,
>>>  {
>>>        EFI_ENTRY("%p, %ld", image_handle, map_key);
>>>  +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
>>> +       /* save any EFI variables that have been written: */
>>> +       saveenv();
>>> +#endif
>>> +
>>>        board_quiesce_devices();
>>>        /* Fix up caches for EFI payloads if necessary */
>>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>>> index dd52755d1d..7615090ba3 100644
>>> --- a/lib/efi_loader/efi_runtime.c
>>> +++ b/lib/efi_loader/efi_runtime.c
>>> @@ -184,7 +184,16 @@ static const struct efi_runtime_detach_list_struct
>>> efi_runtime_detach_list[] = {
>>>                /* Clean up system table */
>>>                .ptr = &systab.boottime,
>>>                .patchto = NULL,
>>> -       },
>>> +       }, {
>>> +               .ptr = &efi_runtime_services.get_variable,
>>> +               .patchto = &efi_device_error,
>>> +       }, {
>>> +               .ptr = &efi_runtime_services.get_next_variable,
>>> +               .patchto = &efi_device_error,
>>> +       }, {
>>> +               .ptr = &efi_runtime_services.set_variable,
>>> +               .patchto = &efi_device_error,
>>> +       }
>>>  };
>>>    static bool efi_runtime_tobedetached(void *p)
>>> @@ -382,9 +391,9 @@ struct efi_runtime_services __efi_runtime_data
>>> efi_runtime_services = {
>>>        .set_wakeup_time = (void *)&efi_unimplemented,
>>>        .set_virtual_address_map = &efi_set_virtual_address_map,
>>>        .convert_pointer = (void *)&efi_invalid_parameter,
>>> -       .get_variable = (void *)&efi_device_error,
>>> -       .get_next_variable = (void *)&efi_device_error,
>>> -       .set_variable = (void *)&efi_device_error,
>>> +       .get_variable = efi_get_variable,
>>> +       .get_next_variable = efi_get_next_variable,
>>> +       .set_variable = efi_set_variable,
>>>        .get_next_high_mono_count = (void *)&efi_device_error,
>>>        .reset_system = &efi_reset_system_boottime,
>>>  };
>>> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
>>> new file mode 100644
>>> index 0000000000..745514e4fa
>>> --- /dev/null
>>> +++ b/lib/efi_loader/efi_variable.c
>>> @@ -0,0 +1,342 @@
>>> +/*
>>> + *  EFI utils
>>> + *
>>> + *  Copyright (c) 2017 Rob Clark
>>> + *
>>> + *  SPDX-License-Identifier:     GPL-2.0+
>>> + */
>>> +
>>> +#include <malloc.h>
>>> +#include <charset.h>
>>> +#include <efi_loader.h>
>>> +
>>> +#define READ_ONLY BIT(31)
>>> +
>>> +/*
>>> + * 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 MAX_VAR_NAME 31
>>> +#define MAX_NATIVE_VAR_NAME \
>>> +       (strlen("efi_xxxxxxxx-xxxx-xxxx-xxxxxxxxxxxxxxxx_") + \
>>> +               (MAX_VAR_NAME * MAX_UTF8_PER_UTF16))
>>> +
>>> +static int hex(unsigned char ch)
>>> +{
>>> +       if (ch >= 'a' && ch <= 'f')
>>> +               return ch-'a'+10;
>>> +       if (ch >= '0' && ch <= '9')
>>> +               return ch-'0';
>>> +       if (ch >= 'A' && ch <= 'F')
>>> +               return ch-'A'+10;
>>> +       return -1;
>>> +}
>>> +
>>> +static const char * hex2mem(u8 *mem, const char *hexstr, int count)
>>> +{
>>> +       memset(mem, 0, count/2);
>>> +
>>> +       do {
>>> +               int nibble;
>>> +
>>> +               *mem = 0;
>>> +
>>> +               if (!count || !*hexstr)
>>> +                       break;
>>> +
>>> +               nibble = hex(*hexstr);
>>> +               if (nibble < 0)
>>> +                       break;
>>> +
>>> +               *mem = nibble;
>>> +               count--;
>>> +               hexstr++;
>>> +
>>> +               if (!count || !*hexstr)
>>> +                       break;
>>> +
>>> +               nibble = hex(*hexstr);
>>> +               if (nibble < 0)
>>> +                       break;
>>> +
>>> +               *mem = (*mem << 4) | nibble;
>>> +               count--;
>>> +               hexstr++;
>>> +               mem++;
>>> +
>>> +       } while (1);
>>> +
>>> +       if (*hexstr)
>>> +               return hexstr;
>>> +
>>> +       return NULL;
>>> +}
>>> +
>>> +static char * mem2hex(char *hexstr, const u8 *mem, int count)
>>> +{
>>> +       static const char hexchars[] = "0123456789abcdef";
>>> +
>>> +       while (count-- > 0) {
>>> +               u8 ch = *mem++;
>>> +               *hexstr++ = hexchars[ch >> 4];
>>> +               *hexstr++ = hexchars[ch & 0xf];
>>> +       }
>>> +
>>> +       return hexstr;
>>> +}
>>> +
>>> +static efi_status_t efi_to_native(char *native, s16 *variable_name,
>>> +               efi_guid_t *vendor)
>>> +{
>>> +       size_t len;
>>> +
>>> +       len = utf16_strlen((u16 *)variable_name);
>>> +       if (len >= MAX_VAR_NAME)
>>> +               return EFI_DEVICE_ERROR;
>>> +
>>> +       native += sprintf(native, "efi_%pUl_", vendor);
>>> +       native  = (char *)utf16_to_utf8((u8 *)native, (u16
>>> *)variable_name, len);
>>> +       *native = '\0';
>>> +
>>> +       return EFI_SUCCESS;
>>> +}
>>> +
>>> +static const char * prefix(const char *str, const char *prefix)
>> 
>> 
>> Isn't this just strncmp(prefix, str, strlen(prefix));?
>> 
> 
> I suppose it could be implemented that way, at the expense of an extra
> strlen(prefix).

Those should get optimized away in most cases, as prefix is usually a known constant and gcc can unfold strlen().

Alex
Rob Clark Aug. 12, 2017, 6:09 p.m.
On Sat, Aug 12, 2017 at 1:28 PM, Alexander Graf <agraf@suse.de> wrote:
>
>
>> Am 12.08.2017 um 16:39 schrieb Rob Clark <robdclark@gmail.com>:
>>
>>> On Sat, Aug 12, 2017 at 9:01 AM, Alexander Graf <agraf@suse.de> wrote:
>>>
>>>
>>>> On 10.08.17 20:29, Rob Clark wrote:
>>>>
>>>> Add EFI variable support, mapping to u-boot environment variables.
>>>> Variables are pretty important for setting up boot order, among other
>>>> things.  If the board supports saveenv, then it will be called in
>>>> ExitBootServices() to persist variables set by the efi payload.  (For
>>>> example, fallback.efi configuring BootOrder and BootXXXX load-option
>>>> variables.)
>>>>
>>>> Variables are *not* currently exposed at runtime, post ExitBootServices.
>>>> On boards without a dedicated device for storage, which the loaded OS
>>>> is not trying to also use, this is rather tricky.  One idea, at least
>>>> for boards that can persist RAM across reboot, is to keep a "journal"
>>>> of modified variables in RAM, and then turn halt into a reboot into
>>>> u-boot, plus store variables, plus halt.  Whatever the solution, it
>>>> likely involves some per-board support.
>>>>
>>>> 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
>>>>
>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>> ---
>>>>  cmd/bootefi.c                 |   4 +
>>>>  include/efi.h                 |  19 +++
>>>>  include/efi_loader.h          |  10 ++
>>>>  lib/efi_loader/Makefile       |   2 +-
>>>>  lib/efi_loader/efi_boottime.c |   5 +
>>>>  lib/efi_loader/efi_runtime.c  |  17 ++-
>>>>  lib/efi_loader/efi_variable.c | 342
>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>  7 files changed, 394 insertions(+), 5 deletions(-)
>>>>  create mode 100644 lib/efi_loader/efi_variable.c
>>>>
>>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>>> index b9e1e5e131..80f52e9e35 100644
>>>> --- a/cmd/bootefi.c
>>>> +++ b/cmd/bootefi.c
>>>> @@ -181,6 +181,10 @@ static unsigned long do_bootefi_exec(void *efi, void
>>>> *fdt,
>>>>                goto exit;
>>>>        }
>>>>  +     /* we don't support much: */
>>>> +
>>>> setenv("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported",
>>>> +                       "{ro,boot}(blob)0000000000000000");
>>>> +
>>>>        /* Call our payload! */
>>>>        debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__,
>>>> (long)entry);
>>>>  diff --git a/include/efi.h b/include/efi.h
>>>> index ddd2b96417..dbd482a5db 100644
>>>> --- a/include/efi.h
>>>> +++ b/include/efi.h
>>>> @@ -324,6 +324,25 @@ extern char image_base[];
>>>>  /* Start and end of U-Boot image (for payload) */
>>>>  extern char _binary_u_boot_bin_start[], _binary_u_boot_bin_end[];
>>>>  +/*
>>>> + * Variable Attributes
>>>> + */
>>>> +#define EFI_VARIABLE_NON_VOLATILE       0x0000000000000001
>>>
>>>
>>> Shouldn't we honor this one too? A UEFI application may set runtime
>>> variables that should not get persisted across boots (think the UEFI shell
>>> setting some internal state thing, then booting Linux).
>>
>> So the thing is, we honor non-volatile (at least to the extent the
>> board can do saveenv()).  What we don't do is make volatile vars
>> disappear on reboot... which isn't terribly easy to do since we don't
>> have any way to mark u-boot env vars as volatile.
>>
>> But my reading of the spec doesn't preclude volatile variables from
>> being persisted.  It only says that non-volatile variables should be
>> persisted.
>
> The spec actually says in the non_volatile definition that volatile vars get stored in ram and will disappear after reboot.
>
> Volatile could just be an attribute like all the others and before saveenv we just search for volatile and remove them?
>

I could add it as an attribute.  Although I'm not sure there is any
easy way to iterate env vars without digging into internals of nvedit
(otherwise I probably would have already added GetNextVariable())

Possibly I should add an nv attribute anyways, for future-compat
(which was the only reason I bothered adding boot and run attributes)

BR,
-R

Patch hide | download patch | download mbox

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index b9e1e5e131..80f52e9e35 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -181,6 +181,10 @@  static unsigned long do_bootefi_exec(void *efi, void *fdt,
 		goto exit;
 	}
 
+	/* we don't support much: */
+	setenv("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported",
+			"{ro,boot}(blob)0000000000000000");
+
 	/* Call our payload! */
 	debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry);
 
diff --git a/include/efi.h b/include/efi.h
index ddd2b96417..dbd482a5db 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -324,6 +324,25 @@  extern char image_base[];
 /* Start and end of U-Boot image (for payload) */
 extern char _binary_u_boot_bin_start[], _binary_u_boot_bin_end[];
 
+/*
+ * Variable Attributes
+ */
+#define EFI_VARIABLE_NON_VOLATILE       0x0000000000000001
+#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0000000000000002
+#define EFI_VARIABLE_RUNTIME_ACCESS     0x0000000000000004
+#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x0000000000000008
+#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x0000000000000010
+#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x0000000000000020
+#define EFI_VARIABLE_APPEND_WRITE	0x0000000000000040
+
+#define EFI_VARIABLE_MASK 	(EFI_VARIABLE_NON_VOLATILE | \
+				EFI_VARIABLE_BOOTSERVICE_ACCESS | \
+				EFI_VARIABLE_RUNTIME_ACCESS | \
+				EFI_VARIABLE_HARDWARE_ERROR_RECORD | \
+				EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \
+				EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS | \
+				EFI_VARIABLE_APPEND_WRITE)
+
 /**
  * efi_get_sys_table() - Get access to the main EFI system table
  *
diff --git a/include/efi_loader.h b/include/efi_loader.h
index efb93f31f7..9cb568e615 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -271,6 +271,16 @@  efi_status_t __efi_runtime EFIAPI efi_get_time(
 			struct efi_time_cap *capabilities);
 void efi_get_time_init(void);
 
+efi_status_t EFIAPI efi_get_variable(s16 *variable_name,
+		efi_guid_t *vendor, u32 *attributes,
+		unsigned long *data_size, void *data);
+efi_status_t EFIAPI efi_get_next_variable(
+		unsigned long *variable_name_size,
+		s16 *variable_name, efi_guid_t *vendor);
+efi_status_t EFIAPI efi_set_variable(s16 *variable_name,
+		efi_guid_t *vendor, u32 attributes,
+		unsigned long data_size, void *data);
+
 #else /* defined(EFI_LOADER) && !defined(CONFIG_SPL_BUILD) */
 
 /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index cce92cfeb5..f58cb13337 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -16,7 +16,7 @@  always := $(efiprogs-y)
 obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
 obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
 obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
-obj-y += efi_file.o
+obj-y += efi_file.o efi_variable.o
 obj-$(CONFIG_LCD) += efi_gop.o
 obj-$(CONFIG_DM_VIDEO) += efi_gop.o
 obj-$(CONFIG_PARTITIONS) += efi_disk.o
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 7e44ba56e2..0bb64f0351 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -927,6 +927,11 @@  static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,
 {
 	EFI_ENTRY("%p, %ld", image_handle, map_key);
 
+#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
+	/* save any EFI variables that have been written: */
+	saveenv();
+#endif
+
 	board_quiesce_devices();
 
 	/* Fix up caches for EFI payloads if necessary */
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index dd52755d1d..7615090ba3 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -184,7 +184,16 @@  static const struct efi_runtime_detach_list_struct efi_runtime_detach_list[] = {
 		/* Clean up system table */
 		.ptr = &systab.boottime,
 		.patchto = NULL,
-	},
+	}, {
+		.ptr = &efi_runtime_services.get_variable,
+		.patchto = &efi_device_error,
+	}, {
+		.ptr = &efi_runtime_services.get_next_variable,
+		.patchto = &efi_device_error,
+	}, {
+		.ptr = &efi_runtime_services.set_variable,
+		.patchto = &efi_device_error,
+	}
 };
 
 static bool efi_runtime_tobedetached(void *p)
@@ -382,9 +391,9 @@  struct efi_runtime_services __efi_runtime_data efi_runtime_services = {
 	.set_wakeup_time = (void *)&efi_unimplemented,
 	.set_virtual_address_map = &efi_set_virtual_address_map,
 	.convert_pointer = (void *)&efi_invalid_parameter,
-	.get_variable = (void *)&efi_device_error,
-	.get_next_variable = (void *)&efi_device_error,
-	.set_variable = (void *)&efi_device_error,
+	.get_variable = efi_get_variable,
+	.get_next_variable = efi_get_next_variable,
+	.set_variable = efi_set_variable,
 	.get_next_high_mono_count = (void *)&efi_device_error,
 	.reset_system = &efi_reset_system_boottime,
 };
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
new file mode 100644
index 0000000000..745514e4fa
--- /dev/null
+++ b/lib/efi_loader/efi_variable.c
@@ -0,0 +1,342 @@ 
+/*
+ *  EFI utils
+ *
+ *  Copyright (c) 2017 Rob Clark
+ *
+ *  SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#include <malloc.h>
+#include <charset.h>
+#include <efi_loader.h>
+
+#define READ_ONLY BIT(31)
+
+/*
+ * 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 MAX_VAR_NAME 31
+#define MAX_NATIVE_VAR_NAME \
+	(strlen("efi_xxxxxxxx-xxxx-xxxx-xxxxxxxxxxxxxxxx_") + \
+		(MAX_VAR_NAME * MAX_UTF8_PER_UTF16))
+
+static int hex(unsigned char ch)
+{
+	if (ch >= 'a' && ch <= 'f')
+		return ch-'a'+10;
+	if (ch >= '0' && ch <= '9')
+		return ch-'0';
+	if (ch >= 'A' && ch <= 'F')
+		return ch-'A'+10;
+	return -1;
+}
+
+static const char * hex2mem(u8 *mem, const char *hexstr, int count)
+{
+	memset(mem, 0, count/2);
+
+	do {
+		int nibble;
+
+		*mem = 0;
+
+		if (!count || !*hexstr)
+			break;
+
+		nibble = hex(*hexstr);
+		if (nibble < 0)
+			break;
+
+		*mem = nibble;
+		count--;
+		hexstr++;
+
+		if (!count || !*hexstr)
+			break;
+
+		nibble = hex(*hexstr);
+		if (nibble < 0)
+			break;
+
+		*mem = (*mem << 4) | nibble;
+		count--;
+		hexstr++;
+		mem++;
+
+	} while (1);
+
+	if (*hexstr)
+		return hexstr;
+
+	return NULL;
+}
+
+static char * mem2hex(char *hexstr, const u8 *mem, int count)
+{
+	static const char hexchars[] = "0123456789abcdef";
+
+	while (count-- > 0) {
+		u8 ch = *mem++;
+		*hexstr++ = hexchars[ch >> 4];
+		*hexstr++ = hexchars[ch & 0xf];
+	}
+
+	return hexstr;
+}
+
+static efi_status_t efi_to_native(char *native, s16 *variable_name,
+		efi_guid_t *vendor)
+{
+	size_t len;
+
+	len = utf16_strlen((u16 *)variable_name);
+	if (len >= MAX_VAR_NAME)
+		return EFI_DEVICE_ERROR;
+
+	native += sprintf(native, "efi_%pUl_", vendor);
+	native  = (char *)utf16_to_utf8((u8 *)native, (u16 *)variable_name, len);
+	*native = '\0';
+
+	return EFI_SUCCESS;
+}
+
+static const char * prefix(const char *str, const char *prefix)
+{
+	while (*str && *prefix) {
+		if (*str != *prefix)
+			break;
+		str++;
+		prefix++;
+	}
+
+	if (*prefix)
+		return NULL;
+
+	return str;
+}
+
+/* parse attributes part of variable value, if present: */
+static const char * parse_attr(const char *str, u32 *attrp)
+{
+	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 |= READ_ONLY;
+		} else if ((s = prefix(str, "boot"))) {
+			attr |= EFI_VARIABLE_BOOTSERVICE_ACCESS;
+		} else if ((s = prefix(str, "run"))) {
+			attr |= EFI_VARIABLE_RUNTIME_ACCESS;
+		} else {
+			printf("invalid attribute: %s\n", str);
+			break;
+		}
+
+		str = s;
+		sep = ',';
+	}
+
+	str++;
+
+	*attrp = attr;
+
+	return str;
+}
+
+/* http://wiki.phoenix.com/wiki/index.php/EFI_RUNTIME_SERVICES#GetVariable.28.29 */
+efi_status_t EFIAPI efi_get_variable(s16 *variable_name,
+		efi_guid_t *vendor, u32 *attributes,
+		unsigned long *data_size, void *data)
+{
+	char native_name[MAX_NATIVE_VAR_NAME + 1];
+	efi_status_t ret;
+	unsigned long in_size;
+	const char *val, *s;
+	u32 attr;
+
+	EFI_ENTRY("%p %p %p %p %p", variable_name, vendor, attributes,
+			data_size, data);
+
+	if (!variable_name || !vendor || !data_size)
+		return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+	ret = efi_to_native(native_name, variable_name, vendor);
+	if (ret)
+		return EFI_EXIT(ret);
+
+	debug("%s: get '%s'\n", __func__, native_name);
+
+	val = getenv(native_name);
+	if (!val)
+		return EFI_EXIT(EFI_NOT_FOUND);
+
+	val = parse_attr(val, &attr);
+
+	in_size = *data_size;
+
+	if ((s = prefix(val, "(blob)"))) {
+		unsigned len = strlen(s);
+
+		/* two characters per byte: */
+		len = DIV_ROUND_UP(len, 2);
+		*data_size = len;
+
+		if (in_size < len)
+			return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
+
+		if (!data)
+			return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+		if (hex2mem(data, s, len * 2))
+			return EFI_EXIT(EFI_DEVICE_ERROR);
+
+		debug("%s: got value: \"%s\"\n", __func__, s);
+	} else if ((s = prefix(val, "(utf8)"))) {
+		unsigned len = strlen(s) + 1;
+
+		*data_size = len;
+
+		if (in_size < len)
+			return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
+
+		if (!data)
+			return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+		memcpy(data, s, len);
+		((char *)data)[len] = '\0';
+
+		debug("%s: got value: \"%s\"\n", __func__, (char *)data);
+	} else {
+		debug("%s: invalid value: '%s'\n", __func__, val);
+		return EFI_EXIT(EFI_DEVICE_ERROR);
+	}
+
+	if (attributes)
+		*attributes = attr & EFI_VARIABLE_MASK;
+
+	return EFI_EXIT(EFI_SUCCESS);
+}
+
+/* http://wiki.phoenix.com/wiki/index.php/EFI_RUNTIME_SERVICES#GetNextVariableName.28.29 */
+efi_status_t EFIAPI efi_get_next_variable(
+		unsigned long *variable_name_size,
+		s16 *variable_name, efi_guid_t *vendor)
+{
+	EFI_ENTRY("%p %p %p", variable_name_size, variable_name, vendor);
+
+	return EFI_EXIT(EFI_DEVICE_ERROR);
+}
+
+/* http://wiki.phoenix.com/wiki/index.php/EFI_RUNTIME_SERVICES#SetVariable.28.29 */
+efi_status_t EFIAPI efi_set_variable(s16 *variable_name,
+		efi_guid_t *vendor, u32 attributes,
+		unsigned long data_size, void *data)
+{
+	char native_name[MAX_NATIVE_VAR_NAME + 1];
+	efi_status_t ret = EFI_SUCCESS;
+	char *val, *s;
+	u32 attr;
+
+	EFI_ENTRY("%p %p %x %lu %p", variable_name, vendor, attributes,
+			data_size, data);
+
+	if (!variable_name || !vendor || !data_size)
+		return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+	ret = efi_to_native(native_name, variable_name, vendor);
+	if (ret)
+		return EFI_EXIT(ret);
+
+#define ACCESS_ATTR (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)
+
+	if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {
+		/* delete the variable: */
+		setenv(native_name, NULL);
+		return EFI_EXIT(EFI_SUCCESS);
+	}
+
+	val = getenv(native_name);
+	if (val) {
+		parse_attr(val, &attr);
+
+		if (attr & READ_ONLY)
+			return EFI_EXIT(EFI_WRITE_PROTECTED);
+	}
+
+	val = malloc(2 * data_size + strlen("{ro,run,boot}(blob)") + 1);
+	if (!val)
+		return EFI_EXIT(EFI_OUT_OF_RESOURCES);
+
+	s = val;
+
+	/* store attributes: */
+	attributes &= (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)
+			s += sprintf(s, "boot");
+		else if (attr == EFI_VARIABLE_RUNTIME_ACCESS)
+			s += sprintf(s, "run");
+
+		attributes &= ~attr;
+		if (attributes)
+			s += sprintf(s, ",");
+	}
+	s += sprintf(s, "}");
+
+	/* store payload: */
+	s += sprintf(s, "(blob)");
+	s = mem2hex(s, data, data_size);
+	*s = '\0';
+
+	debug("%s: setting: %s=%s\n", __func__, native_name, val);
+
+	if (setenv(native_name, val))
+		ret = EFI_DEVICE_ERROR;
+
+	free(val);
+
+	return EFI_EXIT(ret);
+}