| Message ID | cover.1616765869.git.christophe.leroy@csgroup.eu |
|---|---|
| Headers | show |
| Series | Implement GENERIC_CMDLINE | expand |
On Mär 26 2021, Christophe Leroy wrote: > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > index f8f15332caa2..e7c91ee478d1 100644 > --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c > @@ -20,6 +20,7 @@ > #include <linux/swiotlb.h> > #include <linux/smp.h> > #include <linux/efi.h> > +#include <linux/cmdline.h> > > #include <asm/cpu_ops.h> > #include <asm/early_ioremap.h> > @@ -228,10 +229,8 @@ static void __init parse_dtb(void) > } > > pr_err("No DTB passed to the kernel\n"); > -#ifdef CONFIG_CMDLINE_FORCE > - strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE); > + cmdline_build(boot_command_line, NULL, COMMAND_LINE_SIZE); > pr_info("Forcing kernel command line to: %s\n", boot_command_line); Shouldn't that message become conditional in some way? Andreas.
Le 26/03/2021 à 15:08, Andreas Schwab a écrit : > On Mär 26 2021, Christophe Leroy wrote: > >> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c >> index f8f15332caa2..e7c91ee478d1 100644 >> --- a/arch/riscv/kernel/setup.c >> +++ b/arch/riscv/kernel/setup.c >> @@ -20,6 +20,7 @@ >> #include <linux/swiotlb.h> >> #include <linux/smp.h> >> #include <linux/efi.h> >> +#include <linux/cmdline.h> >> >> #include <asm/cpu_ops.h> >> #include <asm/early_ioremap.h> >> @@ -228,10 +229,8 @@ static void __init parse_dtb(void) >> } >> >> pr_err("No DTB passed to the kernel\n"); >> -#ifdef CONFIG_CMDLINE_FORCE >> - strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE); >> + cmdline_build(boot_command_line, NULL, COMMAND_LINE_SIZE); >> pr_info("Forcing kernel command line to: %s\n", boot_command_line); > > Shouldn't that message become conditional in some way? > You are right, I did something similar on ARM but looks like I missed it on RISCV. Christophe
On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > > The purpose of this series is to improve and enhance the > handling of kernel boot arguments. > > It is first focussed on powerpc but also extends the capability > for other arches. > > This is based on suggestion from Daniel Walker <danielwa@cisco.com> > > Main changes in V3: > - Also accept destination equal to source in cmdline_build() by setting a tmp buffer in __initdata. Powerpc provides different source and destination and call __cmdline_build() directly. > - Taken comments received from Will and Rob > - Converted all architectures (Only tested on powerpc) > > Christophe Leroy (17): > cmdline: Add generic function to build command line. > drivers: of: use cmdline building function > cmdline: Gives architectures opportunity to use generically defined > boot cmdline manipulation > powerpc: Convert to GENERIC_CMDLINE > arm: Convert to GENERIC_CMDLINE > arm64: Convert to GENERIC_CMDLINE > hexagon: Convert to GENERIC_CMDLINE > microblaze: Convert to GENERIC_CMDLINE > nios2: Convert to GENERIC_CMDLINE > openrisc: Convert to GENERIC_CMDLINE > riscv: Convert to GENERIC_CMDLINE > sh: Convert to GENERIC_CMDLINE > sparc: Convert to GENERIC_CMDLINE > xtensa: Convert to GENERIC_CMDLINE > x86: Convert to GENERIC_CMDLINE > mips: Convert to GENERIC_CMDLINE > cmdline: Remove CONFIG_CMDLINE_EXTEND > > arch/arm/Kconfig | 38 +------------- > arch/arm/kernel/atags_parse.c | 15 ++---- > arch/arm64/Kconfig | 33 +----------- > arch/arm64/kernel/idreg-override.c | 9 ++-- > arch/hexagon/Kconfig | 11 +--- > arch/hexagon/kernel/setup.c | 10 +--- > arch/microblaze/Kconfig | 24 +-------- > arch/microblaze/configs/mmu_defconfig | 2 +- > arch/microblaze/kernel/head.S | 4 +- > arch/mips/Kconfig | 1 + > arch/mips/Kconfig.debug | 44 ---------------- > arch/mips/configs/ar7_defconfig | 1 - > arch/mips/configs/bcm47xx_defconfig | 1 - > arch/mips/configs/bcm63xx_defconfig | 1 - > arch/mips/configs/bmips_be_defconfig | 1 - > arch/mips/configs/bmips_stb_defconfig | 1 - > arch/mips/configs/capcella_defconfig | 1 - > arch/mips/configs/ci20_defconfig | 1 - > arch/mips/configs/cu1000-neo_defconfig | 1 - > arch/mips/configs/cu1830-neo_defconfig | 1 - > arch/mips/configs/e55_defconfig | 1 - > arch/mips/configs/generic_defconfig | 1 - > arch/mips/configs/gpr_defconfig | 1 - > arch/mips/configs/loongson3_defconfig | 1 - > arch/mips/configs/mpc30x_defconfig | 1 - > arch/mips/configs/rt305x_defconfig | 1 - > arch/mips/configs/tb0219_defconfig | 1 - > arch/mips/configs/tb0226_defconfig | 1 - > arch/mips/configs/tb0287_defconfig | 1 - > arch/mips/configs/workpad_defconfig | 1 - > arch/mips/configs/xway_defconfig | 1 - > arch/mips/kernel/relocate.c | 4 +- > arch/mips/kernel/setup.c | 40 +-------------- > arch/mips/pic32/pic32mzda/early_console.c | 2 +- > arch/mips/pic32/pic32mzda/init.c | 2 - > arch/nios2/Kconfig | 24 +-------- > arch/nios2/kernel/setup.c | 13 ++--- > arch/openrisc/Kconfig | 10 +--- > arch/powerpc/Kconfig | 37 +------------ > arch/powerpc/kernel/prom_init.c | 17 +++--- > arch/riscv/Kconfig | 44 +--------------- > arch/riscv/kernel/setup.c | 5 +- > arch/sh/Kconfig | 30 +---------- > arch/sh/configs/ap325rxa_defconfig | 2 +- > arch/sh/configs/dreamcast_defconfig | 2 +- > arch/sh/configs/ecovec24-romimage_defconfig | 2 +- > arch/sh/configs/ecovec24_defconfig | 2 +- > arch/sh/configs/edosk7760_defconfig | 2 +- > arch/sh/configs/espt_defconfig | 2 +- > arch/sh/configs/j2_defconfig | 2 +- > arch/sh/configs/kfr2r09-romimage_defconfig | 2 +- > arch/sh/configs/kfr2r09_defconfig | 2 +- > arch/sh/configs/lboxre2_defconfig | 2 +- > arch/sh/configs/microdev_defconfig | 2 +- > arch/sh/configs/migor_defconfig | 2 +- > arch/sh/configs/polaris_defconfig | 2 +- > arch/sh/configs/r7780mp_defconfig | 2 +- > arch/sh/configs/r7785rp_defconfig | 2 +- > arch/sh/configs/rsk7201_defconfig | 2 +- > arch/sh/configs/rsk7203_defconfig | 2 +- > arch/sh/configs/rts7751r2d1_defconfig | 2 +- > arch/sh/configs/rts7751r2dplus_defconfig | 2 +- > arch/sh/configs/sdk7780_defconfig | 2 +- > arch/sh/configs/sdk7786_defconfig | 2 +- > arch/sh/configs/se7206_defconfig | 2 +- > arch/sh/configs/se7343_defconfig | 2 +- > arch/sh/configs/se7712_defconfig | 2 +- > arch/sh/configs/se7721_defconfig | 2 +- > arch/sh/configs/se7724_defconfig | 2 +- > arch/sh/configs/se7751_defconfig | 2 +- > arch/sh/configs/se7780_defconfig | 2 +- > arch/sh/configs/sh03_defconfig | 2 +- > arch/sh/configs/sh2007_defconfig | 2 +- > arch/sh/configs/sh7757lcr_defconfig | 2 +- > arch/sh/configs/sh7763rdp_defconfig | 2 +- > arch/sh/configs/shmin_defconfig | 2 +- > arch/sh/configs/shx3_defconfig | 2 +- > arch/sh/configs/titan_defconfig | 2 +- > arch/sh/configs/ul2_defconfig | 2 +- > arch/sh/kernel/setup.c | 11 +--- > arch/sparc/Kconfig | 18 +------ > arch/sparc/prom/bootstr_64.c | 2 +- > arch/x86/Kconfig | 45 +--------------- > arch/x86/kernel/setup.c | 17 +----- > arch/xtensa/Kconfig | 15 +----- > arch/xtensa/configs/audio_kc705_defconfig | 1 - > arch/xtensa/configs/common_defconfig | 1 - > arch/xtensa/configs/generic_kc705_defconfig | 1 - > arch/xtensa/configs/iss_defconfig | 1 - > arch/xtensa/configs/nommu_kc705_defconfig | 1 - > arch/xtensa/configs/smp_lx200_defconfig | 1 - > arch/xtensa/configs/virt_defconfig | 1 - > arch/xtensa/configs/xip_kc705_defconfig | 1 - > arch/xtensa/kernel/setup.c | 10 +--- > drivers/firmware/efi/libstub/x86-stub.c | 26 +++++----- You missed efi-stub.c which has CMDLINE_EXTEND. > drivers/of/fdt.c | 23 ++------- > include/linux/cmdline.h | 57 +++++++++++++++++++++ > init/Kconfig | 46 +++++++++++++++++ > 98 files changed, 209 insertions(+), 580 deletions(-) > create mode 100644 include/linux/cmdline.h > > -- > 2.25.0 >
On Fri, Mar 26, 2021 at 8:20 AM Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > > > > Le 26/03/2021 à 15:08, Andreas Schwab a écrit : > > On Mär 26 2021, Christophe Leroy wrote: > > > >> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > >> index f8f15332caa2..e7c91ee478d1 100644 > >> --- a/arch/riscv/kernel/setup.c > >> +++ b/arch/riscv/kernel/setup.c > >> @@ -20,6 +20,7 @@ > >> #include <linux/swiotlb.h> > >> #include <linux/smp.h> > >> #include <linux/efi.h> > >> +#include <linux/cmdline.h> > >> > >> #include <asm/cpu_ops.h> > >> #include <asm/early_ioremap.h> > >> @@ -228,10 +229,8 @@ static void __init parse_dtb(void) > >> } > >> > >> pr_err("No DTB passed to the kernel\n"); > >> -#ifdef CONFIG_CMDLINE_FORCE > >> - strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE); > >> + cmdline_build(boot_command_line, NULL, COMMAND_LINE_SIZE); > >> pr_info("Forcing kernel command line to: %s\n", boot_command_line); > > > > Shouldn't that message become conditional in some way? > > > > You are right, I did something similar on ARM but looks like I missed it on RISCV. How is this hunk even useful? Under what conditions can you boot without a DTB? Even with a built-in DTB, the DT cmdline handling would be called. Rob
On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > > This code provides architectures with a way to build command line > based on what is built in the kernel and what is handed over by the > bootloader, based on selected compile-time options. Note that I have this patch pending: https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20210316193820.3137-1-alex@ghiti.fr/ It's going to need to be adapted for this. I've held off applying to see if this gets settled. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > v3: > - Addressed comments from Will > - Added capability to have src == dst > --- > include/linux/cmdline.h | 57 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 57 insertions(+) > create mode 100644 include/linux/cmdline.h > > diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h > new file mode 100644 > index 000000000000..dea87edd41be > --- /dev/null > +++ b/include/linux/cmdline.h > @@ -0,0 +1,57 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_CMDLINE_H > +#define _LINUX_CMDLINE_H > + > +#include <linux/string.h> > + > +/* Allow architectures to override strlcat, powerpc can't use strings so early */ > +#ifndef cmdline_strlcat > +#define cmdline_strlcat strlcat > +#endif > + > +/* > + * This function will append or prepend a builtin command line to the command > + * line provided by the bootloader. Kconfig options can be used to alter > + * the behavior of this builtin command line. > + * @dst: The destination of the final appended/prepended string. > + * @src: The starting string or NULL if there isn't one. > + * @len: the length of dest buffer. > + */ > +static __always_inline void __cmdline_build(char *dst, const char *src, size_t len) > +{ > + if (!len || src == dst) > + return; > + > + if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !src) { > + dst[0] = 0; > + cmdline_strlcat(dst, CONFIG_CMDLINE, len); > + return; > + } > + > + if (dst != src) > + dst[0] = 0; > + > + if (IS_ENABLED(CONFIG_CMDLINE_PREPEND)) > + cmdline_strlcat(dst, CONFIG_CMDLINE " ", len); > + > + cmdline_strlcat(dst, src, len); > + > + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) Should be APPEND. > + cmdline_strlcat(dst, " " CONFIG_CMDLINE, len); > +} > + > +#define cmdline_build(dst, src, len) do { \ Perhaps a comment why we need this to be a define. > + char *__c_dst = (dst); \ > + const char *__c_src = (src); \ > + \ > + if (__c_src == __c_dst) { \ > + static char __c_tmp[COMMAND_LINE_SIZE] __initdata = ""; \ > + \ > + cmdline_strlcat(__c_tmp, __c_src, COMMAND_LINE_SIZE); \ > + __cmdline_build(__c_dst, __c_tmp, (len)); \ > + } else { \ > + __cmdline_build(__c_dst, __c_src, (len)); \ > + } \ > +} while (0) > + > +#endif /* _LINUX_CMDLINE_H */ > -- > 2.25.0 >
On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > > This converts the architecture to GENERIC_CMDLINE. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > arch/arm/Kconfig | 38 +---------------------------------- > arch/arm/kernel/atags_parse.c | 15 +++++--------- > 2 files changed, 6 insertions(+), 47 deletions(-) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 5da96f5df48f..67bc75f2da81 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -50,6 +50,7 @@ config ARM > select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY > select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI > select GENERIC_CLOCKEVENTS_BROADCAST if SMP > + select GENERIC_CMDLINE if ATAGS Don't we need this enabled for !ATAGS (i.e. DT boot)? Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE? Rob
Le 26/03/2021 à 16:47, Rob Herring a écrit : > On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy > <christophe.leroy@csgroup.eu> wrote: >> >> This converts the architecture to GENERIC_CMDLINE. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> arch/arm/Kconfig | 38 +---------------------------------- >> arch/arm/kernel/atags_parse.c | 15 +++++--------- >> 2 files changed, 6 insertions(+), 47 deletions(-) >> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig >> index 5da96f5df48f..67bc75f2da81 100644 >> --- a/arch/arm/Kconfig >> +++ b/arch/arm/Kconfig >> @@ -50,6 +50,7 @@ config ARM >> select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY >> select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI >> select GENERIC_CLOCKEVENTS_BROADCAST if SMP >> + select GENERIC_CMDLINE if ATAGS > > Don't we need this enabled for !ATAGS (i.e. DT boot)? > > Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE? > Don't know. Today ARM has: choice prompt "Kernel command line type" if CMDLINE != "" default CMDLINE_FROM_BOOTLOADER depends on ATAGS Christophe
Le 26/03/2021 à 16:42, Rob Herring a écrit : > On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy > <christophe.leroy@csgroup.eu> wrote: >> >> This code provides architectures with a way to build command line >> based on what is built in the kernel and what is handed over by the >> bootloader, based on selected compile-time options. > > Note that I have this patch pending: > > https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20210316193820.3137-1-alex@ghiti.fr/ > > It's going to need to be adapted for this. I've held off applying to > see if this gets settled. good point. Hope we can't have things like option="beautiful weather" > >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> v3: >> - Addressed comments from Will >> - Added capability to have src == dst >> --- >> include/linux/cmdline.h | 57 +++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 57 insertions(+) >> create mode 100644 include/linux/cmdline.h >> >> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h >> new file mode 100644 >> index 000000000000..dea87edd41be >> --- /dev/null >> +++ b/include/linux/cmdline.h >> @@ -0,0 +1,57 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +#ifndef _LINUX_CMDLINE_H >> +#define _LINUX_CMDLINE_H >> + >> +#include <linux/string.h> >> + >> +/* Allow architectures to override strlcat, powerpc can't use strings so early */ >> +#ifndef cmdline_strlcat >> +#define cmdline_strlcat strlcat >> +#endif >> + >> +/* >> + * This function will append or prepend a builtin command line to the command >> + * line provided by the bootloader. Kconfig options can be used to alter >> + * the behavior of this builtin command line. >> + * @dst: The destination of the final appended/prepended string. >> + * @src: The starting string or NULL if there isn't one. >> + * @len: the length of dest buffer. >> + */ >> +static __always_inline void __cmdline_build(char *dst, const char *src, size_t len) >> +{ >> + if (!len || src == dst) >> + return; >> + >> + if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !src) { >> + dst[0] = 0; >> + cmdline_strlcat(dst, CONFIG_CMDLINE, len); >> + return; >> + } >> + >> + if (dst != src) >> + dst[0] = 0; >> + >> + if (IS_ENABLED(CONFIG_CMDLINE_PREPEND)) >> + cmdline_strlcat(dst, CONFIG_CMDLINE " ", len); >> + >> + cmdline_strlcat(dst, src, len); >> + >> + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) > > Should be APPEND. Not yet. For the time being all architectures use EXTEND only. In patch 3 it is changed to: - if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || IS_ENABLED(CONFIG_CMDLINE_APPEND)) Then in last patch, I forgot but I should have done: - if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || IS_ENABLED(CONFIG_CMDLINE_APPEND)) + if (IS_ENABLED(CONFIG_CMDLINE_APPEND)) > >> + cmdline_strlcat(dst, " " CONFIG_CMDLINE, len); >> +} >> + >> +#define cmdline_build(dst, src, len) do { \ > > Perhaps a comment why we need this to be a define. Probably we don't need anymore as I finally decided to use COMMAND_LINE_SIZE instead of 'len' as the size of the temporary buffer. > >> + char *__c_dst = (dst); \ >> + const char *__c_src = (src); \ >> + \ >> + if (__c_src == __c_dst) { \ >> + static char __c_tmp[COMMAND_LINE_SIZE] __initdata = ""; \ >> + \ >> + cmdline_strlcat(__c_tmp, __c_src, COMMAND_LINE_SIZE); \ >> + __cmdline_build(__c_dst, __c_tmp, (len)); \ >> + } else { \ >> + __cmdline_build(__c_dst, __c_src, (len)); \ >> + } \ >> +} while (0) >> + >> +#endif /* _LINUX_CMDLINE_H */ >> -- >> 2.25.0 >> Christophe
+Nico who added the line in question. On Fri, Mar 26, 2021 at 9:50 AM Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > > > > Le 26/03/2021 à 16:47, Rob Herring a écrit : > > On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy > > <christophe.leroy@csgroup.eu> wrote: > >> > >> This converts the architecture to GENERIC_CMDLINE. > >> > >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > >> --- > >> arch/arm/Kconfig | 38 +---------------------------------- > >> arch/arm/kernel/atags_parse.c | 15 +++++--------- > >> 2 files changed, 6 insertions(+), 47 deletions(-) > >> > >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > >> index 5da96f5df48f..67bc75f2da81 100644 > >> --- a/arch/arm/Kconfig > >> +++ b/arch/arm/Kconfig > >> @@ -50,6 +50,7 @@ config ARM > >> select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY > >> select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI > >> select GENERIC_CLOCKEVENTS_BROADCAST if SMP > >> + select GENERIC_CMDLINE if ATAGS > > > > Don't we need this enabled for !ATAGS (i.e. DT boot)? > > > > Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE? > > > > Don't know. > > Today ARM has: > > choice > prompt "Kernel command line type" if CMDLINE != "" > default CMDLINE_FROM_BOOTLOADER > depends on ATAGS I think that's a mistake. In a DT only case (no ATAGS), we'll get different behaviour (in fdt.c) depending if CONFIG_ATAGS is enabled or not. Note that at the time (2012) the above was added, the DT code only supported CONFIG_CMDLINE and CONFIG_CMDLINE_FORCE. CONFIG_CMDLINE_EXTEND was only added in 2016. And that has different behavior for ATAGS vs. DT. In summary, it's a mess. We should drop the depends either before this patch or just as part of this patch IMO. I'd go with the latter given CONFIG_ATAGS is default y and enabled for common configs. Without that, it looks like CONFIG_CMDLINE disappears from menuconfig for at91_dt_defconfig. Also, I think this code should be refactored a bit to eliminate default_command_line. Instead, we should just save a pointer to the ATAGS command line string, and then call cmdline_build here instead of doing the extra copy: /* parse_early_param needs a boot_command_line */ strlcpy(boot_command_line, from, COMMAND_LINE_SIZE); Rob
> -----Original Message----- > From: Christophe Leroy <christophe.leroy@csgroup.eu> > Sent: Friday, March 26, 2021 9:45 PM > To: will@kernel.org; danielwa@cisco.com; robh@kernel.org; > daniel@gimpelevich.san-francisco.ca.us > Cc: linux-arch@vger.kernel.org; devicetree@vger.kernel.org; linuxppc- > dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; linuxppc- > dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org; microblaze > <monstr@monstr.eu>; linux-mips@vger.kernel.org; Tan, Ley Foon > <ley.foon.tan@intel.com>; openrisc@lists.librecores.org; linux- > hexagon@vger.kernel.org; linux-riscv@lists.infradead.org; x86@kernel.org; > linux-xtensa@linux-xtensa.org; linux-sh@vger.kernel.org; > sparclinux@vger.kernel.org > Subject: [PATCH v3 09/17] nios2: Convert to GENERIC_CMDLINE > > This converts the architecture to GENERIC_CMDLINE. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > arch/nios2/Kconfig | 24 +----------------------- > arch/nios2/kernel/setup.c | 13 ++++--------- > 2 files changed, 5 insertions(+), 32 deletions(-) > > diff --git a/arch/nios2/Kconfig b/arch/nios2/Kconfig index > c24955c81c92..f66c97b15813 100644 > --- a/arch/nios2/Kconfig > +++ b/arch/nios2/Kconfig > @@ -90,31 +90,9 @@ config NIOS2_ALIGNMENT_TRAP > > comment "Boot options" > > -config CMDLINE_BOOL > - bool "Default bootloader kernel arguments" > - default y > - > -config CMDLINE > - string "Default kernel command string" > - default "" > - depends on CMDLINE_BOOL > - 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 > - other cases you can specify kernel args so that you don't have > - to set them up in board prom initialization routines. > - > -config CMDLINE_FORCE > - bool "Force default kernel command string" > - depends on CMDLINE_BOOL > - help > - Set this to have arguments from the default kernel command string > - override those passed by the boot loader. > - > config NIOS2_CMDLINE_IGNORE_DTB > bool "Ignore kernel command string from DTB" > - depends on CMDLINE_BOOL > + depends on CMDLINE != "" > depends on !CMDLINE_FORCE > default y > help Missing " select GENERIC_CMDLINE" ? > diff --git a/arch/nios2/kernel/setup.c b/arch/nios2/kernel/setup.c index > d2f21957e99c..42464f457a6d 100644 > --- a/arch/nios2/kernel/setup.c > +++ b/arch/nios2/kernel/setup.c > @@ -20,6 +20,7 @@ > #include <linux/initrd.h> > #include <linux/of_fdt.h> > #include <linux/screen_info.h> > +#include <linux/cmdline.h> > > #include <asm/mmu_context.h> > #include <asm/sections.h> > @@ -108,7 +109,7 @@ asmlinkage void __init nios2_boot_init(unsigned r4, > unsigned r5, unsigned r6, > unsigned r7) > { > unsigned dtb_passed = 0; > - char cmdline_passed[COMMAND_LINE_SIZE] __maybe_unused = > { 0, }; > + char cmdline_passed[COMMAND_LINE_SIZE] = { 0, }; > > #if defined(CONFIG_NIOS2_PASS_CMDLINE) > if (r4 == 0x534f494e) { /* r4 is magic NIOS */ @@ -127,14 +128,8 @@ > asmlinkage void __init nios2_boot_init(unsigned r4, unsigned r5, unsigned r6, > > early_init_devtree((void *)dtb_passed); > > -#ifndef CONFIG_CMDLINE_FORCE > - if (cmdline_passed[0]) > - strlcpy(boot_command_line, cmdline_passed, > COMMAND_LINE_SIZE); > -#ifdef CONFIG_NIOS2_CMDLINE_IGNORE_DTB > - else > - strlcpy(boot_command_line, CONFIG_CMDLINE, > COMMAND_LINE_SIZE); > -#endif > -#endif > + if (cmdline_passed[0] || > IS_ENABLED(CONFIG_NIOS2_CMDLINE_IGNORE_DTB)) > + cmdline_build(boot_command_line, cmdline_passed, > COMMAND_LINE_SIZE); > > parse_early_param(); > } > -- > 2.25.0
Στις 2021-03-26 17:26, Rob Herring έγραψε: > On Fri, Mar 26, 2021 at 8:20 AM Christophe Leroy > <christophe.leroy@csgroup.eu> wrote: >> >> >> >> Le 26/03/2021 à 15:08, Andreas Schwab a écrit : >> > On Mär 26 2021, Christophe Leroy wrote: >> > >> >> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c >> >> index f8f15332caa2..e7c91ee478d1 100644 >> >> --- a/arch/riscv/kernel/setup.c >> >> +++ b/arch/riscv/kernel/setup.c >> >> @@ -20,6 +20,7 @@ >> >> #include <linux/swiotlb.h> >> >> #include <linux/smp.h> >> >> #include <linux/efi.h> >> >> +#include <linux/cmdline.h> >> >> >> >> #include <asm/cpu_ops.h> >> >> #include <asm/early_ioremap.h> >> >> @@ -228,10 +229,8 @@ static void __init parse_dtb(void) >> >> } >> >> >> >> pr_err("No DTB passed to the kernel\n"); >> >> -#ifdef CONFIG_CMDLINE_FORCE >> >> - strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE); >> >> + cmdline_build(boot_command_line, NULL, COMMAND_LINE_SIZE); >> >> pr_info("Forcing kernel command line to: %s\n", boot_command_line); >> > >> > Shouldn't that message become conditional in some way? >> > >> >> You are right, I did something similar on ARM but looks like I missed >> it on RISCV. > > How is this hunk even useful? Under what conditions can you boot > without a DTB? Even with a built-in DTB, the DT cmdline handling would > be called. > > Rob > cced Paul who introduced this: https://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git/commit/arch/riscv/kernel/setup.c?id=8fd6e05c7463b635e51ec7df0a1858c1b5a6e350
On Fri, Mar 26, 2021 at 01:44:48PM +0000, Christophe Leroy wrote: > This code provides architectures with a way to build command line > based on what is built in the kernel and what is handed over by the > bootloader, based on selected compile-time options. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > v3: > - Addressed comments from Will > - Added capability to have src == dst > --- > include/linux/cmdline.h | 57 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 57 insertions(+) > create mode 100644 include/linux/cmdline.h > > diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h > new file mode 100644 > index 000000000000..dea87edd41be > --- /dev/null > +++ b/include/linux/cmdline.h > @@ -0,0 +1,57 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_CMDLINE_H > +#define _LINUX_CMDLINE_H > + > +#include <linux/string.h> > + > +/* Allow architectures to override strlcat, powerpc can't use strings so early */ > +#ifndef cmdline_strlcat > +#define cmdline_strlcat strlcat > +#endif > + > +/* > + * This function will append or prepend a builtin command line to the command > + * line provided by the bootloader. Kconfig options can be used to alter > + * the behavior of this builtin command line. > + * @dst: The destination of the final appended/prepended string. > + * @src: The starting string or NULL if there isn't one. > + * @len: the length of dest buffer. > + */ Append or prepend ? Cisco requires both at the same time. This is why my implementation provides both. I can't use this with both at once. Daniel
> Am 30.03.2021 um 19:27 schrieb Daniel Walker <danielwa@cisco.com>: > > On Fri, Mar 26, 2021 at 01:44:48PM +0000, Christophe Leroy wrote: >> This code provides architectures with a way to build command line >> based on what is built in the kernel and what is handed over by the >> bootloader, based on selected compile-time options. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> v3: >> - Addressed comments from Will >> - Added capability to have src == dst >> --- >> include/linux/cmdline.h | 57 +++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 57 insertions(+) >> create mode 100644 include/linux/cmdline.h >> >> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h >> new file mode 100644 >> index 000000000000..dea87edd41be >> --- /dev/null >> +++ b/include/linux/cmdline.h >> @@ -0,0 +1,57 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +#ifndef _LINUX_CMDLINE_H >> +#define _LINUX_CMDLINE_H >> + >> +#include <linux/string.h> >> + >> +/* Allow architectures to override strlcat, powerpc can't use strings so early */ >> +#ifndef cmdline_strlcat >> +#define cmdline_strlcat strlcat >> +#endif >> + >> +/* >> + * This function will append or prepend a builtin command line to the command >> + * line provided by the bootloader. Kconfig options can be used to alter >> + * the behavior of this builtin command line. >> + * @dst: The destination of the final appended/prepended string. >> + * @src: The starting string or NULL if there isn't one. >> + * @len: the length of dest buffer. >> + */ > > Append or prepend ? Cisco requires both at the same time. This is why my > implementation provides both. I can't use this with both at once. Just an idea: what about defining CMDLINE as a pattern where e.g. "$$" or "%%" is replaced by the boot loader command line? Then you can formulate replace, prepend, append, prepend+append with a single CONFIG setting. It may be a little more complex in code (scanning for the pattern and replacing it and take care to temporary memory) but IMHO it could be worth to consider. BR, Nikolaus Schaller
On Tue, Mar 30, 2021 at 08:07:30PM +0200, H. Nikolaus Schaller wrote: > > > Am 30.03.2021 um 19:27 schrieb Daniel Walker <danielwa@cisco.com>: > > > > On Fri, Mar 26, 2021 at 01:44:48PM +0000, Christophe Leroy wrote: > >> This code provides architectures with a way to build command line > >> based on what is built in the kernel and what is handed over by the > >> bootloader, based on selected compile-time options. > >> > >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > >> --- > >> v3: > >> - Addressed comments from Will > >> - Added capability to have src == dst > >> --- > >> include/linux/cmdline.h | 57 +++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 57 insertions(+) > >> create mode 100644 include/linux/cmdline.h > >> > >> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h > >> new file mode 100644 > >> index 000000000000..dea87edd41be > >> --- /dev/null > >> +++ b/include/linux/cmdline.h > >> @@ -0,0 +1,57 @@ > >> +/* SPDX-License-Identifier: GPL-2.0 */ > >> +#ifndef _LINUX_CMDLINE_H > >> +#define _LINUX_CMDLINE_H > >> + > >> +#include <linux/string.h> > >> + > >> +/* Allow architectures to override strlcat, powerpc can't use strings so early */ > >> +#ifndef cmdline_strlcat > >> +#define cmdline_strlcat strlcat > >> +#endif > >> + > >> +/* > >> + * This function will append or prepend a builtin command line to the command > >> + * line provided by the bootloader. Kconfig options can be used to alter > >> + * the behavior of this builtin command line. > >> + * @dst: The destination of the final appended/prepended string. > >> + * @src: The starting string or NULL if there isn't one. > >> + * @len: the length of dest buffer. > >> + */ > > > > Append or prepend ? Cisco requires both at the same time. This is why my > > implementation provides both. I can't use this with both at once. > > Just an idea: what about defining CMDLINE as a pattern where e.g. "$$" or "%%" > is replaced by the boot loader command line? > > Then you can formulate replace, prepend, append, prepend+append with a single > CONFIG setting. > > It may be a little more complex in code (scanning for the pattern and replacing > it and take care to temporary memory) but IMHO it could be worth to consider. In some cases this code could be used extremely early in boot up. For example in the prom_init.c powerpc code, or in efi changes. The flexibility to find and replace like that is not always an option due to the nature of the environment. It's not impossible of course. Daniel
Le 26/03/2021 à 16:47, Rob Herring a écrit : > On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy > <christophe.leroy@csgroup.eu> wrote: >> >> This converts the architecture to GENERIC_CMDLINE. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> arch/arm/Kconfig | 38 +---------------------------------- >> arch/arm/kernel/atags_parse.c | 15 +++++--------- >> 2 files changed, 6 insertions(+), 47 deletions(-) >> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig >> index 5da96f5df48f..67bc75f2da81 100644 >> --- a/arch/arm/Kconfig >> +++ b/arch/arm/Kconfig >> @@ -50,6 +50,7 @@ config ARM >> select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY >> select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI >> select GENERIC_CLOCKEVENTS_BROADCAST if SMP >> + select GENERIC_CMDLINE if ATAGS > > Don't we need this enabled for !ATAGS (i.e. DT boot)? > > Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE? > Done in v4
Le 26/03/2021 à 16:04, Rob Herring a écrit : > On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy > <christophe.leroy@csgroup.eu> wrote: >> >> The purpose of this series is to improve and enhance the >> handling of kernel boot arguments. >> >> It is first focussed on powerpc but also extends the capability >> for other arches. >> >> This is based on suggestion from Daniel Walker <danielwa@cisco.com> >> >> Main changes in V3: >> - Also accept destination equal to source in cmdline_build() by setting a tmp buffer in __initdata. Powerpc provides different source and destination and call __cmdline_build() directly. >> - Taken comments received from Will and Rob >> - Converted all architectures (Only tested on powerpc) >> >> Christophe Leroy (17): >> cmdline: Add generic function to build command line. >> drivers: of: use cmdline building function >> cmdline: Gives architectures opportunity to use generically defined >> boot cmdline manipulation >> powerpc: Convert to GENERIC_CMDLINE >> arm: Convert to GENERIC_CMDLINE >> arm64: Convert to GENERIC_CMDLINE >> hexagon: Convert to GENERIC_CMDLINE >> microblaze: Convert to GENERIC_CMDLINE >> nios2: Convert to GENERIC_CMDLINE >> openrisc: Convert to GENERIC_CMDLINE >> riscv: Convert to GENERIC_CMDLINE >> sh: Convert to GENERIC_CMDLINE >> sparc: Convert to GENERIC_CMDLINE >> xtensa: Convert to GENERIC_CMDLINE >> x86: Convert to GENERIC_CMDLINE >> mips: Convert to GENERIC_CMDLINE >> cmdline: Remove CONFIG_CMDLINE_EXTEND >> >> arch/arm/Kconfig | 38 +------------- >> arch/arm/kernel/atags_parse.c | 15 ++---- >> arch/arm64/Kconfig | 33 +----------- >> arch/arm64/kernel/idreg-override.c | 9 ++-- >> arch/hexagon/Kconfig | 11 +--- >> arch/hexagon/kernel/setup.c | 10 +--- >> arch/microblaze/Kconfig | 24 +-------- >> arch/microblaze/configs/mmu_defconfig | 2 +- >> arch/microblaze/kernel/head.S | 4 +- >> arch/mips/Kconfig | 1 + >> arch/mips/Kconfig.debug | 44 ---------------- >> arch/mips/configs/ar7_defconfig | 1 - >> arch/mips/configs/bcm47xx_defconfig | 1 - >> arch/mips/configs/bcm63xx_defconfig | 1 - >> arch/mips/configs/bmips_be_defconfig | 1 - >> arch/mips/configs/bmips_stb_defconfig | 1 - >> arch/mips/configs/capcella_defconfig | 1 - >> arch/mips/configs/ci20_defconfig | 1 - >> arch/mips/configs/cu1000-neo_defconfig | 1 - >> arch/mips/configs/cu1830-neo_defconfig | 1 - >> arch/mips/configs/e55_defconfig | 1 - >> arch/mips/configs/generic_defconfig | 1 - >> arch/mips/configs/gpr_defconfig | 1 - >> arch/mips/configs/loongson3_defconfig | 1 - >> arch/mips/configs/mpc30x_defconfig | 1 - >> arch/mips/configs/rt305x_defconfig | 1 - >> arch/mips/configs/tb0219_defconfig | 1 - >> arch/mips/configs/tb0226_defconfig | 1 - >> arch/mips/configs/tb0287_defconfig | 1 - >> arch/mips/configs/workpad_defconfig | 1 - >> arch/mips/configs/xway_defconfig | 1 - >> arch/mips/kernel/relocate.c | 4 +- >> arch/mips/kernel/setup.c | 40 +-------------- >> arch/mips/pic32/pic32mzda/early_console.c | 2 +- >> arch/mips/pic32/pic32mzda/init.c | 2 - >> arch/nios2/Kconfig | 24 +-------- >> arch/nios2/kernel/setup.c | 13 ++--- >> arch/openrisc/Kconfig | 10 +--- >> arch/powerpc/Kconfig | 37 +------------ >> arch/powerpc/kernel/prom_init.c | 17 +++--- >> arch/riscv/Kconfig | 44 +--------------- >> arch/riscv/kernel/setup.c | 5 +- >> arch/sh/Kconfig | 30 +---------- >> arch/sh/configs/ap325rxa_defconfig | 2 +- >> arch/sh/configs/dreamcast_defconfig | 2 +- >> arch/sh/configs/ecovec24-romimage_defconfig | 2 +- >> arch/sh/configs/ecovec24_defconfig | 2 +- >> arch/sh/configs/edosk7760_defconfig | 2 +- >> arch/sh/configs/espt_defconfig | 2 +- >> arch/sh/configs/j2_defconfig | 2 +- >> arch/sh/configs/kfr2r09-romimage_defconfig | 2 +- >> arch/sh/configs/kfr2r09_defconfig | 2 +- >> arch/sh/configs/lboxre2_defconfig | 2 +- >> arch/sh/configs/microdev_defconfig | 2 +- >> arch/sh/configs/migor_defconfig | 2 +- >> arch/sh/configs/polaris_defconfig | 2 +- >> arch/sh/configs/r7780mp_defconfig | 2 +- >> arch/sh/configs/r7785rp_defconfig | 2 +- >> arch/sh/configs/rsk7201_defconfig | 2 +- >> arch/sh/configs/rsk7203_defconfig | 2 +- >> arch/sh/configs/rts7751r2d1_defconfig | 2 +- >> arch/sh/configs/rts7751r2dplus_defconfig | 2 +- >> arch/sh/configs/sdk7780_defconfig | 2 +- >> arch/sh/configs/sdk7786_defconfig | 2 +- >> arch/sh/configs/se7206_defconfig | 2 +- >> arch/sh/configs/se7343_defconfig | 2 +- >> arch/sh/configs/se7712_defconfig | 2 +- >> arch/sh/configs/se7721_defconfig | 2 +- >> arch/sh/configs/se7724_defconfig | 2 +- >> arch/sh/configs/se7751_defconfig | 2 +- >> arch/sh/configs/se7780_defconfig | 2 +- >> arch/sh/configs/sh03_defconfig | 2 +- >> arch/sh/configs/sh2007_defconfig | 2 +- >> arch/sh/configs/sh7757lcr_defconfig | 2 +- >> arch/sh/configs/sh7763rdp_defconfig | 2 +- >> arch/sh/configs/shmin_defconfig | 2 +- >> arch/sh/configs/shx3_defconfig | 2 +- >> arch/sh/configs/titan_defconfig | 2 +- >> arch/sh/configs/ul2_defconfig | 2 +- >> arch/sh/kernel/setup.c | 11 +--- >> arch/sparc/Kconfig | 18 +------ >> arch/sparc/prom/bootstr_64.c | 2 +- >> arch/x86/Kconfig | 45 +--------------- >> arch/x86/kernel/setup.c | 17 +----- >> arch/xtensa/Kconfig | 15 +----- >> arch/xtensa/configs/audio_kc705_defconfig | 1 - >> arch/xtensa/configs/common_defconfig | 1 - >> arch/xtensa/configs/generic_kc705_defconfig | 1 - >> arch/xtensa/configs/iss_defconfig | 1 - >> arch/xtensa/configs/nommu_kc705_defconfig | 1 - >> arch/xtensa/configs/smp_lx200_defconfig | 1 - >> arch/xtensa/configs/virt_defconfig | 1 - >> arch/xtensa/configs/xip_kc705_defconfig | 1 - >> arch/xtensa/kernel/setup.c | 10 +--- >> drivers/firmware/efi/libstub/x86-stub.c | 26 +++++----- > > You missed efi-stub.c which has CMDLINE_EXTEND. > I think I completely missed EFI. Reworked in V4.
Le 26/03/2021 à 16:26, Rob Herring a écrit : > On Fri, Mar 26, 2021 at 8:20 AM Christophe Leroy > <christophe.leroy@csgroup.eu> wrote: >> >> >> >> Le 26/03/2021 à 15:08, Andreas Schwab a écrit : >>> On Mär 26 2021, Christophe Leroy wrote: >>> >>>> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c >>>> index f8f15332caa2..e7c91ee478d1 100644 >>>> --- a/arch/riscv/kernel/setup.c >>>> +++ b/arch/riscv/kernel/setup.c >>>> @@ -20,6 +20,7 @@ >>>> #include <linux/swiotlb.h> >>>> #include <linux/smp.h> >>>> #include <linux/efi.h> >>>> +#include <linux/cmdline.h> >>>> >>>> #include <asm/cpu_ops.h> >>>> #include <asm/early_ioremap.h> >>>> @@ -228,10 +229,8 @@ static void __init parse_dtb(void) >>>> } >>>> >>>> pr_err("No DTB passed to the kernel\n"); >>>> -#ifdef CONFIG_CMDLINE_FORCE >>>> - strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE); >>>> + cmdline_build(boot_command_line, NULL, COMMAND_LINE_SIZE); >>>> pr_info("Forcing kernel command line to: %s\n", boot_command_line); >>> >>> Shouldn't that message become conditional in some way? >>> >> >> You are right, I did something similar on ARM but looks like I missed it on RISCV. > > How is this hunk even useful? Under what conditions can you boot > without a DTB? Even with a built-in DTB, the DT cmdline handling would > be called. > Don't know, I wanted to keep as is today. Do you think the hunk should be completely removed ?
Le 26/03/2021 à 16:42, Rob Herring a écrit : > On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy > <christophe.leroy@csgroup.eu> wrote: >> >> This code provides architectures with a way to build command line >> based on what is built in the kernel and what is handed over by the >> bootloader, based on selected compile-time options. > > Note that I have this patch pending: > > https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20210316193820.3137-1-alex@ghiti.fr/ > > It's going to need to be adapted for this. I've held off applying to > see if this gets settled. When reworking EFI, I found out that they are a similar handling, which in addition takes care of space inside quotes. I added something similar now in cmdline_build() function. > >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> v3: >> - Addressed comments from Will >> - Added capability to have src == dst >> --- >> include/linux/cmdline.h | 57 +++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 57 insertions(+) >> create mode 100644 include/linux/cmdline.h >> >> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h >> new file mode 100644 >> index 000000000000..dea87edd41be >> --- /dev/null >> +++ b/include/linux/cmdline.h >> @@ -0,0 +1,57 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +#ifndef _LINUX_CMDLINE_H >> +#define _LINUX_CMDLINE_H >> + >> +#include <linux/string.h> >> + >> +/* Allow architectures to override strlcat, powerpc can't use strings so early */ >> +#ifndef cmdline_strlcat >> +#define cmdline_strlcat strlcat >> +#endif >> + >> +/* >> + * This function will append or prepend a builtin command line to the command >> + * line provided by the bootloader. Kconfig options can be used to alter >> + * the behavior of this builtin command line. >> + * @dst: The destination of the final appended/prepended string. >> + * @src: The starting string or NULL if there isn't one. >> + * @len: the length of dest buffer. >> + */ >> +static __always_inline void __cmdline_build(char *dst, const char *src, size_t len) >> +{ >> + if (!len || src == dst) >> + return; >> + >> + if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !src) { >> + dst[0] = 0; >> + cmdline_strlcat(dst, CONFIG_CMDLINE, len); >> + return; >> + } >> + >> + if (dst != src) >> + dst[0] = 0; >> + >> + if (IS_ENABLED(CONFIG_CMDLINE_PREPEND)) >> + cmdline_strlcat(dst, CONFIG_CMDLINE " ", len); >> + >> + cmdline_strlcat(dst, src, len); >> + >> + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) > > Should be APPEND. > >> + cmdline_strlcat(dst, " " CONFIG_CMDLINE, len); >> +} >> + >> +#define cmdline_build(dst, src, len) do { \ > > Perhaps a comment why we need this to be a define. > >> + char *__c_dst = (dst); \ >> + const char *__c_src = (src); \ >> + \ >> + if (__c_src == __c_dst) { \ >> + static char __c_tmp[COMMAND_LINE_SIZE] __initdata = ""; \ >> + \ >> + cmdline_strlcat(__c_tmp, __c_src, COMMAND_LINE_SIZE); \ >> + __cmdline_build(__c_dst, __c_tmp, (len)); \ >> + } else { \ >> + __cmdline_build(__c_dst, __c_src, (len)); \ >> + } \ >> +} while (0) >> + >> +#endif /* _LINUX_CMDLINE_H */ >> -- >> 2.25.0 >>
Le 26/03/2021 à 16:47, Rob Herring a écrit : > On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy > <christophe.leroy@csgroup.eu> wrote: >> >> This converts the architecture to GENERIC_CMDLINE. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> arch/arm/Kconfig | 38 +---------------------------------- >> arch/arm/kernel/atags_parse.c | 15 +++++--------- >> 2 files changed, 6 insertions(+), 47 deletions(-) >> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig >> index 5da96f5df48f..67bc75f2da81 100644 >> --- a/arch/arm/Kconfig >> +++ b/arch/arm/Kconfig >> @@ -50,6 +50,7 @@ config ARM >> select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY >> select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI >> select GENERIC_CLOCKEVENTS_BROADCAST if SMP >> + select GENERIC_CMDLINE if ATAGS > > Don't we need this enabled for !ATAGS (i.e. DT boot)? > > Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE? > Yes missed it in v3 after your comment on v2. Done in v4.
Le 30/03/2021 à 19:27, Daniel Walker a écrit : > On Fri, Mar 26, 2021 at 01:44:48PM +0000, Christophe Leroy wrote: >> This code provides architectures with a way to build command line >> based on what is built in the kernel and what is handed over by the >> bootloader, based on selected compile-time options. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> v3: >> - Addressed comments from Will >> - Added capability to have src == dst >> --- >> include/linux/cmdline.h | 57 +++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 57 insertions(+) >> create mode 100644 include/linux/cmdline.h >> >> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h >> new file mode 100644 >> index 000000000000..dea87edd41be >> --- /dev/null >> +++ b/include/linux/cmdline.h >> @@ -0,0 +1,57 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +#ifndef _LINUX_CMDLINE_H >> +#define _LINUX_CMDLINE_H >> + >> +#include <linux/string.h> >> + >> +/* Allow architectures to override strlcat, powerpc can't use strings so early */ >> +#ifndef cmdline_strlcat >> +#define cmdline_strlcat strlcat >> +#endif >> + >> +/* >> + * This function will append or prepend a builtin command line to the command >> + * line provided by the bootloader. Kconfig options can be used to alter >> + * the behavior of this builtin command line. >> + * @dst: The destination of the final appended/prepended string. >> + * @src: The starting string or NULL if there isn't one. >> + * @len: the length of dest buffer. >> + */ > > Append or prepend ? Cisco requires both at the same time. This is why my > implementation provides both. I can't use this with both at once. > I think it can be added as a second step if dimmed necessary. The feeling I have from all the discussion is that it's not what people from the community are looking for at the moment. Anyway, once all architectures are moved to generic handling, I believe it is then easier to split CONFIG_CMDLINE in two configuration items in order to provide both appending and prepending at the same time. I see some concerns about risk of double changes, but I have focussed in changing as little as possible the existing configuration items, in order to minimise that.
Le 29/03/2021 à 03:35, Tan, Ley Foon a écrit : > > >> -----Original Message----- >> From: Christophe Leroy <christophe.leroy@csgroup.eu> >> Sent: Friday, March 26, 2021 9:45 PM >> To: will@kernel.org; danielwa@cisco.com; robh@kernel.org; >> daniel@gimpelevich.san-francisco.ca.us >> Cc: linux-arch@vger.kernel.org; devicetree@vger.kernel.org; linuxppc- >> dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; linuxppc- >> dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org; microblaze >> <monstr@monstr.eu>; linux-mips@vger.kernel.org; Tan, Ley Foon >> <ley.foon.tan@intel.com>; openrisc@lists.librecores.org; linux- >> hexagon@vger.kernel.org; linux-riscv@lists.infradead.org; x86@kernel.org; >> linux-xtensa@linux-xtensa.org; linux-sh@vger.kernel.org; >> sparclinux@vger.kernel.org >> Subject: [PATCH v3 09/17] nios2: Convert to GENERIC_CMDLINE >> >> This converts the architecture to GENERIC_CMDLINE. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> arch/nios2/Kconfig | 24 +----------------------- >> arch/nios2/kernel/setup.c | 13 ++++--------- >> 2 files changed, 5 insertions(+), 32 deletions(-) >> >> diff --git a/arch/nios2/Kconfig b/arch/nios2/Kconfig index >> c24955c81c92..f66c97b15813 100644 >> --- a/arch/nios2/Kconfig >> +++ b/arch/nios2/Kconfig >> @@ -90,31 +90,9 @@ config NIOS2_ALIGNMENT_TRAP >> >> comment "Boot options" >> >> -config CMDLINE_BOOL >> - bool "Default bootloader kernel arguments" >> - default y >> - >> -config CMDLINE >> - string "Default kernel command string" >> - default "" >> - depends on CMDLINE_BOOL >> - 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 >> - other cases you can specify kernel args so that you don't have >> - to set them up in board prom initialization routines. >> - >> -config CMDLINE_FORCE >> - bool "Force default kernel command string" >> - depends on CMDLINE_BOOL >> - help >> - Set this to have arguments from the default kernel command string >> - override those passed by the boot loader. >> - >> config NIOS2_CMDLINE_IGNORE_DTB >> bool "Ignore kernel command string from DTB" >> - depends on CMDLINE_BOOL >> + depends on CMDLINE != "" >> depends on !CMDLINE_FORCE >> default y >> help > > Missing " select GENERIC_CMDLINE" ? Added in v4 Thanks Christophe
Le 26/03/2021 à 14:44, Christophe Leroy a écrit : > This converts the architecture to GENERIC_CMDLINE. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig > index e798e55915c2..fab84f62448c 100644 > --- a/arch/sh/Kconfig > +++ b/arch/sh/Kconfig > @@ -16,6 +16,7 @@ config SUPERH > select CPU_NO_EFFICIENT_FFS > select DMA_DECLARE_COHERENT > select GENERIC_ATOMIC64 > + select GENERIC_CMDLINE > select GENERIC_CMOS_UPDATE if SH_SH03 || SH_DREAMCAST > select GENERIC_IDLE_POLL_SETUP > select GENERIC_IRQ_SHOW > @@ -742,35 +743,6 @@ config ROMIMAGE_MMCIF > first part of the romImage which in turn loads the rest the kernel > image to RAM using the MMCIF hardware block. > > -choice > - prompt "Kernel command line" > - optional > - default CMDLINE_OVERWRITE > - help > - Setting this option allows the kernel command line arguments > - to be set. > - > -config CMDLINE_OVERWRITE > - bool "Overwrite bootloader kernel arguments" > - help > - Given string will overwrite any arguments passed in by > - a bootloader. > - > -config CMDLINE_EXTEND > - bool "Extend bootloader kernel arguments" > - help > - Given string will be concatenated with arguments passed in > - by a bootloader. > - > -endchoice > - > -config CMDLINE > - string "Kernel command line arguments string" > - depends on CMDLINE_OVERWRITE || CMDLINE_EXTEND > - default "console=ttySC1,115200" > - > -endmenu > - That "endmenu" shall not be removed. Fixed in v4, Thanks to Rob L. for the report. Christophe > menu "Bus options" > > config SUPERHYWAY
The purpose of this series is to improve and enhance the handling of kernel boot arguments. It is first focussed on powerpc but also extends the capability for other arches. This is based on suggestion from Daniel Walker <danielwa@cisco.com> Main changes in V3: - Also accept destination equal to source in cmdline_build() by setting a tmp buffer in __initdata. Powerpc provides different source and destination and call __cmdline_build() directly. - Taken comments received from Will and Rob - Converted all architectures (Only tested on powerpc) Christophe Leroy (17): cmdline: Add generic function to build command line. drivers: of: use cmdline building function cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation powerpc: Convert to GENERIC_CMDLINE arm: Convert to GENERIC_CMDLINE arm64: Convert to GENERIC_CMDLINE hexagon: Convert to GENERIC_CMDLINE microblaze: Convert to GENERIC_CMDLINE nios2: Convert to GENERIC_CMDLINE openrisc: Convert to GENERIC_CMDLINE riscv: Convert to GENERIC_CMDLINE sh: Convert to GENERIC_CMDLINE sparc: Convert to GENERIC_CMDLINE xtensa: Convert to GENERIC_CMDLINE x86: Convert to GENERIC_CMDLINE mips: Convert to GENERIC_CMDLINE cmdline: Remove CONFIG_CMDLINE_EXTEND arch/arm/Kconfig | 38 +------------- arch/arm/kernel/atags_parse.c | 15 ++---- arch/arm64/Kconfig | 33 +----------- arch/arm64/kernel/idreg-override.c | 9 ++-- arch/hexagon/Kconfig | 11 +--- arch/hexagon/kernel/setup.c | 10 +--- arch/microblaze/Kconfig | 24 +-------- arch/microblaze/configs/mmu_defconfig | 2 +- arch/microblaze/kernel/head.S | 4 +- arch/mips/Kconfig | 1 + arch/mips/Kconfig.debug | 44 ---------------- arch/mips/configs/ar7_defconfig | 1 - arch/mips/configs/bcm47xx_defconfig | 1 - arch/mips/configs/bcm63xx_defconfig | 1 - arch/mips/configs/bmips_be_defconfig | 1 - arch/mips/configs/bmips_stb_defconfig | 1 - arch/mips/configs/capcella_defconfig | 1 - arch/mips/configs/ci20_defconfig | 1 - arch/mips/configs/cu1000-neo_defconfig | 1 - arch/mips/configs/cu1830-neo_defconfig | 1 - arch/mips/configs/e55_defconfig | 1 - arch/mips/configs/generic_defconfig | 1 - arch/mips/configs/gpr_defconfig | 1 - arch/mips/configs/loongson3_defconfig | 1 - arch/mips/configs/mpc30x_defconfig | 1 - arch/mips/configs/rt305x_defconfig | 1 - arch/mips/configs/tb0219_defconfig | 1 - arch/mips/configs/tb0226_defconfig | 1 - arch/mips/configs/tb0287_defconfig | 1 - arch/mips/configs/workpad_defconfig | 1 - arch/mips/configs/xway_defconfig | 1 - arch/mips/kernel/relocate.c | 4 +- arch/mips/kernel/setup.c | 40 +-------------- arch/mips/pic32/pic32mzda/early_console.c | 2 +- arch/mips/pic32/pic32mzda/init.c | 2 - arch/nios2/Kconfig | 24 +-------- arch/nios2/kernel/setup.c | 13 ++--- arch/openrisc/Kconfig | 10 +--- arch/powerpc/Kconfig | 37 +------------ arch/powerpc/kernel/prom_init.c | 17 +++--- arch/riscv/Kconfig | 44 +--------------- arch/riscv/kernel/setup.c | 5 +- arch/sh/Kconfig | 30 +---------- arch/sh/configs/ap325rxa_defconfig | 2 +- arch/sh/configs/dreamcast_defconfig | 2 +- arch/sh/configs/ecovec24-romimage_defconfig | 2 +- arch/sh/configs/ecovec24_defconfig | 2 +- arch/sh/configs/edosk7760_defconfig | 2 +- arch/sh/configs/espt_defconfig | 2 +- arch/sh/configs/j2_defconfig | 2 +- arch/sh/configs/kfr2r09-romimage_defconfig | 2 +- arch/sh/configs/kfr2r09_defconfig | 2 +- arch/sh/configs/lboxre2_defconfig | 2 +- arch/sh/configs/microdev_defconfig | 2 +- arch/sh/configs/migor_defconfig | 2 +- arch/sh/configs/polaris_defconfig | 2 +- arch/sh/configs/r7780mp_defconfig | 2 +- arch/sh/configs/r7785rp_defconfig | 2 +- arch/sh/configs/rsk7201_defconfig | 2 +- arch/sh/configs/rsk7203_defconfig | 2 +- arch/sh/configs/rts7751r2d1_defconfig | 2 +- arch/sh/configs/rts7751r2dplus_defconfig | 2 +- arch/sh/configs/sdk7780_defconfig | 2 +- arch/sh/configs/sdk7786_defconfig | 2 +- arch/sh/configs/se7206_defconfig | 2 +- arch/sh/configs/se7343_defconfig | 2 +- arch/sh/configs/se7712_defconfig | 2 +- arch/sh/configs/se7721_defconfig | 2 +- arch/sh/configs/se7724_defconfig | 2 +- arch/sh/configs/se7751_defconfig | 2 +- arch/sh/configs/se7780_defconfig | 2 +- arch/sh/configs/sh03_defconfig | 2 +- arch/sh/configs/sh2007_defconfig | 2 +- arch/sh/configs/sh7757lcr_defconfig | 2 +- arch/sh/configs/sh7763rdp_defconfig | 2 +- arch/sh/configs/shmin_defconfig | 2 +- arch/sh/configs/shx3_defconfig | 2 +- arch/sh/configs/titan_defconfig | 2 +- arch/sh/configs/ul2_defconfig | 2 +- arch/sh/kernel/setup.c | 11 +--- arch/sparc/Kconfig | 18 +------ arch/sparc/prom/bootstr_64.c | 2 +- arch/x86/Kconfig | 45 +--------------- arch/x86/kernel/setup.c | 17 +----- arch/xtensa/Kconfig | 15 +----- arch/xtensa/configs/audio_kc705_defconfig | 1 - arch/xtensa/configs/common_defconfig | 1 - arch/xtensa/configs/generic_kc705_defconfig | 1 - arch/xtensa/configs/iss_defconfig | 1 - arch/xtensa/configs/nommu_kc705_defconfig | 1 - arch/xtensa/configs/smp_lx200_defconfig | 1 - arch/xtensa/configs/virt_defconfig | 1 - arch/xtensa/configs/xip_kc705_defconfig | 1 - arch/xtensa/kernel/setup.c | 10 +--- drivers/firmware/efi/libstub/x86-stub.c | 26 +++++----- drivers/of/fdt.c | 23 ++------- include/linux/cmdline.h | 57 +++++++++++++++++++++ init/Kconfig | 46 +++++++++++++++++ 98 files changed, 209 insertions(+), 580 deletions(-) create mode 100644 include/linux/cmdline.h