[U-Boot] [PATCH v5 18/19] env, efi_loader: define env context for UEFI variables
Heinrich Schuchardt
xypron.glpk at gmx.de
Thu Sep 5 19:37:37 UTC 2019
On 9/5/19 10:21 AM, AKASHI Takahiro wrote:
> We will use two environment contexts for implementing UEFI variables,
> one (ctx_efi) for non-volatile variables and the other for volatile
> variables. The latter doesn't have a backing storage.
>
> Those two contexts are currently used only in efi_variable.c and
> can be moved into there if desired. But I let it under env/ for
> future use elsewhere.
>
> In this commit, FAT file system and flash device are only supported
> devices for backing storages, but extending to other devices will be
> quite straightforward.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
> env/Kconfig | 1 +
> env/Kconfig.efi | 152 ++++++++++++++++++++++++++++++++++++++++++++++
> env/Makefile | 1 +
> env/env_ctx_efi.c | 131 +++++++++++++++++++++++++++++++++++++++
> include/env.h | 3 +
> 5 files changed, 288 insertions(+)
> create mode 100644 env/Kconfig.efi
> create mode 100644 env/env_ctx_efi.c
>
> diff --git a/env/Kconfig b/env/Kconfig
> index ae96cf75bbaa..0cd3caa67376 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -47,5 +47,6 @@ config ENV_DRV_UBI
> bool
>
> source "env/Kconfig.uboot"
> +source "env/Kconfig.efi"
>
> endmenu
> diff --git a/env/Kconfig.efi b/env/Kconfig.efi
> new file mode 100644
> index 000000000000..cd4e78989df6
> --- /dev/null
> +++ b/env/Kconfig.efi
> @@ -0,0 +1,152 @@
> +if EFI_LOADER
> +
> +menu "Configure UEFI context"
> +
Essentially the 3 variable below exclude each other.
Please, use a 'choice' statement. drivers/video/Kconfig has an example.
> +config ENV_EFI_IS_NOWHERE
> + bool
> + default y if !ENV_EFI_IS_INFLASH && !ENV_EFI_IS_IN_FAT
> +
> +config ENV_EFI_IS_IN_FAT
> + bool "EFI Environment is in a FAT filesystem"
> + select ENV_DRV_FAT
> + help
> + Define this if you want to use the FAT file system for
> + the environment.
> +
> +config ENV_EFI_IS_IN_FLASH
> + bool "EFI Environment in flash memory"
> + select ENV_DRV_FLASH
> + help
> + Define this if you have a flash device which you want to use
> + for the environment.
> +
> + a) The environment occupies one whole flash sector, which is
> + "embedded" in the text segment with the U-Boot code. This
Are we really limited to exactly one sector. Or is this the minimum size?
> + happens usually with "bottom boot sector" or "top boot
> + sector" type flash chips, which have several smaller
> + sectors at the start or the end. For instance, such a
> + layout can have sector sizes of 8, 2x4, 16, Nx32 kB. In
> + such a case you would place the environment in one of the
> + 4 kB sectors - with U-Boot code before and after it. With
> + "top boot sector" type flash chips, you would put the
> + environment in one of the last sectors, leaving a gap
> + between U-Boot and the environment.
> +
> + CONFIG_ENV_EFI_OFFSET:
> +
> + Offset of environment data (variable area) to the
> + beginning of flash memory; for instance, with bottom boot
> + type flash chips the second sector can be used: the offset
> + for this sector is given here.
> +
> + CONFIG_ENV_EFI_OFFSET is used relative to CONFIG_SYS_FLASH_BASE.
> +
> + CONFIG_ENV_EFI_ADDR:
> +
> + This is just another way to specify the start address of
> + the flash sector containing the environment (instead of
> + CONFIG_ENV_OFFSET).
> +
> + CONFIG_ENV_EFI_SECT_SIZE:
> +
> + Size of the sector containing the environment.
> +
> +
> + b) Sometimes flash chips have few, equal sized, BIG sectors.
> + In such a case you don't want to spend a whole sector for
> + the environment.
> +
> + CONFIG_ENV_EFI_SIZE:
> +
> + If you use this in combination with CONFIG_ENV_IS_IN_FLASH
> + and CONFIG_ENV_SECT_SIZE, you can specify to use only a part
> + of this flash sector for the environment. This saves
> + memory for the RAM copy of the environment.
> +
> + It may also save flash memory if you decide to use this
> + when your environment is "embedded" within U-Boot code,
> + since then the remainder of the flash sector could be used
> + for U-Boot code. It should be pointed out that this is
> + STRONGLY DISCOURAGED from a robustness point of view:
> + updating the environment in flash makes it always
> + necessary to erase the WHOLE sector. If something goes
> + wrong before the contents has been restored from a copy in
> + RAM, your target system will be dead.
> +
> + CONFIG_ENV_EFI_ADDR_REDUND
> + CONFIG_ENV_EFI_SIZE_REDUND
> +
> + These settings describe a second storage area used to hold
> + a redundant copy of the environment data, so that there is
> + a valid backup copy in case there is a power failure during
> + a "saveenv" operation.
> +
> + BE CAREFUL! Any changes to the flash layout, and some changes to the
> + source code will make it necessary to adapt <board>/u-boot.lds*
> + accordingly!
> +
> +config ENV_EFI_FAT_INTERFACE
> + string "Name of the block device for the environment"
> + depends on ENV_EFI_IS_IN_FAT
> + help
> + Define this to a string that is the name of the block device.
> +
> +config ENV_EFI_FAT_DEVICE_AND_PART
> + string "Device and partition for where to store the environment in FAT"
> + depends on ENV_EFI_IS_IN_FAT
> + help
> + Define this to a string to specify the partition of the device.
> + It can be as following:
> +
> + "D:P", "D:0", "D", "D:" or "D:auto" (D, P are integers. And P >= 1)
> + - "D:P": device D partition P. Error occurs if device D has no
> + partition table.
> + - "D:0": device D.
> + - "D" or "D:": device D partition 1 if device D has partition
> + table, or the whole device D if has no partition
> + table.
> + - "D:auto": first partition in device D with bootable flag set.
> + If none, first valid partition in device D. If no
> + partition table then means device D.
> +
> +config ENV_EFI_FAT_FILE
> + string "Name of the FAT file to use for the environment"
> + depends on ENV_EFI_IS_IN_FAT
> + default "uboot_efi.env"
> + help
> + It's a string of the FAT file name. This file use to store the
> + environment.
> +
> +config ENV_EFI_ADDR
'depends on' is missing for this variable and all below.
> + hex "EFI Environment Address"
> + help
> + Start address of the device (or partition)
> +
> +config ENV_EFI_OFFSET
> + hex "EFI Environment Offset"
> + help
> + Offset from the start of the device (or partition)
> +
> +config ENV_EFI_SIZE
> + hex "EFI Environment Size"
> + help
> + Size of the environment storage area
> +
> +config ENV_EFI_SECT_SIZE
> + hex "EFI Environment Sector-Size"
> + help
> + Size of the sector containing the environment.
> +
> +config ENV_EFI_ADDR_REDUND
> + hex "EFI Environment Address for second area"
> + help
> + Start address of the device (or partition)
> +
> +config ENV_EFI_SIZE_REDUND
> + hex "EFI Environment Size for second area"
> + help
> + Size of the environment second storage area
> +
> +endmenu
> +
> +endif
> diff --git a/env/Makefile b/env/Makefile
> index 0168eb47f00d..b6690c73b17f 100644
> --- a/env/Makefile
> +++ b/env/Makefile
> @@ -4,6 +4,7 @@
> # Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>
> obj-y += common.o env.o env_ctx_uboot.o
> +obj-$(CONFIG_EFI_LOADER) += env_ctx_efi.o
>
> ifndef CONFIG_SPL_BUILD
> obj-y += attr.o
> diff --git a/env/env_ctx_efi.c b/env/env_ctx_efi.c
> new file mode 100644
> index 000000000000..9b5b2f392179
> --- /dev/null
> +++ b/env/env_ctx_efi.c
> @@ -0,0 +1,131 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Linaro Limited
> + * Author: AKASHI Takahiro
> + */
> +
> +#include <common.h>
> +#include <env_flags.h>
> +#include <env_internal.h>
> +#include <search.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct hsearch_data efi_htab = {
> +#if CONFIG_IS_ENABLED(ENV_SUPPORT)
> + /* defined in flags.c, only compile with ENV_SUPPORT */
> + .change_ok = env_flags_validate,
> +#endif
> +};
> +
> +struct hsearch_data efi_volatile_htab = {
> +#if CONFIG_IS_ENABLED(ENV_SUPPORT)
> + /* defined in flags.c, only compile with ENV_SUPPORT */
> + .change_ok = env_flags_validate,
> +#endif
> +};
> +
> +static int env_drv_init_efi(struct env_context *ctx, enum env_location loc)
> +{
> + __maybe_unused int ret;
> +
> + switch (loc) {
> +#ifdef CONFIG_ENV_EFI_IS_IN_FLASH
> + case ENVL_FLASH: {
> + env_t *env_ptr;
> + env_t *flash_addr;
> + ulong end_addr;
> + env_t *flash_addr_new;
> + ulong end_addr_new;
> +
> +#if defined(CONFIG_ENV_EFI_ADDR_REDUND) && defined(CMD_SAVEENV) || \
> + !defined(CONFIG_ENV_EFI_ADDR_REDUND) && defined(INITENV)
> +#ifdef ENV_IS_EMBEDDED
> + /* FIXME: not allowed */
Is something wrong in Kconfig that an non-allowed situation can occur?
> + env_ptr = NULL;
> +#else /* ! ENV_IS_EMBEDDED */
> +
> + env_ptr = (env_t *)CONFIG_ENV_EFI_ADDR;
> +#endif /* ENV_IS_EMBEDDED */
> +#else
> + env_ptr = NULL;
> +#endif
> + flash_addr = (env_t *)CONFIG_ENV_EFI_ADDR;
> +
> +/* CONFIG_ENV_EFI_ADDR is supposed to be on sector boundary */
> + end_addr = CONFIG_ENV_EFI_ADDR + CONFIG_ENV_EFI_SECT_SIZE - 1;
> +
> +#ifdef CONFIG_ENV_EFI_ADDR_REDUND
> + flash_addr_new = (env_t *)CONFIG_ENV_EFI_ADDR_REDUND;
> +/* CONFIG_ENV_EFI_ADDR_REDUND is supposed to be on sector boundary */
> + end_addr_new = CONFIG_ENV_EFI_ADDR_REDUND
> + + CONFIG_ENV_EFI_SECT_SIZE - 1;
> +#else
> + flash_addr_new = NULL;
> + end_addr_new = 0;
> +#endif /* CONFIG_ENV_EFI_ADDR_REDUND */
> +
> + ret = env_flash_init_params(ctx, env_ptr, flash_addr, end_addr,
> + flash_addr_new, end_addr_new,
> + NULL);
The code above needs some comments.
If one area is the old one and the other the new one, how do you toggle
between the two?
> + if (ret)
> + return -ENOENT;
> +
> + return 0;
> + }
> +#endif
> +#ifdef CONFIG_ENV_EFI_IS_IN_FAT
> + case ENVL_FAT: {
> + ret = env_fat_init_params(ctx,
> + CONFIG_ENV_EFI_FAT_INTERFACE,
> + CONFIG_ENV_EFI_FAT_DEVICE_AND_PART,
> + CONFIG_ENV_EFI_FAT_FILE);
> +
> + return 0;
> + }
> +#endif
> +#ifdef CONFIG_ENV_DRV_NONE
> + case ENVL_NOWHERE:
> +#ifdef CONFIG_ENV_EFI_IS_NOWHERE
> + /* TODO: what we should do */
> +
> + return -ENOENT;
> +#else
> + return -ENOENT;
> +#endif
> +#endif
> + default:
> + return -ENOENT;
> + }
> +}
> +
> +/*
> + * Env context for UEFI variables
> + */
> +U_BOOT_ENV_CONTEXT(efi) = {
> + .name = "efi",
> + .htab = &efi_htab,
> + .env_size = 0x10000, /* TODO: make this configurable */
> + .drv_init = env_drv_init_efi,
> +};
From the name of this context it is unclear that this is for the
non-volatile variables. Please, adjust the comment.
> +
> +static int env_ctx_init_efi_volatile(struct env_context *ctx)
> +{
> + /* Dummy table creation, or hcreate_r()? */
> + if (!himport_r(ctx->htab, NULL, 0, 0, 0, 0, 0, NULL)) {
> + debug("%s: Creating entry tables failed (ret=%d)\n", __func__,
> + errno);
> + return errno;
> + }
> +
> + env_set_ready(ctx);
> +
> + return 0;
> +}
> +
> +U_BOOT_ENV_CONTEXT(efi_volatile) = {
> + .name = "efi_volatile",
> + .htab = &efi_volatile_htab,
> + .env_size = 0,
> + .init = env_ctx_init_efi_volatile,
> +};
> diff --git a/include/env.h b/include/env.h
> index 8045dda9f811..ec241d419a8a 100644
> --- a/include/env.h
> +++ b/include/env.h
> @@ -66,6 +66,9 @@ enum env_redund_flags {
> };
>
> #define ctx_uboot ll_entry_get(struct env_context, uboot, env_contexts)
> +#define ctx_efi ll_entry_get(struct env_context, efi, env_contexts)
> +#define ctx_efi_volatile ll_entry_get(struct env_context, efi_volatile, \
> + env_contexts)
I do not like that ll_entry_get() is called again and again in patch
19/19. Please, call ll_entry_get() once for each context and store the
reference in a static variable. Than you do not need any of these 3 defines.
Best regards
Heinrich
>
> /* Accessor functions */
> void env_set_ready(struct env_context *ctx);
>
More information about the U-Boot
mailing list