[U-Boot] [PATCH v3 1/7] env: save UEFI non-volatile variables in dedicated storage
Heinrich Schuchardt
xypron.glpk at gmx.de
Tue Jun 4 21:09:56 UTC 2019
On 6/4/19 8:52 AM, AKASHI Takahiro wrote:
> We need a variant of env_save()/env_load() to handle dedicated storage
> for UEFI variables.
> It is assumed that env_efi_load() will be called only ince at init
> and that env_efi_save() will be called at every SetVariable.
>
> In this patch, new parameters will be expected to be configured:
> CONFIG_ENV_EFI_FAT_DEVICE_AND_PART
> CONFIG_ENV_EFI_FAT_FILE
> in case of CONFIG_ENV_IS_IN_FAT.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
> env/Kconfig | 39 ++++++++
> env/env.c | 155 +++++++++++++++++++++++++++++-
> env/fat.c | 105 ++++++++++++++++++++
> include/asm-generic/global_data.h | 3 +
> include/environment.h | 31 ++++++
> 5 files changed, 332 insertions(+), 1 deletion(-)
>
> diff --git a/env/Kconfig b/env/Kconfig
> index 1e10c7a4c46b..a36b6ee48f10 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -399,6 +399,10 @@ config ENV_IS_IN_UBI
> the environment in. This will enable redundant environments in UBI.
> It is assumed that both volumes are in the same MTD partition.
>
Our store will be completely unrelated to U-Boot environment variables.
So why should we put anything into this Kconfig?
> +config ENV_EFI
> + bool
> + default n
> +
> config ENV_FAT_INTERFACE
> string "Name of the block device for the environment"
> depends on ENV_IS_IN_FAT
> @@ -438,6 +442,34 @@ config ENV_FAT_FILE
> It's a string of the FAT file name. This file use to store the
> environment.
>
> +config ENV_EFI_FAT_DEVICE_AND_PART
> + string "Device and partition for where to store the UEFI non-volatile variables in FAT"
> + depends on ENV_IS_IN_FAT
> + depends on ENV_EFI
> + help
> + Define this to a string to specify the partition of the device. It can
> + be as following:
The following information is not specific to this variable. So we can
cut it short:
"String defining the device number and the partition number in the
format <device number>:<partition number>, e.g. 0:1."
Where do you specify on which subsystem (mmc, scsi, sata, nvme) the file
is stored?
I would prefer nopt to have a restriction to FAT. If the EXT4 driver is
enabled writing and reading to an ext4 valume should work as well. When
calling fs_set_blk_dev() you will not specify any file system.
> +
> + "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 UEFI non-volatile variables"
%s/UEFI non-volatile variables/non-volatile UEFI variables/
No need for the file system being FAT.
> + depends on ENV_IS_IN_FAT
> + depends on ENV_EFI
> + default "uboot_efi.env"
> + help
> + It's a string of the FAT file name. This file use to store the
It is obvious that afile name is a string. But why restrict to a file
name? This could also be a path to a non-root file:
"Path of the file used to store non-volatile UEFI variables."
But can't we simply have a single variable, where we put all parts of
the definition, e.g.
mmc 0:1 /EFI/nv-var.store
> + UEFI non-volatile variables.
> +
> config ENV_EXT4_INTERFACE
> string "Name of the block device for the environment"
> depends on ENV_IS_IN_EXT4
> @@ -470,6 +502,13 @@ config ENV_EXT4_FILE
> It's a string of the EXT4 file name. This file use to store the
> environment (explicit path to the file)
>
> +config ENV_EFI_SIZE
> + hex "UEFI Variables Environment Size"
> + depends on ENV_EFI
> + default 0x20000
If we are writing to a file, the available space on the partition and in
RAM is the limit. I see no need for this variable in this case.
> + help
> + Size of the UEFI variables storage area
> +
> if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP || ARCH_VERSAL || ARC || ARCH_STM32MP
>
> config ENV_OFFSET
> diff --git a/env/env.c b/env/env.c
> index 4b417b90a291..202079f6d4c0 100644
> --- a/env/env.c
> +++ b/env/env.c
We are not creating a driver for U-Boot environment variables. So I
would keep away from the code in this directory. You can put the load
and save function into lib/efi_loader/ and use the normal file-system
functions there to read and write the file.
Essentially I expect we will end up with a class of drivers which we can
use as backends for nv variables, e.g.
* file storage
* OP-TEE module
* U-Boot variables
* none
Best regards
Heinrich
> @@ -24,6 +24,12 @@ void env_fix_drivers(void)
> entry->load += gd->reloc_off;
> if (entry->save)
> entry->save += gd->reloc_off;
> +#ifdef CONFIG_ENV_EFI
> + if (entry->efi_load)
> + entry->efi_load += gd->reloc_off;
> + if (entry->efi_save)
> + entry->efi_save += gd->reloc_off;
> +#endif
> if (entry->init)
> entry->init += gd->reloc_off;
> }
> @@ -125,7 +131,8 @@ __weak enum env_location env_get_location(enum env_operation op, int prio)
> if (prio >= ARRAY_SIZE(env_locations))
> return ENVL_UNKNOWN;
>
> - gd->env_load_prio = prio;
> + if (op != ENVOP_EFI)
> + gd->env_load_prio = prio;
>
> return env_locations[prio];
> }
> @@ -280,3 +287,149 @@ int env_init(void)
>
> return ret;
> }
> +
> +#ifdef CONFIG_ENV_EFI
> +struct hsearch_data efi_var_htab;
> +struct hsearch_data efi_nv_var_htab;
> +
> +int env_efi_import(const char *buf, int check)
> +{
> + env_t *ep = (env_t *)buf;
> +
> + if (check) {
> + u32 crc;
> +
> + memcpy(&crc, &ep->crc, sizeof(crc));
> +
> + if (crc32(0, ep->data, CONFIG_ENV_EFI_SIZE - ENV_HEADER_SIZE)
> + != crc) {
> + pr_err("bad CRC of UEFI variables\n");
> + return -ENOMSG; /* needed for env_load() */
> + }
> + }
> +
> + if (himport_r(&efi_nv_var_htab, (char *)ep->data,
> + CONFIG_ENV_EFI_SIZE - ENV_HEADER_SIZE,
> + '\0', 0, 0, 0, NULL))
> + return 0;
> +
> + pr_err("Cannot import environment: errno = %d\n", errno);
> +
> + /* set_default_env("import failed", 0); */
> +
> + return -EIO;
> +}
> +
> +int env_efi_export(env_t *env_out)
> +{
> + char *res;
> + ssize_t len;
> +
> + res = (char *)env_out->data;
> + len = hexport_r(&efi_nv_var_htab, '\0', 0, &res,
> + CONFIG_ENV_EFI_SIZE - ENV_HEADER_SIZE,
> + 0, NULL);
> + if (len < 0) {
> + pr_err("Cannot export environment: errno = %d\n", errno);
> + return 1;
> + }
> +
> + env_out->crc = crc32(0, env_out->data,
> + CONFIG_ENV_EFI_SIZE - ENV_HEADER_SIZE);
> +
> + return 0;
> +}
> +
> +int env_efi_save(void)
> +{
> +#ifdef CONFIG_ENV_IS_NOWHERE
> + return 0;
> +#else
> + struct env_driver *drv = NULL;
> + int ret;
> +
> + if (!efi_nv_var_htab.table)
> + return 0;
> +
> + if (gd->env_efi_prio == -1) {
> + pr_warn("No UEFI non-volatile variable storage\n");
> + return -1;
> + }
> +
> + drv = _env_driver_lookup(env_get_location(ENVOP_EFI, gd->env_efi_prio));
> + if (!drv) {
> + pr_warn("No UEFI non-volatile variable storage\n");
> + return -1;
> + }
> +
> + ret = drv->efi_save();
> + if (ret)
> + pr_err("Saving UEFI non-volatile variable failed\n");
> +
> + return ret;
> +#endif
> +}
> +
> +/* This function should be called only once at init */
> +int env_efi_load(void)
> +{
> +#ifndef CONFIG_ENV_IS_NOWHERE
> + struct env_driver *drv;
> + int prio;
> + enum env_location loc;
> +#endif
> + int ret;
> +
> + /* volatile variables */
> + if (!efi_var_htab.table) {
> + ret = himport_r(&efi_var_htab, NULL, 0, '\0', 0, 0, 0, NULL);
> + if (!ret) {
> + pr_err("Creating UEFI volatile variables failed\n");
> + return -1;
> + }
> + }
> +
> +#ifndef CONFIG_ENV_IS_NOWHERE
> + gd->env_efi_prio = -1;
> +
> + /* non-volatile variables */
> + if (efi_nv_var_htab.table)
> + return 0;
> +
> + for (drv = NULL, prio = 0; prio < ARRAY_SIZE(env_locations); prio++) {
> + loc = env_get_location(ENVOP_EFI, prio);
> + drv = _env_driver_lookup(loc);
> + if (!drv)
> + continue;
> +
> + if (drv->efi_load && drv->efi_save)
> + break;
> + }
> + if (!drv || prio == ARRAY_SIZE(env_locations)) {
> + pr_warn("No UEFI non-volatile variable storage\n");
> + goto skip_load;
> + }
> +
> + gd->env_efi_prio = prio;
> +
> + ret = drv->efi_load();
> + if (ret) {
> + pr_err("Loading UEFI non-volatile variables failed\n");
> + return -1;
> + }
> +skip_load:
> +#endif /* CONFIG_ENV_IS_NOWHERE */
> +
> + if (!efi_nv_var_htab.table) {
> + ret = himport_r(&efi_nv_var_htab, NULL, 0, '\0', 0, 0, 0, NULL);
> + if (!ret) {
> + pr_err("Creating UEFI non-volatile variables failed\n");
> + return -1;
> + }
> +
> + return 0;
> + }
> +
> + return 0;
> +}
> +#endif /* CONFIG_ENV_EFI */
> diff --git a/env/fat.c b/env/fat.c
> index 7f74c64dfe7e..7cd6dc51baea 100644
> --- a/env/fat.c
> +++ b/env/fat.c
> @@ -14,6 +14,7 @@
> #include <malloc.h>
> #include <memalign.h>
> #include <search.h>
> +#include <efi_loader.h>
> #include <errno.h>
> #include <fat.h>
> #include <mmc.h>
> @@ -128,6 +129,106 @@ err_env_relocate:
> }
> #endif /* LOADENV */
>
> +#ifdef CONFIG_ENV_EFI
> +static int env_fat_efi_save(void)
> +{
> + env_t __aligned(ARCH_DMA_MINALIGN) env_new;
> + struct blk_desc *dev_desc = NULL;
> + disk_partition_t info;
> + int dev, part;
> + int err;
> + loff_t size;
> +
> + err = env_efi_export(&env_new);
> + if (err)
> + return err;
> +
> + part = blk_get_device_part_str(CONFIG_ENV_FAT_INTERFACE,
> + CONFIG_ENV_EFI_FAT_DEVICE_AND_PART,
> + &dev_desc, &info, 1);
> + if (part < 0)
> + return 1;
> +
> + dev = dev_desc->devnum;
> + if (fat_set_blk_dev(dev_desc, &info) != 0) {
> + /*
> + * This printf is embedded in the messages from env_save that
> + * will calling it. The missing \n is intentional.
> + */
> + printf("Unable to use %s %d:%d... ",
> + CONFIG_ENV_FAT_INTERFACE, dev, part);
> + return 1;
> + }
> +
> + err = file_fat_write(CONFIG_ENV_EFI_FAT_FILE,
> + (void *)&env_new, 0, sizeof(env_t), &size);
> + if (err < 0) {
> + /*
> + * This printf is embedded in the messages from env_save that
> + * will calling it. The missing \n is intentional.
> + */
> + printf("Unable to write \"%s\" from %s%d:%d... ",
> + CONFIG_ENV_EFI_FAT_FILE, CONFIG_ENV_FAT_INTERFACE,
> + dev, part);
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +static int env_fat_efi_load(void)
> +{
> + ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_EFI_SIZE);
> + struct blk_desc *dev_desc = NULL;
> + disk_partition_t info;
> + int dev, part;
> + int err;
> +
> +#ifdef CONFIG_MMC
> + if (!strcmp(CONFIG_ENV_FAT_INTERFACE, "mmc"))
> + mmc_initialize(NULL);
> +#endif
> +
> + part = blk_get_device_part_str(CONFIG_ENV_FAT_INTERFACE,
> + CONFIG_ENV_EFI_FAT_DEVICE_AND_PART,
> + &dev_desc, &info, 1);
> + if (part < 0)
> + goto err_env_relocate;
> +
> + dev = dev_desc->devnum;
> + if (fat_set_blk_dev(dev_desc, &info) != 0) {
> + /*
> + * This printf is embedded in the messages from env_save that
> + * will calling it. The missing \n is intentional.
> + */
> + printf("Unable to use %s %d:%d...\n",
> + CONFIG_ENV_FAT_INTERFACE, dev, part);
> + goto err_env_relocate;
> + }
> +
> + err = file_fat_read(CONFIG_ENV_EFI_FAT_FILE, buf, CONFIG_ENV_EFI_SIZE);
> + if (err <= 0 && (err != -ENOENT)) {
> + /*
> + * This printf is embedded in the messages from env_save that
> + * will calling it. The missing \n is intentional.
> + */
> + printf("Unable to read \"%s\" from %s %d:%d...\n",
> + CONFIG_ENV_EFI_FAT_FILE, CONFIG_ENV_FAT_INTERFACE,
> + dev, part);
> + goto err_env_relocate;
> + }
> +
> + if (err > 0)
> + return env_efi_import(buf, 1);
> + else
> + return 0;
> +
> +err_env_relocate:
> +
> + return -EIO;
> +}
> +#endif /* CONFIG_ENV_EFI */
> +
> U_BOOT_ENV_LOCATION(fat) = {
> .location = ENVL_FAT,
> ENV_NAME("FAT")
> @@ -137,4 +238,8 @@ U_BOOT_ENV_LOCATION(fat) = {
> #ifdef CMD_SAVEENV
> .save = env_save_ptr(env_fat_save),
> #endif
> +#ifdef CONFIG_ENV_EFI
> + .efi_load = env_fat_efi_load,
> + .efi_save = env_fat_efi_save,
> +#endif
> };
> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> index 02a3ed683821..5ed390cc31c7 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -52,6 +52,9 @@ typedef struct global_data {
> unsigned long env_valid; /* Environment valid? enum env_valid */
> unsigned long env_has_init; /* Bitmask of boolean of struct env_location offsets */
> int env_load_prio; /* Priority of the loaded environment */
> +#ifdef CONFIG_ENV_EFI
> + int env_efi_prio; /* Priority of the UEFI variables */
> +#endif
>
> unsigned long ram_base; /* Base address of RAM used by U-Boot */
> unsigned long ram_top; /* Top address of RAM used by U-Boot */
> diff --git a/include/environment.h b/include/environment.h
> index cd966761416e..32a03014690f 100644
> --- a/include/environment.h
> +++ b/include/environment.h
> @@ -200,6 +200,7 @@ enum env_operation {
> ENVOP_INIT, /* we want to call the init function */
> ENVOP_LOAD, /* we want to call the load function */
> ENVOP_SAVE, /* we want to call the save function */
> + ENVOP_EFI, /* we want to call the efi_load/save function */
> };
>
> struct env_driver {
> @@ -225,6 +226,26 @@ struct env_driver {
> */
> int (*save)(void);
>
> +#ifdef CONFIG_ENV_EFI
> + /**
> + * efi_load() - Load UEFI non-volatile variables from storage
> + *
> + * This method is required for UEFI non-volatile variables
> + *
> + * @return 0 if OK, -ve on error
> + */
> + int (*efi_load)(void);
> +
> + /**
> + * efi_save() - Save UEFI non-volatile variables to storage
> + *
> + * This method is required for UEFI non-volatile variables
> + *
> + * @return 0 if OK, -ve on error
> + */
> + int (*efi_save)(void);
> +#endif
> +
> /**
> * init() - Set up the initial pre-relocation environment
> *
> @@ -312,6 +333,16 @@ void eth_parse_enetaddr(const char *addr, uint8_t *enetaddr);
> int eth_env_get_enetaddr(const char *name, uint8_t *enetaddr);
> int eth_env_set_enetaddr(const char *name, const uint8_t *enetaddr);
>
> +#ifdef CONFIG_ENV_EFI
> +extern struct hsearch_data efi_var_htab;
> +extern struct hsearch_data efi_nv_var_htab;
> +
> +int env_efi_import(const char *buf, int check);
> +int env_efi_export(env_t *env_out);
> +int env_efi_load(void);
> +int env_efi_save(void);
> +#endif
> +
> #endif /* DO_DEPS_ONLY */
>
> #endif /* _ENVIRONMENT_H_ */
>
More information about the U-Boot
mailing list