Re: [PATCH v2 6/7] cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation

Le 03/03/2021 à 18:57, Will Deacon a écrit :

On Tue, Mar 02, 2021 at 05:25:22PM +0000, Christophe Leroy wrote:

quoted

Most architectures have similar boot command line manipulation
options. This patchs adds the definition in init/Kconfig, gated by
CONFIG_HAVE_CMDLINE that the architectures can select to use them.

In order to use this, a few architectures will have to change their
CONFIG options:
- riscv has to replace CMDLINE_FALLBACK by CMDLINE_FROM_BOOTLOADER
- architectures using CONFIG_CMDLINE_OVERRIDE or
CONFIG_CMDLINE_OVERWRITE have to replace them by CONFIG_CMDLINE_FORCE.

Architectures also have to define CONFIG_DEFAULT_CMDLINE.

Signed-off-by: Christophe Leroy <redacted>
---
  init/Kconfig | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 56 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index 22946fe5ded9..a0f2ad9467df 100644
--- a/init/Kconfig
+++ b/init/Kconfig

@@ -117,6 +117,62 @@ config INIT_ENV_ARG_LIMIT
  	  Maximum of each of the number of arguments and environment
  	  variables passed to init from the kernel command line.
  
+config HAVE_CMDLINE
+	bool
+
+config CMDLINE_BOOL
+	bool "Default bootloader kernel arguments"
+	depends on HAVE_CMDLINE
+	help
+	  On some platforms, there is currently no way for the boot loader to
+	  pass arguments to the kernel. For these platforms, you can supply
+	  some command-line options at build time by entering them here.  In
+	  most cases you will need to specify the root device here.

Why is this needed as well as CMDLINE_FROM_BOOTLOADER? IIUC, the latter
will use CONFIG_CMDLINE if it fails to get anything from the bootloader,
which sounds like the same scenario.

quoted

+config CMDLINE
+	string "Initial kernel command string"

s/Initial/Default

which is then consistent with the rest of the text here.

quoted

+	depends on CMDLINE_BOOL

Ah, so this is a bit different and I don't think lines-up with the
CMDLINE_BOOL help text.

quoted

+	default DEFAULT_CMDLINE
+	help
+	  On some platforms, there is currently no way for the boot loader to
+	  pass arguments to the kernel. For these platforms, you can supply
+	  some command-line options at build time by entering them here.  In
+	  most cases you will need to specify the root device here.

(same stale text)

quoted

+choice
+	prompt "Kernel command line type" if CMDLINE != ""
+	default CMDLINE_FROM_BOOTLOADER
+	help
+	  Selects the way you want to use the default kernel arguments.

How about:

"Determines how the default kernel arguments are combined with any
  arguments passed by the bootloader"

quoted

+config CMDLINE_FROM_BOOTLOADER
+	bool "Use bootloader kernel arguments if available"
+	help
+	  Uses the command-line options passed by the boot loader. If
+	  the boot loader doesn't provide any, the default kernel command
+	  string provided in CMDLINE will be used.
+
+config CMDLINE_EXTEND

Can we rename this to CMDLINE_APPEND, please? There is code in the tree
which disagrees about what CMDLINE_EXTEND means, so that will need be
to be updated to be consistent (e.g. the EFI stub parsing order). Having
the generic option with a different name means we won't accidentally end
up with the same inconsistent behaviours.

Argh, yes. Seems like the problem is even larger than that IIUC:

- For ARM it means to append the bootloader arguments to the CONFIG_CMDLINE
- For Powerpc it means to append the CONFIG_CMDLINE to the bootloader arguments
- For SH  it means to append the CONFIG_CMDLINE to the bootloader arguments
- For EFI it means to append the bootloader arguments to the CONFIG_CMDLINE
- For OF it means to append the CONFIG_CMDLINE to the bootloader arguments

So what happens on ARM for instance when it selects CONFIG_OF for instance ?
Or should we consider that EXTEND means APPEND or PREPEND, no matter which ?
Because EXTEND is for instance used for:

	config INITRAMFS_FORCE
		bool "Ignore the initramfs passed by the bootloader"
		depends on CMDLINE_EXTEND || CMDLINE_FORCE


Christophe