[PATCH v2 0/4] Add boot slot locking to bootchooser

[PATCH v2 0/4] Add boot slot locking to bootchooser

[Lars Schmidt]

This patch series adds an option to lock attempts to bootchooser.
When slots are locked, the remaining_attempts counter will not
get counted down. It is optional, so when not set, it does not
have any effect on the counter. The choice of which slot to boot
is also not affected.

v1 -> v2:
  - rename slot locking to attempt locking (Ahmad)
  - drop else clauses when not needed (Ahmad)
  - write commit message bodies
  - change migration notes to 2025.08.0
  - add attempts_locked to bootstate.dtsi

---
Lars Schmidt (4):
      bootchooser: implement locking of attempts counter
      bootchooser: extend cmd tool by option to set attempts_locked
      Documentation: bootchooser: add information about attempts_locked
      Documentation: migration-2025.08.0: add information about attempts_locked

 .../migration-guides/migration-2025.08.0.rst       |  5 +++
 Documentation/user/bootchooser.rst                 | 26 +++++++++++
 arch/arm/dts/bootstate.dtsi                        |  5 +++
 commands/bootchooser.c                             | 16 ++++++-
 common/bootchooser.c                               | 51 +++++++++++++++++++---
 include/bootchooser.h                              |  1 +
 6 files changed, 98 insertions(+), 6 deletions(-)
---
base-commit: f2dc1e3b183ce2bcc05a6ab91801d7ce186d432c
change-id: 20250616-bootchooser-lock-c84a25567412

Best regards,
-- 
Lars Schmidt <l.schmidt@pengutronix.de>

[PATCH v2 1/4] bootchooser: implement locking of attempts counter

[Lars Schmidt]

This this new global slot lock inhibits the remaining attempts counter
from decreasing.
There are ways around this, but both come with a disadvantage:
  - If we mark the old slot as bad after a good update, we lose the
    distinction between old slots that are unbootable and ones that
    are bootable. Maintaining this distinction allows a health monitor
    to explicitly boot an old slot while preventing the bootloader
    from doing it automatically
  - If we set the maximum attempts to a very high number, we keep
    writing the storage every boot, even if we don't do
In both cases, by not decreasing and increasing the remaining attempts
counter constantly the number of write cycles is also lowered.

Signed-off-by: Lars Schmidt <l.schmidt@pengutronix.de>
---
 common/bootchooser.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/common/bootchooser.c b/common/bootchooser.c
index 58032a2b576284fe65ee53ee0e49aea68e09697a..50ce73682fb6bac8e4319119169dfb8334eb8762 100644
--- a/common/bootchooser.c
+++ b/common/bootchooser.c
@@ -49,6 +49,7 @@ struct bootchooser {
 	struct state *state;
 	char *state_prefix;
 	int refs;
+	bool attempts_locked;
 
 	int verbose;
 	int dryrun;
@@ -353,6 +354,7 @@ struct bootchooser *bootchooser_get(void)
 	int ret = -EINVAL, id = 1;
 	uint32_t last_chosen;
 	static int attempts_resetted;
+	uint32_t locked;
 
 	if (bootchooser) {
 		bootchooser->refs++;
@@ -397,6 +399,14 @@ struct bootchooser *bootchooser_get(void)
 		pr_warn("using non-redundant NV instead of barebox-state\n");
 	}
 
+	/* this is an optional value */
+	bc->attempts_locked = false;
+	ret = getenv_u32(bc->state_prefix, "attempts_locked", &locked);
+	if (!ret && locked) {
+		bc->attempts_locked = true;
+		pr_debug("remaining attempt counter is locked\n");
+	}
+
 	INIT_LIST_HEAD(&bc->targets);
 
 	freep = targets = xstrdup(available_targets);
@@ -650,11 +660,14 @@ static struct bootchooser_target *bootchooser_get_target(struct bootchooser *bc)
 	return ERR_PTR(-ENOENT);
 
 found:
-	target->remaining_attempts--;
-
-	if (bc->verbose)
-		pr_info("name=%s decrementing remaining_attempts to %d\n",
-			target->name, target->remaining_attempts);
+	if (!bc->attempts_locked) {
+		target->remaining_attempts--;
+		if (bc->verbose)
+			pr_info("name=%s remaining_attempts %d\n", target->name,
+				target->remaining_attempts);
+	} else {
+		pr_info("Attempts are locked, not decreasing remaining_attempts\n");
+	}
 
 	if (bc->verbose)
 		pr_info("selected target '%s', boot '%s'\n", target->name, target->boot);

-- 
2.39.5

[PATCH v2 2/4] bootchooser: extend cmd tool by option to set attempts_locked

[Lars Schmidt]

This flag is normally set in userspace. But having
the option of setting it in bootloader gives more debugging
options.

Signed-off-by: Lars Schmidt <l.schmidt@pengutronix.de>
---
 commands/bootchooser.c | 16 +++++++++++++++-
 common/bootchooser.c   | 28 ++++++++++++++++++++++++++++
 include/bootchooser.h  |  1 +
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/commands/bootchooser.c b/commands/bootchooser.c
index 46b063e027db30073ada28487bc0434026cc081a..4199085f352b289e069f22870c0239aede38df61 100644
--- a/commands/bootchooser.c
+++ b/commands/bootchooser.c
@@ -48,8 +48,9 @@ static int do_bootchooser(int argc, char *argv[])
 	int info = 0;
 	bool done_something = false;
 	bool last_boot_successful = false;
+	int lock_state = -1;
 
-	while ((opt = getopt(argc, argv, "a:p:is")) > 0) {
+	while ((opt = getopt(argc, argv, "a:p:islL")) > 0) {
 		switch (opt) {
 		case 'a':
 			if (!strcmp(optarg, "default"))
@@ -69,6 +70,12 @@ static int do_bootchooser(int argc, char *argv[])
 		case 's':
 			last_boot_successful = true;
 			break;
+		case 'l':
+			lock_state = true;
+			break;
+		case 'L':
+			lock_state = false;
+			break;
 		default:
 			return COMMAND_ERROR_USAGE;
 		}
@@ -109,6 +116,11 @@ static int do_bootchooser(int argc, char *argv[])
 		done_something = true;
 	}
 
+	if (lock_state >= 0) {
+		ret = bootchooser_lock_attempts(bootchooser, lock_state);
+		done_something = true;
+	}
+
 	if (!done_something) {
 		printf("Nothing to do\n");
 		ret = COMMAND_ERROR_USAGE;
@@ -126,6 +138,8 @@ BAREBOX_CMD_HELP_TEXT("Options:")
 BAREBOX_CMD_HELP_OPT ("-a <n|default> [TARGETS]",  "set remaining attempts of given targets to 'n' or the default attempts")
 BAREBOX_CMD_HELP_OPT ("-p <n|default> [TARGETS]",  "set priority of given targets to 'n' or the default priority")
 BAREBOX_CMD_HELP_OPT ("-i",  "Show information about the bootchooser")
+BAREBOX_CMD_HELP_OPT ("-l",  "Enable locking of remaining attempts")
+BAREBOX_CMD_HELP_OPT ("-L",  "Disable locking of remaining attempts")
 BAREBOX_CMD_HELP_OPT ("-s",  "Mark the last boot successful")
 BAREBOX_CMD_HELP_END
 
diff --git a/common/bootchooser.c b/common/bootchooser.c
index 50ce73682fb6bac8e4319119169dfb8334eb8762..7e4622aad5dfe02d8e798a249c46cd1c048dc3c9 100644
--- a/common/bootchooser.c
+++ b/common/bootchooser.c
@@ -631,6 +631,9 @@ void bootchooser_info(struct bootchooser *bc)
 
 	printf("\nlast booted target: %s\n", bc->last_chosen ?
 	       bc->last_chosen->name : "unknown");
+
+	printf("Locking of boot attempt counter: %s",
+	       bc->attempts_locked ? "enabled" : "disabled");
 }
 
 /**
@@ -817,6 +820,31 @@ struct bootchooser_target *bootchooser_get_last_chosen(struct bootchooser *bc)
 	return bc->last_chosen;
 }
 
+/**
+ * bootchooser_lock_attempts - lock the bootchooser attempt counter
+ * @bc:		The bootchooser
+ * @locked:     Whether the attempt counter is locked or not.
+ *
+ * Instruct bootchooser to lock the boot attempts counter.
+ * This means remaining_attempts will not be counted down.
+ *
+ * Return: 0 for success, negative error code otherwise
+ */
+int bootchooser_lock_attempts(struct bootchooser *bc, bool locked)
+{
+	uint32_t not_needed;
+	/* We just need to check here, if the value exists in the device tree
+	 * So if it doesn't exist, we can inform user about it for easier debugging
+	 */
+	if (getenv_u32(bc->state_prefix, "attempts_locked", &not_needed)) {
+		pr_warn("Missing attempts_locked property in state DT node\n");
+		return -ENOENT;
+	}
+
+	bc->attempts_locked = locked;
+	return 0;
+}
+
 static int bootchooser_boot_one(struct bootchooser *bc, int *tryagain)
 {
 	char *system;
diff --git a/include/bootchooser.h b/include/bootchooser.h
index 31989163b236196d6355acb9e7d30a18b784baa2..69783b0b7494ac303a17a466d599559513fb6d17 100644
--- a/include/bootchooser.h
+++ b/include/bootchooser.h
@@ -17,6 +17,7 @@ void bootchooser_info(struct bootchooser *bootchooser);
 int bootchooser_boot(struct bootchooser *bc);
 
 struct bootchooser_target *bootchooser_get_last_chosen(struct bootchooser *bootchooser);
+int bootchooser_lock_attempts(struct bootchooser *bc, bool locked);
 const char *bootchooser_target_name(struct bootchooser_target *target);
 struct bootchooser_target *bootchooser_target_by_name(struct bootchooser *bootchooser,
 						      const char *name);

-- 
2.39.5

[PATCH v2 3/4] Documentation: bootchooser: add information about attempts_locked

[Lars Schmidt]

The new attempts_locked variable does have influence on
remaining_attempts counter.

While at it, also make mention in the bootstate.dtsi.

Signed-off-by: Lars Schmidt <l.schmidt@pengutronix.de>
---
 Documentation/user/bootchooser.rst | 23 +++++++++++++++++++++++
 arch/arm/dts/bootstate.dtsi        |  5 +++++
 2 files changed, 28 insertions(+)

diff --git a/Documentation/user/bootchooser.rst b/Documentation/user/bootchooser.rst
index 351e1d14e1ead6ba8d329c03c0bc7ed28b523df0..ab0b6c3fc895226f5aa4590944a2cd675a56d985 100644
--- a/Documentation/user/bootchooser.rst
+++ b/Documentation/user/bootchooser.rst
@@ -77,6 +77,16 @@ no remaining attempts left.
 To prevent ending up in an unbootable system after a number of failed boot
 attempts, there is also a built-in mechanism to reset the counters to their defaults,
 controlled by the ``global.bootchooser.reset_attempts`` variable.
+Alternatively, counting down the remaining attempts can be disabled by
+locking bootchooser boot attempts.
+This is done by defining a (32-bit) ``attempts_locked`` variable in the
+bootstate and setting its value to ``1`` (usually from userspace).
+
+The variable affects all targets, is optional and its absence is
+interpreted as ``0``, meaning that attempts are decremented normally.
+
+The ``attempts_locked`` value does not influence the decision on which target
+to boot if any, only whether to decrement the attempts when booting.
 
 If ``global.bootchooser.retry`` is enabled (set to ``1``), the bootchooser
 algorithm will iterate through all valid boot targets (and decrease their
@@ -107,6 +117,19 @@ on the :ref:`reset reason <reset_reason>` (i.e. != WDG) using the
 This will reset the ``remaining_attempts`` counter of the *last chosen* slot to
 its default value (``reset_attempts``).
 
+Another option is to use ``attempts_locked``. Normally this should be controlled from
+Linux userspace using the *barebox-state* tool, i.e.::
+
+  barebox-state -s  bootstate.attempts_locked=1
+
+It can also be locked via the :ref:`bootchooser command <command_bootchooser>`::
+
+  bootchooser -l
+
+or unlocked::
+
+  bootchooser -L
+
 
 .. _dt-utils: https://git.pengutronix.de/cgit/tools/dt-utils
 
diff --git a/arch/arm/dts/bootstate.dtsi b/arch/arm/dts/bootstate.dtsi
index aa767d4e3b966244b01702bbc2bdcd5c95ef7009..733441fb2697b283dc784b00b012e11ad671bb15 100644
--- a/arch/arm/dts/bootstate.dtsi
+++ b/arch/arm/dts/bootstate.dtsi
@@ -42,4 +42,9 @@ last_chosen@10 {
 		reg = <0x10 0x4>;
 		type = "uint32";
 	};
+
+	attempts_locked@14 {
+		reg = <0x14 0x4>;
+		type = "uint32";
+	};
 };

-- 
2.39.5

[PATCH v2 4/4] Documentation: migration-2025.08.0: add information about attempts_locked

[Lars Schmidt]

Document the newly added feature.

Signed-off-by: Lars Schmidt <l.schmidt@pengutronix.de>
---
 Documentation/migration-guides/migration-2025.08.0.rst | 5 +++++
 Documentation/user/bootchooser.rst                     | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/Documentation/migration-guides/migration-2025.08.0.rst b/Documentation/migration-guides/migration-2025.08.0.rst
index d13d5c770d5e7688e0299184b0b8b938db160fca..da60132e7d8c784b08a43c8032ee7e4dc7cb7991 100644
--- a/Documentation/migration-guides/migration-2025.08.0.rst
+++ b/Documentation/migration-guides/migration-2025.08.0.rst
@@ -1,3 +1,8 @@
 Release v2025.08.0
 ==================
 
+Bootchooser
+-----------
+
+Bootchooser now interprets a top-level ``attempts_locked`` variable.
+For more information, see :ref:`attempts lock feature <bootchooser,attempts_lock>`
diff --git a/Documentation/user/bootchooser.rst b/Documentation/user/bootchooser.rst
index ab0b6c3fc895226f5aa4590944a2cd675a56d985..d123e8d34e63fcfb6a725cbe1aeca40a7dbbeb87 100644
--- a/Documentation/user/bootchooser.rst
+++ b/Documentation/user/bootchooser.rst
@@ -77,6 +77,9 @@ no remaining attempts left.
 To prevent ending up in an unbootable system after a number of failed boot
 attempts, there is also a built-in mechanism to reset the counters to their defaults,
 controlled by the ``global.bootchooser.reset_attempts`` variable.
+
+.. _bootchooser,attempts_lock:
+
 Alternatively, counting down the remaining attempts can be disabled by
 locking bootchooser boot attempts.
 This is done by defining a (32-bit) ``attempts_locked`` variable in the

-- 
2.39.5

Re: [PATCH v2 1/4] bootchooser: implement locking of attempts counter

[Ahmad Fatoum]

On 6/16/25 17:06, Lars Schmidt wrote:
> This this new global slot lock inhibits the remaining attempts counter
> from decreasing.
> There are ways around this, but both come with a disadvantage:
>   - If we mark the old slot as bad after a good update, we lose the
>     distinction between old slots that are unbootable and ones that
>     are bootable. Maintaining this distinction allows a health monitor
>     to explicitly boot an old slot while preventing the bootloader
>     from doing it automatically
>   - If we set the maximum attempts to a very high number, we keep
>     writing the storage every boot, even if we don't do
> In both cases, by not decreasing and increasing the remaining attempts
> counter constantly the number of write cycles is also lowered.
> 
> Signed-off-by: Lars Schmidt <l.schmidt@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  common/bootchooser.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/common/bootchooser.c b/common/bootchooser.c
> index 58032a2b576284fe65ee53ee0e49aea68e09697a..50ce73682fb6bac8e4319119169dfb8334eb8762 100644
> --- a/common/bootchooser.c
> +++ b/common/bootchooser.c
> @@ -49,6 +49,7 @@ struct bootchooser {
>  	struct state *state;
>  	char *state_prefix;
>  	int refs;
> +	bool attempts_locked;
>  
>  	int verbose;
>  	int dryrun;
> @@ -353,6 +354,7 @@ struct bootchooser *bootchooser_get(void)
>  	int ret = -EINVAL, id = 1;
>  	uint32_t last_chosen;
>  	static int attempts_resetted;
> +	uint32_t locked;
>  
>  	if (bootchooser) {
>  		bootchooser->refs++;
> @@ -397,6 +399,14 @@ struct bootchooser *bootchooser_get(void)
>  		pr_warn("using non-redundant NV instead of barebox-state\n");
>  	}
>  
> +	/* this is an optional value */
> +	bc->attempts_locked = false;
> +	ret = getenv_u32(bc->state_prefix, "attempts_locked", &locked);
> +	if (!ret && locked) {
> +		bc->attempts_locked = true;
> +		pr_debug("remaining attempt counter is locked\n");
> +	}
> +
>  	INIT_LIST_HEAD(&bc->targets);
>  
>  	freep = targets = xstrdup(available_targets);
> @@ -650,11 +660,14 @@ static struct bootchooser_target *bootchooser_get_target(struct bootchooser *bc)
>  	return ERR_PTR(-ENOENT);
>  
>  found:
> -	target->remaining_attempts--;
> -
> -	if (bc->verbose)
> -		pr_info("name=%s decrementing remaining_attempts to %d\n",
> -			target->name, target->remaining_attempts);
> +	if (!bc->attempts_locked) {
> +		target->remaining_attempts--;
> +		if (bc->verbose)
> +			pr_info("name=%s remaining_attempts %d\n", target->name,
> +				target->remaining_attempts);
> +	} else {
> +		pr_info("Attempts are locked, not decreasing remaining_attempts\n");
> +	}
>  
>  	if (bc->verbose)
>  		pr_info("selected target '%s', boot '%s'\n", target->name, target->boot);
> 

-- 
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 v2 2/4] bootchooser: extend cmd tool by option to set attempts_locked

[Ahmad Fatoum]

On 6/16/25 17:06, Lars Schmidt wrote:
> This flag is normally set in userspace. But having
> the option of setting it in bootloader gives more debugging
> options.
> 
> Signed-off-by: Lars Schmidt <l.schmidt@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

There's a boot test pending review[1]. I intend to submit a test
exercising this feature and using the bootchooser -l/-L command later.

[1]:
https://lore.barebox.org/barebox/20250612085603.4190573-1-a.fatoum@barebox.org/T/#t

Cheers,
Ahmad

> ---
>  commands/bootchooser.c | 16 +++++++++++++++-
>  common/bootchooser.c   | 28 ++++++++++++++++++++++++++++
>  include/bootchooser.h  |  1 +
>  3 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/commands/bootchooser.c b/commands/bootchooser.c
> index 46b063e027db30073ada28487bc0434026cc081a..4199085f352b289e069f22870c0239aede38df61 100644
> --- a/commands/bootchooser.c
> +++ b/commands/bootchooser.c
> @@ -48,8 +48,9 @@ static int do_bootchooser(int argc, char *argv[])
>  	int info = 0;
>  	bool done_something = false;
>  	bool last_boot_successful = false;
> +	int lock_state = -1;
>  
> -	while ((opt = getopt(argc, argv, "a:p:is")) > 0) {
> +	while ((opt = getopt(argc, argv, "a:p:islL")) > 0) {
>  		switch (opt) {
>  		case 'a':
>  			if (!strcmp(optarg, "default"))
> @@ -69,6 +70,12 @@ static int do_bootchooser(int argc, char *argv[])
>  		case 's':
>  			last_boot_successful = true;
>  			break;
> +		case 'l':
> +			lock_state = true;
> +			break;
> +		case 'L':
> +			lock_state = false;
> +			break;
>  		default:
>  			return COMMAND_ERROR_USAGE;
>  		}
> @@ -109,6 +116,11 @@ static int do_bootchooser(int argc, char *argv[])
>  		done_something = true;
>  	}
>  
> +	if (lock_state >= 0) {
> +		ret = bootchooser_lock_attempts(bootchooser, lock_state);
> +		done_something = true;
> +	}
> +
>  	if (!done_something) {
>  		printf("Nothing to do\n");
>  		ret = COMMAND_ERROR_USAGE;
> @@ -126,6 +138,8 @@ BAREBOX_CMD_HELP_TEXT("Options:")
>  BAREBOX_CMD_HELP_OPT ("-a <n|default> [TARGETS]",  "set remaining attempts of given targets to 'n' or the default attempts")
>  BAREBOX_CMD_HELP_OPT ("-p <n|default> [TARGETS]",  "set priority of given targets to 'n' or the default priority")
>  BAREBOX_CMD_HELP_OPT ("-i",  "Show information about the bootchooser")
> +BAREBOX_CMD_HELP_OPT ("-l",  "Enable locking of remaining attempts")
> +BAREBOX_CMD_HELP_OPT ("-L",  "Disable locking of remaining attempts")
>  BAREBOX_CMD_HELP_OPT ("-s",  "Mark the last boot successful")
>  BAREBOX_CMD_HELP_END
>  
> diff --git a/common/bootchooser.c b/common/bootchooser.c
> index 50ce73682fb6bac8e4319119169dfb8334eb8762..7e4622aad5dfe02d8e798a249c46cd1c048dc3c9 100644
> --- a/common/bootchooser.c
> +++ b/common/bootchooser.c
> @@ -631,6 +631,9 @@ void bootchooser_info(struct bootchooser *bc)
>  
>  	printf("\nlast booted target: %s\n", bc->last_chosen ?
>  	       bc->last_chosen->name : "unknown");
> +
> +	printf("Locking of boot attempt counter: %s",
> +	       bc->attempts_locked ? "enabled" : "disabled");
>  }
>  
>  /**
> @@ -817,6 +820,31 @@ struct bootchooser_target *bootchooser_get_last_chosen(struct bootchooser *bc)
>  	return bc->last_chosen;
>  }
>  
> +/**
> + * bootchooser_lock_attempts - lock the bootchooser attempt counter
> + * @bc:		The bootchooser
> + * @locked:     Whether the attempt counter is locked or not.
> + *
> + * Instruct bootchooser to lock the boot attempts counter.
> + * This means remaining_attempts will not be counted down.
> + *
> + * Return: 0 for success, negative error code otherwise
> + */
> +int bootchooser_lock_attempts(struct bootchooser *bc, bool locked)
> +{
> +	uint32_t not_needed;
> +	/* We just need to check here, if the value exists in the device tree
> +	 * So if it doesn't exist, we can inform user about it for easier debugging
> +	 */
> +	if (getenv_u32(bc->state_prefix, "attempts_locked", &not_needed)) {
> +		pr_warn("Missing attempts_locked property in state DT node\n");
> +		return -ENOENT;
> +	}
> +
> +	bc->attempts_locked = locked;
> +	return 0;
> +}
> +
>  static int bootchooser_boot_one(struct bootchooser *bc, int *tryagain)
>  {
>  	char *system;
> diff --git a/include/bootchooser.h b/include/bootchooser.h
> index 31989163b236196d6355acb9e7d30a18b784baa2..69783b0b7494ac303a17a466d599559513fb6d17 100644
> --- a/include/bootchooser.h
> +++ b/include/bootchooser.h
> @@ -17,6 +17,7 @@ void bootchooser_info(struct bootchooser *bootchooser);
>  int bootchooser_boot(struct bootchooser *bc);
>  
>  struct bootchooser_target *bootchooser_get_last_chosen(struct bootchooser *bootchooser);
> +int bootchooser_lock_attempts(struct bootchooser *bc, bool locked);
>  const char *bootchooser_target_name(struct bootchooser_target *target);
>  struct bootchooser_target *bootchooser_target_by_name(struct bootchooser *bootchooser,
>  						      const char *name);
> 

-- 
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 v2 3/4] Documentation: bootchooser: add information about attempts_locked

[Ahmad Fatoum]

On 6/16/25 17:06, Lars Schmidt wrote:
> The new attempts_locked variable does have influence on
> remaining_attempts counter.

This commit message feels incomplete.

> 
> While at it, also make mention in the bootstate.dtsi.
> 
> Signed-off-by: Lars Schmidt <l.schmidt@pengutronix.de>

The diff looks fine though:

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  Documentation/user/bootchooser.rst | 23 +++++++++++++++++++++++
>  arch/arm/dts/bootstate.dtsi        |  5 +++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/Documentation/user/bootchooser.rst b/Documentation/user/bootchooser.rst
> index 351e1d14e1ead6ba8d329c03c0bc7ed28b523df0..ab0b6c3fc895226f5aa4590944a2cd675a56d985 100644
> --- a/Documentation/user/bootchooser.rst
> +++ b/Documentation/user/bootchooser.rst
> @@ -77,6 +77,16 @@ no remaining attempts left.
>  To prevent ending up in an unbootable system after a number of failed boot
>  attempts, there is also a built-in mechanism to reset the counters to their defaults,
>  controlled by the ``global.bootchooser.reset_attempts`` variable.
> +Alternatively, counting down the remaining attempts can be disabled by
> +locking bootchooser boot attempts.
> +This is done by defining a (32-bit) ``attempts_locked`` variable in the
> +bootstate and setting its value to ``1`` (usually from userspace).
> +
> +The variable affects all targets, is optional and its absence is
> +interpreted as ``0``, meaning that attempts are decremented normally.
> +
> +The ``attempts_locked`` value does not influence the decision on which target
> +to boot if any, only whether to decrement the attempts when booting.
>  
>  If ``global.bootchooser.retry`` is enabled (set to ``1``), the bootchooser
>  algorithm will iterate through all valid boot targets (and decrease their
> @@ -107,6 +117,19 @@ on the :ref:`reset reason <reset_reason>` (i.e. != WDG) using the
>  This will reset the ``remaining_attempts`` counter of the *last chosen* slot to
>  its default value (``reset_attempts``).
>  
> +Another option is to use ``attempts_locked``. Normally this should be controlled from
> +Linux userspace using the *barebox-state* tool, i.e.::
> +
> +  barebox-state -s  bootstate.attempts_locked=1
> +
> +It can also be locked via the :ref:`bootchooser command <command_bootchooser>`::
> +
> +  bootchooser -l
> +
> +or unlocked::
> +
> +  bootchooser -L
> +
>  
>  .. _dt-utils: https://git.pengutronix.de/cgit/tools/dt-utils
>  
> diff --git a/arch/arm/dts/bootstate.dtsi b/arch/arm/dts/bootstate.dtsi
> index aa767d4e3b966244b01702bbc2bdcd5c95ef7009..733441fb2697b283dc784b00b012e11ad671bb15 100644
> --- a/arch/arm/dts/bootstate.dtsi
> +++ b/arch/arm/dts/bootstate.dtsi
> @@ -42,4 +42,9 @@ last_chosen@10 {
>  		reg = <0x10 0x4>;
>  		type = "uint32";
>  	};
> +
> +	attempts_locked@14 {
> +		reg = <0x14 0x4>;
> +		type = "uint32";
> +	};
>  };
> 

-- 
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 v2 3/4] Documentation: bootchooser: add information about attempts_locked

[Sascha Hauer]

On Mon, Jun 16, 2025 at 05:06:56PM +0200, Lars Schmidt wrote:
> The new attempts_locked variable does have influence on
> remaining_attempts counter.
> 
> While at it, also make mention in the bootstate.dtsi.
> 
> Signed-off-by: Lars Schmidt <l.schmidt@pengutronix.de>
> ---
>  Documentation/user/bootchooser.rst | 23 +++++++++++++++++++++++
>  arch/arm/dts/bootstate.dtsi        |  5 +++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/Documentation/user/bootchooser.rst b/Documentation/user/bootchooser.rst
> index 351e1d14e1ead6ba8d329c03c0bc7ed28b523df0..ab0b6c3fc895226f5aa4590944a2cd675a56d985 100644
> --- a/Documentation/user/bootchooser.rst
> +++ b/Documentation/user/bootchooser.rst
> @@ -77,6 +77,16 @@ no remaining attempts left.
>  To prevent ending up in an unbootable system after a number of failed boot
>  attempts, there is also a built-in mechanism to reset the counters to their defaults,
>  controlled by the ``global.bootchooser.reset_attempts`` variable.
> +Alternatively, counting down the remaining attempts can be disabled by
> +locking bootchooser boot attempts.
> +This is done by defining a (32-bit) ``attempts_locked`` variable in the
> +bootstate and setting its value to ``1`` (usually from userspace).
> +
> +The variable affects all targets, is optional and its absence is
> +interpreted as ``0``, meaning that attempts are decremented normally.
> +
> +The ``attempts_locked`` value does not influence the decision on which target
> +to boot if any, only whether to decrement the attempts when booting.

This describes what it does, but not what it's good for. Could you add a
few sentences about it?

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 v2 3/4] Documentation: bootchooser: add information about attempts_locked

[Lars Schmidt]

On 17.06.25 10:42, Sascha Hauer wrote:
> On Mon, Jun 16, 2025 at 05:06:56PM +0200, Lars Schmidt wrote:
>> The new attempts_locked variable does have influence on
>> remaining_attempts counter.
>>
>> While at it, also make mention in the bootstate.dtsi.
>>
>> Signed-off-by: Lars Schmidt <l.schmidt@pengutronix.de>
>> ---
>>  Documentation/user/bootchooser.rst | 23 +++++++++++++++++++++++
>>  arch/arm/dts/bootstate.dtsi        |  5 +++++
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/Documentation/user/bootchooser.rst b/Documentation/user/bootchooser.rst
>> index 351e1d14e1ead6ba8d329c03c0bc7ed28b523df0..ab0b6c3fc895226f5aa4590944a2cd675a56d985 100644
>> --- a/Documentation/user/bootchooser.rst
>> +++ b/Documentation/user/bootchooser.rst
>> @@ -77,6 +77,16 @@ no remaining attempts left.
>>  To prevent ending up in an unbootable system after a number of failed boot
>>  attempts, there is also a built-in mechanism to reset the counters to their defaults,
>>  controlled by the ``global.bootchooser.reset_attempts`` variable.
>> +Alternatively, counting down the remaining attempts can be disabled by
>> +locking bootchooser boot attempts.
>> +This is done by defining a (32-bit) ``attempts_locked`` variable in the
>> +bootstate and setting its value to ``1`` (usually from userspace).
>> +
>> +The variable affects all targets, is optional and its absence is
>> +interpreted as ``0``, meaning that attempts are decremented normally.
>> +
>> +The ``attempts_locked`` value does not influence the decision on which target
>> +to boot if any, only whether to decrement the attempts when booting.
> This describes what it does, but not what it's good for. Could you add a
> few sentences about it?

I was hoping this is understandable if you read in the context of the already
existing paragraph right before that: "To prevent ending up in an unbootable system....."
As it is meant to be an alternative to global.bootchooser.reset_attempts

I didn't want to get too repetitive, but if you want, I can try to rephrase it.

>
> Sascha
>
Lars

Re: [PATCH v2 3/4] Documentation: bootchooser: add information about attempts_locked

[Sascha Hauer]

On Tue, Jun 17, 2025 at 11:00:10AM +0200, Lars Schmidt wrote:
> 
> On 17.06.25 10:42, Sascha Hauer wrote:
> > On Mon, Jun 16, 2025 at 05:06:56PM +0200, Lars Schmidt wrote:
> >> The new attempts_locked variable does have influence on
> >> remaining_attempts counter.
> >>
> >> While at it, also make mention in the bootstate.dtsi.
> >>
> >> Signed-off-by: Lars Schmidt <l.schmidt@pengutronix.de>
> >> ---
> >>  Documentation/user/bootchooser.rst | 23 +++++++++++++++++++++++
> >>  arch/arm/dts/bootstate.dtsi        |  5 +++++
> >>  2 files changed, 28 insertions(+)
> >>
> >> diff --git a/Documentation/user/bootchooser.rst b/Documentation/user/bootchooser.rst
> >> index 351e1d14e1ead6ba8d329c03c0bc7ed28b523df0..ab0b6c3fc895226f5aa4590944a2cd675a56d985 100644
> >> --- a/Documentation/user/bootchooser.rst
> >> +++ b/Documentation/user/bootchooser.rst
> >> @@ -77,6 +77,16 @@ no remaining attempts left.
> >>  To prevent ending up in an unbootable system after a number of failed boot
> >>  attempts, there is also a built-in mechanism to reset the counters to their defaults,
> >>  controlled by the ``global.bootchooser.reset_attempts`` variable.
> >> +Alternatively, counting down the remaining attempts can be disabled by
> >> +locking bootchooser boot attempts.
> >> +This is done by defining a (32-bit) ``attempts_locked`` variable in the
> >> +bootstate and setting its value to ``1`` (usually from userspace).
> >> +
> >> +The variable affects all targets, is optional and its absence is
> >> +interpreted as ``0``, meaning that attempts are decremented normally.
> >> +
> >> +The ``attempts_locked`` value does not influence the decision on which target
> >> +to boot if any, only whether to decrement the attempts when booting.
> > This describes what it does, but not what it's good for. Could you add a
> > few sentences about it?
> 
> I was hoping this is understandable if you read in the context of the already
> existing paragraph right before that: "To prevent ending up in an unbootable system....."
> As it is meant to be an alternative to global.bootchooser.reset_attempts
> 
> I didn't want to get too repetitive, but if you want, I can try to rephrase it.

How I understand it the purpose of the attempts_locked variable is that
I can mark a slot as always-use-this-one.

There are some differences to the reset_attempts approach. With
reset_attempts barebox decreases the counter and Linux increases it
again. When a board is power cycled often enough (due to an impatient
user or weak power supply) after barebox has decreased the counter but
before Linux has increased it again then we'll fall back to another slot.
This might be undesired in some cases and the attempts_lock doesn't have
this issue, but it comes with the price that a slot that gets broken
over time (filesystem corruption or the like) won't be detected anymore
and will be booted forever.

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 |