[PATCH] environment: add new global.env.noload parameter

[PATCH] environment: add new global.env.noload parameter

[Ahmad Fatoum]

It can be useful for board code to deny loading an environment without
disabling it altogether, e.g. to disable load of the environment when
entering a recovery mode. Add a global variable that allows for just
that.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/environment.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/common/environment.c b/common/environment.c
index d7f5f51cfc6d..d49f255ef007 100644
--- a/common/environment.c
+++ b/common/environment.c
@@ -54,6 +54,7 @@ struct action_data {
 #define TMPDIR "/.defaultenv"
 
 static int global_env_autoprobe = IS_ENABLED(CONFIG_INSECURE);
+static int global_env_noload = false;
 static char *default_environment_path;
 
 void default_environment_path_set(const char *path)
@@ -455,7 +456,7 @@ int envfs_load(const char *filename, const char *dir, unsigned flags)
 	size_t size, rsize;
 
 #ifdef __BAREBOX__
-	if (!IS_ALLOWED(SCONFIG_ENVIRONMENT_LOAD))
+	if (!IS_ALLOWED(SCONFIG_ENVIRONMENT_LOAD) || global_env_noload)
 		return -EPERM;
 #endif
 
@@ -539,12 +540,17 @@ int envfs_load(const char *filename, const char *dir, unsigned flags)
 }
 
 #ifdef __BAREBOX__
+
 static int register_env_vars(void)
 {
 	globalvar_add_simple_bool("env.autoprobe", &global_env_autoprobe);
+	globalvar_add_simple_bool("env.noload", &global_env_noload);
+
 	return 0;
 }
 postcore_initcall(register_env_vars);
 BAREBOX_MAGICVAR(global.env.autoprobe,
                  "Automatically probe known block devices for environment");
+BAREBOX_MAGICVAR(global.env.noload,
+                 "Suppress loading of externally persisted environment");
 #endif
-- 
2.47.3

Re: [PATCH] environment: add new global.env.noload parameter

[Sascha Hauer]

On Fri, Nov 28, 2025 at 06:21:14PM +0100, Ahmad Fatoum wrote:
> It can be useful for board code to deny loading an environment without
> disabling it altogether, e.g. to disable load of the environment when
> entering a recovery mode. Add a global variable that allows for just
> that.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  common/environment.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/common/environment.c b/common/environment.c
> index d7f5f51cfc6d..d49f255ef007 100644
> --- a/common/environment.c
> +++ b/common/environment.c
> @@ -54,6 +54,7 @@ struct action_data {
>  #define TMPDIR "/.defaultenv"
>  
>  static int global_env_autoprobe = IS_ENABLED(CONFIG_INSECURE);
> +static int global_env_noload = false;
>  static char *default_environment_path;
>  
>  void default_environment_path_set(const char *path)
> @@ -455,7 +456,7 @@ int envfs_load(const char *filename, const char *dir, unsigned flags)
>  	size_t size, rsize;
>  
>  #ifdef __BAREBOX__
> -	if (!IS_ALLOWED(SCONFIG_ENVIRONMENT_LOAD))
> +	if (!IS_ALLOWED(SCONFIG_ENVIRONMENT_LOAD) || global_env_noload)
>  		return -EPERM;

With this you forbid to ever load an environment. The commit message
suggests you are looking for a way to skip loading of the environment,
so I would rather expect the check in the caller in common/startup.c

>  #ifdef __BAREBOX__
> +
>  static int register_env_vars(void)
>  {
>  	globalvar_add_simple_bool("env.autoprobe", &global_env_autoprobe);
> +	globalvar_add_simple_bool("env.noload", &global_env_noload);

globalvars are usually meant for variables that are set/read from the
shell. What's your usecase for setting this from shell?

Having this as globalvar implies there is a corresponding nvvar. If I
read the code correctly then setting nv.env.noload=true in the builtin
default environment will not have the effect that loading of the
persistent environment is suppressed. Setting nv.env.noload=true in the
persistent environment obviously also isn't meaningful. The only effect
will be that a loadenv command no longer works.

Making this a globalvar seems rather confusing to me.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] environment: add new global.env.noload parameter

[Ahmad Fatoum]

Hi,

On 12/1/25 10:37 AM, Sascha Hauer wrote:
> On Fri, Nov 28, 2025 at 06:21:14PM +0100, Ahmad Fatoum wrote:
>> It can be useful for board code to deny loading an environment without
>> disabling it altogether, e.g. to disable load of the environment when
>> entering a recovery mode. Add a global variable that allows for just
>> that.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  common/environment.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/environment.c b/common/environment.c
>> index d7f5f51cfc6d..d49f255ef007 100644
>> --- a/common/environment.c
>> +++ b/common/environment.c
>> @@ -54,6 +54,7 @@ struct action_data {
>>  #define TMPDIR "/.defaultenv"
>>  
>>  static int global_env_autoprobe = IS_ENABLED(CONFIG_INSECURE);
>> +static int global_env_noload = false;
>>  static char *default_environment_path;
>>  
>>  void default_environment_path_set(const char *path)
>> @@ -455,7 +456,7 @@ int envfs_load(const char *filename, const char *dir, unsigned flags)
>>  	size_t size, rsize;
>>  
>>  #ifdef __BAREBOX__
>> -	if (!IS_ALLOWED(SCONFIG_ENVIRONMENT_LOAD))
>> +	if (!IS_ALLOWED(SCONFIG_ENVIRONMENT_LOAD) || global_env_noload)
>>  		return -EPERM;
> 
> With this you forbid to ever load an environment. The commit message
> suggests you are looking for a way to skip loading of the environment,
> so I would rather expect the check in the caller in common/startup.c

Well, a user could change the value of the variable later, but I agree
checking in common/startup.c makes more sense.

> 
>>  #ifdef __BAREBOX__
>> +
>>  static int register_env_vars(void)
>>  {
>>  	globalvar_add_simple_bool("env.autoprobe", &global_env_autoprobe);
>> +	globalvar_add_simple_bool("env.noload", &global_env_noload);
> 
> globalvars are usually meant for variables that are set/read from the
> shell. What's your usecase for setting this from shell?

This is supposed to set by board code. Shell would be too late.

> Having this as globalvar implies there is a corresponding nvvar. If I
> read the code correctly then setting nv.env.noload=true in the builtin
> default environment will not have the effect that loading of the
> persistent environment is suppressed. Setting nv.env.noload=true in the
> persistent environment obviously also isn't meaningful. The only effect
> will be that a loadenv command no longer works.
> 
> Making this a globalvar seems rather confusing to me.

So you prefer I turn this into a function instead?

Cheers,
Ahmad

> 
> Sascha
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH] environment: add new global.env.noload parameter

[Sascha Hauer]

On Mon, Dec 01, 2025 at 10:42:56AM +0100, Ahmad Fatoum wrote:
> Hi,
> 
> On 12/1/25 10:37 AM, Sascha Hauer wrote:
> > On Fri, Nov 28, 2025 at 06:21:14PM +0100, Ahmad Fatoum wrote:
> >> It can be useful for board code to deny loading an environment without
> >> disabling it altogether, e.g. to disable load of the environment when
> >> entering a recovery mode. Add a global variable that allows for just
> >> that.
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  common/environment.c | 8 +++++++-
> >>  1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/common/environment.c b/common/environment.c
> >> index d7f5f51cfc6d..d49f255ef007 100644
> >> --- a/common/environment.c
> >> +++ b/common/environment.c
> >> @@ -54,6 +54,7 @@ struct action_data {
> >>  #define TMPDIR "/.defaultenv"
> >>  
> >>  static int global_env_autoprobe = IS_ENABLED(CONFIG_INSECURE);
> >> +static int global_env_noload = false;
> >>  static char *default_environment_path;
> >>  
> >>  void default_environment_path_set(const char *path)
> >> @@ -455,7 +456,7 @@ int envfs_load(const char *filename, const char *dir, unsigned flags)
> >>  	size_t size, rsize;
> >>  
> >>  #ifdef __BAREBOX__
> >> -	if (!IS_ALLOWED(SCONFIG_ENVIRONMENT_LOAD))
> >> +	if (!IS_ALLOWED(SCONFIG_ENVIRONMENT_LOAD) || global_env_noload)
> >>  		return -EPERM;
> > 
> > With this you forbid to ever load an environment. The commit message
> > suggests you are looking for a way to skip loading of the environment,
> > so I would rather expect the check in the caller in common/startup.c
> 
> Well, a user could change the value of the variable later, but I agree
> checking in common/startup.c makes more sense.
> 
> > 
> >>  #ifdef __BAREBOX__
> >> +
> >>  static int register_env_vars(void)
> >>  {
> >>  	globalvar_add_simple_bool("env.autoprobe", &global_env_autoprobe);
> >> +	globalvar_add_simple_bool("env.noload", &global_env_noload);
> > 
> > globalvars are usually meant for variables that are set/read from the
> > shell. What's your usecase for setting this from shell?
> 
> This is supposed to set by board code. Shell would be too late.
> 
> > Having this as globalvar implies there is a corresponding nvvar. If I
> > read the code correctly then setting nv.env.noload=true in the builtin
> > default environment will not have the effect that loading of the
> > persistent environment is suppressed. Setting nv.env.noload=true in the
> > persistent environment obviously also isn't meaningful. The only effect
> > will be that a loadenv command no longer works.
> > 
> > Making this a globalvar seems rather confusing to me.
> 
> So you prefer I turn this into a function instead?

Yes please.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |