[PATCH] startup: add new AUTOBOOT_HALT state

[PATCH] startup: add new AUTOBOOT_HALT state

[Ahmad Fatoum]

To make the barebox shell more convenient to use, barebox executes some
actions only when dropping to an interactive shell:

  - If dropping due to global.autoboot=abort, all watchdogs are
    automatically quiesced
  - In every case, network interfaces are brought up, so link
    negotiation happens in parallel to waiting for user input

This is useful for development and debugging, but bad for automatic
tests as the system behaves differently when automatic test aborts
barebox to set e.g. a boot argument and when barebox is left to boot
uninterrupted.

For use in such scenarios, let's introduce a new AUTOBOOT_HALT state.
This will not do any automatic interactive magic and can be entered by
either setting nv.autoboot=halt or by aborting the boot with ctrl+d.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Thoughts?

Alternatively, we could make only aborting the boot via e.g. enter
enable the network interfaces.
---
 common/startup.c  | 11 ++++++++---
 include/barebox.h |  3 +++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/common/startup.c b/common/startup.c
index 47b70a7756cb..5d0ac116b98c 100644
--- a/common/startup.c
+++ b/common/startup.c
@@ -28,6 +28,7 @@
 #include <linux/stat.h>
 #include <envfs.h>
 #include <magicvar.h>
+#include <readkey.h>
 #include <linux/reboot-mode.h>
 #include <asm/sections.h>
 #include <uncompress.h>
@@ -103,6 +104,7 @@ static int global_autoboot_timeout = 3;
 static const char * const global_autoboot_states[] = {
 	[AUTOBOOT_COUNTDOWN] = "countdown",
 	[AUTOBOOT_ABORT] = "abort",
+	[AUTOBOOT_HALT] = "halt",
 	[AUTOBOOT_MENU] = "menu",
 	[AUTOBOOT_BOOT] = "boot",
 };
@@ -176,7 +178,8 @@ enum autoboot_state do_autoboot_countdown(void)
 		return autoboot_state;
 
 	if (!console_get_first_active() &&
-	    global_autoboot_state != AUTOBOOT_ABORT) {
+	    global_autoboot_state != AUTOBOOT_ABORT &&
+	    global_autoboot_state != AUTOBOOT_HALT) {
 		printf("\nNon-interactive console, booting system\n");
 		return autoboot_state = AUTOBOOT_BOOT;
 	}
@@ -215,6 +218,8 @@ enum autoboot_state do_autoboot_countdown(void)
 		autoboot_state = AUTOBOOT_BOOT;
 	else if (menu_exists && outkey == 'm')
 		autoboot_state = AUTOBOOT_MENU;
+	else if (outkey == CTL_CH('d'))
+		autoboot_state = AUTOBOOT_HALT;
 	else
 		autoboot_state = AUTOBOOT_ABORT;
 
@@ -310,7 +315,7 @@ static int run_init(void)
 	if (autoboot == AUTOBOOT_BOOT)
 		run_command("boot");
 
-	if (IS_ENABLED(CONFIG_NET))
+	if (IS_ENABLED(CONFIG_NET) && autoboot != AUTOBOOT_HALT)
 		eth_open_all();
 
 	if (autoboot == AUTOBOOT_MENU)
@@ -400,7 +405,7 @@ void shutdown_barebox(void)
 }
 
 BAREBOX_MAGICVAR(global.autoboot,
-                 "Autoboot state. Possible values: countdown (default), abort, menu, boot");
+                 "Autoboot state. Possible values: countdown (default), abort, halt, menu, boot");
 BAREBOX_MAGICVAR(global.autoboot_abort_key,
                  "Which key allows to interrupt autoboot. Possible values: any, ctrl-c");
 BAREBOX_MAGICVAR(global.autoboot_timeout,
diff --git a/include/barebox.h b/include/barebox.h
index 45e95f187462..b3d34b7b739b 100644
--- a/include/barebox.h
+++ b/include/barebox.h
@@ -22,6 +22,8 @@ extern int (*barebox_main)(void);
  * enum autoboot_state - autoboot action after init
  * @AUTOBOOT_COUNTDOWN: Count down to automatic boot action
  * @AUTOBOOT_ABORT: Abort boot and drop to barebox shell
+ * @AUTOBOOT_HALT: Halt boot and drop to barebox shell, _without_ running
+ *                 any interactive hooks (e.g. bringing up network)
  * AUTOBOOT_MENU: Show main menu directly
  * @AUTOBOOT_BOOT: Boot right away without an interruptible countdown
  * @AUTOBOOT_UNKNOWN: Boot right away without an interruptible countdown
@@ -32,6 +34,7 @@ extern int (*barebox_main)(void);
 enum autoboot_state {
 	AUTOBOOT_COUNTDOWN,
 	AUTOBOOT_ABORT,
+	AUTOBOOT_HALT,
 	AUTOBOOT_MENU,
 	AUTOBOOT_BOOT,
 	AUTOBOOT_UNKNOWN,
-- 
2.39.5

Re: [PATCH] startup: add new AUTOBOOT_HALT state

[Marco Felsch]

Hi Ahmad,

On 24-10-21, Ahmad Fatoum wrote:
> To make the barebox shell more convenient to use, barebox executes some
> actions only when dropping to an interactive shell:
> 
>   - If dropping due to global.autoboot=abort, all watchdogs are
>     automatically quiesced
>   - In every case, network interfaces are brought up, so link
>     negotiation happens in parallel to waiting for user input
> 
> This is useful for development and debugging, but bad for automatic
> tests as the system behaves differently when automatic test aborts
> barebox to set e.g. a boot argument and when barebox is left to boot
> uninterrupted.
> 
> For use in such scenarios, let's introduce a new AUTOBOOT_HALT state.
> This will not do any automatic interactive magic and can be entered by
> either setting nv.autoboot=halt or by aborting the boot with ctrl+d.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> Thoughts?
> 
> Alternatively, we could make only aborting the boot via e.g. enter
> enable the network interfaces.

To have a more fine grained option we could use
global.autoboot.skip_eth_auto_enable instead of making use of new
autoboot state the user need to check first to know the differences
between 'abort' and 'halt'. Of course this comes with the drawback of
having a new parameter the user need to set but IMHO is more future
proof e.g. if autoboot does more automagic in the future which can be
controlled via global.autoboot.* params as well.

Regards,
  Marco

> ---
>  common/startup.c  | 11 ++++++++---
>  include/barebox.h |  3 +++
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/common/startup.c b/common/startup.c
> index 47b70a7756cb..5d0ac116b98c 100644
> --- a/common/startup.c
> +++ b/common/startup.c
> @@ -28,6 +28,7 @@
>  #include <linux/stat.h>
>  #include <envfs.h>
>  #include <magicvar.h>
> +#include <readkey.h>
>  #include <linux/reboot-mode.h>
>  #include <asm/sections.h>
>  #include <uncompress.h>
> @@ -103,6 +104,7 @@ static int global_autoboot_timeout = 3;
>  static const char * const global_autoboot_states[] = {
>  	[AUTOBOOT_COUNTDOWN] = "countdown",
>  	[AUTOBOOT_ABORT] = "abort",
> +	[AUTOBOOT_HALT] = "halt",
>  	[AUTOBOOT_MENU] = "menu",
>  	[AUTOBOOT_BOOT] = "boot",
>  };
> @@ -176,7 +178,8 @@ enum autoboot_state do_autoboot_countdown(void)
>  		return autoboot_state;
>  
>  	if (!console_get_first_active() &&
> -	    global_autoboot_state != AUTOBOOT_ABORT) {
> +	    global_autoboot_state != AUTOBOOT_ABORT &&
> +	    global_autoboot_state != AUTOBOOT_HALT) {
>  		printf("\nNon-interactive console, booting system\n");
>  		return autoboot_state = AUTOBOOT_BOOT;
>  	}
> @@ -215,6 +218,8 @@ enum autoboot_state do_autoboot_countdown(void)
>  		autoboot_state = AUTOBOOT_BOOT;
>  	else if (menu_exists && outkey == 'm')
>  		autoboot_state = AUTOBOOT_MENU;
> +	else if (outkey == CTL_CH('d'))
> +		autoboot_state = AUTOBOOT_HALT;
>  	else
>  		autoboot_state = AUTOBOOT_ABORT;
>  
> @@ -310,7 +315,7 @@ static int run_init(void)
>  	if (autoboot == AUTOBOOT_BOOT)
>  		run_command("boot");
>  
> -	if (IS_ENABLED(CONFIG_NET))
> +	if (IS_ENABLED(CONFIG_NET) && autoboot != AUTOBOOT_HALT)
>  		eth_open_all();
>  
>  	if (autoboot == AUTOBOOT_MENU)
> @@ -400,7 +405,7 @@ void shutdown_barebox(void)
>  }
>  
>  BAREBOX_MAGICVAR(global.autoboot,
> -                 "Autoboot state. Possible values: countdown (default), abort, menu, boot");
> +                 "Autoboot state. Possible values: countdown (default), abort, halt, menu, boot");
>  BAREBOX_MAGICVAR(global.autoboot_abort_key,
>                   "Which key allows to interrupt autoboot. Possible values: any, ctrl-c");
>  BAREBOX_MAGICVAR(global.autoboot_timeout,
> diff --git a/include/barebox.h b/include/barebox.h
> index 45e95f187462..b3d34b7b739b 100644
> --- a/include/barebox.h
> +++ b/include/barebox.h
> @@ -22,6 +22,8 @@ extern int (*barebox_main)(void);
>   * enum autoboot_state - autoboot action after init
>   * @AUTOBOOT_COUNTDOWN: Count down to automatic boot action
>   * @AUTOBOOT_ABORT: Abort boot and drop to barebox shell
> + * @AUTOBOOT_HALT: Halt boot and drop to barebox shell, _without_ running
> + *                 any interactive hooks (e.g. bringing up network)
>   * AUTOBOOT_MENU: Show main menu directly
>   * @AUTOBOOT_BOOT: Boot right away without an interruptible countdown
>   * @AUTOBOOT_UNKNOWN: Boot right away without an interruptible countdown
> @@ -32,6 +34,7 @@ extern int (*barebox_main)(void);
>  enum autoboot_state {
>  	AUTOBOOT_COUNTDOWN,
>  	AUTOBOOT_ABORT,
> +	AUTOBOOT_HALT,
>  	AUTOBOOT_MENU,
>  	AUTOBOOT_BOOT,
>  	AUTOBOOT_UNKNOWN,
> -- 
> 2.39.5
> 
> 
> 

Re: [PATCH] startup: add new AUTOBOOT_HALT state

[Ahmad Fatoum]

Hello Marco,

On 22.10.24 08:18, Marco Felsch wrote:
> Hi Ahmad,
> 
> On 24-10-21, Ahmad Fatoum wrote:
>> To make the barebox shell more convenient to use, barebox executes some
>> actions only when dropping to an interactive shell:
>>
>>   - If dropping due to global.autoboot=abort, all watchdogs are
>>     automatically quiesced
>>   - In every case, network interfaces are brought up, so link
>>     negotiation happens in parallel to waiting for user input
>>
>> This is useful for development and debugging, but bad for automatic
>> tests as the system behaves differently when automatic test aborts
>> barebox to set e.g. a boot argument and when barebox is left to boot
>> uninterrupted.
>>
>> For use in such scenarios, let's introduce a new AUTOBOOT_HALT state.
>> This will not do any automatic interactive magic and can be entered by
>> either setting nv.autoboot=halt or by aborting the boot with ctrl+d.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> Thoughts?
>>
>> Alternatively, we could make only aborting the boot via e.g. enter
>> enable the network interfaces.
> 
> To have a more fine grained option we could use
> global.autoboot.skip_eth_auto_enable instead of making use of new
> autoboot state the user need to check first to know the differences
> between 'abort' and 'halt'. Of course this comes with the drawback of
> having a new parameter the user need to set but IMHO is more future
> proof e.g. if autoboot does more automagic in the future which can be
> controlled via global.autoboot.* params as well.

We still need a way for a test suite to abort boot without bringing
up network interfaces automatically - and without writing the environment.

Cheers,
Ahmad

> 
> Regards,
>   Marco
> 
>> ---
>>  common/startup.c  | 11 ++++++++---
>>  include/barebox.h |  3 +++
>>  2 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/common/startup.c b/common/startup.c
>> index 47b70a7756cb..5d0ac116b98c 100644
>> --- a/common/startup.c
>> +++ b/common/startup.c
>> @@ -28,6 +28,7 @@
>>  #include <linux/stat.h>
>>  #include <envfs.h>
>>  #include <magicvar.h>
>> +#include <readkey.h>
>>  #include <linux/reboot-mode.h>
>>  #include <asm/sections.h>
>>  #include <uncompress.h>
>> @@ -103,6 +104,7 @@ static int global_autoboot_timeout = 3;
>>  static const char * const global_autoboot_states[] = {
>>  	[AUTOBOOT_COUNTDOWN] = "countdown",
>>  	[AUTOBOOT_ABORT] = "abort",
>> +	[AUTOBOOT_HALT] = "halt",
>>  	[AUTOBOOT_MENU] = "menu",
>>  	[AUTOBOOT_BOOT] = "boot",
>>  };
>> @@ -176,7 +178,8 @@ enum autoboot_state do_autoboot_countdown(void)
>>  		return autoboot_state;
>>  
>>  	if (!console_get_first_active() &&
>> -	    global_autoboot_state != AUTOBOOT_ABORT) {
>> +	    global_autoboot_state != AUTOBOOT_ABORT &&
>> +	    global_autoboot_state != AUTOBOOT_HALT) {
>>  		printf("\nNon-interactive console, booting system\n");
>>  		return autoboot_state = AUTOBOOT_BOOT;
>>  	}
>> @@ -215,6 +218,8 @@ enum autoboot_state do_autoboot_countdown(void)
>>  		autoboot_state = AUTOBOOT_BOOT;
>>  	else if (menu_exists && outkey == 'm')
>>  		autoboot_state = AUTOBOOT_MENU;
>> +	else if (outkey == CTL_CH('d'))
>> +		autoboot_state = AUTOBOOT_HALT;
>>  	else
>>  		autoboot_state = AUTOBOOT_ABORT;
>>  
>> @@ -310,7 +315,7 @@ static int run_init(void)
>>  	if (autoboot == AUTOBOOT_BOOT)
>>  		run_command("boot");
>>  
>> -	if (IS_ENABLED(CONFIG_NET))
>> +	if (IS_ENABLED(CONFIG_NET) && autoboot != AUTOBOOT_HALT)
>>  		eth_open_all();
>>  
>>  	if (autoboot == AUTOBOOT_MENU)
>> @@ -400,7 +405,7 @@ void shutdown_barebox(void)
>>  }
>>  
>>  BAREBOX_MAGICVAR(global.autoboot,
>> -                 "Autoboot state. Possible values: countdown (default), abort, menu, boot");
>> +                 "Autoboot state. Possible values: countdown (default), abort, halt, menu, boot");
>>  BAREBOX_MAGICVAR(global.autoboot_abort_key,
>>                   "Which key allows to interrupt autoboot. Possible values: any, ctrl-c");
>>  BAREBOX_MAGICVAR(global.autoboot_timeout,
>> diff --git a/include/barebox.h b/include/barebox.h
>> index 45e95f187462..b3d34b7b739b 100644
>> --- a/include/barebox.h
>> +++ b/include/barebox.h
>> @@ -22,6 +22,8 @@ extern int (*barebox_main)(void);
>>   * enum autoboot_state - autoboot action after init
>>   * @AUTOBOOT_COUNTDOWN: Count down to automatic boot action
>>   * @AUTOBOOT_ABORT: Abort boot and drop to barebox shell
>> + * @AUTOBOOT_HALT: Halt boot and drop to barebox shell, _without_ running
>> + *                 any interactive hooks (e.g. bringing up network)
>>   * AUTOBOOT_MENU: Show main menu directly
>>   * @AUTOBOOT_BOOT: Boot right away without an interruptible countdown
>>   * @AUTOBOOT_UNKNOWN: Boot right away without an interruptible countdown
>> @@ -32,6 +34,7 @@ extern int (*barebox_main)(void);
>>  enum autoboot_state {
>>  	AUTOBOOT_COUNTDOWN,
>>  	AUTOBOOT_ABORT,
>> +	AUTOBOOT_HALT,
>>  	AUTOBOOT_MENU,
>>  	AUTOBOOT_BOOT,
>>  	AUTOBOOT_UNKNOWN,
>> -- 
>> 2.39.5
>>
>>
>>
> 


-- 
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 |