From 96434a76fd254248ded19e95dc967d28e65a5edf Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 5 Nov 2020 10:33:37 -0700 Subject: env: Allow returning errors from hdelete_r() At present this function returns 1 on success and 0 on failure. But in the latter case it provides no indication of what went wrong. If an attempt is made to delete a non-existent variable, the caller may want to ignore this error. This happens when setting a non-existent variable to "", for example. Update the function to return 0 on success and a useful error code on failure. Add a function comment too. Make sure that env_set() does not return an error if it is deleting a variable that doesn't exist. We could update env_set() to return useful error numbers also, but that is beyond the scope of this change. Signed-off-by: Simon Glass wip --- test/env/hashtable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'test') diff --git a/test/env/hashtable.c b/test/env/hashtable.c index 339cc19ba14..70102f9121c 100644 --- a/test/env/hashtable.c +++ b/test/env/hashtable.c @@ -80,7 +80,7 @@ static int htab_create_delete(struct unit_test_state *uts, ut_asserteq_str(key, ritem->key); ut_asserteq_str(key, ritem->data); - ut_asserteq(1, hdelete_r(key, htab, 0)); + ut_asserteq(0, hdelete_r(key, htab, 0)); } return 0; -- cgit v1.3.1 From f158ba15ee0f9f756193b60420adfdc0a9c1eb96 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 5 Nov 2020 10:33:38 -0700 Subject: bootm: Add tests for fixup_silent_linux() This function currently has no tests. Export it so that we can implement a simple test on sandbox. Use IS_ENABLED() to remove the unused code, instead #ifdef. Signed-off-by: Simon Glass --- arch/Kconfig | 1 + common/bootm.c | 14 ++++++------ include/bootm.h | 3 +++ include/test/suites.h | 1 + test/Makefile | 1 + test/bootm.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++ test/cmd_ut.c | 1 + 7 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 test/bootm.c (limited to 'test') diff --git a/arch/Kconfig b/arch/Kconfig index 3aa99e08fce..accd4df5b0c 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -146,6 +146,7 @@ config SANDBOX imply ACPI_PMC_SANDBOX imply CMD_PMC imply CMD_CLONE + imply SILENT_CONSOLE config SH bool "SuperH architecture" diff --git a/common/bootm.c b/common/bootm.c index 167eea4a1e9..0d36c572101 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -465,18 +465,21 @@ ulong bootm_disable_interrupts(void) return iflag; } -#if defined(CONFIG_SILENT_CONSOLE) && !defined(CONFIG_SILENT_U_BOOT_ONLY) - #define CONSOLE_ARG "console=" #define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1) -static void fixup_silent_linux(void) +void fixup_silent_linux(void) { char *buf; const char *env_val; - char *cmdline = env_get("bootargs"); + char *cmdline; int want_silent; + if (!IS_ENABLED(CONFIG_SILENT_CONSOLE) && + !IS_ENABLED(CONFIG_SILENT_U_BOOT_ONLY)) + return; + cmdline = env_get("bootargs"); + /* * Only fix cmdline when requested. The environment variable can be: * @@ -523,7 +526,6 @@ static void fixup_silent_linux(void) debug("after silent fix-up: %s\n", env_val); free(buf); } -#endif /* CONFIG_SILENT_CONSOLE */ /** * Execute selected states of the bootm command. @@ -627,10 +629,8 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc, if (!ret && (states & BOOTM_STATE_OS_BD_T)) ret = boot_fn(BOOTM_STATE_OS_BD_T, argc, argv, images); if (!ret && (states & BOOTM_STATE_OS_PREP)) { -#if defined(CONFIG_SILENT_CONSOLE) && !defined(CONFIG_SILENT_U_BOOT_ONLY) if (images->os.os == IH_OS_LINUX) fixup_silent_linux(); -#endif ret = boot_fn(BOOTM_STATE_OS_PREP, argc, argv, images); } diff --git a/include/bootm.h b/include/bootm.h index a812a6bf24f..6d675e64559 100644 --- a/include/bootm.h +++ b/include/bootm.h @@ -85,4 +85,7 @@ void arch_preboot_os(void); */ void board_preboot_os(void); +/* Adjust the 'bootargs' to ensure that Linux boots silently, if required */ +void fixup_silent_linux(void); + #endif diff --git a/include/test/suites.h b/include/test/suites.h index 5c97846e7f5..52e8fc8155a 100644 --- a/include/test/suites.h +++ b/include/test/suites.h @@ -26,6 +26,7 @@ int cmd_ut_category(const char *name, const char *prefix, struct unit_test *tests, int n_ents, int argc, char *const argv[]); +int do_ut_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_bloblist(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_compression(struct cmd_tbl *cmdtp, int flag, int argc, diff --git a/test/Makefile b/test/Makefile index 8296734eb3c..d4323f99634 100644 --- a/test/Makefile +++ b/test/Makefile @@ -5,6 +5,7 @@ ifneq ($(CONFIG_SANDBOX),) obj-$(CONFIG_$(SPL_)CMDLINE) += bloblist.o endif +obj-$(CONFIG_$(SPL_)CMDLINE) += bootm.o obj-$(CONFIG_$(SPL_)CMDLINE) += cmd/ obj-$(CONFIG_$(SPL_)CMDLINE) += cmd_ut.o obj-$(CONFIG_$(SPL_)CMDLINE) += command_ut.o diff --git a/test/bootm.c b/test/bootm.c new file mode 100644 index 00000000000..59d16cb3df6 --- /dev/null +++ b/test/bootm.c @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Tests for bootm routines + * + * Copyright 2020 Google LLC + */ + +#include +#include +#include +#include +#include + +DECLARE_GLOBAL_DATA_PTR; + +#define BOOTM_TEST(_name, _flags) UNIT_TEST(_name, _flags, bootm_test) + +#define CONSOLE_STR "console=/dev/ttyS0" + +/* Test silent processing in the bootargs variable */ +static int bootm_test_silent_var(struct unit_test_state *uts) +{ + /* 'silent_linux' not set should do nothing */ + env_set("silent_linux", NULL); + env_set("bootargs", CONSOLE_STR); + fixup_silent_linux(); + ut_asserteq_str(CONSOLE_STR, env_get("bootargs")); + + env_set("bootargs", NULL); + fixup_silent_linux(); + ut_assertnull(env_get("bootargs")); + + ut_assertok(env_set("silent_linux", "no")); + env_set("bootargs", CONSOLE_STR); + fixup_silent_linux(); + ut_asserteq_str(CONSOLE_STR, env_get("bootargs")); + + ut_assertok(env_set("silent_linux", "yes")); + env_set("bootargs", CONSOLE_STR); + fixup_silent_linux(); + ut_asserteq_str("console=", env_get("bootargs")); + + /* Empty buffer should still add the string */ + env_set("bootargs", NULL); + fixup_silent_linux(); + ut_asserteq_str("console=", env_get("bootargs")); + + return 0; +} +BOOTM_TEST(bootm_test_silent_var, 0); + +int do_ut_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +{ + struct unit_test *tests = ll_entry_start(struct unit_test, bootm_test); + const int n_ents = ll_entry_count(struct unit_test, bootm_test); + + return cmd_ut_category("bootm", "bootm_test_", tests, n_ents, + argc, argv); +} diff --git a/test/cmd_ut.c b/test/cmd_ut.c index f79109e6f8e..fad1c899a4e 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -88,6 +88,7 @@ static struct cmd_tbl cmd_ut_sub[] = { "", ""), U_BOOT_CMD_MKENT(bloblist, CONFIG_SYS_MAXARGS, 1, do_ut_bloblist, "", ""), + U_BOOT_CMD_MKENT(bootm, CONFIG_SYS_MAXARGS, 1, do_ut_bootm, "", ""), U_BOOT_CMD_MKENT(str, CONFIG_SYS_MAXARGS, 1, do_ut_str, "", ""), #endif -- cgit v1.3.1 From 4ae42643d0d71dbb5af45d19fa05b7a6807150c0 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 5 Nov 2020 10:33:39 -0700 Subject: bootm: Update fixup_silent_linux() to return an error At present this function fails silently on error. Update it to produce an error code. Report this error to the user and abort the boot, since it likely will prevent a successful start. No tests are added at this stage, since additional refactoring is taking place in subsequent patches. Signed-off-by: Simon Glass --- common/bootm.c | 22 +++++++++++++++------- include/bootm.h | 11 +++++++++-- test/bootm.c | 10 +++++----- 3 files changed, 29 insertions(+), 14 deletions(-) (limited to 'test') diff --git a/common/bootm.c b/common/bootm.c index 0d36c572101..950ff7cff62 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -468,7 +468,7 @@ ulong bootm_disable_interrupts(void) #define CONSOLE_ARG "console=" #define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1) -void fixup_silent_linux(void) +int fixup_silent_linux(void) { char *buf; const char *env_val; @@ -477,7 +477,7 @@ void fixup_silent_linux(void) if (!IS_ENABLED(CONFIG_SILENT_CONSOLE) && !IS_ENABLED(CONFIG_SILENT_U_BOOT_ONLY)) - return; + return 0; cmdline = env_get("bootargs"); /* @@ -489,9 +489,9 @@ void fixup_silent_linux(void) */ want_silent = env_get_yesno("silent_linux"); if (want_silent == 0) - return; + return 0; else if (want_silent == -1 && !(gd->flags & GD_FLG_SILENT)) - return; + return 0; debug("before silent fix-up: %s\n", cmdline); if (cmdline && (cmdline[0] != '\0')) { @@ -501,7 +501,7 @@ void fixup_silent_linux(void) buf = malloc(strlen(cmdline) + 1 + CONSOLE_ARG_LEN + 1); if (!buf) { debug("%s: out of memory\n", __func__); - return; + return -ENOSPC; } if (start) { @@ -525,6 +525,8 @@ void fixup_silent_linux(void) env_set("bootargs", env_val); debug("after silent fix-up: %s\n", env_val); free(buf); + + return 0; } /** @@ -629,8 +631,14 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc, if (!ret && (states & BOOTM_STATE_OS_BD_T)) ret = boot_fn(BOOTM_STATE_OS_BD_T, argc, argv, images); if (!ret && (states & BOOTM_STATE_OS_PREP)) { - if (images->os.os == IH_OS_LINUX) - fixup_silent_linux(); + if (images->os.os == IH_OS_LINUX) { + ret = fixup_silent_linux(); + if (ret) { + printf("Cmdline setup failed (err=%d)\n", ret); + ret = CMD_RET_FAILURE; + goto err; + } + } ret = boot_fn(BOOTM_STATE_OS_PREP, argc, argv, images); } diff --git a/include/bootm.h b/include/bootm.h index 6d675e64559..438829af0fe 100644 --- a/include/bootm.h +++ b/include/bootm.h @@ -85,7 +85,14 @@ void arch_preboot_os(void); */ void board_preboot_os(void); -/* Adjust the 'bootargs' to ensure that Linux boots silently, if required */ -void fixup_silent_linux(void); +/* + * fixup_silent_linux() - Process fix-ups for the command line + * + * Updates the 'bootargs' envvar as required. This handles making Linux boot + * silently if requested ('silent_linux' envvar) + * + * @return 0 if OK, -ENOMEM if out of memory + */ +int fixup_silent_linux(void); #endif diff --git a/test/bootm.c b/test/bootm.c index 59d16cb3df6..ab1711609ba 100644 --- a/test/bootm.c +++ b/test/bootm.c @@ -23,26 +23,26 @@ static int bootm_test_silent_var(struct unit_test_state *uts) /* 'silent_linux' not set should do nothing */ env_set("silent_linux", NULL); env_set("bootargs", CONSOLE_STR); - fixup_silent_linux(); + ut_assertok(fixup_silent_linux()); ut_asserteq_str(CONSOLE_STR, env_get("bootargs")); env_set("bootargs", NULL); - fixup_silent_linux(); + ut_assertok(fixup_silent_linux()); ut_assertnull(env_get("bootargs")); ut_assertok(env_set("silent_linux", "no")); env_set("bootargs", CONSOLE_STR); - fixup_silent_linux(); + ut_assertok(fixup_silent_linux()); ut_asserteq_str(CONSOLE_STR, env_get("bootargs")); ut_assertok(env_set("silent_linux", "yes")); env_set("bootargs", CONSOLE_STR); - fixup_silent_linux(); + ut_assertok(fixup_silent_linux()); ut_asserteq_str("console=", env_get("bootargs")); /* Empty buffer should still add the string */ env_set("bootargs", NULL); - fixup_silent_linux(); + ut_assertok(fixup_silent_linux()); ut_asserteq_str("console=", env_get("bootargs")); return 0; -- cgit v1.3.1 From 4dcb81545ab09b6cb4ee1f3c870fb3131984cf6f Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 5 Nov 2020 10:33:40 -0700 Subject: bootm: Rename fixup_silent_linux() We want to add more processing to this function. Before doing so, rename it to bootm_process_cmdline_env(), which is more generic. Signed-off-by: Simon Glass --- common/bootm.c | 4 ++-- include/bootm.h | 4 ++-- test/bootm.c | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) (limited to 'test') diff --git a/common/bootm.c b/common/bootm.c index 950ff7cff62..54f64128f86 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -468,7 +468,7 @@ ulong bootm_disable_interrupts(void) #define CONSOLE_ARG "console=" #define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1) -int fixup_silent_linux(void) +int bootm_process_cmdline_env(void) { char *buf; const char *env_val; @@ -632,7 +632,7 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc, ret = boot_fn(BOOTM_STATE_OS_BD_T, argc, argv, images); if (!ret && (states & BOOTM_STATE_OS_PREP)) { if (images->os.os == IH_OS_LINUX) { - ret = fixup_silent_linux(); + ret = bootm_process_cmdline_env(); if (ret) { printf("Cmdline setup failed (err=%d)\n", ret); ret = CMD_RET_FAILURE; diff --git a/include/bootm.h b/include/bootm.h index 438829af0fe..35c27ab9609 100644 --- a/include/bootm.h +++ b/include/bootm.h @@ -86,13 +86,13 @@ void arch_preboot_os(void); void board_preboot_os(void); /* - * fixup_silent_linux() - Process fix-ups for the command line + * bootm_process_cmdline_env() - Process fix-ups for the command line * * Updates the 'bootargs' envvar as required. This handles making Linux boot * silently if requested ('silent_linux' envvar) * * @return 0 if OK, -ENOMEM if out of memory */ -int fixup_silent_linux(void); +int bootm_process_cmdline_env(void); #endif diff --git a/test/bootm.c b/test/bootm.c index ab1711609ba..b69bfad4f67 100644 --- a/test/bootm.c +++ b/test/bootm.c @@ -23,26 +23,26 @@ static int bootm_test_silent_var(struct unit_test_state *uts) /* 'silent_linux' not set should do nothing */ env_set("silent_linux", NULL); env_set("bootargs", CONSOLE_STR); - ut_assertok(fixup_silent_linux()); + ut_assertok(bootm_process_cmdline_env()); ut_asserteq_str(CONSOLE_STR, env_get("bootargs")); env_set("bootargs", NULL); - ut_assertok(fixup_silent_linux()); + ut_assertok(bootm_process_cmdline_env()); ut_assertnull(env_get("bootargs")); ut_assertok(env_set("silent_linux", "no")); env_set("bootargs", CONSOLE_STR); - ut_assertok(fixup_silent_linux()); + ut_assertok(bootm_process_cmdline_env()); ut_asserteq_str(CONSOLE_STR, env_get("bootargs")); ut_assertok(env_set("silent_linux", "yes")); env_set("bootargs", CONSOLE_STR); - ut_assertok(fixup_silent_linux()); + ut_assertok(bootm_process_cmdline_env()); ut_asserteq_str("console=", env_get("bootargs")); /* Empty buffer should still add the string */ env_set("bootargs", NULL); - ut_assertok(fixup_silent_linux()); + ut_assertok(bootm_process_cmdline_env()); ut_asserteq_str("console=", env_get("bootargs")); return 0; -- cgit v1.3.1 From d9477a0a4d4c4561941007a8b3db588385aaad07 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 5 Nov 2020 10:33:41 -0700 Subject: bootm: Add a bool parameter to bootm_process_cmdline_env() This function will soon do more than just handle the 'silent linux' feature. As a first step, update it to take a boolean parameter, indicating whether or not the processing is required. Signed-off-by: Simon Glass --- common/bootm.c | 20 ++++++++++---------- include/bootm.h | 3 ++- test/bootm.c | 10 +++++----- 3 files changed, 17 insertions(+), 16 deletions(-) (limited to 'test') diff --git a/common/bootm.c b/common/bootm.c index 54f64128f86..9b0c81d6534 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -468,15 +468,17 @@ ulong bootm_disable_interrupts(void) #define CONSOLE_ARG "console=" #define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1) -int bootm_process_cmdline_env(void) +int bootm_process_cmdline_env(bool do_silent) { char *buf; const char *env_val; char *cmdline; int want_silent; - if (!IS_ENABLED(CONFIG_SILENT_CONSOLE) && - !IS_ENABLED(CONFIG_SILENT_U_BOOT_ONLY)) + /* First check if any action is needed */ + do_silent = IS_ENABLED(CONFIG_SILENT_CONSOLE) && + !IS_ENABLED(CONFIG_SILENT_U_BOOT_ONLY) && do_silent; + if (!do_silent) return 0; cmdline = env_get("bootargs"); @@ -631,13 +633,11 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc, if (!ret && (states & BOOTM_STATE_OS_BD_T)) ret = boot_fn(BOOTM_STATE_OS_BD_T, argc, argv, images); if (!ret && (states & BOOTM_STATE_OS_PREP)) { - if (images->os.os == IH_OS_LINUX) { - ret = bootm_process_cmdline_env(); - if (ret) { - printf("Cmdline setup failed (err=%d)\n", ret); - ret = CMD_RET_FAILURE; - goto err; - } + ret = bootm_process_cmdline_env(images->os.os == IH_OS_LINUX); + if (ret) { + printf("Cmdline setup failed (err=%d)\n", ret); + ret = CMD_RET_FAILURE; + goto err; } ret = boot_fn(BOOTM_STATE_OS_PREP, argc, argv, images); } diff --git a/include/bootm.h b/include/bootm.h index 35c27ab9609..f12ee2b3cb3 100644 --- a/include/bootm.h +++ b/include/bootm.h @@ -91,8 +91,9 @@ void board_preboot_os(void); * Updates the 'bootargs' envvar as required. This handles making Linux boot * silently if requested ('silent_linux' envvar) * + * @do_silent: Process bootargs for silent console * @return 0 if OK, -ENOMEM if out of memory */ -int bootm_process_cmdline_env(void); +int bootm_process_cmdline_env(bool do_silent); #endif diff --git a/test/bootm.c b/test/bootm.c index b69bfad4f67..c203f0acd60 100644 --- a/test/bootm.c +++ b/test/bootm.c @@ -23,26 +23,26 @@ static int bootm_test_silent_var(struct unit_test_state *uts) /* 'silent_linux' not set should do nothing */ env_set("silent_linux", NULL); env_set("bootargs", CONSOLE_STR); - ut_assertok(bootm_process_cmdline_env()); + ut_assertok(bootm_process_cmdline_env(true)); ut_asserteq_str(CONSOLE_STR, env_get("bootargs")); env_set("bootargs", NULL); - ut_assertok(bootm_process_cmdline_env()); + ut_assertok(bootm_process_cmdline_env(true)); ut_assertnull(env_get("bootargs")); ut_assertok(env_set("silent_linux", "no")); env_set("bootargs", CONSOLE_STR); - ut_assertok(bootm_process_cmdline_env()); + ut_assertok(bootm_process_cmdline_env(true)); ut_asserteq_str(CONSOLE_STR, env_get("bootargs")); ut_assertok(env_set("silent_linux", "yes")); env_set("bootargs", CONSOLE_STR); - ut_assertok(bootm_process_cmdline_env()); + ut_assertok(bootm_process_cmdline_env(true)); ut_asserteq_str("console=", env_get("bootargs")); /* Empty buffer should still add the string */ env_set("bootargs", NULL); - ut_assertok(bootm_process_cmdline_env()); + ut_assertok(bootm_process_cmdline_env(true)); ut_asserteq_str("console=", env_get("bootargs")); return 0; -- cgit v1.3.1 From b3c01678fdb15c63b231743481b9b77c7c4f8549 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 5 Nov 2020 10:33:44 -0700 Subject: bootm: Update bootm_process_cmdline_env() to use flags At present only one transformation is supported: making the Linux console silent. To prepare for adding more, convert the boolean parameter into a flag value. Signed-off-by: Simon Glass --- common/bootm.c | 8 +++++--- include/bootm.h | 11 +++++++++-- test/bootm.c | 10 +++++----- 3 files changed, 19 insertions(+), 10 deletions(-) (limited to 'test') diff --git a/common/bootm.c b/common/bootm.c index 4fa909f23bc..912ed906ef3 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -542,16 +542,17 @@ static int fixup_silent_linux(char *buf, int maxlen) return 0; } -int bootm_process_cmdline_env(bool do_silent) +int bootm_process_cmdline_env(int flags) { const int maxlen = MAX_CMDLINE_SIZE; + bool do_silent; const char *env; char *buf; int ret; /* First check if any action is needed */ do_silent = IS_ENABLED(CONFIG_SILENT_CONSOLE) && - !IS_ENABLED(CONFIG_SILENT_U_BOOT_ONLY) && do_silent; + !IS_ENABLED(CONFIG_SILENT_U_BOOT_ONLY) && (flags & BOOTM_CL_SILENT); if (!do_silent) return 0; @@ -685,7 +686,8 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc, if (!ret && (states & BOOTM_STATE_OS_BD_T)) ret = boot_fn(BOOTM_STATE_OS_BD_T, argc, argv, images); if (!ret && (states & BOOTM_STATE_OS_PREP)) { - ret = bootm_process_cmdline_env(images->os.os == IH_OS_LINUX); + ret = bootm_process_cmdline_env(images->os.os == IH_OS_LINUX ? + BOOTM_CL_SILENT : 0); if (ret) { printf("Cmdline setup failed (err=%d)\n", ret); ret = CMD_RET_FAILURE; diff --git a/include/bootm.h b/include/bootm.h index f12ee2b3cb3..4876d7b2882 100644 --- a/include/bootm.h +++ b/include/bootm.h @@ -75,6 +75,13 @@ void board_quiesce_devices(void); */ void switch_to_non_secure_mode(void); +/* Flags to control bootm_process_cmdline() */ +enum bootm_cmdline_t { + BOOTM_CL_SILENT = 1 << 0, /* Do silent console processing */ + + BOOTM_CL_ALL = 1, /* All substitutions */ +}; + /** * arch_preboot_os() - arch specific configuration before booting */ @@ -91,9 +98,9 @@ void board_preboot_os(void); * Updates the 'bootargs' envvar as required. This handles making Linux boot * silently if requested ('silent_linux' envvar) * - * @do_silent: Process bootargs for silent console + * @flags: Flags to control what happens (see bootm_cmdline_t) * @return 0 if OK, -ENOMEM if out of memory */ -int bootm_process_cmdline_env(bool do_silent); +int bootm_process_cmdline_env(int flags); #endif diff --git a/test/bootm.c b/test/bootm.c index c203f0acd60..ba08920bb17 100644 --- a/test/bootm.c +++ b/test/bootm.c @@ -23,26 +23,26 @@ static int bootm_test_silent_var(struct unit_test_state *uts) /* 'silent_linux' not set should do nothing */ env_set("silent_linux", NULL); env_set("bootargs", CONSOLE_STR); - ut_assertok(bootm_process_cmdline_env(true)); + ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); ut_asserteq_str(CONSOLE_STR, env_get("bootargs")); env_set("bootargs", NULL); - ut_assertok(bootm_process_cmdline_env(true)); + ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); ut_assertnull(env_get("bootargs")); ut_assertok(env_set("silent_linux", "no")); env_set("bootargs", CONSOLE_STR); - ut_assertok(bootm_process_cmdline_env(true)); + ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); ut_asserteq_str(CONSOLE_STR, env_get("bootargs")); ut_assertok(env_set("silent_linux", "yes")); env_set("bootargs", CONSOLE_STR); - ut_assertok(bootm_process_cmdline_env(true)); + ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); ut_asserteq_str("console=", env_get("bootargs")); /* Empty buffer should still add the string */ env_set("bootargs", NULL); - ut_assertok(bootm_process_cmdline_env(true)); + ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); ut_asserteq_str("console=", env_get("bootargs")); return 0; -- cgit v1.3.1 From 4448fe8e4e7cc4dc5336a2d27fa6048057eaf1a6 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 5 Nov 2020 10:33:45 -0700 Subject: bootm: Allow updating the bootargs in a buffer At present we only support updating the 'bootargs' environment variable. Add another function to update a buffer instead. This will allow zimage to use this feature. Also add a lot more tests to cover various cases. Signed-off-by: Simon Glass --- common/bootm.c | 18 ++++++++- include/bootm.h | 16 ++++++++ test/bootm.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 132 insertions(+), 16 deletions(-) (limited to 'test') diff --git a/common/bootm.c b/common/bootm.c index 912ed906ef3..020449db3fb 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -542,6 +542,22 @@ static int fixup_silent_linux(char *buf, int maxlen) return 0; } +int bootm_process_cmdline(char *buf, int maxlen, int flags) +{ + int ret; + + /* Check config first to enable compiler to eliminate code */ + if (IS_ENABLED(CONFIG_SILENT_CONSOLE) && + !IS_ENABLED(CONFIG_SILENT_U_BOOT_ONLY) && + (flags & BOOTM_CL_SILENT)) { + ret = fixup_silent_linux(buf, maxlen); + if (ret) + return log_msg_ret("silent", ret); + } + + return 0; +} + int bootm_process_cmdline_env(int flags) { const int maxlen = MAX_CMDLINE_SIZE; @@ -566,7 +582,7 @@ int bootm_process_cmdline_env(int flags) strcpy(buf, env); else *buf = '\0'; - ret = fixup_silent_linux(buf, maxlen); + ret = bootm_process_cmdline(buf, maxlen, flags); if (!ret) { ret = env_set("bootargs", buf); diff --git a/include/bootm.h b/include/bootm.h index 4876d7b2882..8d95fb2a90a 100644 --- a/include/bootm.h +++ b/include/bootm.h @@ -93,6 +93,22 @@ void arch_preboot_os(void); void board_preboot_os(void); /* + * bootm_process_cmdline() - Process fix-ups for the command line + * + * This handles: making Linux boot silently if requested ('silent_linux' envvar) + * + * @maxlen must provide enough space for the string being processed plus the + * resulting string + * + * @buf: buffer holding commandline string to adjust + * @maxlen: Maximum length of buffer at @buf (including \0) + * @flags: Flags to control what happens (see bootm_cmdline_t) + * @return 0 if OK, -ENOMEM if out of memory, -ENOSPC if the commandline is too + * long + */ +int bootm_process_cmdline(char *buf, int maxlen, int flags); + +/** * bootm_process_cmdline_env() - Process fix-ups for the command line * * Updates the 'bootargs' envvar as required. This handles making Linux boot diff --git a/test/bootm.c b/test/bootm.c index ba08920bb17..d0b29441d65 100644 --- a/test/bootm.c +++ b/test/bootm.c @@ -15,33 +15,117 @@ DECLARE_GLOBAL_DATA_PTR; #define BOOTM_TEST(_name, _flags) UNIT_TEST(_name, _flags, bootm_test) +enum { + BUF_SIZE = 1024, +}; + #define CONSOLE_STR "console=/dev/ttyS0" -/* Test silent processing in the bootargs variable */ -static int bootm_test_silent_var(struct unit_test_state *uts) +/* Test cmdline processing where nothing happens */ +static int bootm_test_nop(struct unit_test_state *uts) +{ + char buf[BUF_SIZE]; + + *buf = '\0'; + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, true)); + ut_asserteq_str("", buf); + + strcpy(buf, "test"); + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, true)); + ut_asserteq_str("test", buf); + + return 0; +} +BOOTM_TEST(bootm_test_nop, 0); + +/* Test cmdline processing when out of space */ +static int bootm_test_nospace(struct unit_test_state *uts) +{ + char buf[BUF_SIZE]; + + /* Zero buffer size */ + *buf = '\0'; + ut_asserteq(-ENOSPC, bootm_process_cmdline(buf, 0, true)); + + /* Buffer string not terminated */ + memset(buf, 'a', BUF_SIZE); + ut_asserteq(-ENOSPC, bootm_process_cmdline(buf, BUF_SIZE, true)); + + /* Not enough space to copy string */ + memset(buf, '\0', BUF_SIZE); + memset(buf, 'a', BUF_SIZE / 2); + ut_asserteq(-ENOSPC, bootm_process_cmdline(buf, BUF_SIZE, true)); + + /* Just enough space */ + memset(buf, '\0', BUF_SIZE); + memset(buf, 'a', BUF_SIZE / 2 - 1); + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, true)); + + return 0; +} +BOOTM_TEST(bootm_test_nospace, 0); + +/* Test silent processing */ +static int bootm_test_silent(struct unit_test_state *uts) { + char buf[BUF_SIZE]; + /* 'silent_linux' not set should do nothing */ env_set("silent_linux", NULL); - env_set("bootargs", CONSOLE_STR); - ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); - ut_asserteq_str(CONSOLE_STR, env_get("bootargs")); - - env_set("bootargs", NULL); - ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); - ut_assertnull(env_get("bootargs")); + strcpy(buf, CONSOLE_STR); + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, BOOTM_CL_SILENT)); + ut_asserteq_str(CONSOLE_STR, buf); ut_assertok(env_set("silent_linux", "no")); - env_set("bootargs", CONSOLE_STR); - ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); - ut_asserteq_str(CONSOLE_STR, env_get("bootargs")); + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, BOOTM_CL_SILENT)); + ut_asserteq_str(CONSOLE_STR, buf); ut_assertok(env_set("silent_linux", "yes")); - env_set("bootargs", CONSOLE_STR); - ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); - ut_asserteq_str("console=", env_get("bootargs")); + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, BOOTM_CL_SILENT)); + ut_asserteq_str("console=", buf); /* Empty buffer should still add the string */ + *buf = '\0'; + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, BOOTM_CL_SILENT)); + ut_asserteq_str("console=", buf); + + /* Check nothing happens when do_silent is false */ + *buf = '\0'; + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, 0)); + ut_asserteq_str("", buf); + + /* Not enough space */ + *buf = '\0'; + ut_asserteq(-ENOSPC, bootm_process_cmdline(buf, 8, BOOTM_CL_SILENT)); + + /* Just enough space */ + *buf = '\0'; + ut_assertok(bootm_process_cmdline(buf, 9, BOOTM_CL_SILENT)); + + /* add at end */ + strcpy(buf, "something"); + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, BOOTM_CL_SILENT)); + ut_asserteq_str("something console=", buf); + + /* change at start */ + strcpy(buf, CONSOLE_STR " something"); + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, BOOTM_CL_SILENT)); + ut_asserteq_str("console= something", buf); + + return 0; +} +BOOTM_TEST(bootm_test_silent, 0); + +/* Test silent processing in the bootargs variable */ +static int bootm_test_silent_var(struct unit_test_state *uts) +{ env_set("bootargs", NULL); + env_set("silent_linux", NULL); + ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); + ut_assertnull(env_get("bootargs")); + + env_set("bootargs", CONSOLE_STR); + env_set("silent_linux", "yes"); ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); ut_asserteq_str("console=", env_get("bootargs")); -- cgit v1.3.1 From 51bb33846ad2b045799d2c43ca773fafa36e6ec8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 5 Nov 2020 10:33:48 -0700 Subject: bootm: Support string substitution in bootargs In some cases it is necessary to pass parameters to Linux so that it will boot correctly. For example, the rootdev parameter is often used to specify the root device. However the root device may change depending on whence U-Boot loads the kernel. At present it is necessary to build up the command line by adding device strings to it one by one. It is often more convenient to provide a template for bootargs, with U-Boot doing the substitution from other environment variables. Add a way to substitute strings in the bootargs variable. This allows things like "rootdev=${rootdev}" to be used in bootargs, with the ${rootdev} substitution providing the UUID of the root device. For example, to substitute the GUID of the kernel partition: setenv bootargs "console=/dev/ttyS0 rootdev=${uuid}/PARTNROFF=1 kern_guid=${uuid}" part uuid mmc 2:2 uuid bootm This is particularly useful when the command line from another place. For example, Chrome OS stores the command line next to the kernel itself. It depends on the kernel version being used as well as the hardware features, so it is extremely difficult to devise a U-Boot script that works on all boards and kernel versions. With this feature, the command line can be read from disk and used directly, with a few substitutions set up. Signed-off-by: Simon Glass --- arch/Kconfig | 1 + common/Kconfig.boot | 17 ++++++++ common/bootm.c | 38 ++++++++++++++++-- include/bootm.h | 14 +++++-- test/bootm.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++--- 5 files changed, 172 insertions(+), 12 deletions(-) (limited to 'test') diff --git a/arch/Kconfig b/arch/Kconfig index accd4df5b0c..356193f9ece 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -147,6 +147,7 @@ config SANDBOX imply CMD_PMC imply CMD_CLONE imply SILENT_CONSOLE + imply BOOTARGS_SUBST config SH bool "SuperH architecture" diff --git a/common/Kconfig.boot b/common/Kconfig.boot index 3f6d9c1a258..58e98548de0 100644 --- a/common/Kconfig.boot +++ b/common/Kconfig.boot @@ -865,6 +865,23 @@ config BOOTARGS CONFIG_BOOTARGS goes into the environment value "bootargs". Note that this value will also override the "chosen" node in FDT blob. +config BOOTARGS_SUBST + bool "Support substituting strings in boot arguments" + help + This allows substituting string values in the boot arguments. These + are applied after the commandline has been built. + + One use for this is to insert the root-disk UUID into the command + line where bootargs contains "root=${uuid}" + + setenv bootargs "console= root=${uuid}" + # Set the 'uuid' environment variable + part uuid mmc 2:2 uuid + + # Command-line substitution will put the real uuid into the + # kernel command line + bootm + config USE_BOOTCOMMAND bool "Enable a default value for bootcmd" help diff --git a/common/bootm.c b/common/bootm.c index 020449db3fb..8298693900d 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -7,6 +7,7 @@ #ifndef USE_HOSTCC #include #include +#include #include #include #include @@ -542,6 +543,33 @@ static int fixup_silent_linux(char *buf, int maxlen) return 0; } +/** + * process_subst() - Handle substitution of ${...} fields in the environment + * + * Handle variable substitution in the provided buffer + * + * @buf: Buffer containing the string to process + * @maxlen: Maximum length of buffer + * @return 0 if OK, -ENOSPC if @maxlen is too small + */ +static int process_subst(char *buf, int maxlen) +{ + char *cmdline; + int size; + int ret; + + /* Move to end of buffer */ + size = strlen(buf) + 1; + cmdline = buf + maxlen - size; + if (buf + size > cmdline) + return -ENOSPC; + memmove(cmdline, buf, size); + + ret = cli_simple_process_macros(cmdline, buf, cmdline - buf); + + return ret; +} + int bootm_process_cmdline(char *buf, int maxlen, int flags) { int ret; @@ -554,6 +582,11 @@ int bootm_process_cmdline(char *buf, int maxlen, int flags) if (ret) return log_msg_ret("silent", ret); } + if (IS_ENABLED(CONFIG_BOOTARGS_SUBST) && (flags & BOOTM_CL_SUBST)) { + ret = process_subst(buf, maxlen); + if (ret) + return log_msg_ret("silent", ret); + } return 0; } @@ -569,7 +602,7 @@ int bootm_process_cmdline_env(int flags) /* First check if any action is needed */ do_silent = IS_ENABLED(CONFIG_SILENT_CONSOLE) && !IS_ENABLED(CONFIG_SILENT_U_BOOT_ONLY) && (flags & BOOTM_CL_SILENT); - if (!do_silent) + if (!do_silent && !IS_ENABLED(CONFIG_BOOTARGS_SUBST)) return 0; env = env_get("bootargs"); @@ -702,8 +735,7 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc, if (!ret && (states & BOOTM_STATE_OS_BD_T)) ret = boot_fn(BOOTM_STATE_OS_BD_T, argc, argv, images); if (!ret && (states & BOOTM_STATE_OS_PREP)) { - ret = bootm_process_cmdline_env(images->os.os == IH_OS_LINUX ? - BOOTM_CL_SILENT : 0); + ret = bootm_process_cmdline_env(images->os.os == IH_OS_LINUX); if (ret) { printf("Cmdline setup failed (err=%d)\n", ret); ret = CMD_RET_FAILURE; diff --git a/include/bootm.h b/include/bootm.h index 8d95fb2a90a..7f88ec718b8 100644 --- a/include/bootm.h +++ b/include/bootm.h @@ -78,8 +78,9 @@ void switch_to_non_secure_mode(void); /* Flags to control bootm_process_cmdline() */ enum bootm_cmdline_t { BOOTM_CL_SILENT = 1 << 0, /* Do silent console processing */ + BOOTM_CL_SUBST = 1 << 1, /* Do substitution */ - BOOTM_CL_ALL = 1, /* All substitutions */ + BOOTM_CL_ALL = 3, /* All substitutions */ }; /** @@ -95,7 +96,10 @@ void board_preboot_os(void); /* * bootm_process_cmdline() - Process fix-ups for the command line * - * This handles: making Linux boot silently if requested ('silent_linux' envvar) + * This handles: + * + * - making Linux boot silently if requested ('silent_linux' envvar) + * - performing substitutions in the command line ('bootargs_subst' envvar) * * @maxlen must provide enough space for the string being processed plus the * resulting string @@ -111,8 +115,10 @@ int bootm_process_cmdline(char *buf, int maxlen, int flags); /** * bootm_process_cmdline_env() - Process fix-ups for the command line * - * Updates the 'bootargs' envvar as required. This handles making Linux boot - * silently if requested ('silent_linux' envvar) + * Updates the 'bootargs' envvar as required. This handles: + * + * - making Linux boot silently if requested ('silent_linux' envvar) + * - performing substitutions in the command line ('bootargs_subst' envvar) * * @flags: Flags to control what happens (see bootm_cmdline_t) * @return 0 if OK, -ENOMEM if out of memory diff --git a/test/bootm.c b/test/bootm.c index d0b29441d65..92dc2b6e173 100644 --- a/test/bootm.c +++ b/test/bootm.c @@ -116,22 +116,126 @@ static int bootm_test_silent(struct unit_test_state *uts) } BOOTM_TEST(bootm_test_silent, 0); +/* Test substitution processing */ +static int bootm_test_subst(struct unit_test_state *uts) +{ + char buf[BUF_SIZE]; + + /* try with an unset variable */ + ut_assertok(env_set("var", NULL)); + strcpy(buf, "some${var}thing"); + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, BOOTM_CL_SUBST)); + ut_asserteq_str("something", buf); + + /* Replace with shorter string */ + ut_assertok(env_set("var", "bb")); + strcpy(buf, "some${var}thing"); + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, BOOTM_CL_SUBST)); + ut_asserteq_str("somebbthing", buf); + + /* Replace with same-length string */ + ut_assertok(env_set("var", "abc")); + strcpy(buf, "some${var}thing"); + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, BOOTM_CL_SUBST)); + ut_asserteq_str("someabcthing", buf); + + /* Replace with longer string */ + ut_assertok(env_set("var", "abcde")); + strcpy(buf, "some${var}thing"); + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, BOOTM_CL_SUBST)); + ut_asserteq_str("someabcdething", buf); + + /* Check it is case sensitive */ + ut_assertok(env_set("VAR", NULL)); + strcpy(buf, "some${VAR}thing"); + ut_assertok(bootm_process_cmdline(buf, BUF_SIZE, BOOTM_CL_SUBST)); + ut_asserteq_str("something", buf); + + /* Check too long - need 12 bytes for each string */ + strcpy(buf, "some${var}thing"); + ut_asserteq(-ENOSPC, + bootm_process_cmdline(buf, 12 * 2 - 1, BOOTM_CL_SUBST)); + + /* Check just enough space */ + strcpy(buf, "some${var}thing"); + ut_assertok(bootm_process_cmdline(buf, 16 * 2, BOOTM_CL_SUBST)); + ut_asserteq_str("someabcdething", buf); + + /* + * Check the substition string being too long. This results in a string + * of 12 (13 bytes). We need enough space for that plus the original + * "a${var}c" string of 9 bytes. So 12 + 9 = 21 bytes. + */ + ut_assertok(env_set("var", "1234567890")); + strcpy(buf, "a${var}c"); + ut_asserteq(-ENOSPC, bootm_process_cmdline(buf, 21, BOOTM_CL_SUBST)); + + strcpy(buf, "a${var}c"); + ut_asserteq(0, bootm_process_cmdline(buf, 22, BOOTM_CL_SUBST)); + + /* Check multiple substitutions */ + ut_assertok(env_set("var", "abc")); + strcpy(buf, "some${var}thing${bvar}else"); + ut_asserteq(0, bootm_process_cmdline(buf, BUF_SIZE, BOOTM_CL_SUBST)); + ut_asserteq_str("someabcthingelse", buf); + + /* Check multiple substitutions */ + ut_assertok(env_set("bvar", "123")); + strcpy(buf, "some${var}thing${bvar}else"); + ut_asserteq(0, bootm_process_cmdline(buf, BUF_SIZE, BOOTM_CL_SUBST)); + ut_asserteq_str("someabcthing123else", buf); + + return 0; +} +BOOTM_TEST(bootm_test_subst, 0); + /* Test silent processing in the bootargs variable */ static int bootm_test_silent_var(struct unit_test_state *uts) { env_set("bootargs", NULL); - env_set("silent_linux", NULL); - ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); + ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SUBST)); ut_assertnull(env_get("bootargs")); - env_set("bootargs", CONSOLE_STR); - env_set("silent_linux", "yes"); + ut_assertok(env_set("bootargs", "some${var}thing")); + ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SUBST)); + ut_asserteq_str("something", env_get("bootargs")); + + return 0; +} +BOOTM_TEST(bootm_test_silent_var, 0); + +/* Test substitution processing in the bootargs variable */ +static int bootm_test_subst_var(struct unit_test_state *uts) +{ + env_set("bootargs", NULL); ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); ut_asserteq_str("console=", env_get("bootargs")); + ut_assertok(env_set("var", "abc")); + ut_assertok(env_set("bootargs", "some${var}thing")); + ut_assertok(bootm_process_cmdline_env(BOOTM_CL_SILENT)); + ut_asserteq_str("some${var}thing console=", env_get("bootargs")); + return 0; } -BOOTM_TEST(bootm_test_silent_var, 0); +BOOTM_TEST(bootm_test_subst_var, 0); + +/* Test substitution and silent console processing in the bootargs variable */ +static int bootm_test_subst_both(struct unit_test_state *uts) +{ + ut_assertok(env_set("silent_linux", "yes")); + env_set("bootargs", NULL); + ut_assertok(bootm_process_cmdline_env(BOOTM_CL_ALL)); + ut_asserteq_str("console=", env_get("bootargs")); + + ut_assertok(env_set("bootargs", "some${var}thing " CONSOLE_STR)); + ut_assertok(env_set("var", "1234567890")); + ut_assertok(bootm_process_cmdline_env(BOOTM_CL_ALL)); + ut_asserteq_str("some1234567890thing console=", env_get("bootargs")); + + return 0; +} +BOOTM_TEST(bootm_test_subst_both, 0); int do_ut_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { -- cgit v1.3.1