* [PATCH v2 01/12] env: Allow returning errors from hdelete_r()
2020-11-05 17:33 [PATCH v2 00/12] bootm: Support substitions in bootargs and add tests Simon Glass
@ 2020-11-05 17:33 ` Simon Glass
2020-12-07 22:18 ` Tom Rini
2020-11-05 17:33 ` [PATCH v2 02/12] bootm: Add tests for fixup_silent_linux() Simon Glass
` (10 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2020-11-05 17:33 UTC (permalink / raw)
To: u-boot
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 <sjg@chromium.org>
wip
---
(no changes since v1)
cmd/nvedit.c | 6 ++++--
include/search.h | 11 ++++++++++-
lib/hashtable.c | 12 ++++++------
test/env/hashtable.c | 2 +-
4 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 7fce723800d..d0d2eca9047 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -266,7 +266,9 @@ static int _do_env_set(int flag, int argc, char *const argv[], int env_flag)
/* Delete only ? */
if (argc < 3 || argv[2] == NULL) {
int rc = hdelete_r(name, &env_htab, env_flag);
- return !rc;
+
+ /* If the variable didn't exist, don't report an error */
+ return rc && rc != -ENOENT ? 1 : 0;
}
/*
@@ -895,7 +897,7 @@ static int do_env_delete(struct cmd_tbl *cmdtp, int flag,
while (--argc > 0) {
char *name = *++argv;
- if (!hdelete_r(name, &env_htab, env_flag))
+ if (hdelete_r(name, &env_htab, env_flag))
ret = 1;
}
diff --git a/include/search.h b/include/search.h
index e56843c26fd..d0bb44388e1 100644
--- a/include/search.h
+++ b/include/search.h
@@ -80,7 +80,16 @@ int hsearch_r(struct env_entry item, enum env_action action,
int hmatch_r(const char *match, int last_idx, struct env_entry **retval,
struct hsearch_data *htab);
-/* Search and delete entry matching "key" in internal hash table. */
+/**
+ * hdelete_r() - Search and delete entry in internal hash table
+ *
+ * @key: Name of entry to delete
+ * @htab: Hash table
+ * @flag: Flags to use (H_...)
+ * @return 0 on success, -ENOENT if not found, -EPERM if the hash table callback
+ * rejected changing the variable, -EINVAL if the hash table refused to
+ * delete the variable
+ */
int hdelete_r(const char *key, struct hsearch_data *htab, int flag);
ssize_t hexport_r(struct hsearch_data *htab, const char sep, int flag,
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 7c08f5c8055..ff5ff726394 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -472,7 +472,7 @@ int hdelete_r(const char *key, struct hsearch_data *htab, int flag)
idx = hsearch_r(e, ENV_FIND, &ep, htab, 0);
if (idx == 0) {
__set_errno(ESRCH);
- return 0; /* not found */
+ return -ENOENT; /* not found */
}
/* Check for permission */
@@ -481,7 +481,7 @@ int hdelete_r(const char *key, struct hsearch_data *htab, int flag)
debug("change_ok() rejected deleting variable "
"%s, skipping it!\n", key);
__set_errno(EPERM);
- return 0;
+ return -EPERM;
}
/* If there is a callback, call it */
@@ -490,12 +490,12 @@ int hdelete_r(const char *key, struct hsearch_data *htab, int flag)
debug("callback() rejected deleting variable "
"%s, skipping it!\n", key);
__set_errno(EINVAL);
- return 0;
+ return -EINVAL;
}
_hdelete(key, htab, ep, idx);
- return 1;
+ return 0;
}
#if !(defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_SAVEENV))
@@ -917,7 +917,7 @@ int himport_r(struct hsearch_data *htab,
if (!drop_var_from_set(name, nvars, localvars))
continue;
- if (hdelete_r(name, htab, flag) == 0)
+ if (hdelete_r(name, htab, flag))
debug("DELETE ERROR ##############################\n");
continue;
@@ -979,7 +979,7 @@ int himport_r(struct hsearch_data *htab,
* b) if the variable was not present in current env, we notify
* it might be a typo
*/
- if (hdelete_r(localvars[i], htab, flag) == 0)
+ if (hdelete_r(localvars[i], htab, flag))
printf("WARNING: '%s' neither in running nor in imported env!\n", localvars[i]);
else
printf("WARNING: '%s' not in imported env, deleting it!\n", localvars[i]);
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;
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 01/12] env: Allow returning errors from hdelete_r()
2020-11-05 17:33 ` [PATCH v2 01/12] env: Allow returning errors from hdelete_r() Simon Glass
@ 2020-12-07 22:18 ` Tom Rini
0 siblings, 0 replies; 25+ messages in thread
From: Tom Rini @ 2020-12-07 22:18 UTC (permalink / raw)
To: u-boot
On Thu, Nov 05, 2020 at 10:33:37AM -0700, Simon Glass wrote:
> 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 <sjg@chromium.org>
>
> wip
Applied to u-boot/next, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201207/6352312d/attachment.sig>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 02/12] bootm: Add tests for fixup_silent_linux()
2020-11-05 17:33 [PATCH v2 00/12] bootm: Support substitions in bootargs and add tests Simon Glass
2020-11-05 17:33 ` [PATCH v2 01/12] env: Allow returning errors from hdelete_r() Simon Glass
@ 2020-11-05 17:33 ` Simon Glass
2020-12-07 22:18 ` Tom Rini
2020-11-05 17:33 ` [PATCH v2 03/12] bootm: Update fixup_silent_linux() to return an error Simon Glass
` (9 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2020-11-05 17:33 UTC (permalink / raw)
To: u-boot
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 <sjg@chromium.org>
---
(no changes since v1)
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
diff --git a/arch/Kconfig b/arch/Kconfig
index 683e3843190..6caf2338bcf 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -140,6 +140,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 ab7b3bd9cad..c1d8fa9a650 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 1c930b31485..244ac8be139 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -3,6 +3,7 @@
# (C) Copyright 2012 The Chromium Authors
obj-$(CONFIG_$(SPL_)CMDLINE) += bloblist.o
+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 <common.h>
+#include <bootm.h>
+#include <test/suites.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+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 8f0bc688a22..8d5c28b4319 100644
--- a/test/cmd_ut.c
+++ b/test/cmd_ut.c
@@ -86,6 +86,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
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 03/12] bootm: Update fixup_silent_linux() to return an error
2020-11-05 17:33 [PATCH v2 00/12] bootm: Support substitions in bootargs and add tests Simon Glass
2020-11-05 17:33 ` [PATCH v2 01/12] env: Allow returning errors from hdelete_r() Simon Glass
2020-11-05 17:33 ` [PATCH v2 02/12] bootm: Add tests for fixup_silent_linux() Simon Glass
@ 2020-11-05 17:33 ` Simon Glass
2020-12-07 22:18 ` Tom Rini
2020-11-05 17:33 ` [PATCH v2 04/12] bootm: Rename fixup_silent_linux() Simon Glass
` (8 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2020-11-05 17:33 UTC (permalink / raw)
To: u-boot
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 <sjg@chromium.org>
---
(no changes since v1)
common/bootm.c | 22 +++++++++++++++-------
include/bootm.h | 11 +++++++++--
test/bootm.c | 10 +++++-----
3 files changed, 29 insertions(+), 14 deletions(-)
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;
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 04/12] bootm: Rename fixup_silent_linux()
2020-11-05 17:33 [PATCH v2 00/12] bootm: Support substitions in bootargs and add tests Simon Glass
` (2 preceding siblings ...)
2020-11-05 17:33 ` [PATCH v2 03/12] bootm: Update fixup_silent_linux() to return an error Simon Glass
@ 2020-11-05 17:33 ` Simon Glass
2020-12-07 22:18 ` Tom Rini
2020-11-05 17:33 ` [PATCH v2 05/12] bootm: Add a bool parameter to bootm_process_cmdline_env() Simon Glass
` (7 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2020-11-05 17:33 UTC (permalink / raw)
To: u-boot
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 <sjg@chromium.org>
---
(no changes since v1)
common/bootm.c | 4 ++--
include/bootm.h | 4 ++--
test/bootm.c | 10 +++++-----
3 files changed, 9 insertions(+), 9 deletions(-)
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;
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 05/12] bootm: Add a bool parameter to bootm_process_cmdline_env()
2020-11-05 17:33 [PATCH v2 00/12] bootm: Support substitions in bootargs and add tests Simon Glass
` (3 preceding siblings ...)
2020-11-05 17:33 ` [PATCH v2 04/12] bootm: Rename fixup_silent_linux() Simon Glass
@ 2020-11-05 17:33 ` Simon Glass
2020-12-07 22:19 ` Tom Rini
2020-11-05 17:33 ` [PATCH v2 06/12] bootm: Use size rather than length for CONSOLE_ARG Simon Glass
` (6 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2020-11-05 17:33 UTC (permalink / raw)
To: u-boot
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 <sjg@chromium.org>
---
(no changes since v1)
common/bootm.c | 20 ++++++++++----------
include/bootm.h | 3 ++-
test/bootm.c | 10 +++++-----
3 files changed, 17 insertions(+), 16 deletions(-)
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;
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 06/12] bootm: Use size rather than length for CONSOLE_ARG
2020-11-05 17:33 [PATCH v2 00/12] bootm: Support substitions in bootargs and add tests Simon Glass
` (4 preceding siblings ...)
2020-11-05 17:33 ` [PATCH v2 05/12] bootm: Add a bool parameter to bootm_process_cmdline_env() Simon Glass
@ 2020-11-05 17:33 ` Simon Glass
2020-12-07 22:19 ` Tom Rini
2020-11-05 17:33 ` [PATCH v2 07/12] bootm: Split out bootargs environment reading / writing Simon Glass
` (5 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2020-11-05 17:33 UTC (permalink / raw)
To: u-boot
Use the size (including terminator) for in this function, rather than
the length. This is arguably easier to follow, with the coming
refactor.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
common/bootm.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c
index 9b0c81d6534..9295cf5cd3e 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -465,8 +465,8 @@ ulong bootm_disable_interrupts(void)
return iflag;
}
-#define CONSOLE_ARG "console="
-#define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1)
+#define CONSOLE_ARG "console="
+#define CONSOLE_ARG_SIZE sizeof(CONSOLE_ARG)
int bootm_process_cmdline_env(bool do_silent)
{
@@ -500,7 +500,7 @@ int bootm_process_cmdline_env(bool do_silent)
char *start = strstr(cmdline, CONSOLE_ARG);
/* Allocate space for maximum possible new command line */
- buf = malloc(strlen(cmdline) + 1 + CONSOLE_ARG_LEN + 1);
+ buf = malloc(strlen(cmdline) + 1 + CONSOLE_ARG_SIZE);
if (!buf) {
debug("%s: out of memory\n", __func__);
return -ENOSPC;
@@ -508,13 +508,14 @@ int bootm_process_cmdline_env(bool do_silent)
if (start) {
char *end = strchr(start, ' ');
- int num_start_bytes = start - cmdline + CONSOLE_ARG_LEN;
+ int start_bytes;
- strncpy(buf, cmdline, num_start_bytes);
+ start_bytes = start - cmdline + CONSOLE_ARG_SIZE - 1;
+ strncpy(buf, cmdline, start_bytes);
if (end)
- strcpy(buf + num_start_bytes, end);
+ strcpy(buf + start_bytes, end);
else
- buf[num_start_bytes] = '\0';
+ buf[start_bytes] = '\0';
} else {
sprintf(buf, "%s %s", cmdline, CONSOLE_ARG);
}
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 07/12] bootm: Split out bootargs environment reading / writing
2020-11-05 17:33 [PATCH v2 00/12] bootm: Support substitions in bootargs and add tests Simon Glass
` (5 preceding siblings ...)
2020-11-05 17:33 ` [PATCH v2 06/12] bootm: Use size rather than length for CONSOLE_ARG Simon Glass
@ 2020-11-05 17:33 ` Simon Glass
2020-12-07 22:19 ` Tom Rini
2020-11-05 17:33 ` [PATCH v2 08/12] bootm: Update bootm_process_cmdline_env() to use flags Simon Glass
` (4 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2020-11-05 17:33 UTC (permalink / raw)
To: u-boot
At present bootm_process_cmdline_env() reads the 'bootargs' variable and
then writes it back afterwards. This is painful for tests, which would
rather use a simple buffer.
It is also useful for zimage to use a buffer, since it does not actually
put the Linux command line in the bootargs variable.
Refactor the existing code into two pieces. One handles reading and
writing the environment variable, as well as allocating a buffer for use
by the rest of the code, which now operates on a buffer.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
common/bootm.c | 95 ++++++++++++++++++++++++++++++++++++++------------
1 file changed, 73 insertions(+), 22 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c
index 9295cf5cd3e..4fa909f23bc 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -19,6 +19,7 @@
#include <net.h>
#include <asm/cache.h>
#include <asm/io.h>
+#include <linux/sizes.h>
#if defined(CONFIG_CMD_USB)
#include <usb.h>
#endif
@@ -35,6 +36,8 @@
#define CONFIG_SYS_BOOTM_LEN 0x800000
#endif
+#define MAX_CMDLINE_SIZE SZ_4K
+
#define IH_INITRD_ARCH IH_ARCH_DEFAULT
#ifndef USE_HOSTCC
@@ -468,20 +471,31 @@ ulong bootm_disable_interrupts(void)
#define CONSOLE_ARG "console="
#define CONSOLE_ARG_SIZE sizeof(CONSOLE_ARG)
-int bootm_process_cmdline_env(bool do_silent)
+/**
+ * fixup_silent_linux() - Handle silencing the linux boot if required
+ *
+ * This uses the silent_linux envvar to control whether to add/set a "console="
+ * parameter to the command line
+ *
+ * @buf: Buffer containing the string to process
+ * @maxlen: Maximum length of buffer
+ * @return 0 if OK, -ENOSPC if @maxlen is too small
+ */
+static int fixup_silent_linux(char *buf, int maxlen)
{
- char *buf;
- const char *env_val;
- char *cmdline;
int want_silent;
+ char *cmdline;
+ int size;
- /* 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");
-
+ /*
+ * Move the input string to the end of buffer. The output string will be
+ * built up at the start.
+ */
+ size = strlen(buf) + 1;
+ if (size * 2 > maxlen)
+ return -ENOSPC;
+ cmdline = buf + maxlen - size;
+ memmove(cmdline, buf, size);
/*
* Only fix cmdline when requested. The environment variable can be:
*
@@ -496,15 +510,12 @@ int bootm_process_cmdline_env(bool do_silent)
return 0;
debug("before silent fix-up: %s\n", cmdline);
- if (cmdline && (cmdline[0] != '\0')) {
+ if (*cmdline) {
char *start = strstr(cmdline, CONSOLE_ARG);
- /* Allocate space for maximum possible new command line */
- buf = malloc(strlen(cmdline) + 1 + CONSOLE_ARG_SIZE);
- if (!buf) {
- debug("%s: out of memory\n", __func__);
+ /* Check space for maximum possible new command line */
+ if (size + CONSOLE_ARG_SIZE > maxlen)
return -ENOSPC;
- }
if (start) {
char *end = strchr(start, ' ');
@@ -519,15 +530,55 @@ int bootm_process_cmdline_env(bool do_silent)
} else {
sprintf(buf, "%s %s", cmdline, CONSOLE_ARG);
}
- env_val = buf;
+ if (buf + strlen(buf) >= cmdline)
+ return -ENOSPC;
} else {
- buf = NULL;
- env_val = CONSOLE_ARG;
+ if (maxlen < sizeof(CONSOLE_ARG))
+ return -ENOSPC;
+ strcpy(buf, CONSOLE_ARG);
}
+ debug("after silent fix-up: %s\n", buf);
- env_set("bootargs", env_val);
- debug("after silent fix-up: %s\n", env_val);
+ return 0;
+}
+
+int bootm_process_cmdline_env(bool do_silent)
+{
+ const int maxlen = MAX_CMDLINE_SIZE;
+ 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;
+ if (!do_silent)
+ return 0;
+
+ env = env_get("bootargs");
+ if (env && strlen(env) >= maxlen)
+ return -E2BIG;
+ buf = malloc(maxlen);
+ if (!buf)
+ return -ENOMEM;
+ if (env)
+ strcpy(buf, env);
+ else
+ *buf = '\0';
+ ret = fixup_silent_linux(buf, maxlen);
+ if (!ret) {
+ ret = env_set("bootargs", buf);
+
+ /*
+ * If buf is "" and bootargs does not exist, this will produce
+ * an error trying to delete bootargs. Ignore it
+ */
+ if (ret == -ENOENT)
+ ret = 0;
+ }
free(buf);
+ if (ret)
+ return log_msg_ret("env", ret);
return 0;
}
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 08/12] bootm: Update bootm_process_cmdline_env() to use flags
2020-11-05 17:33 [PATCH v2 00/12] bootm: Support substitions in bootargs and add tests Simon Glass
` (6 preceding siblings ...)
2020-11-05 17:33 ` [PATCH v2 07/12] bootm: Split out bootargs environment reading / writing Simon Glass
@ 2020-11-05 17:33 ` Simon Glass
2020-12-07 22:19 ` Tom Rini
2020-11-05 17:33 ` [PATCH v2 09/12] bootm: Allow updating the bootargs in a buffer Simon Glass
` (3 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2020-11-05 17:33 UTC (permalink / raw)
To: u-boot
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 <sjg@chromium.org>
---
(no changes since v1)
common/bootm.c | 8 +++++---
include/bootm.h | 11 +++++++++--
test/bootm.c | 10 +++++-----
3 files changed, 19 insertions(+), 10 deletions(-)
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;
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 09/12] bootm: Allow updating the bootargs in a buffer
2020-11-05 17:33 [PATCH v2 00/12] bootm: Support substitions in bootargs and add tests Simon Glass
` (7 preceding siblings ...)
2020-11-05 17:33 ` [PATCH v2 08/12] bootm: Update bootm_process_cmdline_env() to use flags Simon Glass
@ 2020-11-05 17:33 ` Simon Glass
2020-12-07 22:19 ` Tom Rini
2020-11-05 17:33 ` [PATCH v2 10/12] x86: zimage: Add silent-console processing Simon Glass
` (2 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2020-11-05 17:33 UTC (permalink / raw)
To: u-boot
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 <sjg@chromium.org>
---
(no changes since v1)
common/bootm.c | 18 +++++++-
include/bootm.h | 16 +++++++
test/bootm.c | 114 +++++++++++++++++++++++++++++++++++++++++-------
3 files changed, 132 insertions(+), 16 deletions(-)
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"));
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 10/12] x86: zimage: Add silent-console processing
2020-11-05 17:33 [PATCH v2 00/12] bootm: Support substitions in bootargs and add tests Simon Glass
` (8 preceding siblings ...)
2020-11-05 17:33 ` [PATCH v2 09/12] bootm: Allow updating the bootargs in a buffer Simon Glass
@ 2020-11-05 17:33 ` Simon Glass
2020-12-07 22:19 ` Tom Rini
2020-11-05 17:33 ` [PATCH v2 11/12] cli: Support macro processing with a fixed-size buffer Simon Glass
2020-11-05 17:33 ` [PATCH v2 12/12] bootm: Support string substitution in bootargs Simon Glass
11 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2020-11-05 17:33 UTC (permalink / raw)
To: u-boot
At present zimage does its own command-line processing and does not
support the 'silent console' feature. There doesn't seem to be any good
reason for this.
Add support for silent console to zimage.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
arch/x86/lib/zimage.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c
index a00964cc8d9..8e1e8017bd8 100644
--- a/arch/x86/lib/zimage.c
+++ b/arch/x86/lib/zimage.c
@@ -13,6 +13,7 @@
*/
#include <common.h>
+#include <bootm.h>
#include <command.h>
#include <env.h>
#include <irq_func.h>
@@ -317,6 +318,11 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot,
}
if (cmd_line) {
+ int max_size = 0xff;
+ int ret;
+
+ if (bootproto >= 0x0206)
+ max_size = hdr->cmdline_size;
if (bootproto >= 0x0202) {
hdr->cmd_line_ptr = (uintptr_t)cmd_line;
} else if (bootproto >= 0x0200) {
@@ -332,6 +338,14 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot,
strcpy(cmd_line, (char *)cmdline_force);
else
build_command_line(cmd_line, auto_boot);
+ ret = bootm_process_cmdline(cmd_line, max_size, BOOTM_CL_ALL);
+ if (ret) {
+ printf("Cmdline setup failed (err=%d)\n", ret);
+ return ret;
+ }
+ printf("Kernel command line: \"");
+ puts(cmd_line);
+ printf("\"\n");
}
if (IS_ENABLED(CONFIG_INTEL_MID) && bootproto >= 0x0207)
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 11/12] cli: Support macro processing with a fixed-size buffer
2020-11-05 17:33 [PATCH v2 00/12] bootm: Support substitions in bootargs and add tests Simon Glass
` (9 preceding siblings ...)
2020-11-05 17:33 ` [PATCH v2 10/12] x86: zimage: Add silent-console processing Simon Glass
@ 2020-11-05 17:33 ` Simon Glass
2020-12-07 22:19 ` Tom Rini
2020-11-05 17:33 ` [PATCH v2 12/12] bootm: Support string substitution in bootargs Simon Glass
11 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2020-11-05 17:33 UTC (permalink / raw)
To: u-boot
At present cli_simple_process_macros() requires that the caller provide
an output buffer that is exactly CONFIG_SYS_CBSIZE bytes in length. This
makes sense since it is designed to be used from the command line. But we
also want to use it for bootargs substitution.
Update the function to allow the caller to specify the buffer size. Also
return an error if the buffer is exhausted. The caller can ignore that if
preferred.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Add a new patch to support macro processing with a fixed-size buffer
cmd/pxe_utils.c | 6 ++++--
common/cli_simple.c | 17 ++++++++++++-----
include/cli.h | 4 +++-
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c
index 8716e782f6a..69cac149c0c 100644
--- a/cmd/pxe_utils.c
+++ b/cmd/pxe_utils.c
@@ -322,7 +322,8 @@ static int label_localboot(struct pxe_label *label)
if (label->append) {
char bootargs[CONFIG_SYS_CBSIZE];
- cli_simple_process_macros(label->append, bootargs);
+ cli_simple_process_macros(label->append, bootargs,
+ sizeof(bootargs));
env_set("bootargs", bootargs);
}
@@ -430,7 +431,8 @@ static int label_boot(struct cmd_tbl *cmdtp, struct pxe_label *label)
strcat(bootargs, ip_str);
strcat(bootargs, mac_str);
- cli_simple_process_macros(bootargs, finalbootargs);
+ cli_simple_process_macros(bootargs, finalbootargs,
+ sizeof(finalbootargs));
env_set("bootargs", finalbootargs);
printf("append: %s\n", finalbootargs);
}
diff --git a/common/cli_simple.c b/common/cli_simple.c
index 7d91316a0fb..e80ba488a5e 100644
--- a/common/cli_simple.c
+++ b/common/cli_simple.c
@@ -60,13 +60,14 @@ int cli_simple_parse_line(char *line, char *argv[])
return nargs;
}
-void cli_simple_process_macros(const char *input, char *output)
+int cli_simple_process_macros(const char *input, char *output, int max_size)
{
char c, prev;
const char *varname_start = NULL;
int inputcnt = strlen(input);
- int outputcnt = CONFIG_SYS_CBSIZE;
+ int outputcnt = max_size;
int state = 0; /* 0 = waiting for '$' */
+ int ret;
/* 1 = waiting for '(' or '{' */
/* 2 = waiting for ')' or '}' */
@@ -157,13 +158,18 @@ void cli_simple_process_macros(const char *input, char *output)
prev = c;
}
- if (outputcnt)
+ ret = inputcnt ? -ENOSPC : 0;
+ if (outputcnt) {
*output = 0;
- else
+ } else {
*(output - 1) = 0;
+ ret = -ENOSPC;
+ }
debug_parser("[PROCESS_MACROS] OUTPUT len %zd: \"%s\"\n",
strlen(output_start), output_start);
+
+ return ret;
}
/*
@@ -239,7 +245,8 @@ int cli_simple_run_command(const char *cmd, int flag)
debug_parser("token: \"%s\"\n", token);
/* find macros in this token and replace them */
- cli_simple_process_macros(token, finaltoken);
+ cli_simple_process_macros(token, finaltoken,
+ sizeof(finaltoken));
/* Extract arguments */
argc = cli_simple_parse_line(finaltoken, argv);
diff --git a/include/cli.h b/include/cli.h
index 39b913743b5..3449fa6ae72 100644
--- a/include/cli.h
+++ b/include/cli.h
@@ -34,8 +34,10 @@ int cli_simple_run_command(const char *cmd, int flag);
*
* @param input Input string possible containing $() / ${} vars
* @param output Output string with $() / ${} vars expanded
+ * @param max_size Maximum size of @output (including terminator)
+ * @return 0 if OK, -ENOSPC if we ran out of space in @output
*/
-void cli_simple_process_macros(const char *input, char *output);
+int cli_simple_process_macros(const char *input, char *output, int max_size);
/**
* cli_simple_run_command_list() - Execute a list of command
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 12/12] bootm: Support string substitution in bootargs
2020-11-05 17:33 [PATCH v2 00/12] bootm: Support substitions in bootargs and add tests Simon Glass
` (10 preceding siblings ...)
2020-11-05 17:33 ` [PATCH v2 11/12] cli: Support macro processing with a fixed-size buffer Simon Glass
@ 2020-11-05 17:33 ` Simon Glass
2020-12-07 22:19 ` Tom Rini
11 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2020-11-05 17:33 UTC (permalink / raw)
To: u-boot
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 <sjg@chromium.org>
---
Changes in v2:
- Use ${} for subtitution instead of %
- Tweak cover letter
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(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 6caf2338bcf..421ea9a9b51 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -141,6 +141,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 522f5f3d6a4..27e5d45fd70 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -848,6 +848,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..6c286942a41 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -7,6 +7,7 @@
#ifndef USE_HOSTCC
#include <common.h>
#include <bootstage.h>
+#include <cli.h>
#include <cpu_func.h>
#include <env.h>
#include <errno.h>
@@ -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[])
{
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 12/12] bootm: Support string substitution in bootargs
2020-11-05 17:33 ` [PATCH v2 12/12] bootm: Support string substitution in bootargs Simon Glass
@ 2020-12-07 22:19 ` Tom Rini
0 siblings, 0 replies; 25+ messages in thread
From: Tom Rini @ 2020-12-07 22:19 UTC (permalink / raw)
To: u-boot
On Thu, Nov 05, 2020 at 10:33:48AM -0700, Simon Glass wrote:
> 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 <sjg@chromium.org>
Applied to u-boot/next, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201207/53e90eb1/attachment.sig>
^ permalink raw reply [flat|nested] 25+ messages in thread