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

[Rob Clark]

On Sat, Aug 12, 2017 at 1:28 PM, Alexander Graf <agraf at suse.de> wrote:
>
>
>> Am 12.08.2017 um 16:39 schrieb Rob Clark <robdclark at gmail.com>:
>>
>>> On Sat, Aug 12, 2017 at 9:01 AM, Alexander Graf <agraf at 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 at 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