[PATCH 03/10] boot: revert globalvar changes after each boot entry

[Sascha Hauer <s.hauer@pengutronix.de>]

Boot entries, especially boot scripts, configure the boot by setting
global.bootm.* and global.linux.bootargs.* variables. To keep these from
leaking into the boot entries tried afterwards, there are currently two
mechanisms:

 - bootscript_boot() saves the global.bootm.* variables that are part of
   struct bootm_data and restores them afterwards. Variables not part of
   this struct, like global.bootm.tee, are not restored, and empty load
   addresses come back as the hex values of UIMAGE_SOME_ADDRESS resp.
   UIMAGE_INVALID_ADDRESS.
 - Kernel command line options have to be put into
   global.linux.bootargs.dyn.*, which boot_entry() clears after each boot
   entry. Options set by a boot entry in any other variable stay.

Replace both with a globalvar scope around the boot entry in
boot_entry(). This reverts all changes to the globalvars covered by
boot_globalvar_scope_push(), whatever the variable, and also restores
values that were set before the boot entry instead of clearing them.
This includes global.boot.watchdog_timeout, which boot_entry() reads
before running the boot entry, so a value set by a boot entry so far only
took effect for the following ones.

global.linux.bootargs.dyn.* still works as before for boot entries, but
values set in this namespace before calling boot are now kept instead of
being cleared once the first boot entry returns.

bootm_data_restore_defaults() has no users anymore, so remove it.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/user/booting-linux.rst | 20 ++++++++++++++------
 Documentation/user/devboot.rst       |  4 ++--
 common/boot.c                        | 35 ++++++++---------------------------
 common/bootm.c                       | 21 ---------------------
 include/bootm.h                      |  1 -
 5 files changed, 24 insertions(+), 57 deletions(-)

diff --git a/Documentation/user/booting-linux.rst b/Documentation/user/booting-linux.rst
index afc705ac86..0fbdc52c68 100644
--- a/Documentation/user/booting-linux.rst
+++ b/Documentation/user/booting-linux.rst
@@ -164,16 +164,24 @@ A simple boot entry in ``/env/boot/mmc`` could look like this:
   global.bootm.image=/mnt/mmc1/zImage
   global.bootm.oftree=/env/oftree
 
-  global linux.bootargs.dyn.root="root=PARTUUID=deadbeef:01"
+  global linux.bootargs.root="root=PARTUUID=deadbeef:01"
 
 This takes the kernel from ``/mnt/mmc1/zImage`` (which could be an
 :ref:`automount` path registered earlier). The devicetree will be used from
 ``/env/oftree``. The Kernel gets the command line
-``root=PARTUUID=deadbeef:01``. Note the ``.dyn`` in the bootargs variable name.
-boot entries should always add Kernel command line parameters to variables with
-``.dyn`` in it. These will be cleared before booting different boot entries.
-This is done so that following boot entries do not leak command line
-parameters from the previous boot entries.
+``root=PARTUUID=deadbeef:01``.
+
+All changes a boot entry makes to ``global.bootm.*``, ``global.linux.*``,
+``global.of.*`` and
+:ref:`global.boot.watchdog_timeout <magicvar_global_boot_watchdog_timeout>`
+are reverted when the boot entry returns:
+modified variables get their previous value back, variables created by
+the boot entry are removed and variables it removed are created again.
+This way a boot entry that fails to boot doesn't leak its settings into
+the following boot entries. Older boot entries put their Kernel command
+line parameters into ``global.linux.bootargs.dyn.*`` for this reason,
+which still works, but is no longer necessary. Settings meant for all
+boot entries belong into an init script or an nv variable instead.
 
 This entry can be booted with ``boot mmc``. It can also be made the default by
 setting the :ref:`global.boot.default <magicvar_global_boot_default>` variable
diff --git a/Documentation/user/devboot.rst b/Documentation/user/devboot.rst
index e319636994..62cc357c3a 100644
--- a/Documentation/user/devboot.rst
+++ b/Documentation/user/devboot.rst
@@ -191,8 +191,8 @@ when ``CONFIG_BOOTM_OFTREE_FALLBACK`` is enabled), and appends the
 contains.
 
 Kernel command line fragments set by the script should use the
-``global.linux.bootargs.devboot.`` namespace. Unlike
-``global.linux.bootargs.dyn.``, which is cleared whenever a boot entry
+``global.linux.bootargs.devboot.`` namespace. Unlike variables set
+by the boot entries themselves, which are reverted whenever a boot entry
 returns, it survives falling through to the next boot target, e.g. for
 ``devboot system0 system1``. The ``devboot`` command removes the
 namespace after the boot command returns, so a subsequent plain
diff --git a/common/boot.c b/common/boot.c
index 1d8768b864..eaeff7796e 100644
--- a/common/boot.c
+++ b/common/boot.c
@@ -97,7 +97,7 @@ static int bootscript_boot(struct bootentry *entry, int verbose, int dryrun)
 	int bootm_nattempts;
 	int ret;
 
-	struct bootm_data backup = {}, data = {};
+	struct bootm_data data = {};
 
 	if (dryrun == 1) {
 		printf("Would run %s%s%s\n", bs->entry.path,
@@ -105,18 +105,6 @@ static int bootscript_boot(struct bootentry *entry, int verbose, int dryrun)
 		return 0;
 	}
 
-	bootm_data_init_defaults(&backup);
-
-	/*
-	 * The strings point to the globalvars' current values, which the
-	 * script frees when it assigns new ones. Keep our own copies.
-	 */
-	backup.os_file = xstrdup(backup.os_file);
-	backup.initrd_file = xstrdup(backup.initrd_file);
-	backup.oftree_file = xstrdup(backup.oftree_file);
-	backup.root_dev = xstrdup(backup.root_dev);
-	backup.root_param = xstrdup(backup.root_param);
-
 	globalvar_add_simple("linux.bootargs.dyn.ip", NULL);
 	globalvar_add_simple("linux.bootargs.dyn.root", NULL);
 
@@ -129,12 +117,12 @@ static int bootscript_boot(struct bootentry *entry, int verbose, int dryrun)
 	ret = execute_binfmt(bs->args ? 2 : 1, argv);
 	if (ret) {
 		pr_err("Running script '%s' failed: %s\n", bs->entry.path, strerror(-ret));
-		goto out;
+		return ret;
 	}
 
 	/* No point in repeating bootm if script already called it */
 	if (bootm_nattempts != bootm_command_attempts())
-		goto out;
+		return 0;
 
 	bootm_data_init_defaults(&data);
 
@@ -143,17 +131,7 @@ static int bootscript_boot(struct bootentry *entry, int verbose, int dryrun)
 	if (dryrun >= 2)
 		data.dryrun = dryrun - 1;
 
-	ret = bootm_entry(entry, &data);
-out:
-	bootm_data_restore_defaults(&backup);
-
-	free_const(backup.os_file);
-	free_const(backup.initrd_file);
-	free_const(backup.oftree_file);
-	free_const(backup.root_dev);
-	free_const(backup.root_param);
-
-	return ret;
+	return bootm_entry(entry, &data);
 }
 
 static unsigned int boot_watchdog_timeout;
@@ -210,6 +188,7 @@ BAREBOX_MAGICVAR(global.user,
 
 int boot_entry(struct bootentry *be, int verbose, int dryrun)
 {
+	struct globalvar_scope *scope;
 	int ret;
 
 	pr_info("Booting entry '%s'\n", be->title);
@@ -224,11 +203,13 @@ int boot_entry(struct bootentry *be, int verbose, int dryrun)
 		}
 	}
 
+	scope = boot_globalvar_scope_push();
+
 	ret = be->boot(be, verbose, dryrun);
 	if (ret && ret != -ENOMEDIUM)
 		pr_err("Booting entry '%s' failed: %pe\n", be->title, ERR_PTR(ret));
 
-	globalvar_set_match("linux.bootargs.dyn.", "");
+	globalvar_scope_pop(scope);
 
 	return ret;
 }
diff --git a/common/bootm.c b/common/bootm.c
index 22f2ca4081..2ad8c34bb4 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -101,27 +101,6 @@ void bootm_data_init_defaults(struct bootm_data *data)
 	data->efi_boot = bootm_efi_mode;
 }
 
-void bootm_data_restore_defaults(const struct bootm_data *data)
-{
-	globalvar_set("bootm.oftree", data->oftree_file);
-	globalvar_set("bootm.image", data->os_file);
-	pr_setenv("global.bootm.image.loadaddr", "0x%lx", data->os_address);
-	if (IS_ENABLED(CONFIG_BOOTM_INITRD)) {
-		pr_setenv("global.bootm.initrd.loadaddr", "0x%lx", data->initrd_address);
-		globalvar_set("bootm.initrd", data->initrd_file);
-	}
-	globalvar_set("bootm.root_dev", data->root_dev);
-	globalvar_set("bootm.root_param", data->root_param);
-	bootm_set_verify_mode(data->verify);
-	bootm_appendroot = data->appendroot;
-	bootm_provide_machine_id = data->provide_machine_id;
-	bootm_provide_hostname = data->provide_hostname;
-	bootm_provide_policy = data->provide_policy;
-	bootm_verbosity = data->verbose;
-	bootm_dryrun = data->dryrun;
-	bootm_efi_mode = data->efi_boot;
-}
-
 static enum bootm_verify bootm_verify_mode = BOOTM_VERIFY_AVAILABLE;
 
 enum bootm_verify bootm_get_verify_mode(void)
diff --git a/include/bootm.h b/include/bootm.h
index bbdbb8e69d..06eb90c5ed 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -196,7 +196,6 @@ static inline int bootm_verbose(struct image_data *data)
 #endif
 
 void bootm_data_init_defaults(struct bootm_data *data);
-void bootm_data_restore_defaults(const struct bootm_data *data);
 
 const struct resource *
 bootm_load_os(struct image_data *data,

-- 
2.47.3