On Thu, 2019-08-01 at 08:14 +0200, Christophe Leroy wrote: > > Le 01/08/2019 à 04:12, Chris Packham a écrit : > > > > Bring powerpc in line with other architectures that support > > extending or > > overriding the bootloader provided command line. > > > > The current behaviour is most like CMDLINE_FROM_BOOTLOADER where > > the > > bootloader command line is preferred but the kernel config can > > provide a > > fallback so CMDLINE_FROM_BOOTLOADER is the default. CMDLINE_EXTEND > > can > > be used to append the CMDLINE from the kernel config to the one > > provided > > by the bootloader. > > > > Signed-off-by: Chris Packham <chris.pack...@alliedtelesis.co.nz> > > --- > > While I'm at it does anyone think it's worth getting rid of the > > default CMDLINE > > value if CMDLINE_BOOL and maybe CMDLINE_BOOL? Every defconfig in > > the kernel > > that sets CMDLINE_BOOL=y also sets CMDLINE to something other than > > "console=ttyS0,9600 console=tty0 root=/dev/sda2". Removing > > CMDLINE_BOOL and > > unconditionally setting the default value of CMDLINE to "" would > > clean up the > > Kconfig even more. > Note > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/co > mmit/?id=cbe46bd4f5104552b612505b73d366f66efc2341 > which is already a step forward. > > I guess that default is for users selecting this option manually to > get > a first sensitive CMDLINE. But is it really worth it ? >
I'm not even sure if it is working as intended right now. Even without my changes if I use menuconfig and select CMDLINE_BOOL I end up with CONFIG_CMDLINE="" in the resulting .config. > > > > > > Changes in v2: > > - incorporate ideas from Christope's patch https://patchwork.ozlabs > > .org/patch/1074126/ > > > > arch/powerpc/Kconfig | 20 +++++++++++++++++++- > > arch/powerpc/kernel/prom_init.c | 26 +++++++++++++++++++++++++- > > 2 files changed, 44 insertions(+), 2 deletions(-) > > > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > > index 77f6ebf97113..d413fe1b4058 100644 > > --- a/arch/powerpc/Kconfig > > +++ b/arch/powerpc/Kconfig > > @@ -852,15 +852,33 @@ config CMDLINE > > 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 > > + > > +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 command-line arguments provided by the boot loader > > will be > > + appended to the default kernel command string. > > + > > config CMDLINE_FORCE > > bool "Always use the default kernel command string" > > - depends on CMDLINE_BOOL > > 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 EXTRA_TARGETS > > string "Additional default image types" > > help > > diff --git a/arch/powerpc/kernel/prom_init.c > > b/arch/powerpc/kernel/prom_init.c > > index 514707ef6779..df29f141dbd2 100644 > > --- a/arch/powerpc/kernel/prom_init.c > > +++ b/arch/powerpc/kernel/prom_init.c > > @@ -310,6 +310,25 @@ static size_t __init prom_strlcpy(char *dest, > > const char *src, size_t size) > > return ret; > > } > > > > +static size_t __init prom_strlcat(char *dest, const char *src, > > size_t count) > > +{ > > + size_t dsize = prom_strlen(dest); > > + size_t len = prom_strlen(src); > > + size_t res = dsize + len; > > + > > + /* This would be a bug */ > > + BUG_ON(dsize >= count); > Has you pointed in another mail, BUG_ON() should be avoided here. > I guess if the destination is already full, just return count: > > if (dsize >= count) > return count; > Will fix in v3. > > > > + > > + dest += dsize; > > + count -= dsize; > > + if (len >= count) > > + len = count-1; > > + memcpy(dest, src, len); > > + dest[len] = 0; > > + return res; > > + > > +} > > + > > #ifdef CONFIG_PPC_PSERIES > > static int __init prom_strtobool(const char *s, bool *res) > > { > > @@ -761,8 +780,13 @@ static void __init early_cmdline_parse(void) > > p = prom_cmd_line; > > if ((long)prom.chosen > 0) > > l = prom_getprop(prom.chosen, "bootargs", p, > > COMMAND_LINE_SIZE-1); > The above is pointless in case CONFIG_CMDLINE_FORCE is selected. > Will fix in v3. > > > > - if (IS_ENABLED(CONFIG_CMDLINE_BOOL) && (l <= 0 || p[0] == > > '\0')) /* dbl check */ > > + > > + if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || l <= 0 || p[0] == > > '\0') > > prom_strlcpy(prom_cmd_line, CONFIG_CMDLINE, > > sizeof(prom_cmd_line)); > If we can ensure that prom_cmd_line remains empty when > CONFIG_CMDLINE_FORCE is selected (see above comment), then > prom_strlcat() can be used in lieu of prom_strlcpy() > > > > > + else if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) > > + prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE, > > + sizeof(prom_cmd_line)); > > + > > prom_printf("command line: %s\n", prom_cmd_line); > > > > #ifdef CONFIG_PPC64 > > > Christophe
