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

[Rob Clark]

On Sat, Aug 12, 2017 at 1:28 PM, Alexander Graf  wrote:
>
>
>> Am 12.08.2017 um 16:39 schrieb Rob Clark :
>>
>>> On Sat, Aug 12, 2017 at 9:01 AM, Alexander Graf  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 Boot 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)"
   efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_BootOrder=
  "(blob)0001"

 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 
 ---
  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)");
 +
/* 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   0x0001
>>>
>>>
>>> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Alexander Graf]

> Am 12.08.2017 um 16:39 schrieb Rob Clark :
> 
>> On Sat, Aug 12, 2017 at 9:01 AM, Alexander Graf  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 Boot 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)"
>>>   efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_BootOrder=
>>>  "(blob)0001"
>>> 
>>> 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 
>>> ---
>>>  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)");
>>> +
>>>/* 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   0x0001
>> 
>> 
>> 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 0x0002
>>> +#define EFI_VARIABLE_RUNTIME_ACCESS 0x0004
>>> +#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x0008
>>> +#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x0010
>>> +#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
>>> 0x0020
>>> +#define EFI_VARIABLE_APPEND_WRITE  0x0040
>>> +
>>> +#define EFI_VARIABLE_MASK  (EFI_VARIABLE_NON_VOLATILE | \
>>> +   EFI_VARIABLE_BOOTSERVICE_ACCESS | \
>>> +   EFI_VARIABLE_RUNTIME_ACCESS | \
>>> +   

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

[Rob Clark]

On Sat, Aug 12, 2017 at 9:01 AM, Alexander Graf  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 Boot 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)"
>>efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_BootOrder=
>>   "(blob)0001"
>>
>> 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 
>> ---
>>   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)");
>> +
>> /* 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   0x0001
>
>
> 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 0x0002
>> +#define EFI_VARIABLE_RUNTIME_ACCESS 0x0004
>> +#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x0008
>> +#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x0010
>> +#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
>> 0x0020
>> +#define EFI_VARIABLE_APPEND_WRITE  0x0040
>> +
>> +#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

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

[Alexander Graf]

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 Boot 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)"
   efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_BootOrder=
  "(blob)0001"

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 
---
  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)");
+
/* 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   0x0001


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 0x0002
+#define EFI_VARIABLE_RUNTIME_ACCESS 0x0004
+#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x0008
+#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x0010
+#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x0020
+#define EFI_VARIABLE_APPEND_WRITE  0x0040
+
+#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) &&