[PATCH master v2 1/2] bootm: associate bootm overrides with struct bootentry

[PATCH master v2 1/2] bootm: associate bootm overrides with struct bootentry

[Ahmad Fatoum]

The boot override logic is currently controlled by the boot command,
which makes it cumbersome to exercise from custom bootentry providers.

A more natural place would be associating it with the boot entry logic,
so move it there.

No functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - no change
---
 commands/boot.c           | 11 ++---------
 common/boot.c             |  4 ++++
 common/bootm.c            |  3 ++-
 include/boot.h            |  2 ++
 include/bootm-overrides.h | 30 ++++++++++++++++++++++++++++++
 include/bootm.h           | 18 ------------------
 6 files changed, 40 insertions(+), 28 deletions(-)
 create mode 100644 include/bootm-overrides.h

diff --git a/commands/boot.c b/commands/boot.c
index 820708380191..aad01fdc6c95 100644
--- a/commands/boot.c
+++ b/commands/boot.c
@@ -55,11 +55,6 @@ static int boot_add_override(struct bootm_overrides *overrides, char *var)
 	return 0;
 }
 
-static inline bool boot_has_overrides(const struct bootm_overrides *overrides)
-{
-	return overrides->os_file || overrides->oftree_file || overrides->initrd_file;
-}
-
 static int do_boot(int argc, char *argv[])
 {
 	char *freep = NULL;
@@ -128,8 +123,6 @@ static int do_boot(int argc, char *argv[])
 	}
 
 	entries = bootentries_alloc();
-	if (boot_has_overrides(&overrides))
-		bootm_set_overrides(&overrides);
 
 	while ((name = next(&handle)) != NULL) {
 		if (!*name)
@@ -142,6 +135,8 @@ static int do_boot(int argc, char *argv[])
 			continue;
 
 		bootentries_for_each_entry(entries, entry) {
+			bootm_merge_overrides(&entry->overrides, &overrides);
+
 			ret = boot_entry(entry, verbose, dryrun);
 			if (!ret)
 				goto out;
@@ -164,8 +159,6 @@ static int do_boot(int argc, char *argv[])
 
 	ret = 0;
 out:
-	if (boot_has_overrides(&overrides))
-		bootm_unset_overrides();
 	bootentries_free(entries);
 	free(freep);
 
diff --git a/common/boot.c b/common/boot.c
index 2530a5bbeeac..e6edb816eee0 100644
--- a/common/boot.c
+++ b/common/boot.c
@@ -162,10 +162,14 @@ int boot_entry(struct bootentry *be, int verbose, int dryrun)
 		}
 	}
 
+	bootm_set_overrides(&be->overrides);
+
 	ret = be->boot(be, verbose, dryrun);
 	if (ret && ret != -ENOMEDIUM)
 		pr_err("Booting entry '%s' failed: %pe\n", be->title, ERR_PTR(ret));
 
+	bootm_set_overrides(NULL);
+
 	globalvar_set_match("linux.bootargs.dyn.", "");
 
 	return ret;
diff --git a/common/bootm.c b/common/bootm.c
index fe4d51681d73..6b63987f0900 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -2,6 +2,7 @@
 
 #include <common.h>
 #include <bootm.h>
+#include <bootm-overrides.h>
 #include <fs.h>
 #include <malloc.h>
 #include <memory.h>
@@ -1004,7 +1005,7 @@ int bootm_boot(struct bootm_data *bootm_data)
 #ifdef CONFIG_BOOT_OVERRIDE
 void bootm_set_overrides(const struct bootm_overrides *overrides)
 {
-	bootm_overrides = *overrides;
+	bootm_overrides = overrides ? *overrides : (struct bootm_overrides){};
 }
 #endif
 
diff --git a/include/boot.h b/include/boot.h
index 53ad1360a5f3..9af397db5b62 100644
--- a/include/boot.h
+++ b/include/boot.h
@@ -5,6 +5,7 @@
 #include <of.h>
 #include <menu.h>
 #include <environment.h>
+#include <bootm-overrides.h>
 
 #ifdef CONFIG_FLEXIBLE_BOOTARGS
 const char *linux_bootargs_get(void);
@@ -33,6 +34,7 @@ struct bootentry {
 	char *description;
 	int (*boot)(struct bootentry *entry, int verbose, int dryrun);
 	void (*release)(struct bootentry *entry);
+	struct bootm_overrides overrides;
 };
 
 int bootentries_add_entry(struct bootentries *entries, struct bootentry *entry);
diff --git a/include/bootm-overrides.h b/include/bootm-overrides.h
new file mode 100644
index 000000000000..231c913709e2
--- /dev/null
+++ b/include/bootm-overrides.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __BOOTM_OVERRIDES_H
+#define __BOOTM_OVERRIDES_H
+
+struct bootm_overrides {
+	const char *os_file;
+	const char *oftree_file;
+	const char *initrd_file;
+};
+
+#ifdef CONFIG_BOOT_OVERRIDE
+void bootm_set_overrides(const struct bootm_overrides *overrides);
+#else
+static inline void bootm_set_overrides(const struct bootm_overrides *overrides) {}
+#endif
+
+static inline void bootm_merge_overrides(struct bootm_overrides *dst,
+					 const struct bootm_overrides *src)
+{
+	if (!IS_ENABLED(CONFIG_BOOT_OVERRIDE))
+		return;
+	if (src->os_file)
+		dst->os_file = src->os_file;
+	if (src->oftree_file)
+		dst->oftree_file = src->oftree_file;
+	if (src->initrd_file)
+		dst->initrd_file = src->initrd_file;
+}
+
+#endif
diff --git a/include/bootm.h b/include/bootm.h
index 0e5e99773a26..b86d06b0f55d 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -160,22 +160,4 @@ void bootm_force_signed_images(void);
 
 void *booti_load_image(struct image_data *data, phys_addr_t *oftree);
 
-struct bootm_overrides {
-	const char *os_file;
-	const char *oftree_file;
-	const char *initrd_file;
-};
-
-#ifdef CONFIG_BOOT_OVERRIDE
-void bootm_set_overrides(const struct bootm_overrides *overrides);
-#else
-static inline void bootm_set_overrides(const struct bootm_overrides *overrides) {}
-#endif
-
-static inline void bootm_unset_overrides(void)
-{
-	struct bootm_overrides overrides = {};
-	bootm_set_overrides(&overrides);
-}
-
 #endif /* __BOOTM_H */
-- 
2.39.5

[PATCH master v2 2/2] bootm: remove ability for late override of bootm.image

[Ahmad Fatoum]

Overriding struct image_data::os_file at the location where we do it
now is not robust as the bootm handler is chosen by then and there
is no check if the override image is suitable for the original handler.

This can't be fixed in a straight-forward manner by moving the override
earlier as that would break appendroot for example.

To be able to do this robustly, some rework of the boot code may be in
order, so for now allow only overriding the oftree and the initrd.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - squash removal of unreferenced goto label and ensure it applies
    cleanly
---
 commands/boot.c           |  4 ++--
 common/bootm.c            | 36 +++++++++---------------------------
 include/bootm-overrides.h |  3 ---
 3 files changed, 11 insertions(+), 32 deletions(-)

diff --git a/commands/boot.c b/commands/boot.c
index aad01fdc6c95..07f4db4b8033 100644
--- a/commands/boot.c
+++ b/commands/boot.c
@@ -43,7 +43,7 @@ static int boot_add_override(struct bootm_overrides *overrides, char *var)
 	if (!strcmp(var, "bootm.image")) {
 		if (isempty(val))
 			return -EINVAL;
-		overrides->os_file = val;
+		return -ENOSYS;
 	} else if (!strcmp(var, "bootm.oftree")) {
 		overrides->oftree_file = val;
 	} else if (!strcmp(var, "bootm.initrd")) {
@@ -191,7 +191,7 @@ BAREBOX_CMD_HELP_OPT ("-m","Show a menu with boot options")
 BAREBOX_CMD_HELP_OPT ("-M INDEX","Show a menu with boot options with entry INDEX preselected")
 BAREBOX_CMD_HELP_OPT ("-w SECS","Start watchdog with timeout SECS before booting")
 #ifdef CONFIG_BOOT_OVERRIDE
-BAREBOX_CMD_HELP_OPT ("-o VAR[=VAL]","override VAR (bootm.{image,oftree,initrd}) with VAL")
+BAREBOX_CMD_HELP_OPT ("-o VAR[=VAL]","override VAR (bootm.{oftree,initrd}) with VAL")
 BAREBOX_CMD_HELP_OPT ("            ","if VAL is not specified, the value of VAR is taken")
 #endif
 BAREBOX_CMD_HELP_OPT ("-t SECS","specify timeout in SECS")
diff --git a/common/bootm.c b/common/bootm.c
index 6b63987f0900..a5065c3860b3 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -175,12 +175,6 @@ static bool bootm_get_override(char **oldpath, const char *newpath)
  */
 int bootm_load_os(struct image_data *data, unsigned long load_address)
 {
-	if (bootm_get_override(&data->os_file, bootm_overrides.os_file)) {
-		if (load_address == UIMAGE_INVALID_ADDRESS)
-			return -EINVAL;
-		goto os_file;
-	}
-
 	if (data->os_res)
 		return 0;
 
@@ -220,7 +214,6 @@ int bootm_load_os(struct image_data *data, unsigned long load_address)
 	if (IS_ENABLED(CONFIG_ELF) && data->elf)
 		return elf_load(data->elf);
 
-os_file:
 	if (!data->os_file)
 		return -EINVAL;
 
@@ -569,19 +562,15 @@ int bootm_get_os_size(struct image_data *data)
 	struct stat s;
 	int ret;
 
-	if (bootm_get_override(NULL, bootm_overrides.os_file)) {
-		os_file = bootm_overrides.os_file;
-	} else {
-		if (data->elf)
-			return elf_get_mem_size(data->elf);
-		if (image_is_uimage(data))
-			return uimage_get_size(data->os, uimage_part_num(data->os_part));
-		if (data->os_fit)
-			return data->fit_kernel_size;
-		if (!data->os_file)
-			return -EINVAL;
-		os_file = data->os_file;
-	}
+	if (data->elf)
+		return elf_get_mem_size(data->elf);
+	if (image_is_uimage(data))
+		return uimage_get_size(data->os, uimage_part_num(data->os_part));
+	if (data->os_fit)
+		return data->fit_kernel_size;
+	if (!data->os_file)
+		return -EINVAL;
+	os_file = data->os_file;
 
 	ret = stat(os_file, &s);
 	if (ret)
@@ -949,13 +938,6 @@ int bootm_boot(struct bootm_data *bootm_data)
 		printf("Passing control to %s handler\n", handler->name);
 	}
 
-	if (bootm_get_override(&data->os_file, bootm_overrides.os_file)) {
-		if (data->os_res) {
-			release_sdram_region(data->os_res);
-			data->os_res = NULL;
-		}
-	}
-
 	bootm_get_override(&data->oftree_file, bootm_overrides.oftree_file);
 
 	if (bootm_get_override(&data->initrd_file, bootm_overrides.initrd_file)) {
diff --git a/include/bootm-overrides.h b/include/bootm-overrides.h
index 231c913709e2..19557f63acd9 100644
--- a/include/bootm-overrides.h
+++ b/include/bootm-overrides.h
@@ -3,7 +3,6 @@
 #define __BOOTM_OVERRIDES_H
 
 struct bootm_overrides {
-	const char *os_file;
 	const char *oftree_file;
 	const char *initrd_file;
 };
@@ -19,8 +18,6 @@ static inline void bootm_merge_overrides(struct bootm_overrides *dst,
 {
 	if (!IS_ENABLED(CONFIG_BOOT_OVERRIDE))
 		return;
-	if (src->os_file)
-		dst->os_file = src->os_file;
 	if (src->oftree_file)
 		dst->oftree_file = src->oftree_file;
 	if (src->initrd_file)
-- 
2.39.5

Re: [PATCH master v2 1/2] bootm: associate bootm overrides with struct bootentry

[Sascha Hauer]

On Mon, 07 Apr 2025 12:47:55 +0200, Ahmad Fatoum wrote:
> The boot override logic is currently controlled by the boot command,
> which makes it cumbersome to exercise from custom bootentry providers.
> 
> A more natural place would be associating it with the boot entry logic,
> so move it there.
> 
> No functional change.
> 
> [...]

Applied, thanks!

[1/2] bootm: associate bootm overrides with struct bootentry
      https://git.pengutronix.de/cgit/barebox/commit/?id=95b7413668a7 (link may not be stable)
[2/2] bootm: remove ability for late override of bootm.image
      https://git.pengutronix.de/cgit/barebox/commit/?id=2d6aaf0100f0 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

Re: [PATCH master v2 1/2] bootm: associate bootm overrides with struct bootentry

[Sascha Hauer]

On Mon, 07 Apr 2025 12:47:55 +0200, Ahmad Fatoum wrote:
> The boot override logic is currently controlled by the boot command,
> which makes it cumbersome to exercise from custom bootentry providers.
> 
> A more natural place would be associating it with the boot entry logic,
> so move it there.
> 
> No functional change.
> 
> [...]

Applied, thanks!

[1/2] bootm: associate bootm overrides with struct bootentry
      https://git.pengutronix.de/cgit/barebox/commit/?id=95b7413668a7 (link may not be stable)
[2/2] bootm: remove ability for late override of bootm.image
      https://git.pengutronix.de/cgit/barebox/commit/?id=2d6aaf0100f0 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>