[PATCH v3 00/14] env: ext4: corrections and add test for env in ext4
Hi,
V3 of the serie [1].
In this serie, I add sandbox test with CONFIG_ENV_IS_NOWHERE activated with EXT4 location: load, save and erase.
To test this feature, I add 2 new commands to change the ENV location: - env select [target] - env load
This serie depends on previous env test introduced in [2] "cmd: env: add option for quiet output on env info"
To be able to test invalid file (bad CRC), I also add the support of the command "env erase" for EXT4 env location.
[1] http://patchwork.ozlabs.org/project/uboot/list/?series=183620 [2] http://patchwork.ozlabs.org/project/uboot/list/?series=184539
Regards
Patrick
Changes in v3: - new - new - new: add ?load ops in nowhere - new: load operation becomes mandatory - new: add 'env load' command - new: add 'env select' command - change env_get_location to avoid gd->env_load_prio modification - replace specific sandbox command by generic command 'env select' and 'env load' - change title "sandbox: support the change of env location" - replace specific sandbox command by generic command 'env select' and 'env load' - update after Stephen Warren comments - replace sandbox command by generic command 'env load' in test_env
Changes in v2: - change cmd_tbl_t to struct cmd_tbl - use CONFIG_IS_ENABLED to set .erase (same as .save)
Patrick Delaunay (14): env: add absolute path at CONFIG_ENV_EXT4_FILE env: ext4: set gd->env_valid env: sf: avoid space in backend name env: correctly handle env_load_prio env: nowhere: add .load ops env: the ops driver load becomes mandatory in struct env_driver cmd: env: add env load command cmd: env: add env select command configs: sandbox: activate env in ext4 support configs: sandbox: activate command env select and env load test: environment in ext4 env: ext4: introduce new function env_ext4_save_buffer env: ext4: add support of command env erase test: sandbox: add test for erase command
board/sandbox/sandbox.c | 15 ++++ cmd/Kconfig | 11 +++ cmd/nvedit.c | 29 ++++++++ configs/sandbox64_defconfig | 7 ++ configs/sandbox_defconfig | 7 ++ configs/sandbox_flattree_defconfig | 7 ++ configs/sandbox_spl_defconfig | 7 ++ env/Kconfig | 2 +- env/env.c | 80 ++++++++++++++++++-- env/ext4.c | 54 ++++++++++++-- env/nowhere.c | 9 +++ env/sf.c | 2 +- include/env.h | 15 +++- include/env_internal.h | 3 +- test/py/tests/test_env.py | 113 ++++++++++++++++++++++++++++- 15 files changed, 341 insertions(+), 20 deletions(-)
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 trini@konsulko.com Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
For information, it is the value used today by all the boards:
dragonboard820c_defconfig:29:CONFIG_ENV_EXT4_FILE="/uboot.env" hikey960_defconfig:25:CONFIG_ENV_EXT4_FILE="/uboot.env" stm32mp15_basic_defconfig:64:CONFIG_ENV_EXT4_FILE="/uboot.env" stm32mp15_trusted_defconfig:50:CONFIG_ENV_EXT4_FILE="/uboot.env"
(no changes since v1)
env/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/env/Kconfig b/env/Kconfig index ca7fef682b..9dad1cf8c1 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -469,7 +469,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)
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 trini@konsulko.com Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
(no changes since v1)
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 <ext4fs.h> #include <mmc.h>
+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);
Remove space in ENV backend name for SPI Flash (SF) to avoid issue with env select command.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v3: - new
env/sf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/env/sf.c b/env/sf.c index 02ed846fc7..515ffd4f1d 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)
On Thu, Jun 25, 2020 at 09:59:47AM +0200, Patrick Delaunay wrote:
Remove space in ENV backend name for SPI Flash (SF) to avoid issue with env select command.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Tom Rini trini@konsulko.com
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 patrick.delaunay@st.com ---
Changes in v3: - new
env/env.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/env/env.c b/env/env.c index dcc25c030b..181aaa222c 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; }
On Thu, Jun 25, 2020 at 09:59:48AM +0200, Patrick Delaunay wrote:
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 patrick.delaunay@st.com
Reviewed-by: Tom Rini trini@konsulko.com
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 patrick.delaunay@st.com ---
Changes in v3: - new: add ?load ops in nowhere
env/nowhere.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/env/nowhere.c b/env/nowhere.c index f5b0a17652..6949810a1f 100644 --- a/env/nowhere.c +++ b/env/nowhere.c @@ -27,8 +27,17 @@ static int env_nowhere_init(void) return 0; }
+static int env_nowhere_load(void) +{ + 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") };
On Thu, Jun 25, 2020 at 09:59:49AM +0200, Patrick Delaunay wrote:
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 patrick.delaunay@st.com
Reviewed-by: Tom Rini trini@konsulko.com
On Thu, Jun 25, 2020 at 09:59:49AM +0200, Patrick Delaunay wrote:
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 patrick.delaunay@st.com Reviewed-by: Tom Rini trini@konsulko.com
Changes in v3:
- new: add ?load ops in nowhere
env/nowhere.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/env/nowhere.c b/env/nowhere.c index f5b0a17652..6949810a1f 100644 --- a/env/nowhere.c +++ b/env/nowhere.c @@ -27,8 +27,17 @@ static int env_nowhere_init(void) return 0; }
+static int env_nowhere_load(void) +{
- 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")
};
Build testing this, we get 8KiB size increase in SPL in targets which have ENV_NOWHERE in SPL. Can we guard this somehow, with a logical tie-in to being needed for 'env load' ? Thanks!
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 patrick.delaunay@st.com ---
Changes in v3: - new: load operation becomes mandatory
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 181aaa222c..a1696cd77e 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 66550434c3..795941daee 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 */
On Thu, Jun 25, 2020 at 09:59:50AM +0200, Patrick Delaunay wrote:
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 patrick.delaunay@st.com
Reviewed-by: Tom Rini trini@konsulko.com
Add the new command env load to load the environment from the current location gd->env_load_prio.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v3: - new: add 'env load' command
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 1de57988ae..679b1c32b4 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -610,6 +610,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 0f9cea96f3..7a39d9cd66 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 a1696cd77e..7ad9623ef2 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 *
On Thu, Jun 25, 2020 at 09:59:51AM +0200, Patrick Delaunay wrote:
Add the new command env load to load the environment from the current location gd->env_load_prio.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Tom Rini trini@konsulko.com
Add the new command 'env select' to force the persistent storage of environment, saved in gd->env_load_prio.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v3: - new: add 'env select' command
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 679b1c32b4..5673b56343 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -616,6 +616,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 7a39d9cd66..9fc33d7db1 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 7ad9623ef2..91c76133b0 100644 --- a/env/env.c +++ b/env/env.c @@ -340,3 +340,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
On Thu, Jun 25, 2020 at 09:59:52AM +0200, Patrick Delaunay wrote:
Add the new command 'env select' to force the persistent storage of environment, saved in gd->env_load_prio.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
[snip]
- /* 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;}- }
So, after we do this, is some follow up env command required to initialize the environment to now exist somewhere else? Or will we have initialized all configured locations during boot, and don't have to? But what will happen if we select say "nand" but it's not present so didn't init. Will things fail gracefully (not panic) ? Thanks!
Hi Tom
From: Tom Rini trini@konsulko.com Sent: vendredi 26 juin 2020 22:55
On Thu, Jun 25, 2020 at 09:59:52AM +0200, Patrick Delaunay wrote:
Add the new command 'env select' to force the persistent storage of environment, saved in gd->env_load_prio.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
[snip]
- /* 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;}- }
So, after we do this, is some follow up env command required to initialize the environment to now exist somewhere else? Or will we have initialized all configured locations during boot, and don't have to? But what will happen if we select say "nand" but it's not present so didn't init. Will things fail gracefully (not panic) ? Thanks!
I was not sure if a automatic load was needed in this command, as I add a sparate load command in this patch I don't add an automatic load => and default env is used
My expected sequence for the env commands was:
env select <target> env load ... env save
when user try to select a not compiled backend the command return "driver not found" (tested on sandbox, I will add unitary test for this point)
or when backend is not accessible by any priority, the select command return pritority not found manual test on sandbox, with env_locations[] = { ENVL_NOWHERE };
=> env select EXT4 Select Environment on EXT4: priority not found
I don't think that abort can occur for other commands because - the env backend for all priotity are initialized at boot (env_init use the same loop than env select)
- in all env function (env_load / env_reload / env_save it is tested again (to check if the backend was correctly initilializaed, if .init() retur 0= if (!env_has_inited(drv->location)) return -ENODEV;
- all direct acces (in env_get_char for example) are intercepted because gd->env_valid == ENV_INVALID => by default (without explicite reload) the default environent is used, exactly when the load failed. => "gd->env_addr" is never used
but user can also execute the sequence
env select <target> env save
and here the command behavior is strange.... because the default is forced on each select
env select EXT4 env load
env select MMC (reset on default environment) env save
=> I expect at the end of the sequence the EXT4 env was copied in MMC.... But in finally only the default environment is saved in MMC
I don't sure of the expected behavior by user for these commands....
what it is better solution ? add a option to select command ? force the load after the select ?
Patrick
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 patrick.delaunay@st.com ---
Changes in v3: - change env_get_location to avoid gd->env_load_prio modification
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 1372003018..ef886f6378 100644 --- a/board/sandbox/sandbox.c +++ b/board/sandbox/sandbox.c @@ -7,6 +7,7 @@ #include <cpu_func.h> #include <cros_ec.h> #include <dm.h> +#include <env_internal.h> #include <init.h> #include <led.h> #include <os.h> @@ -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 a2410b3368..b70272c0b0 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -81,6 +81,10 @@ CONFIG_OF_CONTROL=y CONFIG_OF_LIVE=y CONFIG_OF_HOSTFILE=y CONFIG_DEFAULT_DEVICE_TREE="sandbox64" +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 0c9e0b9f1f..715f5dc39d 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -90,6 +90,10 @@ CONFIG_OF_CONTROL=y CONFIG_OF_LIVE=y CONFIG_OF_HOSTFILE=y CONFIG_DEFAULT_DEVICE_TREE="sandbox" +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 bc4819f998..ce806270bd 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -64,6 +64,10 @@ CONFIG_AMIGA_PARTITION=y CONFIG_OF_CONTROL=y CONFIG_OF_HOSTFILE=y CONFIG_DEFAULT_DEVICE_TREE="sandbox" +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 64f2ed5102..ea11c9bded 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -80,6 +80,10 @@ CONFIG_SPL_OF_CONTROL=y CONFIG_OF_HOSTFILE=y CONFIG_DEFAULT_DEVICE_TREE="sandbox" CONFIG_SPL_OF_PLATDATA=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
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 patrick.delaunay@st.com ---
Changes in v3: - replace specific sandbox command by generic command 'env select' and 'env load' - change title "sandbox: support the change of env location"
Changes in v2: - change cmd_tbl_t to struct cmd_tbl
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 b70272c0b0..6b9b3a7b75 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 715f5dc39d..5b26fff7c9 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -35,6 +35,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 ce806270bd..ccbc6374e7 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 ea11c9bded..534f2d3239 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -35,6 +35,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
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 patrick.delaunay@st.com ---
Changes in v3: - replace specific sandbox command by generic command 'env select' and 'env load' - update after Stephen Warren comments
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)
On 6/25/20 1:59 AM, Patrick Delaunay wrote:
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.
Acked-by: Stephen Warren swarren@nvidia.com
A couple nits below; feel free not to skip them, but if you end up sending another revision for other reasons, may as well fix them.
diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
+from subprocess import call, check_call, CalledProcessError
I believe only CalledProcessError is used now.
+def test_env_ext4(state_test_env):
- """Test ENV in EXT4 on sandbox."""
- c = state_test_env.u_boot_console
- fs_img = ''
"None" would be a better unset value.
Split the function env_ext4_save to prepare the erase support.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
(no changes since v1)
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; }
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 patrick.delaunay@st.com ---
(no changes since v2)
Changes in v2: - use CONFIG_IS_ENABLED to set .erase (same as .save)
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, };
Add test for the erase command tested on ENV in EXT4.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v3: - replace sandbox command by generic command 'env load' in test_env
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 6b9b3a7b75..906b92de8c 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 5b26fff7c9..97953fda08 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -31,6 +31,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 ccbc6374e7..ba9f652689 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 534f2d3239..fb54c86bf6 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -32,6 +32,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
On 6/25/20 1:59 AM, Patrick Delaunay wrote:
Add test for the erase command tested on ENV in EXT4.
Acked-by: Stephen Warren swarren@nvidia.com
participants (4)
-
Patrick DELAUNAY -
Patrick Delaunay
-
Stephen Warren -
Tom Rini