On Thu, Apr 25, 2019 at 12:12:39AM +0200, Heinrich Schuchardt wrote:
> On 4/24/19 8:30 AM, AKASHI Takahiro wrote:
> >This patch set is an attempt to implement non-volatile attribute for
> >UEFI variables. Under the current implementation,
> >* SetVariable API doesn't recognize non-volatile attribute
> >* While some variables are defined non-volatile in UEFI specification,
> > they are NOT marked as non-volatile in the code.
> >* env_save() (or "env save" command) allows us to save all the variables
> > into persistent storage, but it may cause volatile UEFI variables,
> > along with irrelevant U-Boot variables, to be saved unconditionally.
> >
> >Those observation rationalizes that the implementation of UEFI variables
> >should be revamped utilizing dedicated storage for them.
> >
> >This patch set is yet experimental and rough-edged(See known issues below),
> >but shows how UEFI variables can be split from U-Boot environment.
> >This enhancement will also be vital when we introduce UEFI secure boot
> >where secure and tamper-resistant storage (with authentication) is
> >required.
> >
> >Usage:
> >To enable this feature, the following configs must be enabled:
> > CONFIG_ENV_IS_IN_FAT
> > CONFIG_ENV_FAT_INTERFACE
> > CONFIG_ENV_EFI_FAT_DEVICE_AND_PART
> > CONFIG_ENV_EFI_FAT_FILE
> >
> >You can also define a non-volatile variable from command interface:
> >=> setenv -e -nv FOO baa
> >
> >Known issues/restriction:
> >* UEFI spec defines "globally defined variables" with specific
> > attributes, but with this patch, we don't check against the user-supplied
> > attribute for any variable.
> >* Only FAT can be enabled for persistent storage for UEFI non-volatile
> > variables.
> >* The whole area of storage will be saved at every update of one variable.
> > It can be optimized.
> >* An error during saving may cause inconsistency between cache (hash table)
> > and the storage.
> >* Cache is of fixed size and can be quite big for normal usage.
>
> Hello Takahiro,
>
> thanks a lot for looking into persisting non-volatile UEFI variables.
>
> Before diving into the details of the patches let us discuss the overall
> design.
I like this kind of discussions as this patch set is more or less RFC.
> My understanding is that we need a buffer that lives in
> EFI_RUNTIME_SERVICES_DATA and holds all variables. It is this buffer
> that the operating system will access when getting or setting variables.
>
> If a non-volatile variable is set via SetVariable() we will call a
> backend driver.
Get/SetVariable() is not mandatory in runtime services, and EBBR,
at least, says:
If any EFI_RUNTIME_SERVICES functions are only available during
boot services then firmware shall provide
the global RuntimeServicesSupported variable to indicate which
functions are available during runtime services.
(in section 2.5), and
Even when SetVariable() is not supported during runtime services,
firmware should cache variable names and
values in EfiRuntimeServicesData memory so that GetVariable() and
GetNextVeriableName() can behave as specified.
(in section 2.5.3)
# Variable update can also be done via 'capsule.'
> Our current backend is using U-Boot environment variables. It can only
> be accessed at boottime.
Right.
> Currently there is no guarantee that the U-Boot
> variables are saved before booting.
Right, but if 'saved before booting' were the only issue,
we could call env_save() in efi_start_image().
The problems are not there.
> So it does not support persisting
> non-volatile variables reliably.
>
> Your patch series supplies a new backend using a file for persisting
> non-volatile variables. It can only be accessed at boottime. Essentially
> this solution requires no restriction to FAT file systems. We could as
> well use the EXT4 driver for reading or writing the file.
Right, we can also use flash, nand, emmc and others like U-Boot environment
as we still reply on helper functions for U-Boot environment.
> This file
> storage is completely independent of the U-Boot environment.
No. We don't have to care about format in persistent storage.
What is required is that we should provide Get/SetVariable APIs.
> So it shall
> not involve any changes to files env/*. I think a documentation of the
> file format provided in a README would be helpful.
>
> In case of both backends we would return EFI_UNSUPPORTED when trying to
> set a non-volatile variable at runtime. Setting a volatile variable at
> runtime should be allowable as long as we have sufficient space in our
> buffer.
Well, there are several possibilities:
1. We don't support Get/SetVariable in runtime at all
2. GetVariable is available through 'cache' (U-Boot environment uses
a on-memory hash tables to maintain variables.)
Optionally, SetVariable can be achieved via capsule.
3. Get/SetVariable is implemented by an independent entity and gets
avaiable even in runtime.
This will be true if persistent storage is provided by secure world,
in particular EL3 on arm. It will be quite likely for secure boot.
> We can remove these backends based on the EVT_SIGNAL_EXIT_BOOT_SERVICES
> event.
>
> A future backend may support persisting non-volatile variables at runtime.
>
> I would prefer if we could first introduce patches that provide the
> buffer for EFI variables and link it to the UEFI variable based backend
> before adding the new backend as alternative.
I'm not sure whether I understand your point fully, but
my current patch does that, doesn't it?
# We have to implement (2) above yet.
Thanks,
-Takahiro Akashi
> Best regards
>
> Heinrich
>
>
> >
> >Patch#1 to #4 are preparatory so that we won't rely on U-Boot environment,
> >that is, env_get/set() helper functions.
> >Patch#5 to #8 are core part of changes.
> >Patch#9 to #11 are for modifying variable attributes.
> >
> >Changes in v2 (Apr 24, 2019)
> >* rebased on efi-2019-07
> >* revamp the implementation
> >
> >v1 (Nov 28, 2018)
> >* initial
> >
> >AKASHI Takahiro (11):
> > lib: charset: add u16_strcmp()
> > lib: charset: add u16_strncmp()
> > cmd: efidebug: rework "boot dump" sub-command using
> > GetNextVariableName()
> > efi_loader: set OsIndicationsSupported at init
> > env: save UEFI non-volatile variables in dedicated storage
> > efi_loader: variable: support non-volatile attribute
> > efi_loader: variable: split UEFI variables from U-Boot environment
> > efi_loader: load saved non-volatile variables at init
> > efi_loader: bootmgr: handle BootNext as non-volatile
> > cmd: env: add -nv option for UEFI non-volatile variable
> > cmd: efidebug: make some boot variables non-volatile
> >
> > cmd/bootefi.c | 4 -
> > cmd/efidebug.c | 95 +++++++++++-----
> > cmd/nvedit.c | 3 +-
> > cmd/nvedit_efi.c | 15 ++-
> > env/Kconfig | 34 ++++++
> > env/env.c | 98 ++++++++++++++++-
> > env/fat.c | 109 +++++++++++++++++++
> > include/asm-generic/global_data.h | 1 +
> > include/charset.h | 10 ++
> > include/environment.h | 24 +++++
> > lib/charset.c | 23 ++++
> > lib/efi_loader/efi_bootmgr.c | 3 +-
> > lib/efi_loader/efi_setup.c | 13 +++
> > lib/efi_loader/efi_variable.c | 174 ++++++++++++++++++++++++++++--
> > 14 files changed, 560 insertions(+), 46 deletions(-)
> >
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
