2021-03-04 06:42:38

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v2 0/7] Improve boot command line handling

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 <[email protected]>

Christophe Leroy (7):
cmdline: Add generic function to build command line.
drivers: of: use cmdline building function
powerpc: convert to generic builtin command line
cmdline: Add capability to prepend the command line
powerpc: add capability to prepend default command line
cmdline: Gives architectures opportunity to use generically defined
boot cmdline manipulation
powerpc: use generic CMDLINE manipulations

arch/powerpc/Kconfig | 37 ++-----------------
arch/powerpc/kernel/prom_init.c | 35 +++---------------
drivers/of/fdt.c | 23 ++----------
include/linux/cmdline.h | 65 +++++++++++++++++++++++++++++++++
init/Kconfig | 56 ++++++++++++++++++++++++++++
5 files changed, 133 insertions(+), 83 deletions(-)
create mode 100644 include/linux/cmdline.h

--
2.25.0


2021-03-04 12:34:27

by Daniel Walker (danielwa)

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] Improve boot command line handling

On Wed, Mar 03, 2021 at 07:07:45PM +0100, Christophe Leroy wrote:
>
>
> Le 03/03/2021 ? 18:39, Daniel Walker a ?crit?:
> > On Tue, Mar 02, 2021 at 08:01:01PM -0600, Rob Herring wrote:
> > > +Will D
> > >
> > > On Tue, Mar 2, 2021 at 11:36 AM Daniel Walker <[email protected]> wrote:
> > > >
> > > > On Tue, Mar 02, 2021 at 05:25:16PM +0000, Christophe Leroy 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 <[email protected]>
> > > > >
> > > >
> > > >
> > > > I don't see a point in your changes at this time. My changes are much more
> > > > mature, and you changes don't really make improvements.
> > >
> > > Not really a helpful comment. What we merge here will be from whomever
> > > is persistent and timely in their efforts. But please, work together
> > > on a common solution.
> > >
> > > This one meets my requirements of moving the kconfig and code out of
> > > the arches, supports prepend/append, and is up to date.
> >
> >
> > Maintainers are capable of merging whatever they want to merge. However, I
> > wouldn't make hasty choices. The changes I've been submitting have been deployed
> > on millions of router instances and are more feature rich.
> >
> > I believe I worked with you on this change, or something like it,
> >
> > https://lkml.org/lkml/2019/3/19/970
> >
> > I don't think Christophe has even addressed this.
>
> I thing I have, see https://patchwork.ozlabs.org/project/linuxppc-dev/patch/3b4291271ce4af4941a771e5af5cbba3c8fa1b2a.1614705851.git.christophe.leroy@csgroup.eu/
>
> If you see something missing in that patch, can you tell me.

Ok, must have missed that one.


> > I've converted many
> > architectures, and Cisco uses my changes on at least 4 different
> > architecture. With products deployed and tested.
>
> As far as we know, only powerpc was converted in the last series you
> submitted, see
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=98106&state=*


Me and others submitted my changes many times, and other architectures have been included. The patch
you submitted I've submitted similar at Rob's request years ago.

Here a fuller submissions some time ago,

https://lore.kernel.org/patchwork/cover/992768/

You've only been involved in prior powerpc only submissions.

Daniel

2021-03-04 20:26:19

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v2 2/7] drivers: of: use cmdline building function

This patch uses the new cmdline building function to
concatenate the of provided cmdline with built-in parts
based on compile-time options.

Signed-off-by: Christophe Leroy <[email protected]>
---
drivers/of/fdt.c | 23 ++++-------------------
1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index dcc1dd96911a..cf2b95b8f298 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -25,6 +25,7 @@
#include <linux/serial_core.h>
#include <linux/sysfs.h>
#include <linux/random.h>
+#include <linux/cmdline.h>

#include <asm/setup.h> /* for COMMAND_LINE_SIZE */
#include <asm/page.h>
@@ -1050,26 +1051,10 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,

/* Retrieve command line */
p = of_get_flat_dt_prop(node, "bootargs", &l);
- if (p != NULL && l > 0)
- strlcpy(data, p, min(l, COMMAND_LINE_SIZE));
+ if (l <= 0)
+ p = NULL;

- /*
- * CONFIG_CMDLINE is meant to be a default in case nothing else
- * managed to set the command line, unless CONFIG_CMDLINE_FORCE
- * is set in which case we override whatever was found earlier.
- */
-#ifdef CONFIG_CMDLINE
-#if defined(CONFIG_CMDLINE_EXTEND)
- strlcat(data, " ", COMMAND_LINE_SIZE);
- strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#elif defined(CONFIG_CMDLINE_FORCE)
- strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#else
- /* No arguments from boot loader, use kernel's cmdl*/
- if (!((char *)data)[0])
- strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#endif
-#endif /* CONFIG_CMDLINE */
+ cmdline_build(data, p, COMMAND_LINE_SIZE);

pr_debug("Command line is: %s\n", (char *)data);

--
2.25.0

2021-03-04 20:26:42

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v2 3/7] powerpc: convert to generic builtin command line

This updates the powerpc code to use the new cmdline building function.

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/prom_init.c | 35 +++++----------------------------
1 file changed, 5 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index ccf77b985c8f..24157e526f80 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -27,6 +27,7 @@
#include <linux/initrd.h>
#include <linux/bitops.h>
#include <linux/pgtable.h>
+#include <linux/cmdline.h>
#include <asm/prom.h>
#include <asm/rtas.h>
#include <asm/page.h>
@@ -152,7 +153,7 @@ static struct prom_t __prombss prom;
static unsigned long __prombss prom_entry;

static char __prombss of_stdout_device[256];
-static char __prombss prom_scratch[256];
+static char __prombss prom_scratch[COMMAND_LINE_SIZE];

static unsigned long __prombss dt_header_start;
static unsigned long __prombss dt_struct_start, dt_struct_end;
@@ -304,26 +305,6 @@ static char __init *prom_strstr(const char *s1, const char *s2)
return NULL;
}

-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 */
- 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)
{
@@ -768,19 +749,13 @@ static unsigned long prom_memparse(const char *ptr, const char **retptr)
static void __init early_cmdline_parse(void)
{
const char *opt;
-
- char *p;
int l = 0;

- prom_cmd_line[0] = 0;
- p = prom_cmd_line;
-
if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && (long)prom.chosen > 0)
- l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
+ l = prom_getprop(prom.chosen, "bootargs", prom_scratch,
+ COMMAND_LINE_SIZE - 1);

- if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\0')
- prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
- sizeof(prom_cmd_line));
+ cmdline_build(prom_cmd_line, l > 0 ? prom_scratch : NULL, sizeof(prom_scratch));

prom_printf("command line: %s\n", prom_cmd_line);

--
2.25.0

2021-03-04 20:27:20

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v2 1/7] cmdline: Add generic function to build command line.

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 <[email protected]>
---
include/linux/cmdline.h | 62 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 62 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..ae3610bb0ee2
--- /dev/null
+++ b/include/linux/cmdline.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_CMDLINE_H
+#define _LINUX_CMDLINE_H
+
+static __always_inline size_t cmdline_strlen(const char *s)
+{
+ const char *sc;
+
+ for (sc = s; *sc != '\0'; ++sc)
+ ; /* nothing */
+ return sc - s;
+}
+
+static __always_inline size_t cmdline_strlcat(char *dest, const char *src, size_t count)
+{
+ size_t dsize = cmdline_strlen(dest);
+ size_t len = cmdline_strlen(src);
+ size_t res = dsize + len;
+
+ /* This would be a bug */
+ if (dsize >= count)
+ return count;
+
+ dest += dsize;
+ count -= dsize;
+ if (len >= count)
+ len = count - 1;
+ memcpy(dest, src, len);
+ dest[len] = 0;
+ return res;
+}
+
+/*
+ * This function will append 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.
+ * @dest: The destination of the final appended/prepended string.
+ * @src: The starting string or NULL if there isn't one. Must not equal dest.
+ * @length: the length of dest buffer.
+ */
+static __always_inline void cmdline_build(char *dest, const char *src, size_t length)
+{
+ if (length <= 0)
+ return;
+
+ dest[0] = 0;
+
+#ifdef CONFIG_CMDLINE
+ if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !src || !src[0]) {
+ cmdline_strlcat(dest, CONFIG_CMDLINE, length);
+ return;
+ }
+#endif
+ if (dest != src)
+ cmdline_strlcat(dest, src, length);
+#ifdef CONFIG_CMDLINE
+ if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) && sizeof(CONFIG_CMDLINE) > 1)
+ cmdline_strlcat(dest, " " CONFIG_CMDLINE, length);
+#endif
+}
+
+#endif /* _LINUX_CMDLINE_H */
--
2.25.0