linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] add generic builtin command line
@ 2019-03-01 19:44 Daniel Walker
  2019-03-04 14:05 ` Christophe Leroy
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Walker @ 2019-03-01 19:44 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	linuxppc-dev, Daniel Walker, xe-linux-external, Maksym Kokhan,
	linux-kernel

This code allows architectures to use a generic builtin command line.
The state of the builtin command line options across architecture is
diverse. On x86 and mips they have pretty much the same code and the
code prepends the builtin command line onto the boot loader provided
one. On powerpc there is only a builtin override and nothing else.

The code in this commit unifies the mips and x86 code into a generic
header file under the CONFIG_GENERIC_CMDLINE option. When this
option is enabled the architecture can call the cmdline_add_builtin()
to add the builtin command line.

[maksym.kokhan@globallogic.com: fix cmdline_add_builtin() macro]
Cc: Daniel Walker <dwalker@fifo99.com>
Cc: Daniel Walker <danielwa@cisco.com>
Cc: xe-linux-external@cisco.com
Signed-off-by: Daniel Walker <danielwa@cisco.com>
Signed-off-by: Maksym Kokhan <maksym.kokhan@globallogic.com>
---
 include/linux/cmdline.h | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
 init/Kconfig            | 69 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 141 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..126fa52f55d2
--- /dev/null
+++ b/include/linux/cmdline.h
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_CMDLINE_H
+#define _LINUX_CMDLINE_H
+
+/*
+ *
+ * Copyright (C) 2015. Cisco Systems, Inc.
+ *
+ * Generic Append/Prepend cmdline support.
+ */
+
+#if defined(CONFIG_GENERIC_CMDLINE) && defined(CONFIG_CMDLINE_BOOL)
+
+#ifndef CONFIG_CMDLINE_OVERRIDE
+/*
+ * 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.
+ * @dest: The destination of the final appended/prepended string
+ * @src: The starting string or NULL if there isn't one.
+ * @tmp: temporary space used for prepending
+ * @length: the maximum length of the strings above.
+ */
+static inline void
+_cmdline_add_builtin(char *dest, char *src, char *tmp, unsigned long length)
+{
+	if (src != dest && src != NULL) {
+		strlcpy(dest, " ", length);
+		strlcat(dest, src, length);
+	}
+
+	strlcat(dest, " ", length);
+
+	if (sizeof(CONFIG_CMDLINE_APPEND) > 1)
+		strlcat(dest, CONFIG_CMDLINE_APPEND, length);
+
+	if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
+		strlcpy(tmp, CONFIG_CMDLINE_PREPEND, length);
+		strlcat(tmp, " ", length);
+		strlcat(tmp, dest, length);
+		strlcpy(dest, tmp, length);
+	}
+}
+
+#define cmdline_add_builtin_section(dest, src, length, section) 	    \
+{									    \
+	if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {			    \
+		static char cmdline_tmp_space[length] section;	            \
+		_cmdline_add_builtin(dest, src, cmdline_tmp_space, length); \
+	} else {							    \
+		_cmdline_add_builtin(dest, src, NULL, length);		    \
+	}								    \
+}
+#else
+#define cmdline_add_builtin_section(dest, src, length, section) 	   \
+{									   \
+	strlcpy(dest, CONFIG_CMDLINE_PREPEND " " CONFIG_CMDLINE_APPEND,    \
+		length);						   \
+}
+#endif /* !CONFIG_CMDLINE_OVERRIDE */
+
+#else
+#define cmdline_add_builtin_section(dest, src, length, section) {          \
+	if (src != NULL)						   \
+		strlcpy(dest, src, length);				   \
+}
+#endif /* CONFIG_GENERIC_CMDLINE */
+
+#define cmdline_add_builtin(dest, src, length) \
+	cmdline_add_builtin_section(dest, src, length, __initdata)
+
+#endif /* _LINUX_CMDLINE_H */
diff --git a/init/Kconfig b/init/Kconfig
index a4112e95724a..b59e856511e1 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1775,6 +1775,75 @@ config PROFILING
 config TRACEPOINTS
 	bool
 
+config GENERIC_CMDLINE
+	bool
+
+if GENERIC_CMDLINE
+
+config CMDLINE_BOOL
+	bool "Built-in kernel command line"
+	help
+	  Allow for specifying boot arguments to the kernel at
+	  build time.  On some systems (e.g. embedded ones), it is
+	  necessary or convenient to provide some or all of the
+	  kernel boot arguments with the kernel itself (that is,
+	  to not rely on the boot loader to provide them.)
+
+	  To compile command line arguments into the kernel,
+	  set this option to 'Y', then fill in the
+	  the boot arguments in CONFIG_CMDLINE.
+
+	  Systems with fully functional boot loaders (i.e. non-embedded)
+	  should leave this option set to 'N'.
+
+config CMDLINE_APPEND
+	string "Built-in kernel command string append"
+	depends on CMDLINE_BOOL
+	default ""
+	help
+	  Enter arguments here that should be compiled into the kernel
+	  image and used at boot time.  If the boot loader provides a
+	  command line at boot time, this string is appended to it to
+	  form the full kernel command line, when the system boots.
+
+	  However, you can use the CONFIG_CMDLINE_OVERRIDE option to
+	  change this behavior.
+
+	  In most cases, the command line (whether built-in or provided
+	  by the boot loader) should specify the device for the root
+	  file system.
+
+config CMDLINE_PREPEND
+	string "Built-in kernel command string prepend"
+	depends on CMDLINE_BOOL
+	default ""
+	help
+	  Enter arguments here that should be compiled into the kernel
+	  image and used at boot time.  If the boot loader provides a
+	  command line at boot time, this string is prepended to it to
+	  form the full kernel command line, when the system boots.
+
+	  However, you can use the CONFIG_CMDLINE_OVERRIDE option to
+	  change this behavior.
+
+	  In most cases, the command line (whether built-in or provided
+	  by the boot loader) should specify the device for the root
+	  file system.
+
+config CMDLINE_OVERRIDE
+	bool "Built-in command line overrides boot loader arguments"
+	depends on CMDLINE_BOOL
+	help
+	  Set this option to 'Y' to have the kernel ignore the boot loader
+	  command line, and use ONLY the built-in command line. In this case
+	  append and prepend strings are concatenated to form the full
+	  command line.
+
+	  This is used to work around broken boot loaders.  This should
+	  be set to 'N' under normal conditions.
+
+endif
+
 endmenu		# General setup
 
 source "arch/Kconfig"
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] add generic builtin command line
  2019-03-01 19:44 [PATCH 1/3] add generic builtin command line Daniel Walker
@ 2019-03-04 14:05 ` Christophe Leroy
  0 siblings, 0 replies; 5+ messages in thread
From: Christophe Leroy @ 2019-03-04 14:05 UTC (permalink / raw)
  To: Daniel Walker, Andrew Morton
  Cc: Maksym Kokhan, linux-kernel, Paul Mackerras, xe-linux-external,
	Daniel Walker, linuxppc-dev



Le 01/03/2019 à 20:44, Daniel Walker a écrit :
> This code allows architectures to use a generic builtin command line.
> The state of the builtin command line options across architecture is
> diverse. On x86 and mips they have pretty much the same code and the
> code prepends the builtin command line onto the boot loader provided
> one. On powerpc there is only a builtin override and nothing else.
> 
> The code in this commit unifies the mips and x86 code into a generic
> header file under the CONFIG_GENERIC_CMDLINE option. When this
> option is enabled the architecture can call the cmdline_add_builtin()
> to add the builtin command line.
> 
> [maksym.kokhan@globallogic.com: fix cmdline_add_builtin() macro]
> Cc: Daniel Walker <dwalker@fifo99.com>
> Cc: Daniel Walker <danielwa@cisco.com>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> Signed-off-by: Maksym Kokhan <maksym.kokhan@globallogic.com>
> ---
>   include/linux/cmdline.h | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
>   init/Kconfig            | 69 +++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 141 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..126fa52f55d2
> --- /dev/null
> +++ b/include/linux/cmdline.h
> @@ -0,0 +1,72 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_CMDLINE_H
> +#define _LINUX_CMDLINE_H
> +
> +/*
> + *
> + * Copyright (C) 2015. Cisco Systems, Inc.
> + *
> + * Generic Append/Prepend cmdline support.
> + */
> +
> +#if defined(CONFIG_GENERIC_CMDLINE) && defined(CONFIG_CMDLINE_BOOL)
> +
> +#ifndef CONFIG_CMDLINE_OVERRIDE
> +/*
> + * 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.
> + * @dest: The destination of the final appended/prepended string
> + * @src: The starting string or NULL if there isn't one.
> + * @tmp: temporary space used for prepending
> + * @length: the maximum length of the strings above.
> + */
> +static inline void
> +_cmdline_add_builtin(char *dest, char *src, char *tmp, unsigned long length)
> +{
> +	if (src != dest && src != NULL) {
> +		strlcpy(dest, " ", length);

Why prepend a space here ?

> +		strlcat(dest, src, length);
> +	}
> +
> +	strlcat(dest, " ", length);

Why a space here ? When there is nothing to append, there is no need of 
a space here. The space could simply be concatenated below in from of 
CONFIG_CMDLINE_APPEND

> +
> +	if (sizeof(CONFIG_CMDLINE_APPEND) > 1)
> +		strlcat(dest, CONFIG_CMDLINE_APPEND, length);
> +
> +	if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
> +		strlcpy(tmp, CONFIG_CMDLINE_PREPEND, length);
> +		strlcat(tmp, " ", length);

Why not concatenate the space to above, ie:
strlcpy(tmp, CONFIG_CMDLINE_PREPEND " ", length);

> +		strlcat(tmp, dest, length);
> +		strlcpy(dest, tmp, length);
> +	}
> +}
> +
> +#define cmdline_add_builtin_section(dest, src, length, section) 	    \
> +{									    \
> +	if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {			    \
> +		static char cmdline_tmp_space[length] section;	            \
> +		_cmdline_add_builtin(dest, src, cmdline_tmp_space, length); \
> +	} else {							    \
> +		_cmdline_add_builtin(dest, src, NULL, length);		    \
> +	}								    \
> +}
> +#else
> +#define cmdline_add_builtin_section(dest, src, length, section) 	   \
> +{									   \
> +	strlcpy(dest, CONFIG_CMDLINE_PREPEND " " CONFIG_CMDLINE_APPEND,    \
> +		length);						   \
> +}
> +#endif /* !CONFIG_CMDLINE_OVERRIDE */
> +
> +#else
> +#define cmdline_add_builtin_section(dest, src, length, section) {          \
> +	if (src != NULL)						   \
> +		strlcpy(dest, src, length);				   \
> +}
> +#endif /* CONFIG_GENERIC_CMDLINE */
> +
> +#define cmdline_add_builtin(dest, src, length) \
> +	cmdline_add_builtin_section(dest, src, length, __initdata)

All those ifdefs are uggly, I don't think it is worth making something 
so complex just for the fun of getting something generic. See my 
comments to following patch.

> +
> +#endif /* _LINUX_CMDLINE_H */
> diff --git a/init/Kconfig b/init/Kconfig
> index a4112e95724a..b59e856511e1 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1775,6 +1775,75 @@ config PROFILING
>   config TRACEPOINTS
>   	bool
>   
> +config GENERIC_CMDLINE
> +	bool
> +
> +if GENERIC_CMDLINE
> +
> +config CMDLINE_BOOL
> +	bool "Built-in kernel command line"
> +	help
> +	  Allow for specifying boot arguments to the kernel at
> +	  build time.  On some systems (e.g. embedded ones), it is
> +	  necessary or convenient to provide some or all of the
> +	  kernel boot arguments with the kernel itself (that is,
> +	  to not rely on the boot loader to provide them.)
> +
> +	  To compile command line arguments into the kernel,
> +	  set this option to 'Y', then fill in the
> +	  the boot arguments in CONFIG_CMDLINE.
> +
> +	  Systems with fully functional boot loaders (i.e. non-embedded)
> +	  should leave this option set to 'N'.
> +
> +config CMDLINE_APPEND
> +	string "Built-in kernel command string append"
> +	depends on CMDLINE_BOOL

Would be better to remove this 'depends' and put the condition on the 
previous line. That way we would always have CONFIG_CMDLINE_APPEND and 
we could do something more simple in the code.

> +	default ""
> +	help
> +	  Enter arguments here that should be compiled into the kernel
> +	  image and used at boot time.  If the boot loader provides a
> +	  command line at boot time, this string is appended to it to
> +	  form the full kernel command line, when the system boots.
> +
> +	  However, you can use the CONFIG_CMDLINE_OVERRIDE option to
> +	  change this behavior.
> +
> +	  In most cases, the command line (whether built-in or provided
> +	  by the boot loader) should specify the device for the root
> +	  file system.
> +
> +config CMDLINE_PREPEND
> +	string "Built-in kernel command string prepend"
> +	depends on CMDLINE_BOOL

Would be better to remove this 'depends' and put the condition on the 
previous line. That way we would always have CONFIG_CMDLINE_APPEND and 
we could do something more simple in the code.

> +	default ""
> +	help
> +	  Enter arguments here that should be compiled into the kernel
> +	  image and used at boot time.  If the boot loader provides a
> +	  command line at boot time, this string is prepended to it to
> +	  form the full kernel command line, when the system boots.
> +
> +	  However, you can use the CONFIG_CMDLINE_OVERRIDE option to
> +	  change this behavior.
> +
> +	  In most cases, the command line (whether built-in or provided
> +	  by the boot loader) should specify the device for the root
> +	  file system.
> +
> +config CMDLINE_OVERRIDE
> +	bool "Built-in command line overrides boot loader arguments"
> +	depends on CMDLINE_BOOL
> +	help
> +	  Set this option to 'Y' to have the kernel ignore the boot loader
> +	  command line, and use ONLY the built-in command line. In this case
> +	  append and prepend strings are concatenated to form the full
> +	  command line.
> +
> +	  This is used to work around broken boot loaders.  This should
> +	  be set to 'N' under normal conditions.
> +
> +endif
> +
>   endmenu		# General setup
>   
>   source "arch/Kconfig"
> 

Christophe

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] add generic builtin command line
  2018-11-29  5:12 ` Andrew Morton
@ 2018-11-29 15:51   ` Daniel Walker
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Walker @ 2018-11-29 15:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Daniel Walker, xe-linux-external, Maksym Kokhan, linux-kernel

On Wed, Nov 28, 2018 at 09:12:12PM -0800, Andrew Morton wrote:
> On Fri,  9 Nov 2018 09:34:31 -0800 Daniel Walker <danielwa@cisco.com> wrote:
> 
> > This code allows architectures to use a generic builtin command line.
> > The state of the builtin command line options across architecture is
> > diverse. On x86 and mips they have pretty much the same code and the
> > code prepends the builtin command line onto the boot loader provided
> > one. On powerpc there is only a builtin override and nothing else.
> > 
> > The code in this commit unifies the mips and x86 code into a generic
> > header file under the CONFIG_GENERIC_CMDLINE option. When this
> > option is enabled the architecture can call the cmdline_add_builtin()
> > to add the builtin command line.
> 
> I'm not sure what's happened to this and I haven't seen the other two
> patches but...

It's been sitting in -next since the last merge window. 

> 
> > [maksym.kokhan@globallogic.com: fix cmdline_add_builtin() macro]
> > Cc: Daniel Walker <dwalker@fifo99.com>
> > Cc: Daniel Walker <danielwa@cisco.com>
> > Cc: xe-linux-external@cisco.com
> > Signed-off-by: Daniel Walker <danielwa@cisco.com>
> > Signed-off-by: Maksym Kokhan <maksym.kokhan@globallogic.com>
> 
> Two SOB's is nice, but some reviews and acks would be nicer.
 
Would love to add some, but no one is reviewing it.. 

> > --- /dev/null
> > +++ b/include/linux/cmdline.h
> > @@ -0,0 +1,79 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _LINUX_CMDLINE_H
> > +#define _LINUX_CMDLINE_H
> > +
> > +/*
> > + *
> > + * Copyright (C) 2015. Cisco Systems, Inc.
> > + *
> > + * Generic Append/Prepend cmdline support.
> > + */
> > +
> > +#if defined(CONFIG_GENERIC_CMDLINE) && defined(CONFIG_CMDLINE_BOOL)
> > +
> > +#ifndef CONFIG_CMDLINE_OVERRIDE
> > +/*
> > + * 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.
> > + * @dest: The destination of the final appended/prepended string
> > + * @src: The starting string or NULL if there isn't one.
> > + * @tmp: temporary space used for prepending
> > + * @length: the maximum length of the strings above.
> > + */
> > +static inline void
> > +_cmdline_add_builtin(char *dest, char *src, char *tmp, unsigned long length)
> > +{
> > +	if (src != dest && src != NULL) {
> > +		strlcpy(dest, " ", length);
> > +		strlcat(dest, src, length);
> > +	}
> > +
> > +	strlcat(dest, " ", length);
> > +
> > +	if (sizeof(CONFIG_CMDLINE_APPEND) > 1)
> > +		strlcat(dest, CONFIG_CMDLINE_APPEND, length);
> > +
> > +	/*
> > +	 * You need to convert you old style CONFIG_CMDLINE to use
> 
> "your"
> 
> > +	 * the prepend, or append defines. Some architectures use one
> 
> "one and"
> 
> > +	 * some use the other. You need to figure out which ones is
> 
> "one"

Can fix these..


> 
> > +	 * right for your situation. I would recommend prepending
> > +	 * because it's the safest (i.e. CONFIG_CMDLINE_PREPEND).
> > +	 */
> > +	BUILD_BUG_ON(sizeof(CONFIG_CMDLINE) != 1);
> > +
> > +	if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
> > +		strlcpy(tmp, CONFIG_CMDLINE_PREPEND, length);
> > +		strlcat(tmp, " ", length);
> > +		strlcat(tmp, dest, length);
> > +		strlcpy(dest, tmp, length);
> > +	}
> > +}
> 
> And... holy cow.  Does this monster really need to be inlined?
> 
> > +#define cmdline_add_builtin(dest, src, length)				    \
> > +{									    \
> > +	if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {			    \
> > +		static char cmdline_tmp_space[length] __initdata;	    \
> > +		_cmdline_add_builtin(dest, src, cmdline_tmp_space, length); \
> > +	} else {							    \
> > +		_cmdline_add_builtin(dest, src, NULL, length);		    \
> > +	}								    \
> > +}
> 
> And this will generate __initdata storage at each invocation site.  Can
> it be redone in real, non-inlined C?


It's intended to be used once, maybe twice, per architecture. Powerpc uses it
twice I think. There's no reason for it to be used more than twice, and it replaces
code which is similar to the code above. I can' imagine a situation where people
go nuts and start calling this everywhere.

So it seemed easier to just inline it.

Daniel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] add generic builtin command line
  2018-11-09 17:34 Daniel Walker
@ 2018-11-29  5:12 ` Andrew Morton
  2018-11-29 15:51   ` Daniel Walker
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2018-11-29  5:12 UTC (permalink / raw)
  To: Daniel Walker
  Cc: Daniel Walker, xe-linux-external, Maksym Kokhan, linux-kernel

On Fri,  9 Nov 2018 09:34:31 -0800 Daniel Walker <danielwa@cisco.com> wrote:

> This code allows architectures to use a generic builtin command line.
> The state of the builtin command line options across architecture is
> diverse. On x86 and mips they have pretty much the same code and the
> code prepends the builtin command line onto the boot loader provided
> one. On powerpc there is only a builtin override and nothing else.
> 
> The code in this commit unifies the mips and x86 code into a generic
> header file under the CONFIG_GENERIC_CMDLINE option. When this
> option is enabled the architecture can call the cmdline_add_builtin()
> to add the builtin command line.

I'm not sure what's happened to this and I haven't seen the other two
patches but...

> [maksym.kokhan@globallogic.com: fix cmdline_add_builtin() macro]
> Cc: Daniel Walker <dwalker@fifo99.com>
> Cc: Daniel Walker <danielwa@cisco.com>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> Signed-off-by: Maksym Kokhan <maksym.kokhan@globallogic.com>

Two SOB's is nice, but some reviews and acks would be nicer.

> --- /dev/null
> +++ b/include/linux/cmdline.h
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_CMDLINE_H
> +#define _LINUX_CMDLINE_H
> +
> +/*
> + *
> + * Copyright (C) 2015. Cisco Systems, Inc.
> + *
> + * Generic Append/Prepend cmdline support.
> + */
> +
> +#if defined(CONFIG_GENERIC_CMDLINE) && defined(CONFIG_CMDLINE_BOOL)
> +
> +#ifndef CONFIG_CMDLINE_OVERRIDE
> +/*
> + * 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.
> + * @dest: The destination of the final appended/prepended string
> + * @src: The starting string or NULL if there isn't one.
> + * @tmp: temporary space used for prepending
> + * @length: the maximum length of the strings above.
> + */
> +static inline void
> +_cmdline_add_builtin(char *dest, char *src, char *tmp, unsigned long length)
> +{
> +	if (src != dest && src != NULL) {
> +		strlcpy(dest, " ", length);
> +		strlcat(dest, src, length);
> +	}
> +
> +	strlcat(dest, " ", length);
> +
> +	if (sizeof(CONFIG_CMDLINE_APPEND) > 1)
> +		strlcat(dest, CONFIG_CMDLINE_APPEND, length);
> +
> +	/*
> +	 * You need to convert you old style CONFIG_CMDLINE to use

"your"

> +	 * the prepend, or append defines. Some architectures use one

"one and"

> +	 * some use the other. You need to figure out which ones is

"one"

> +	 * right for your situation. I would recommend prepending
> +	 * because it's the safest (i.e. CONFIG_CMDLINE_PREPEND).
> +	 */
> +	BUILD_BUG_ON(sizeof(CONFIG_CMDLINE) != 1);
> +
> +	if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
> +		strlcpy(tmp, CONFIG_CMDLINE_PREPEND, length);
> +		strlcat(tmp, " ", length);
> +		strlcat(tmp, dest, length);
> +		strlcpy(dest, tmp, length);
> +	}
> +}

And... holy cow.  Does this monster really need to be inlined?

> +#define cmdline_add_builtin(dest, src, length)				    \
> +{									    \
> +	if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {			    \
> +		static char cmdline_tmp_space[length] __initdata;	    \
> +		_cmdline_add_builtin(dest, src, cmdline_tmp_space, length); \
> +	} else {							    \
> +		_cmdline_add_builtin(dest, src, NULL, length);		    \
> +	}								    \
> +}

And this will generate __initdata storage at each invocation site.  Can
it be redone in real, non-inlined C?

> +#else
> +#define cmdline_add_builtin(dest, src, length)				   \
> +{									   \
> +	strlcpy(dest, CONFIG_CMDLINE_PREPEND " " CONFIG_CMDLINE_APPEND,    \
> +		length);						   \
> +}
> +#endif /* !CONFIG_CMDLINE_OVERRIDE */
> +
> +#else
> +#define cmdline_add_builtin(dest, src, length) { \
> +	if (src != NULL)						   \
> +		strlcpy(dest, src, length);				   \
> +}
> +#endif /* CONFIG_GENERIC_CMDLINE */
> +
> +
> +#endif /* _LINUX_CMDLINE_H */


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] add generic builtin command line
@ 2018-11-09 17:34 Daniel Walker
  2018-11-29  5:12 ` Andrew Morton
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Walker @ 2018-11-09 17:34 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Daniel Walker, xe-linux-external, Maksym Kokhan, linux-kernel

This code allows architectures to use a generic builtin command line.
The state of the builtin command line options across architecture is
diverse. On x86 and mips they have pretty much the same code and the
code prepends the builtin command line onto the boot loader provided
one. On powerpc there is only a builtin override and nothing else.

The code in this commit unifies the mips and x86 code into a generic
header file under the CONFIG_GENERIC_CMDLINE option. When this
option is enabled the architecture can call the cmdline_add_builtin()
to add the builtin command line.

[maksym.kokhan@globallogic.com: fix cmdline_add_builtin() macro]
Cc: Daniel Walker <dwalker@fifo99.com>
Cc: Daniel Walker <danielwa@cisco.com>
Cc: xe-linux-external@cisco.com
Signed-off-by: Daniel Walker <danielwa@cisco.com>
Signed-off-by: Maksym Kokhan <maksym.kokhan@globallogic.com>
---
 include/linux/cmdline.h | 79 +++++++++++++++++++++++++++++++++++++++++++++++++
 init/Kconfig            | 72 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 151 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..e7e5cdd66d3a
--- /dev/null
+++ b/include/linux/cmdline.h
@@ -0,0 +1,79 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_CMDLINE_H
+#define _LINUX_CMDLINE_H
+
+/*
+ *
+ * Copyright (C) 2015. Cisco Systems, Inc.
+ *
+ * Generic Append/Prepend cmdline support.
+ */
+
+#if defined(CONFIG_GENERIC_CMDLINE) && defined(CONFIG_CMDLINE_BOOL)
+
+#ifndef CONFIG_CMDLINE_OVERRIDE
+/*
+ * 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.
+ * @dest: The destination of the final appended/prepended string
+ * @src: The starting string or NULL if there isn't one.
+ * @tmp: temporary space used for prepending
+ * @length: the maximum length of the strings above.
+ */
+static inline void
+_cmdline_add_builtin(char *dest, char *src, char *tmp, unsigned long length)
+{
+	if (src != dest && src != NULL) {
+		strlcpy(dest, " ", length);
+		strlcat(dest, src, length);
+	}
+
+	strlcat(dest, " ", length);
+
+	if (sizeof(CONFIG_CMDLINE_APPEND) > 1)
+		strlcat(dest, CONFIG_CMDLINE_APPEND, length);
+
+	/*
+	 * You need to convert you old style CONFIG_CMDLINE to use
+	 * the prepend, or append defines. Some architectures use one
+	 * some use the other. You need to figure out which ones is
+	 * right for your situation. I would recommend prepending
+	 * because it's the safest (i.e. CONFIG_CMDLINE_PREPEND).
+	 */
+	BUILD_BUG_ON(sizeof(CONFIG_CMDLINE) != 1);
+
+	if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
+		strlcpy(tmp, CONFIG_CMDLINE_PREPEND, length);
+		strlcat(tmp, " ", length);
+		strlcat(tmp, dest, length);
+		strlcpy(dest, tmp, length);
+	}
+}
+
+#define cmdline_add_builtin(dest, src, length)				    \
+{									    \
+	if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {			    \
+		static char cmdline_tmp_space[length] __initdata;	    \
+		_cmdline_add_builtin(dest, src, cmdline_tmp_space, length); \
+	} else {							    \
+		_cmdline_add_builtin(dest, src, NULL, length);		    \
+	}								    \
+}
+#else
+#define cmdline_add_builtin(dest, src, length)				   \
+{									   \
+	strlcpy(dest, CONFIG_CMDLINE_PREPEND " " CONFIG_CMDLINE_APPEND,    \
+		length);						   \
+}
+#endif /* !CONFIG_CMDLINE_OVERRIDE */
+
+#else
+#define cmdline_add_builtin(dest, src, length) { \
+	if (src != NULL)						   \
+		strlcpy(dest, src, length);				   \
+}
+#endif /* CONFIG_GENERIC_CMDLINE */
+
+
+#endif /* _LINUX_CMDLINE_H */
diff --git a/init/Kconfig b/init/Kconfig
index a4112e95724a..bac58530e082 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1775,6 +1775,78 @@ config PROFILING
 config TRACEPOINTS
 	bool
 
+config GENERIC_CMDLINE
+	bool
+
+if GENERIC_CMDLINE
+
+config CMDLINE_BOOL
+	bool "Built-in kernel command line"
+	help
+	  Allow for specifying boot arguments to the kernel at
+	  build time.  On some systems (e.g. embedded ones), it is
+	  necessary or convenient to provide some or all of the
+	  kernel boot arguments with the kernel itself (that is,
+	  to not rely on the boot loader to provide them.)
+
+	  To compile command line arguments into the kernel,
+	  set this option to 'Y', then fill in the
+	  the boot arguments in CONFIG_CMDLINE.
+
+	  Systems with fully functional boot loaders (i.e. non-embedded)
+	  should leave this option set to 'N'.
+
+config CMDLINE_APPEND
+	string "Built-in kernel command string append"
+	depends on CMDLINE_BOOL
+	default ""
+	help
+	  Enter arguments here that should be compiled into the kernel
+	  image and used at boot time.  If the boot loader provides a
+	  command line at boot time, this string is appended to it to
+	  form the full kernel command line, when the system boots.
+
+	  However, you can use the CONFIG_CMDLINE_OVERRIDE option to
+	  change this behavior.
+
+	  In most cases, the command line (whether built-in or provided
+	  by the boot loader) should specify the device for the root
+	  file system.
+
+config CMDLINE_PREPEND
+	string "Built-in kernel command string prepend"
+	depends on CMDLINE_BOOL
+	default ""
+	help
+	  Enter arguments here that should be compiled into the kernel
+	  image and used at boot time.  If the boot loader provides a
+	  command line at boot time, this string is prepended to it to
+	  form the full kernel command line, when the system boots.
+
+	  However, you can use the CONFIG_CMDLINE_OVERRIDE option to
+	  change this behavior.
+
+	  In most cases, the command line (whether built-in or provided
+	  by the boot loader) should specify the device for the root
+	  file system.
+
+config CMDLINE_OVERRIDE
+	bool "Built-in command line overrides boot loader arguments"
+	depends on CMDLINE_BOOL
+	help
+	  Set this option to 'Y' to have the kernel ignore the boot loader
+	  command line, and use ONLY the built-in command line. In this case
+	  append and prepend strings are concatenated to form the full
+	  command line.
+
+	  This is used to work around broken boot loaders.  This should
+	  be set to 'N' under normal conditions.
+
+config CMDLINE
+	string ""
+
+endif
+
 endmenu		# General setup
 
 source "arch/Kconfig"
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-03-04 14:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-01 19:44 [PATCH 1/3] add generic builtin command line Daniel Walker
2019-03-04 14:05 ` Christophe Leroy
  -- strict thread matches above, loose matches on Subject: below --
2018-11-09 17:34 Daniel Walker
2018-11-29  5:12 ` Andrew Morton
2018-11-29 15:51   ` Daniel Walker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).