From: Christophe Leroy <christophe.leroy@csgroup.eu> To: Will Deacon <will@kernel.org> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>, Paul Mackerras <paulus@samba.org>, Michael Ellerman <mpe@ellerman.id.au>, danielwa@cisco.com, robh@kernel.org, daniel@gimpelevich.san-francisco.ca.us, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-arch@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH v2 1/7] cmdline: Add generic function to build command line. Date: Wed, 3 Mar 2021 18:38:16 +0100 [thread overview] Message-ID: <a0cfef11-efba-2e5c-6f58-ed63a2c3bfa0@csgroup.eu> (raw) In-Reply-To: <20210303172810.GA19713@willie-the-truck> Le 03/03/2021 à 18:28, Will Deacon a écrit : > On Tue, Mar 02, 2021 at 05:25:17PM +0000, Christophe Leroy wrote: >> This code provides architectures with a way to build command line >> based on what is built in the kernel and what is handed over by the >> bootloader, based on selected compile-time options. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> include/linux/cmdline.h | 62 +++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 62 insertions(+) >> create mode 100644 include/linux/cmdline.h >> >> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h >> new file mode 100644 >> index 000000000000..ae3610bb0ee2 >> --- /dev/null >> +++ b/include/linux/cmdline.h >> @@ -0,0 +1,62 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +#ifndef _LINUX_CMDLINE_H >> +#define _LINUX_CMDLINE_H >> + >> +static __always_inline size_t cmdline_strlen(const char *s) >> +{ >> + const char *sc; >> + >> + for (sc = s; *sc != '\0'; ++sc) >> + ; /* nothing */ >> + return sc - s; >> +} >> + >> +static __always_inline size_t cmdline_strlcat(char *dest, const char *src, size_t count) >> +{ >> + size_t dsize = cmdline_strlen(dest); >> + size_t len = cmdline_strlen(src); >> + size_t res = dsize + len; >> + >> + /* This would be a bug */ >> + if (dsize >= count) >> + return count; >> + >> + dest += dsize; >> + count -= dsize; >> + if (len >= count) >> + len = count - 1; >> + memcpy(dest, src, len); >> + dest[len] = 0; >> + return res; >> +} > > Why are these needed instead of using strlen and strlcat directly? Because on powerpc (at least), it will be used in prom_init, it is very early in the boot and KASAN shadow memory is not set up yet so calling generic string functions would crash the board. > >> +/* >> + * This function will append a builtin command line to the command >> + * line provided by the bootloader. Kconfig options can be used to alter >> + * the behavior of this builtin command line. >> + * @dest: The destination of the final appended/prepended string. >> + * @src: The starting string or NULL if there isn't one. Must not equal dest. >> + * @length: the length of dest buffer. >> + */ >> +static __always_inline void cmdline_build(char *dest, const char *src, size_t length) >> +{ >> + if (length <= 0) >> + return; >> + >> + dest[0] = 0; >> + >> +#ifdef CONFIG_CMDLINE >> + if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !src || !src[0]) { >> + cmdline_strlcat(dest, CONFIG_CMDLINE, length); >> + return; >> + } >> +#endif > > CONFIG_CMDLINE_FORCE implies CONFIG_CMDLINE, and even if it didn't, > CONFIG_CMDLINE is at worst an empty string. Can you drop the #ifdef? Ah yes, since cbe46bd4f510 ("powerpc: remove CONFIG_CMDLINE #ifdef mess") it is feasible. I can change that now. > >> + if (dest != src) >> + cmdline_strlcat(dest, src, length); >> +#ifdef CONFIG_CMDLINE >> + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) && sizeof(CONFIG_CMDLINE) > 1) >> + cmdline_strlcat(dest, " " CONFIG_CMDLINE, length); >> +#endif > > Likewise, but also I'm not sure why the sizeof() is required. It is to avoid adding a white space at the end of the command line when CONFIG_CMDLINE is empty. But maybe it doesn't matter ? Christophe
WARNING: multiple messages have this Message-ID (diff)
From: Christophe Leroy <christophe.leroy@csgroup.eu> To: Will Deacon <will@kernel.org> Cc: linux-arch@vger.kernel.org, robh@kernel.org, daniel@gimpelevich.san-francisco.ca.us, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>, linuxppc-dev@lists.ozlabs.org, danielwa@cisco.com Subject: Re: [PATCH v2 1/7] cmdline: Add generic function to build command line. Date: Wed, 3 Mar 2021 18:38:16 +0100 [thread overview] Message-ID: <a0cfef11-efba-2e5c-6f58-ed63a2c3bfa0@csgroup.eu> (raw) In-Reply-To: <20210303172810.GA19713@willie-the-truck> Le 03/03/2021 à 18:28, Will Deacon a écrit : > On Tue, Mar 02, 2021 at 05:25:17PM +0000, Christophe Leroy wrote: >> This code provides architectures with a way to build command line >> based on what is built in the kernel and what is handed over by the >> bootloader, based on selected compile-time options. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> include/linux/cmdline.h | 62 +++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 62 insertions(+) >> create mode 100644 include/linux/cmdline.h >> >> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h >> new file mode 100644 >> index 000000000000..ae3610bb0ee2 >> --- /dev/null >> +++ b/include/linux/cmdline.h >> @@ -0,0 +1,62 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +#ifndef _LINUX_CMDLINE_H >> +#define _LINUX_CMDLINE_H >> + >> +static __always_inline size_t cmdline_strlen(const char *s) >> +{ >> + const char *sc; >> + >> + for (sc = s; *sc != '\0'; ++sc) >> + ; /* nothing */ >> + return sc - s; >> +} >> + >> +static __always_inline size_t cmdline_strlcat(char *dest, const char *src, size_t count) >> +{ >> + size_t dsize = cmdline_strlen(dest); >> + size_t len = cmdline_strlen(src); >> + size_t res = dsize + len; >> + >> + /* This would be a bug */ >> + if (dsize >= count) >> + return count; >> + >> + dest += dsize; >> + count -= dsize; >> + if (len >= count) >> + len = count - 1; >> + memcpy(dest, src, len); >> + dest[len] = 0; >> + return res; >> +} > > Why are these needed instead of using strlen and strlcat directly? Because on powerpc (at least), it will be used in prom_init, it is very early in the boot and KASAN shadow memory is not set up yet so calling generic string functions would crash the board. > >> +/* >> + * This function will append a builtin command line to the command >> + * line provided by the bootloader. Kconfig options can be used to alter >> + * the behavior of this builtin command line. >> + * @dest: The destination of the final appended/prepended string. >> + * @src: The starting string or NULL if there isn't one. Must not equal dest. >> + * @length: the length of dest buffer. >> + */ >> +static __always_inline void cmdline_build(char *dest, const char *src, size_t length) >> +{ >> + if (length <= 0) >> + return; >> + >> + dest[0] = 0; >> + >> +#ifdef CONFIG_CMDLINE >> + if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !src || !src[0]) { >> + cmdline_strlcat(dest, CONFIG_CMDLINE, length); >> + return; >> + } >> +#endif > > CONFIG_CMDLINE_FORCE implies CONFIG_CMDLINE, and even if it didn't, > CONFIG_CMDLINE is at worst an empty string. Can you drop the #ifdef? Ah yes, since cbe46bd4f510 ("powerpc: remove CONFIG_CMDLINE #ifdef mess") it is feasible. I can change that now. > >> + if (dest != src) >> + cmdline_strlcat(dest, src, length); >> +#ifdef CONFIG_CMDLINE >> + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) && sizeof(CONFIG_CMDLINE) > 1) >> + cmdline_strlcat(dest, " " CONFIG_CMDLINE, length); >> +#endif > > Likewise, but also I'm not sure why the sizeof() is required. It is to avoid adding a white space at the end of the command line when CONFIG_CMDLINE is empty. But maybe it doesn't matter ? Christophe
next prev parent reply other threads:[~2021-03-03 20:58 UTC|newest] Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-02 17:25 [PATCH v2 0/7] Improve boot command line handling Christophe Leroy 2021-03-02 17:25 ` Christophe Leroy 2021-03-02 17:25 ` [PATCH v2 1/7] cmdline: Add generic function to build command line Christophe Leroy 2021-03-02 17:25 ` Christophe Leroy 2021-03-03 17:28 ` Will Deacon 2021-03-03 17:28 ` Will Deacon 2021-03-03 17:38 ` Christophe Leroy [this message] 2021-03-03 17:38 ` Christophe Leroy 2021-03-03 17:46 ` Will Deacon 2021-03-03 17:46 ` Will Deacon 2021-03-03 17:57 ` Christophe Leroy 2021-03-03 17:57 ` Christophe Leroy 2021-03-03 18:16 ` Will Deacon 2021-03-03 18:16 ` Will Deacon 2021-03-05 11:58 ` Michael Ellerman 2021-03-05 11:58 ` Michael Ellerman 2021-03-05 12:49 ` Christophe Leroy 2021-03-05 12:49 ` Christophe Leroy 2021-03-05 18:35 ` Segher Boessenkool 2021-03-05 18:35 ` Segher Boessenkool 2021-03-05 18:33 ` Segher Boessenkool 2021-03-05 18:33 ` Segher Boessenkool 2021-03-03 17:39 ` Will Deacon 2021-03-03 17:39 ` Will Deacon 2021-03-02 17:25 ` [PATCH v2 2/7] drivers: of: use cmdline building function Christophe Leroy 2021-03-02 17:25 ` Christophe Leroy 2021-03-02 17:25 ` [PATCH v2 3/7] powerpc: convert to generic builtin command line Christophe Leroy 2021-03-02 17:25 ` Christophe Leroy 2021-03-02 17:25 ` [PATCH v2 4/7] cmdline: Add capability to prepend the " Christophe Leroy 2021-03-02 17:25 ` Christophe Leroy 2021-03-03 18:19 ` Will Deacon 2021-03-03 18:19 ` Will Deacon 2021-03-02 17:25 ` [PATCH v2 5/7] powerpc: add capability to prepend default " Christophe Leroy 2021-03-02 17:25 ` Christophe Leroy 2021-03-02 17:25 ` [PATCH v2 6/7] cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation Christophe Leroy 2021-03-02 17:25 ` Christophe Leroy 2021-03-02 19:30 ` kernel test robot 2021-03-02 19:30 ` kernel test robot 2021-03-02 19:30 ` kernel test robot 2021-03-03 17:57 ` Will Deacon 2021-03-03 17:57 ` Will Deacon 2021-03-25 11:18 ` Christophe Leroy 2021-03-25 11:18 ` Christophe Leroy 2021-03-25 19:32 ` Will Deacon 2021-03-25 19:32 ` Will Deacon 2021-03-26 6:18 ` Christophe Leroy 2021-03-26 6:18 ` Christophe Leroy 2021-03-26 6:44 ` Christophe Leroy 2021-03-26 6:44 ` Christophe Leroy 2021-03-02 17:25 ` [PATCH v2 7/7] powerpc: use generic CMDLINE manipulations Christophe Leroy 2021-03-02 17:25 ` Christophe Leroy 2021-03-02 17:35 ` [PATCH v2 0/7] Improve boot command line handling Daniel Walker 2021-03-02 17:35 ` Daniel Walker 2021-03-02 17:39 ` Christophe Leroy 2021-03-02 17:39 ` Christophe Leroy 2021-03-03 2:01 ` Rob Herring 2021-03-03 2:01 ` Rob Herring 2021-03-03 17:39 ` Daniel Walker 2021-03-03 17:39 ` Daniel Walker 2021-03-03 18:07 ` Christophe Leroy 2021-03-03 18:07 ` Christophe Leroy 2021-03-03 18:53 ` Daniel Walker 2021-03-03 18:53 ` Daniel Walker
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=a0cfef11-efba-2e5c-6f58-ed63a2c3bfa0@csgroup.eu \ --to=christophe.leroy@csgroup.eu \ --cc=benh@kernel.crashing.org \ --cc=daniel@gimpelevich.san-francisco.ca.us \ --cc=danielwa@cisco.com \ --cc=devicetree@vger.kernel.org \ --cc=linux-arch@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mpe@ellerman.id.au \ --cc=paulus@samba.org \ --cc=robh@kernel.org \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.