From 87438b5e14b5737ad7544b2ca24192c692e000ef Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Fri, 24 Jul 2020 20:55:37 +0200 Subject: [PATCH 01/23] test: do not rely on => being the prompt In our tests we should use the customized prompt for testing. Reported-by: Tom Rini Signed-off-by: Heinrich Schuchardt Tested-by: Tom Rini --- test/py/tests/test_efi_loader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/py/tests/test_efi_loader.py b/test/py/tests/test_efi_loader.py index ca68626cec..fc8d6b8655 100644 --- a/test/py/tests/test_efi_loader.py +++ b/test/py/tests/test_efi_loader.py @@ -199,6 +199,6 @@ def test_efi_grub_net(u_boot_console): # Then exit cleanly u_boot_console.wait_for('grub>') u_boot_console.run_command('exit', wait_for_prompt=False, wait_for_echo=False) - u_boot_console.wait_for('=>') + u_boot_console.wait_for(u_boot_console.prompt) # And give us our U-Boot prompt back u_boot_console.run_command('') From 8a5cdf601f8da6704c9146a253c1bcbfcd0e6286 Mon Sep 17 00:00:00 2001 From: Tom Rini Date: Fri, 31 Jul 2020 09:45:02 -0400 Subject: [PATCH 02/23] test: efi_selftest: Do not force serial# setting As part of the EFI self test we set and check the serial# variable. However, we should not be forcing this setting. In the case where we are allowed to change the variable it will change, and we will pass the test. In the case where we cannot change it, force may or may not be allowed, depending on further environment restrictions. Drop the -f flag here as we do not need it. Cc: Heinrich Schuchardt Signed-off-by: Tom Rini --- test/py/tests/test_efi_selftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/py/tests/test_efi_selftest.py b/test/py/tests/test_efi_selftest.py index 971c9f6053..9b520c2070 100644 --- a/test/py/tests/test_efi_selftest.py +++ b/test/py/tests/test_efi_selftest.py @@ -36,7 +36,7 @@ def test_efi_selftest_device_tree(u_boot_console): output = u_boot_console.run_command('bootefi selftest') assert '\'device tree\'' in output u_boot_console.run_command(cmd='setenv efi_selftest device tree') - u_boot_console.run_command(cmd='setenv -f serial# Testing DT') + u_boot_console.run_command(cmd='setenv serial# Testing DT') u_boot_console.run_command(cmd='bootefi selftest ${fdtcontroladdr}', wait_for_prompt=False) m = u_boot_console.p.expect(['serial-number: Testing DT', 'U-Boot']) if m != 0: From 879369ea92b97639c69b70829dbbce6d1aabb14c Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Mon, 15 Jun 2020 16:52:17 +0200 Subject: [PATCH 03/23] env: add prototypes for weak function This patch adds prototypes for several weak functions: - env_ext4_get_intf - env_ext4_get_dev_part - env_get_location It solves the following warnings when compiling with W=1 on stm32mp1 board: board/st/stm32mp1/stm32mp1.c:849:19: warning: no previous prototype for 'env_get_location' [-Wmissing-prototypes] enum env_location env_get_location(enum env_operation op, int prio) ^~~~~~~~~~~~~~~~ board/st/stm32mp1/stm32mp1.c:876:13: warning: no previous prototype for 'env_ext4_get_intf' [-Wmissing-prototypes] const char *env_ext4_get_intf(void) ^~~~~~~~~~~~~~~~~ board/st/stm32mp1/stm32mp1.c:889:13: warning: no previous prototype for 'env_ext4_get_dev_part' [-Wmissing-prototypes] const char *env_ext4_get_dev_part(void) ^~~~~~~~~~~~~~~~~~~~~ Signed-off-by: Patrice Chotard Signed-off-by: Patrick Delaunay --- include/env_internal.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/include/env_internal.h b/include/env_internal.h index 66550434c3..b9459f926c 100644 --- a/include/env_internal.h +++ b/include/env_internal.h @@ -211,6 +211,26 @@ struct env_driver { extern struct hsearch_data env_htab; +/** + * env_ext4_get_intf() - Provide the interface for env in EXT4 + * + * It is a weak function allowing board to overidde the default interface for + * U-Boot env in EXT4: CONFIG_ENV_EXT4_INTERFACE + * + * @return string of interface, empty if not supported + */ +const char *env_ext4_get_intf(void); + +/** + * env_ext4_get_dev_part() - Provide the device and partition for env in EXT4 + * + * It is a weak function allowing board to overidde the default device and + * partition used for U-Boot env in EXT4: CONFIG_ENV_EXT4_DEVICE_AND_PART + * + * @return string of device and partition + */ +const char *env_ext4_get_dev_part(void); + /** * env_get_location()- Provide the best location for the U-Boot environment * From 87dac740122304dd196d090ab38edd1299130880 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:14 +0200 Subject: [PATCH 04/23] env: add absolute path at CONFIG_ENV_EXT4_FILE Add the absolute path to the default value of CONFIG_ENV_EXT4_FILE = "/uboot.env". This patch avoid the error : Saving Environment to EXT4... File System is consistent Please supply Absolute path Reviewed-by: Tom Rini Signed-off-by: Patrick Delaunay --- env/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/env/Kconfig b/env/Kconfig index 4113628f49..dcc525d4ed 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -479,7 +479,7 @@ config ENV_EXT4_DEVICE_AND_PART config ENV_EXT4_FILE string "Name of the EXT4 file to use for the environment" depends on ENV_IS_IN_EXT4 - default "uboot.env" + default "/uboot.env" help It's a string of the EXT4 file name. This file use to store the environment (explicit path to the file) From 286fee5062ed31bd0373d3fc43ebe22d85bca4ae Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:15 +0200 Subject: [PATCH 05/23] env: ext4: set gd->env_valid Add a missing initialization of gd->env_valid in env_ext4_load as it is already done in some other env device. Set gd->env_valid = ENV_VALID in env_ext4_save() and env_ext4_load(). This patch allows to have a correct information in 'env info' command. Reviewed-by: Tom Rini Signed-off-by: Patrick Delaunay --- env/ext4.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/env/ext4.c b/env/ext4.c index 8e90bb71b7..ac9f126bec 100644 --- a/env/ext4.c +++ b/env/ext4.c @@ -32,6 +32,8 @@ #include #include +DECLARE_GLOBAL_DATA_PTR; + __weak const char *env_ext4_get_intf(void) { return (const char *)CONFIG_ENV_EXT4_INTERFACE; @@ -79,6 +81,7 @@ static int env_ext4_save(void) CONFIG_ENV_EXT4_FILE, ifname, dev, part); return 1; } + gd->env_valid = ENV_VALID; puts("done\n"); return 0; @@ -124,7 +127,11 @@ static int env_ext4_load(void) goto err_env_relocate; } - return env_import(buf, 1); + err = env_import(buf, 1); + if (!err) + gd->env_valid = ENV_VALID; + + return err; err_env_relocate: env_set_default(NULL, 0); From 22140d16e56cec3c2ea17a0abaf3d061ccbee039 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:16 +0200 Subject: [PATCH 06/23] env: sf: avoid space in backend name Remove space in ENV backend name for SPI Flash (SF) to avoid issue with env select command. Signed-off-by: Patrick Delaunay --- env/sf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/env/sf.c b/env/sf.c index 3e524f2947..a059561cb0 100644 --- a/env/sf.c +++ b/env/sf.c @@ -305,7 +305,7 @@ static int env_sf_init(void) U_BOOT_ENV_LOCATION(sf) = { .location = ENVL_SPI_FLASH, - ENV_NAME("SPI Flash") + ENV_NAME("SPIFlash") .load = env_sf_load, .save = CONFIG_IS_ENABLED(SAVEENV) ? ENV_SAVE_PTR(env_sf_save) : NULL, #if defined(INITENV) && (CONFIG_ENV_ADDR != 0x0) From 6d8d8400a2bee44e0efbd7dbcfa33965560370d6 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:17 +0200 Subject: [PATCH 07/23] env: correctly handle env_load_prio Only update gd->env_load_prio in generic function env_load() and no more in the weak function env_get_location() which is called in many place (for example in env_driver_lookup, even for ENVOP_SAVE operation). This patch is a preliminary step to use env_driver_lookup()/ env_get_location() in new function env_select() without updating gd->env_load_prio. Signed-off-by: Patrick Delaunay --- env/env.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/env/env.c b/env/env.c index 2e64346438..bcc68c6bce 100644 --- a/env/env.c +++ b/env/env.c @@ -131,8 +131,6 @@ __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; - return env_locations[prio]; } @@ -204,6 +202,8 @@ int env_load(void) ret = drv->load(); if (!ret) { printf("OK\n"); + gd->env_load_prio = prio; + return 0; } else if (ret == -ENOMSG) { /* Handle "bad CRC" case */ @@ -227,7 +227,8 @@ int env_load(void) debug("Selecting environment with bad CRC\n"); else best_prio = 0; - env_get_location(ENVOP_LOAD, best_prio); + + gd->env_load_prio = best_prio; return -ENODEV; } From ad3fec2364ebfc896ce5bd5269f56df9f0c3e5a9 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:18 +0200 Subject: [PATCH 08/23] env: nowhere: add .load ops Add the ops .load for nowhere ENV backend to load the default environment. This ops is needed for the command 'env load' Signed-off-by: Patrick Delaunay --- env/nowhere.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/env/nowhere.c b/env/nowhere.c index f5b0a17652..d33fdf27d0 100644 --- a/env/nowhere.c +++ b/env/nowhere.c @@ -27,8 +27,25 @@ static int env_nowhere_init(void) return 0; } +static int env_nowhere_load(void) +{ + /* + * for SPL, set env_valid = ENV_INVALID is enougth as env_get_char() + * return the default env if env_get is used + * and SPL don't used env_import to reduce its size + * For U-Boot proper, import the default environment to allow reload. + */ + if (!IS_ENABLED(CONFIG_SPL_BUILD)) + env_set_default(NULL, 0); + + gd->env_valid = ENV_INVALID; + + return 0; +} + U_BOOT_ENV_LOCATION(nowhere) = { .location = ENVL_NOWHERE, .init = env_nowhere_init, + .load = env_nowhere_load, ENV_NAME("nowhere") }; From 466d9855d4ee828c998ee3ea29e5685e38d3064e Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:19 +0200 Subject: [PATCH 09/23] env: the ops driver load becomes mandatory in struct env_driver The ops driver load becomes mandatory in struct env_drive, change the comment for this ops and remove unnecessary test. Signed-off-by: Patrick Delaunay --- env/env.c | 3 --- include/env_internal.h | 3 +-- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/env/env.c b/env/env.c index bcc68c6bce..5cf5bd238f 100644 --- a/env/env.c +++ b/env/env.c @@ -187,9 +187,6 @@ int env_load(void) for (prio = 0; (drv = env_driver_lookup(ENVOP_LOAD, prio)); prio++) { int ret; - if (!drv->load) - continue; - if (!env_has_inited(drv->location)) continue; diff --git a/include/env_internal.h b/include/env_internal.h index b9459f926c..b26dc6239c 100644 --- a/include/env_internal.h +++ b/include/env_internal.h @@ -154,8 +154,7 @@ struct env_driver { /** * load() - Load the environment from storage * - * This method is optional. If not provided, no environment will be - * loaded. + * This method is required for loading environment * * @return 0 if OK, -ve on error */ From 0115dd3a6a144e9c974e00a9f3f41c5bb053236e Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:20 +0200 Subject: [PATCH 10/23] cmd: env: add env load command Add the new command env load to load the environment from the current location gd->env_load_prio. Signed-off-by: Patrick Delaunay --- cmd/Kconfig | 6 ++++++ cmd/nvedit.c | 14 ++++++++++++++ env/env.c | 28 ++++++++++++++++++++++++++++ include/env.h | 7 +++++++ 4 files changed, 55 insertions(+) diff --git a/cmd/Kconfig b/cmd/Kconfig index bea2ddf830..498fd31b10 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -604,6 +604,12 @@ config CMD_NVEDIT_INFO [-q] : quiet output The result of multiple evaluations will be combined with AND. +config CMD_NVEDIT_LOAD + bool "env load" + help + Load all environment variables from the compiled-in persistent + storage. + endmenu menu "Memory commands" diff --git a/cmd/nvedit.c b/cmd/nvedit.c index acd9f82667..f730e2e754 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -794,6 +794,14 @@ U_BOOT_CMD( ); #endif #endif + +#if defined(CONFIG_CMD_NVEDIT_LOAD) +static int do_env_load(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + return env_reload() ? 1 : 0; +} +#endif #endif /* CONFIG_SPL_BUILD */ int env_match(uchar *s1, int i2) @@ -1346,6 +1354,9 @@ static struct cmd_tbl cmd_env_sub[] = { #endif #if defined(CONFIG_CMD_NVEDIT_INFO) U_BOOT_CMD_MKENT(info, 3, 0, do_env_info, "", ""), +#endif +#if defined(CONFIG_CMD_NVEDIT_LOAD) + U_BOOT_CMD_MKENT(load, 1, 0, do_env_load, "", ""), #endif U_BOOT_CMD_MKENT(print, CONFIG_SYS_MAXARGS, 1, do_env_print, "", ""), #if defined(CONFIG_CMD_RUN) @@ -1442,6 +1453,9 @@ static char env_help_text[] = "env erase - erase environment\n" #endif #endif +#if defined(CONFIG_CMD_NVEDIT_LOAD) + "env load - load environment\n" +#endif #if defined(CONFIG_CMD_NVEDIT_EFI) "env set -e [-nv][-bs][-rt][-at][-a][-i addr,size][-v] name [arg ...]\n" " - set UEFI variable; unset if '-i' or 'arg' not specified\n" diff --git a/env/env.c b/env/env.c index 5cf5bd238f..785a2b8552 100644 --- a/env/env.c +++ b/env/env.c @@ -230,6 +230,34 @@ int env_load(void) return -ENODEV; } +int env_reload(void) +{ + struct env_driver *drv; + + drv = env_driver_lookup(ENVOP_LOAD, gd->env_load_prio); + if (drv) { + int ret; + + printf("Loading Environment from %s... ", drv->name); + + if (!env_has_inited(drv->location)) { + printf("not initialized\n"); + return -ENODEV; + } + + ret = drv->load(); + if (ret) + printf("Failed (%d)\n", ret); + else + printf("OK\n"); + + if (!ret) + return 0; + } + + return -ENODEV; +} + int env_save(void) { struct env_driver *drv; diff --git a/include/env.h b/include/env.h index d6c2d751d6..68e0f4fa56 100644 --- a/include/env.h +++ b/include/env.h @@ -265,6 +265,13 @@ int env_set_default_vars(int nvars, char *const vars[], int flags); */ int env_load(void); +/** + * env_reload() - Re-Load the environment from current storage + * + * @return 0 if OK, -ve on error + */ +int env_reload(void); + /** * env_save() - Save the environment to storage * From a97d22ebba2305f2d0aee714544c72c6a53026d9 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:21 +0200 Subject: [PATCH 11/23] cmd: env: add env select command Add the new command 'env select' to force the persistent storage of environment, saved in gd->env_load_prio. Signed-off-by: Patrick Delaunay --- cmd/Kconfig | 5 +++++ cmd/nvedit.c | 15 +++++++++++++++ env/env.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/env.h | 8 +++++++- 4 files changed, 69 insertions(+), 1 deletion(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index 498fd31b10..d7136b0e79 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -610,6 +610,11 @@ config CMD_NVEDIT_LOAD Load all environment variables from the compiled-in persistent storage. +config CMD_NVEDIT_SELECT + bool "env select" + help + Select the compiled-in persistent storage of environment variables. + endmenu menu "Memory commands" diff --git a/cmd/nvedit.c b/cmd/nvedit.c index f730e2e754..d188c6aa6b 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -802,6 +802,15 @@ static int do_env_load(struct cmd_tbl *cmdtp, int flag, int argc, return env_reload() ? 1 : 0; } #endif + +#if defined(CONFIG_CMD_NVEDIT_SELECT) +static int do_env_select(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + return env_select(argv[1]) ? 1 : 0; +} +#endif + #endif /* CONFIG_SPL_BUILD */ int env_match(uchar *s1, int i2) @@ -1367,6 +1376,9 @@ static struct cmd_tbl cmd_env_sub[] = { #if defined(CONFIG_CMD_ERASEENV) U_BOOT_CMD_MKENT(erase, 1, 0, do_env_erase, "", ""), #endif +#endif +#if defined(CONFIG_CMD_NVEDIT_SELECT) + U_BOOT_CMD_MKENT(select, 2, 0, do_env_select, "", ""), #endif U_BOOT_CMD_MKENT(set, CONFIG_SYS_MAXARGS, 0, do_env_set, "", ""), #if defined(CONFIG_CMD_ENV_EXISTS) @@ -1456,6 +1468,9 @@ static char env_help_text[] = #if defined(CONFIG_CMD_NVEDIT_LOAD) "env load - load environment\n" #endif +#if defined(CONFIG_CMD_NVEDIT_SELECT) + "env select [target] - select environment target\n" +#endif #if defined(CONFIG_CMD_NVEDIT_EFI) "env set -e [-nv][-bs][-rt][-at][-a][-i addr,size][-v] name [arg ...]\n" " - set UEFI variable; unset if '-i' or 'arg' not specified\n" diff --git a/env/env.c b/env/env.c index 785a2b8552..2af2fae23c 100644 --- a/env/env.c +++ b/env/env.c @@ -344,3 +344,45 @@ int env_init(void) return ret; } + +int env_select(const char *name) +{ + struct env_driver *drv; + const int n_ents = ll_entry_count(struct env_driver, env_driver); + struct env_driver *entry; + int prio; + bool found = false; + + printf("Select Environment on %s: ", name); + + /* search ENV driver by name */ + drv = ll_entry_start(struct env_driver, env_driver); + for (entry = drv; entry != drv + n_ents; entry++) { + if (!strcmp(entry->name, name)) { + found = true; + break; + } + } + + if (!found) { + printf("driver not found\n"); + return -ENODEV; + } + + /* search priority by driver */ + for (prio = 0; (drv = env_driver_lookup(ENVOP_INIT, prio)); prio++) { + if (entry->location == env_get_location(ENVOP_LOAD, prio)) { + /* when priority change, reset the ENV flags */ + if (gd->env_load_prio != prio) { + gd->env_load_prio = prio; + gd->env_valid = ENV_INVALID; + gd->flags &= ~GD_FLG_ENV_DEFAULT; + } + printf("OK\n"); + return 0; + } + } + printf("priority not found\n"); + + return -ENODEV; +} diff --git a/include/env.h b/include/env.h index 68e0f4fa56..665857f032 100644 --- a/include/env.h +++ b/include/env.h @@ -286,6 +286,13 @@ int env_save(void); */ int env_erase(void); +/** + * env_select() - Select the environment storage + * + * @return 0 if OK, -ve on error + */ +int env_select(const char *name); + /** * env_import() - Import from a binary representation into hash table * @@ -349,5 +356,4 @@ int env_get_char(int index); * This is used for those unfortunate archs with crappy toolchains */ void env_reloc(void); - #endif From 4df087ccf997d766df188ca8badcdaf523c24314 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:22 +0200 Subject: [PATCH 12/23] configs: sandbox: activate env in ext4 support Activate ENV in EXT4 support in sandbox. The sandbox behavior don't change; the default environment with the nowhere backend (CONFIG_ENV_IS_NOWHERE)is still used: the weak function env_get_location() return ENVL_NOWHERE for priority 0. Signed-off-by: Patrick Delaunay --- board/sandbox/sandbox.c | 15 +++++++++++++++ configs/sandbox64_defconfig | 4 ++++ configs/sandbox_defconfig | 4 ++++ configs/sandbox_flattree_defconfig | 4 ++++ configs/sandbox_spl_defconfig | 4 ++++ 5 files changed, 31 insertions(+) diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c index c1317a8ca3..937ce28411 100644 --- a/board/sandbox/sandbox.c +++ b/board/sandbox/sandbox.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -44,6 +45,20 @@ unsigned long timer_read_counter(void) } #endif +/* specific order for sandbox: nowhere is the first value, used by default */ +static enum env_location env_locations[] = { + ENVL_NOWHERE, + ENVL_EXT4, +}; + +enum env_location env_get_location(enum env_operation op, int prio) +{ + if (prio >= ARRAY_SIZE(env_locations)) + return ENVL_UNKNOWN; + + return env_locations[prio]; +} + int dram_init(void) { gd->ram_size = CONFIG_SYS_SDRAM_SIZE; diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index e8a1d57285..9a16621b03 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -83,6 +83,10 @@ CONFIG_OF_CONTROL=y CONFIG_OF_LIVE=y CONFIG_OF_HOSTFILE=y CONFIG_BOOTP_SEND_HOSTNAME=y +CONFIG_ENV_IS_NOWHERE=y +CONFIG_ENV_IS_IN_EXT4=y +CONFIG_ENV_EXT4_INTERFACE="host" +CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0" CONFIG_NETCONSOLE=y CONFIG_IP_DEFRAG=y CONFIG_REGMAP=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 5bb44c780f..09b6dcb646 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -94,6 +94,10 @@ CONFIG_OF_CONTROL=y CONFIG_OF_LIVE=y CONFIG_OF_HOSTFILE=y CONFIG_BOOTP_SEND_HOSTNAME=y +CONFIG_ENV_IS_NOWHERE=y +CONFIG_ENV_IS_IN_EXT4=y +CONFIG_ENV_EXT4_INTERFACE="host" +CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0" CONFIG_NETCONSOLE=y CONFIG_IP_DEFRAG=y CONFIG_REGMAP=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 4b28b8e71c..c76c0e83d2 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -66,6 +66,10 @@ CONFIG_AMIGA_PARTITION=y CONFIG_OF_CONTROL=y CONFIG_OF_HOSTFILE=y CONFIG_BOOTP_SEND_HOSTNAME=y +CONFIG_ENV_IS_NOWHERE=y +CONFIG_ENV_IS_IN_EXT4=y +CONFIG_ENV_EXT4_INTERFACE="host" +CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0" CONFIG_NETCONSOLE=y CONFIG_IP_DEFRAG=y CONFIG_REGMAP=y diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 4549a81ff0..85a3e9c0b0 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -83,6 +83,10 @@ CONFIG_SPL_OF_CONTROL=y CONFIG_OF_HOSTFILE=y CONFIG_SPL_OF_PLATDATA=y CONFIG_BOOTP_SEND_HOSTNAME=y +CONFIG_ENV_IS_NOWHERE=y +CONFIG_ENV_IS_IN_EXT4=y +CONFIG_ENV_EXT4_INTERFACE="host" +CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0" CONFIG_NETCONSOLE=y CONFIG_IP_DEFRAG=y CONFIG_SPL_DM=y From 72773c06557a5ffadc5ecac8929e87c37d016f72 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:23 +0200 Subject: [PATCH 13/23] configs: sandbox: activate command env select and env load Add support of environment location with the new env command: 'env select' and 'env load' The ENV backend is selected by priority order - 0 = "nowhere" (default at boot) - 1 = "EXT4" To test EXT4 env support, this backend is selected by name: > env select EXT4 Signed-off-by: Patrick Delaunay --- configs/sandbox64_defconfig | 2 ++ configs/sandbox_defconfig | 2 ++ configs/sandbox_flattree_defconfig | 2 ++ configs/sandbox_spl_defconfig | 2 ++ 4 files changed, 8 insertions(+) diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index 9a16621b03..8a142850bf 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -31,6 +31,8 @@ CONFIG_CMD_ENV_CALLBACK=y CONFIG_CMD_ENV_FLAGS=y CONFIG_CMD_NVEDIT_EFI=y CONFIG_CMD_NVEDIT_INFO=y +CONFIG_CMD_NVEDIT_LOAD=y +CONFIG_CMD_NVEDIT_SELECT=y CONFIG_LOOPW=y CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 09b6dcb646..0fce901f67 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -36,6 +36,8 @@ CONFIG_CMD_ENV_CALLBACK=y CONFIG_CMD_ENV_FLAGS=y CONFIG_CMD_NVEDIT_EFI=y CONFIG_CMD_NVEDIT_INFO=y +CONFIG_CMD_NVEDIT_LOAD=y +CONFIG_CMD_NVEDIT_SELECT=y CONFIG_LOOPW=y CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index c76c0e83d2..e91e9def23 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -25,6 +25,8 @@ CONFIG_CMD_BOOTEFI_HELLO=y CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y CONFIG_CMD_NVEDIT_INFO=y +CONFIG_CMD_NVEDIT_LOAD=y +CONFIG_CMD_NVEDIT_SELECT=y CONFIG_LOOPW=y CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 85a3e9c0b0..5298b78d6d 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -37,6 +37,8 @@ CONFIG_CMD_GREPENV=y CONFIG_CMD_ENV_CALLBACK=y CONFIG_CMD_ENV_FLAGS=y CONFIG_CMD_NVEDIT_INFO=y +CONFIG_CMD_NVEDIT_LOAD=y +CONFIG_CMD_NVEDIT_SELECT=y CONFIG_LOOPW=y CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y From ad04576b2754a41879d5aef6351945fd7ce9353b Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:24 +0200 Subject: [PATCH 14/23] test: environment in ext4 Add basic test to persistent environment in ext4: save and load in host ext4 file 'uboot.env'. On first execution an empty EXT4 file system is created in persistent data dir: env.ext4.img. Signed-off-by: Patrick Delaunay --- test/py/tests/test_env.py | 97 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py index a64aaa9bc5..70913c8d9a 100644 --- a/test/py/tests/test_env.py +++ b/test/py/tests/test_env.py @@ -4,6 +4,10 @@ # Test operation of shell commands relating to environment variables. +import os +import os.path +from subprocess import call, check_call, CalledProcessError + import pytest import u_boot_utils @@ -374,7 +378,6 @@ def test_env_info(state_test_env): @pytest.mark.buildconfigspec('cmd_nvedit_info') @pytest.mark.buildconfigspec('cmd_echo') def test_env_info_sandbox(state_test_env): - """Test 'env info' command result with several options on sandbox with a known ENV configuration: ready & default & persistent """ @@ -399,3 +402,95 @@ def test_env_info_sandbox(state_test_env): response = c.run_command('env info -d -p -q') response = c.run_command('echo $?') assert response == "1" + +def mk_env_ext4(state_test_env): + + """Create a empty ext4 file system volume.""" + c = state_test_env.u_boot_console + filename = 'env.ext4.img' + persistent = c.config.persistent_data_dir + '/' + filename + fs_img = c.config.result_dir + '/' + filename + + if os.path.exists(persistent): + c.log.action('Disk image file ' + persistent + ' already exists') + else: + try: + u_boot_utils.run_and_log(c, 'dd if=/dev/zero of=%s bs=1M count=16' % persistent) + u_boot_utils.run_and_log(c, 'mkfs.ext4 -O ^metadata_csum %s' % persistent) + except CalledProcessError: + call('rm -f %s' % persistent, shell=True) + raise + + u_boot_utils.run_and_log(c, ['cp', '-f', persistent, fs_img]) + return fs_img + +@pytest.mark.boardspec('sandbox') +@pytest.mark.buildconfigspec('cmd_echo') +@pytest.mark.buildconfigspec('cmd_nvedit_info') +@pytest.mark.buildconfigspec('cmd_nvedit_load') +@pytest.mark.buildconfigspec('cmd_nvedit_select') +@pytest.mark.buildconfigspec('env_is_in_ext4') +def test_env_ext4(state_test_env): + + """Test ENV in EXT4 on sandbox.""" + c = state_test_env.u_boot_console + fs_img = '' + try: + fs_img = mk_env_ext4(state_test_env) + + c.run_command('host bind 0 %s' % fs_img) + + response = c.run_command('ext4ls host 0:0') + assert 'uboot.env' not in response + + # force env location: EXT4 (prio 1 in sandbox) + response = c.run_command('env select EXT4') + assert 'Select Environment on EXT4: OK' in response + + response = c.run_command('env save') + assert 'Saving Environment to EXT4' in response + + response = c.run_command('env load') + assert 'Loading Environment from EXT4... OK' in response + + response = c.run_command('ext4ls host 0:0') + assert '8192 uboot.env' in response + + response = c.run_command('env info') + assert 'env_valid = valid' in response + assert 'env_ready = true' in response + assert 'env_use_default = false' in response + + response = c.run_command('env info -p -d') + assert 'Environment was loaded from persistent storage' in response + assert 'Environment can be persisted' in response + + response = c.run_command('env info -d -q') + assert response == "" + response = c.run_command('echo $?') + assert response == "1" + + response = c.run_command('env info -p -q') + assert response == "" + response = c.run_command('echo $?') + assert response == "0" + + # restore env location: NOWHERE (prio 0 in sandbox) + response = c.run_command('env select nowhere') + assert 'Select Environment on nowhere: OK' in response + + response = c.run_command('env load') + assert 'Loading Environment from nowhere... OK' in response + + response = c.run_command('env info') + assert 'env_valid = invalid' in response + assert 'env_ready = true' in response + assert 'env_use_default = true' in response + + response = c.run_command('env info -p -d') + assert 'Default environment is used' in response + assert 'Environment cannot be persisted' in response + + finally: + if fs_img: + call('rm -f %s' % fs_img, shell=True) From f6de047e02bee81ecec711a19e11619b108ce1b1 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:25 +0200 Subject: [PATCH 15/23] env: ext4: introduce new function env_ext4_save_buffer Split the function env_ext4_save to prepare the erase support. Signed-off-by: Patrick Delaunay --- env/ext4.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/env/ext4.c b/env/ext4.c index ac9f126bec..0a10a5e050 100644 --- a/env/ext4.c +++ b/env/ext4.c @@ -44,9 +44,8 @@ __weak const char *env_ext4_get_dev_part(void) return (const char *)CONFIG_ENV_EXT4_DEVICE_AND_PART; } -static int env_ext4_save(void) +static int env_ext4_save_buffer(env_t *env_new) { - env_t env_new; struct blk_desc *dev_desc = NULL; struct disk_partition info; int dev, part; @@ -54,10 +53,6 @@ static int env_ext4_save(void) const char *ifname = env_ext4_get_intf(); const char *dev_and_part = env_ext4_get_dev_part(); - err = env_export(&env_new); - if (err) - return err; - part = blk_get_device_part_str(ifname, dev_and_part, &dev_desc, &info, 1); if (part < 0) @@ -72,7 +67,7 @@ static int env_ext4_save(void) return 1; } - err = ext4fs_write(CONFIG_ENV_EXT4_FILE, (void *)&env_new, + err = ext4fs_write(CONFIG_ENV_EXT4_FILE, (void *)env_new, sizeof(env_t), FILETYPE_REG); ext4fs_close(); @@ -81,9 +76,26 @@ static int env_ext4_save(void) CONFIG_ENV_EXT4_FILE, ifname, dev, part); return 1; } - gd->env_valid = ENV_VALID; + return 0; +} + +static int env_ext4_save(void) +{ + env_t env_new; + int err; + + err = env_export(&env_new); + if (err) + return err; + + err = env_ext4_save_buffer(&env_new); + if (err) + return err; + + gd->env_valid = ENV_VALID; puts("done\n"); + return 0; } From 0718f7432749af284828e22b0934366244c3ad62 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:26 +0200 Subject: [PATCH 16/23] env: ext4: add support of command env erase Add support of opts erase for env in ext4, this opts is used by command 'env erase'. This command only fill the env file (CONFIG_ENV_EXT4_FILE) with 0, the CRC and the saved environment becomes invalid. Signed-off-by: Patrick Delaunay --- env/ext4.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/env/ext4.c b/env/ext4.c index 0a10a5e050..cc36504154 100644 --- a/env/ext4.c +++ b/env/ext4.c @@ -99,6 +99,23 @@ static int env_ext4_save(void) return 0; } +static int env_ext4_erase(void) +{ + env_t env_new; + int err; + + memset(&env_new, 0, sizeof(env_t)); + + err = env_ext4_save_buffer(&env_new); + if (err) + return err; + + gd->env_valid = ENV_INVALID; + puts("done\n"); + + return 0; +} + static int env_ext4_load(void) { ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE); @@ -156,4 +173,6 @@ U_BOOT_ENV_LOCATION(ext4) = { ENV_NAME("EXT4") .load = env_ext4_load, .save = ENV_SAVE_PTR(env_ext4_save), + .erase = CONFIG_IS_ENABLED(CMD_ERASEENV) ? env_ext4_erase : + NULL, }; From ef5cc2e5c6f0fe36832230c23aa2e2d8c0764caf Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 28 Jul 2020 11:51:27 +0200 Subject: [PATCH 17/23] test: sandbox: add test for erase command Add test for the erase command tested on ENV in EXT4. Signed-off-by: Patrick Delaunay --- configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + configs/sandbox_flattree_defconfig | 1 + configs/sandbox_spl_defconfig | 1 + test/py/tests/test_env.py | 16 ++++++++++++++++ 5 files changed, 20 insertions(+) diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index 8a142850bf..1228c49727 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -27,6 +27,7 @@ CONFIG_CMD_BOOTEFI_HELLO=y # CONFIG_CMD_ELF is not set CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y +CONFIG_CMD_ERASEENV=y CONFIG_CMD_ENV_CALLBACK=y CONFIG_CMD_ENV_FLAGS=y CONFIG_CMD_NVEDIT_EFI=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 0fce901f67..f4f97f34ff 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -32,6 +32,7 @@ CONFIG_CMD_ABOOTIMG=y # CONFIG_CMD_ELF is not set CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y +CONFIG_CMD_ERASEENV=y CONFIG_CMD_ENV_CALLBACK=y CONFIG_CMD_ENV_FLAGS=y CONFIG_CMD_NVEDIT_EFI=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index e91e9def23..78d732d0bc 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -24,6 +24,7 @@ CONFIG_CMD_BOOTEFI_HELLO=y # CONFIG_CMD_ELF is not set CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y +CONFIG_CMD_ERASEENV=y CONFIG_CMD_NVEDIT_INFO=y CONFIG_CMD_NVEDIT_LOAD=y CONFIG_CMD_NVEDIT_SELECT=y diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 5298b78d6d..b846487f23 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -34,6 +34,7 @@ CONFIG_CMD_BOOTEFI_HELLO=y # CONFIG_CMD_ELF is not set CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y +CONFIG_CMD_ERASEENV=y CONFIG_CMD_ENV_CALLBACK=y CONFIG_CMD_ENV_FLAGS=y CONFIG_CMD_NVEDIT_INFO=y diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py index 70913c8d9a..86ec1b36d3 100644 --- a/test/py/tests/test_env.py +++ b/test/py/tests/test_env.py @@ -475,6 +475,22 @@ def test_env_ext4(state_test_env): response = c.run_command('echo $?') assert response == "0" + response = c.run_command('env erase') + assert 'OK' in response + + response = c.run_command('env load') + assert 'Loading Environment from EXT4... ' in response + assert 'bad CRC, using default environment' in response + + response = c.run_command('env info') + assert 'env_valid = invalid' in response + assert 'env_ready = true' in response + assert 'env_use_default = true' in response + + response = c.run_command('env info -p -d') + assert 'Default environment is used' in response + assert 'Environment can be persisted' in response + # restore env location: NOWHERE (prio 0 in sandbox) response = c.run_command('env select nowhere') assert 'Select Environment on nowhere: OK' in response From 0f036bf4b87e6416f5c4d23865a62a62d9073c20 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Tue, 7 Jul 2020 20:51:33 +0200 Subject: [PATCH 18/23] env: Warn on force access if ENV_ACCESS_IGNORE_FORCE set If the ENV_ACCESS_IGNORE_FORCE is set, inform user that the variable cannot be force-set if such attempt happens. Signed-off-by: Marek Vasut Reviewed-by: Tom Rini --- env/flags.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/env/flags.c b/env/flags.c index b88fe7ba9c..f7a53775c4 100644 --- a/env/flags.c +++ b/env/flags.c @@ -524,8 +524,10 @@ int env_flags_validate(const struct env_entry *item, const char *newval, /* check for access permission */ #ifndef CONFIG_ENV_ACCESS_IGNORE_FORCE - if (flag & H_FORCE) + if (flag & H_FORCE) { + printf("## Error: Can't force access to \"%s\"\n", name); return 0; + } #endif switch (op) { case env_op_delete: From ef9bef2bfed36424a3a6678d76d9eb7b710de1ac Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Tue, 7 Jul 2020 20:51:34 +0200 Subject: [PATCH 19/23] env: Add H_DEFAULT flag Add another internal environment flag which indicates that the operation is resetting the environment to the default one. This allows the env code to discern between import of external environment and reset to default. Signed-off-by: Marek Vasut Reviewed-by: Tom Rini --- env/common.c | 3 ++- include/search.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/env/common.c b/env/common.c index 088b2aebb4..0db56e610a 100644 --- a/env/common.c +++ b/env/common.c @@ -81,6 +81,7 @@ void env_set_default(const char *s, int flags) debug("Using default environment\n"); } + flags |= H_DEFAULT; if (himport_r(&env_htab, (char *)default_environment, sizeof(default_environment), '\0', flags, 0, 0, NULL) == 0) @@ -99,7 +100,7 @@ int env_set_default_vars(int nvars, char * const vars[], int flags) * Special use-case: import from default environment * (and use \0 as a separator) */ - flags |= H_NOCLEAR; + flags |= H_NOCLEAR | H_DEFAULT; return himport_r(&env_htab, (const char *)default_environment, sizeof(default_environment), '\0', flags, 0, nvars, vars); diff --git a/include/search.h b/include/search.h index bca36d3abc..c4b50c9630 100644 --- a/include/search.h +++ b/include/search.h @@ -112,5 +112,6 @@ int hwalk_r(struct hsearch_data *htab, #define H_MATCH_METHOD (H_MATCH_IDENT | H_MATCH_SUBSTR | H_MATCH_REGEX) #define H_PROGRAMMATIC (1 << 9) /* indicate that an import is from env_set() */ #define H_ORIGIN_FLAGS (H_INTERACTIVE | H_PROGRAMMATIC) +#define H_DEFAULT (1 << 10) /* indicate that an import is default env */ #endif /* _SEARCH_H_ */ From 890feecaab72a630eac3344443e053173f4ad02f Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Tue, 7 Jul 2020 20:51:35 +0200 Subject: [PATCH 20/23] env: Discern environment coming from external storage Add another custom environment flag which discerns environment coming from external storage from environment set by U-Boot itself. Signed-off-by: Marek Vasut Reviewed-by: Tom Rini --- env/common.c | 13 +++++++------ env/eeprom.c | 2 +- env/ext4.c | 2 +- env/fat.c | 2 +- env/flash.c | 2 +- env/mmc.c | 4 ++-- env/nand.c | 4 ++-- env/nvram.c | 2 +- env/onenand.c | 2 +- env/remote.c | 2 +- env/sata.c | 2 +- env/sf.c | 4 ++-- env/ubi.c | 4 ++-- include/env.h | 7 +++++-- include/search.h | 1 + 15 files changed, 29 insertions(+), 24 deletions(-) diff --git a/env/common.c b/env/common.c index 0db56e610a..ed18378000 100644 --- a/env/common.c +++ b/env/common.c @@ -110,7 +110,7 @@ int env_set_default_vars(int nvars, char * const vars[], int flags) * Check if CRC is valid and (if yes) import the environment. * Note that "buf" may or may not be aligned. */ -int env_import(const char *buf, int check) +int env_import(const char *buf, int check, int flags) { env_t *ep = (env_t *)buf; @@ -125,7 +125,7 @@ int env_import(const char *buf, int check) } } - if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0, 0, + if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', flags, 0, 0, NULL)) { gd->flags |= GD_FLG_ENV_READY; return 0; @@ -142,7 +142,8 @@ int env_import(const char *buf, int check) static unsigned char env_flags; int env_import_redund(const char *buf1, int buf1_read_fail, - const char *buf2, int buf2_read_fail) + const char *buf2, int buf2_read_fail, + int flags) { int crc1_ok, crc2_ok; env_t *ep, *tmp_env1, *tmp_env2; @@ -162,10 +163,10 @@ int env_import_redund(const char *buf1, int buf1_read_fail, return -EIO; } else if (!buf1_read_fail && buf2_read_fail) { gd->env_valid = ENV_VALID; - return env_import((char *)tmp_env1, 1); + return env_import((char *)tmp_env1, 1, flags); } else if (buf1_read_fail && !buf2_read_fail) { gd->env_valid = ENV_REDUND; - return env_import((char *)tmp_env2, 1); + return env_import((char *)tmp_env2, 1, flags); } crc1_ok = crc32(0, tmp_env1->data, ENV_SIZE) == @@ -200,7 +201,7 @@ int env_import_redund(const char *buf1, int buf1_read_fail, ep = tmp_env2; env_flags = ep->flags; - return env_import((char *)ep, 0); + return env_import((char *)ep, 0, flags); } #endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */ diff --git a/env/eeprom.c b/env/eeprom.c index e8126cfe39..e300470ad0 100644 --- a/env/eeprom.c +++ b/env/eeprom.c @@ -188,7 +188,7 @@ static int env_eeprom_load(void) eeprom_bus_read(CONFIG_SYS_DEF_EEPROM_ADDR, off, (uchar *)buf_env, CONFIG_ENV_SIZE); - return env_import(buf_env, 1); + return env_import(buf_env, 1, H_EXTERNAL); } static int env_eeprom_save(void) diff --git a/env/ext4.c b/env/ext4.c index cc36504154..f823b69409 100644 --- a/env/ext4.c +++ b/env/ext4.c @@ -156,7 +156,7 @@ static int env_ext4_load(void) goto err_env_relocate; } - err = env_import(buf, 1); + err = env_import(buf, 1, H_EXTERNAL); if (!err) gd->env_valid = ENV_VALID; diff --git a/env/fat.c b/env/fat.c index 63aced9317..71bf8bfa18 100644 --- a/env/fat.c +++ b/env/fat.c @@ -144,7 +144,7 @@ static int env_fat_load(void) goto err_env_relocate; } - return env_import(buf, 1); + return env_import(buf, 1, H_EXTERNAL); err_env_relocate: env_set_default(NULL, 0); diff --git a/env/flash.c b/env/flash.c index 3198147c38..722d5adf8b 100644 --- a/env/flash.c +++ b/env/flash.c @@ -351,7 +351,7 @@ static int env_flash_load(void) "reading environment; recovered successfully\n\n"); #endif /* CONFIG_ENV_ADDR_REDUND */ - return env_import((char *)flash_addr, 1); + return env_import((char *)flash_addr, 1, H_EXTERNAL); } #endif /* LOADENV */ diff --git a/env/mmc.c b/env/mmc.c index aca61b75e9..af7e5fbac3 100644 --- a/env/mmc.c +++ b/env/mmc.c @@ -338,7 +338,7 @@ static int env_mmc_load(void) read2_fail = read_env(mmc, CONFIG_ENV_SIZE, offset2, tmp_env2); ret = env_import_redund((char *)tmp_env1, read1_fail, (char *)tmp_env2, - read2_fail); + read2_fail, H_EXTERNAL); fini: fini_mmc_for_env(mmc); @@ -380,7 +380,7 @@ static int env_mmc_load(void) goto fini; } - ret = env_import(buf, 1); + ret = env_import(buf, 1, H_EXTERNAL); if (!ret) { ep = (env_t *)buf; gd->env_addr = (ulong)&ep->data; diff --git a/env/nand.c b/env/nand.c index 8b0027d304..0d7ee19bc2 100644 --- a/env/nand.c +++ b/env/nand.c @@ -331,7 +331,7 @@ static int env_nand_load(void) read2_fail = readenv(CONFIG_ENV_OFFSET_REDUND, (u_char *) tmp_env2); ret = env_import_redund((char *)tmp_env1, read1_fail, (char *)tmp_env2, - read2_fail); + read2_fail, H_EXTERNAL); done: free(tmp_env1); @@ -372,7 +372,7 @@ static int env_nand_load(void) return -EIO; } - return env_import(buf, 1); + return env_import(buf, 1, H_EXTERNAL); #endif /* ! ENV_IS_EMBEDDED */ return 0; diff --git a/env/nvram.c b/env/nvram.c index 1a9fcf1c06..7c8ea26f96 100644 --- a/env/nvram.c +++ b/env/nvram.c @@ -64,7 +64,7 @@ static int env_nvram_load(void) #else memcpy(buf, (void *)CONFIG_ENV_ADDR, CONFIG_ENV_SIZE); #endif - return env_import(buf, 1); + return env_import(buf, 1, H_EXTERNAL); } static int env_nvram_save(void) diff --git a/env/onenand.c b/env/onenand.c index dfd4e939f8..a2477cef9b 100644 --- a/env/onenand.c +++ b/env/onenand.c @@ -55,7 +55,7 @@ static int env_onenand_load(void) mtd->writesize = MAX_ONENAND_PAGESIZE; #endif /* !ENV_IS_EMBEDDED */ - rc = env_import(buf, 1); + rc = env_import(buf, 1, H_EXTERNAL); if (!rc) gd->env_valid = ENV_VALID; diff --git a/env/remote.c b/env/remote.c index e3f0608b16..d93a137376 100644 --- a/env/remote.c +++ b/env/remote.c @@ -45,7 +45,7 @@ static int env_remote_save(void) static int env_remote_load(void) { #ifndef ENV_IS_EMBEDDED - return env_import((char *)env_ptr, 1); + return env_import((char *)env_ptr, 1, H_EXTERNAL); #endif return 0; diff --git a/env/sata.c b/env/sata.c index 8bfcc94306..9442cfcaf3 100644 --- a/env/sata.c +++ b/env/sata.c @@ -111,7 +111,7 @@ static void env_sata_load(void) return -EIO; } - return env_import(buf, 1); + return env_import(buf, 1, H_EXTERNAL); } U_BOOT_ENV_LOCATION(sata) = { diff --git a/env/sf.c b/env/sf.c index a059561cb0..937778aa37 100644 --- a/env/sf.c +++ b/env/sf.c @@ -172,7 +172,7 @@ static int env_sf_load(void) CONFIG_ENV_SIZE, tmp_env2); ret = env_import_redund((char *)tmp_env1, read1_fail, (char *)tmp_env2, - read2_fail); + read2_fail, H_EXTERNAL); spi_flash_free(env_flash); env_flash = NULL; @@ -265,7 +265,7 @@ static int env_sf_load(void) goto err_read; } - ret = env_import(buf, 1); + ret = env_import(buf, 1, H_EXTERNAL); if (!ret) gd->env_valid = ENV_VALID; diff --git a/env/ubi.c b/env/ubi.c index 08aac47df2..5502efe28b 100644 --- a/env/ubi.c +++ b/env/ubi.c @@ -141,7 +141,7 @@ static int env_ubi_load(void) CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME_REDUND); return env_import_redund((char *)tmp_env1, read1_fail, (char *)tmp_env2, - read2_fail); + read2_fail, H_EXTERNAL); } #else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */ static int env_ubi_load(void) @@ -172,7 +172,7 @@ static int env_ubi_load(void) return -EIO; } - return env_import(buf, 1); + return env_import(buf, 1, H_EXTERNAL); } #endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */ diff --git a/include/env.h b/include/env.h index 665857f032..af405955b0 100644 --- a/include/env.h +++ b/include/env.h @@ -302,10 +302,11 @@ int env_select(const char *name); * @buf: Buffer containing the environment (struct environemnt_s *) * @check: non-zero to check the CRC at the start of the environment, 0 to * ignore it + * @flags: Flags controlling matching (H_... - see search.h) * @return 0 if imported successfully, -ENOMSG if the CRC was bad, -EIO if * something else went wrong */ -int env_import(const char *buf, int check); +int env_import(const char *buf, int check, int flags); /** * env_export() - Export the environment to a buffer @@ -324,10 +325,12 @@ int env_export(struct environment_s *env_out); * @buf1_read_fail: 0 if buf1 is valid, non-zero if invalid * @buf2: Second environment (struct environemnt_s *) * @buf2_read_fail: 0 if buf2 is valid, non-zero if invalid + * @flags: Flags controlling matching (H_... - see search.h) * @return 0 if OK, -EIO if no environment is valid, -ENOMSG if the CRC was bad */ int env_import_redund(const char *buf1, int buf1_read_fail, - const char *buf2, int buf2_read_fail); + const char *buf2, int buf2_read_fail, + int flags); /** * env_get_default() - Look up a variable from the default environment diff --git a/include/search.h b/include/search.h index c4b50c9630..e56843c26f 100644 --- a/include/search.h +++ b/include/search.h @@ -113,5 +113,6 @@ int hwalk_r(struct hsearch_data *htab, #define H_PROGRAMMATIC (1 << 9) /* indicate that an import is from env_set() */ #define H_ORIGIN_FLAGS (H_INTERACTIVE | H_PROGRAMMATIC) #define H_DEFAULT (1 << 10) /* indicate that an import is default env */ +#define H_EXTERNAL (1 << 11) /* indicate that an import is external env */ #endif /* _SEARCH_H_ */ From 47f3b1f243acfe755340753c5d467ba781618fa6 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Tue, 7 Jul 2020 20:51:38 +0200 Subject: [PATCH 21/23] env: Add option to only ever append environment Add configuration option which prevents the environment hash table to be ever cleared and reloaded with different content. This is useful in case the first environment loaded into the hash table contains e.g. sensitive content which must not be dropped or reloaded. Signed-off-by: Marek Vasut Reviewed-by: Tom Rini --- env/Kconfig | 9 +++++++++ env/env.c | 2 ++ lib/hashtable.c | 4 ++++ 3 files changed, 15 insertions(+) diff --git a/env/Kconfig b/env/Kconfig index dcc525d4ed..1cae1edf6a 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -614,6 +614,15 @@ config DELAY_ENVIRONMENT later by U-Boot code. With CONFIG_OF_CONTROL this is instead controlled by the value of /config/load-environment. +config ENV_APPEND + bool "Always append the environment with new data" + default n + help + If defined, the environment hash table is only ever appended with new + data, but the existing hash table can never be dropped and reloaded + with newly imported data. This may be used in combination with static + flags to e.g. to protect variables which must not be modified. + config ENV_ACCESS_IGNORE_FORCE bool "Block forced environment operations" default n diff --git a/env/env.c b/env/env.c index 2af2fae23c..42c7d8155e 100644 --- a/env/env.c +++ b/env/env.c @@ -201,7 +201,9 @@ int env_load(void) printf("OK\n"); gd->env_load_prio = prio; +#if !CONFIG_IS_ENABLED(ENV_APPEND) return 0; +#endif } else if (ret == -ENOMSG) { /* Handle "bad CRC" case */ if (best_prio == -1) diff --git a/lib/hashtable.c b/lib/hashtable.c index 7b6781bc35..ef834badc5 100644 --- a/lib/hashtable.c +++ b/lib/hashtable.c @@ -826,6 +826,10 @@ int himport_r(struct hsearch_data *htab, if (nvars) memcpy(localvars, vars, sizeof(vars[0]) * nvars); +#if CONFIG_IS_ENABLED(ENV_APPEND) + flag |= H_NOCLEAR; +#endif + if ((flag & H_NOCLEAR) == 0 && !nvars) { /* Destroy old hash table if one exists */ debug("Destroy Hash Table: %p table = %p\n", htab, From d045cbacf2529266bb312add023e12c0d400bf67 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Tue, 7 Jul 2020 20:51:39 +0200 Subject: [PATCH 22/23] env: Add support for explicit write access list This option marks any U-Boot variable which does not have explicit 'w' writeable flag set as read-only. This way the environment can be locked down and only variables explicitly configured to be writeable can ever be changed by either 'env import', 'env set' or loading user environment from environment storage. Signed-off-by: Marek Vasut Reviewed-by: Tom Rini --- env/Kconfig | 8 ++++++ env/flags.c | 62 +++++++++++++++++++++++++++++++++++++-------- include/env_flags.h | 6 ++++- lib/hashtable.c | 5 +++- 4 files changed, 68 insertions(+), 13 deletions(-) diff --git a/env/Kconfig b/env/Kconfig index 1cae1edf6a..5d0a8ecea0 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -623,6 +623,14 @@ config ENV_APPEND with newly imported data. This may be used in combination with static flags to e.g. to protect variables which must not be modified. +config ENV_WRITEABLE_LIST + bool "Permit write access only to listed variables" + default n + help + If defined, only environment variables which explicitly set the 'w' + writeable flag can be written and modified at runtime. No variables + can be otherwise created, written or imported into the environment. + config ENV_ACCESS_IGNORE_FORCE bool "Block forced environment operations" default n diff --git a/env/flags.c b/env/flags.c index f7a53775c4..df4aed26b2 100644 --- a/env/flags.c +++ b/env/flags.c @@ -28,8 +28,15 @@ #define ENV_FLAGS_NET_VARTYPE_REPS "" #endif +#ifdef CONFIG_ENV_WRITEABLE_LIST +#define ENV_FLAGS_WRITEABLE_VARACCESS_REPS "w" +#else +#define ENV_FLAGS_WRITEABLE_VARACCESS_REPS "" +#endif + static const char env_flags_vartype_rep[] = "sdxb" ENV_FLAGS_NET_VARTYPE_REPS; -static const char env_flags_varaccess_rep[] = "aroc"; +static const char env_flags_varaccess_rep[] = + "aroc" ENV_FLAGS_WRITEABLE_VARACCESS_REPS; static const int env_flags_varaccess_mask[] = { 0, ENV_FLAGS_VARACCESS_PREVENT_DELETE | @@ -38,7 +45,11 @@ static const int env_flags_varaccess_mask[] = { ENV_FLAGS_VARACCESS_PREVENT_DELETE | ENV_FLAGS_VARACCESS_PREVENT_OVERWR, ENV_FLAGS_VARACCESS_PREVENT_DELETE | - ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR}; + ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR, +#ifdef CONFIG_ENV_WRITEABLE_LIST + ENV_FLAGS_VARACCESS_WRITEABLE, +#endif + }; #ifdef CONFIG_CMD_ENV_FLAGS static const char * const env_flags_vartype_names[] = { @@ -56,6 +67,9 @@ static const char * const env_flags_varaccess_names[] = { "read-only", "write-once", "change-default", +#ifdef CONFIG_ENV_WRITEABLE_LIST + "writeable", +#endif }; /* @@ -130,21 +144,25 @@ enum env_flags_vartype env_flags_parse_vartype(const char *flags) */ enum env_flags_varaccess env_flags_parse_varaccess(const char *flags) { + enum env_flags_varaccess va_default = env_flags_varaccess_any; + enum env_flags_varaccess va; char *access; if (strlen(flags) <= ENV_FLAGS_VARACCESS_LOC) - return env_flags_varaccess_any; + return va_default; access = strchr(env_flags_varaccess_rep, flags[ENV_FLAGS_VARACCESS_LOC]); - if (access != NULL) - return (enum env_flags_varaccess) + if (access != NULL) { + va = (enum env_flags_varaccess) (access - &env_flags_varaccess_rep[0]); + return va; + } printf("## Warning: Unknown environment variable access method '%c'\n", flags[ENV_FLAGS_VARACCESS_LOC]); - return env_flags_varaccess_any; + return va_default; } /* @@ -152,17 +170,21 @@ enum env_flags_varaccess env_flags_parse_varaccess(const char *flags) */ enum env_flags_varaccess env_flags_parse_varaccess_from_binflags(int binflags) { + enum env_flags_varaccess va_default = env_flags_varaccess_any; + enum env_flags_varaccess va; int i; for (i = 0; i < ARRAY_SIZE(env_flags_varaccess_mask); i++) if (env_flags_varaccess_mask[i] == - (binflags & ENV_FLAGS_VARACCESS_BIN_MASK)) - return (enum env_flags_varaccess)i; + (binflags & ENV_FLAGS_VARACCESS_BIN_MASK)) { + va = (enum env_flags_varaccess)i; + return va; + } printf("Warning: Non-standard access flags. (0x%x)\n", binflags & ENV_FLAGS_VARACCESS_BIN_MASK); - return env_flags_varaccess_any; + return va_default; } static inline int is_hex_prefix(const char *value) @@ -326,13 +348,14 @@ enum env_flags_vartype env_flags_get_type(const char *name) enum env_flags_varaccess env_flags_get_varaccess(const char *name) { const char *flags_list = env_get(ENV_FLAGS_VAR); + enum env_flags_varaccess va_default = env_flags_varaccess_any; char flags[ENV_FLAGS_ATTR_MAX_LEN + 1]; if (env_flags_lookup(flags_list, name, flags)) - return env_flags_varaccess_any; + return va_default; if (strlen(flags) <= ENV_FLAGS_VARACCESS_LOC) - return env_flags_varaccess_any; + return va_default; return env_flags_parse_varaccess(flags); } @@ -426,7 +449,11 @@ void env_flags_init(struct env_entry *var_entry) int ret = 1; if (first_call) { +#ifdef CONFIG_ENV_WRITEABLE_LIST + flags_list = ENV_FLAGS_LIST_STATIC; +#else flags_list = env_get(ENV_FLAGS_VAR); +#endif first_call = 0; } /* look in the ".flags" and static for a reference to this variable */ @@ -523,6 +550,19 @@ int env_flags_validate(const struct env_entry *item, const char *newval, } /* check for access permission */ +#ifdef CONFIG_ENV_WRITEABLE_LIST + if (flag & H_DEFAULT) + return 0; /* Default env is always OK */ + + /* + * External writeable variables can be overwritten by external env, + * anything else can not be overwritten by external env. + */ + if ((flag & H_EXTERNAL) && + !(item->flags & ENV_FLAGS_VARACCESS_WRITEABLE)) + return 1; +#endif + #ifndef CONFIG_ENV_ACCESS_IGNORE_FORCE if (flag & H_FORCE) { printf("## Error: Can't force access to \"%s\"\n", name); diff --git a/include/env_flags.h b/include/env_flags.h index 725841a891..313cb8c49a 100644 --- a/include/env_flags.h +++ b/include/env_flags.h @@ -24,6 +24,9 @@ enum env_flags_varaccess { env_flags_varaccess_readonly, env_flags_varaccess_writeonce, env_flags_varaccess_changedefault, +#ifdef CONFIG_ENV_WRITEABLE_LIST + env_flags_varaccess_writeable, +#endif env_flags_varaccess_end }; @@ -173,6 +176,7 @@ int env_flags_validate(const struct env_entry *item, const char *newval, #define ENV_FLAGS_VARACCESS_PREVENT_CREATE 0x00000010 #define ENV_FLAGS_VARACCESS_PREVENT_OVERWR 0x00000020 #define ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR 0x00000040 -#define ENV_FLAGS_VARACCESS_BIN_MASK 0x00000078 +#define ENV_FLAGS_VARACCESS_WRITEABLE 0x00000080 +#define ENV_FLAGS_VARACCESS_BIN_MASK 0x000000f8 #endif /* __ENV_FLAGS_H__ */ diff --git a/lib/hashtable.c b/lib/hashtable.c index ef834badc5..4a8c50b4b8 100644 --- a/lib/hashtable.c +++ b/lib/hashtable.c @@ -950,9 +950,12 @@ int himport_r(struct hsearch_data *htab, e.data = value; hsearch_r(e, ENV_ENTER, &rv, htab, flag); - if (rv == NULL) +#if !CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST) + if (rv == NULL) { printf("himport_r: can't insert \"%s=%s\" into hash table\n", name, value); + } +#endif debug("INSERT: table %p, filled %d/%d rv %p ==> name=\"%s\" value=\"%s\"\n", htab, htab->filled, htab->size, From db82015929aeff6b58982a22d61ab8c5b87752f3 Mon Sep 17 00:00:00 2001 From: Ivan Mikhaylov Date: Fri, 10 Jul 2020 19:54:18 +0300 Subject: [PATCH 23/23] fw_setenv: lock the flash only if it was locked before With current implementation of fw_setenv, it is always locks u-boot-env region if lock interface is implemented for such mtd device. You can not control lock of this region with fw_setenv, there is no option for it in config or in application itself. Because of this situation may happen problems like in this thread on xilinx forum: https://forums.xilinx.com/t5/Embedded-Linux/Flash-be-locked-after-use-fw-setenv-from-user-space /td-p/1027851 A short summary of that link is: some person has issue with some spi chip which has lock interface but doesn't locks properly which leads to lock of whole flash memory on lock of u-boot-env region. As resulted solution hack was added into spi-nor.c driver for this chip with lock disablement. Instead fix this problem by adding logic to fw_setenv only lock the flash if it was already locked when we attempted to use it. Signed-off-by: Ivan Mikhaylov --- tools/env/fw_env.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index c6378ecf34..3ab1ae69c7 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -995,6 +995,7 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count) of the data */ loff_t blockstart; /* running start of the current block - MEMGETBADBLOCK needs 64 bits */ + int was_locked; /* flash lock flag */ int rc; /* @@ -1080,6 +1081,12 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count) } erase.length = erasesize; + if (DEVTYPE(dev) != MTD_ABSENT) { + was_locked = ioctl(fd, MEMISLOCKED, &erase); + /* treat any errors as unlocked flash */ + if (was_locked < 0) + was_locked = 0; + } /* This only runs once on NOR flash and SPI-dataflash */ while (processed < write_total) { @@ -1099,7 +1106,8 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count) if (DEVTYPE(dev) != MTD_ABSENT) { erase.start = blockstart; - ioctl(fd, MEMUNLOCK, &erase); + if (was_locked) + ioctl(fd, MEMUNLOCK, &erase); /* These do not need an explicit erase cycle */ if (DEVTYPE(dev) != MTD_DATAFLASH) if (ioctl(fd, MEMERASE, &erase) != 0) { @@ -1127,8 +1135,10 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count) return -1; } - if (DEVTYPE(dev) != MTD_ABSENT) - ioctl(fd, MEMLOCK, &erase); + if (DEVTYPE(dev) != MTD_ABSENT) { + if (was_locked) + ioctl(fd, MEMLOCK, &erase); + } processed += erasesize; block_seek = 0; @@ -1149,7 +1159,9 @@ static int flash_flag_obsolete(int dev, int fd, off_t offset) int rc; struct erase_info_user erase; char tmp = ENV_REDUND_OBSOLETE; + int was_locked; /* flash lock flag */ + was_locked = ioctl(fd, MEMISLOCKED, &erase); erase.start = DEVOFFSET(dev); erase.length = DEVESIZE(dev); /* This relies on the fact, that ENV_REDUND_OBSOLETE == 0 */ @@ -1159,9 +1171,11 @@ static int flash_flag_obsolete(int dev, int fd, off_t offset) DEVNAME(dev)); return rc; } - ioctl(fd, MEMUNLOCK, &erase); + if (was_locked) + ioctl(fd, MEMUNLOCK, &erase); rc = write(fd, &tmp, sizeof(tmp)); - ioctl(fd, MEMLOCK, &erase); + if (was_locked) + ioctl(fd, MEMLOCK, &erase); if (rc < 0) perror("Could not set obsolete flag");