This code allows architectures to use a generic builtin command line.
The state of the builtin command line options across architecture is
diverse. MIPS and X86 once has similar systems, then mips added some
options to allow extending the command line. Powerpc did something
simiar in adding the ability to extend. Even with mips and powerpc
enhancement the needs of Cisco are not met on these platforms.
The code in this commit unifies the 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. The generic code provides both
append and/or prepend options and provides a way to redefine these
option after the kernel is compiled.
This code also includes test's which are meant to confirm
functionality.
This unified implementation offers the same functionality needed by
Cisco on all platform which we enable it on.
Cc: [email protected]
Signed-off-by: Ruslan Bilovol <[email protected]>
Signed-off-by: Daniel Walker <[email protected]>
---
include/linux/cmdline.h | 106 ++++++++++++++++++++++++++++++
init/Kconfig | 78 ++++++++++++++++++++++
lib/Kconfig | 4 ++
lib/Makefile | 3 +
lib/generic_cmdline.S | 53 +++++++++++++++
lib/test_cmdline1.c | 139 ++++++++++++++++++++++++++++++++++++++++
6 files changed, 383 insertions(+)
create mode 100644 include/linux/cmdline.h
create mode 100644 lib/generic_cmdline.S
create mode 100644 lib/test_cmdline1.c
diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
new file mode 100644
index 000000000000..a94758a0f257
--- /dev/null
+++ b/include/linux/cmdline.h
@@ -0,0 +1,106 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_CMDLINE_H
+#define _LINUX_CMDLINE_H
+/*
+ *
+ * Copyright (C) 2006,2021. Cisco Systems, Inc.
+ *
+ * Generic Append/Prepend cmdline support.
+ */
+
+
+#include <linux/ctype.h>
+#include <linux/cache.h>
+#include <asm/setup.h>
+
+#ifdef CONFIG_CMDLINE_BOOL
+extern char cmdline_prepend[];
+extern char cmdline_append[];
+extern char cmdline_tmp[];
+#define CMDLINE_PREPEND cmdline_prepend
+#define CMDLINE_APPEND cmdline_append
+#define CMDLINE_TMP cmdline_tmp
+#define CMDLINE_STATIC_PREPEND CONFIG_CMDLINE_PREPEND
+#define CMDLINE_STATIC_APPEND CONFIG_CMDLINE_APPEND
+#else
+#define CMDLINE_PREPEND ""
+#define CMDLINE_APPEND ""
+#define CMDLINE_TMP ""
+#define CMDLINE_STATIC_PREPEND ""
+#define CMDLINE_STATIC_APPEND ""
+#endif
+
+#ifndef CMDLINE_STRLCAT
+#define CMDLINE_STRLCAT strlcat
+#endif
+
+#ifndef CMDLINE_STRLEN
+#define CMDLINE_STRLEN strlen
+#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.
+ * @dest: The destination of the final appended/prepended string
+ * @tmp: temporary space used for prepending
+ * @prepend: string to prepend to @dest
+ * @append: string to append to @dest
+ * @length: the maximum length of the strings above.
+ * @cmdline_strlen: point to a compatible strlen
+ * @cmdline_strlcat: point to a compatible strlcat
+ * This function returns true when the builtin command line was copied successfully
+ * and false when there was not enough room to copy all parts of the command line.
+ */
+static inline bool
+__cmdline_add_builtin(
+ char *dest,
+ char *tmp,
+ char *prepend,
+ char *append,
+ unsigned long length,
+ size_t (*cmdline_strlen)(const char *s),
+ size_t (*cmdline_strlcat)(char *dest, const char *src, size_t count))
+{
+ size_t total_length = 0, tmp_length;
+
+ if (!IS_ENABLED(CONFIG_GENERIC_CMDLINE))
+ return true;
+
+ if (!IS_ENABLED(CONFIG_CMDLINE_BOOL))
+ return true;
+
+ if (IS_ENABLED(CONFIG_CMDLINE_OVERRIDE))
+ dest[0] = '\0';
+ else
+ total_length += cmdline_strlen(dest);
+
+ tmp_length = cmdline_strlen(append);
+ if (tmp_length > 0) {
+ cmdline_strlcat(dest, append, length);
+ total_length += tmp_length;
+ }
+
+ tmp_length = cmdline_strlen(prepend);
+ if (tmp_length > 0) {
+ cmdline_strlcat(tmp, prepend, length);
+ cmdline_strlcat(tmp, dest, length);
+ dest[0] = '\0';
+ cmdline_strlcat(dest, tmp, length);
+ total_length += tmp_length;
+ }
+
+ tmp[0] = '\0';
+
+ if (total_length > length)
+ return false;
+
+ return true;
+}
+
+#define cmdline_add_builtin(dest) \
+ __cmdline_add_builtin(dest, CMDLINE_TMP, CMDLINE_PREPEND, CMDLINE_APPEND, COMMAND_LINE_SIZE, CMDLINE_STRLEN, CMDLINE_STRLCAT)
+
+#define cmdline_get_static_builtin(dest) \
+ (CMDLINE_STATIC_PREPEND CMDLINE_STATIC_APPEND)
+#endif
diff --git a/init/Kconfig b/init/Kconfig
index 532362fcfe31..941d8fe097ee 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1906,6 +1906,84 @@ 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_EXTRA
+ bool "Reserve more space for inserting prepend and append without recompiling"
+ depends on CMDLINE_BOOL
+ help
+ If set, space for an append and prepend will be reserved in the kernel
+ image. This allows updating or changing the append and prepend to a large
+ string then the kernel was compiled for without recompiling the kernel.
+
+ The maximum size is the command line size for each prepend and append.
+
+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"
diff --git a/lib/Kconfig b/lib/Kconfig
index dc1ab2ed1dc6..82da453238c2 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -706,6 +706,10 @@ config PARMAN
config OBJAGG
tristate "objagg" if COMPILE_TEST
+config TEST_CMDLINE
+ depends on CMDLINE_BOOL && !CMDLINE_OVERRIDE
+ tristate "Test generic command line handling"
+
endmenu
config GENERIC_IOREMAP
diff --git a/lib/Makefile b/lib/Makefile
index ffabc30a27d4..7626821dc046 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -416,3 +416,6 @@ $(obj)/$(TEST_FORTIFY_LOG): $(addprefix $(obj)/, $(TEST_FORTIFY_LOGS)) FORCE
ifeq ($(CONFIG_FORTIFY_SOURCE),y)
$(obj)/string.o: $(obj)/$(TEST_FORTIFY_LOG)
endif
+
+obj-$(CONFIG_TEST_CMDLINE) += test_cmdline1.o
+obj-$(CONFIG_CMDLINE_BOOL) += generic_cmdline.o
diff --git a/lib/generic_cmdline.S b/lib/generic_cmdline.S
new file mode 100644
index 000000000000..223763f9eeb6
--- /dev/null
+++ b/lib/generic_cmdline.S
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/export.h>
+#include <linux/init.h>
+
+#include <asm/setup.h>
+
+ __INITDATA
+
+ .align 8
+ .global cmdline_prepend
+cmdline_prepend:
+ .ifnc CONFIG_CMDLINE_PREPEND,""
+ .ascii CONFIG_CMDLINE_PREPEND
+ .string " "
+ .else
+ .byte 0x0
+ .endif
+#ifdef CONFIG_CMDLINE_EXTRA
+ .space COMMAND_LINE_SIZE - (.-cmdline_prepend)
+ .size cmdline_prepend, COMMAND_LINE_SIZE
+#endif /* CONFIG_CMDLINE_EXTRA */
+
+cmdline_prepend_end:
+ .size cmdline_prepend, (cmdline_prepend_end - cmdline_prepend)
+
+ .align 8
+ .global cmdline_tmp
+cmdline_tmp:
+ .ifnc CONFIG_CMDLINE_PREPEND,""
+ .size cmdline_tmp, COMMAND_LINE_SIZE
+ .space COMMAND_LINE_SIZE
+ .else
+ .byte 0x0
+ .endif
+cmdline_tmp_end:
+ .size cmdline_tmp, (cmdline_tmp_end - cmdline_tmp)
+
+ .align 8
+ .global cmdline_append
+ .size cmdline_append, COMMAND_LINE_SIZE
+cmdline_append:
+ .ifnc CONFIG_CMDLINE_APPEND,""
+ .ascii " "
+ .string CONFIG_CMDLINE_APPEND
+ .else
+ .byte 0x0
+ .endif
+#ifdef CONFIG_CMDLINE_EXTRA
+ .space COMMAND_LINE_SIZE - (.-cmdline_append)
+#endif /* CONFIG_CMDLINE_EXTRA */
+cmdline_append_end:
+ .size cmdline_append, (cmdline_append_end - cmdline_append)
+
diff --git a/lib/test_cmdline1.c b/lib/test_cmdline1.c
new file mode 100644
index 000000000000..ae03cf198165
--- /dev/null
+++ b/lib/test_cmdline1.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/bitmap.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/cmdline.h>
+#include <linux/uaccess.h>
+
+#include "../tools/testing/selftests/kselftest_module.h"
+
+KSTM_MODULE_GLOBALS();
+
+char test1_prepend[] = "prepend ";
+char test1_append[] = " append";
+char test1_bootloader_args[] = "console=ttyS0 log_level=3";
+char test1_result[] = "prepend console=ttyS0 log_level=3 append";
+
+char test2_append[] = " append";
+char test2_bootloader_args[] = "console=ttyS0 log_level=3";
+char test2_result[] = "console=ttyS0 log_level=3 append";
+
+char test3_prepend[] = "prepend ";
+char test3_bootloader_args[] = "console=ttyS0 log_level=3";
+char test3_result[] = "prepend console=ttyS0 log_level=3";
+
+char test4_bootloader_args[] = "console=ttyS0 log_level=3";
+char test4_result[] = "console=ttyS0 log_level=3";
+
+char test5_prepend[] = "prepend ";
+char test5_append[] = " append";
+static char test5_bootloader_args[] =
+"00000000000000 011111111111111 0222222222222222 033333333333333 "
+"10000000000000 111111111111111 1222222222222222 133333333333333 "
+"20000000000000 211111111111111 2222222222222222 233333333333333 "
+"30000000000000 311111111111111 3222222222222222 333333333333333 "
+"40000000000000 411111111111111 4222222222222222 433333333333333 "
+"50000000000000 511111111111111 5222222222222222 533333333333333 "
+"60000000000000 611111111111111 6222222222222222 633333333333333 "
+"70000000000000 711111111111111 7222222222222222 733333333333333";
+static char test5_result[] =
+"prepend 00000000000000 011111111111111 0222222222222222 033333333333333 "
+"10000000000000 111111111111111 1222222222222222 133333333333333 "
+"20000000000000 211111111111111 2222222222222222 233333333333333 "
+"30000000000000 311111111111111 3222222222222222 333333333333333 "
+"40000000000000 411111111111111 4222222222222222 433333333333333 "
+"50000000000000 511111111111111 5222222222222222 533333333333333 "
+"60000000000000 611111111111111 6222222222222222 633333333333333 "
+"70000000000000 711111111111111 7222222222222222 7333333";
+
+char test5_boot_command_line[512];
+
+char test_tmp[COMMAND_LINE_SIZE];
+
+char test_boot_command_line[COMMAND_LINE_SIZE];
+
+static void __init selftest(void)
+{
+ bool result;
+
+ /* Normal operation */
+ strcpy(test_boot_command_line, test1_bootloader_args);
+ test_tmp[0] = '\0';
+ result = __cmdline_add_builtin(test_boot_command_line, test_tmp, test1_prepend, test1_append, COMMAND_LINE_SIZE, CMDLINE_STRLEN, CMDLINE_STRLCAT);
+
+ if (result == true && !strncmp(test_boot_command_line, test1_result, COMMAND_LINE_SIZE)) {
+ pr_info("test1 success.\n");
+ } else {
+ pr_info("test1 failed. OUTPUT BELOW:\n");
+ pr_info("\"%s\"\n", test_boot_command_line);
+ failed_tests++;
+ }
+ total_tests++;
+
+ /* Missing prepend */
+ strcpy(test_boot_command_line, test2_bootloader_args);
+ test_tmp[0] = '\0';
+ result = __cmdline_add_builtin(test_boot_command_line, test_tmp, "", test2_append, COMMAND_LINE_SIZE, CMDLINE_STRLEN, CMDLINE_STRLCAT);
+
+ if (result == true && !strncmp(test_boot_command_line, test2_result, COMMAND_LINE_SIZE)) {
+ pr_info("test2 success.\n");
+ } else {
+ pr_info("test2 failed. OUTPUT BELOW:\n");
+ pr_info("\"%s\"\n", test_boot_command_line);
+ failed_tests++;
+ }
+ total_tests++;
+
+ /* Missing append */
+ strcpy(test_boot_command_line, test3_bootloader_args);
+ test_tmp[0] = '\0';
+ result = __cmdline_add_builtin(test_boot_command_line, test_tmp, test3_prepend, "", COMMAND_LINE_SIZE, CMDLINE_STRLEN, CMDLINE_STRLCAT);
+
+ if (result == true && !strncmp(test_boot_command_line, test3_result, COMMAND_LINE_SIZE)) {
+ pr_info("test3 success.\n");
+ } else {
+ pr_info("test3 failed. OUTPUT BELOW:\n");
+ pr_info("\"%s\"\n", test_boot_command_line);
+ failed_tests++;
+ }
+ total_tests++;
+
+ /* Missing append and prepend */
+ strcpy(test_boot_command_line, test4_bootloader_args);
+ test_tmp[0] = '\0';
+ result = __cmdline_add_builtin(test_boot_command_line, test_tmp, "", "", COMMAND_LINE_SIZE, CMDLINE_STRLEN, CMDLINE_STRLCAT);
+
+ if (result == true && !strncmp(test_boot_command_line, test4_result, COMMAND_LINE_SIZE)) {
+ pr_info("test4 success.\n");
+ } else {
+ pr_info("test4 failed. OUTPUT BELOW:\n");
+ pr_info("\"%s\"\n", test_boot_command_line);
+ failed_tests++;
+ }
+ total_tests++;
+
+ /* Already full boot arguments */
+ strcpy(test5_boot_command_line, test5_bootloader_args);
+ test_tmp[0] = '\0';
+ result = __cmdline_add_builtin(test5_boot_command_line, test_tmp, test5_prepend, test5_append, 512, CMDLINE_STRLEN, CMDLINE_STRLCAT);
+
+ if (result == false && !strncmp(test5_boot_command_line, test5_result, COMMAND_LINE_SIZE)) {
+ pr_info("test5 success.\n");
+ } else {
+ pr_info("test5 failed. OUTPUT BELOW:\n");
+ pr_info("\"%s\"\n", test5_boot_command_line);
+ failed_tests++;
+ }
+ total_tests++;
+}
+
+KSTM_MODULE_LOADERS(cmdline_test);
+MODULE_AUTHOR("Daniel Walker <[email protected]>");
+MODULE_LICENSE("GPL");
--
2.25.1
Le 10/11/2023 à 02:38, 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. MIPS and X86 once has similar systems, then mips added some
> options to allow extending the command line. Powerpc did something
> simiar in adding the ability to extend. Even with mips and powerpc
> enhancement the needs of Cisco are not met on these platforms.
What are those needs, can you list them in the cover letter, and
probably also in here ?
Are those needs specific to Cisco or can they interest the entire Linux
community ?
>
> The code in this commit unifies the 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. The generic code provides both
> append and/or prepend options and provides a way to redefine these
> option after the kernel is compiled.
Explain how.
>
> This code also includes test's which are meant to confirm
> functionality.
Would be better to have test part as a separate patch if possible.
>
> This unified implementation offers the same functionality needed by
> Cisco on all platform which we enable it on.
Cisco ... cisco ... cisco ...
>
> Cc: [email protected]
> Signed-off-by: Ruslan Bilovol <[email protected]>
> Signed-off-by: Daniel Walker <[email protected]>
> ---
> include/linux/cmdline.h | 106 ++++++++++++++++++++++++++++++
> init/Kconfig | 79 +++++++++++++++++++++++
> lib/Kconfig | 4 ++
> lib/Makefile | 3 +
> lib/generic_cmdline.S | 53 +++++++++++++++
> lib/test_cmdline1.c | 139 ++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 384 insertions(+)
> create mode 100644 include/linux/cmdline.h
> create mode 100644 lib/generic_cmdline.S
> create mode 100644 lib/test_cmdline1.c
>
> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
> new file mode 100644
> index 000000000000..a94758a0f257
> --- /dev/null
> +++ b/include/linux/cmdline.h
> @@ -0,0 +1,106 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_CMDLINE_H
> +#define _LINUX_CMDLINE_H
> +/*
> + *
> + * Copyright (C) 2006,2021. Cisco Systems, Inc.
> + *
> + * Generic Append/Prepend cmdline support.
> + */
> +
> +
> +#include <linux/ctype.h>
> +#include <linux/cache.h>
> +#include <asm/setup.h>
> +
> +#ifdef CONFIG_CMDLINE_BOOL
> +extern char cmdline_prepend[];
> +extern char cmdline_append[];
> +extern char cmdline_tmp[];
> +#define CMDLINE_PREPEND cmdline_prepend
> +#define CMDLINE_APPEND cmdline_append
> +#define CMDLINE_TMP cmdline_tmp
> +#define CMDLINE_STATIC_PREPEND CONFIG_CMDLINE_PREPEND
> +#define CMDLINE_STATIC_APPEND CONFIG_CMDLINE_APPEND
Too many macros reduces the readability of code, avoid them when possible.
> +#else
Explain why this else leg is needed. It should be possible to set
default values directly in Kconfig.
> +#define CMDLINE_PREPEND ""
> +#define CMDLINE_APPEND ""
> +#define CMDLINE_TMP ""
> +#define CMDLINE_STATIC_PREPEND ""
> +#define CMDLINE_STATIC_APPEND ""
> +#endif
> +
> +#ifndef CMDLINE_STRLCAT
> +#define CMDLINE_STRLCAT strlcat
> +#endif
> +
> +#ifndef CMDLINE_STRLEN
> +#define CMDLINE_STRLEN strlen
> +#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.
> + * @dest: The destination of the final appended/prepended string
> + * @tmp: temporary space used for prepending
> + * @prepend: string to prepend to @dest
> + * @append: string to append to @dest
> + * @length: the maximum length of the strings above.
> + * @cmdline_strlen: point to a compatible strlen
Remove that function pointer argument and use macros.
> + * @cmdline_strlcat: point to a compatible strlcat
Same
> + * This function returns true when the builtin command line was copied successfully
> + * and false when there was not enough room to copy all parts of the command line.
What happens when it returns false, is it partially copied or nothing is
done ?
> + */
> +static inline bool
> +__cmdline_add_builtin(
> + char *dest,
> + char *tmp,
> + char *prepend,
> + char *append,
> + unsigned long length,
> + size_t (*cmdline_strlen)(const char *s),
> + size_t (*cmdline_strlcat)(char *dest, const char *src, size_t count))
This cmdline feature is used in early deep parts of architectures, so in
a way more or less comparable to linux-mm. Approach with linux-mm has
always been to define macros that can be overriden by architectures.
Please do the same and define cmdline_strlen() and cmdline_strlcat() as
macros that can be overriden by architectures instead of passing
function pointers. And keep macro names as lower case for this type of
macros.
> +{
> + size_t total_length = 0, tmp_length;
Try to use shorter names for local variables, see kernel codying style.
> +
> + if (!IS_ENABLED(CONFIG_GENERIC_CMDLINE))
> + return true;
> +
> + if (!IS_ENABLED(CONFIG_CMDLINE_BOOL))
> + return true;
> +
> + if (IS_ENABLED(CONFIG_CMDLINE_OVERRIDE))
> + dest[0] = '\0';
> + else
> + total_length += cmdline_strlen(dest);
All those Kconfig options should be explained in the commit message,
that would help understanding the patch.
Impact on existing defconfigs should also be taken care of and minimised.
> +
> + tmp_length = cmdline_strlen(append);
> + if (tmp_length > 0) {
> + cmdline_strlcat(dest, append, length);
> + total_length += tmp_length;
> + }
> +
> + tmp_length = cmdline_strlen(prepend);
> + if (tmp_length > 0) {
> + cmdline_strlcat(tmp, prepend, length);
> + cmdline_strlcat(tmp, dest, length);
> + dest[0] = '\0';
> + cmdline_strlcat(dest, tmp, length);
> + total_length += tmp_length;
> + }
> +
> + tmp[0] = '\0';
What's the purpose of setting tmp[0] to 0 ?
> +
> + if (total_length > length)
> + return false;
> +
> + return true;
Can be writen as:
return total_length <= length;
> +}
> +
> +#define cmdline_add_builtin(dest) \
> + __cmdline_add_builtin(dest, CMDLINE_TMP, CMDLINE_PREPEND, CMDLINE_APPEND, COMMAND_LINE_SIZE, CMDLINE_STRLEN, CMDLINE_STRLCAT)
> +
> +#define cmdline_get_static_builtin(dest) \
> + (CMDLINE_STATIC_PREPEND CMDLINE_STATIC_APPEND)
> +#endif
> diff --git a/init/Kconfig b/init/Kconfig
> index 6d35728b94b2..703eed88d140 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1922,6 +1922,85 @@ config TRACEPOINTS
>
> source "kernel/Kconfig.kexec"
>
> +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_EXTRA
> + bool "Reserve more space for inserting prepend and append without recompiling"
> + depends on CMDLINE_BOOL
> + select SYSTEM_EXTRA_CERTIFICATE
> + help
> + If set, space for an append and prepend will be reserved in the kernel
> + image. This allows updating or changing the append and prepend to a large
> + string then the kernel was compiled for without recompiling the kernel.
s/large/larger
s/then/than
> +
> + The maximum size is the command line size for each prepend and append.
> +
> +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
An analysis of what exists on each existing architecture should
demonstrate that you have defined all needed options.
> +
> endmenu # General setup
>
> source "arch/Kconfig"
> diff --git a/lib/Kconfig b/lib/Kconfig
> index c686f4adc124..d520f1aa7c32 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -729,6 +729,10 @@ config PARMAN
> config OBJAGG
> tristate "objagg" if COMPILE_TEST
>
> +config TEST_CMDLINE
> + depends on CMDLINE_BOOL && !CMDLINE_OVERRIDE
> + tristate "Test generic command line handling"
> +
Put the test part in a second patch.
> endmenu
>
> config GENERIC_IOREMAP
> diff --git a/lib/Makefile b/lib/Makefile
> index 740109b6e2c8..aa7b14a0ced7 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -438,3 +438,6 @@ $(obj)/$(TEST_FORTIFY_LOG): $(addprefix $(obj)/, $(TEST_FORTIFY_LOGS)) FORCE
> ifeq ($(CONFIG_FORTIFY_SOURCE),y)
> $(obj)/string.o: $(obj)/$(TEST_FORTIFY_LOG)
> endif
> +
> +obj-$(CONFIG_TEST_CMDLINE) += test_cmdline1.o
> +obj-$(CONFIG_CMDLINE_BOOL) += generic_cmdline.o
> diff --git a/lib/generic_cmdline.S b/lib/generic_cmdline.S
> new file mode 100644
> index 000000000000..223763f9eeb6
> --- /dev/null
> +++ b/lib/generic_cmdline.S
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <linux/export.h>
> +#include <linux/init.h>
> +
> +#include <asm/setup.h>
> +
> + __INITDATA
> +
> + .align 8
> + .global cmdline_prepend
> +cmdline_prepend:
> + .ifnc CONFIG_CMDLINE_PREPEND,""
> + .ascii CONFIG_CMDLINE_PREPEND
> + .string " "
> + .else
> + .byte 0x0
> + .endif
> +#ifdef CONFIG_CMDLINE_EXTRA
> + .space COMMAND_LINE_SIZE - (.-cmdline_prepend)
> + .size cmdline_prepend, COMMAND_LINE_SIZE
> +#endif /* CONFIG_CMDLINE_EXTRA */
> +
> +cmdline_prepend_end:
> + .size cmdline_prepend, (cmdline_prepend_end - cmdline_prepend)
> +
> + .align 8
> + .global cmdline_tmp
> +cmdline_tmp:
> + .ifnc CONFIG_CMDLINE_PREPEND,""
> + .size cmdline_tmp, COMMAND_LINE_SIZE
> + .space COMMAND_LINE_SIZE
> + .else
> + .byte 0x0
> + .endif
> +cmdline_tmp_end:
> + .size cmdline_tmp, (cmdline_tmp_end - cmdline_tmp)
> +
> + .align 8
> + .global cmdline_append
> + .size cmdline_append, COMMAND_LINE_SIZE
> +cmdline_append:
> + .ifnc CONFIG_CMDLINE_APPEND,""
> + .ascii " "
> + .string CONFIG_CMDLINE_APPEND
> + .else
> + .byte 0x0
> + .endif
> +#ifdef CONFIG_CMDLINE_EXTRA
> + .space COMMAND_LINE_SIZE - (.-cmdline_append)
> +#endif /* CONFIG_CMDLINE_EXTRA */
> +cmdline_append_end:
> + .size cmdline_append, (cmdline_append_end - cmdline_append)
> +
Can all this be done in a C file, possibly with inline assembly if
required, instead of an assembly file ?
> diff --git a/lib/test_cmdline1.c b/lib/test_cmdline1.c
> new file mode 100644
> index 000000000000..bcaffcc024e4
> --- /dev/null
> +++ b/lib/test_cmdline1.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0-only
Why 2.0-only ? Not sure it is the preferred licence for new files unless
they are tied to something already existing.
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/bitmap.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/printk.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/cmdline.h>
> +#include <linux/uaccess.h>
> +
> +#include "../tools/testing/selftests/kselftest_module.h"
Is it common practice to include tools/testing/ stuff into kernel ?
> +
> +KSTM_MODULE_GLOBALS();
> +
> +char test1_prepend[] = "prepend ";
> +char test1_append[] = " append";
> +char test1_bootloader_args[] = "console=ttyS0 log_level=3";
> +char test1_result[] = "prepend console=ttyS0 log_level=3 append";
> +
> +char test2_append[] = " append";
> +char test2_bootloader_args[] = "console=ttyS0 log_level=3";
> +char test2_result[] = "console=ttyS0 log_level=3 append";
> +
> +char test3_prepend[] = "prepend ";
> +char test3_bootloader_args[] = "console=ttyS0 log_level=3";
> +char test3_result[] = "prepend console=ttyS0 log_level=3";
> +
> +char test4_bootloader_args[] = "console=ttyS0 log_level=3";
> +char test4_result[] = "console=ttyS0 log_level=3";
> +
> +char test5_prepend[] = "prepend ";
> +char test5_append[] = " append";
> +static char test5_bootloader_args[] =
> +"00000000000000 011111111111111 0222222222222222 033333333333333 "
> +"10000000000000 111111111111111 1222222222222222 133333333333333 "
> +"20000000000000 211111111111111 2222222222222222 233333333333333 "
> +"30000000000000 311111111111111 3222222222222222 333333333333333 "
> +"40000000000000 411111111111111 4222222222222222 433333333333333 "
> +"50000000000000 511111111111111 5222222222222222 533333333333333 "
> +"60000000000000 611111111111111 6222222222222222 633333333333333 "
> +"70000000000000 711111111111111 7222222222222222 733333333333333";
> +static char test5_result[] =
> +"prepend 00000000000000 011111111111111 0222222222222222 033333333333333 "
> +"10000000000000 111111111111111 1222222222222222 133333333333333 "
> +"20000000000000 211111111111111 2222222222222222 233333333333333 "
> +"30000000000000 311111111111111 3222222222222222 333333333333333 "
> +"40000000000000 411111111111111 4222222222222222 433333333333333 "
> +"50000000000000 511111111111111 5222222222222222 533333333333333 "
> +"60000000000000 611111111111111 6222222222222222 633333333333333 "
> +"70000000000000 711111111111111 7222222222222222 7333333";
> +
> +char test5_boot_command_line[COMMAND_LINE_SIZE];
> +
> +char test_tmp[COMMAND_LINE_SIZE];
> +
> +char test_boot_command_line[COMMAND_LINE_SIZE];
> +
> +static void __init selftest(void)
> +{
> + bool result;
> +
> + /* Normal operation */
> + strcpy(test_boot_command_line, test1_bootloader_args);
> + test_tmp[0] = '\0';
> + result = __cmdline_add_builtin(test_boot_command_line, test_tmp, test1_prepend, test1_append, COMMAND_LINE_SIZE, CMDLINE_STRLEN, CMDLINE_STRLCAT);
> +
> + if (result == true && !strncmp(test_boot_command_line, test1_result, COMMAND_LINE_SIZE)) {
> + pr_info("test1 success.\n");
> + } else {
> + pr_info("test1 failed. OUTPUT BELOW:\n");
> + pr_info("\"%s\"\n", test_boot_command_line);
> + failed_tests++;
> + }
> + total_tests++;
> +
> + /* Missing prepend */
> + strcpy(test_boot_command_line, test2_bootloader_args);
> + test_tmp[0] = '\0';
> + result = __cmdline_add_builtin(test_boot_command_line, test_tmp, "", test2_append, COMMAND_LINE_SIZE, CMDLINE_STRLEN, CMDLINE_STRLCAT);
> +
> + if (result == true && !strncmp(test_boot_command_line, test2_result, COMMAND_LINE_SIZE)) {
> + pr_info("test2 success.\n");
> + } else {
> + pr_info("test2 failed. OUTPUT BELOW:\n");
> + pr_info("\"%s\"\n", test_boot_command_line);
> + failed_tests++;
> + }
Can you refactor and avoid repeating those 7 lines five times ?
> + total_tests++;
> +
> + /* Missing append */
> + strcpy(test_boot_command_line, test3_bootloader_args);
> + test_tmp[0] = '\0';
> + result = __cmdline_add_builtin(test_boot_command_line, test_tmp, test3_prepend, "", COMMAND_LINE_SIZE, CMDLINE_STRLEN, CMDLINE_STRLCAT);
> +
> + if (result == true && !strncmp(test_boot_command_line, test3_result, COMMAND_LINE_SIZE)) {
> + pr_info("test3 success.\n");
> + } else {
> + pr_info("test3 failed. OUTPUT BELOW:\n");
> + pr_info("\"%s\"\n", test_boot_command_line);
> + failed_tests++;
> + }
> + total_tests++;
> +
> + /* Missing append and prepend */
> + strcpy(test_boot_command_line, test4_bootloader_args);
> + test_tmp[0] = '\0';
> + result = __cmdline_add_builtin(test_boot_command_line, test_tmp, "", "", COMMAND_LINE_SIZE, CMDLINE_STRLEN, CMDLINE_STRLCAT);
> +
> + if (result == true && !strncmp(test_boot_command_line, test4_result, COMMAND_LINE_SIZE)) {
> + pr_info("test4 success.\n");
> + } else {
> + pr_info("test4 failed. OUTPUT BELOW:\n");
> + pr_info("\"%s\"\n", test_boot_command_line);
> + failed_tests++;
> + }
> + total_tests++;
> +
> + /* Already full boot arguments */
> + strcpy(test5_boot_command_line, test5_bootloader_args);
> + test_tmp[0] = '\0';
> + result = __cmdline_add_builtin(test5_boot_command_line, test_tmp, test5_prepend, test5_append, 512, CMDLINE_STRLEN, CMDLINE_STRLCAT);
> +
> + if (result == false && !strncmp(test5_boot_command_line, test5_result, COMMAND_LINE_SIZE)) {
> + pr_info("test5 success.\n");
> + } else {
> + pr_info("test5 failed. OUTPUT BELOW:\n");
> + pr_info("\"%s\"\n", test5_boot_command_line);
> + failed_tests++;
> + }
> + total_tests++;
> +}
> +
> +KSTM_MODULE_LOADERS(cmdline_test);
> +MODULE_AUTHOR("Daniel Walker <[email protected]>");
> +MODULE_LICENSE("GPL");
On 11/10/2023 7:08 AM, Daniel Walker wrote:
> diff --git a/lib/generic_cmdline.S b/lib/generic_cmdline.S
> new file mode 100644
> index 000000000000..223763f9eeb6
> --- /dev/null
> +++ b/lib/generic_cmdline.S
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <linux/export.h>
> +#include <linux/init.h>
> +
> +#include <asm/setup.h>
> +
> + __INITDATA
> +
> + .align 8
> + .global cmdline_prepend
> +cmdline_prepend:
> + .ifnc CONFIG_CMDLINE_PREPEND,""
> + .ascii CONFIG_CMDLINE_PREPEND
> + .string " "
> + .else
> + .byte 0x0
> + .endif
> +#ifdef CONFIG_CMDLINE_EXTRA
> + .space COMMAND_LINE_SIZE - (.-cmdline_prepend)
> + .size cmdline_prepend, COMMAND_LINE_SIZE
> +#endif /* CONFIG_CMDLINE_EXTRA */
> +
> +cmdline_prepend_end:
> + .size cmdline_prepend, (cmdline_prepend_end - cmdline_prepend)
> +
> + .align 8
> + .global cmdline_tmp
> +cmdline_tmp:
> + .ifnc CONFIG_CMDLINE_PREPEND,""
> + .size cmdline_tmp, COMMAND_LINE_SIZE
> + .space COMMAND_LINE_SIZE
> + .else
> + .byte 0x0
> + .endif
> +cmdline_tmp_end:
> + .size cmdline_tmp, (cmdline_tmp_end - cmdline_tmp)
> +
> + .align 8
> + .global cmdline_append
> + .size cmdline_append, COMMAND_LINE_SIZE
> +cmdline_append:
> + .ifnc CONFIG_CMDLINE_APPEND,""
> + .ascii " "
> + .string CONFIG_CMDLINE_APPEND
> + .else
> + .byte 0x0
> + .endif
> +#ifdef CONFIG_CMDLINE_EXTRA
> + .space COMMAND_LINE_SIZE - (.-cmdline_append)
> +#endif /* CONFIG_CMDLINE_EXTRA */
> +cmdline_append_end:
> + .size cmdline_append, (cmdline_append_end - cmdline_append)
> +
Hi Daniel,
I picked these patches to test a usecase of ours. generic_cmdline.S does
not escape semicolons in the CMDLINE_APPEND and CMDLINE_PREPEND strings.
Take this config snippet for example:
CONFIG_CMDLINE_APPEND="slub_debug=FZP,zs_handle,zspage;FZPU page_owner=on"
CONFIG_CMDLINE_BOOL=y
# CONFIG_CMDLINE_EXTRA is not set
# CONFIG_CMDLINE_OVERRIDE is not set
# CONFIG_CMDLINE_PREPEND is not set
# CONFIG_TEST_CMDLINE is not set
While compiling, the word FZPU is considered as a mnemonic because of
the semicolon preceding it causing parsing to fail:
kernel/lib/generic_cmdline.S: Assembler messages:
kernel/lib/generic_cmdline.S:42: Warning: missing closing `"'
kernel/lib/generic_cmdline.S:42: Error: unknown mnemonic `fzpu' -- `fzpu
page_owner=on",""'
Maybe Christophe's suggestion of moving this code to a C file and using
inline assembly helps mitigate similar problems?
Thanks,
Jaskaran.