Re: [PATCH 07/11] bootm: Split out bootargs environment reading / writing
Hi Wolfgang, On Wed, 21 Oct 2020 at 01:02, Wolfgang Denk wrote: > > Dear Simon, > > In message > you > wrote: > > > > > > It is also useful for zimage to use a buffer, since it does not actually > > > > put the Linux command line in the bootargs variable. > > > > > > ...which I consider a bug that should be fixed. > > > > OK I was wondering about that. > > > > The messy thing about zimage is that the command line comes from the > > setup.bin in the kernel, and then needs to be modified. So you can't > > just blindly use the 'bootargs' var. Perhaps that is why it wasn't > > done? > > I can't say. I never used zImages, and probably never will. > > > I also feel eventually that bootm could subsume zboot given the > > similarities. Or maybe zboot just dies if people stop using the old > > boot approach? > > I would not shed a tear. > > I cannot understand why people still use this. We should more > actively encourage the use of FIT images in favour of all these old > formats. Well x86 does support FIT, but oddly it still has the setup.bin business, because the kernel requires it! At least it uses bootm, though. Something for the future... Regards, Simon
Re: [PATCH 07/11] bootm: Split out bootargs environment reading / writing
Dear Simon, In message you wrote: > > > > It is also useful for zimage to use a buffer, since it does not actually > > > put the Linux command line in the bootargs variable. > > > > ...which I consider a bug that should be fixed. > > OK I was wondering about that. > > The messy thing about zimage is that the command line comes from the > setup.bin in the kernel, and then needs to be modified. So you can't > just blindly use the 'bootargs' var. Perhaps that is why it wasn't > done? I can't say. I never used zImages, and probably never will. > I also feel eventually that bootm could subsume zboot given the > similarities. Or maybe zboot just dies if people stop using the old > boot approach? I would not shed a tear. I cannot understand why people still use this. We should more actively encourage the use of FIT images in favour of all these old formats. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de "If it ain't broke, don't fix it." - Bert Lantz
Re: [PATCH 07/11] bootm: Split out bootargs environment reading / writing
Hi Wolfgang, On Mon, 19 Oct 2020 at 08:45, Wolfgang Denk wrote: > > Dear Simon, > > In message <20201019135602.3943835-8-...@chromium.org> you wrote: > ... > > > > It is also useful for zimage to use a buffer, since it does not actually > > put the Linux command line in the bootargs variable. > > ...which I consider a bug that should be fixed. OK I was wondering about that. The messy thing about zimage is that the command line comes from the setup.bin in the kernel, and then needs to be modified. So you can't just blindly use the 'bootargs' var. Perhaps that is why it wasn't done? I also feel eventually that bootm could subsume zboot given the similarities. Or maybe zboot just dies if people stop using the old boot approach? Regards, Simon
Re: [PATCH 07/11] bootm: Split out bootargs environment reading / writing
Dear Simon, In message <20201019135602.3943835-8-...@chromium.org> you wrote: ... > > It is also useful for zimage to use a buffer, since it does not actually > put the Linux command line in the bootargs variable. ...which I consider a bug that should be fixed. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de A good marriage would be between a blind wife and deaf husband. -- Michel de Montaigne
[PATCH 07/11] bootm: Split out bootargs environment reading / writing
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
---
common/bootm.c | 95 ++
1 file changed, 73 insertions(+), 22 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c
index bbe59752609..41965fa304c 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#if defined(CONFIG_CMD_USB)
#include
#endif
@@ -35,6 +36,8 @@
#define CONFIG_SYS_BOOTM_LEN 0x80
#endif
+#define MAX_CMDLINE_SIZE SZ_4K
+
#define IH_INITRD_ARCH IH_ARCH_DEFAULT
#ifndef USE_HOSTCC
@@ -466,20 +469,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:
*
@@ -494,15 +508,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, ' ');
@@ -517,15 +528,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;
+
