[PATCH RFC bootconfig 0/2] Distinguish bootloader and embedded kernel parameters

[PATCH RFC bootconfig 0/2] Distinguish bootloader and embedded kernel parameters

[Paul E. McKenney]

Hello!

This series provides /proc interfaces parallel to /proc/cmdline that
provide only those kernel boot parameters that were provided by the
bootloader (/proc/cmdline_load) and only those parameters that were
embedded in the kernel image (/proc/cmdline_image, in boot-config format).
This is especially important when these parameters are presented to the
boot loader by automation that might gather them from diverse sources,
and also when a kexec-based reboot process pulls the kernel boot
parameters from /proc.  If such a reboot process uses /proc/cmdline,
the kernel parameters from the image are replicated on every reboot,
which can be frustrating when the new kernel has different embedded
kernel boot parameters.

Why put these in /proc?  Because they is quite similar to /proc/cmdline,
so it makes sense to put it in the same place that /proc/cmdline is
located.

1.	fs/proc: Add /proc/cmdline_load for boot loader arguments.

2.	fs/proc: Add /proc/cmdline_image for embedded arguments.

						Thanx, Paul

------------------------------------------------------------------------

 b/fs/proc/cmdline.c    |   13 +++++++++++++
 b/include/linux/init.h |    3 ++-
 b/init/main.c          |    2 +-
 fs/proc/cmdline.c      |   12 ++++++++++++
 include/linux/init.h   |   11 ++++++-----
 init/main.c            |    9 +++++++++
 6 files changed, 43 insertions(+), 7 deletions(-)

[PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Paul E. McKenney]

In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
show all kernel boot parameters, both those supplied by the boot loader
and those embedded in the kernel image.  This works well for those who
just want to see all of the kernel boot parameters, but is not helpful to
those who need to see only those parameters supplied by the boot loader.
This is especially important when these parameters are presented to the
boot loader by automation that might gather them from diverse sources.

Therefore, provide a /proc/cmdline_load file that shows only those kernel
boot parameters supplied by the boot loader.

Why put this in /proc?  Because it is quite similar to /proc/cmdline, so
it makes sense to put it in the same place that /proc/cmdline is located.

[ sfr: Apply kernel test robot feedback. ]

Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: <linux-fsdevel@vger.kernel.org>
---
 fs/proc/cmdline.c    | 13 +++++++++++++
 include/linux/init.h |  3 ++-
 init/main.c          |  2 +-
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c
index a6f76121955f..1d0ef9d2949d 100644
--- a/fs/proc/cmdline.c
+++ b/fs/proc/cmdline.c
@@ -3,6 +3,7 @@
 #include <linux/init.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
+#include <asm/setup.h>
 #include "internal.h"
 
 static int cmdline_proc_show(struct seq_file *m, void *v)
@@ -12,6 +13,13 @@ static int cmdline_proc_show(struct seq_file *m, void *v)
 	return 0;
 }
 
+static int cmdline_load_proc_show(struct seq_file *m, void *v)
+{
+	seq_puts(m, boot_command_line);
+	seq_putc(m, '\n');
+	return 0;
+}
+
 static int __init proc_cmdline_init(void)
 {
 	struct proc_dir_entry *pde;
@@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
 	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
 	pde_make_permanent(pde);
 	pde->size = saved_command_line_len + 1;
+	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
+		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
+		pde_make_permanent(pde);
+		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
+	}
 	return 0;
 }
 fs_initcall(proc_cmdline_init);
diff --git a/include/linux/init.h b/include/linux/init.h
index 266c3e1640d4..29e75bbe7984 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -112,6 +112,7 @@
 #define __REFCONST       .section       ".ref.rodata", "a"
 
 #ifndef __ASSEMBLY__
+
 /*
  * Used for initialization calls..
  */
@@ -143,7 +144,7 @@ struct file_system_type;
 
 /* Defined in init/main.c */
 extern int do_one_initcall(initcall_t fn);
-extern char __initdata boot_command_line[];
+extern char boot_command_line[];
 extern char *saved_command_line;
 extern unsigned int saved_command_line_len;
 extern unsigned int reset_devices;
diff --git a/init/main.c b/init/main.c
index ad920fac325c..2121685c479a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
 void (*__initdata late_time_init)(void);
 
 /* Untouched command line saved by arch-specific code. */
-char __initdata boot_command_line[COMMAND_LINE_SIZE];
+char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
 /* Untouched saved command line (eg. for /proc) */
 char *saved_command_line __ro_after_init;
 unsigned int saved_command_line_len __ro_after_init;
-- 
2.40.1

[PATCH RFC bootconfig] 2/2] fs/proc: Add /proc/cmdline_image for embedded arguments

[Paul E. McKenney]

In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will show
all kernel boot parameters, both those supplied by the boot loader and
those embedded in the kernel image.  This works well for those who just
want to see all of the kernel boot parameters, but is not helpful to those
who need to see only those parameters that were embedded into the kernel
image.  This is especially important in situations where there are many
kernel images for different kernel versions and kernel configurations,
all of which opens the door to a great deal of human error.

Therefore, provide a /proc/cmdline_image file that shows only those kernel
boot parameters that were embedded in the kernel image.  The output
is in boot-image format, which allows easy reconcilation against the
boot-config source file.

Why put this in /proc?  Because it is quite similar to /proc/cmdline, so
it makes sense to put it in the same place that /proc/cmdline is located.

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: <linux-fsdevel@vger.kernel.org>

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 fs/proc/cmdline.c    | 12 ++++++++++++
 include/linux/init.h | 11 ++++++-----
 init/main.c          |  9 +++++++++
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c
index 1d0ef9d2949d..4ab5223198cb 100644
--- a/fs/proc/cmdline.c
+++ b/fs/proc/cmdline.c
@@ -20,6 +20,15 @@ static int cmdline_load_proc_show(struct seq_file *m, void *v)
 	return 0;
 }
 
+static int cmdline_image_proc_show(struct seq_file *m, void *v)
+{
+#ifdef CONFIG_BOOT_CONFIG_FORCE
+	seq_puts(m, saved_bootconfig_string);
+	seq_putc(m, '\n');
+#endif
+	return 0;
+}
+
 static int __init proc_cmdline_init(void)
 {
 	struct proc_dir_entry *pde;
@@ -31,6 +40,9 @@ static int __init proc_cmdline_init(void)
 		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
 		pde_make_permanent(pde);
 		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
+		pde = proc_create_single("cmdline_image", 0, NULL, cmdline_image_proc_show);
+		pde_make_permanent(pde);
+		pde->size = strnlen(saved_bootconfig_string, COMMAND_LINE_SIZE) + 1;
 	}
 	return 0;
 }
diff --git a/include/linux/init.h b/include/linux/init.h
index 29e75bbe7984..c075983c5015 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -14,7 +14,7 @@
 #define __noinitretpoline
 #endif
 
-/* These macros are used to mark some functions or 
+/* These macros are used to mark some functions or
  * initialized data (doesn't apply to uninitialized data)
  * as `initialization' functions. The kernel can take this
  * as hint that the function is used only during the initialization
@@ -22,7 +22,7 @@
  *
  * Usage:
  * For functions:
- * 
+ *
  * You should add __init immediately before the function name, like:
  *
  * static void __init initme(int x, int y)
@@ -148,6 +148,7 @@ extern char boot_command_line[];
 extern char *saved_command_line;
 extern unsigned int saved_command_line_len;
 extern unsigned int reset_devices;
+extern char saved_bootconfig_string[];
 
 /* used by init/main.c */
 void setup_arch(char **);
@@ -184,7 +185,7 @@ extern void (*late_time_init)(void);
 extern bool initcall_debug;
 
 #endif
-  
+
 #ifndef MODULE
 
 #ifndef __ASSEMBLY__
@@ -192,8 +193,8 @@ extern bool initcall_debug;
 /*
  * initcalls are now grouped by functionality into separate
  * subsections. Ordering inside the subsections is determined
- * by link order. 
- * For backwards compatibility, initcall() puts the call in 
+ * by link order.
+ * For backwards compatibility, initcall() puts the call in
  * the device init subsection.
  *
  * The `id' arg to __define_initcall() is needed so that multiple initcalls
diff --git a/init/main.c b/init/main.c
index 2121685c479a..981170da0b1c 100644
--- a/init/main.c
+++ b/init/main.c
@@ -146,6 +146,11 @@ static char *extra_command_line;
 /* Extra init arguments */
 static char *extra_init_args;
 
+/* Untouched boot-config string */
+#ifdef CONFIG_BOOT_CONFIG_FORCE
+char saved_bootconfig_string[COMMAND_LINE_SIZE] __ro_after_init;
+#endif
+
 #ifdef CONFIG_BOOT_CONFIG
 /* Is bootconfig on command line? */
 static bool bootconfig_found;
@@ -435,6 +440,10 @@ static void __init setup_boot_config(void)
 		return;
 	}
 
+#ifdef CONFIG_BOOT_CONFIG_FORCE
+	strncpy(saved_bootconfig_string, data, COMMAND_LINE_SIZE);
+#endif
+
 	if (size >= XBC_DATA_MAX) {
 		pr_err("bootconfig size %ld greater than max size %d\n",
 			(long)size, XBC_DATA_MAX);
-- 
2.40.1

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]

Hi Paul,

Just a couple of nits:

On Thu, 27 Jul 2023 20:37:00 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote:
>
> [ sfr: Apply kernel test robot feedback. ]

This was a fix for my own build testing (I am not a bot (yet) :-)).

> diff --git a/include/linux/init.h b/include/linux/init.h
> index 266c3e1640d4..29e75bbe7984 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -112,6 +112,7 @@
>  #define __REFCONST       .section       ".ref.rodata", "a"
>  
>  #ifndef __ASSEMBLY__
> +

Please remove this added blank line.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH RFC bootconfig 0/2] Distinguish bootloader and embedded kernel parameters

[Randy Dunlap]

On 7/27/23 20:35, Paul E. McKenney wrote:
> Hello!
> 
> This series provides /proc interfaces parallel to /proc/cmdline that
> provide only those kernel boot parameters that were provided by the
> bootloader (/proc/cmdline_load) and only those parameters that were
> embedded in the kernel image (/proc/cmdline_image, in boot-config format).
> This is especially important when these parameters are presented to the
> boot loader by automation that might gather them from diverse sources,
> and also when a kexec-based reboot process pulls the kernel boot
> parameters from /proc.  If such a reboot process uses /proc/cmdline,
> the kernel parameters from the image are replicated on every reboot,
> which can be frustrating when the new kernel has different embedded
> kernel boot parameters.
> 
> Why put these in /proc?  Because they is quite similar to /proc/cmdline,
> so it makes sense to put it in the same place that /proc/cmdline is
> located.
> 
> 1.	fs/proc: Add /proc/cmdline_load for boot loader arguments.
> 
> 2.	fs/proc: Add /proc/cmdline_image for embedded arguments.
> 
> 						Thanx, Paul
> 

Hi Paul,

This series seems to be missing updates to
Documentation/filesystems/proc.rst.

Please add them.
Thanks.

> ------------------------------------------------------------------------
> 
>  b/fs/proc/cmdline.c    |   13 +++++++++++++
>  b/include/linux/init.h |    3 ++-
>  b/init/main.c          |    2 +-
>  fs/proc/cmdline.c      |   12 ++++++++++++
>  include/linux/init.h   |   11 ++++++-----
>  init/main.c            |    9 +++++++++
>  6 files changed, 43 insertions(+), 7 deletions(-)

-- 
~Randy

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Paul E. McKenney]

On Fri, Jul 28, 2023 at 02:06:35PM +1000, Stephen Rothwell wrote:
> Hi Paul,
> 
> Just a couple of nits:
> 
> On Thu, 27 Jul 2023 20:37:00 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote:
> >
> > [ sfr: Apply kernel test robot feedback. ]
> 
> This was a fix for my own build testing (I am not a bot (yet) :-)).

Ah!

This says "Stephen Rothwell made a change in response to kernel test robot
feedback".  I saw your change and saw the kernel test robot complaint,
and gave the robot the benefit of the doubt.

But if you saw this build bug yourself and object to crediting kernel
test robot for also spotting it, please let me know and I will remove
this line.

> > diff --git a/include/linux/init.h b/include/linux/init.h
> > index 266c3e1640d4..29e75bbe7984 100644
> > --- a/include/linux/init.h
> > +++ b/include/linux/init.h
> > @@ -112,6 +112,7 @@
> >  #define __REFCONST       .section       ".ref.rodata", "a"
> >  
> >  #ifndef __ASSEMBLY__
> > +
> 
> Please remove this added blank line.

Good eyes, done!

I should also split out the modifications to include/linux/init.h,
which are now whitespace-only, shouldn't I?

							Thanx, Paul

Re: [PATCH RFC bootconfig 0/2] Distinguish bootloader and embedded kernel parameters

[Paul E. McKenney]

On Thu, Jul 27, 2023 at 09:25:06PM -0700, Randy Dunlap wrote:
> 
> 
> On 7/27/23 20:35, Paul E. McKenney wrote:
> > Hello!
> > 
> > This series provides /proc interfaces parallel to /proc/cmdline that
> > provide only those kernel boot parameters that were provided by the
> > bootloader (/proc/cmdline_load) and only those parameters that were
> > embedded in the kernel image (/proc/cmdline_image, in boot-config format).
> > This is especially important when these parameters are presented to the
> > boot loader by automation that might gather them from diverse sources,
> > and also when a kexec-based reboot process pulls the kernel boot
> > parameters from /proc.  If such a reboot process uses /proc/cmdline,
> > the kernel parameters from the image are replicated on every reboot,
> > which can be frustrating when the new kernel has different embedded
> > kernel boot parameters.
> > 
> > Why put these in /proc?  Because they is quite similar to /proc/cmdline,
> > so it makes sense to put it in the same place that /proc/cmdline is
> > located.
> > 
> > 1.	fs/proc: Add /proc/cmdline_load for boot loader arguments.
> > 
> > 2.	fs/proc: Add /proc/cmdline_image for embedded arguments.
> > 
> > 						Thanx, Paul
> > 
> 
> Hi Paul,
> 
> This series seems to be missing updates to
> Documentation/filesystems/proc.rst.
> 
> Please add them.

Good catch, thank you!

I will fold the diff below into the three respective commits on my next
rebase, but in the meantime, please let me know what you think.

							Thanx, Paul

diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 7897a7dafcbc..98c43c5ef1ee 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -686,7 +686,10 @@ files are there, and which are missing.
  apm          Advanced power management info
  buddyinfo    Kernel memory allocator information (see text)	(2.5)
  bus          Directory containing bus specific information
- cmdline      Kernel command line
+ cmdline      Kernel command line, both from bootloader and embedded
+ 	      in the kernel image
+ cmdline_image Kernel command line obtained from boot loader	(6.6)
+ cmdline_load Kernel command line obtained from kernel image	(6.6)
  cpuinfo      Info about the CPU
  devices      Available devices (block and character)
  dma          Used DMS channels

Re: [PATCH RFC bootconfig] 2/2] fs/proc: Add /proc/cmdline_image for embedded arguments

[Masami Hiramatsu]

Hi Paul,

On Thu, 27 Jul 2023 20:37:01 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will show
> all kernel boot parameters, both those supplied by the boot loader and
> those embedded in the kernel image.  This works well for those who just
> want to see all of the kernel boot parameters, but is not helpful to those
> who need to see only those parameters that were embedded into the kernel
> image.  This is especially important in situations where there are many
> kernel images for different kernel versions and kernel configurations,
> all of which opens the door to a great deal of human error.

There is /proc/bootconfig file which shows all bootconfig entries and is
formatted as easily filter by grep (or any other line-based commands).
(e.g. `grep ^kernel\\. /proc/cmdline` will filter all kernel cmdline
parameters in the bootconfig)
Could you clarify the reason why you need a dump of bootconfig file?

Thank you,

> 
> Therefore, provide a /proc/cmdline_image file that shows only those kernel
> boot parameters that were embedded in the kernel image.  The output
> is in boot-image format, which allows easy reconcilation against the
> boot-config source file.
> 
> Why put this in /proc?  Because it is quite similar to /proc/cmdline, so
> it makes sense to put it in the same place that /proc/cmdline is located.
> 
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: <linux-fsdevel@vger.kernel.org>
> 
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> ---
>  fs/proc/cmdline.c    | 12 ++++++++++++
>  include/linux/init.h | 11 ++++++-----
>  init/main.c          |  9 +++++++++
>  3 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c
> index 1d0ef9d2949d..4ab5223198cb 100644
> --- a/fs/proc/cmdline.c
> +++ b/fs/proc/cmdline.c
> @@ -20,6 +20,15 @@ static int cmdline_load_proc_show(struct seq_file *m, void *v)
>  	return 0;
>  }
>  
> +static int cmdline_image_proc_show(struct seq_file *m, void *v)
> +{
> +#ifdef CONFIG_BOOT_CONFIG_FORCE
> +	seq_puts(m, saved_bootconfig_string);
> +	seq_putc(m, '\n');
> +#endif
> +	return 0;
> +}
> +
>  static int __init proc_cmdline_init(void)
>  {
>  	struct proc_dir_entry *pde;
> @@ -31,6 +40,9 @@ static int __init proc_cmdline_init(void)
>  		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
>  		pde_make_permanent(pde);
>  		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> +		pde = proc_create_single("cmdline_image", 0, NULL, cmdline_image_proc_show);
> +		pde_make_permanent(pde);
> +		pde->size = strnlen(saved_bootconfig_string, COMMAND_LINE_SIZE) + 1;
>  	}
>  	return 0;
>  }
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 29e75bbe7984..c075983c5015 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -14,7 +14,7 @@
>  #define __noinitretpoline
>  #endif
>  
> -/* These macros are used to mark some functions or 
> +/* These macros are used to mark some functions or
>   * initialized data (doesn't apply to uninitialized data)
>   * as `initialization' functions. The kernel can take this
>   * as hint that the function is used only during the initialization
> @@ -22,7 +22,7 @@
>   *
>   * Usage:
>   * For functions:
> - * 
> + *
>   * You should add __init immediately before the function name, like:
>   *
>   * static void __init initme(int x, int y)
> @@ -148,6 +148,7 @@ extern char boot_command_line[];
>  extern char *saved_command_line;
>  extern unsigned int saved_command_line_len;
>  extern unsigned int reset_devices;
> +extern char saved_bootconfig_string[];
>  
>  /* used by init/main.c */
>  void setup_arch(char **);
> @@ -184,7 +185,7 @@ extern void (*late_time_init)(void);
>  extern bool initcall_debug;
>  
>  #endif
> -  
> +
>  #ifndef MODULE
>  
>  #ifndef __ASSEMBLY__
> @@ -192,8 +193,8 @@ extern bool initcall_debug;
>  /*
>   * initcalls are now grouped by functionality into separate
>   * subsections. Ordering inside the subsections is determined
> - * by link order. 
> - * For backwards compatibility, initcall() puts the call in 
> + * by link order.
> + * For backwards compatibility, initcall() puts the call in
>   * the device init subsection.
>   *
>   * The `id' arg to __define_initcall() is needed so that multiple initcalls
> diff --git a/init/main.c b/init/main.c
> index 2121685c479a..981170da0b1c 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -146,6 +146,11 @@ static char *extra_command_line;
>  /* Extra init arguments */
>  static char *extra_init_args;
>  
> +/* Untouched boot-config string */
> +#ifdef CONFIG_BOOT_CONFIG_FORCE
> +char saved_bootconfig_string[COMMAND_LINE_SIZE] __ro_after_init;
> +#endif
> +
>  #ifdef CONFIG_BOOT_CONFIG
>  /* Is bootconfig on command line? */
>  static bool bootconfig_found;
> @@ -435,6 +440,10 @@ static void __init setup_boot_config(void)
>  		return;
>  	}
>  
> +#ifdef CONFIG_BOOT_CONFIG_FORCE
> +	strncpy(saved_bootconfig_string, data, COMMAND_LINE_SIZE);
> +#endif
> +
>  	if (size >= XBC_DATA_MAX) {
>  		pr_err("bootconfig size %ld greater than max size %d\n",
>  			(long)size, XBC_DATA_MAX);
> -- 
> 2.40.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Masami Hiramatsu]

Hi Paul,

On Thu, 27 Jul 2023 20:37:00 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> show all kernel boot parameters, both those supplied by the boot loader
> and those embedded in the kernel image.  This works well for those who
> just want to see all of the kernel boot parameters, but is not helpful to
> those who need to see only those parameters supplied by the boot loader.
> This is especially important when these parameters are presented to the
> boot loader by automation that might gather them from diverse sources.
> 
> Therefore, provide a /proc/cmdline_load file that shows only those kernel
> boot parameters supplied by the boot loader.

If I understand correctly, /proc/cmdline_load is something like
/proc/cmdline_load - `/proc/bootconfig | grep ^kernel\\.`.

BTW, what about CONFIG_CMDLINE? We already have that Kconfig and it is also
merged with the command line specified by boot loader. Should we also
expose that? (when CONFIG_CMDLINE_OVERRIDE=y, we don't need it because
cmdline is always overridden by the CONFIG_CMDLINE) Unfortunatelly, this
option is implemented in each arch init, so we have to change all of them...

Thank you,

> 
> Why put this in /proc?  Because it is quite similar to /proc/cmdline, so
> it makes sense to put it in the same place that /proc/cmdline is located.
> 
> [ sfr: Apply kernel test robot feedback. ]
> 
> Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: <linux-fsdevel@vger.kernel.org>
> ---
>  fs/proc/cmdline.c    | 13 +++++++++++++
>  include/linux/init.h |  3 ++-
>  init/main.c          |  2 +-
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c
> index a6f76121955f..1d0ef9d2949d 100644
> --- a/fs/proc/cmdline.c
> +++ b/fs/proc/cmdline.c
> @@ -3,6 +3,7 @@
>  #include <linux/init.h>
>  #include <linux/proc_fs.h>
>  #include <linux/seq_file.h>
> +#include <asm/setup.h>
>  #include "internal.h"
>  
>  static int cmdline_proc_show(struct seq_file *m, void *v)
> @@ -12,6 +13,13 @@ static int cmdline_proc_show(struct seq_file *m, void *v)
>  	return 0;
>  }
>  
> +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> +{
> +	seq_puts(m, boot_command_line);
> +	seq_putc(m, '\n');
> +	return 0;
> +}
> +
>  static int __init proc_cmdline_init(void)
>  {
>  	struct proc_dir_entry *pde;
> @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
>  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
>  	pde_make_permanent(pde);
>  	pde->size = saved_command_line_len + 1;
> +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> +		pde_make_permanent(pde);
> +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> +	}
>  	return 0;
>  }
>  fs_initcall(proc_cmdline_init);
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 266c3e1640d4..29e75bbe7984 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -112,6 +112,7 @@
>  #define __REFCONST       .section       ".ref.rodata", "a"
>  
>  #ifndef __ASSEMBLY__
> +
>  /*
>   * Used for initialization calls..
>   */
> @@ -143,7 +144,7 @@ struct file_system_type;
>  
>  /* Defined in init/main.c */
>  extern int do_one_initcall(initcall_t fn);
> -extern char __initdata boot_command_line[];
> +extern char boot_command_line[];

FYI, boot_command_line[] is mixture of built-in cmdline string with
bootloader cmdline string.

>  extern char *saved_command_line;
>  extern unsigned int saved_command_line_len;
>  extern unsigned int reset_devices;
> diff --git a/init/main.c b/init/main.c
> index ad920fac325c..2121685c479a 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
>  void (*__initdata late_time_init)(void);
>  
>  /* Untouched command line saved by arch-specific code. */
> -char __initdata boot_command_line[COMMAND_LINE_SIZE];
> +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
>  /* Untouched saved command line (eg. for /proc) */
>  char *saved_command_line __ro_after_init;
>  unsigned int saved_command_line_len __ro_after_init;
> -- 
> 2.40.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH RFC bootconfig] 2/2] fs/proc: Add /proc/cmdline_image for embedded arguments

[Paul E. McKenney]

On Sat, Jul 29, 2023 at 11:23:46PM +0900, Masami Hiramatsu wrote:
> Hi Paul,
> 
> On Thu, 27 Jul 2023 20:37:01 -0700
> "Paul E. McKenney" <paulmck@kernel.org> wrote:
> 
> > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will show
> > all kernel boot parameters, both those supplied by the boot loader and
> > those embedded in the kernel image.  This works well for those who just
> > want to see all of the kernel boot parameters, but is not helpful to those
> > who need to see only those parameters that were embedded into the kernel
> > image.  This is especially important in situations where there are many
> > kernel images for different kernel versions and kernel configurations,
> > all of which opens the door to a great deal of human error.
> 
> There is /proc/bootconfig file which shows all bootconfig entries and is
> formatted as easily filter by grep (or any other line-based commands).
> (e.g. `grep ^kernel\\. /proc/cmdline` will filter all kernel cmdline
> parameters in the bootconfig)
> Could you clarify the reason why you need a dump of bootconfig file?

Because I was unaware of /proc/bootconfig?  ;-)

So how about if I replace this patch of mine with the following?

And thank you for pointing me at /proc/bootconfig.

							Thanx, Paul

------------------------------------------------------------------------

diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 98c43c5ef1ee..832d66d4e396 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -684,6 +684,7 @@ files are there, and which are missing.
  File         Content
  ============ ===============================================================
  apm          Advanced power management info
+ bootconfig   Kernel command line obtained from boot config	(5.5)
  buddyinfo    Kernel memory allocator information (see text)	(2.5)
  bus          Directory containing bus specific information
  cmdline      Kernel command line, both from bootloader and embedded

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Paul E. McKenney]

On Sat, Jul 29, 2023 at 11:29:29PM +0900, Masami Hiramatsu wrote:
> Hi Paul,
> 
> On Thu, 27 Jul 2023 20:37:00 -0700
> "Paul E. McKenney" <paulmck@kernel.org> wrote:
> 
> > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > show all kernel boot parameters, both those supplied by the boot loader
> > and those embedded in the kernel image.  This works well for those who
> > just want to see all of the kernel boot parameters, but is not helpful to
> > those who need to see only those parameters supplied by the boot loader.
> > This is especially important when these parameters are presented to the
> > boot loader by automation that might gather them from diverse sources.
> > 
> > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > boot parameters supplied by the boot loader.
> 
> If I understand correctly, /proc/cmdline_load is something like
> /proc/cmdline_load - `/proc/bootconfig | grep ^kernel\\.`.

Yes, very much something like that.

For one use case, suppose you have a kernel that gets some boot parameters
from the boot loader and some from bootconfig.  If you want to kexec()
into a new kernel, you must tell kexec() what the kernel boot parameters
are.  However, you must *not* tell kexec() about any of the current
kernel's parameters that came from bootconfig, because those should
instead be supplied by the new kernel being kexec()ed into.

So you must pass in only those parameters that came from the boot loader,
hence my proposed /proc/cmdline_load.

> BTW, what about CONFIG_CMDLINE? We already have that Kconfig and it is also
> merged with the command line specified by boot loader. Should we also
> expose that? (when CONFIG_CMDLINE_OVERRIDE=y, we don't need it because
> cmdline is always overridden by the CONFIG_CMDLINE) Unfortunatelly, this
> option is implemented in each arch init, so we have to change all of them...

The use case is embedded systems, right?  I have no idea whether they
have a use case requiring this.  Do those sorts of embedded systems
use kexec()?  (I don't know of any that do, but then again, I haven't
been looking.)

This arch init is in setup_arch(), correct?  If so, one option is to
make start_kernel() or something that it invokes make a copy of the
command line just before invoking setup_arch().  Full disclosure: I
have not yet looked at all the ins and outs of CONFIG_CMDLINE, so this
suggestion should be viewed with appropriate skepticism.

> Thank you,
> 
> > 
> > Why put this in /proc?  Because it is quite similar to /proc/cmdline, so
> > it makes sense to put it in the same place that /proc/cmdline is located.
> > 
> > [ sfr: Apply kernel test robot feedback. ]
> > 
> > Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Alexey Dobriyan <adobriyan@gmail.com>
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Cc: <linux-fsdevel@vger.kernel.org>
> > ---
> >  fs/proc/cmdline.c    | 13 +++++++++++++
> >  include/linux/init.h |  3 ++-
> >  init/main.c          |  2 +-
> >  3 files changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c
> > index a6f76121955f..1d0ef9d2949d 100644
> > --- a/fs/proc/cmdline.c
> > +++ b/fs/proc/cmdline.c
> > @@ -3,6 +3,7 @@
> >  #include <linux/init.h>
> >  #include <linux/proc_fs.h>
> >  #include <linux/seq_file.h>
> > +#include <asm/setup.h>
> >  #include "internal.h"
> >  
> >  static int cmdline_proc_show(struct seq_file *m, void *v)
> > @@ -12,6 +13,13 @@ static int cmdline_proc_show(struct seq_file *m, void *v)
> >  	return 0;
> >  }
> >  
> > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > +{
> > +	seq_puts(m, boot_command_line);
> > +	seq_putc(m, '\n');
> > +	return 0;
> > +}
> > +
> >  static int __init proc_cmdline_init(void)
> >  {
> >  	struct proc_dir_entry *pde;
> > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> >  	pde_make_permanent(pde);
> >  	pde->size = saved_command_line_len + 1;
> > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > +		pde_make_permanent(pde);
> > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > +	}
> >  	return 0;
> >  }
> >  fs_initcall(proc_cmdline_init);
> > diff --git a/include/linux/init.h b/include/linux/init.h
> > index 266c3e1640d4..29e75bbe7984 100644
> > --- a/include/linux/init.h
> > +++ b/include/linux/init.h
> > @@ -112,6 +112,7 @@
> >  #define __REFCONST       .section       ".ref.rodata", "a"
> >  
> >  #ifndef __ASSEMBLY__
> > +
> >  /*
> >   * Used for initialization calls..
> >   */
> > @@ -143,7 +144,7 @@ struct file_system_type;
> >  
> >  /* Defined in init/main.c */
> >  extern int do_one_initcall(initcall_t fn);
> > -extern char __initdata boot_command_line[];
> > +extern char boot_command_line[];
> 
> FYI, boot_command_line[] is mixture of built-in cmdline string with
> bootloader cmdline string.

So if we also need to separate out the CONFIG_CMDLINE arguments, then
/proc/cmdline_load will need to come from some string saved off before
the CONFIG_CMDLINE processing, correct?  I would expect that to be a
separate patch series, but if it is needed, I would be happy to look
into setting it up, as long as I am in the area.

My tests indicate that boot_command_line[] doesn't contain any bootconfig
(and opposed to CONFIG_CMDLINE) arguments, but I could easily have missed
some other corner-case configuration.

And thank you for looking this over!

							Thanx, Paul

> >  extern char *saved_command_line;
> >  extern unsigned int saved_command_line_len;
> >  extern unsigned int reset_devices;
> > diff --git a/init/main.c b/init/main.c
> > index ad920fac325c..2121685c479a 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
> >  void (*__initdata late_time_init)(void);
> >  
> >  /* Untouched command line saved by arch-specific code. */
> > -char __initdata boot_command_line[COMMAND_LINE_SIZE];
> > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
> >  /* Untouched saved command line (eg. for /proc) */
> >  char *saved_command_line __ro_after_init;
> >  unsigned int saved_command_line_len __ro_after_init;
> > -- 
> > 2.40.1
> > 
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH RFC bootconfig] 2/2] fs/proc: Add /proc/cmdline_image for embedded arguments

[Masami Hiramatsu]

On Sat, 29 Jul 2023 08:41:46 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> On Sat, Jul 29, 2023 at 11:23:46PM +0900, Masami Hiramatsu wrote:
> > Hi Paul,
> > 
> > On Thu, 27 Jul 2023 20:37:01 -0700
> > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > 
> > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will show
> > > all kernel boot parameters, both those supplied by the boot loader and
> > > those embedded in the kernel image.  This works well for those who just
> > > want to see all of the kernel boot parameters, but is not helpful to those
> > > who need to see only those parameters that were embedded into the kernel
> > > image.  This is especially important in situations where there are many
> > > kernel images for different kernel versions and kernel configurations,
> > > all of which opens the door to a great deal of human error.
> > 
> > There is /proc/bootconfig file which shows all bootconfig entries and is
> > formatted as easily filter by grep (or any other line-based commands).
> > (e.g. `grep ^kernel\\. /proc/cmdline` will filter all kernel cmdline
> > parameters in the bootconfig)
> > Could you clarify the reason why you need a dump of bootconfig file?
> 
> Because I was unaware of /proc/bootconfig?  ;-)

Oh :)

> 
> So how about if I replace this patch of mine with the following?

Yes, I missed to update the proc.rst. Thanks!

> 
> And thank you for pointing me at /proc/bootconfig.
> 
> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> index 98c43c5ef1ee..832d66d4e396 100644
> --- a/Documentation/filesystems/proc.rst
> +++ b/Documentation/filesystems/proc.rst
> @@ -684,6 +684,7 @@ files are there, and which are missing.
>   File         Content
>   ============ ===============================================================
>   apm          Advanced power management info
> + bootconfig   Kernel command line obtained from boot config	(5.5)
>   buddyinfo    Kernel memory allocator information (see text)	(2.5)
>   bus          Directory containing bus specific information
>   cmdline      Kernel command line, both from bootloader and embedded


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Masami Hiramatsu]

On Sat, 29 Jul 2023 09:16:56 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> On Sat, Jul 29, 2023 at 11:29:29PM +0900, Masami Hiramatsu wrote:
> > Hi Paul,
> > 
> > On Thu, 27 Jul 2023 20:37:00 -0700
> > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > 
> > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > show all kernel boot parameters, both those supplied by the boot loader
> > > and those embedded in the kernel image.  This works well for those who
> > > just want to see all of the kernel boot parameters, but is not helpful to
> > > those who need to see only those parameters supplied by the boot loader.
> > > This is especially important when these parameters are presented to the
> > > boot loader by automation that might gather them from diverse sources.
> > > 
> > > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > > boot parameters supplied by the boot loader.
> > 
> > If I understand correctly, /proc/cmdline_load is something like
> > /proc/cmdline_load - `/proc/bootconfig | grep ^kernel\\.`.

^^^ /proc/cmdline - `/proc/bootconfig | grep ^kernel\\.`

> 
> Yes, very much something like that.
> 
> For one use case, suppose you have a kernel that gets some boot parameters
> from the boot loader and some from bootconfig.  If you want to kexec()
> into a new kernel, you must tell kexec() what the kernel boot parameters
> are.  However, you must *not* tell kexec() about any of the current
> kernel's parameters that came from bootconfig, because those should
> instead be supplied by the new kernel being kexec()ed into.
> 
> So you must pass in only those parameters that came from the boot loader,
> hence my proposed /proc/cmdline_load.

Ah, I got it. Indeed, for kexec, we need to drop the options from
the bootconfig.

> 
> > BTW, what about CONFIG_CMDLINE? We already have that Kconfig and it is also
> > merged with the command line specified by boot loader. Should we also
> > expose that? (when CONFIG_CMDLINE_OVERRIDE=y, we don't need it because
> > cmdline is always overridden by the CONFIG_CMDLINE) Unfortunatelly, this
> > option is implemented in each arch init, so we have to change all of them...
> 
> The use case is embedded systems, right?  I have no idea whether they
> have a use case requiring this.  Do those sorts of embedded systems
> use kexec()?  (I don't know of any that do, but then again, I haven't
> been looking.)

Not sure, I guess it is possible to use kexec() for kdump or warm reboot,
but it should be rare and we can expand this if someone need it.

> 
> This arch init is in setup_arch(), correct?  If so, one option is to
> make start_kernel() or something that it invokes make a copy of the
> command line just before invoking setup_arch().  Full disclosure: I
> have not yet looked at all the ins and outs of CONFIG_CMDLINE, so this
> suggestion should be viewed with appropriate skepticism.

Yeah, maybe it is the best way to do.
Anyway, I understand the reason why we need this interface.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you!

> 
> > Thank you,
> > 
> > > 
> > > Why put this in /proc?  Because it is quite similar to /proc/cmdline, so
> > > it makes sense to put it in the same place that /proc/cmdline is located.
> > > 
> > > [ sfr: Apply kernel test robot feedback. ]
> > > 
> > > Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > > Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Cc: Alexey Dobriyan <adobriyan@gmail.com>
> > > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > > Cc: <linux-fsdevel@vger.kernel.org>
> > > ---
> > >  fs/proc/cmdline.c    | 13 +++++++++++++
> > >  include/linux/init.h |  3 ++-
> > >  init/main.c          |  2 +-
> > >  3 files changed, 16 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c
> > > index a6f76121955f..1d0ef9d2949d 100644
> > > --- a/fs/proc/cmdline.c
> > > +++ b/fs/proc/cmdline.c
> > > @@ -3,6 +3,7 @@
> > >  #include <linux/init.h>
> > >  #include <linux/proc_fs.h>
> > >  #include <linux/seq_file.h>
> > > +#include <asm/setup.h>
> > >  #include "internal.h"
> > >  
> > >  static int cmdline_proc_show(struct seq_file *m, void *v)
> > > @@ -12,6 +13,13 @@ static int cmdline_proc_show(struct seq_file *m, void *v)
> > >  	return 0;
> > >  }
> > >  
> > > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > > +{
> > > +	seq_puts(m, boot_command_line);
> > > +	seq_putc(m, '\n');
> > > +	return 0;
> > > +}
> > > +
> > >  static int __init proc_cmdline_init(void)
> > >  {
> > >  	struct proc_dir_entry *pde;
> > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> > >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> > >  	pde_make_permanent(pde);
> > >  	pde->size = saved_command_line_len + 1;
> > > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > > +		pde_make_permanent(pde);
> > > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > +	}
> > >  	return 0;
> > >  }
> > >  fs_initcall(proc_cmdline_init);
> > > diff --git a/include/linux/init.h b/include/linux/init.h
> > > index 266c3e1640d4..29e75bbe7984 100644
> > > --- a/include/linux/init.h
> > > +++ b/include/linux/init.h
> > > @@ -112,6 +112,7 @@
> > >  #define __REFCONST       .section       ".ref.rodata", "a"
> > >  
> > >  #ifndef __ASSEMBLY__
> > > +
> > >  /*
> > >   * Used for initialization calls..
> > >   */
> > > @@ -143,7 +144,7 @@ struct file_system_type;
> > >  
> > >  /* Defined in init/main.c */
> > >  extern int do_one_initcall(initcall_t fn);
> > > -extern char __initdata boot_command_line[];
> > > +extern char boot_command_line[];
> > 
> > FYI, boot_command_line[] is mixture of built-in cmdline string with
> > bootloader cmdline string.
> 
> So if we also need to separate out the CONFIG_CMDLINE arguments, then
> /proc/cmdline_load will need to come from some string saved off before
> the CONFIG_CMDLINE processing, correct?  I would expect that to be a
> separate patch series, but if it is needed, I would be happy to look
> into setting it up, as long as I am in the area.
> 
> My tests indicate that boot_command_line[] doesn't contain any bootconfig
> (and opposed to CONFIG_CMDLINE) arguments, but I could easily have missed
> some other corner-case configuration.
> 
> And thank you for looking this over!
> 
> 							Thanx, Paul
> 
> > >  extern char *saved_command_line;
> > >  extern unsigned int saved_command_line_len;
> > >  extern unsigned int reset_devices;
> > > diff --git a/init/main.c b/init/main.c
> > > index ad920fac325c..2121685c479a 100644
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
> > >  void (*__initdata late_time_init)(void);
> > >  
> > >  /* Untouched command line saved by arch-specific code. */
> > > -char __initdata boot_command_line[COMMAND_LINE_SIZE];
> > > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
> > >  /* Untouched saved command line (eg. for /proc) */
> > >  char *saved_command_line __ro_after_init;
> > >  unsigned int saved_command_line_len __ro_after_init;
> > > -- 
> > > 2.40.1
> > > 
> > 
> > 
> > -- 
> > Masami Hiramatsu (Google) <mhiramat@kernel.org>


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Paul E. McKenney]

On Sun, Jul 30, 2023 at 10:58:44AM +0900, Masami Hiramatsu wrote:
> On Sat, 29 Jul 2023 09:16:56 -0700
> "Paul E. McKenney" <paulmck@kernel.org> wrote:
> 
> > On Sat, Jul 29, 2023 at 11:29:29PM +0900, Masami Hiramatsu wrote:
> > > Hi Paul,
> > > 
> > > On Thu, 27 Jul 2023 20:37:00 -0700
> > > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > 
> > > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > > show all kernel boot parameters, both those supplied by the boot loader
> > > > and those embedded in the kernel image.  This works well for those who
> > > > just want to see all of the kernel boot parameters, but is not helpful to
> > > > those who need to see only those parameters supplied by the boot loader.
> > > > This is especially important when these parameters are presented to the
> > > > boot loader by automation that might gather them from diverse sources.
> > > > 
> > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > > > boot parameters supplied by the boot loader.
> > > 
> > > If I understand correctly, /proc/cmdline_load is something like
> > > /proc/cmdline_load - `/proc/bootconfig | grep ^kernel\\.`.
> 
> ^^^ /proc/cmdline - `/proc/bootconfig | grep ^kernel\\.`

Heh!  My mind autocorrected without me noticing.  ;-)

> > Yes, very much something like that.
> > 
> > For one use case, suppose you have a kernel that gets some boot parameters
> > from the boot loader and some from bootconfig.  If you want to kexec()
> > into a new kernel, you must tell kexec() what the kernel boot parameters
> > are.  However, you must *not* tell kexec() about any of the current
> > kernel's parameters that came from bootconfig, because those should
> > instead be supplied by the new kernel being kexec()ed into.
> > 
> > So you must pass in only those parameters that came from the boot loader,
> > hence my proposed /proc/cmdline_load.
> 
> Ah, I got it. Indeed, for kexec, we need to drop the options from
> the bootconfig.
> 
> > > BTW, what about CONFIG_CMDLINE? We already have that Kconfig and it is also
> > > merged with the command line specified by boot loader. Should we also
> > > expose that? (when CONFIG_CMDLINE_OVERRIDE=y, we don't need it because
> > > cmdline is always overridden by the CONFIG_CMDLINE) Unfortunatelly, this
> > > option is implemented in each arch init, so we have to change all of them...
> > 
> > The use case is embedded systems, right?  I have no idea whether they
> > have a use case requiring this.  Do those sorts of embedded systems
> > use kexec()?  (I don't know of any that do, but then again, I haven't
> > been looking.)
> 
> Not sure, I guess it is possible to use kexec() for kdump or warm reboot,
> but it should be rare and we can expand this if someone need it.

Works for me!

> > This arch init is in setup_arch(), correct?  If so, one option is to
> > make start_kernel() or something that it invokes make a copy of the
> > command line just before invoking setup_arch().  Full disclosure: I
> > have not yet looked at all the ins and outs of CONFIG_CMDLINE, so this
> > suggestion should be viewed with appropriate skepticism.
> 
> Yeah, maybe it is the best way to do.
> Anyway, I understand the reason why we need this interface.
> 
> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you!  I will apply your Acked-by on my next rebase.

							Thanx, Paul

> Thank you!
> 
> > 
> > > Thank you,
> > > 
> > > > 
> > > > Why put this in /proc?  Because it is quite similar to /proc/cmdline, so
> > > > it makes sense to put it in the same place that /proc/cmdline is located.
> > > > 
> > > > [ sfr: Apply kernel test robot feedback. ]
> > > > 
> > > > Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > > > Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> > > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > > Cc: Alexey Dobriyan <adobriyan@gmail.com>
> > > > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > > > Cc: <linux-fsdevel@vger.kernel.org>
> > > > ---
> > > >  fs/proc/cmdline.c    | 13 +++++++++++++
> > > >  include/linux/init.h |  3 ++-
> > > >  init/main.c          |  2 +-
> > > >  3 files changed, 16 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c
> > > > index a6f76121955f..1d0ef9d2949d 100644
> > > > --- a/fs/proc/cmdline.c
> > > > +++ b/fs/proc/cmdline.c
> > > > @@ -3,6 +3,7 @@
> > > >  #include <linux/init.h>
> > > >  #include <linux/proc_fs.h>
> > > >  #include <linux/seq_file.h>
> > > > +#include <asm/setup.h>
> > > >  #include "internal.h"
> > > >  
> > > >  static int cmdline_proc_show(struct seq_file *m, void *v)
> > > > @@ -12,6 +13,13 @@ static int cmdline_proc_show(struct seq_file *m, void *v)
> > > >  	return 0;
> > > >  }
> > > >  
> > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > > > +{
> > > > +	seq_puts(m, boot_command_line);
> > > > +	seq_putc(m, '\n');
> > > > +	return 0;
> > > > +}
> > > > +
> > > >  static int __init proc_cmdline_init(void)
> > > >  {
> > > >  	struct proc_dir_entry *pde;
> > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> > > >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> > > >  	pde_make_permanent(pde);
> > > >  	pde->size = saved_command_line_len + 1;
> > > > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > > > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > > > +		pde_make_permanent(pde);
> > > > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > > +	}
> > > >  	return 0;
> > > >  }
> > > >  fs_initcall(proc_cmdline_init);
> > > > diff --git a/include/linux/init.h b/include/linux/init.h
> > > > index 266c3e1640d4..29e75bbe7984 100644
> > > > --- a/include/linux/init.h
> > > > +++ b/include/linux/init.h
> > > > @@ -112,6 +112,7 @@
> > > >  #define __REFCONST       .section       ".ref.rodata", "a"
> > > >  
> > > >  #ifndef __ASSEMBLY__
> > > > +
> > > >  /*
> > > >   * Used for initialization calls..
> > > >   */
> > > > @@ -143,7 +144,7 @@ struct file_system_type;
> > > >  
> > > >  /* Defined in init/main.c */
> > > >  extern int do_one_initcall(initcall_t fn);
> > > > -extern char __initdata boot_command_line[];
> > > > +extern char boot_command_line[];
> > > 
> > > FYI, boot_command_line[] is mixture of built-in cmdline string with
> > > bootloader cmdline string.
> > 
> > So if we also need to separate out the CONFIG_CMDLINE arguments, then
> > /proc/cmdline_load will need to come from some string saved off before
> > the CONFIG_CMDLINE processing, correct?  I would expect that to be a
> > separate patch series, but if it is needed, I would be happy to look
> > into setting it up, as long as I am in the area.
> > 
> > My tests indicate that boot_command_line[] doesn't contain any bootconfig
> > (and opposed to CONFIG_CMDLINE) arguments, but I could easily have missed
> > some other corner-case configuration.
> > 
> > And thank you for looking this over!
> > 
> > 							Thanx, Paul
> > 
> > > >  extern char *saved_command_line;
> > > >  extern unsigned int saved_command_line_len;
> > > >  extern unsigned int reset_devices;
> > > > diff --git a/init/main.c b/init/main.c
> > > > index ad920fac325c..2121685c479a 100644
> > > > --- a/init/main.c
> > > > +++ b/init/main.c
> > > > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
> > > >  void (*__initdata late_time_init)(void);
> > > >  
> > > >  /* Untouched command line saved by arch-specific code. */
> > > > -char __initdata boot_command_line[COMMAND_LINE_SIZE];
> > > > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
> > > >  /* Untouched saved command line (eg. for /proc) */
> > > >  char *saved_command_line __ro_after_init;
> > > >  unsigned int saved_command_line_len __ro_after_init;
> > > > -- 
> > > > 2.40.1
> > > > 
> > > 
> > > 
> > > -- 
> > > Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

[PATCH RFC v2 bootconfig 0/3] Distinguish bootloader and embedded kernel parameters

[Paul E. McKenney]

On Thu, Jul 27, 2023 at 08:35:10PM -0700, Paul E. McKenney wrote:
Hello!

This series provides a /proc interface parallel to /proc/cmdline that
provides only those kernel boot parameters that were provided by the
bootloader in a new /proc/cmdline_load.  This is especially important
when these parameters are presented to the boot loader by automation
that might gather them from diverse sources, and also when a kexec-based
reboot process pulls the kernel boot parameters from /proc.  If such a
reboot process uses /proc/cmdline, the kernel parameters from the image
are replicated on every reboot, which can be frustrating when the new
kernel has different embedded kernel boot parameters.

Why put these in /proc?  Because they is quite similar to /proc/cmdline,
so it makes sense to put it in the same place that /proc/cmdline is
located.

1.	Update /proc/cmdline documentation to include boot config.

2.	fs/proc: Add /proc/cmdline_load for boot loader arguments.

3.	Add /proc/bootconfig to proc.rst.

Changes since v1:

o	Dropped /proc/cmdline_image in favor of the existing
	/proc/bootconfig.

o	Pulled in fixes from Stephen and Arnd.

o	Added documentation for /proc/bootconfig.

						Thanx, Paul

------------------------------------------------------------------------

 Documentation/filesystems/proc.rst   |    2 ++
 b/Documentation/filesystems/proc.rst |    3 ++-
 b/fs/proc/cmdline.c                  |   13 +++++++++++++
 b/include/linux/init.h               |    3 ++-
 b/init/main.c                        |    2 +-
 5 files changed, 20 insertions(+), 3 deletions(-)

[PATCH RFC v2 bootconfig 1/3] doc: Update /proc/cmdline documentation to include boot config

[Paul E. McKenney]

Update the /proc/cmdline documentation to explicitly state that this
file provides kernel boot parameters obtained via boot config from the
kernel image as well as those supplied by the boot loader.

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Arnd Bergmann <arnd@kernel.org>
---
 Documentation/filesystems/proc.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 7897a7dafcbc..75a8c899ebcc 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -686,7 +686,8 @@ files are there, and which are missing.
  apm          Advanced power management info
  buddyinfo    Kernel memory allocator information (see text)	(2.5)
  bus          Directory containing bus specific information
- cmdline      Kernel command line
+ cmdline      Kernel command line, both from bootloader and embedded
+ 	      in the kernel image.
  cpuinfo      Info about the CPU
  devices      Available devices (block and character)
  dma          Used DMS channels
-- 
2.40.1

[PATCH RFC v2 bootconfig 2/3] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Paul E. McKenney]

In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
show all kernel boot parameters, both those supplied by the boot loader
and those embedded in the kernel image.  This works well for those who
just want to see all of the kernel boot parameters, but is not helpful to
those who need to see only those parameters supplied by the boot loader.
This is especially important when these parameters are presented to the
boot loader by automation that might gather them from diverse sources.
It is also useful when booting the next kernel via kexec(), in which
case it is necessary to supply only those kernel command-line arguments
from the boot loader, and most definitely not those that were embedded
into the current kernel.

Therefore, provide a /proc/cmdline_load file that shows only those kernel
boot parameters supplied by the boot loader.

Why put this in /proc?  Because it is quite similar to /proc/cmdline, so
it makes sense to put it in the same place that /proc/cmdline is located.

[ sfr: Apply kernel test robot feedback. ]
[ paulmck: Apply Randy Dunlap feedback. ]

Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: <linux-fsdevel@vger.kernel.org>
---
 Documentation/filesystems/proc.rst |  1 +
 fs/proc/cmdline.c                  | 13 +++++++++++++
 include/linux/init.h               |  3 ++-
 init/main.c                        |  2 +-
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 75a8c899ebcc..c2aee55248a8 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -688,6 +688,7 @@ files are there, and which are missing.
  bus          Directory containing bus specific information
  cmdline      Kernel command line, both from bootloader and embedded
  	      in the kernel image.
+ cmdline_load Kernel command line obtained from kernel image	(6.6)
  cpuinfo      Info about the CPU
  devices      Available devices (block and character)
  dma          Used DMS channels
diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c
index a6f76121955f..1d0ef9d2949d 100644
--- a/fs/proc/cmdline.c
+++ b/fs/proc/cmdline.c
@@ -3,6 +3,7 @@
 #include <linux/init.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
+#include <asm/setup.h>
 #include "internal.h"
 
 static int cmdline_proc_show(struct seq_file *m, void *v)
@@ -12,6 +13,13 @@ static int cmdline_proc_show(struct seq_file *m, void *v)
 	return 0;
 }
 
+static int cmdline_load_proc_show(struct seq_file *m, void *v)
+{
+	seq_puts(m, boot_command_line);
+	seq_putc(m, '\n');
+	return 0;
+}
+
 static int __init proc_cmdline_init(void)
 {
 	struct proc_dir_entry *pde;
@@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
 	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
 	pde_make_permanent(pde);
 	pde->size = saved_command_line_len + 1;
+	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
+		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
+		pde_make_permanent(pde);
+		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
+	}
 	return 0;
 }
 fs_initcall(proc_cmdline_init);
diff --git a/include/linux/init.h b/include/linux/init.h
index 266c3e1640d4..29e75bbe7984 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -112,6 +112,7 @@
 #define __REFCONST       .section       ".ref.rodata", "a"
 
 #ifndef __ASSEMBLY__
+
 /*
  * Used for initialization calls..
  */
@@ -143,7 +144,7 @@ struct file_system_type;
 
 /* Defined in init/main.c */
 extern int do_one_initcall(initcall_t fn);
-extern char __initdata boot_command_line[];
+extern char boot_command_line[];
 extern char *saved_command_line;
 extern unsigned int saved_command_line_len;
 extern unsigned int reset_devices;
diff --git a/init/main.c b/init/main.c
index ad920fac325c..2121685c479a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
 void (*__initdata late_time_init)(void);
 
 /* Untouched command line saved by arch-specific code. */
-char __initdata boot_command_line[COMMAND_LINE_SIZE];
+char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
 /* Untouched saved command line (eg. for /proc) */
 char *saved_command_line __ro_after_init;
 unsigned int saved_command_line_len __ro_after_init;
-- 
2.40.1

[PATCH RFC v2 bootconfig 3/3] doc: Add /proc/bootconfig to proc.rst

[Paul E. McKenney]

Add /proc/bootconfig description to Documentation/filesystems/proc.rst.

Reported-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 Documentation/filesystems/proc.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index c2aee55248a8..4b9e9510a53f 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -684,6 +684,7 @@ files are there, and which are missing.
  File         Content
  ============ ===============================================================
  apm          Advanced power management info
+ bootconfig   Kernel command line obtained from boot config	(5.5)
  buddyinfo    Kernel memory allocator information (see text)	(2.5)
  bus          Directory containing bus specific information
  cmdline      Kernel command line, both from bootloader and embedded
-- 
2.40.1

Re: [PATCH RFC v2 bootconfig 1/3] doc: Update /proc/cmdline documentation to include boot config

[Masami Hiramatsu]

On Mon, 31 Jul 2023 16:31:28 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> Update the /proc/cmdline documentation to explicitly state that this
> file provides kernel boot parameters obtained via boot config from the
> kernel image as well as those supplied by the boot loader.
> 
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Arnd Bergmann <arnd@kernel.org>

Looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you!

> ---
>  Documentation/filesystems/proc.rst | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> index 7897a7dafcbc..75a8c899ebcc 100644
> --- a/Documentation/filesystems/proc.rst
> +++ b/Documentation/filesystems/proc.rst
> @@ -686,7 +686,8 @@ files are there, and which are missing.
>   apm          Advanced power management info
>   buddyinfo    Kernel memory allocator information (see text)	(2.5)
>   bus          Directory containing bus specific information
> - cmdline      Kernel command line
> + cmdline      Kernel command line, both from bootloader and embedded
> + 	      in the kernel image.
>   cpuinfo      Info about the CPU
>   devices      Available devices (block and character)
>   dma          Used DMS channels
> -- 
> 2.40.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH RFC v2 bootconfig 3/3] doc: Add /proc/bootconfig to proc.rst

[Masami Hiramatsu]

On Mon, 31 Jul 2023 16:31:30 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> Add /proc/bootconfig description to Documentation/filesystems/proc.rst.
> 

Looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>


Thank you!

> Reported-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> ---
>  Documentation/filesystems/proc.rst | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> index c2aee55248a8..4b9e9510a53f 100644
> --- a/Documentation/filesystems/proc.rst
> +++ b/Documentation/filesystems/proc.rst
> @@ -684,6 +684,7 @@ files are there, and which are missing.
>   File         Content
>   ============ ===============================================================
>   apm          Advanced power management info
> + bootconfig   Kernel command line obtained from boot config	(5.5)
>   buddyinfo    Kernel memory allocator information (see text)	(2.5)
>   bus          Directory containing bus specific information
>   cmdline      Kernel command line, both from bootloader and embedded
> -- 
> 2.40.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH RFC v2 bootconfig 1/3] doc: Update /proc/cmdline documentation to include boot config

[Randy Dunlap]

On 7/31/23 16:31, Paul E. McKenney wrote:
> Update the /proc/cmdline documentation to explicitly state that this
> file provides kernel boot parameters obtained via boot config from the
> kernel image as well as those supplied by the boot loader.
> 
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Arnd Bergmann <arnd@kernel.org>

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

> ---
>  Documentation/filesystems/proc.rst | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> index 7897a7dafcbc..75a8c899ebcc 100644
> --- a/Documentation/filesystems/proc.rst
> +++ b/Documentation/filesystems/proc.rst
> @@ -686,7 +686,8 @@ files are there, and which are missing.
>   apm          Advanced power management info
>   buddyinfo    Kernel memory allocator information (see text)	(2.5)
>   bus          Directory containing bus specific information
> - cmdline      Kernel command line
> + cmdline      Kernel command line, both from bootloader and embedded
> + 	      in the kernel image.
>   cpuinfo      Info about the CPU
>   devices      Available devices (block and character)
>   dma          Used DMS channels

-- 
~Randy

Re: [PATCH RFC v2 bootconfig 1/3] doc: Update /proc/cmdline documentation to include boot config

[Paul E. McKenney]

On Mon, Jul 31, 2023 at 07:00:22PM -0700, Randy Dunlap wrote:
> On 7/31/23 16:31, Paul E. McKenney wrote:
> > Update the /proc/cmdline documentation to explicitly state that this
> > file provides kernel boot parameters obtained via boot config from the
> > kernel image as well as those supplied by the boot loader.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Randy Dunlap <rdunlap@infradead.org>
> > Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> > Cc: Arnd Bergmann <arnd@kernel.org>
> 
> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

Thank you both!  I will apply these on my next rebase.

							Thanx, Paul

> Thanks.
> 
> > ---
> >  Documentation/filesystems/proc.rst | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> > index 7897a7dafcbc..75a8c899ebcc 100644
> > --- a/Documentation/filesystems/proc.rst
> > +++ b/Documentation/filesystems/proc.rst
> > @@ -686,7 +686,8 @@ files are there, and which are missing.
> >   apm          Advanced power management info
> >   buddyinfo    Kernel memory allocator information (see text)	(2.5)
> >   bus          Directory containing bus specific information
> > - cmdline      Kernel command line
> > + cmdline      Kernel command line, both from bootloader and embedded
> > + 	      in the kernel image.
> >   cpuinfo      Info about the CPU
> >   devices      Available devices (block and character)
> >   dma          Used DMS channels
> 
> -- 
> ~Randy

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Alexey Dobriyan]

On Thu, Jul 27, 2023 at 08:37:00PM -0700, Paul E. McKenney wrote:
> In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> show all kernel boot parameters, both those supplied by the boot loader
> and those embedded in the kernel image.  This works well for those who
> just want to see all of the kernel boot parameters, but is not helpful to
> those who need to see only those parameters supplied by the boot loader.
> This is especially important when these parameters are presented to the
> boot loader by automation that might gather them from diverse sources.
> 
> Therefore, provide a /proc/cmdline_load file that shows only those kernel
> boot parameters supplied by the boot loader.

> +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> +{
> +	seq_puts(m, boot_command_line);
> +	seq_putc(m, '\n');
> +	return 0;
> +}
> +
>  static int __init proc_cmdline_init(void)
>  {
>  	struct proc_dir_entry *pde;
> @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
>  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
>  	pde_make_permanent(pde);
>  	pde->size = saved_command_line_len + 1;
> +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> +		pde_make_permanent(pde);
> +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> +	}

Please add it as separate fs/proc/cmdline_load.c file so that name of
the file matches name of the /proc file.

The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader"
somewhere should improve things.

Re: [PATCH RFC bootconfig] 2/2] fs/proc: Add /proc/cmdline_image for embedded arguments

[Alexey Dobriyan]

On Thu, Jul 27, 2023 at 08:37:01PM -0700, Paul E. McKenney wrote:
> In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will show
> all kernel boot parameters, both those supplied by the boot loader and
> those embedded in the kernel image.  This works well for those who just
> want to see all of the kernel boot parameters, but is not helpful to those
> who need to see only those parameters that were embedded into the kernel
> image.  This is especially important in situations where there are many
> kernel images for different kernel versions and kernel configurations,
> all of which opens the door to a great deal of human error.
> 
> Therefore, provide a /proc/cmdline_image file that shows only those kernel
> boot parameters that were embedded in the kernel image.  The output
> is in boot-image format, which allows easy reconcilation against the
> boot-config source file.
> 
> Why put this in /proc?  Because it is quite similar to /proc/cmdline, so
> it makes sense to put it in the same place that /proc/cmdline is located.
> 
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: <linux-fsdevel@vger.kernel.org>
> 
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> ---
>  fs/proc/cmdline.c    | 12 ++++++++++++
>  include/linux/init.h | 11 ++++++-----
>  init/main.c          |  9 +++++++++

Same thing,

Please if possible put /proc/x into fs/proc/x.c so that it is easier to
find source. Not all /proc follows this convention but still.

I don't like this name too (but less than the other one).
Is it Boot Image Format (BIF). If yes, maybe add it as /proc/cmdline.bif ?

I don't know what's the good name.

Re: [PATCH RFC bootconfig] 2/2] fs/proc: Add /proc/cmdline_image for embedded arguments

[Paul E. McKenney]

On Fri, Aug 04, 2023 at 08:28:08PM +0300, Alexey Dobriyan wrote:
> On Thu, Jul 27, 2023 at 08:37:01PM -0700, Paul E. McKenney wrote:
> > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will show
> > all kernel boot parameters, both those supplied by the boot loader and
> > those embedded in the kernel image.  This works well for those who just
> > want to see all of the kernel boot parameters, but is not helpful to those
> > who need to see only those parameters that were embedded into the kernel
> > image.  This is especially important in situations where there are many
> > kernel images for different kernel versions and kernel configurations,
> > all of which opens the door to a great deal of human error.
> > 
> > Therefore, provide a /proc/cmdline_image file that shows only those kernel
> > boot parameters that were embedded in the kernel image.  The output
> > is in boot-image format, which allows easy reconcilation against the
> > boot-config source file.
> > 
> > Why put this in /proc?  Because it is quite similar to /proc/cmdline, so
> > it makes sense to put it in the same place that /proc/cmdline is located.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Alexey Dobriyan <adobriyan@gmail.com>
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Cc: <linux-fsdevel@vger.kernel.org>
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > ---
> >  fs/proc/cmdline.c    | 12 ++++++++++++
> >  include/linux/init.h | 11 ++++++-----
> >  init/main.c          |  9 +++++++++
> 
> Same thing,
> 
> Please if possible put /proc/x into fs/proc/x.c so that it is easier to
> find source. Not all /proc follows this convention but still.
> 
> I don't like this name too (but less than the other one).
> Is it Boot Image Format (BIF). If yes, maybe add it as /proc/cmdline.bif ?
> 
> I don't know what's the good name.

It turns out that the already existing /proc/bootconfig makes this
new /proc/bootconfig_image unnecessary, so I have dropped this patch.
Imagine my embarrassment upon learning that /proc/bootconfig has been
around since v5.5!  ;-)

							Thanx, Paul

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Paul E. McKenney]

On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote:
> On Thu, Jul 27, 2023 at 08:37:00PM -0700, Paul E. McKenney wrote:
> > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > show all kernel boot parameters, both those supplied by the boot loader
> > and those embedded in the kernel image.  This works well for those who
> > just want to see all of the kernel boot parameters, but is not helpful to
> > those who need to see only those parameters supplied by the boot loader.
> > This is especially important when these parameters are presented to the
> > boot loader by automation that might gather them from diverse sources.
> > 
> > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > boot parameters supplied by the boot loader.
> 
> > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > +{
> > +	seq_puts(m, boot_command_line);
> > +	seq_putc(m, '\n');
> > +	return 0;
> > +}
> > +
> >  static int __init proc_cmdline_init(void)
> >  {
> >  	struct proc_dir_entry *pde;
> > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> >  	pde_make_permanent(pde);
> >  	pde->size = saved_command_line_len + 1;
> > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > +		pde_make_permanent(pde);
> > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > +	}
> 
> Please add it as separate fs/proc/cmdline_load.c file so that name of
> the file matches name of the /proc file.

Thank you, will do!

> The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader"
> somewhere should improve things.

If we can all quickly come to agreement on a name, I can of course easily
change it.

/proc/cmdline_bootloader?  Better than /proc/cmdline_from_bootloader,
I suppose.  /proc/cmdline_bootldr?  /proc/bootloader by analogy with
/proc/bootconfig?  Something else?

							Thanx, Paul

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Paul E. McKenney]

On Fri, Aug 04, 2023 at 10:36:17AM -0700, Paul E. McKenney wrote:
> On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote:
> > On Thu, Jul 27, 2023 at 08:37:00PM -0700, Paul E. McKenney wrote:
> > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > show all kernel boot parameters, both those supplied by the boot loader
> > > and those embedded in the kernel image.  This works well for those who
> > > just want to see all of the kernel boot parameters, but is not helpful to
> > > those who need to see only those parameters supplied by the boot loader.
> > > This is especially important when these parameters are presented to the
> > > boot loader by automation that might gather them from diverse sources.
> > > 
> > > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > > boot parameters supplied by the boot loader.
> > 
> > > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > > +{
> > > +	seq_puts(m, boot_command_line);
> > > +	seq_putc(m, '\n');
> > > +	return 0;
> > > +}
> > > +
> > >  static int __init proc_cmdline_init(void)
> > >  {
> > >  	struct proc_dir_entry *pde;
> > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> > >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> > >  	pde_make_permanent(pde);
> > >  	pde->size = saved_command_line_len + 1;
> > > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > > +		pde_make_permanent(pde);
> > > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > +	}
> > 
> > Please add it as separate fs/proc/cmdline_load.c file so that name of
> > the file matches name of the /proc file.
> 
> Thank you, will do!

And here is an untested sneak preview, which I will fold into the original
after testing.

Thoughts?

							Thanx, Paul

------------------------------------------------------------------------

diff --git a/fs/proc/Makefile b/fs/proc/Makefile
index bd08616ed8ba..094f3102eb9f 100644
--- a/fs/proc/Makefile
+++ b/fs/proc/Makefile
@@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE)	+= vmcore.o
 proc-$(CONFIG_PRINTK)	+= kmsg.o
 proc-$(CONFIG_PROC_PAGE_MONITOR)	+= page.o
 proc-$(CONFIG_BOOT_CONFIG)	+= bootconfig.o
+proc-$(CONFIG_BOOT_CONFIG_FORCE)	+= cmdline_load.o
diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c
index 1d0ef9d2949d..082def2c1cc6 100644
--- a/fs/proc/cmdline.c
+++ b/fs/proc/cmdline.c
@@ -13,13 +13,6 @@ static int cmdline_proc_show(struct seq_file *m, void *v)
 	return 0;
 }
 
-static int cmdline_load_proc_show(struct seq_file *m, void *v)
-{
-	seq_puts(m, boot_command_line);
-	seq_putc(m, '\n');
-	return 0;
-}
-
 static int __init proc_cmdline_init(void)
 {
 	struct proc_dir_entry *pde;
@@ -27,11 +20,6 @@ static int __init proc_cmdline_init(void)
 	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
 	pde_make_permanent(pde);
 	pde->size = saved_command_line_len + 1;
-	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
-		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
-		pde_make_permanent(pde);
-		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
-	}
 	return 0;
 }
 fs_initcall(proc_cmdline_init);
diff --git a/fs/proc/cmdline_load.c b/fs/proc/cmdline_load.c
new file mode 100644
index 000000000000..e3dccb3441ce
--- /dev/null
+++ b/fs/proc/cmdline_load.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+#include <asm/setup.h>
+#include "internal.h"
+
+static int cmdline_load_proc_show(struct seq_file *m, void *v)
+{
+	seq_puts(m, boot_command_line);
+	seq_putc(m, '\n');
+	return 0;
+}
+
+static int __init proc_cmdline_load_init(void)
+{
+	struct proc_dir_entry *pde;
+
+	pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
+	pde_make_permanent(pde);
+	pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
+	return 0;
+}
+fs_initcall(proc_cmdline_load_init);

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Masami Hiramatsu]

On Fri, 4 Aug 2023 10:36:17 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote:
> > On Thu, Jul 27, 2023 at 08:37:00PM -0700, Paul E. McKenney wrote:
> > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > show all kernel boot parameters, both those supplied by the boot loader
> > > and those embedded in the kernel image.  This works well for those who
> > > just want to see all of the kernel boot parameters, but is not helpful to
> > > those who need to see only those parameters supplied by the boot loader.
> > > This is especially important when these parameters are presented to the
> > > boot loader by automation that might gather them from diverse sources.
> > > 
> > > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > > boot parameters supplied by the boot loader.
> > 
> > > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > > +{
> > > +	seq_puts(m, boot_command_line);
> > > +	seq_putc(m, '\n');
> > > +	return 0;
> > > +}
> > > +
> > >  static int __init proc_cmdline_init(void)
> > >  {
> > >  	struct proc_dir_entry *pde;
> > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> > >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> > >  	pde_make_permanent(pde);
> > >  	pde->size = saved_command_line_len + 1;
> > > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > > +		pde_make_permanent(pde);
> > > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > +	}
> > 
> > Please add it as separate fs/proc/cmdline_load.c file so that name of
> > the file matches name of the /proc file.
> 
> Thank you, will do!
> 
> > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader"
> > somewhere should improve things.
> 
> If we can all quickly come to agreement on a name, I can of course easily
> change it.
> 
> /proc/cmdline_bootloader?  Better than /proc/cmdline_from_bootloader,
> I suppose.  /proc/cmdline_bootldr?  /proc/bootloader by analogy with
> /proc/bootconfig?  Something else?

What about "/proc/raw_cmdline" ?

Thank you,


> 
> 							Thanx, Paul


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Paul E. McKenney]

On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote:
> On Fri, 4 Aug 2023 10:36:17 -0700
> "Paul E. McKenney" <paulmck@kernel.org> wrote:
> 
> > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote:
> > > On Thu, Jul 27, 2023 at 08:37:00PM -0700, Paul E. McKenney wrote:
> > > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > > show all kernel boot parameters, both those supplied by the boot loader
> > > > and those embedded in the kernel image.  This works well for those who
> > > > just want to see all of the kernel boot parameters, but is not helpful to
> > > > those who need to see only those parameters supplied by the boot loader.
> > > > This is especially important when these parameters are presented to the
> > > > boot loader by automation that might gather them from diverse sources.
> > > > 
> > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > > > boot parameters supplied by the boot loader.
> > > 
> > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > > > +{
> > > > +	seq_puts(m, boot_command_line);
> > > > +	seq_putc(m, '\n');
> > > > +	return 0;
> > > > +}
> > > > +
> > > >  static int __init proc_cmdline_init(void)
> > > >  {
> > > >  	struct proc_dir_entry *pde;
> > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> > > >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> > > >  	pde_make_permanent(pde);
> > > >  	pde->size = saved_command_line_len + 1;
> > > > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > > > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > > > +		pde_make_permanent(pde);
> > > > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > > +	}
> > > 
> > > Please add it as separate fs/proc/cmdline_load.c file so that name of
> > > the file matches name of the /proc file.
> > 
> > Thank you, will do!
> > 
> > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader"
> > > somewhere should improve things.
> > 
> > If we can all quickly come to agreement on a name, I can of course easily
> > change it.
> > 
> > /proc/cmdline_bootloader?  Better than /proc/cmdline_from_bootloader,
> > I suppose.  /proc/cmdline_bootldr?  /proc/bootloader by analogy with
> > /proc/bootconfig?  Something else?
> 
> What about "/proc/raw_cmdline" ?

That would work of me!

Any objections to /proc/raw_cmdline?

Going once...

							Thanx, Paul

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Paul E. McKenney]

On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote:
> On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote:
> > On Fri, 4 Aug 2023 10:36:17 -0700
> > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > 
> > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote:
> > > > On Thu, Jul 27, 2023 at 08:37:00PM -0700, Paul E. McKenney wrote:
> > > > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > > > show all kernel boot parameters, both those supplied by the boot loader
> > > > > and those embedded in the kernel image.  This works well for those who
> > > > > just want to see all of the kernel boot parameters, but is not helpful to
> > > > > those who need to see only those parameters supplied by the boot loader.
> > > > > This is especially important when these parameters are presented to the
> > > > > boot loader by automation that might gather them from diverse sources.
> > > > > 
> > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > > > > boot parameters supplied by the boot loader.
> > > > 
> > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > > > > +{
> > > > > +	seq_puts(m, boot_command_line);
> > > > > +	seq_putc(m, '\n');
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > >  static int __init proc_cmdline_init(void)
> > > > >  {
> > > > >  	struct proc_dir_entry *pde;
> > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> > > > >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> > > > >  	pde_make_permanent(pde);
> > > > >  	pde->size = saved_command_line_len + 1;
> > > > > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > > > > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > > > > +		pde_make_permanent(pde);
> > > > > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > > > +	}
> > > > 
> > > > Please add it as separate fs/proc/cmdline_load.c file so that name of
> > > > the file matches name of the /proc file.
> > > 
> > > Thank you, will do!
> > > 
> > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader"
> > > > somewhere should improve things.
> > > 
> > > If we can all quickly come to agreement on a name, I can of course easily
> > > change it.
> > > 
> > > /proc/cmdline_bootloader?  Better than /proc/cmdline_from_bootloader,
> > > I suppose.  /proc/cmdline_bootldr?  /proc/bootloader by analogy with
> > > /proc/bootconfig?  Something else?
> > 
> > What about "/proc/raw_cmdline" ?
> 
> That would work of me!
> 
> Any objections to /proc/raw_cmdline?
> 
> Going once...

Going twice...

If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT.

 							Thanx, Paul

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Paul E. McKenney]

On Sat, Aug 12, 2023 at 04:30:41PM -0700, Paul E. McKenney wrote:
> On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote:
> > On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote:
> > > On Fri, 4 Aug 2023 10:36:17 -0700
> > > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > 
> > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote:
> > > > > On Thu, Jul 27, 2023 at 08:37:00PM -0700, Paul E. McKenney wrote:
> > > > > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > > > > show all kernel boot parameters, both those supplied by the boot loader
> > > > > > and those embedded in the kernel image.  This works well for those who
> > > > > > just want to see all of the kernel boot parameters, but is not helpful to
> > > > > > those who need to see only those parameters supplied by the boot loader.
> > > > > > This is especially important when these parameters are presented to the
> > > > > > boot loader by automation that might gather them from diverse sources.
> > > > > > 
> > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > > > > > boot parameters supplied by the boot loader.
> > > > > 
> > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > > > > > +{
> > > > > > +	seq_puts(m, boot_command_line);
> > > > > > +	seq_putc(m, '\n');
> > > > > > +	return 0;
> > > > > > +}
> > > > > > +
> > > > > >  static int __init proc_cmdline_init(void)
> > > > > >  {
> > > > > >  	struct proc_dir_entry *pde;
> > > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> > > > > >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> > > > > >  	pde_make_permanent(pde);
> > > > > >  	pde->size = saved_command_line_len + 1;
> > > > > > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > > > > > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > > > > > +		pde_make_permanent(pde);
> > > > > > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > > > > +	}
> > > > > 
> > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of
> > > > > the file matches name of the /proc file.
> > > > 
> > > > Thank you, will do!
> > > > 
> > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader"
> > > > > somewhere should improve things.
> > > > 
> > > > If we can all quickly come to agreement on a name, I can of course easily
> > > > change it.
> > > > 
> > > > /proc/cmdline_bootloader?  Better than /proc/cmdline_from_bootloader,
> > > > I suppose.  /proc/cmdline_bootldr?  /proc/bootloader by analogy with
> > > > /proc/bootconfig?  Something else?
> > > 
> > > What about "/proc/raw_cmdline" ?
> > 
> > That would work of me!
> > 
> > Any objections to /proc/raw_cmdline?
> > 
> > Going once...
> 
> Going twice...
> 
> If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT.

And gone!

Please see below for the updated version.

								Thanx, Paul

------------------------------------------------------------------------

commit 0fe10f0d1873a6f6e287c0c5b45e9203b0e33c83
Author: Paul E. McKenney <paulmck@kernel.org>
Date:   Fri Jul 21 16:05:38 2023 -0700

    fs/proc: Add /proc/raw_cmdline for boot loader arguments
    
    In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
    show all kernel boot parameters, both those supplied by the boot loader
    and those embedded in the kernel image.  This works well for those who
    just want to see all of the kernel boot parameters, but is not helpful to
    those who need to see only those parameters supplied by the boot loader.
    This is especially important when these parameters are presented to the
    boot loader by automation that might gather them from diverse sources.
    It is also useful when booting the next kernel via kexec(), in which
    case it is necessary to supply only those kernel command-line arguments
    from the boot loader, and most definitely not those that were embedded
    into the current kernel.
    
    Therefore, provide a /proc/raw_cmdline file that shows only those kernel
    boot parameters supplied by the boot loader.
    
    Why put this in /proc?  Because it is quite similar to /proc/cmdline,
    and /proc/bootconfig, so it makes sense to put it in the same place that
    those files are located.
    
    [ sfr: Apply kernel test robot feedback. ]
    [ paulmck: Apply Randy Dunlap feedback. ]
    [ paulmck: Apply naming feedback from Alexey Dobriyan and Masami Hiramatsu. ]
    
    Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au>
    Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
    Co-developed-by: Arnd Bergmann <arnd@kernel.org>
    Signed-off-by: Arnd Bergmann <arnd@kernel.org>
    Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
    Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
    Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
    Cc: Andrew Morton <akpm@linux-foundation.org>
    Cc: Alexey Dobriyan <adobriyan@gmail.com>
    Cc: <linux-fsdevel@vger.kernel.org>

diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 75a8c899ebcc..61419270c38f 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -723,6 +723,7 @@ files are there, and which are missing.
  partitions   Table of partitions known to the system
  pci 	      Deprecated info of PCI bus (new way -> /proc/bus/pci/,
               decoupled by lspci				(2.4)
+ raw_cmdline  Kernel command line obtained from kernel image	(6.6)
  rtc          Real time clock
  scsi         SCSI info (see text)
  slabinfo     Slab pool info
diff --git a/fs/proc/Makefile b/fs/proc/Makefile
index bd08616ed8ba..6182296f3c6b 100644
--- a/fs/proc/Makefile
+++ b/fs/proc/Makefile
@@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE)	+= vmcore.o
 proc-$(CONFIG_PRINTK)	+= kmsg.o
 proc-$(CONFIG_PROC_PAGE_MONITOR)	+= page.o
 proc-$(CONFIG_BOOT_CONFIG)	+= bootconfig.o
+proc-$(CONFIG_BOOT_CONFIG_FORCE)	+= raw_cmdline.o
diff --git a/fs/proc/raw_cmdline.c b/fs/proc/raw_cmdline.c
new file mode 100644
index 000000000000..2e19eb89fc8e
--- /dev/null
+++ b/fs/proc/raw_cmdline.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+#include <asm/setup.h>
+#include "internal.h"
+
+static int raw_cmdline_proc_show(struct seq_file *m, void *v)
+{
+	seq_puts(m, boot_command_line);
+	seq_putc(m, '\n');
+	return 0;
+}
+
+static int __init proc_raw_cmdline_init(void)
+{
+	struct proc_dir_entry *pde;
+
+	pde = proc_create_single("raw_cmdline", 0, NULL, raw_cmdline_proc_show);
+	pde_make_permanent(pde);
+	pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
+	return 0;
+}
+fs_initcall(proc_raw_cmdline_init);
diff --git a/include/linux/init.h b/include/linux/init.h
index 266c3e1640d4..29e75bbe7984 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -112,6 +112,7 @@
 #define __REFCONST       .section       ".ref.rodata", "a"
 
 #ifndef __ASSEMBLY__
+
 /*
  * Used for initialization calls..
  */
@@ -143,7 +144,7 @@ struct file_system_type;
 
 /* Defined in init/main.c */
 extern int do_one_initcall(initcall_t fn);
-extern char __initdata boot_command_line[];
+extern char boot_command_line[];
 extern char *saved_command_line;
 extern unsigned int saved_command_line_len;
 extern unsigned int reset_devices;
diff --git a/init/main.c b/init/main.c
index ad920fac325c..2121685c479a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
 void (*__initdata late_time_init)(void);
 
 /* Untouched command line saved by arch-specific code. */
-char __initdata boot_command_line[COMMAND_LINE_SIZE];
+char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
 /* Untouched saved command line (eg. for /proc) */
 char *saved_command_line __ro_after_init;
 unsigned int saved_command_line_len __ro_after_init;

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Masami Hiramatsu]

On Mon, 14 Aug 2023 16:08:29 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> On Sat, Aug 12, 2023 at 04:30:41PM -0700, Paul E. McKenney wrote:
> > On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote:
> > > On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote:
> > > > On Fri, 4 Aug 2023 10:36:17 -0700
> > > > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > > 
> > > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote:
> > > > > > On Thu, Jul 27, 2023 at 08:37:00PM -0700, Paul E. McKenney wrote:
> > > > > > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > > > > > show all kernel boot parameters, both those supplied by the boot loader
> > > > > > > and those embedded in the kernel image.  This works well for those who
> > > > > > > just want to see all of the kernel boot parameters, but is not helpful to
> > > > > > > those who need to see only those parameters supplied by the boot loader.
> > > > > > > This is especially important when these parameters are presented to the
> > > > > > > boot loader by automation that might gather them from diverse sources.
> > > > > > > 
> > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > > > > > > boot parameters supplied by the boot loader.
> > > > > > 
> > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > > > > > > +{
> > > > > > > +	seq_puts(m, boot_command_line);
> > > > > > > +	seq_putc(m, '\n');
> > > > > > > +	return 0;
> > > > > > > +}
> > > > > > > +
> > > > > > >  static int __init proc_cmdline_init(void)
> > > > > > >  {
> > > > > > >  	struct proc_dir_entry *pde;
> > > > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> > > > > > >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> > > > > > >  	pde_make_permanent(pde);
> > > > > > >  	pde->size = saved_command_line_len + 1;
> > > > > > > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > > > > > > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > > > > > > +		pde_make_permanent(pde);
> > > > > > > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > > > > > +	}
> > > > > > 
> > > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of
> > > > > > the file matches name of the /proc file.
> > > > > 
> > > > > Thank you, will do!
> > > > > 
> > > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader"
> > > > > > somewhere should improve things.
> > > > > 
> > > > > If we can all quickly come to agreement on a name, I can of course easily
> > > > > change it.
> > > > > 
> > > > > /proc/cmdline_bootloader?  Better than /proc/cmdline_from_bootloader,
> > > > > I suppose.  /proc/cmdline_bootldr?  /proc/bootloader by analogy with
> > > > > /proc/bootconfig?  Something else?
> > > > 
> > > > What about "/proc/raw_cmdline" ?
> > > 
> > > That would work of me!
> > > 
> > > Any objections to /proc/raw_cmdline?
> > > 
> > > Going once...
> > 
> > Going twice...
> > 
> > If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT.
> 
> And gone!
> 
> Please see below for the updated version.

OK, I'll pick this.

Thanks!

> 
> 								Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> commit 0fe10f0d1873a6f6e287c0c5b45e9203b0e33c83
> Author: Paul E. McKenney <paulmck@kernel.org>
> Date:   Fri Jul 21 16:05:38 2023 -0700
> 
>     fs/proc: Add /proc/raw_cmdline for boot loader arguments
>     
>     In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
>     show all kernel boot parameters, both those supplied by the boot loader
>     and those embedded in the kernel image.  This works well for those who
>     just want to see all of the kernel boot parameters, but is not helpful to
>     those who need to see only those parameters supplied by the boot loader.
>     This is especially important when these parameters are presented to the
>     boot loader by automation that might gather them from diverse sources.
>     It is also useful when booting the next kernel via kexec(), in which
>     case it is necessary to supply only those kernel command-line arguments
>     from the boot loader, and most definitely not those that were embedded
>     into the current kernel.
>     
>     Therefore, provide a /proc/raw_cmdline file that shows only those kernel
>     boot parameters supplied by the boot loader.
>     
>     Why put this in /proc?  Because it is quite similar to /proc/cmdline,
>     and /proc/bootconfig, so it makes sense to put it in the same place that
>     those files are located.
>     
>     [ sfr: Apply kernel test robot feedback. ]
>     [ paulmck: Apply Randy Dunlap feedback. ]
>     [ paulmck: Apply naming feedback from Alexey Dobriyan and Masami Hiramatsu. ]
>     
>     Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au>
>     Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
>     Co-developed-by: Arnd Bergmann <arnd@kernel.org>
>     Signed-off-by: Arnd Bergmann <arnd@kernel.org>
>     Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
>     Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>     Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>     Cc: Andrew Morton <akpm@linux-foundation.org>
>     Cc: Alexey Dobriyan <adobriyan@gmail.com>
>     Cc: <linux-fsdevel@vger.kernel.org>
> 
> diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> index 75a8c899ebcc..61419270c38f 100644
> --- a/Documentation/filesystems/proc.rst
> +++ b/Documentation/filesystems/proc.rst
> @@ -723,6 +723,7 @@ files are there, and which are missing.
>   partitions   Table of partitions known to the system
>   pci 	      Deprecated info of PCI bus (new way -> /proc/bus/pci/,
>                decoupled by lspci				(2.4)
> + raw_cmdline  Kernel command line obtained from kernel image	(6.6)
>   rtc          Real time clock
>   scsi         SCSI info (see text)
>   slabinfo     Slab pool info
> diff --git a/fs/proc/Makefile b/fs/proc/Makefile
> index bd08616ed8ba..6182296f3c6b 100644
> --- a/fs/proc/Makefile
> +++ b/fs/proc/Makefile
> @@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE)	+= vmcore.o
>  proc-$(CONFIG_PRINTK)	+= kmsg.o
>  proc-$(CONFIG_PROC_PAGE_MONITOR)	+= page.o
>  proc-$(CONFIG_BOOT_CONFIG)	+= bootconfig.o
> +proc-$(CONFIG_BOOT_CONFIG_FORCE)	+= raw_cmdline.o
> diff --git a/fs/proc/raw_cmdline.c b/fs/proc/raw_cmdline.c
> new file mode 100644
> index 000000000000..2e19eb89fc8e
> --- /dev/null
> +++ b/fs/proc/raw_cmdline.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/proc_fs.h>
> +#include <linux/seq_file.h>
> +#include <asm/setup.h>
> +#include "internal.h"
> +
> +static int raw_cmdline_proc_show(struct seq_file *m, void *v)
> +{
> +	seq_puts(m, boot_command_line);
> +	seq_putc(m, '\n');
> +	return 0;
> +}
> +
> +static int __init proc_raw_cmdline_init(void)
> +{
> +	struct proc_dir_entry *pde;
> +
> +	pde = proc_create_single("raw_cmdline", 0, NULL, raw_cmdline_proc_show);
> +	pde_make_permanent(pde);
> +	pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> +	return 0;
> +}
> +fs_initcall(proc_raw_cmdline_init);
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 266c3e1640d4..29e75bbe7984 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -112,6 +112,7 @@
>  #define __REFCONST       .section       ".ref.rodata", "a"
>  
>  #ifndef __ASSEMBLY__
> +
>  /*
>   * Used for initialization calls..
>   */
> @@ -143,7 +144,7 @@ struct file_system_type;
>  
>  /* Defined in init/main.c */
>  extern int do_one_initcall(initcall_t fn);
> -extern char __initdata boot_command_line[];
> +extern char boot_command_line[];
>  extern char *saved_command_line;
>  extern unsigned int saved_command_line_len;
>  extern unsigned int reset_devices;
> diff --git a/init/main.c b/init/main.c
> index ad920fac325c..2121685c479a 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
>  void (*__initdata late_time_init)(void);
>  
>  /* Untouched command line saved by arch-specific code. */
> -char __initdata boot_command_line[COMMAND_LINE_SIZE];
> +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
>  /* Untouched saved command line (eg. for /proc) */
>  char *saved_command_line __ro_after_init;
>  unsigned int saved_command_line_len __ro_after_init;


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Masami Hiramatsu]

On Wed, 16 Aug 2023 18:40:03 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> On Mon, 14 Aug 2023 16:08:29 -0700
> "Paul E. McKenney" <paulmck@kernel.org> wrote:
> 
> > On Sat, Aug 12, 2023 at 04:30:41PM -0700, Paul E. McKenney wrote:
> > > On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote:
> > > > On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote:
> > > > > On Fri, 4 Aug 2023 10:36:17 -0700
> > > > > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > > > 
> > > > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote:
> > > > > > > On Thu, Jul 27, 2023 at 08:37:00PM -0700, Paul E. McKenney wrote:
> > > > > > > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > > > > > > show all kernel boot parameters, both those supplied by the boot loader
> > > > > > > > and those embedded in the kernel image.  This works well for those who
> > > > > > > > just want to see all of the kernel boot parameters, but is not helpful to
> > > > > > > > those who need to see only those parameters supplied by the boot loader.
> > > > > > > > This is especially important when these parameters are presented to the
> > > > > > > > boot loader by automation that might gather them from diverse sources.
> > > > > > > > 
> > > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > > > > > > > boot parameters supplied by the boot loader.
> > > > > > > 
> > > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > > > > > > > +{
> > > > > > > > +	seq_puts(m, boot_command_line);
> > > > > > > > +	seq_putc(m, '\n');
> > > > > > > > +	return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > >  static int __init proc_cmdline_init(void)
> > > > > > > >  {
> > > > > > > >  	struct proc_dir_entry *pde;
> > > > > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> > > > > > > >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> > > > > > > >  	pde_make_permanent(pde);
> > > > > > > >  	pde->size = saved_command_line_len + 1;
> > > > > > > > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > > > > > > > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > > > > > > > +		pde_make_permanent(pde);
> > > > > > > > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > > > > > > +	}
> > > > > > > 
> > > > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of
> > > > > > > the file matches name of the /proc file.
> > > > > > 
> > > > > > Thank you, will do!
> > > > > > 
> > > > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader"
> > > > > > > somewhere should improve things.
> > > > > > 
> > > > > > If we can all quickly come to agreement on a name, I can of course easily
> > > > > > change it.
> > > > > > 
> > > > > > /proc/cmdline_bootloader?  Better than /proc/cmdline_from_bootloader,
> > > > > > I suppose.  /proc/cmdline_bootldr?  /proc/bootloader by analogy with
> > > > > > /proc/bootconfig?  Something else?
> > > > > 
> > > > > What about "/proc/raw_cmdline" ?
> > > > 
> > > > That would work of me!
> > > > 
> > > > Any objections to /proc/raw_cmdline?
> > > > 
> > > > Going once...
> > > 
> > > Going twice...
> > > 
> > > If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT.
> > 
> > And gone!
> > 
> > Please see below for the updated version.
> 
> OK, I'll pick this.

Wait, is it OK to push this through bootconfig tree? Since this is not directly
related to the bootconfig, fsdevel maintainer can pick this.
I would like to ping to fsdevel people at first.

Thank you,

> 
> Thanks!
> 
> > 
> > 								Thanx, Paul
> > 
> > ------------------------------------------------------------------------
> > 
> > commit 0fe10f0d1873a6f6e287c0c5b45e9203b0e33c83
> > Author: Paul E. McKenney <paulmck@kernel.org>
> > Date:   Fri Jul 21 16:05:38 2023 -0700
> > 
> >     fs/proc: Add /proc/raw_cmdline for boot loader arguments
> >     
> >     In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> >     show all kernel boot parameters, both those supplied by the boot loader
> >     and those embedded in the kernel image.  This works well for those who
> >     just want to see all of the kernel boot parameters, but is not helpful to
> >     those who need to see only those parameters supplied by the boot loader.
> >     This is especially important when these parameters are presented to the
> >     boot loader by automation that might gather them from diverse sources.
> >     It is also useful when booting the next kernel via kexec(), in which
> >     case it is necessary to supply only those kernel command-line arguments
> >     from the boot loader, and most definitely not those that were embedded
> >     into the current kernel.
> >     
> >     Therefore, provide a /proc/raw_cmdline file that shows only those kernel
> >     boot parameters supplied by the boot loader.
> >     
> >     Why put this in /proc?  Because it is quite similar to /proc/cmdline,
> >     and /proc/bootconfig, so it makes sense to put it in the same place that
> >     those files are located.
> >     
> >     [ sfr: Apply kernel test robot feedback. ]
> >     [ paulmck: Apply Randy Dunlap feedback. ]
> >     [ paulmck: Apply naming feedback from Alexey Dobriyan and Masami Hiramatsu. ]
> >     
> >     Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au>
> >     Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> >     Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> >     Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> >     Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> >     Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> >     Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> >     Cc: Andrew Morton <akpm@linux-foundation.org>
> >     Cc: Alexey Dobriyan <adobriyan@gmail.com>
> >     Cc: <linux-fsdevel@vger.kernel.org>
> > 
> > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> > index 75a8c899ebcc..61419270c38f 100644
> > --- a/Documentation/filesystems/proc.rst
> > +++ b/Documentation/filesystems/proc.rst
> > @@ -723,6 +723,7 @@ files are there, and which are missing.
> >   partitions   Table of partitions known to the system
> >   pci 	      Deprecated info of PCI bus (new way -> /proc/bus/pci/,
> >                decoupled by lspci				(2.4)
> > + raw_cmdline  Kernel command line obtained from kernel image	(6.6)
> >   rtc          Real time clock
> >   scsi         SCSI info (see text)
> >   slabinfo     Slab pool info
> > diff --git a/fs/proc/Makefile b/fs/proc/Makefile
> > index bd08616ed8ba..6182296f3c6b 100644
> > --- a/fs/proc/Makefile
> > +++ b/fs/proc/Makefile
> > @@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE)	+= vmcore.o
> >  proc-$(CONFIG_PRINTK)	+= kmsg.o
> >  proc-$(CONFIG_PROC_PAGE_MONITOR)	+= page.o
> >  proc-$(CONFIG_BOOT_CONFIG)	+= bootconfig.o
> > +proc-$(CONFIG_BOOT_CONFIG_FORCE)	+= raw_cmdline.o
> > diff --git a/fs/proc/raw_cmdline.c b/fs/proc/raw_cmdline.c
> > new file mode 100644
> > index 000000000000..2e19eb89fc8e
> > --- /dev/null
> > +++ b/fs/proc/raw_cmdline.c
> > @@ -0,0 +1,25 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <linux/fs.h>
> > +#include <linux/init.h>
> > +#include <linux/proc_fs.h>
> > +#include <linux/seq_file.h>
> > +#include <asm/setup.h>
> > +#include "internal.h"
> > +
> > +static int raw_cmdline_proc_show(struct seq_file *m, void *v)
> > +{
> > +	seq_puts(m, boot_command_line);
> > +	seq_putc(m, '\n');
> > +	return 0;
> > +}
> > +
> > +static int __init proc_raw_cmdline_init(void)
> > +{
> > +	struct proc_dir_entry *pde;
> > +
> > +	pde = proc_create_single("raw_cmdline", 0, NULL, raw_cmdline_proc_show);
> > +	pde_make_permanent(pde);
> > +	pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > +	return 0;
> > +}
> > +fs_initcall(proc_raw_cmdline_init);
> > diff --git a/include/linux/init.h b/include/linux/init.h
> > index 266c3e1640d4..29e75bbe7984 100644
> > --- a/include/linux/init.h
> > +++ b/include/linux/init.h
> > @@ -112,6 +112,7 @@
> >  #define __REFCONST       .section       ".ref.rodata", "a"
> >  
> >  #ifndef __ASSEMBLY__
> > +
> >  /*
> >   * Used for initialization calls..
> >   */
> > @@ -143,7 +144,7 @@ struct file_system_type;
> >  
> >  /* Defined in init/main.c */
> >  extern int do_one_initcall(initcall_t fn);
> > -extern char __initdata boot_command_line[];
> > +extern char boot_command_line[];
> >  extern char *saved_command_line;
> >  extern unsigned int saved_command_line_len;
> >  extern unsigned int reset_devices;
> > diff --git a/init/main.c b/init/main.c
> > index ad920fac325c..2121685c479a 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
> >  void (*__initdata late_time_init)(void);
> >  
> >  /* Untouched command line saved by arch-specific code. */
> > -char __initdata boot_command_line[COMMAND_LINE_SIZE];
> > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
> >  /* Untouched saved command line (eg. for /proc) */
> >  char *saved_command_line __ro_after_init;
> >  unsigned int saved_command_line_len __ro_after_init;
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Paul E. McKenney]

On Thu, Aug 17, 2023 at 12:17:31AM +0900, Masami Hiramatsu wrote:
> On Wed, 16 Aug 2023 18:40:03 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > On Mon, 14 Aug 2023 16:08:29 -0700
> > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > On Sat, Aug 12, 2023 at 04:30:41PM -0700, Paul E. McKenney wrote:
> > > > On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote:
> > > > > On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote:
> > > > > > On Fri, 4 Aug 2023 10:36:17 -0700
> > > > > > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > > > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote:
> > > > > > > > On Thu, Jul 27, 2023 at 08:37:00PM -0700, Paul E. McKenney wrote:
> > > > > > > > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > > > > > > > show all kernel boot parameters, both those supplied by the boot loader
> > > > > > > > > and those embedded in the kernel image.  This works well for those who
> > > > > > > > > just want to see all of the kernel boot parameters, but is not helpful to
> > > > > > > > > those who need to see only those parameters supplied by the boot loader.
> > > > > > > > > This is especially important when these parameters are presented to the
> > > > > > > > > boot loader by automation that might gather them from diverse sources.
> > > > > > > > > 
> > > > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > > > > > > > > boot parameters supplied by the boot loader.
> > > > > > > > 
> > > > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > > > > > > > > +{
> > > > > > > > > +	seq_puts(m, boot_command_line);
> > > > > > > > > +	seq_putc(m, '\n');
> > > > > > > > > +	return 0;
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > >  static int __init proc_cmdline_init(void)
> > > > > > > > >  {
> > > > > > > > >  	struct proc_dir_entry *pde;
> > > > > > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> > > > > > > > >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> > > > > > > > >  	pde_make_permanent(pde);
> > > > > > > > >  	pde->size = saved_command_line_len + 1;
> > > > > > > > > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > > > > > > > > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > > > > > > > > +		pde_make_permanent(pde);
> > > > > > > > > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > > > > > > > +	}
> > > > > > > > 
> > > > > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of
> > > > > > > > the file matches name of the /proc file.
> > > > > > > 
> > > > > > > Thank you, will do!
> > > > > > > 
> > > > > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader"
> > > > > > > > somewhere should improve things.
> > > > > > > 
> > > > > > > If we can all quickly come to agreement on a name, I can of course easily
> > > > > > > change it.
> > > > > > > 
> > > > > > > /proc/cmdline_bootloader?  Better than /proc/cmdline_from_bootloader,
> > > > > > > I suppose.  /proc/cmdline_bootldr?  /proc/bootloader by analogy with
> > > > > > > /proc/bootconfig?  Something else?
> > > > > > 
> > > > > > What about "/proc/raw_cmdline" ?
> > > > > 
> > > > > That would work of me!
> > > > > 
> > > > > Any objections to /proc/raw_cmdline?
> > > > > 
> > > > > Going once...
> > > > 
> > > > Going twice...
> > > > 
> > > > If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT.
> > > 
> > > And gone!
> > > 
> > > Please see below for the updated version.
> > 
> > OK, I'll pick this.
> 
> Wait, is it OK to push this through bootconfig tree? Since this is not directly
> related to the bootconfig, fsdevel maintainer can pick this.
> I would like to ping to fsdevel people at first.

Whichever path works best works for me!

Here are the three commits I have queued, just please let me know when any
of them are pulled into some other tree and I will drop them from -rcu.

97edd1291847 ("doc: Update /proc/cmdline documentation to include boot config")
0fe10f0d1873 ("fs/proc: Add /proc/raw_cmdline for boot loader arguments")
9192c6ac7516 ("doc: Add /proc/bootconfig to proc.rst")

							Thanx, Paul

> Thank you,
> 
> > 
> > Thanks!
> > 
> > > 
> > > 								Thanx, Paul
> > > 
> > > ------------------------------------------------------------------------
> > > 
> > > commit 0fe10f0d1873a6f6e287c0c5b45e9203b0e33c83
> > > Author: Paul E. McKenney <paulmck@kernel.org>
> > > Date:   Fri Jul 21 16:05:38 2023 -0700
> > > 
> > >     fs/proc: Add /proc/raw_cmdline for boot loader arguments
> > >     
> > >     In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > >     show all kernel boot parameters, both those supplied by the boot loader
> > >     and those embedded in the kernel image.  This works well for those who
> > >     just want to see all of the kernel boot parameters, but is not helpful to
> > >     those who need to see only those parameters supplied by the boot loader.
> > >     This is especially important when these parameters are presented to the
> > >     boot loader by automation that might gather them from diverse sources.
> > >     It is also useful when booting the next kernel via kexec(), in which
> > >     case it is necessary to supply only those kernel command-line arguments
> > >     from the boot loader, and most definitely not those that were embedded
> > >     into the current kernel.
> > >     
> > >     Therefore, provide a /proc/raw_cmdline file that shows only those kernel
> > >     boot parameters supplied by the boot loader.
> > >     
> > >     Why put this in /proc?  Because it is quite similar to /proc/cmdline,
> > >     and /proc/bootconfig, so it makes sense to put it in the same place that
> > >     those files are located.
> > >     
> > >     [ sfr: Apply kernel test robot feedback. ]
> > >     [ paulmck: Apply Randy Dunlap feedback. ]
> > >     [ paulmck: Apply naming feedback from Alexey Dobriyan and Masami Hiramatsu. ]
> > >     
> > >     Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > >     Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > >     Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > >     Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> > >     Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > >     Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > >     Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > >     Cc: Andrew Morton <akpm@linux-foundation.org>
> > >     Cc: Alexey Dobriyan <adobriyan@gmail.com>
> > >     Cc: <linux-fsdevel@vger.kernel.org>
> > > 
> > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> > > index 75a8c899ebcc..61419270c38f 100644
> > > --- a/Documentation/filesystems/proc.rst
> > > +++ b/Documentation/filesystems/proc.rst
> > > @@ -723,6 +723,7 @@ files are there, and which are missing.
> > >   partitions   Table of partitions known to the system
> > >   pci 	      Deprecated info of PCI bus (new way -> /proc/bus/pci/,
> > >                decoupled by lspci				(2.4)
> > > + raw_cmdline  Kernel command line obtained from kernel image	(6.6)
> > >   rtc          Real time clock
> > >   scsi         SCSI info (see text)
> > >   slabinfo     Slab pool info
> > > diff --git a/fs/proc/Makefile b/fs/proc/Makefile
> > > index bd08616ed8ba..6182296f3c6b 100644
> > > --- a/fs/proc/Makefile
> > > +++ b/fs/proc/Makefile
> > > @@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE)	+= vmcore.o
> > >  proc-$(CONFIG_PRINTK)	+= kmsg.o
> > >  proc-$(CONFIG_PROC_PAGE_MONITOR)	+= page.o
> > >  proc-$(CONFIG_BOOT_CONFIG)	+= bootconfig.o
> > > +proc-$(CONFIG_BOOT_CONFIG_FORCE)	+= raw_cmdline.o
> > > diff --git a/fs/proc/raw_cmdline.c b/fs/proc/raw_cmdline.c
> > > new file mode 100644
> > > index 000000000000..2e19eb89fc8e
> > > --- /dev/null
> > > +++ b/fs/proc/raw_cmdline.c
> > > @@ -0,0 +1,25 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +#include <linux/fs.h>
> > > +#include <linux/init.h>
> > > +#include <linux/proc_fs.h>
> > > +#include <linux/seq_file.h>
> > > +#include <asm/setup.h>
> > > +#include "internal.h"
> > > +
> > > +static int raw_cmdline_proc_show(struct seq_file *m, void *v)
> > > +{
> > > +	seq_puts(m, boot_command_line);
> > > +	seq_putc(m, '\n');
> > > +	return 0;
> > > +}
> > > +
> > > +static int __init proc_raw_cmdline_init(void)
> > > +{
> > > +	struct proc_dir_entry *pde;
> > > +
> > > +	pde = proc_create_single("raw_cmdline", 0, NULL, raw_cmdline_proc_show);
> > > +	pde_make_permanent(pde);
> > > +	pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > +	return 0;
> > > +}
> > > +fs_initcall(proc_raw_cmdline_init);
> > > diff --git a/include/linux/init.h b/include/linux/init.h
> > > index 266c3e1640d4..29e75bbe7984 100644
> > > --- a/include/linux/init.h
> > > +++ b/include/linux/init.h
> > > @@ -112,6 +112,7 @@
> > >  #define __REFCONST       .section       ".ref.rodata", "a"
> > >  
> > >  #ifndef __ASSEMBLY__
> > > +
> > >  /*
> > >   * Used for initialization calls..
> > >   */
> > > @@ -143,7 +144,7 @@ struct file_system_type;
> > >  
> > >  /* Defined in init/main.c */
> > >  extern int do_one_initcall(initcall_t fn);
> > > -extern char __initdata boot_command_line[];
> > > +extern char boot_command_line[];
> > >  extern char *saved_command_line;
> > >  extern unsigned int saved_command_line_len;
> > >  extern unsigned int reset_devices;
> > > diff --git a/init/main.c b/init/main.c
> > > index ad920fac325c..2121685c479a 100644
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
> > >  void (*__initdata late_time_init)(void);
> > >  
> > >  /* Untouched command line saved by arch-specific code. */
> > > -char __initdata boot_command_line[COMMAND_LINE_SIZE];
> > > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
> > >  /* Untouched saved command line (eg. for /proc) */
> > >  char *saved_command_line __ro_after_init;
> > >  unsigned int saved_command_line_len __ro_after_init;
> > 
> > 
> > -- 
> > Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Masami Hiramatsu]

On Wed, 16 Aug 2023 09:13:30 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> On Thu, Aug 17, 2023 at 12:17:31AM +0900, Masami Hiramatsu wrote:
> > On Wed, 16 Aug 2023 18:40:03 +0900
> > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > > On Mon, 14 Aug 2023 16:08:29 -0700
> > > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > > On Sat, Aug 12, 2023 at 04:30:41PM -0700, Paul E. McKenney wrote:
> > > > > On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote:
> > > > > > On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote:
> > > > > > > On Fri, 4 Aug 2023 10:36:17 -0700
> > > > > > > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > > > > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote:
> > > > > > > > > On Thu, Jul 27, 2023 at 08:37:00PM -0700, Paul E. McKenney wrote:
> > > > > > > > > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > > > > > > > > show all kernel boot parameters, both those supplied by the boot loader
> > > > > > > > > > and those embedded in the kernel image.  This works well for those who
> > > > > > > > > > just want to see all of the kernel boot parameters, but is not helpful to
> > > > > > > > > > those who need to see only those parameters supplied by the boot loader.
> > > > > > > > > > This is especially important when these parameters are presented to the
> > > > > > > > > > boot loader by automation that might gather them from diverse sources.
> > > > > > > > > > 
> > > > > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > > > > > > > > > boot parameters supplied by the boot loader.
> > > > > > > > > 
> > > > > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > > > > > > > > > +{
> > > > > > > > > > +	seq_puts(m, boot_command_line);
> > > > > > > > > > +	seq_putc(m, '\n');
> > > > > > > > > > +	return 0;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > >  static int __init proc_cmdline_init(void)
> > > > > > > > > >  {
> > > > > > > > > >  	struct proc_dir_entry *pde;
> > > > > > > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> > > > > > > > > >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> > > > > > > > > >  	pde_make_permanent(pde);
> > > > > > > > > >  	pde->size = saved_command_line_len + 1;
> > > > > > > > > > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > > > > > > > > > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > > > > > > > > > +		pde_make_permanent(pde);
> > > > > > > > > > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > > > > > > > > +	}
> > > > > > > > > 
> > > > > > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of
> > > > > > > > > the file matches name of the /proc file.
> > > > > > > > 
> > > > > > > > Thank you, will do!
> > > > > > > > 
> > > > > > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader"
> > > > > > > > > somewhere should improve things.
> > > > > > > > 
> > > > > > > > If we can all quickly come to agreement on a name, I can of course easily
> > > > > > > > change it.
> > > > > > > > 
> > > > > > > > /proc/cmdline_bootloader?  Better than /proc/cmdline_from_bootloader,
> > > > > > > > I suppose.  /proc/cmdline_bootldr?  /proc/bootloader by analogy with
> > > > > > > > /proc/bootconfig?  Something else?
> > > > > > > 
> > > > > > > What about "/proc/raw_cmdline" ?
> > > > > > 
> > > > > > That would work of me!
> > > > > > 
> > > > > > Any objections to /proc/raw_cmdline?
> > > > > > 
> > > > > > Going once...
> > > > > 
> > > > > Going twice...
> > > > > 
> > > > > If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT.
> > > > 
> > > > And gone!
> > > > 
> > > > Please see below for the updated version.
> > > 
> > > OK, I'll pick this.
> > 
> > Wait, is it OK to push this through bootconfig tree? Since this is not directly
> > related to the bootconfig, fsdevel maintainer can pick this.
> > I would like to ping to fsdevel people at first.
> 
> Whichever path works best works for me!
> 
> Here are the three commits I have queued, just please let me know when any
> of them are pulled into some other tree and I will drop them from -rcu.
> 
> 97edd1291847 ("doc: Update /proc/cmdline documentation to include boot config")
> 0fe10f0d1873 ("fs/proc: Add /proc/raw_cmdline for boot loader arguments")
> 9192c6ac7516 ("doc: Add /proc/bootconfig to proc.rst")

OK, I picked and pushed to linux-trace's bootconfig/for-next branch.

Thank you!

> 
> 							Thanx, Paul
> 
> > Thank you,
> > 
> > > 
> > > Thanks!
> > > 
> > > > 
> > > > 								Thanx, Paul
> > > > 
> > > > ------------------------------------------------------------------------
> > > > 
> > > > commit 0fe10f0d1873a6f6e287c0c5b45e9203b0e33c83
> > > > Author: Paul E. McKenney <paulmck@kernel.org>
> > > > Date:   Fri Jul 21 16:05:38 2023 -0700
> > > > 
> > > >     fs/proc: Add /proc/raw_cmdline for boot loader arguments
> > > >     
> > > >     In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > >     show all kernel boot parameters, both those supplied by the boot loader
> > > >     and those embedded in the kernel image.  This works well for those who
> > > >     just want to see all of the kernel boot parameters, but is not helpful to
> > > >     those who need to see only those parameters supplied by the boot loader.
> > > >     This is especially important when these parameters are presented to the
> > > >     boot loader by automation that might gather them from diverse sources.
> > > >     It is also useful when booting the next kernel via kexec(), in which
> > > >     case it is necessary to supply only those kernel command-line arguments
> > > >     from the boot loader, and most definitely not those that were embedded
> > > >     into the current kernel.
> > > >     
> > > >     Therefore, provide a /proc/raw_cmdline file that shows only those kernel
> > > >     boot parameters supplied by the boot loader.
> > > >     
> > > >     Why put this in /proc?  Because it is quite similar to /proc/cmdline,
> > > >     and /proc/bootconfig, so it makes sense to put it in the same place that
> > > >     those files are located.
> > > >     
> > > >     [ sfr: Apply kernel test robot feedback. ]
> > > >     [ paulmck: Apply Randy Dunlap feedback. ]
> > > >     [ paulmck: Apply naming feedback from Alexey Dobriyan and Masami Hiramatsu. ]
> > > >     
> > > >     Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > >     Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > >     Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > > >     Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> > > >     Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > > >     Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > > >     Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > > >     Cc: Andrew Morton <akpm@linux-foundation.org>
> > > >     Cc: Alexey Dobriyan <adobriyan@gmail.com>
> > > >     Cc: <linux-fsdevel@vger.kernel.org>
> > > > 
> > > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> > > > index 75a8c899ebcc..61419270c38f 100644
> > > > --- a/Documentation/filesystems/proc.rst
> > > > +++ b/Documentation/filesystems/proc.rst
> > > > @@ -723,6 +723,7 @@ files are there, and which are missing.
> > > >   partitions   Table of partitions known to the system
> > > >   pci 	      Deprecated info of PCI bus (new way -> /proc/bus/pci/,
> > > >                decoupled by lspci				(2.4)
> > > > + raw_cmdline  Kernel command line obtained from kernel image	(6.6)
> > > >   rtc          Real time clock
> > > >   scsi         SCSI info (see text)
> > > >   slabinfo     Slab pool info
> > > > diff --git a/fs/proc/Makefile b/fs/proc/Makefile
> > > > index bd08616ed8ba..6182296f3c6b 100644
> > > > --- a/fs/proc/Makefile
> > > > +++ b/fs/proc/Makefile
> > > > @@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE)	+= vmcore.o
> > > >  proc-$(CONFIG_PRINTK)	+= kmsg.o
> > > >  proc-$(CONFIG_PROC_PAGE_MONITOR)	+= page.o
> > > >  proc-$(CONFIG_BOOT_CONFIG)	+= bootconfig.o
> > > > +proc-$(CONFIG_BOOT_CONFIG_FORCE)	+= raw_cmdline.o
> > > > diff --git a/fs/proc/raw_cmdline.c b/fs/proc/raw_cmdline.c
> > > > new file mode 100644
> > > > index 000000000000..2e19eb89fc8e
> > > > --- /dev/null
> > > > +++ b/fs/proc/raw_cmdline.c
> > > > @@ -0,0 +1,25 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +#include <linux/fs.h>
> > > > +#include <linux/init.h>
> > > > +#include <linux/proc_fs.h>
> > > > +#include <linux/seq_file.h>
> > > > +#include <asm/setup.h>
> > > > +#include "internal.h"
> > > > +
> > > > +static int raw_cmdline_proc_show(struct seq_file *m, void *v)
> > > > +{
> > > > +	seq_puts(m, boot_command_line);
> > > > +	seq_putc(m, '\n');
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int __init proc_raw_cmdline_init(void)
> > > > +{
> > > > +	struct proc_dir_entry *pde;
> > > > +
> > > > +	pde = proc_create_single("raw_cmdline", 0, NULL, raw_cmdline_proc_show);
> > > > +	pde_make_permanent(pde);
> > > > +	pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > > +	return 0;
> > > > +}
> > > > +fs_initcall(proc_raw_cmdline_init);
> > > > diff --git a/include/linux/init.h b/include/linux/init.h
> > > > index 266c3e1640d4..29e75bbe7984 100644
> > > > --- a/include/linux/init.h
> > > > +++ b/include/linux/init.h
> > > > @@ -112,6 +112,7 @@
> > > >  #define __REFCONST       .section       ".ref.rodata", "a"
> > > >  
> > > >  #ifndef __ASSEMBLY__
> > > > +
> > > >  /*
> > > >   * Used for initialization calls..
> > > >   */
> > > > @@ -143,7 +144,7 @@ struct file_system_type;
> > > >  
> > > >  /* Defined in init/main.c */
> > > >  extern int do_one_initcall(initcall_t fn);
> > > > -extern char __initdata boot_command_line[];
> > > > +extern char boot_command_line[];
> > > >  extern char *saved_command_line;
> > > >  extern unsigned int saved_command_line_len;
> > > >  extern unsigned int reset_devices;
> > > > diff --git a/init/main.c b/init/main.c
> > > > index ad920fac325c..2121685c479a 100644
> > > > --- a/init/main.c
> > > > +++ b/init/main.c
> > > > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
> > > >  void (*__initdata late_time_init)(void);
> > > >  
> > > >  /* Untouched command line saved by arch-specific code. */
> > > > -char __initdata boot_command_line[COMMAND_LINE_SIZE];
> > > > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
> > > >  /* Untouched saved command line (eg. for /proc) */
> > > >  char *saved_command_line __ro_after_init;
> > > >  unsigned int saved_command_line_len __ro_after_init;
> > > 
> > > 
> > > -- 
> > > Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > 
> > -- 
> > Masami Hiramatsu (Google) <mhiramat@kernel.org>


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH RFC bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments

[Paul E. McKenney]

On Sun, Aug 20, 2023 at 11:14:01PM +0900, Masami Hiramatsu wrote:
> On Wed, 16 Aug 2023 09:13:30 -0700
> "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > On Thu, Aug 17, 2023 at 12:17:31AM +0900, Masami Hiramatsu wrote:
> > > On Wed, 16 Aug 2023 18:40:03 +0900
> > > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > > > On Mon, 14 Aug 2023 16:08:29 -0700
> > > > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > > > On Sat, Aug 12, 2023 at 04:30:41PM -0700, Paul E. McKenney wrote:
> > > > > > On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote:
> > > > > > > On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote:
> > > > > > > > On Fri, 4 Aug 2023 10:36:17 -0700
> > > > > > > > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > > > > > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote:
> > > > > > > > > > On Thu, Jul 27, 2023 at 08:37:00PM -0700, Paul E. McKenney wrote:
> > > > > > > > > > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > > > > > > > > > show all kernel boot parameters, both those supplied by the boot loader
> > > > > > > > > > > and those embedded in the kernel image.  This works well for those who
> > > > > > > > > > > just want to see all of the kernel boot parameters, but is not helpful to
> > > > > > > > > > > those who need to see only those parameters supplied by the boot loader.
> > > > > > > > > > > This is especially important when these parameters are presented to the
> > > > > > > > > > > boot loader by automation that might gather them from diverse sources.
> > > > > > > > > > > 
> > > > > > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel
> > > > > > > > > > > boot parameters supplied by the boot loader.
> > > > > > > > > > 
> > > > > > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v)
> > > > > > > > > > > +{
> > > > > > > > > > > +	seq_puts(m, boot_command_line);
> > > > > > > > > > > +	seq_putc(m, '\n');
> > > > > > > > > > > +	return 0;
> > > > > > > > > > > +}
> > > > > > > > > > > +
> > > > > > > > > > >  static int __init proc_cmdline_init(void)
> > > > > > > > > > >  {
> > > > > > > > > > >  	struct proc_dir_entry *pde;
> > > > > > > > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
> > > > > > > > > > >  	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
> > > > > > > > > > >  	pde_make_permanent(pde);
> > > > > > > > > > >  	pde->size = saved_command_line_len + 1;
> > > > > > > > > > > +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
> > > > > > > > > > > +		pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> > > > > > > > > > > +		pde_make_permanent(pde);
> > > > > > > > > > > +		pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > > > > > > > > > +	}
> > > > > > > > > > 
> > > > > > > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of
> > > > > > > > > > the file matches name of the /proc file.
> > > > > > > > > 
> > > > > > > > > Thank you, will do!
> > > > > > > > > 
> > > > > > > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader"
> > > > > > > > > > somewhere should improve things.
> > > > > > > > > 
> > > > > > > > > If we can all quickly come to agreement on a name, I can of course easily
> > > > > > > > > change it.
> > > > > > > > > 
> > > > > > > > > /proc/cmdline_bootloader?  Better than /proc/cmdline_from_bootloader,
> > > > > > > > > I suppose.  /proc/cmdline_bootldr?  /proc/bootloader by analogy with
> > > > > > > > > /proc/bootconfig?  Something else?
> > > > > > > > 
> > > > > > > > What about "/proc/raw_cmdline" ?
> > > > > > > 
> > > > > > > That would work of me!
> > > > > > > 
> > > > > > > Any objections to /proc/raw_cmdline?
> > > > > > > 
> > > > > > > Going once...
> > > > > > 
> > > > > > Going twice...
> > > > > > 
> > > > > > If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT.
> > > > > 
> > > > > And gone!
> > > > > 
> > > > > Please see below for the updated version.
> > > > 
> > > > OK, I'll pick this.
> > > 
> > > Wait, is it OK to push this through bootconfig tree? Since this is not directly
> > > related to the bootconfig, fsdevel maintainer can pick this.
> > > I would like to ping to fsdevel people at first.
> > 
> > Whichever path works best works for me!
> > 
> > Here are the three commits I have queued, just please let me know when any
> > of them are pulled into some other tree and I will drop them from -rcu.
> > 
> > 97edd1291847 ("doc: Update /proc/cmdline documentation to include boot config")
> > 0fe10f0d1873 ("fs/proc: Add /proc/raw_cmdline for boot loader arguments")
> > 9192c6ac7516 ("doc: Add /proc/bootconfig to proc.rst")
> 
> OK, I picked and pushed to linux-trace's bootconfig/for-next branch.

Thank you, and I will drop them from -rcu on my next rebase.

							Thanx, Paul

> > > Thank you,
> > > 
> > > > 
> > > > Thanks!
> > > > 
> > > > > 
> > > > > 								Thanx, Paul
> > > > > 
> > > > > ------------------------------------------------------------------------
> > > > > 
> > > > > commit 0fe10f0d1873a6f6e287c0c5b45e9203b0e33c83
> > > > > Author: Paul E. McKenney <paulmck@kernel.org>
> > > > > Date:   Fri Jul 21 16:05:38 2023 -0700
> > > > > 
> > > > >     fs/proc: Add /proc/raw_cmdline for boot loader arguments
> > > > >     
> > > > >     In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will
> > > > >     show all kernel boot parameters, both those supplied by the boot loader
> > > > >     and those embedded in the kernel image.  This works well for those who
> > > > >     just want to see all of the kernel boot parameters, but is not helpful to
> > > > >     those who need to see only those parameters supplied by the boot loader.
> > > > >     This is especially important when these parameters are presented to the
> > > > >     boot loader by automation that might gather them from diverse sources.
> > > > >     It is also useful when booting the next kernel via kexec(), in which
> > > > >     case it is necessary to supply only those kernel command-line arguments
> > > > >     from the boot loader, and most definitely not those that were embedded
> > > > >     into the current kernel.
> > > > >     
> > > > >     Therefore, provide a /proc/raw_cmdline file that shows only those kernel
> > > > >     boot parameters supplied by the boot loader.
> > > > >     
> > > > >     Why put this in /proc?  Because it is quite similar to /proc/cmdline,
> > > > >     and /proc/bootconfig, so it makes sense to put it in the same place that
> > > > >     those files are located.
> > > > >     
> > > > >     [ sfr: Apply kernel test robot feedback. ]
> > > > >     [ paulmck: Apply Randy Dunlap feedback. ]
> > > > >     [ paulmck: Apply naming feedback from Alexey Dobriyan and Masami Hiramatsu. ]
> > > > >     
> > > > >     Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > > >     Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > > >     Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > > > >     Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> > > > >     Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > > > >     Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > > > >     Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > > > >     Cc: Andrew Morton <akpm@linux-foundation.org>
> > > > >     Cc: Alexey Dobriyan <adobriyan@gmail.com>
> > > > >     Cc: <linux-fsdevel@vger.kernel.org>
> > > > > 
> > > > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> > > > > index 75a8c899ebcc..61419270c38f 100644
> > > > > --- a/Documentation/filesystems/proc.rst
> > > > > +++ b/Documentation/filesystems/proc.rst
> > > > > @@ -723,6 +723,7 @@ files are there, and which are missing.
> > > > >   partitions   Table of partitions known to the system
> > > > >   pci 	      Deprecated info of PCI bus (new way -> /proc/bus/pci/,
> > > > >                decoupled by lspci				(2.4)
> > > > > + raw_cmdline  Kernel command line obtained from kernel image	(6.6)
> > > > >   rtc          Real time clock
> > > > >   scsi         SCSI info (see text)
> > > > >   slabinfo     Slab pool info
> > > > > diff --git a/fs/proc/Makefile b/fs/proc/Makefile
> > > > > index bd08616ed8ba..6182296f3c6b 100644
> > > > > --- a/fs/proc/Makefile
> > > > > +++ b/fs/proc/Makefile
> > > > > @@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE)	+= vmcore.o
> > > > >  proc-$(CONFIG_PRINTK)	+= kmsg.o
> > > > >  proc-$(CONFIG_PROC_PAGE_MONITOR)	+= page.o
> > > > >  proc-$(CONFIG_BOOT_CONFIG)	+= bootconfig.o
> > > > > +proc-$(CONFIG_BOOT_CONFIG_FORCE)	+= raw_cmdline.o
> > > > > diff --git a/fs/proc/raw_cmdline.c b/fs/proc/raw_cmdline.c
> > > > > new file mode 100644
> > > > > index 000000000000..2e19eb89fc8e
> > > > > --- /dev/null
> > > > > +++ b/fs/proc/raw_cmdline.c
> > > > > @@ -0,0 +1,25 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +#include <linux/fs.h>
> > > > > +#include <linux/init.h>
> > > > > +#include <linux/proc_fs.h>
> > > > > +#include <linux/seq_file.h>
> > > > > +#include <asm/setup.h>
> > > > > +#include "internal.h"
> > > > > +
> > > > > +static int raw_cmdline_proc_show(struct seq_file *m, void *v)
> > > > > +{
> > > > > +	seq_puts(m, boot_command_line);
> > > > > +	seq_putc(m, '\n');
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > > +static int __init proc_raw_cmdline_init(void)
> > > > > +{
> > > > > +	struct proc_dir_entry *pde;
> > > > > +
> > > > > +	pde = proc_create_single("raw_cmdline", 0, NULL, raw_cmdline_proc_show);
> > > > > +	pde_make_permanent(pde);
> > > > > +	pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> > > > > +	return 0;
> > > > > +}
> > > > > +fs_initcall(proc_raw_cmdline_init);
> > > > > diff --git a/include/linux/init.h b/include/linux/init.h
> > > > > index 266c3e1640d4..29e75bbe7984 100644
> > > > > --- a/include/linux/init.h
> > > > > +++ b/include/linux/init.h
> > > > > @@ -112,6 +112,7 @@
> > > > >  #define __REFCONST       .section       ".ref.rodata", "a"
> > > > >  
> > > > >  #ifndef __ASSEMBLY__
> > > > > +
> > > > >  /*
> > > > >   * Used for initialization calls..
> > > > >   */
> > > > > @@ -143,7 +144,7 @@ struct file_system_type;
> > > > >  
> > > > >  /* Defined in init/main.c */
> > > > >  extern int do_one_initcall(initcall_t fn);
> > > > > -extern char __initdata boot_command_line[];
> > > > > +extern char boot_command_line[];
> > > > >  extern char *saved_command_line;
> > > > >  extern unsigned int saved_command_line_len;
> > > > >  extern unsigned int reset_devices;
> > > > > diff --git a/init/main.c b/init/main.c
> > > > > index ad920fac325c..2121685c479a 100644
> > > > > --- a/init/main.c
> > > > > +++ b/init/main.c
> > > > > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
> > > > >  void (*__initdata late_time_init)(void);
> > > > >  
> > > > >  /* Untouched command line saved by arch-specific code. */
> > > > > -char __initdata boot_command_line[COMMAND_LINE_SIZE];
> > > > > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
> > > > >  /* Untouched saved command line (eg. for /proc) */
> > > > >  char *saved_command_line __ro_after_init;
> > > > >  unsigned int saved_command_line_len __ro_after_init;
> > > > 
> > > > 
> > > > -- 
> > > > Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > > 
> > > 
> > > -- 
> > > Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>