* [U-Boot] [PATCH 01/11] env: fix ret not being set and fails when no env could have been init
2017-12-22 21:13 [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Quentin Schulz
@ 2017-12-22 21:13 ` Quentin Schulz
2017-12-22 21:13 ` [U-Boot] [PATCH 02/11] lib: hashtable: support whitelisting env variables Quentin Schulz
` (11 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Quentin Schulz @ 2017-12-22 21:13 UTC (permalink / raw)
To: u-boot
This patch is fixing things that most likely will be done by Maxime in
the v2 of his patch series for multiple environments support, one way or
the other.
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
env/env.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/env/env.c b/env/env.c
index 9b8b38c..5e70ddf 100644
--- a/env/env.c
+++ b/env/env.c
@@ -182,24 +182,25 @@ int env_init(void)
struct env_driver *drv;
int ret = -ENOENT;
int prio;
+ bool init = false;
for (prio = 0; (drv = env_driver_lookup(ENVO_INIT, prio)); prio++) {
- if (!drv->init || !drv->init())
+ if (drv->init)
+ ret = drv->init(drv);
+ else
+ ret = 0;
+
+ if (!ret) {
gd->env_has_init |= BIT(drv->location);
+ init = true;
+ }
debug("%s: Environment %s init done (ret=%d)\n", __func__,
drv->name, ret);
}
- if (!prio)
+ if (!prio || !init)
return -ENODEV;
- if (ret == -ENOENT) {
- gd->env_addr = (ulong)&default_environment[0];
- gd->env_valid = ENV_VALID;
-
- return 0;
- }
-
- return ret;
+ return 0;
}
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH 02/11] lib: hashtable: support whitelisting env variables
2017-12-22 21:13 [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Quentin Schulz
2017-12-22 21:13 ` [U-Boot] [PATCH 01/11] env: fix ret not being set and fails when no env could have been init Quentin Schulz
@ 2017-12-22 21:13 ` Quentin Schulz
2017-12-22 21:13 ` [U-Boot] [PATCH 03/11] env: add support for whitelisting variables from secondary environments Quentin Schulz
` (10 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Quentin Schulz @ 2017-12-22 21:13 UTC (permalink / raw)
To: u-boot
When passing the vars array to himport_r, variables in the array that
are not in the loaded environment are removed from the current
environment.
Of course, this isn't suitable for whitelisting some variables. Let's
introduce a whitelisting boolean that will not remove array variables
that are not in the loaded environment from the current environment.
The remaining of the code will rightfully override any variable that
already exists.
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
board/sunxi/board.c | 2 +-
cmd/nvedit.c | 2 +-
env/common.c | 6 +++---
include/search.h | 2 +-
lib/hashtable.c | 17 ++++++++++++++++-
5 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 8891961..aac4904 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -647,7 +647,7 @@ static void parse_spl_header(const uint32_t spl_addr)
* import -t" the string(s) at fel_script_address right away.
*/
himport_r(&env_htab, (char *)(uintptr_t)spl->fel_script_address,
- spl->fel_uEnv_length, '\n', H_NOCLEAR, 0, 0, NULL);
+ spl->fel_uEnv_length, '\n', H_NOCLEAR, 0, 0, NULL, 0);
return;
}
/* otherwise assume .scr format (mkimage-type script) */
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index a690d74..c00e1da 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -1079,7 +1079,7 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
}
if (himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR,
- crlf_is_lf, 0, NULL) == 0) {
+ crlf_is_lf, 0, NULL, false) == 0) {
pr_err("Environment import failed: errno = %d\n", errno);
return 1;
}
diff --git a/env/common.c b/env/common.c
index fb66432..9d97541 100644
--- a/env/common.c
+++ b/env/common.c
@@ -83,7 +83,7 @@ void set_default_env(const char *s)
if (himport_r(&env_htab, (char *)default_environment,
sizeof(default_environment), '\0', flags, 0,
- 0, NULL) == 0)
+ 0, NULL, false) == 0)
pr_err("Environment import failed: errno = %d\n", errno);
gd->flags |= GD_FLG_ENV_READY;
@@ -100,7 +100,7 @@ int set_default_vars(int nvars, char * const vars[])
*/
return himport_r(&env_htab, (const char *)default_environment,
sizeof(default_environment), '\0',
- H_NOCLEAR | H_INTERACTIVE, 0, nvars, vars);
+ H_NOCLEAR | H_INTERACTIVE, 0, nvars, vars, 0);
}
#ifdef CONFIG_ENV_AES
@@ -178,7 +178,7 @@ int env_import(const char *buf, int check)
}
if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0, 0,
- 0, NULL)) {
+ 0, NULL, false)) {
gd->flags |= GD_FLG_ENV_READY;
return 1;
}
diff --git a/include/search.h b/include/search.h
index df5d61c..ed2d9bf 100644
--- a/include/search.h
+++ b/include/search.h
@@ -101,7 +101,7 @@ extern ssize_t hexport_r(struct hsearch_data *__htab,
extern int himport_r(struct hsearch_data *__htab,
const char *__env, size_t __size, const char __sep,
int __flag, int __crlf_is_lf, int nvars,
- char * const vars[]);
+ char * const vars[], bool whitelisting);
/* Walk the whole table calling the callback on each element */
extern int hwalk_r(struct hsearch_data *__htab, int (*callback)(ENTRY *));
diff --git a/lib/hashtable.c b/lib/hashtable.c
index f088477..432fe14 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -772,11 +772,17 @@ static int drop_var_from_set(const char *name, int nvars, char * vars[])
*
* In theory, arbitrary separator characters can be used, but only
* '\0' and '\n' have really been tested.
+ *
+ * "whitelisting" makes the function to not remove variables from the
+ * env that were not found in "vars".
+ * if "whitelisting" is false, the function remove variables from env
+ * that were not found in "vars".
*/
int himport_r(struct hsearch_data *htab,
const char *env, size_t size, const char sep, int flag,
- int crlf_is_lf, int nvars, char * const vars[])
+ int crlf_is_lf, int nvars, char * const vars[],
+ bool whitelisting)
{
char *data, *sp, *dp, *name, *value;
char *localvars[nvars];
@@ -935,6 +941,14 @@ int himport_r(struct hsearch_data *htab,
free(data);
/* process variables which were not considered */
+ /*
+ * If we are importing variables from a second env and checking they're
+ * whitelisted, we don't want to delete the variables in current env
+ * because it was not in the whitelist.
+ */
+ if (whitelisting)
+ goto out;
+
for (i = 0; i < nvars; i++) {
if (localvars[i] == NULL)
continue;
@@ -952,6 +966,7 @@ int himport_r(struct hsearch_data *htab,
printf("WARNING: '%s' not in imported env, deleting it!\n", localvars[i]);
}
+out:
debug("INSERT: done\n");
return 1; /* everything OK */
}
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH 03/11] env: add support for whitelisting variables from secondary environments
2017-12-22 21:13 [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Quentin Schulz
2017-12-22 21:13 ` [U-Boot] [PATCH 01/11] env: fix ret not being set and fails when no env could have been init Quentin Schulz
2017-12-22 21:13 ` [U-Boot] [PATCH 02/11] lib: hashtable: support whitelisting env variables Quentin Schulz
@ 2017-12-22 21:13 ` Quentin Schulz
2017-12-26 11:21 ` Lukasz Majewski
2017-12-22 21:13 ` [U-Boot] [PATCH 04/11] env: make nowhere an env medium like the others Quentin Schulz
` (9 subsequent siblings)
12 siblings, 1 reply; 18+ messages in thread
From: Quentin Schulz @ 2017-12-22 21:13 UTC (permalink / raw)
To: u-boot
This patch makes it possible to have a first environment that will be
the base environment, secondary environment that will be pre-loaded and
have its variables passing through the whitelist "mask" to override
variables in the base environment.
The base environment will be the environment with the highest priority
(0 is highest) that can load and the next loadable environment will be
the secondary environment (with a lowest priority than the base
environment of course).
The whitelist "mask" is built with the content of space-separated list
of environment variables in ENV_VAR_WHITELIST_LIST in Kconfig.
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
env/Kconfig | 17 +++++++-
env/common.c | 7 +++-
env/env.c | 100 +++++++++++++++++++++++++++++++++++++++----
include/environment.h | 6 +++-
4 files changed, 123 insertions(+), 7 deletions(-)
diff --git a/env/Kconfig b/env/Kconfig
index 1952463..d57594d 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -438,4 +438,21 @@ config ENV_UBI_VOLUME
endif
+config ENV_VAR_WHITELIST
+ bool "Enable overriding some variables from secondary environments"
+ help
+ This allows overriding some variables from secondary environments.
+ After the first valid environment is loaded, a secondary environment
+ is pre-loaded and its variables that are present in the whitelist will
+ be added to the current environment or will override existing
+ variables.
+
+config ENV_VAR_WHITELIST_LIST
+ depends on ENV_VAR_WHITELIST
+ string "Whitelist of variables from secondary environments"
+ help
+ This defines the variables that are allowed to be overriden by
+ ones from secondary environments.
+ This is a list of environment variables that are space-separated.
+
endmenu
diff --git a/env/common.c b/env/common.c
index 9d97541..00c454d 100644
--- a/env/common.c
+++ b/env/common.c
@@ -177,8 +177,15 @@ int env_import(const char *buf, int check)
return ret;
}
+#ifdef CONFIG_ENV_VAR_WHITELIST
+ if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0',
+ whitelisting ? H_NOCLEAR : 0, 0,
+ whitelisting ? whitelist_nvars : 0,
+ whitelisting ? whitelist : NULL, whitelisting)) {
+#else
if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0, 0,
0, NULL, false)) {
+#endif
gd->flags |= GD_FLG_ENV_READY;
return 1;
}
diff --git a/env/env.c b/env/env.c
index 5e70ddf..43a62b8 100644
--- a/env/env.c
+++ b/env/env.c
@@ -7,9 +7,18 @@
#include <common.h>
#include <environment.h>
+#ifdef CONFIG_ENV_VAR_WHITELIST
+# include <malloc.h>
+#endif
DECLARE_GLOBAL_DATA_PTR;
+#ifdef CONFIG_ENV_VAR_WHITELIST
+char *const *whitelist;
+bool whitelisting;
+unsigned int whitelist_nvars;
+#endif
+
static struct env_driver *_env_driver_lookup(enum env_location loc)
{
struct env_driver *drv;
@@ -62,6 +71,31 @@ static enum env_location env_locations[] = {
static enum env_location env_load_location;
+#ifdef CONFIG_ENV_VAR_WHITELIST
+void env_create_whitelist(void)
+{
+ char **wl_vars;
+ char *wl = strchr(CONFIG_ENV_VAR_WHITELIST_LIST, ' ');
+ unsigned int nvars = 0;
+
+ while(wl) {
+ nvars++;
+ wl = strchr(wl + 1, ' ');
+ }
+
+ whitelist_nvars = nvars + 1;
+
+ wl_vars = malloc(sizeof(char *) * (nvars + 1));
+
+ wl_vars[nvars] = strtok(CONFIG_ENV_VAR_WHITELIST_LIST, " ");
+ while (nvars) {
+ wl_vars[--nvars] = strtok(NULL, " ");
+ }
+
+ whitelist = wl_vars;
+}
+#endif
+
__weak enum env_location env_get_location(enum env_operation op, int prio)
{
switch (op) {
@@ -74,7 +108,11 @@ __weak enum env_location env_get_location(enum env_operation op, int prio)
return env_load_location = env_locations[prio];
case ENVO_SAVE:
+#ifdef CONFIG_ENV_VAR_WHITELIST
+ return env_locations[prio];
+#else
return env_load_location;
+#endif
}
return ENVL_UNKNOWN;
@@ -130,10 +168,12 @@ int env_load(void)
{
struct env_driver *drv;
int prio;
+ int ret = 1;
+#ifdef CONFIG_ENV_VAR_WHITELIST
+ bool found = false;
+#endif
for (prio = 0; (drv = env_driver_lookup(ENVO_LOAD, prio)); prio++) {
- int ret;
-
if (!drv->load)
continue;
@@ -144,16 +184,52 @@ int env_load(void)
ret = drv->load();
printf("%s\n", ret ? "Failed" : "OK");
if (!ret)
- return 0;
+ break;
}
- return -ENODEV;
+ if (ret)
+ return -ENODEV;
+
+#ifdef CONFIG_ENV_VAR_WHITELIST
+ env_create_whitelist();
+
+ whitelisting = true;
+ /* When whitelisting, env from prio x+n will override env from prio x */
+ for (prio++; prio < ARRAY_SIZE(env_locations); prio++) {
+ drv = env_driver_lookup(ENVO_LOAD, prio);
+ if (!drv)
+ continue;
+
+ if (!drv->load)
+ continue;
+
+ printf("Overriding env variables with ones from %s env...",
+ __func__, drv->name);
+ ret = drv->load();
+ printf("%s\n", ret ? "Failed" : "OK");
+ if (!ret) {
+ found = true;
+ continue;
+ }
+ }
+
+out:
+ if (!found)
+ debug("%s: Couldn't find other valid env for whitelisting\n",
+ __func__);
+
+ whitelisting = false;
+#endif
+ return 0;
}
int env_save(void)
{
struct env_driver *drv;
int prio;
+#ifdef CONFIG_ENV_VAR_WHITELIST
+ bool saved = false;
+#endif
for (prio = 0; (drv = env_driver_lookup(ENVO_SAVE, prio)); prio++) {
int ret;
@@ -167,13 +243,23 @@ int env_save(void)
printf("Saving Environment to %s... ", drv->name);
ret = drv->save();
printf("%s\n", ret ? "Failed" : "OK");
+#ifdef CONFIG_ENV_VAR_WHITELIST
+ /* When whitelisting, we want to save to all media available */
+ if (!ret) {
+ saved = true;
+ continue;
+ }
+#else
if (!ret)
return 0;
-
- debug("%s: Environment %s failed to save (err=%d)\n", __func__,
- drv->name, ret);
+#endif
}
+#ifdef CONFIG_ENV_VAR_WHITELIST
+ if (saved)
+ return 0;
+#endif
+
return -ENODEV;
}
diff --git a/include/environment.h b/include/environment.h
index 7fa8b98..33e47ba 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -267,6 +267,12 @@ struct env_driver {
int (*init)(void);
};
+#ifdef CONFIG_ENV_VAR_WHITELIST
+extern char *const *whitelist;
+extern unsigned int whitelist_nvars;
+extern bool whitelisting;
+#endif
+
/* Declare a new environment location driver */
#define U_BOOT_ENV_LOCATION(__name) \
ll_entry_declare(struct env_driver, __name, env_driver)
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH 03/11] env: add support for whitelisting variables from secondary environments
2017-12-22 21:13 ` [U-Boot] [PATCH 03/11] env: add support for whitelisting variables from secondary environments Quentin Schulz
@ 2017-12-26 11:21 ` Lukasz Majewski
2018-01-03 11:07 ` Quentin Schulz
0 siblings, 1 reply; 18+ messages in thread
From: Lukasz Majewski @ 2017-12-26 11:21 UTC (permalink / raw)
To: u-boot
Hi Quentin,
> +config ENV_VAR_WHITELIST
> + bool "Enable overriding some variables from secondary
> environments"
> + help
> + This allows overriding some variables from secondary
> environments.
> + After the first valid environment is loaded, a secondary
> environment
> + is pre-loaded and its variables that are present in the
> whitelist will
> + be added to the current environment or will override
> existing
> + variables.
This description seems to do what is already done in the "env import"
command.
It also allows for setting env variables from HUSH prompt.
Maybe it would be possible to re-use or extend it to suits your needs?
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171226/96eb08db/attachment.sig>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 03/11] env: add support for whitelisting variables from secondary environments
2017-12-26 11:21 ` Lukasz Majewski
@ 2018-01-03 11:07 ` Quentin Schulz
0 siblings, 0 replies; 18+ messages in thread
From: Quentin Schulz @ 2018-01-03 11:07 UTC (permalink / raw)
To: u-boot
Hi Lukasz,
On 26/12/2017 12:21, Lukasz Majewski wrote:
> Hi Quentin,
>
>> +config ENV_VAR_WHITELIST
>> + bool "Enable overriding some variables from secondary
>> environments"
>> + help
>> + This allows overriding some variables from secondary
>> environments.
>> + After the first valid environment is loaded, a secondary
>> environment
>> + is pre-loaded and its variables that are present in the
>> whitelist will
>> + be added to the current environment or will override
>> existing
>> + variables.
>
> This description seems to do what is already done in the "env import"
> command.
>
Didn't know that one, thanks.
> It also allows for setting env variables from HUSH prompt.
>
> Maybe it would be possible to re-use or extend it to suits your needs?
>
It's pretty much everything I needed except the whitelisting of variables.
While it's possible for env export to take a list of variables to
export, for what I've seen, env import does not take a list of variables
to import.
It's needed since the env I'll import is unsafe. I'll have a look at
that and make an other RFC :)
Thanks for the review, very helpful,
Quentin
--
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180103/c0b3f77b/attachment.sig>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 04/11] env: make nowhere an env medium like the others
2017-12-22 21:13 [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Quentin Schulz
` (2 preceding siblings ...)
2017-12-22 21:13 ` [U-Boot] [PATCH 03/11] env: add support for whitelisting variables from secondary environments Quentin Schulz
@ 2017-12-22 21:13 ` Quentin Schulz
2017-12-22 21:13 ` [U-Boot] [PATCH 05/11] cmd: saveenv: enable the saveenv command when ENV_IS_NOWHERE is defined Quentin Schulz
` (8 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Quentin Schulz @ 2017-12-22 21:13 UTC (permalink / raw)
To: u-boot
Since we now allow the loading of different environment media by
priority, we can mimic the current fallback system with nowhere medium
by adding a load function to the nowhere driver (and make it the medium
with the lowest priority).
Nowhere then becomes a valid environment medium.
This also makes it possible to either enforce the value of some
variables by using nowhere medium when using it as secondary environment
or to completely enforce all the environment variables except a few from
a secondary environment when using it as base environment.
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
env/Kconfig | 12 ++----------
env/common.c | 2 +-
env/nowhere.c | 8 +++++++-
3 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/env/Kconfig b/env/Kconfig
index d57594d..f99a701 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -2,16 +2,6 @@ menu "Environment"
config ENV_IS_NOWHERE
bool "Environment is not stored"
- depends on !ENV_IS_IN_EEPROM
- depends on !ENV_IS_IN_FAT
- depends on !ENV_IS_IN_FLASH
- depends on !ENV_IS_IN_MMC
- depends on !ENV_IS_IN_NAND
- depends on !ENV_IS_IN_NVRAM
- depends on !ENV_IS_IN_ONENAND
- depends on !ENV_IS_IN_REMOTE
- depends on !ENV_IS_IN_SPI_FLASH
- depends on !ENV_IS_IN_UBI
default y
help
Define this if you don't want to or can't have an environment stored
@@ -19,6 +9,8 @@ config ENV_IS_NOWHERE
while U-Boot is running, but once U-Boot exits it will not be
stored. U-Boot will therefore always start up with a default
environment.
+ When whitelisting is enabled, define this to be able to use the
+ default environment as either base or secondary environment.
config ENV_IS_IN_EEPROM
bool "Environment in EEPROM"
diff --git a/env/common.c b/env/common.c
index 00c454d..c8d8993 100644
--- a/env/common.c
+++ b/env/common.c
@@ -279,7 +279,7 @@ void env_relocate(void)
env_htab.change_ok += gd->reloc_off;
#endif
if (gd->env_valid == ENV_INVALID) {
-#if defined(CONFIG_ENV_IS_NOWHERE) || defined(CONFIG_SPL_BUILD)
+#if defined(CONFIG_SPL_BUILD)
/* Environment not changable */
set_default_env(NULL);
#else
diff --git a/env/nowhere.c b/env/nowhere.c
index f654883..7a37909 100644
--- a/env/nowhere.c
+++ b/env/nowhere.c
@@ -15,6 +15,11 @@
DECLARE_GLOBAL_DATA_PTR;
+static int env_nowhere_load(void)
+{
+ return !env_import((char *)default_environment, 0);
+}
+
/*
* Because we only ever have the default environment available we must mark
* it as invalid.
@@ -22,7 +27,7 @@ DECLARE_GLOBAL_DATA_PTR;
static int env_nowhere_init(void)
{
gd->env_addr = (ulong)&default_environment[0];
- gd->env_valid = ENV_INVALID;
+ gd->env_valid = ENV_VALID;
return 0;
}
@@ -31,4 +36,5 @@ U_BOOT_ENV_LOCATION(nowhere) = {
.location = ENVL_NOWHERE,
.init = env_nowhere_init,
ENV_NAME("nowhere")
+ .load = env_nowhere_load,
};
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH 05/11] cmd: saveenv: enable the saveenv command when ENV_IS_NOWHERE is defined
2017-12-22 21:13 [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Quentin Schulz
` (3 preceding siblings ...)
2017-12-22 21:13 ` [U-Boot] [PATCH 04/11] env: make nowhere an env medium like the others Quentin Schulz
@ 2017-12-22 21:13 ` Quentin Schulz
2017-12-22 21:13 ` [U-Boot] [PATCH 06/11] env: add env_driver to env_driver functions' arguments Quentin Schulz
` (7 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Quentin Schulz @ 2017-12-22 21:13 UTC (permalink / raw)
To: u-boot
but another env medium is enabled too
Now that it is possible to have multiple environments at the same time,
nowhere included, enable the saveenv command when nowhere medium is
enabled but accompanied by another medium on which the saveenv command
is possible.
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
cmd/nvedit.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index c00e1da..84a7004 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -704,7 +704,19 @@ ulong env_get_ulong(const char *name, int base, ulong default_val)
}
#ifndef CONFIG_SPL_BUILD
-#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
+#if defined(CONFIG_CMD_SAVEENV) && (!defined(CONFIG_ENV_IS_NOWHERE) || (\
+ defined(CONFIG_ENV_IS_IN_EEPROM) || \
+ defined(CONFIG_ENV_IS_IN_FLASH) || \
+ defined(CONFIG_ENV_IS_IN_MMC) || \
+ defined(CONFIG_ENV_IS_IN_FAT) || \
+ defined(CONFIG_ENV_IS_IN_EXT4) || \
+ defined(CONFIG_ENV_IS_IN_NAND) || \
+ defined(CONFIG_ENV_IS_IN_NVRAM) || \
+ defined(CONFIG_ENV_IS_IN_ONENAND) || \
+ defined(CONFIG_ENV_IS_IN_SATA) || \
+ defined(CONFIG_ENV_IS_IN_SPI_FLASH) || \
+ defined(CONFIG_ENV_IS_IN_REMOTE) || \
+ defined(CONFIG_ENV_IS_IN_UBI)))
static int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
{
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH 06/11] env: add env_driver to env_driver functions' arguments
2017-12-22 21:13 [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Quentin Schulz
` (4 preceding siblings ...)
2017-12-22 21:13 ` [U-Boot] [PATCH 05/11] cmd: saveenv: enable the saveenv command when ENV_IS_NOWHERE is defined Quentin Schulz
@ 2017-12-22 21:13 ` Quentin Schulz
2017-12-22 21:13 ` [U-Boot] [PATCH 07/11] env: gd flags without ENV_READY is enough to discriminate in Quentin Schulz
` (6 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Quentin Schulz @ 2017-12-22 21:13 UTC (permalink / raw)
To: u-boot
Might be interesting to get some infos about the driver (e.g. its
location) when inside one of its functions.
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
env/eeprom.c | 6 +++---
env/env.c | 8 ++++----
env/ext4.c | 4 ++--
env/fat.c | 4 ++--
env/flash.c | 10 +++++-----
env/mmc.c | 6 +++---
env/nand.c | 8 ++++----
env/nowhere.c | 4 ++--
env/nvram.c | 8 ++++----
env/onenand.c | 4 ++--
env/remote.c | 6 +++---
env/sata.c | 4 ++--
env/sf.c | 8 ++++----
env/ubi.c | 8 ++++----
include/environment.h | 14 ++++++++++----
15 files changed, 54 insertions(+), 48 deletions(-)
diff --git a/env/eeprom.c b/env/eeprom.c
index 584379e..b0ffce4 100644
--- a/env/eeprom.c
+++ b/env/eeprom.c
@@ -61,7 +61,7 @@ static int eeprom_bus_write(unsigned dev_addr, unsigned offset,
return rcode;
}
-static int env_eeprom_get_char(int index)
+static int env_eeprom_get_char(struct env_driver *drv, int index)
{
uchar c;
unsigned int off = CONFIG_ENV_OFFSET;
@@ -76,7 +76,7 @@ static int env_eeprom_get_char(int index)
return c;
}
-static int env_eeprom_load(void)
+static int env_eeprom_load(struct env_driver *drv)
{
char buf_env[CONFIG_ENV_SIZE];
unsigned int off = CONFIG_ENV_OFFSET;
@@ -186,7 +186,7 @@ static int env_eeprom_load(void)
return 0;
}
-static int env_eeprom_save(void)
+static int env_eeprom_save(struct env_driver *drv)
{
env_t env_new;
int rc;
diff --git a/env/env.c b/env/env.c
index 43a62b8..7c98083 100644
--- a/env/env.c
+++ b/env/env.c
@@ -153,7 +153,7 @@ int env_get_char(int index)
if (!(gd->env_has_init & BIT(drv->location)))
continue;
- ret = drv->get_char(index);
+ ret = drv->get_char(drv, index);
if (!ret)
return 0;
@@ -181,7 +181,7 @@ int env_load(void)
continue;
printf("Loading Environment from %s... ", drv->name);
- ret = drv->load();
+ ret = drv->load(drv);
printf("%s\n", ret ? "Failed" : "OK");
if (!ret)
break;
@@ -205,7 +205,7 @@ int env_load(void)
printf("Overriding env variables with ones from %s env...",
__func__, drv->name);
- ret = drv->load();
+ ret = drv->load(drv);
printf("%s\n", ret ? "Failed" : "OK");
if (!ret) {
found = true;
@@ -241,7 +241,7 @@ int env_save(void)
continue;
printf("Saving Environment to %s... ", drv->name);
- ret = drv->save();
+ ret = drv->save(drv);
printf("%s\n", ret ? "Failed" : "OK");
#ifdef CONFIG_ENV_VAR_WHITELIST
/* When whitelisting, we want to save to all media available */
diff --git a/env/ext4.c b/env/ext4.c
index 6520221..77c0389 100644
--- a/env/ext4.c
+++ b/env/ext4.c
@@ -34,7 +34,7 @@
DECLARE_GLOBAL_DATA_PTR;
#ifdef CONFIG_CMD_SAVEENV
-static int env_ext4_save(void)
+static int env_ext4_save(struct env_driver *drv)
{
env_t env_new;
struct blk_desc *dev_desc = NULL;
@@ -75,7 +75,7 @@ static int env_ext4_save(void)
}
#endif /* CONFIG_CMD_SAVEENV */
-static int env_ext4_load(void)
+static int env_ext4_load(struct env_driver *drv)
{
ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
struct blk_desc *dev_desc = NULL;
diff --git a/env/fat.c b/env/fat.c
index 51c4ced..05b5971 100644
--- a/env/fat.c
+++ b/env/fat.c
@@ -34,7 +34,7 @@
DECLARE_GLOBAL_DATA_PTR;
#ifdef CMD_SAVEENV
-static int env_fat_save(void)
+static int env_fat_save(struct env_driver *drv)
{
env_t env_new;
struct blk_desc *dev_desc = NULL;
@@ -73,7 +73,7 @@ static int env_fat_save(void)
#endif /* CMD_SAVEENV */
#ifdef LOADENV
-static int env_fat_load(void)
+static int env_fat_load(struct env_driver *drv)
{
ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
struct blk_desc *dev_desc = NULL;
diff --git a/env/flash.c b/env/flash.c
index bac10ff..45e58b4 100644
--- a/env/flash.c
+++ b/env/flash.c
@@ -71,7 +71,7 @@ static ulong __maybe_unused end_addr_new =
#ifdef CONFIG_ENV_ADDR_REDUND
#ifdef INITENV
-static int env_flash_init(void)
+static int env_flash_init(struct env_driver *drv)
{
int crc1_ok = 0, crc2_ok = 0;
@@ -117,7 +117,7 @@ static int env_flash_init(void)
#endif
#ifdef CMD_SAVEENV
-static int env_flash_save(void)
+static int env_flash_save(struct env_driver *drv)
{
env_t env_new;
char *saved_data = NULL;
@@ -222,7 +222,7 @@ done:
#else /* ! CONFIG_ENV_ADDR_REDUND */
#ifdef INITENV
-static int env_flash_init(void)
+static int env_flash_init(struct env_driver *drv)
{
if (crc32(0, env_ptr->data, ENV_SIZE) == env_ptr->crc) {
gd->env_addr = (ulong)&(env_ptr->data);
@@ -237,7 +237,7 @@ static int env_flash_init(void)
#endif
#ifdef CMD_SAVEENV
-static int env_flash_save(void)
+static int env_flash_save(struct env_driver *drv)
{
env_t env_new;
int rc = 1;
@@ -308,7 +308,7 @@ done:
#endif /* CONFIG_ENV_ADDR_REDUND */
#ifdef LOADENV
-static int env_flash_load(void)
+static int env_flash_load(struct env_driver *drv)
{
#ifdef CONFIG_ENV_ADDR_REDUND
if (gd->env_addr != (ulong)&(flash_addr->data)) {
diff --git a/env/mmc.c b/env/mmc.c
index 885e000..64d1404 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -143,7 +143,7 @@ static inline int write_env(struct mmc *mmc, unsigned long size,
return (n == blk_cnt) ? 0 : -1;
}
-static int env_mmc_save(void)
+static int env_mmc_save(struct env_driver *drv)
{
ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
int dev = mmc_get_env_dev();
@@ -206,7 +206,7 @@ static inline int read_env(struct mmc *mmc, unsigned long size,
}
#ifdef CONFIG_ENV_OFFSET_REDUND
-static int env_mmc_load(void)
+static int env_mmc_load(struct env_driver *drv)
{
#if !defined(ENV_IS_EMBEDDED)
struct mmc *mmc;
@@ -268,7 +268,7 @@ err:
return ret;
}
#else /* ! CONFIG_ENV_OFFSET_REDUND */
-static int env_mmc_load(void)
+static int env_mmc_load(struct env_driver *drv)
{
#if !defined(ENV_IS_EMBEDDED)
ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
diff --git a/env/nand.c b/env/nand.c
index 8058b55..6ed3c26 100644
--- a/env/nand.c
+++ b/env/nand.c
@@ -62,7 +62,7 @@ DECLARE_GLOBAL_DATA_PTR;
* This way the SPL loads not only the U-Boot image from NAND but
* also the environment.
*/
-static int env_nand_init(void)
+static int env_nand_init(struct env_driver *drv)
{
#if defined(ENV_IS_EMBEDDED) || defined(CONFIG_NAND_ENV_DST)
int crc1_ok = 0, crc2_ok = 0;
@@ -183,7 +183,7 @@ static int erase_and_write_env(const struct nand_env_location *location,
return ret;
}
-static int env_nand_save(void)
+static int env_nand_save(struct env_driver *drv)
{
int ret = 0;
ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
@@ -315,7 +315,7 @@ int get_nand_env_oob(struct mtd_info *mtd, unsigned long *result)
#endif
#ifdef CONFIG_ENV_OFFSET_REDUND
-static int env_nand_load(void)
+static int env_nand_load(struct env_driver *drv)
{
#if defined(ENV_IS_EMBEDDED)
return 0;
@@ -368,7 +368,7 @@ done:
* device i.e., nand_dev_desc + 0. This is also the behaviour using
* the new NAND code.
*/
-static int env_nand_load(void)
+static int env_nand_load(struct env_driver *drv)
{
#if !defined(ENV_IS_EMBEDDED)
int ret;
diff --git a/env/nowhere.c b/env/nowhere.c
index 7a37909..fb6ea9e 100644
--- a/env/nowhere.c
+++ b/env/nowhere.c
@@ -15,7 +15,7 @@
DECLARE_GLOBAL_DATA_PTR;
-static int env_nowhere_load(void)
+static int env_nowhere_load(struct env_driver *drv)
{
return !env_import((char *)default_environment, 0);
}
@@ -24,7 +24,7 @@ static int env_nowhere_load(void)
* Because we only ever have the default environment available we must mark
* it as invalid.
*/
-static int env_nowhere_init(void)
+static int env_nowhere_init(struct env_driver *drv)
{
gd->env_addr = (ulong)&default_environment[0];
gd->env_valid = ENV_VALID;
diff --git a/env/nvram.c b/env/nvram.c
index c8b3475..aad341d 100644
--- a/env/nvram.c
+++ b/env/nvram.c
@@ -41,7 +41,7 @@ env_t *env_ptr = (env_t *)CONFIG_ENV_ADDR;
#endif
#ifdef CONFIG_SYS_NVRAM_ACCESS_ROUTINE
-static int env_nvram_get_char(int index)
+static int env_nvram_get_char(struct env_driver *drv, int index)
{
uchar c;
@@ -51,7 +51,7 @@ static int env_nvram_get_char(int index)
}
#endif
-static int env_nvram_load(void)
+static int env_nvram_load(struct env_driver *drv)
{
char buf[CONFIG_ENV_SIZE];
@@ -65,7 +65,7 @@ static int env_nvram_load(void)
return 0;
}
-static int env_nvram_save(void)
+static int env_nvram_save(struct env_driver *drv)
{
env_t env_new;
int rcode = 0;
@@ -88,7 +88,7 @@ static int env_nvram_save(void)
*
* We are still running from ROM, so data use is limited
*/
-static int env_nvram_init(void)
+static int env_nvram_init(struct env_driver *drv)
{
#if defined(CONFIG_SYS_NVRAM_ACCESS_ROUTINE)
ulong crc;
diff --git a/env/onenand.c b/env/onenand.c
index 2e3045c..e633ebe 100644
--- a/env/onenand.c
+++ b/env/onenand.c
@@ -26,7 +26,7 @@
DECLARE_GLOBAL_DATA_PTR;
-static int env_onenand_load(void)
+static int env_onenand_load(struct env_driver *drv)
{
struct mtd_info *mtd = &onenand_mtd;
#ifdef CONFIG_ENV_ADDR_FLEX
@@ -63,7 +63,7 @@ static int env_onenand_load(void)
return rc ? 0 : -EIO;
}
-static int env_onenand_save(void)
+static int env_onenand_save(struct env_driver *drv)
{
env_t env_new;
int ret;
diff --git a/env/remote.c b/env/remote.c
index c013fdd..b004964 100644
--- a/env/remote.c
+++ b/env/remote.c
@@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR;
#define CONFIG_ENV_OFFSET 0
#endif
-static int env_remote_init(void)
+static int env_remote_init(struct env_driver *drv)
{
if (crc32(0, env_ptr->data, ENV_SIZE) == env_ptr->crc) {
gd->env_addr = (ulong)&(env_ptr->data);
@@ -35,7 +35,7 @@ static int env_remote_init(void)
}
#ifdef CONFIG_CMD_SAVEENV
-static int env_remote_save(void)
+static int env_remote_save(struct env_driver *drv)
{
#ifdef CONFIG_SRIO_PCIE_BOOT_SLAVE
printf("Can not support the 'saveenv' when boot from SRIO or PCIE!\n");
@@ -46,7 +46,7 @@ static int env_remote_save(void)
}
#endif /* CONFIG_CMD_SAVEENV */
-static int env_remote_load(void)
+static int env_remote_load(struct env_driver *drv)
{
#ifndef ENV_IS_EMBEDDED
env_import((char *)env_ptr, 1);
diff --git a/env/sata.c b/env/sata.c
index a770297..93289be 100644
--- a/env/sata.c
+++ b/env/sata.c
@@ -45,7 +45,7 @@ static inline int write_env(struct blk_desc *sata, unsigned long size,
return (n == blk_cnt) ? 0 : -1;
}
-static int env_sata_save(void)
+static int env_sata_save(struct env_driver *drv)
{
ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
struct blk_desc *sata = NULL;
@@ -91,7 +91,7 @@ static inline int read_env(struct blk_desc *sata, unsigned long size,
return (n == blk_cnt) ? 0 : -1;
}
-static void env_sata_load(void)
+static void env_sata_load(struct env_driver *drv)
{
ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
struct blk_desc *sata = NULL;
diff --git a/env/sf.c b/env/sf.c
index e51b1ae..e811cde 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -82,7 +82,7 @@ static int setup_flash_device(void)
#if defined(CONFIG_ENV_OFFSET_REDUND)
#ifdef CMD_SAVEENV
-static int env_sf_save(void)
+static int env_sf_save(struct env_driver *drv)
{
env_t env_new;
char *saved_buffer = NULL, flag = OBSOLETE_FLAG;
@@ -162,7 +162,7 @@ static int env_sf_save(void)
}
#endif /* CMD_SAVEENV */
-static int env_sf_load(void)
+static int env_sf_load(struct env_driver *drv)
{
int ret;
int crc1_ok = 0, crc2_ok = 0;
@@ -251,7 +251,7 @@ out:
}
#else
#ifdef CMD_SAVEENV
-static int env_sf_save(void)
+static int env_sf_save(struct env_driver *drv)
{
u32 saved_size, saved_offset, sector;
char *saved_buffer = NULL;
@@ -312,7 +312,7 @@ static int env_sf_save(void)
}
#endif /* CMD_SAVEENV */
-static int env_sf_load(void)
+static int env_sf_load(struct env_driver *drv)
{
int ret;
char *buf = NULL;
diff --git a/env/ubi.c b/env/ubi.c
index 1c4653d..a649999 100644
--- a/env/ubi.c
+++ b/env/ubi.c
@@ -20,7 +20,7 @@ DECLARE_GLOBAL_DATA_PTR;
#ifdef CONFIG_CMD_SAVEENV
#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
-static int env_ubi_save(void)
+static int env_ubi_save(struct env_driver *drv)
{
ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
int ret;
@@ -62,7 +62,7 @@ static int env_ubi_save(void)
return 0;
}
#else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
-static int env_ubi_save(void)
+static int env_ubi_save(struct env_driver *drv)
{
ALLOC_CACHE_ALIGN_BUFFER(env_t, env_new, 1);
int ret;
@@ -91,7 +91,7 @@ static int env_ubi_save(void)
#endif /* CONFIG_CMD_SAVEENV */
#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
-static int env_ubi_load(void)
+static int env_ubi_load(struct env_driver *drv)
{
ALLOC_CACHE_ALIGN_BUFFER(char, env1_buf, CONFIG_ENV_SIZE);
ALLOC_CACHE_ALIGN_BUFFER(char, env2_buf, CONFIG_ENV_SIZE);
@@ -135,7 +135,7 @@ static int env_ubi_load(void)
return 0;
}
#else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
-static int env_ubi_load(void)
+static int env_ubi_load(struct env_driver *drv)
{
ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
diff --git a/include/environment.h b/include/environment.h
index 33e47ba..34a0d5e 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -222,6 +222,8 @@ enum env_operation {
ENVO_SAVE,
};
+struct env_driver;
+
struct env_driver {
const char *name;
enum env_location location;
@@ -232,10 +234,11 @@ struct env_driver {
* This method is optional. If not provided, a default implementation
* will read from gd->env_addr.
*
+ * @drv: the driver whose get_char function is about to be called
* @index: Index of character to read (0=first)
* @return character read, or -ve on error
*/
- int (*get_char)(int index);
+ int (*get_char)(struct env_driver *drv, int index);
/**
* load() - Load the environment from storage
@@ -243,28 +246,31 @@ struct env_driver {
* This method is optional. If not provided, no environment will be
* loaded.
*
+ * @drv: the driver whose load function is about to be called
* @return 0 if OK, -ve on error
*/
- int (*load)(void);
+ int (*load)(struct env_driver *drv);
/**
* save() - Save the environment to storage
*
* This method is required for 'saveenv' to work.
*
+ * @drv: the driver whose save function is about to be called
* @return 0 if OK, -ve on error
*/
- int (*save)(void);
+ int (*save)(struct env_driver *drv);
/**
* init() - Set up the initial pre-relocation environment
*
* This method is optional.
*
+ * @drv: the driver whose init function is about to be called
* @return 0 if OK, -ENOENT if no initial environment could be found,
* other -ve on error
*/
- int (*init)(void);
+ int (*init)(struct env_driver *drv);
};
#ifdef CONFIG_ENV_VAR_WHITELIST
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH 07/11] env: gd flags without ENV_READY is enough to discriminate in
2017-12-22 21:13 [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Quentin Schulz
` (5 preceding siblings ...)
2017-12-22 21:13 ` [U-Boot] [PATCH 06/11] env: add env_driver to env_driver functions' arguments Quentin Schulz
@ 2017-12-22 21:13 ` Quentin Schulz
2017-12-22 21:13 ` [U-Boot] [PATCH 08/11] env: add env_driver parameter to env_import_redund Quentin Schulz
` (5 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Quentin Schulz @ 2017-12-22 21:13 UTC (permalink / raw)
To: u-boot
env_get_default
To prepare the removal of env_valid in an upcoming patch, and since the
ENV_READY flag in gd is enough to discriminate in env_get_char, let's
update env_get_char to return the variable in the default environment
when ENV_READY flag is not set and remove env_valid setting and getting
from env_get_default.
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
env/common.c | 3 ---
env/env.c | 2 +-
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/env/common.c b/env/common.c
index c8d8993..75b8334 100644
--- a/env/common.c
+++ b/env/common.c
@@ -47,14 +47,11 @@ int env_get_yesno(const char *var)
char *env_get_default(const char *name)
{
char *ret_val;
- unsigned long really_valid = gd->env_valid;
unsigned long real_gd_flags = gd->flags;
/* Pretend that the image is bad. */
gd->flags &= ~GD_FLG_ENV_READY;
- gd->env_valid = ENV_INVALID;
ret_val = env_get(name);
- gd->env_valid = really_valid;
gd->flags = real_gd_flags;
return ret_val;
}
diff --git a/env/env.c b/env/env.c
index 7c98083..2565e7a 100644
--- a/env/env.c
+++ b/env/env.c
@@ -141,7 +141,7 @@ int env_get_char(int index)
struct env_driver *drv;
int prio;
- if (gd->env_valid == ENV_INVALID)
+ if (!(gd->flags & GD_FLG_ENV_READY))
return default_environment[index];
for (prio = 0; (drv = env_driver_lookup(ENVO_GET_CHAR, prio)); prio++) {
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH 08/11] env: add env_driver parameter to env_import_redund
2017-12-22 21:13 [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Quentin Schulz
` (6 preceding siblings ...)
2017-12-22 21:13 ` [U-Boot] [PATCH 07/11] env: gd flags without ENV_READY is enough to discriminate in Quentin Schulz
@ 2017-12-22 21:13 ` Quentin Schulz
2017-12-22 21:13 ` [U-Boot] [PATCH 09/11] env: make env_locations a global variable Quentin Schulz
` (4 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Quentin Schulz @ 2017-12-22 21:13 UTC (permalink / raw)
To: u-boot
To prepare for an upcoming patch that uses the location of the driver
used when doing an env_import_redund, add env_driver as an argument of
env_import_redund so that it can be used within.
Update all calls to this function and the prototype.
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
env/common.c | 2 +-
env/nand.c | 2 +-
env/ubi.c | 2 +-
include/environment.h | 3 ++-
4 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/env/common.c b/env/common.c
index 75b8334..24cf41b 100644
--- a/env/common.c
+++ b/env/common.c
@@ -197,7 +197,7 @@ int env_import(const char *buf, int check)
#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
static unsigned char env_flags;
-int env_import_redund(const char *buf1, const char *buf2)
+int env_import_redund(struct env_driver *drv, const char *buf1, const char *buf2)
{
int crc1_ok, crc2_ok;
env_t *ep, *tmp_env1, *tmp_env2;
diff --git a/env/nand.c b/env/nand.c
index 6ed3c26..49e506e 100644
--- a/env/nand.c
+++ b/env/nand.c
@@ -352,7 +352,7 @@ static int env_nand_load(struct env_driver *drv)
gd->env_valid = ENV_REDUND;
env_import((char *)tmp_env2, 1);
} else {
- env_import_redund((char *)tmp_env1, (char *)tmp_env2);
+ env_import_redund(drv, (char *)tmp_env1, (char *)tmp_env2);
}
done:
diff --git a/env/ubi.c b/env/ubi.c
index a649999..51b61eb 100644
--- a/env/ubi.c
+++ b/env/ubi.c
@@ -130,7 +130,7 @@ static int env_ubi_load(struct env_driver *drv)
CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME_REDUND);
}
- env_import_redund((char *)tmp_env1, (char *)tmp_env2);
+ env_import_redund(drv, (char *)tmp_env1, (char *)tmp_env2);
return 0;
}
diff --git a/include/environment.h b/include/environment.h
index 34a0d5e..1e9254f 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -318,7 +318,8 @@ int env_export(env_t *env_out);
#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
/* Select and import one of two redundant environments */
-int env_import_redund(const char *buf1, const char *buf2);
+int env_import_redund(struct env_driver *drv, const char *buf1,
+ const char *buf2);
#endif
/**
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH 09/11] env: make env_locations a global variable
2017-12-22 21:13 [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Quentin Schulz
` (7 preceding siblings ...)
2017-12-22 21:13 ` [U-Boot] [PATCH 08/11] env: add env_driver parameter to env_import_redund Quentin Schulz
@ 2017-12-22 21:13 ` Quentin Schulz
2017-12-22 21:13 ` [U-Boot] [PATCH 10/11] env: introducing env_info struct for storing info per env Quentin Schulz
` (3 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Quentin Schulz @ 2017-12-22 21:13 UTC (permalink / raw)
To: u-boot
Make env_locations a global variable so that it can be used in env
drivers.
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
env/env.c | 2 +-
include/environment.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/env/env.c b/env/env.c
index 2565e7a..ccddc2a 100644
--- a/env/env.c
+++ b/env/env.c
@@ -35,7 +35,7 @@ static struct env_driver *_env_driver_lookup(enum env_location loc)
return NULL;
}
-static enum env_location env_locations[] = {
+enum env_location env_locations[] = {
#ifdef CONFIG_ENV_IS_IN_EEPROM
ENVL_EEPROM,
#endif
diff --git a/include/environment.h b/include/environment.h
index 1e9254f..dd6450c 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -215,6 +215,8 @@ enum env_location {
ENVL_UNKNOWN,
};
+extern enum env_location env_locations[];
+
enum env_operation {
ENVO_GET_CHAR,
ENVO_INIT,
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH 10/11] env: introducing env_info struct for storing info per env
2017-12-22 21:13 [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Quentin Schulz
` (8 preceding siblings ...)
2017-12-22 21:13 ` [U-Boot] [PATCH 09/11] env: make env_locations a global variable Quentin Schulz
@ 2017-12-22 21:13 ` Quentin Schulz
2017-12-22 21:13 ` [U-Boot] [PATCH 11/11] env: store information about each environment in gd Quentin Schulz
` (2 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Quentin Schulz @ 2017-12-22 21:13 UTC (permalink / raw)
To: u-boot
To introduce the ability to use multiple environments at the same time,
it is required to store a few information about each of the environment.
Each environment has a different env_addr, "validity" (redund or main)
and status of init (failed or succeeded).
This structure is meant to be used in include/asm-generic/global_data.h
and thus needs to be put out of the #ifndef DO_DEPS_ONLY.
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
include/environment.h | 34 ++++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/include/environment.h b/include/environment.h
index dd6450c..14df43f 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -183,20 +183,6 @@ extern uint mmc_get_env_part(struct mmc *mmc);
# endif
#endif
-#ifndef DO_DEPS_ONLY
-
-#include <env_attr.h>
-#include <env_callback.h>
-#include <env_flags.h>
-#include <search.h>
-
-/* Value for environment validity */
-enum env_valid {
- ENV_INVALID, /* No valid environment */
- ENV_VALID, /* First or only environment is valid */
- ENV_REDUND, /* Redundant environment is valid */
-};
-
enum env_location {
ENVL_EEPROM,
ENVL_EXT4,
@@ -215,6 +201,26 @@ enum env_location {
ENVL_UNKNOWN,
};
+/* Value for environment validity */
+enum env_valid {
+ ENV_INVALID, /* No valid environment */
+ ENV_VALID, /* First or only environment is valid */
+ ENV_REDUND, /* Redundant environment is valid */
+};
+
+struct env_info {
+ unsigned long env_addr;
+ enum env_valid env_valid;
+ bool has_init;
+};
+
+#ifndef DO_DEPS_ONLY
+
+#include <env_attr.h>
+#include <env_callback.h>
+#include <env_flags.h>
+#include <search.h>
+
extern enum env_location env_locations[];
enum env_operation {
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH 11/11] env: store information about each environment in gd
2017-12-22 21:13 [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Quentin Schulz
` (9 preceding siblings ...)
2017-12-22 21:13 ` [U-Boot] [PATCH 10/11] env: introducing env_info struct for storing info per env Quentin Schulz
@ 2017-12-22 21:13 ` Quentin Schulz
2017-12-26 11:25 ` [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Lukasz Majewski
2017-12-29 3:13 ` Simon Glass
12 siblings, 0 replies; 18+ messages in thread
From: Quentin Schulz @ 2017-12-22 21:13 UTC (permalink / raw)
To: u-boot
To store the "validity" (redund or main), the env_addr (different when
redund or main env is used) and the init status of the environment for
each environment, an array of env_info structure is created in the
global data pointer.
Move all calls to env_addr, env_valid and env_has_init to the new model
in the different env drivers.
Note that env_addr and env_valid defined in board file are intentionally
left untouched and thus, will break compilation for a few boards.
The question is: why do we need to set env_addr and env_valid in board
files? In addition, env_addr is never used (only when using redundant
env and it's set before being used).
Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
common/board_r.c | 8 ++++-
env/common.c | 25 ++++++++++-------
env/eeprom.c | 34 +++++++++++------------
env/env.c | 13 +++++----
env/flash.c | 48 ++++++++++++++++----------------
env/mmc.c | 8 ++---
env/nand.c | 36 ++++++++++++------------
env/nowhere.c | 4 +--
env/nvram.c | 10 +++----
env/onenand.c | 2 +-
env/remote.c | 4 +--
env/sf.c | 26 ++++++++---------
env/ubi.c | 4 +--
include/asm-generic/global_data.h | 5 +--
14 files changed, 121 insertions(+), 106 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c
index a3b9bfb..512e109 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -125,6 +125,10 @@ __weak int fixup_cpu(void)
static int initr_reloc_global_data(void)
{
+#ifdef CONFIG_SYS_EXTRA_ENV_RELOC
+ int i;
+#endif
+
#ifdef __ARM__
monitor_flash_len = _end - __image_copy_start;
#elif defined(CONFIG_NDS32)
@@ -154,7 +158,9 @@ static int initr_reloc_global_data(void)
* in SRAM mode and initialize that cache from SRAM mode back to being
* a cache in cpu_init_r.
*/
- gd->env_addr += gd->relocaddr - CONFIG_SYS_MONITOR_BASE;
+ for (i = 0; i < ENVL_COUNT; i++)
+ gd->env_infos[i].env_addr += gd->relocaddr -
+ CONFIG_SYS_MONITOR_BASE;
#endif
#ifdef CONFIG_OF_EMBED
/*
diff --git a/env/common.c b/env/common.c
index 24cf41b..72359fe 100644
--- a/env/common.c
+++ b/env/common.c
@@ -214,24 +214,24 @@ int env_import_redund(struct env_driver *drv, const char *buf1, const char *buf2
set_default_env("!bad CRC");
return 0;
} else if (crc1_ok && !crc2_ok) {
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
} else if (!crc1_ok && crc2_ok) {
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
} else {
/* both ok - check serial */
if (tmp_env1->flags == 255 && tmp_env2->flags == 0)
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
else if (tmp_env2->flags == 255 && tmp_env1->flags == 0)
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
else if (tmp_env1->flags > tmp_env2->flags)
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
else if (tmp_env2->flags > tmp_env1->flags)
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
else /* flags are equal - almost impossible */
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
}
- if (gd->env_valid == ENV_VALID)
+ if (gd->env_infos[drv->location].env_valid == ENV_VALID)
ep = tmp_env1;
else
ep = tmp_env2;
@@ -271,11 +271,18 @@ int env_export(env_t *env_out)
void env_relocate(void)
{
+ int i;
+
#if defined(CONFIG_NEEDS_MANUAL_RELOC)
env_reloc();
env_htab.change_ok += gd->reloc_off;
+
#endif
- if (gd->env_valid == ENV_INVALID) {
+ for(i = 0; i < ENVL_COUNT; i++)
+ if (gd->env_infos[i].has_init)
+ break;
+
+ if (i == ENVL_COUNT) {
#if defined(CONFIG_SPL_BUILD)
/* Environment not changable */
set_default_env(NULL);
diff --git a/env/eeprom.c b/env/eeprom.c
index b0ffce4..b48aa97 100644
--- a/env/eeprom.c
+++ b/env/eeprom.c
@@ -67,7 +67,7 @@ static int env_eeprom_get_char(struct env_driver *drv, int index)
unsigned int off = CONFIG_ENV_OFFSET;
#ifdef CONFIG_ENV_OFFSET_REDUND
- if (gd->env_valid == ENV_REDUND)
+ if (gd->env_infos[drv->location].env_valid == ENV_REDUND)
off = CONFIG_ENV_OFFSET_REDUND;
#endif
eeprom_bus_read(CONFIG_SYS_DEF_EEPROM_ADDR,
@@ -121,24 +121,24 @@ static int env_eeprom_load(struct env_driver *drv)
}
if (!crc_ok[0] && !crc_ok[1]) {
- gd->env_addr = 0;
- gd->env_valid = ENV_INVALID;
+ gd->env_infos[drv->location].env_addr = 0;
+ gd->env_infos[drv->location].env_valid = ENV_INVALID;
} else if (crc_ok[0] && !crc_ok[1]) {
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
} else if (!crc_ok[0] && crc_ok[1]) {
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
} else {
/* both ok - check serial */
if (flags[0] == ACTIVE_FLAG && flags[1] == OBSOLETE_FLAG)
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
else if (flags[0] == OBSOLETE_FLAG && flags[1] == ACTIVE_FLAG)
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
else if (flags[0] == 0xFF && flags[1] == 0)
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
else if (flags[1] == 0xFF && flags[0] == 0)
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
else /* flags are equal - almost impossible */
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
}
#else /* CONFIG_ENV_OFFSET_REDUND */
@@ -166,15 +166,15 @@ static int env_eeprom_load(struct env_driver *drv)
}
if (crc == new) {
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
} else {
- gd->env_valid = ENV_INVALID;
+ gd->env_infos[drv->location].env_valid = ENV_INVALID;
}
#endif /* CONFIG_ENV_OFFSET_REDUND */
off = CONFIG_ENV_OFFSET;
#ifdef CONFIG_ENV_OFFSET_REDUND
- if (gd->env_valid == ENV_REDUND)
+ if (gd->env_infos[drv->location].env_valid == ENV_REDUND)
off = CONFIG_ENV_OFFSET_REDUND;
#endif
@@ -201,7 +201,7 @@ static int env_eeprom_save(struct env_driver *drv)
return rc;
#ifdef CONFIG_ENV_OFFSET_REDUND
- if (gd->env_valid == ENV_VALID) {
+ if (gd->env_infos[drv->location].env_valid == ENV_VALID) {
off = CONFIG_ENV_OFFSET_REDUND;
off_red = CONFIG_ENV_OFFSET;
}
@@ -218,10 +218,10 @@ static int env_eeprom_save(struct env_driver *drv)
off_red + offsetof(env_t, flags),
(uchar *)&flag_obsolete, 1);
- if (gd->env_valid == ENV_VALID)
- gd->env_valid = ENV_REDUND;
+ if (gd->env_infos[drv->location].env_valid == ENV_VALID)
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
else
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
}
#endif
return rc;
diff --git a/env/env.c b/env/env.c
index ccddc2a..01233ce 100644
--- a/env/env.c
+++ b/env/env.c
@@ -150,7 +150,7 @@ int env_get_char(int index)
if (!drv->get_char)
continue;
- if (!(gd->env_has_init & BIT(drv->location)))
+ if (!gd->env_infos[drv->location].has_init)
continue;
ret = drv->get_char(drv, index);
@@ -177,7 +177,7 @@ int env_load(void)
if (!drv->load)
continue;
- if (!(gd->env_has_init & BIT(drv->location)))
+ if (!gd->env_infos[drv->location].has_init)
continue;
printf("Loading Environment from %s... ", drv->name);
@@ -203,7 +203,10 @@ int env_load(void)
if (!drv->load)
continue;
- printf("Overriding env variables with ones from %s env...",
+ if (!gd->env_infos[drv->location].has_init)
+ continue;
+
+ printf("%s: Overriding env variables with ones from %s env...",
__func__, drv->name);
ret = drv->load(drv);
printf("%s\n", ret ? "Failed" : "OK");
@@ -237,7 +240,7 @@ int env_save(void)
if (!drv->save)
continue;
- if (!(gd->env_has_init & BIT(drv->location)))
+ if (!gd->env_infos[drv->location].has_init)
continue;
printf("Saving Environment to %s... ", drv->name);
@@ -277,7 +280,7 @@ int env_init(void)
ret = 0;
if (!ret) {
- gd->env_has_init |= BIT(drv->location);
+ gd->env_infos[drv->location].has_init = true;
init = true;
}
diff --git a/env/flash.c b/env/flash.c
index 45e58b4..1c7666a 100644
--- a/env/flash.c
+++ b/env/flash.c
@@ -87,29 +87,29 @@ static int env_flash_init(struct env_driver *drv)
crc32(0, flash_addr_new->data, ENV_SIZE) == flash_addr_new->crc;
if (crc1_ok && !crc2_ok) {
- gd->env_addr = addr1;
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_addr = addr1;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
} else if (!crc1_ok && crc2_ok) {
- gd->env_addr = addr2;
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_addr = addr2;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
} else if (!crc1_ok && !crc2_ok) {
- gd->env_addr = addr_default;
- gd->env_valid = ENV_INVALID;
+ gd->env_infos[drv->location].env_addr = addr_default;
+ gd->env_infos[drv->location].env_valid = ENV_INVALID;
} else if (flag1 == ACTIVE_FLAG && flag2 == OBSOLETE_FLAG) {
- gd->env_addr = addr1;
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_addr = addr1;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
} else if (flag1 == OBSOLETE_FLAG && flag2 == ACTIVE_FLAG) {
- gd->env_addr = addr2;
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_addr = addr2;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
} else if (flag1 == flag2) {
- gd->env_addr = addr1;
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_addr = addr1;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
} else if (flag1 == 0xFF) {
- gd->env_addr = addr1;
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_addr = addr1;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
} else if (flag2 == 0xFF) {
- gd->env_addr = addr2;
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_addr = addr2;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
}
return 0;
@@ -225,13 +225,13 @@ done:
static int env_flash_init(struct env_driver *drv)
{
if (crc32(0, env_ptr->data, ENV_SIZE) == env_ptr->crc) {
- gd->env_addr = (ulong)&(env_ptr->data);
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_addr = (ulong)&(env_ptr->data);
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
return 0;
}
- gd->env_addr = (ulong)&default_environment[0];
- gd->env_valid = ENV_INVALID;
+ gd->env_infos[drv->location].env_addr = (ulong)&default_environment[0];
+ gd->env_infos[drv->location].env_valid = ENV_INVALID;
return 0;
}
#endif
@@ -311,7 +311,7 @@ done:
static int env_flash_load(struct env_driver *drv)
{
#ifdef CONFIG_ENV_ADDR_REDUND
- if (gd->env_addr != (ulong)&(flash_addr->data)) {
+ if (gd->env_infos[drv->location].env_addr != (ulong)&(flash_addr->data)) {
env_t *etmp = flash_addr;
ulong ltmp = end_addr;
@@ -326,7 +326,7 @@ static int env_flash_load(struct env_driver *drv)
crc32(0, flash_addr_new->data, ENV_SIZE) == flash_addr_new->crc) {
char flag = OBSOLETE_FLAG;
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
flash_sect_protect(0, (ulong)flash_addr_new, end_addr_new);
flash_write(&flag,
(ulong)&(flash_addr_new->flags),
@@ -338,7 +338,7 @@ static int env_flash_load(struct env_driver *drv)
(flash_addr->flags & ACTIVE_FLAG) == ACTIVE_FLAG) {
char flag = ACTIVE_FLAG;
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
flash_sect_protect(0, (ulong)flash_addr, end_addr);
flash_write(&flag,
(ulong)&(flash_addr->flags),
@@ -346,7 +346,7 @@ static int env_flash_load(struct env_driver *drv)
flash_sect_protect(1, (ulong)flash_addr, end_addr);
}
- if (gd->env_valid == ENV_REDUND)
+ if (gd->env_infos[drv->location].env_valid == ENV_REDUND)
puts("*** Warning - some problems detected "
"reading environment; recovered successfully\n\n");
#endif /* CONFIG_ENV_ADDR_REDUND */
diff --git a/env/mmc.c b/env/mmc.c
index 64d1404..250108f 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -163,7 +163,7 @@ static int env_mmc_save(struct env_driver *drv)
goto fini;
#ifdef CONFIG_ENV_OFFSET_REDUND
- if (gd->env_valid == ENV_VALID)
+ if (gd->env_infos[drv->location].env_valid == ENV_VALID)
copy = 1;
#endif
@@ -182,7 +182,7 @@ static int env_mmc_save(struct env_driver *drv)
ret = 0;
#ifdef CONFIG_ENV_OFFSET_REDUND
- gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = gd->env_infos[drv->location].env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
#endif
fini:
@@ -247,10 +247,10 @@ static int env_mmc_load(struct env_driver *drv)
ret = -EIO;
goto fini;
} else if (!read1_fail && read2_fail) {
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
env_import((char *)tmp_env1, 1);
} else if (read1_fail && !read2_fail) {
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
env_import((char *)tmp_env2, 1);
} else {
env_import_redund((char *)tmp_env1, (char *)tmp_env2);
diff --git a/env/nand.c b/env/nand.c
index 49e506e..77b9ca1 100644
--- a/env/nand.c
+++ b/env/nand.c
@@ -78,42 +78,42 @@ static int env_nand_init(struct env_driver *drv)
crc1_ok = crc32(0, tmp_env1->data, ENV_SIZE) == tmp_env1->crc;
if (!crc1_ok && !crc2_ok) {
- gd->env_addr = 0;
- gd->env_valid = ENV_INVALID;
+ gd->env_infos[drv->location].env_addr = 0;
+ gd->env_infos[drv->location].env_valid = ENV_INVALID;
return 0;
} else if (crc1_ok && !crc2_ok) {
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
}
#ifdef CONFIG_ENV_OFFSET_REDUND
else if (!crc1_ok && crc2_ok) {
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
} else {
/* both ok - check serial */
if (tmp_env1->flags == 255 && tmp_env2->flags == 0)
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
else if (tmp_env2->flags == 255 && tmp_env1->flags == 0)
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
else if (tmp_env1->flags > tmp_env2->flags)
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
else if (tmp_env2->flags > tmp_env1->flags)
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
else /* flags are equal - almost impossible */
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
}
- if (gd->env_valid == ENV_REDUND)
+ if (gd->env_infos[drv->location].env_valid == ENV_REDUND)
env_ptr = tmp_env2;
else
#endif
- if (gd->env_valid == ENV_VALID)
+ if (gd->env_infos[drv->location].env_valid == ENV_VALID)
env_ptr = tmp_env1;
- gd->env_addr = (ulong)env_ptr->data;
+ gd->env_infos[drv->location].env_addr = (ulong)env_ptr->data;
#else /* ENV_IS_EMBEDDED || CONFIG_NAND_ENV_DST */
- gd->env_addr = (ulong)&default_environment[0];
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_addr = (ulong)&default_environment[0];
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
#endif /* ENV_IS_EMBEDDED || CONFIG_NAND_ENV_DST */
return 0;
@@ -216,14 +216,14 @@ static int env_nand_save(struct env_driver *drv)
return ret;
#ifdef CONFIG_ENV_OFFSET_REDUND
- env_idx = (gd->env_valid == ENV_VALID);
+ env_idx = (gd->env_infos[drv->location].env_valid == ENV_VALID);
#endif
ret = erase_and_write_env(&location[env_idx], (u_char *)env_new);
#ifdef CONFIG_ENV_OFFSET_REDUND
if (!ret) {
/* preset other copy for next write */
- gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID :
+ gd->env_infos[drv->location].env_valid = gd->env_infos[drv->location].env_valid == ENV_REDUND ? ENV_VALID :
ENV_REDUND;
return ret;
}
@@ -346,10 +346,10 @@ static int env_nand_load(struct env_driver *drv)
set_default_env("!bad env area");
goto done;
} else if (!read1_fail && read2_fail) {
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
env_import((char *)tmp_env1, 1);
} else if (read1_fail && !read2_fail) {
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
env_import((char *)tmp_env2, 1);
} else {
env_import_redund(drv, (char *)tmp_env1, (char *)tmp_env2);
diff --git a/env/nowhere.c b/env/nowhere.c
index fb6ea9e..7d81a57 100644
--- a/env/nowhere.c
+++ b/env/nowhere.c
@@ -26,8 +26,8 @@ static int env_nowhere_load(struct env_driver *drv)
*/
static int env_nowhere_init(struct env_driver *drv)
{
- gd->env_addr = (ulong)&default_environment[0];
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_addr = (ulong)&default_environment[0];
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
return 0;
}
diff --git a/env/nvram.c b/env/nvram.c
index aad341d..c36c1af 100644
--- a/env/nvram.c
+++ b/env/nvram.c
@@ -98,15 +98,15 @@ static int env_nvram_init(struct env_driver *drv)
nvram_read(data, CONFIG_ENV_ADDR + sizeof(ulong), ENV_SIZE);
if (crc32(0, data, ENV_SIZE) == crc) {
- gd->env_addr = (ulong)CONFIG_ENV_ADDR + sizeof(long);
+ gd->env_infos[drv->location].env_addr = (ulong)CONFIG_ENV_ADDR + sizeof(long);
#else
if (crc32(0, env_ptr->data, ENV_SIZE) == env_ptr->crc) {
- gd->env_addr = (ulong)&env_ptr->data;
+ gd->env_infos[drv->location].env_addr = (ulong)&env_ptr->data;
#endif
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
} else {
- gd->env_addr = (ulong)&default_environment[0];
- gd->env_valid = ENV_INVALID;
+ gd->env_infos[drv->location].env_addr = (ulong)&default_environment[0];
+ gd->env_infos[drv->location].env_valid = ENV_INVALID;
}
return 0;
diff --git a/env/onenand.c b/env/onenand.c
index e633ebe..68790f1 100644
--- a/env/onenand.c
+++ b/env/onenand.c
@@ -58,7 +58,7 @@ static int env_onenand_load(struct env_driver *drv)
rc = env_import(buf, 1);
if (rc)
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
return rc ? 0 : -EIO;
}
diff --git a/env/remote.c b/env/remote.c
index b004964..895759e 100644
--- a/env/remote.c
+++ b/env/remote.c
@@ -26,8 +26,8 @@ DECLARE_GLOBAL_DATA_PTR;
static int env_remote_init(struct env_driver *drv)
{
if (crc32(0, env_ptr->data, ENV_SIZE) == env_ptr->crc) {
- gd->env_addr = (ulong)&(env_ptr->data);
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_addr = (ulong)&(env_ptr->data);
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
return 0;
}
diff --git a/env/sf.c b/env/sf.c
index e811cde..40e2671 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -98,7 +98,7 @@ static int env_sf_save(struct env_driver *drv)
return -EIO;
env_new.flags = ACTIVE_FLAG;
- if (gd->env_valid == ENV_VALID) {
+ if (gd->env_infos[drv->location].env_valid == ENV_VALID) {
env_new_offset = CONFIG_ENV_OFFSET_REDUND;
env_offset = CONFIG_ENV_OFFSET;
} else {
@@ -150,9 +150,9 @@ static int env_sf_save(struct env_driver *drv)
puts("done\n");
- gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = gd->env_infos[drv->location].env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
- printf("Valid environment: %d\n", (int)gd->env_valid);
+ printf("Valid environment: %d\n", (int)gd->env_infos[drv->location].env_valid);
done:
if (saved_buffer)
@@ -206,30 +206,30 @@ static int env_sf_load(struct env_driver *drv)
ret = -EIO;
goto err_read;
} else if (crc1_ok && !crc2_ok) {
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
} else if (!crc1_ok && crc2_ok) {
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
} else if (tmp_env1->flags == ACTIVE_FLAG &&
tmp_env2->flags == OBSOLETE_FLAG) {
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
} else if (tmp_env1->flags == OBSOLETE_FLAG &&
tmp_env2->flags == ACTIVE_FLAG) {
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
} else if (tmp_env1->flags == tmp_env2->flags) {
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
} else if (tmp_env1->flags == 0xFF) {
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
} else if (tmp_env2->flags == 0xFF) {
- gd->env_valid = ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = ENV_REDUND;
} else {
/*
* this differs from code in env_flash.c, but I think a sane
* default path is desirable.
*/
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
}
- if (gd->env_valid == ENV_VALID)
+ if (gd->env_infos[drv->location].env_valid == ENV_VALID)
ep = tmp_env1;
else
ep = tmp_env2;
@@ -336,7 +336,7 @@ static int env_sf_load(struct env_driver *drv)
ret = env_import(buf, 1);
if (ret)
- gd->env_valid = ENV_VALID;
+ gd->env_infos[drv->location].env_valid = ENV_VALID;
err_read:
spi_flash_free(env_flash);
diff --git a/env/ubi.c b/env/ubi.c
index 51b61eb..bd137e9 100644
--- a/env/ubi.c
+++ b/env/ubi.c
@@ -35,7 +35,7 @@ static int env_ubi_save(struct env_driver *drv)
return 1;
}
- if (gd->env_valid == ENV_VALID) {
+ if (gd->env_infos[drv->location].env_valid == ENV_VALID) {
puts("Writing to redundant UBI... ");
if (ubi_volume_write(CONFIG_ENV_UBI_VOLUME_REDUND,
(void *)env_new, CONFIG_ENV_SIZE)) {
@@ -57,7 +57,7 @@ static int env_ubi_save(struct env_driver *drv)
puts("done\n");
- gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
+ gd->env_infos[drv->location].env_valid = gd->env_infos[drv->location].env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
return 0;
}
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 1d0611f..17cf1b7 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -23,6 +23,7 @@
#ifndef __ASSEMBLY__
#include <membuff.h>
#include <linux/list.h>
+#include <environment.h>
typedef struct global_data {
bd_t *bd;
@@ -48,9 +49,7 @@ typedef struct global_data {
#if CONFIG_IS_ENABLED(PRE_CONSOLE_BUFFER)
unsigned long precon_buf_idx; /* Pre-Console buffer index */
#endif
- unsigned long env_addr; /* Address of Environment struct */
- unsigned long env_valid; /* Environment valid? enum env_valid */
- unsigned long env_has_init; /* Bitmask of boolean of struct env_location offsets */
+ struct env_info env_infos[ENVL_COUNT];
unsigned long ram_top; /* Top address of RAM used by U-Boot */
unsigned long relocaddr; /* Start address of U-Boot in RAM */
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment
2017-12-22 21:13 [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Quentin Schulz
` (10 preceding siblings ...)
2017-12-22 21:13 ` [U-Boot] [PATCH 11/11] env: store information about each environment in gd Quentin Schulz
@ 2017-12-26 11:25 ` Lukasz Majewski
2017-12-29 3:13 ` Simon Glass
12 siblings, 0 replies; 18+ messages in thread
From: Lukasz Majewski @ 2017-12-26 11:25 UTC (permalink / raw)
To: u-boot
Hi Quentin,
> 2) Why do we consider ENV_IS_NOWHERE an invalid environment? The only
> place I found a use for it was to just say that if the environment is
> invalid, we should set to default environment (in env_relocate in
> env/common.c). With my patch series I guess that we could remove this
> fallback and force ENV_IS_NOWHERE to be always there.
It is perfectly valid to use ENV_IS_NOWHERE to get envs from hardcoded
board file (for example for production u-boot).
However, some time ago we had extension in the envs to get the support
for envs from different mediums (with set priorities).
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171226/1b8894d9/attachment.sig>
^ permalink raw reply [flat|nested] 18+ messages in thread* [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment
2017-12-22 21:13 [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Quentin Schulz
` (11 preceding siblings ...)
2017-12-26 11:25 ` [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment Lukasz Majewski
@ 2017-12-29 3:13 ` Simon Glass
2018-01-03 9:18 ` Quentin Schulz
12 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2017-12-29 3:13 UTC (permalink / raw)
To: u-boot
Hi Quentin,
On 22 December 2017 at 14:13, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> This patch series is based on this[1] patch series from Maxime.
>
> This is an RFC. It's been only tested in a specific use case on a custom
> i.MX6 board. It's known to break compilation on a few boards.
>
> I have a use case where we want some variables from a first environment to
> be overriden by variables from a second environment. For example, we want
> to load variables from the default env (ENV_IS_NOWHERE) and then load only
> a handful of other variables from, e.g., NAND.
>
> In our use case, we basically can be sure that the default env in the
> U-Boot binary is secure but we want only a few variables to be modified,
> thus keeping control over the overall behaviour of U-Boot in secure mode.
>
> It works in that way:
> - from highest to lowest priority, the first environment that can be
> loaded (that has successfully init and whose load function has returned
> no errors) will be the main environment,
> - then, all the following environment that could be successfully loaded
> (same conditions as the main environment) are secondary environment. The
> env variables that are defined both in CONFIG_ENV_VAR_WHITELIST_LIST and
> in the secondary environments override the ones in the main environment,
> - for saving, we save the whole environment to all environments
> available, be they main or secondary (it does not matter to save the
> whole environment on secondary environments as only the whitelisted
> variables will be overriden in the loading process,
>
> I have also a few questions that could help me to get the whole thing to
> work.
>
> 1) I can't really get my head around the use of gd->env_addr, what is it used
> for? It is set in a bunch of different places but only once is it
> explicitly used (basically to alternate the env_addr between the one
> associated to main and redundant environment (in NAND for example)).
>
> 2) Why do we consider ENV_IS_NOWHERE an invalid environment? The only place I
> found a use for it was to just say that if the environment is invalid, we
> should set to default environment (in env_relocate in env/common.c). With
> my patch series I guess that we could remove this fallback and force
> ENV_IS_NOWHERE to be always there.
>
> 3) There are a few (20) boards that set gd->env_addr and gd->env_valid in
> their board file. What is the reason to do such a thing? Isn't those
> overriden anyway by the environment driver?
>
> I'm looking forward to getting your feedback on this patch series.
>
I certainly understand the goal and it seems generally useful.
But I wonder whether this is the best way to implement it.
We could have a UCLASS_ENV uclass, with driver-model drivers which
load the environment (i.e. have load() and save() methods). Config for
the drivers would come from the device tree. Useful drivers would be:
- one that loads the env from a single location
- one that loads multiple envs from different locations in priority order
- one that does what you want
Then people could set their own policy by adding a driver.
I worry that what we have here is quite heavyweight, and will be
imposed on all users, e.g. the size increase of gd, the new logic.
Where does it end? I think splitting things up into different use
cases makes sense.
When I did the env refactor I envisaged using driver-model for the
post-reloc env load/save at some point in the future. It solves the
problem of getting the config (can use device tree) and different
boards wanting to do different things (boards can provide an env
driver).
Regards,
Simon
> Thanks,
> Quentin
>
> [1] https://patchwork.ozlabs.org/cover/842057/
>
> Quentin Schulz (11):
> env: fix ret not being set and fails when no env could have been init
> lib: hashtable: support whitelisting env variables
> env: add support for whitelisting variables from secondary environments
> env: make nowhere an env medium like the others
> cmd: saveenv: enable the saveenv command when ENV_IS_NOWHERE is defined but another env medium is enabled too
> env: add env_driver to env_driver functions' arguments
> env: gd flags without ENV_READY is enough to discriminate in env_get_default
> env: add env_driver parameter to env_import_redund
> env: make env_locations a global variable
> env: introducing env_info struct for storing info per env
> env: store information about each environment in gd
>
> board/sunxi/board.c | 2 +-
> cmd/nvedit.c | 16 ++-
> common/board_r.c | 8 +-
> env/Kconfig | 29 +++---
> env/common.c | 45 ++++++----
> env/eeprom.c | 40 ++++-----
> env/env.c | 142 +++++++++++++++++++++++++------
> env/ext4.c | 4 +-
> env/fat.c | 4 +-
> env/flash.c | 58 ++++++-------
> env/mmc.c | 14 +--
> env/nand.c | 46 +++++-----
> env/nowhere.c | 12 ++-
> env/nvram.c | 18 ++--
> env/onenand.c | 6 +-
> env/remote.c | 10 +-
> env/sata.c | 4 +-
> env/sf.c | 34 +++----
> env/ubi.c | 14 +--
> include/asm-generic/global_data.h | 5 +-
> include/environment.h | 59 ++++++++-----
> include/search.h | 2 +-
> lib/hashtable.c | 17 +++-
> 23 files changed, 379 insertions(+), 210 deletions(-)
>
> base-commit: 5d41e28058e7b378c9fa5c61ecc074a682ba2db4
> --
> git-series 0.9.1
^ permalink raw reply [flat|nested] 18+ messages in thread* [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment
2017-12-29 3:13 ` Simon Glass
@ 2018-01-03 9:18 ` Quentin Schulz
2018-01-08 4:50 ` Simon Glass
0 siblings, 1 reply; 18+ messages in thread
From: Quentin Schulz @ 2018-01-03 9:18 UTC (permalink / raw)
To: u-boot
Hi Simon,
On 29/12/2017 04:13, Simon Glass wrote:
> Hi Quentin,
>
> On 22 December 2017 at 14:13, Quentin Schulz
> <quentin.schulz@free-electrons.com> wrote:
>> This patch series is based on this[1] patch series from Maxime.
>>
>> This is an RFC. It's been only tested in a specific use case on a custom
>> i.MX6 board. It's known to break compilation on a few boards.
>>
>> I have a use case where we want some variables from a first environment to
>> be overriden by variables from a second environment. For example, we want
>> to load variables from the default env (ENV_IS_NOWHERE) and then load only
>> a handful of other variables from, e.g., NAND.
>>
>> In our use case, we basically can be sure that the default env in the
>> U-Boot binary is secure but we want only a few variables to be modified,
>> thus keeping control over the overall behaviour of U-Boot in secure mode.
>>
>> It works in that way:
>> - from highest to lowest priority, the first environment that can be
>> loaded (that has successfully init and whose load function has returned
>> no errors) will be the main environment,
>> - then, all the following environment that could be successfully loaded
>> (same conditions as the main environment) are secondary environment. The
>> env variables that are defined both in CONFIG_ENV_VAR_WHITELIST_LIST and
>> in the secondary environments override the ones in the main environment,
>> - for saving, we save the whole environment to all environments
>> available, be they main or secondary (it does not matter to save the
>> whole environment on secondary environments as only the whitelisted
>> variables will be overriden in the loading process,
>>
>> I have also a few questions that could help me to get the whole thing to
>> work.
>>
>> 1) I can't really get my head around the use of gd->env_addr, what is it used
>> for? It is set in a bunch of different places but only once is it
>> explicitly used (basically to alternate the env_addr between the one
>> associated to main and redundant environment (in NAND for example)).
>>
>> 2) Why do we consider ENV_IS_NOWHERE an invalid environment? The only place I
>> found a use for it was to just say that if the environment is invalid, we
>> should set to default environment (in env_relocate in env/common.c). With
>> my patch series I guess that we could remove this fallback and force
>> ENV_IS_NOWHERE to be always there.
>>
>> 3) There are a few (20) boards that set gd->env_addr and gd->env_valid in
>> their board file. What is the reason to do such a thing? Isn't those
>> overriden anyway by the environment driver?
>>
>> I'm looking forward to getting your feedback on this patch series.
>>
>
> I certainly understand the goal and it seems generally useful.
>
> But I wonder whether this is the best way to implement it.
>
> We could have a UCLASS_ENV uclass, with driver-model drivers which
> load the environment (i.e. have load() and save() methods). Config for
> the drivers would come from the device tree. Useful drivers would be:
>
I'm not sure the device tree is the place to set/get such info. That has
nothing to do with hardware, it's only logic for the bootloader.
> - one that loads the env from a single location
> - one that loads multiple envs from different locations in priority order
> - one that does what you want
>
> Then people could set their own policy by adding a driver.
>
I'll have to document myself on driver-model and how U-Boot implement it
then :)
> I worry that what we have here is quite heavyweight, and will be
> imposed on all users, e.g. the size increase of gd, the new logic.
> Where does it end? I think splitting things up into different use
> cases makes sense.
>
Agree on that.
> When I did the env refactor I envisaged using driver-model for the
> post-reloc env load/save at some point in the future. It solves the
> problem of getting the config (can use device tree) and different
> boards wanting to do different things (boards can provide an env
> driver).
>
Overall, I prefer Lukasz's suggestion as it's quicker and easier to
implement and require (I think) less changes in the code.
Thanks for the review,
Quentin
--
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 00/11] Introduce variables whitelisting in environment
2018-01-03 9:18 ` Quentin Schulz
@ 2018-01-08 4:50 ` Simon Glass
0 siblings, 0 replies; 18+ messages in thread
From: Simon Glass @ 2018-01-08 4:50 UTC (permalink / raw)
To: u-boot
Hi Quentin,
On 3 January 2018 at 02:18, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> Hi Simon,
>
> On 29/12/2017 04:13, Simon Glass wrote:
>> Hi Quentin,
>>
>> On 22 December 2017 at 14:13, Quentin Schulz
>> <quentin.schulz@free-electrons.com> wrote:
>>> This patch series is based on this[1] patch series from Maxime.
>>>
>>> This is an RFC. It's been only tested in a specific use case on a custom
>>> i.MX6 board. It's known to break compilation on a few boards.
>>>
>>> I have a use case where we want some variables from a first environment to
>>> be overriden by variables from a second environment. For example, we want
>>> to load variables from the default env (ENV_IS_NOWHERE) and then load only
>>> a handful of other variables from, e.g., NAND.
>>>
>>> In our use case, we basically can be sure that the default env in the
>>> U-Boot binary is secure but we want only a few variables to be modified,
>>> thus keeping control over the overall behaviour of U-Boot in secure mode.
>>>
>>> It works in that way:
>>> - from highest to lowest priority, the first environment that can be
>>> loaded (that has successfully init and whose load function has returned
>>> no errors) will be the main environment,
>>> - then, all the following environment that could be successfully loaded
>>> (same conditions as the main environment) are secondary environment. The
>>> env variables that are defined both in CONFIG_ENV_VAR_WHITELIST_LIST and
>>> in the secondary environments override the ones in the main environment,
>>> - for saving, we save the whole environment to all environments
>>> available, be they main or secondary (it does not matter to save the
>>> whole environment on secondary environments as only the whitelisted
>>> variables will be overriden in the loading process,
>>>
>>> I have also a few questions that could help me to get the whole thing to
>>> work.
>>>
>>> 1) I can't really get my head around the use of gd->env_addr, what is it used
>>> for? It is set in a bunch of different places but only once is it
>>> explicitly used (basically to alternate the env_addr between the one
>>> associated to main and redundant environment (in NAND for example)).
>>>
>>> 2) Why do we consider ENV_IS_NOWHERE an invalid environment? The only place I
>>> found a use for it was to just say that if the environment is invalid, we
>>> should set to default environment (in env_relocate in env/common.c). With
>>> my patch series I guess that we could remove this fallback and force
>>> ENV_IS_NOWHERE to be always there.
>>>
>>> 3) There are a few (20) boards that set gd->env_addr and gd->env_valid in
>>> their board file. What is the reason to do such a thing? Isn't those
>>> overriden anyway by the environment driver?
>>>
>>> I'm looking forward to getting your feedback on this patch series.
>>>
>>
>> I certainly understand the goal and it seems generally useful.
>>
>> But I wonder whether this is the best way to implement it.
>>
>> We could have a UCLASS_ENV uclass, with driver-model drivers which
>> load the environment (i.e. have load() and save() methods). Config for
>> the drivers would come from the device tree. Useful drivers would be:
>>
>
> I'm not sure the device tree is the place to set/get such info. That has
> nothing to do with hardware, it's only logic for the bootloader.
>
>> - one that loads the env from a single location
>> - one that loads multiple envs from different locations in priority order
>> - one that does what you want
>>
>> Then people could set their own policy by adding a driver.
>>
>
> I'll have to document myself on driver-model and how U-Boot implement it
> then :)
>
>> I worry that what we have here is quite heavyweight, and will be
>> imposed on all users, e.g. the size increase of gd, the new logic.
>> Where does it end? I think splitting things up into different use
>> cases makes sense.
>>
>
> Agree on that.
>
>> When I did the env refactor I envisaged using driver-model for the
>> post-reloc env load/save at some point in the future. It solves the
>> problem of getting the config (can use device tree) and different
>> boards wanting to do different things (boards can provide an env
>> driver).
>>
>
> Overall, I prefer Lukasz's suggestion as it's quicker and easier to
> implement and require (I think) less changes in the code.
Do you mean selecting from different locations? Yes that is a good
thing, but is orthogonal to this series.
Here you are trying to add functionality which will apply to any env
location, or to them as a whole. So we should think of your change as
implementing a new policy rather than a new env location.
Regards,
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread