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 +, 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 --- 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. You are right, the help text is duplicated, I will change the text for the CMDLINE_BOOL + 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. + bool "Extend bootloader kernel arguments" "Append to the bootloader kernel arguments" + help + The default kernel command string will be appended to the + command-line arguments provided during boot. s/provided during boot/provided by the bootloader/ + +config CMDLINE_PREPEND + bool "Prepend bootloader kernel arguments" "Prepend to the bootloader kernel arguments" + help + The default kernel command string will be prepend to the + command-line arguments provided during boot. s/prepend/prepended/ s/provided during boot/provided by the bootloader/ + +config CMDLINE_FORCE + bool "Always use the default kernel command string" + help + Always use the default kernel command string, even if the boot + loader passes other arguments to the kernel. + This is useful if you cannot or don't want to change the + command-line options your boot loader passes to the kernel. I find the "This is useful if ..." sentence really confusing, perhaps just remove it? I'd then tweak it to be: "Always use the default kernel command string, ignoring any arguments provided by the bootloader." Taken all your suggested text. Thanks Christophe
Re: [PATCH v2 6/7] cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation
Le 25/03/2021 à 20:32, Will Deacon a écrit : On Thu, Mar 25, 2021 at 12:18:38PM +0100, Christophe Leroy wrote: - 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. As far as I can see, ARM uses either ATAGs only or both ATAGs and FDT. ATAGs is forced to 'y' when USE_OF is set. Do I miss something ? 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). Christophe
Re: [PATCH v2 6/7] cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation
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 +, 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 > > > --- > > > 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
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 +, 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 --- 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 ? 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
Re: [PATCH v2 6/7] cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation
On Tue, Mar 02, 2021 at 05:25:22PM +, 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 > --- > 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. > + bool "Extend bootloader kernel arguments" "Append to the bootloader kernel arguments" > + help > + The default kernel command string will be appended to the > + command-line arguments provided during boot. s/provided during boot/provided by the bootloader/ > + > +config CMDLINE_PREPEND > + bool "Prepend bootloader kernel arguments" "Prepend to the bootloader kernel arguments" > + help > + The default kernel command string will be prepend to the > + command-line arguments provided during boot. s/prepend/prepended/ s/provided during boot/provided by the bootloader/ > + > +config CMDLINE_FORCE > + bool "Always use the default kernel command string" > + help > + Always use the default kernel command string, even if the boot > + loader passes other arguments to the kernel. > + This is useful if you cannot or don't want to change the > + command-line options your boot loader passes to the kernel. I find the "This is useful if ..." sentence really confusing, perhaps just remove it? I'd then tweak it to be: "Always use the default kernel command string, ignoring any arguments provided by the bootloader." Will
Re: [PATCH v2 6/7] cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation
Hi Christophe, I love your patch! Yet something to improve: [auto build test ERROR on powerpc/next] [also build test ERROR on robh/for-next linus/master v5.12-rc1 next-20210302] [cannot apply to mpe/next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/Improve-boot-command-line-handling/20210303-014039 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: sh-randconfig-s031-20210302 (attached as .config) compiler: sh4-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.3-241-geaceeafa-dirty # https://github.com/0day-ci/linux/commit/edc3f8320d1dcb21a71e4cfdb26a3d2b64215c30 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Christophe-Leroy/Improve-boot-command-line-handling/20210303-014039 git checkout edc3f8320d1dcb21a71e4cfdb26a3d2b64215c30 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=sh If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): arch/sh/Kconfig:760:warning: choice value used outside its choice group >> init/Kconfig:142:error: recursive dependency detected! init/Kconfig:142: choice contains symbol CMDLINE init/Kconfig:132: symbol CMDLINE depends on CMDLINE_EXTEND init/Kconfig:155: symbol CMDLINE_EXTEND is part of choice For a resolution refer to Documentation/kbuild/kconfig-language.rst subsection "Kconfig recursive dependency limitations" vim +142 init/Kconfig 103 104 config BROKEN 105 bool 106 107 config BROKEN_ON_SMP 108 bool 109 depends on BROKEN || !SMP 110 default y 111 112 config INIT_ENV_ARG_LIMIT 113 int 114 default 32 if !UML 115 default 128 if UML 116 help 117Maximum of each of the number of arguments and environment 118variables passed to init from the kernel command line. 119 120 config HAVE_CMDLINE 121 bool 122 123 config CMDLINE_BOOL 124 bool "Default bootloader kernel arguments" 125 depends on HAVE_CMDLINE 126 help 127On some platforms, there is currently no way for the boot loader to 128pass arguments to the kernel. For these platforms, you can supply 129some command-line options at build time by entering them here. In 130most cases you will need to specify the root device here. 131 132 config CMDLINE 133 string "Initial kernel command string" 134 depends on CMDLINE_BOOL 135 default DEFAULT_CMDLINE 136 help 137On some platforms, there is currently no way for the boot loader to 138pass arguments to the kernel. For these platforms, you can supply 139some command-line options at build time by entering them here. In 140most cases you will need to specify the root device here. 141 > 142 choice 143 prompt "Kernel command line type" if CMDLINE != "" 144 default CMDLINE_FROM_BOOTLOADER 145 help 146Selects the way you want to use the default kernel arguments. 147 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
[PATCH v2 6/7] cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation
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 --- 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. + +config CMDLINE + string "Initial kernel command string" + depends on CMDLINE_BOOL + 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. + +choice + prompt "Kernel command line type" if CMDLINE != "" + default CMDLINE_FROM_BOOTLOADER + help + Selects the way you want to use the default kernel arguments. + +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 + bool "Extend bootloader kernel arguments" + help + The default kernel command string will be appended to the + command-line arguments provided during boot. + +config CMDLINE_PREPEND + bool "Prepend bootloader kernel arguments" + help + The default kernel command string will be prepend to the + command-line arguments provided during boot. + +config CMDLINE_FORCE + bool "Always use the default kernel command string" + help + Always use the default kernel command string, even if the boot + loader passes other arguments to the kernel. + This is useful if you cannot or don't want to change the + command-line options your boot loader passes to the kernel. +endchoice + config COMPILE_TEST bool "Compile also drivers which will not load" depends on !UML && !S390 -- 2.25.0
