mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Add boot slot locking to bootchooser
@ 2025-06-19  8:25 Lars Schmidt
  2025-06-19  8:25 ` [PATCH v3 1/4] bootchooser: implement locking of attempts counter Lars Schmidt
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Lars Schmidt @ 2025-06-19  8:25 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: 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.

---
Changes in v3:
- documentation: improve documentation to better describe the use case
- reword commit message for documentation
- change comment in common/bootchooser.c to fit into 80 chars
- Link to v2: https://lore.kernel.org/r/20250616-bootchooser-lock-v2-0-df1f0d118635@pengutronix.de

Changes in 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
- Link to v2: https://lore.kernel.org/barebox/20250613140805.1229525-1-l.schmidt@pengutronix.de

---
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                 | 33 ++++++++++++++
 arch/arm/dts/bootstate.dtsi                        |  5 +++
 commands/bootchooser.c                             | 16 ++++++-
 common/bootchooser.c                               | 51 +++++++++++++++++++---
 include/bootchooser.h                              |  1 +
 6 files changed, 105 insertions(+), 6 deletions(-)
---
base-commit: f2dc1e3b183ce2bcc05a6ab91801d7ce186d432c
change-id: 20250616-bootchooser-lock-c84a25567412

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




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/4] bootchooser: implement locking of attempts counter
  2025-06-19  8:25 [PATCH v3 0/4] Add boot slot locking to bootchooser Lars Schmidt
@ 2025-06-19  8:25 ` Lars Schmidt
  2025-06-19  8:25 ` [PATCH v3 2/4] bootchooser: extend cmd tool by option to set attempts_locked Lars Schmidt
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Lars Schmidt @ 2025-06-19  8:25 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: 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




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 2/4] bootchooser: extend cmd tool by option to set attempts_locked
  2025-06-19  8:25 [PATCH v3 0/4] Add boot slot locking to bootchooser Lars Schmidt
  2025-06-19  8:25 ` [PATCH v3 1/4] bootchooser: implement locking of attempts counter Lars Schmidt
@ 2025-06-19  8:25 ` Lars Schmidt
  2025-06-19  8:25 ` [PATCH v3 3/4] Documentation: bootchooser: add information about attempts_locked Lars Schmidt
  2025-06-19  8:25 ` [PATCH v3 4/4] Documentation: migration-2025.08.0: " Lars Schmidt
  3 siblings, 0 replies; 5+ messages in thread
From: Lars Schmidt @ 2025-06-19  8:25 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: 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..f2beed543db008f916f8a915574bdbb32a1946af 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, 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




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 3/4] Documentation: bootchooser: add information about attempts_locked
  2025-06-19  8:25 [PATCH v3 0/4] Add boot slot locking to bootchooser Lars Schmidt
  2025-06-19  8:25 ` [PATCH v3 1/4] bootchooser: implement locking of attempts counter Lars Schmidt
  2025-06-19  8:25 ` [PATCH v3 2/4] bootchooser: extend cmd tool by option to set attempts_locked Lars Schmidt
@ 2025-06-19  8:25 ` Lars Schmidt
  2025-06-19  8:25 ` [PATCH v3 4/4] Documentation: migration-2025.08.0: " Lars Schmidt
  3 siblings, 0 replies; 5+ messages in thread
From: Lars Schmidt @ 2025-06-19  8:25 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Lars Schmidt

The new variable behaves quite differently from the already existing
method via reset_attempts. So a proper description, including a use case,
is added to the documentation.
Additionally an example how to add it is added to bootstate.dtsi.

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

diff --git a/Documentation/user/bootchooser.rst b/Documentation/user/bootchooser.rst
index 351e1d14e1ead6ba8d329c03c0bc7ed28b523df0..1d7ece6c4b1255b41d359d3303daaf78bacfa17a 100644
--- a/Documentation/user/bootchooser.rst
+++ b/Documentation/user/bootchooser.rst
@@ -77,6 +77,23 @@ 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).
+
+In scenarios where the system is rebootet too frequently (after the ``remaining_attempts``
+counter is decremented, but before it got incremented again after a successful boot) and falls
+back to the other boot target, the ``attempts_locked`` variable can be used to avoid this behavior
+Bootchooser is prevented from decrementing the ``remaining_attempts`` counter and falling back
+to the other target. It comes with the trade-off that a slot, that becomes broken
+over time, it won't be detected anymore and will be booted indefinitely.
+
+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 +124,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




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 4/4] Documentation: migration-2025.08.0: add information about attempts_locked
  2025-06-19  8:25 [PATCH v3 0/4] Add boot slot locking to bootchooser Lars Schmidt
                   ` (2 preceding siblings ...)
  2025-06-19  8:25 ` [PATCH v3 3/4] Documentation: bootchooser: add information about attempts_locked Lars Schmidt
@ 2025-06-19  8:25 ` Lars Schmidt
  3 siblings, 0 replies; 5+ messages in thread
From: Lars Schmidt @ 2025-06-19  8:25 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: 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 1d7ece6c4b1255b41d359d3303daaf78bacfa17a..c094dca15a0e415d9076bf210ffdbece05ade62e 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




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-06-19  8:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-19  8:25 [PATCH v3 0/4] Add boot slot locking to bootchooser Lars Schmidt
2025-06-19  8:25 ` [PATCH v3 1/4] bootchooser: implement locking of attempts counter Lars Schmidt
2025-06-19  8:25 ` [PATCH v3 2/4] bootchooser: extend cmd tool by option to set attempts_locked Lars Schmidt
2025-06-19  8:25 ` [PATCH v3 3/4] Documentation: bootchooser: add information about attempts_locked Lars Schmidt
2025-06-19  8:25 ` [PATCH v3 4/4] Documentation: migration-2025.08.0: " Lars Schmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox