[PATCH] bootchooser: honour reset source

[PATCH] bootchooser: honour reset source

[Holger Assmann]

With some systems it is possible to determine the source that triggered
the most recent reset (see $global.system.reset).

This information can be used at the subsequent boot to evaluate whether
the respective target works as intended by defining reset reasons that
are to be used exclusively. Any other reset source would therefore be
considered to be bad, which should lead to a decrease of the counter
variable "remaining_attempts" for that target.
Until now, such an investigation and the consecutive decision of marking
the last boot successful has either to be done by a custom script in
barebox or by a service that runs on the eventually booted operating
system.

Since (if supported by the system) analyzing the reset reason is
probably the most obvious factor for the marking decision, we might as
well integrate the functionality into bootchooser directly.

This patch therefore introduces "$global.bootchooser.good_reset_sources"
as a new variable in which good reset sources can be listed to be
matched against the actual reset reason.

Signed-off-by: Holger Assmann <h.assmann@pengutronix.de>
---
 Documentation/user/bootchooser.rst | 67 +++++++++++++++++++++++++-----
 common/bootchooser.c               | 22 ++++++++++
 2 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/Documentation/user/bootchooser.rst b/Documentation/user/bootchooser.rst
index db0a4f8898..bce1aed24d 100644
--- a/Documentation/user/bootchooser.rst
+++ b/Documentation/user/bootchooser.rst
@@ -108,6 +108,20 @@ While the bootchooser algorithm handles attempts decrementation, retries and
 selection of the right boot target itself, it cannot decide if the system
 booted successfully on its own.
 
+However, for systems where barebox is able to detect the actual :ref:`reset reason <reset_reason>`
+(e.g. WDG), bootchooser will look for matches in ``global.bootchooser.good_reset_sources``.
+This variable may contain a space-separated list of those reset reasons that are
+considered *good*. If a matching entry is found, bootchooser will mark the last
+boot successful.
+
+This marking can also be performed manually or from within a script by calling
+the barebox :ref:`bootchoser command <command_bootchooser>`:: 
+
+  bootchooser -s
+
+Marking the preceding boot successful will result in the ``remaining_attempts`` counter of the
+*last chosen* slot to be reset to its default value (``reset_attempts``).
+
 In case only the booted system itself knows when it is in a good state,
 it can report this to the bootchooser from Linux userspace using the
 *barebox-state* tool from the dt-utils_ package.::
@@ -116,16 +130,6 @@ it can report this to the bootchooser from Linux userspace using the
   barebox-state -n system_state -s bootstate.system1.remaining_attempts=3
   barebox-state -s system1.remaining_attempts=3
 
-If instead the bootchooser can detect a failed boot itself using the
-:ref:`reset reason <reset_reason>` (WDG), one can mark the boot successful
-using the barebox :ref:`bootchoser command <command_bootchooser>`::
-
-  bootchooser -s
-
-to mark the last boot successful.
-This will reset the ``remaining_attempts`` counter of the *last chosen* slot to
-its default value (``reset_attempts``).
-
 
 .. _dt-utils: https://git.pengutronix.de/cgit/tools/dt-utils
 
@@ -147,6 +151,8 @@ options not specific to any boot target.
   specific variable of the same name.
 ``global.bootchooser.reset_attempts``
   Already described in :ref:`Bootchooser Algorithm <bootchooser,algorithm>`
+``bootchooser.good_reset_sources``
+  Already described in :ref:`Bootchooser Algorithm <bootchooser,algorithm>`
 ``global.bootchooser.reset_priorities``
   A space-separated list of events that cause *bootchooser* to reset the priorities of
   all boot targets. Possible values:
@@ -268,6 +274,7 @@ are reset to their defaults and the first boot target is tried again.
 Settings
 ^^^^^^^^
 - ``global.bootchooser.reset_attempts="all-zero"``
+- ``global.bootchooser.good_reset_sources=""``
 - ``global.bootchooser.reset_priorities="all-zero"``
 - ``global.bootchooser.disable_on_zero_attempts=0``
 - ``global.bootchooser.retry=1``
@@ -302,6 +309,7 @@ Settings
 ^^^^^^^^
 
 - ``global.bootchooser.reset_attempts=""``
+- ``global.bootchooser.good_reset_sources=""``
 - ``global.bootchooser.reset_priorities=""``
 - ``global.bootchooser.disable_on_zero_attempts=0``
 - ``global.bootchooser.retry=1``
@@ -337,6 +345,7 @@ Settings
 ^^^^^^^^
 
 - ``global.bootchooser.reset_attempts="power-on"``
+- ``global.bootchooser.good_reset_sources=""``
 - ``global.bootchooser.reset_priorities=""``
 - ``global.bootchooser.disable_on_zero_attempts=1``
 - ``global.bootchooser.retry=1``
@@ -358,6 +367,44 @@ through due to the lack of bootable targets. This target can be:
 - a system that will be booted as recovery.
 - a barebox script that will be started.
 
+Scenario 4
+##########
+
+- a system with multiple boot targets
+- one recovery system
+- detection of the :ref:`cause of the preceding reset <reset_reason>` is
+  supported by the hardware
+
+  - POR (Power On Reset) and RST (ReSeT) are considered *good* causes
+
+Booting a boot target three times without success disables it.
+
+Settings
+^^^^^^^^
+
+- ``global.bootchooser.reset_attempts=""``
+- ``global.bootchooser.good_reset_sources="POR RST"``
+- ``global.bootchooser.reset_priorities=""``
+- ``global.bootchooser.disable_on_zero_attempts=1``
+- ``global.bootchooser.retry=1``
+- ``global.boot.default="bootchooser recovery"``
+- bootchooser marks as good.
+
+Deployment
+^^^^^^^^^^
+
+#. barebox or flash robot fills all boot targets with valid systems.
+#. barebox or flash robot marks boot targets as good.
+
+Recovery
+^^^^^^^^
+
+Done by 'recovery' boot target which is booted after the *bootchooser* falls
+through due to the lack of bootable targets. This target can be:
+
+- a system that will be booted as recovery.
+- a barebox script that will be started.
+
 .. _bootchooser,state_framework:
 
 Using the *State* Framework as Backend for Run-Time Variable Data
diff --git a/common/bootchooser.c b/common/bootchooser.c
index eb3dda52ab..ecc9c33312 100644
--- a/common/bootchooser.c
+++ b/common/bootchooser.c
@@ -35,6 +35,7 @@
 #define BOOTCHOOSER_PREFIX "global.bootchooser"
 
 static char *available_targets;
+static char *global_good_reset_sources;
 static char *state_prefix;
 static int global_default_attempts = 3;
 static int global_default_priority = 1;
@@ -348,6 +349,7 @@ struct bootchooser *bootchooser_get(void)
 {
 	struct bootchooser *bc;
 	struct bootchooser_target *target;
+	enum reset_src_type type = reset_source_get();
 	char *targets, *str, *freep = NULL, *delim;
 	int ret = -EINVAL, id = 1;
 	uint32_t last_chosen;
@@ -451,6 +453,20 @@ struct bootchooser *bootchooser_get(void)
 		}
 	}
 
+  	/* no multiple substrings in reset_src_type */
+  	if (!last_boot_successful &&
+		strstr(global_good_reset_sources, reset_source_to_string(type))) {
+		/*
+		 * If there are reset sources defined that are considered good
+		 * and the reset source detected by the system fits an entry of
+		 * that list, we do not want to further decrement the
+		 * remaining_attempts counter.
+		 */
+  		pr_info("Good Reset '%s', marking last boot successful\n",
+  				reset_source_to_string(type));
+		last_boot_successful = true;
+  	}
+
 	ret = getenv_u32(bc->state_prefix, "last_chosen", &last_chosen);
 	if (!ret && last_chosen > 0) {
 		bc->last_chosen = bootchooser_target_by_id(bc, last_chosen);
@@ -925,11 +941,13 @@ static int bootchooser_init(void)
 {
 	state_prefix = xstrdup("");
 	available_targets = xstrdup("");
+	global_good_reset_sources = xstrdup("");
 
 	globalvar_add_simple_bool("bootchooser.disable_on_zero_attempts", &disable_on_zero_attempts);
 	globalvar_add_simple_bool("bootchooser.retry", &retry);
 	globalvar_add_simple_string("bootchooser.targets", &available_targets);
 	globalvar_add_simple_string("bootchooser.state_prefix", &state_prefix);
+	globalvar_add_simple_string("bootchooser.good_reset_sources", &global_good_reset_sources);
 	globalvar_add_simple_int("bootchooser.default_attempts", &global_default_attempts, "%u");
 	globalvar_add_simple_int("bootchooser.default_priority", &global_default_priority, "%u");
 	globalvar_add_simple_bitmask("bootchooser.reset_attempts", &reset_attempts,
@@ -951,6 +969,10 @@ BAREBOX_MAGICVAR(global.bootchooser.targets,
 		 "bootchooser: Space separated list of target names");
 BAREBOX_MAGICVAR(global.bootchooser.default_attempts,
 		 "bootchooser: Default number of attempts for a target");
+BAREBOX_MAGICVAR(global.bootchooser.reset_attempts,
+		"bootchooser: Choose condition to reset number of attempts for all targets ('power-on', 'all-zero')");
+BAREBOX_MAGICVAR(global.bootchooser.good_reset_sources,
+		"bootchooser: Space-separated list of good reset sources consulted to not decrease remainin attempts counter");
 BAREBOX_MAGICVAR(global.bootchooser.default_priority,
 		 "bootchooser: Default priority for a target");
 BAREBOX_MAGICVAR(global.bootchooser.state_prefix,
-- 
2.39.2

Re: [PATCH] bootchooser: honour reset source

[Sascha Hauer]

On Mon, Jul 24, 2023 at 09:58:17AM +0200, Holger Assmann wrote:
> With some systems it is possible to determine the source that triggered
> the most recent reset (see $global.system.reset).
> 
> This information can be used at the subsequent boot to evaluate whether
> the respective target works as intended by defining reset reasons that
> are to be used exclusively. Any other reset source would therefore be
> considered to be bad, which should lead to a decrease of the counter
> variable "remaining_attempts" for that target.
> Until now, such an investigation and the consecutive decision of marking
> the last boot successful has either to be done by a custom script in
> barebox or by a service that runs on the eventually booted operating
> system.
> 
> Since (if supported by the system) analyzing the reset reason is
> probably the most obvious factor for the marking decision, we might as
> well integrate the functionality into bootchooser directly.
> 
> This patch therefore introduces "$global.bootchooser.good_reset_sources"
> as a new variable in which good reset sources can be listed to be
> matched against the actual reset reason.
> 
> Signed-off-by: Holger Assmann <h.assmann@pengutronix.de>
> ---
>  Documentation/user/bootchooser.rst | 67 +++++++++++++++++++++++++-----
>  common/bootchooser.c               | 22 ++++++++++
>  2 files changed, 79 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/user/bootchooser.rst b/Documentation/user/bootchooser.rst
> index db0a4f8898..bce1aed24d 100644
> --- a/Documentation/user/bootchooser.rst
> +++ b/Documentation/user/bootchooser.rst
> @@ -108,6 +108,20 @@ While the bootchooser algorithm handles attempts decrementation, retries and
>  selection of the right boot target itself, it cannot decide if the system
>  booted successfully on its own.
>  
> +However, for systems where barebox is able to detect the actual :ref:`reset reason <reset_reason>`
> +(e.g. WDG), bootchooser will look for matches in ``global.bootchooser.good_reset_sources``.
> +This variable may contain a space-separated list of those reset reasons that are
> +considered *good*. If a matching entry is found, bootchooser will mark the last
> +boot successful.
> +
> +This marking can also be performed manually or from within a script by calling
> +the barebox :ref:`bootchoser command <command_bootchooser>`:: 
> +
> +  bootchooser -s
> +
> +Marking the preceding boot successful will result in the ``remaining_attempts`` counter of the
> +*last chosen* slot to be reset to its default value (``reset_attempts``).
> +
>  In case only the booted system itself knows when it is in a good state,
>  it can report this to the bootchooser from Linux userspace using the
>  *barebox-state* tool from the dt-utils_ package.::
> @@ -116,16 +130,6 @@ it can report this to the bootchooser from Linux userspace using the
>    barebox-state -n system_state -s bootstate.system1.remaining_attempts=3
>    barebox-state -s system1.remaining_attempts=3
>  
> -If instead the bootchooser can detect a failed boot itself using the
> -:ref:`reset reason <reset_reason>` (WDG), one can mark the boot successful
> -using the barebox :ref:`bootchoser command <command_bootchooser>`::
> -
> -  bootchooser -s
> -
> -to mark the last boot successful.
> -This will reset the ``remaining_attempts`` counter of the *last chosen* slot to
> -its default value (``reset_attempts``).
> -
>  
>  .. _dt-utils: https://git.pengutronix.de/cgit/tools/dt-utils
>  
> @@ -147,6 +151,8 @@ options not specific to any boot target.
>    specific variable of the same name.
>  ``global.bootchooser.reset_attempts``
>    Already described in :ref:`Bootchooser Algorithm <bootchooser,algorithm>`
> +``bootchooser.good_reset_sources``
> +  Already described in :ref:`Bootchooser Algorithm <bootchooser,algorithm>`
>  ``global.bootchooser.reset_priorities``
>    A space-separated list of events that cause *bootchooser* to reset the priorities of
>    all boot targets. Possible values:
> @@ -268,6 +274,7 @@ are reset to their defaults and the first boot target is tried again.
>  Settings
>  ^^^^^^^^
>  - ``global.bootchooser.reset_attempts="all-zero"``
> +- ``global.bootchooser.good_reset_sources=""``
>  - ``global.bootchooser.reset_priorities="all-zero"``
>  - ``global.bootchooser.disable_on_zero_attempts=0``
>  - ``global.bootchooser.retry=1``
> @@ -302,6 +309,7 @@ Settings
>  ^^^^^^^^
>  
>  - ``global.bootchooser.reset_attempts=""``
> +- ``global.bootchooser.good_reset_sources=""``
>  - ``global.bootchooser.reset_priorities=""``
>  - ``global.bootchooser.disable_on_zero_attempts=0``
>  - ``global.bootchooser.retry=1``
> @@ -337,6 +345,7 @@ Settings
>  ^^^^^^^^
>  
>  - ``global.bootchooser.reset_attempts="power-on"``
> +- ``global.bootchooser.good_reset_sources=""``
>  - ``global.bootchooser.reset_priorities=""``
>  - ``global.bootchooser.disable_on_zero_attempts=1``
>  - ``global.bootchooser.retry=1``
> @@ -358,6 +367,44 @@ through due to the lack of bootable targets. This target can be:
>  - a system that will be booted as recovery.
>  - a barebox script that will be started.
>  
> +Scenario 4
> +##########
> +
> +- a system with multiple boot targets
> +- one recovery system
> +- detection of the :ref:`cause of the preceding reset <reset_reason>` is
> +  supported by the hardware
> +
> +  - POR (Power On Reset) and RST (ReSeT) are considered *good* causes
> +
> +Booting a boot target three times without success disables it.
> +
> +Settings
> +^^^^^^^^
> +
> +- ``global.bootchooser.reset_attempts=""``
> +- ``global.bootchooser.good_reset_sources="POR RST"``
> +- ``global.bootchooser.reset_priorities=""``
> +- ``global.bootchooser.disable_on_zero_attempts=1``
> +- ``global.bootchooser.retry=1``
> +- ``global.boot.default="bootchooser recovery"``
> +- bootchooser marks as good.
> +
> +Deployment
> +^^^^^^^^^^
> +
> +#. barebox or flash robot fills all boot targets with valid systems.
> +#. barebox or flash robot marks boot targets as good.
> +
> +Recovery
> +^^^^^^^^
> +
> +Done by 'recovery' boot target which is booted after the *bootchooser* falls
> +through due to the lack of bootable targets. This target can be:
> +
> +- a system that will be booted as recovery.
> +- a barebox script that will be started.
> +
>  .. _bootchooser,state_framework:
>  
>  Using the *State* Framework as Backend for Run-Time Variable Data
> diff --git a/common/bootchooser.c b/common/bootchooser.c
> index eb3dda52ab..ecc9c33312 100644
> --- a/common/bootchooser.c
> +++ b/common/bootchooser.c
> @@ -35,6 +35,7 @@
>  #define BOOTCHOOSER_PREFIX "global.bootchooser"
>  
>  static char *available_targets;
> +static char *global_good_reset_sources;
>  static char *state_prefix;
>  static int global_default_attempts = 3;
>  static int global_default_priority = 1;
> @@ -348,6 +349,7 @@ struct bootchooser *bootchooser_get(void)
>  {
>  	struct bootchooser *bc;
>  	struct bootchooser_target *target;
> +	enum reset_src_type type = reset_source_get();
>  	char *targets, *str, *freep = NULL, *delim;
>  	int ret = -EINVAL, id = 1;
>  	uint32_t last_chosen;
> @@ -451,6 +453,20 @@ struct bootchooser *bootchooser_get(void)
>  		}
>  	}
>  
> +  	/* no multiple substrings in reset_src_type */

I think we should better not rely on that and do proper matching
instead.

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 |