bootconfig: Cleanup and reorder the init parameter from bootconfig

Hi Steve,

Here is the 3rd version of the patches to update the bootconfig. I added 3 patches to fix some issues. One issue is from Julio which fixes a compilation error ([5/6]*). And the other 2 are fixing ftrace2bconf script ([4/6]) and cleanup test script ([6/6]).

* https://lore.kernel.org/all/[email protected]/T/#u

The 1st one is for bootconfig memory cleanup when exiting init stage. The 2nd one and 3rd one are for reordering the init parameters from bootconfig.
Since the current kernel concatenate the kernel cmdline and the bootconfig parameters as below.

[bootconfig kernel params][cmdline] -- [init cmdline][bootconfig init params]

This is bit odd because for the kernel parameters, user passed cmdline can override the previous (bootconfig) one, but for the init cmdline, bootconfig may override the parameter.
Thus, this series invert the order as the following.

[bootconfig kernel params][cmdline] -- [bootconfig init params][init cmdline]

The 3rd patch is adding how to use the bootconfig for passing kernel and init parameters.

Thank you,

---

Julio Faracco (1):
bootconfig: Fix missing return check of xbc_node_compose_key function

Masami Hiramatsu (5):
init: bootconfig: Remove all bootconfig data when the init memory is removed init/bootconfig: Reorder init parameter from bootconfig and cmdline
docs: bootconfig: Add how to use bootconfig for kernel parameters tools/bootconfig: Fix tracing_on option checking in ftrace2bconf.sh tools/bootconfig: Show whole test command for each test case

 Documentation/admin-guide/bootconfig.rst |   39 +++++++++++++++++++++++++++++-
 init/main.c                              |   37 ++++++++++++++++++++--------
 tools/bootconfig/main.c                  |    4 ++-
 tools/bootconfig/scripts/ftrace2bconf.sh |    4 ++-
 tools/bootconfig/test-bootconfig.sh      |    4 ++-
 5 files changed, 71 insertions(+), 17 deletions(-)

--
Masami Hiramatsu (Linaro) <[email protected]>

Since the bootconfig is used only in the init functions, it doesn't need to keep the data after boot. Free it when the init memory is removed.

---
Changes in v2:
- introduce exit_boot_config() wrapper for !CONFIG_BOOT_CONFIG

---
 init/main.c |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

index 8d97aba78c3a..d35c4a865adb 100644
@@ -468,7 +468,12 @@ static void __init setup_boot_config(void)
 	return;
 }
 
-#else
+static void __init exit_boot_config(void)
+{
+	xbc_destroy_all();
+}
+
+#else	/* !CONFIG_BOOT_CONFIG */
 
 static void __init setup_boot_config(void)
 {

@@ -481,7 +486,11 @@ static int __init warn_bootconfig(char *str)
 	pr_warn("WARNING: 'bootconfig' found on the kernel command line but CONFIG_BOOT_CONFIG is not set.\n");
 	return 0;
 }
-#endif
+
+#define exit_boot_config()	do {} while (0)
+
+#endif	/* CONFIG_BOOT_CONFIG */
+
 early_param("bootconfig", warn_bootconfig);
 
 /* Change NUL term back to "=", to make "param" the whole string. */

@@ -1493,6 +1502,7 @@ static int __ref kernel_init(void *unused)
 	kprobe_free_init_mem();
 	ftrace_free_init_mem();
 	kgdb_free_init_mem();
+	exit_boot_config();
 	free_initmem();
 	mark_readonly();
 

Reorder the init parameters from bootconfig and kernel cmdline so that the kernel cmdline always be the last part of the parameters as below.

" -- "[bootconfig init params][cmdline init params]

This change will help us to prevent that bootconfig init params overwrite the init params which user gives in the command line.

---
 init/main.c |   23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

index d35c4a865adb..d08caed17c7f 100644
@@ -153,10 +153,10 @@ static char *extra_init_args;
 #ifdef CONFIG_BOOT_CONFIG
 /* Is bootconfig on command line? */
 static bool bootconfig_found;
-static bool initargs_found;
+static size_t initargs_offs;
 #else
 # define bootconfig_found false
-# define initargs_found false
+# define initargs_offs 0
 #endif
 
 static char *execute_command;

@@ -422,9 +422,9 @@ static void __init setup_boot_config(void)
 	if (IS_ERR(err) || !bootconfig_found)
 		return;
 
-	/* parse_args() stops at '--' and returns an address */
+	/* parse_args() stops at the next param of '--' and returns an address */
 	if (err)
-		initargs_found = true;
+		initargs_offs = err - tmp_cmdline;
 
 	if (!data) {
 		pr_err("'bootconfig' found on command line, but no bootconfig found\n");

@@ -655,16 +655,21 @@ static void __init setup_command_line(char *command_line)
 		 * Append supplemental init boot args to saved_command_line
 		 * so that user can check what command line options passed
 		 * to init.
+		 * The order should always be
+		 * " -- "[bootconfig init-param][cmdline init-param]
 		 */
-		len = strlen(saved_command_line);
-		if (initargs_found) {
-			saved_command_line[len++] = ' ';
+		if (initargs_offs) {
+			len = xlen + initargs_offs;
+			strcpy(saved_command_line + len, extra_init_args);
+			len += ilen - 4;	/* strlen(extra_init_args) */
+			strcpy(saved_command_line + len,
+				boot_command_line + initargs_offs - 1);
 		} else {
+			len = strlen(saved_command_line);
 			strcpy(saved_command_line + len, " -- ");
 			len += 4;
+			strcpy(saved_command_line + len, extra_init_args);
 		}
-
-		strcpy(saved_command_line + len, extra_init_args);
 	}
 }
 

Add a section to describe how to use the bootconfig for specifying kernel and init parameters. This is an important section because it is the reason why this document is under the admin-guide.

---
 Documentation/admin-guide/bootconfig.rst |   39 +++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

index 6a79f2e59396..a1860fc0ca88 100644
@@ -178,7 +178,7 @@ update the boot loader and the kernel image itself as long as the boot
 loader passes the correct initrd file size. If by any chance, the boot
 loader passes a longer size, the kernel fails to find the bootconfig data.
 
-To do this operation, Linux kernel provides "bootconfig" command under
+To do this operation, Linux kernel provides ``bootconfig`` command under
 tools/bootconfig, which allows admin to apply or delete the config file
 to/from initrd image. You can build it by the following command::
 

@@ -196,6 +196,43 @@ To remove the config from the image, you can use -d option as below::
 Then add "bootconfig" on the normal kernel command line to tell the
 kernel to look for the bootconfig at the end of the initrd file.
 
+
+Kernel parameters via Boot Config
+=================================
+
+In addition to the kernel command line, the boot config can be used for
+passing the kernel parameters. All the key-value pairs under ``kernel``
+key will be passed to kernel cmdline directly. Moreover, the key-value
+pairs under ``init`` will be passed to init process via the cmdline.
+The parameters are concatinated with user-given kernel cmdline string
+as the following order, so that the command line parameter can override
+bootconfig parameters (this depends on how the subsystem handles parameters
+but in general, earlier parameter will be overwritten by later one.)::
+
+ [bootconfig params][cmdline params] -- [bootconfig init params][cmdline init params]
+
+Here is an example of the bootconfig file for kernel/init parameters.::
+
+ kernel {
+   root = 01234567-89ab-cdef-0123-456789abcd
+ }
+ init {
+  splash
+ }
+
+This will be copied into the kernel cmdline string as the following::
+
+ root="01234567-89ab-cdef-0123-456789abcd" -- splash
+
+If user gives some other command line like,::
+
+ ro bootconfig -- quiet
+
+The final kernel cmdline will be the following::
+
+ root="01234567-89ab-cdef-0123-456789abcd" ro bootconfig -- splash quiet
+
+
 Config File Limitation
 ======================
 

Since tracing_on indicates only "1" (default) or "0", ftrace2bconf.sh only need to check the value is "0".

---
 tools/bootconfig/scripts/ftrace2bconf.sh |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index fbaf07dc91bf..6183b36c6846 100755
@@ -239,8 +239,8 @@ instance_options() { # [instance-name]
 		emit_kv $PREFIX.cpumask = $val
 	fi
 	val=`cat $INSTANCE/tracing_on`
-	if [ `echo $val | sed -e s/f//g`x != x ]; then
-		emit_kv $PREFIX.tracing_on = $val
+	if [ "$val" = "0" ]; then
+		emit_kv $PREFIX.tracing_on = 0
 	fi
 
 	val=`cat $INSTANCE/current_tracer`

From: Julio Faracco <[email protected]>

The function xbc_show_list should handle the keys during the composition. Even the errors returned by the compose function. Instead of removing the ret variable, it should save the value and show the exact error. This missing variable is causing a compilation issue also.

---
 tools/bootconfig/main.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index f45fa992e01d..fd67496a947f 100644
@@ -111,9 +111,11 @@ static void xbc_show_list(void)
 	char key[XBC_KEYLEN_MAX];
 	struct xbc_node *leaf;
 	const char *val;
+	int ret;
 
 	xbc_for_each_key_value(leaf, val) {
-		if (xbc_node_compose_key(leaf, key, XBC_KEYLEN_MAX) < 0) {
+		ret = xbc_node_compose_key(leaf, key, XBC_KEYLEN_MAX);
+		if (ret < 0) {
 			fprintf(stderr, "Failed to compose key %d\n", ret);
 			break;
 		}

Show whole test command instead of only the 3rd argument. This will clear to show what will be actually tested by each test case.

---
 tools/bootconfig/test-bootconfig.sh |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index baed891d0ba4..f68e2e9eef8b 100755
@@ -26,7 +26,7 @@ trap cleanup EXIT TERM
 NO=1
 
 xpass() { # pass test command
-  echo "test case $NO ($3)... "
+  echo "test case $NO ($*)... "
   if ! ($@ && echo "\t\t[OK]"); then
      echo "\t\t[NG]"; NG=$((NG + 1))
   fi

@@ -34,7 +34,7 @@ xpass() { # pass test command
 }
 
 xfail() { # fail test command
-  echo "test case $NO ($3)... "
+  echo "test case $NO ($*)... "
   if ! (! $@ && echo "\t\t[OK]"); then
      echo "\t\t[NG]"; NG=$((NG + 1))
   fi

Hi Steve,

BTW, this includes Julio's fix patch we discussed in the other thread. If you picked it, please ignore that patch in this series.

Thank you,

Masami Hiramatsu <[email protected]>

Yep, I noticed this, and simply pulled in this series over the other ones (including Julio's patch).

-- Steve