On Thu, Mar 25, 2021 at 12:18:38PM +0100, Christophe Leroy wrote: > > > Le 03/03/2021 à 18:57, Will Deacon a écrit : > > On Tue, Mar 02, 2021 at 05:25:22PM +0000, Christophe Leroy wrote: > > > 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 <christophe.leroy@xxxxxxxxxx> > > > --- > > > 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. > > > > > +config CMDLINE > > > + string "Initial kernel command string" > > > > s/Initial/Default > > > > which is then consistent with the rest of the text here. > > > > > + depends on CMDLINE_BOOL > > > > Ah, so this is a bit different and I don't think lines-up with the > > CMDLINE_BOOL help text. > > > > > + 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) > > > > > +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" > > > > > +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 ? I think ARM gets different behaviour depending on whether it uses ATAGs or FDT. > 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 Oh man, I didn't spot that one :( I think I would make the generic options explicit: either APPEND or PREPEND. Then architectures which choose to define CMDLINE_EXTEND in their Kconfigs can select the generic option that matches their behaviour. INITRAMFS_FORCE sounds like it should depend on APPEND (assuming that means CONFIG_CMDLINE is appended to the bootloader arguments). Will