lkml.org 
[lkml]   [2019]   [Aug]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH v2] powerpc: Support CMDLINE_EXTEND
From
Date


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.packham@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/commit/?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 ?

>
> 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;

> +
> + 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.


> - 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

\
 
 \ /
  Last update: 2019-08-01 08:14    [W:0.131 / U:0.084 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site