| Message ID | 20190905082133.18996-19-takahiro.akashi@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: non-volatile variables support | expand |
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@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@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); >
On Thu, Sep 05, 2019 at 09:37:37PM +0200, Heinrich Schuchardt wrote: > 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@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. The same comment as uboot context. > >+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? I have not changed this 'help' text from the original env/Kconfig. So I don't know whether or not you are right. > >+ 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. Okay. > >+ 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@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? Again, I just mimicked the original init code in env/flash.c. AFAIK, this is not old-or-new stuff, but *redundant* or original-and-copy storages for robustness. The logic exists in env/flash.c. > >+ 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. Okay. > >+ > >+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. Okay. Thank you for your review, -Takahiro Akashi > Best regards > > Heinrich > > > > > /* Accessor functions */ > > void env_set_ready(struct env_context *ctx); > > >
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" + +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 + 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 + 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@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 */ + 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); + 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, +}; + +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) /* Accessor functions */ void env_set_ready(struct env_context *ctx);
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@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