From 8d61237edbf6314a701cf78da2c5893a73ff5438 Mon Sep 17 00:00:00 2001 From: "Felix.Vietmeyer@jila.colorado.edu" Date: Tue, 20 Apr 2021 20:04:26 -0600 Subject: [PATCH 1/8] env: Load env when ENV_IS_NOWHERE is only location selected This patch prevent u-boot from hanging on a UltraZed EG board (zynqmp). Without the patch, (drv = env_driver_lookup(ENVOP_INIT, prio)) evaluates to 0, causing prio = 0 Then, (!prio) is hit, returning -ENODEV causing a stall. With the patch, instead of returning -ENODEV and causing a stall, we set gd->env_addr (is this really needed?) and then mark gd->env_valid = ENV_INVALID to use the default env. --- env/env.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/env/env.c b/env/env.c index e4dfb92e154..7168cb9d318 100644 --- a/env/env.c +++ b/env/env.c @@ -322,17 +322,18 @@ int env_init(void) debug("%s: Environment %s init done (ret=%d)\n", __func__, drv->name, ret); - - if (gd->env_valid == ENV_INVALID) - ret = -ENOENT; } - if (!prio) - return -ENODEV; + if (!prio) { + gd->env_addr = (ulong)&default_environment[0]; + gd->env_valid = ENV_INVALID; + + return 0; + } if (ret == -ENOENT) { gd->env_addr = (ulong)&default_environment[0]; - gd->env_valid = ENV_VALID; + gd->env_valid = ENV_INVALID; return 0; } From ec57bd745470fc47b278c21d2eebc95c27c442e5 Mon Sep 17 00:00:00 2001 From: Samuel Dionne-Riel Date: Mon, 20 Dec 2021 18:31:56 -0500 Subject: [PATCH 2/8] cmd: env: Add `indirect` to indirectly set values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows an ergonomic-enough approximation of ${!variable} expansion. This could be used the following way: ``` for target in ${boot_targets}; do env indirect target_name target_name_${target} # ... done ``` Assuming `target_name_mmc0` and similar are set appropriately. A default value can be optionally provided. Note: this acts on environment variables, not hush variables. Signed-off-by: Samuel Dionne-Riel Cc: Simon Glass Cc: "Marek Behún" [trini: Don't enable by default] --- cmd/Kconfig | 3 +++ cmd/nvedit.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/cmd/Kconfig b/cmd/Kconfig index 7bd95466131..c5eb71cea6c 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -518,6 +518,9 @@ config CMD_NVEDIT_EFI If enabled, we are allowed to set/print UEFI variables using "env" command with "-e" option without knowing details. +config CMD_NVEDIT_INDIRECT + bool "env indirect - Sets environment value from another" + config CMD_NVEDIT_INFO bool "env info - print or evaluate environment information" help diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 3bb6e764c08..53e6b57b60e 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -1018,6 +1018,45 @@ sep_err: } #endif +#if defined(CONFIG_CMD_NVEDIT_INDIRECT) +static int do_env_indirect(struct cmd_tbl *cmdtp, int flag, + int argc, char *const argv[]) +{ + char *to = argv[1]; + char *from = argv[2]; + char *default_value = NULL; + int ret = 0; + + if (argc < 3 || argc > 4) { + return CMD_RET_USAGE; + } + + if (argc == 4) { + default_value = argv[3]; + } + + if (env_get(from) == NULL && default_value == NULL) { + printf("## env indirect: Environment variable for (%s) does not exist.\n", from); + + return CMD_RET_FAILURE; + } + + if (env_get(from) == NULL) { + ret = env_set(to, default_value); + } + else { + ret = env_set(to, env_get(from)); + } + + if (ret == 0) { + return CMD_RET_SUCCESS; + } + else { + return CMD_RET_FAILURE; + } +} +#endif + #if defined(CONFIG_CMD_NVEDIT_INFO) /* * print_env_info - print environment information @@ -1181,6 +1220,9 @@ static struct cmd_tbl cmd_env_sub[] = { #if defined(CONFIG_CMD_IMPORTENV) U_BOOT_CMD_MKENT(import, 5, 0, do_env_import, "", ""), #endif +#if defined(CONFIG_CMD_NVEDIT_INDIRECT) + U_BOOT_CMD_MKENT(indirect, 3, 0, do_env_indirect, "", ""), +#endif #if defined(CONFIG_CMD_NVEDIT_INFO) U_BOOT_CMD_MKENT(info, 3, 0, do_env_info, "", ""), #endif @@ -1265,6 +1307,9 @@ static char env_help_text[] = #if defined(CONFIG_CMD_IMPORTENV) "env import [-d] [-t [-r] | -b | -c] addr [size] [var ...] - import environment\n" #endif +#if defined(CONFIG_CMD_NVEDIT_INDIRECT) + "env indirect [default] - sets to the value of , using [default] when unset\n" +#endif #if defined(CONFIG_CMD_NVEDIT_INFO) "env info - display environment information\n" "env info [-d] [-p] [-q] - evaluate environment information\n" From eb68ead2d3f093dfeffae68045e8921d93ff05cf Mon Sep 17 00:00:00 2001 From: He Yong Date: Fri, 18 Feb 2022 00:07:25 +0800 Subject: [PATCH 3/8] env: fat: Allow overriding interface, device and partition For platform which can boot on different device, this allows to override interface, device and partition from board code Signed-off-by: He Yong --- env/fat.c | 34 +++++++++++++++++++--------------- include/env_internal.h | 20 ++++++++++++++++++++ 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/env/fat.c b/env/fat.c index fdccd6cd2a8..6251d9649b1 100644 --- a/env/fat.c +++ b/env/fat.c @@ -32,7 +32,12 @@ DECLARE_GLOBAL_DATA_PTR; -static char *env_fat_device_and_part(void) +__weak const char *env_fat_get_intf(void) +{ + return (const char *)CONFIG_ENV_FAT_INTERFACE; +} + +__weak char *env_fat_get_dev_part(void) { #ifdef CONFIG_MMC static char *part_str; @@ -60,14 +65,15 @@ static int env_fat_save(void) int dev, part; int err; loff_t size; + const char *ifname = env_fat_get_intf(); + const char *dev_and_part = env_fat_get_dev_part(); err = env_export(&env_new); if (err) return err; - part = blk_get_device_part_str(CONFIG_ENV_FAT_INTERFACE, - env_fat_device_and_part(), - &dev_desc, &info, 1); + part = blk_get_device_part_str(ifname, dev_and_part, + &dev_desc, &info, 1); if (part < 0) return 1; @@ -77,8 +83,7 @@ static int env_fat_save(void) * 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); + printf("Unable to use %s %d:%d...\n", ifname, dev, part); return 1; } @@ -93,8 +98,7 @@ static int env_fat_save(void) * 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... \n", - file, CONFIG_ENV_FAT_INTERFACE, dev, part); + printf("Unable to write \"%s\" from %s%d:%d...\n", file, ifname, dev, part); return 1; } @@ -117,15 +121,16 @@ static int env_fat_load(void) struct disk_partition info; int dev, part; int err1; + const char *ifname = env_fat_get_intf(); + const char *dev_and_part = env_fat_get_dev_part(); #ifdef CONFIG_MMC - if (!strcmp(CONFIG_ENV_FAT_INTERFACE, "mmc")) + if (!strcmp(ifname, "mmc")) mmc_initialize(NULL); #endif - part = blk_get_device_part_str(CONFIG_ENV_FAT_INTERFACE, - env_fat_device_and_part(), - &dev_desc, &info, 1); + part = blk_get_device_part_str(ifname, dev_and_part, + &dev_desc, &info, 1); if (part < 0) goto err_env_relocate; @@ -135,8 +140,7 @@ static int env_fat_load(void) * 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); + printf("Unable to use %s %d:%d...\n", ifname, dev, part); goto err_env_relocate; } @@ -154,7 +158,7 @@ static int env_fat_load(void) * will calling it. The missing \n is intentional. */ printf("Unable to read \"%s\" from %s%d:%d... \n", - CONFIG_ENV_FAT_FILE, CONFIG_ENV_FAT_INTERFACE, dev, part); + CONFIG_ENV_FAT_FILE, ifname, dev, part); goto err_env_relocate; } diff --git a/include/env_internal.h b/include/env_internal.h index 07c227ecc03..b704c033631 100644 --- a/include/env_internal.h +++ b/include/env_internal.h @@ -245,6 +245,26 @@ const char *env_ext4_get_dev_part(void); * Return: an enum env_location value on success, or -ve error code. */ enum env_location env_get_location(enum env_operation op, int prio); + +/** + * env_fat_get_intf() - Provide the interface for env in FAT + * + * It is a weak function allowing board to overidde the default interface for + * U-Boot env in FAT: CONFIG_ENV_FAT_INTERFACE + * + * Return: string of interface, empty if not supported + */ +const char *env_fat_get_intf(void); + +/** + * env_fat_get_dev_part() - Provide the device and partition for env in FAT + * + * It is a weak function allowing board to overidde the default device and + * partition used for U-Boot env in FAT: CONFIG_ENV_FAT_DEVICE_AND_PART + * + * Return: string of device and partition + */ +char *env_fat_get_dev_part(void); #endif /* DO_DEPS_ONLY */ #endif /* _ENV_INTERNAL_H_ */ From 7d79cba28ca9bb02ba589eca2e80ce837d7d0ca8 Mon Sep 17 00:00:00 2001 From: Christoph Niedermaier Date: Wed, 23 Feb 2022 10:33:36 +0100 Subject: [PATCH 4/8] scripts/get_default_envs.sh: Remove blank lines After sorting, unnoticed blank lines at the end move to the top. Avoid this by removing it before sorting. Signed-off-by: Christoph Niedermaier Cc: Lukasz Majewski Cc: Rasmus Villemoes Cc: Tom Rini To: u-boot@lists.denx.de --- scripts/get_default_envs.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/get_default_envs.sh b/scripts/get_default_envs.sh index 3c6fdc45e11..fedf7206fe1 100755 --- a/scripts/get_default_envs.sh +++ b/scripts/get_default_envs.sh @@ -35,8 +35,8 @@ cp ${env_obj_file_path} ${ENV_OBJ_FILE_COPY} ${OBJCOPY} --dump-section .rodata.default_environment=${ENV_OBJ_FILE_COPY} \ ${env_obj_file_path} -# Replace default '\0' with '\n' and sort entries -tr '\0' '\n' < ${ENV_OBJ_FILE_COPY} | sort --field-separator== -k1,1 --stable +# Replace default '\0' with '\n' , remove blank lines and sort entries +tr '\0' '\n' < ${ENV_OBJ_FILE_COPY} | sed -e '/^\s*$/d' | sort --field-separator== -k1,1 --stable rm ${ENV_OBJ_FILE_COPY} From cc5a490cf46565d3a42f86beaac05f56f4f40741 Mon Sep 17 00:00:00 2001 From: Christoph Niedermaier Date: Tue, 1 Mar 2022 09:38:51 +0100 Subject: [PATCH 5/8] Makefile: Sort u-boot-initial-env output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will sort the default environment file generated by "make u-boot-initial-env", but won't sort the default environment in the compiled u-boot binary. The file u-boot-initial-env is considered to use for the userpace environment access tools [1] in case of that the environments is written the first time into its location. This is done on the one hand for a better overview and comparison of the generated environment file. On the other hand it is to synchronize the output with the script get_default_env.sh, which generated a sorted default environment file. The sorting preserves the order of equal variable names by sorting only the variable name, and disable the last-resort comparison. After sorting, unnoticed blank lines at the end move to the top. Avoid that by removing it before sorting. [1] https://github.com/sbabic/libubootenv Signed-off-by: Christoph Niedermaier Reviewed-by: Stefano Babic Tested-by: Stefano Babic Cc: Stefano Babic Cc: Simon Glass Cc: Marek Behún To: u-boot@lists.denx.de Reviewed-by: Simon Glass --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 4b347d3603a..9b25536c301 100644 --- a/Makefile +++ b/Makefile @@ -2459,7 +2459,8 @@ endif quiet_cmd_genenv = GENENV $@ cmd_genenv = $(OBJCOPY) --dump-section .rodata.default_environment=$@ env/common.o; \ - sed --in-place -e 's/\x00/\x0A/g' $@ + sed --in-place -e 's/\x00/\x0A/g' $@; sed --in-place -e '/^\s*$$/d' $@; \ + sort --field-separator== -k1,1 --stable $@ -o $@ u-boot-initial-env: u-boot.bin $(call if_changed,genenv) From 9d4e88c02163e1f1824afa3ff142ec7a116fbe5c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 11 Mar 2022 16:22:38 -0700 Subject: [PATCH 6/8] env: Move the doc comment to the code This doesn't really make much sense in the documentation. Add a code comment instead. Suggested-by: Heinrich Schuchardt Signed-off-by: Simon Glass --- common/autoboot.c | 5 +++++ doc/usage/environment.rst | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/common/autoboot.c b/common/autoboot.c index b8861d56218..63f2587941d 100644 --- a/common/autoboot.c +++ b/common/autoboot.c @@ -446,6 +446,11 @@ const char *bootdelay_process(void) s = env_get("bootdelay"); bootdelay = s ? (int)simple_strtol(s, NULL, 10) : CONFIG_BOOTDELAY; + /* + * Does it really make sense that the devicetree overrides the user + * setting? It is possibly helpful for security since the device tree + * may be signed whereas the environment is often loaded from storage. + */ if (IS_ENABLED(CONFIG_OF_CONTROL)) bootdelay = ofnode_conf_read_int("bootdelay", bootdelay); diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst index d295cc89878..4b228c1de3c 100644 --- a/doc/usage/environment.rst +++ b/doc/usage/environment.rst @@ -120,7 +120,6 @@ bootdelay The default value is defined by CONFIG_BOOTDELAY. The value of 'bootdelay' is overridden by the /config/bootdelay value in the device-tree if CONFIG_OF_CONTROL=y. - Does it really make sense that the devicetree overrides the user setting? bootcmd The command that is run if the user does not enter the shell during the From 754a722d1b9572aad7900ad2a8e58f2f8a7cd2cc Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 11 Mar 2022 16:22:39 -0700 Subject: [PATCH 7/8] env: Drop the unncessary protocol mention in autoload Drop this text at the end since it already mentions bootp and dhcp. Signed-off-by: Simon Glass Suggested-by: Heinrich Schuchardt --- doc/usage/environment.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst index 4b228c1de3c..80550fc4478 100644 --- a/doc/usage/environment.rst +++ b/doc/usage/environment.rst @@ -170,7 +170,7 @@ autoload if set to "no" (any string beginning with 'n'), "bootp" and "dhcp" will just load perform a lookup of the configuration from the BOOTP server, but not try to - load any image using TFTP or DHCP. + load any image. autostart if set to "yes", an image loaded using the "bootp", "dhcp", From 6910dbe3413e684bff9a194945df60345ecbc623 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 12 Mar 2022 22:47:49 -0700 Subject: [PATCH 8/8] env: Allow text-env tests to run with awk At present the tests assume that gawk is being used. Adjust the tests so that the names are inserted in alphabetical order, so that awk is happy. Also use PROCINFO to make gawk output in alphabetical order. This is not ideal, since it changes the env-car ordering from what the user provided, but it may be acceptable. Signed-off-by: Simon Glass Reported-by: Patrick Delaunay Fixes: https://source.denx.de/u-boot/u-boot/-/issues/10 --- scripts/env2string.awk | 5 ++++- test/py/tests/test_env.py | 28 ++++++++++++++-------------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/scripts/env2string.awk b/scripts/env2string.awk index 1bfe9ed07a4..de470a49941 100644 --- a/scripts/env2string.awk +++ b/scripts/env2string.awk @@ -81,7 +81,10 @@ END { if (do_output) { printf("%s", "#define CONFIG_EXTRA_ENV_TEXT \"") - # Print out all the variables + # Print out all the variables by alphabetic order, if using + # gawk. This allows test_env_test.py to work on both awk (where + # this next line does nothing) + PROCINFO["sorted_in"] = "@ind_str_asc" for (var in vars) { env = vars[var] print var "=" vars[var] "\\0" diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py index b2f3470de94..6d08565f0b5 100644 --- a/test/py/tests/test_env.py +++ b/test/py/tests/test_env.py @@ -554,42 +554,42 @@ def test_env_text(u_boot_console): # two vars check_script('''fred=123 -ernie=456''', 'fred=123\\0ernie=456\\0') +mary=456''', 'fred=123\\0mary=456\\0') # blank lines check_script('''fred=123 -ernie=456 +mary=456 -''', 'fred=123\\0ernie=456\\0') +''', 'fred=123\\0mary=456\\0') # append check_script('''fred=123 -ernie=456 -fred+= 456''', 'fred=123 456\\0ernie=456\\0') +mary=456 +fred+= 456''', 'fred=123 456\\0mary=456\\0') # append from empty check_script('''fred= -ernie=456 -fred+= 456''', 'fred= 456\\0ernie=456\\0') +mary=456 +fred+= 456''', 'fred= 456\\0mary=456\\0') # variable with + in it - check_script('fred+ernie=123', 'fred+ernie=123\\0') + check_script('fred+mary=123', 'fred+mary=123\\0') # ignores variables that are empty check_script('''fred= fred+= -ernie=456''', 'ernie=456\\0') +mary=456''', 'mary=456\\0') # single-character env name - check_script('''f=123 + check_script('''m=123 e=456 -f+= 456''', 'e=456\\0f=123 456\\0') +m+= 456''', 'e=456\\0m=123 456\\0') # contains quotes check_script('''fred="my var" -ernie=another"''', 'fred=\\"my var\\"\\0ernie=another\\"\\0') +mary=another"''', 'fred=\\"my var\\"\\0mary=another\\"\\0') # variable name ending in + check_script('''fred\\+=my var @@ -598,7 +598,7 @@ fred++= again''', 'fred+=my var again\\0') # variable name containing + check_script('''fred+jane=both fred+jane+=again -ernie=456''', 'fred+jane=bothagain\\0ernie=456\\0') +mary=456''', 'fred+jane=bothagain\\0mary=456\\0') # multi-line vars - new vars always start at column 1 check_script('''fred=first @@ -607,7 +607,7 @@ ernie=456''', 'fred+jane=bothagain\\0ernie=456\\0') after blank confusing=oops -ernie=another"''', 'fred=first second third with tab after blank confusing=oops\\0ernie=another\\"\\0') +mary=another"''', 'fred=first second third with tab after blank confusing=oops\\0mary=another\\"\\0') # real-world example check_script('''ubifs_boot=