* [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs"
@ 2021-02-25 12:59 Will Deacon
2021-02-25 12:59 ` [PATCH 1/2] arm64: cpufeatures: Fix handling of CONFIG_CMDLINE for idreg overrides Will Deacon
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Will Deacon @ 2021-02-25 12:59 UTC (permalink / raw)
To: linux-kernel
Cc: Rob Herring, kernel-team, Catalin Marinas, Arnd Bergmann,
Frank Rowand, devicetree, Marc Zyngier, Doug Anderson,
Tyler Hicks, Palmer Dabbelt, Greg Kroah-Hartman, Max Uvarov,
Will Deacon, Ard Biesheuvel, linux-arm-kernel
Hi folks,
We recently [1] enabled support for CMDLINE_EXTEND on arm64, however
when I started looking at replacing Android's out-of-tree implementation [2]
with the upstream version, I noticed that the two behave significantly
differently: Android follows the Kconfig help text of appending the
bootloader arguments to the kernel command line, whereas upstream appends
the kernel command line to the bootloader arguments. That is, except for
the EFI stub, which follows the documented behaviour.
I think the documented behaviour is more useful, so this patch series
reworks the FDT code to follow that and updates the very recently merged
arm64 idreg early command-line parsing as well.
I'd like to take the first patch as a fix via the arm64 tree.
Cheers,
Will
[1] https://lore.kernel.org/r/160650434702.20875.12520970127987518808.b4-ty@arm.com
[2] https://android-review.googlesource.com/c/kernel/common/+/841045
--->8
Cc: Max Uvarov <muvarov@gmail.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Doug Anderson <dianders@chromium.org>
Cc: Tyler Hicks <tyhicks@linux.microsoft.com>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: <kernel-team@android.com>
Cc: <linux-arm-kernel@lists.infradead.org>
Cc: <devicetree@vger.kernel.org>
Will Deacon (2):
arm64: cpufeatures: Fix handling of CONFIG_CMDLINE for idreg overrides
of/fdt: Append bootloader arguments when CMDLINE_EXTEND=y
arch/arm64/kernel/idreg-override.c | 44 +++++++++++---------
drivers/of/fdt.c | 64 ++++++++++++++++++------------
2 files changed, 64 insertions(+), 44 deletions(-)
--
2.30.1.766.gb4fecdf3b7-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/2] arm64: cpufeatures: Fix handling of CONFIG_CMDLINE for idreg overrides 2021-02-25 12:59 [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" Will Deacon @ 2021-02-25 12:59 ` Will Deacon 2021-02-25 13:53 ` Marc Zyngier 2021-02-25 12:59 ` [PATCH 2/2] of/fdt: Append bootloader arguments when CMDLINE_EXTEND=y Will Deacon 2021-03-01 14:19 ` [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" Rob Herring 2 siblings, 1 reply; 13+ messages in thread From: Will Deacon @ 2021-02-25 12:59 UTC (permalink / raw) To: linux-kernel Cc: Rob Herring, kernel-team, Catalin Marinas, Arnd Bergmann, Frank Rowand, devicetree, Marc Zyngier, Doug Anderson, Tyler Hicks, Palmer Dabbelt, Greg Kroah-Hartman, Max Uvarov, Will Deacon, Ard Biesheuvel, linux-arm-kernel The built-in kernel commandline (CONFIG_CMDLINE) can be configured in three different ways: 1. CMDLINE_FORCE: Use CONFIG_CMDLINE instead of any bootloader args 2. CMDLINE_EXTEND: Append the bootloader args to CONFIG_CMDLINE 3. CMDLINE_FROM_BOOTLOADER: Only use CONFIG_CMDLINE if there aren't any bootloader args. The early cmdline parsing to detect idreg overrides gets (2) and (3) slightly wrong: in the case of (2) the bootloader args are parsed first and in the case of (3) the CMDLINE is always parsed. Fix these issues by moving the bootargs parsing out into a helper function and following the same logic as that used by the EFI stub. Cc: Marc Zyngier <maz@kernel.org> Fixes: 33200303553d ("arm64: cpufeature: Add an early command-line cpufeature override facility") Signed-off-by: Will Deacon <will@kernel.org> --- arch/arm64/kernel/idreg-override.c | 44 +++++++++++++++++------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c index dffb16682330..cc071712c6f9 100644 --- a/arch/arm64/kernel/idreg-override.c +++ b/arch/arm64/kernel/idreg-override.c @@ -163,33 +163,39 @@ static __init void __parse_cmdline(const char *cmdline, bool parse_aliases) } while (1); } -static __init void parse_cmdline(void) +static __init const u8 *get_bootargs_cmdline(void) { - if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) { - const u8 *prop; - void *fdt; - int node; + const u8 *prop; + void *fdt; + int node; - fdt = get_early_fdt_ptr(); - if (!fdt) - goto out; + fdt = get_early_fdt_ptr(); + if (!fdt) + return NULL; - node = fdt_path_offset(fdt, "/chosen"); - if (node < 0) - goto out; + node = fdt_path_offset(fdt, "/chosen"); + if (node < 0) + return NULL; - prop = fdt_getprop(fdt, node, "bootargs", NULL); - if (!prop) - goto out; + prop = fdt_getprop(fdt, node, "bootargs", NULL); + if (!prop) + return NULL; - __parse_cmdline(prop, true); + return strlen(prop) ? prop : NULL; +} - if (!IS_ENABLED(CONFIG_CMDLINE_EXTEND)) - return; +static __init void parse_cmdline(void) +{ + const u8 *prop = get_bootargs_cmdline(); + + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || + IS_ENABLED(CONFIG_CMDLINE_FORCE) || + !prop) { + __parse_cmdline(CONFIG_CMDLINE, true); } -out: - __parse_cmdline(CONFIG_CMDLINE, true); + if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && prop) + __parse_cmdline(prop, true); } /* Keep checkers quiet */ -- 2.30.1.766.gb4fecdf3b7-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] arm64: cpufeatures: Fix handling of CONFIG_CMDLINE for idreg overrides 2021-02-25 12:59 ` [PATCH 1/2] arm64: cpufeatures: Fix handling of CONFIG_CMDLINE for idreg overrides Will Deacon @ 2021-02-25 13:53 ` Marc Zyngier 2021-02-25 14:04 ` Will Deacon 0 siblings, 1 reply; 13+ messages in thread From: Marc Zyngier @ 2021-02-25 13:53 UTC (permalink / raw) To: Will Deacon Cc: Rob Herring, kernel-team, Arnd Bergmann, devicetree, Greg Kroah-Hartman, Doug Anderson, linux-kernel, Tyler Hicks, Palmer Dabbelt, Catalin Marinas, Max Uvarov, Frank Rowand, Ard Biesheuvel, linux-arm-kernel On Thu, 25 Feb 2021 12:59:20 +0000, Will Deacon <will@kernel.org> wrote: > > The built-in kernel commandline (CONFIG_CMDLINE) can be configured in > three different ways: > > 1. CMDLINE_FORCE: Use CONFIG_CMDLINE instead of any bootloader args > 2. CMDLINE_EXTEND: Append the bootloader args to CONFIG_CMDLINE > 3. CMDLINE_FROM_BOOTLOADER: Only use CONFIG_CMDLINE if there aren't > any bootloader args. > > The early cmdline parsing to detect idreg overrides gets (2) and (3) > slightly wrong: in the case of (2) the bootloader args are parsed first > and in the case of (3) the CMDLINE is always parsed. > > Fix these issues by moving the bootargs parsing out into a helper > function and following the same logic as that used by the EFI stub. > > Cc: Marc Zyngier <maz@kernel.org> > Fixes: 33200303553d ("arm64: cpufeature: Add an early command-line cpufeature override facility") > Signed-off-by: Will Deacon <will@kernel.org> > --- > arch/arm64/kernel/idreg-override.c | 44 +++++++++++++++++------------- > 1 file changed, 25 insertions(+), 19 deletions(-) > > diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c > index dffb16682330..cc071712c6f9 100644 > --- a/arch/arm64/kernel/idreg-override.c > +++ b/arch/arm64/kernel/idreg-override.c > @@ -163,33 +163,39 @@ static __init void __parse_cmdline(const char *cmdline, bool parse_aliases) > } while (1); > } > > -static __init void parse_cmdline(void) > +static __init const u8 *get_bootargs_cmdline(void) > { > - if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) { > - const u8 *prop; > - void *fdt; > - int node; > + const u8 *prop; > + void *fdt; > + int node; > > - fdt = get_early_fdt_ptr(); > - if (!fdt) > - goto out; > + fdt = get_early_fdt_ptr(); > + if (!fdt) > + return NULL; > > - node = fdt_path_offset(fdt, "/chosen"); > - if (node < 0) > - goto out; > + node = fdt_path_offset(fdt, "/chosen"); > + if (node < 0) > + return NULL; > > - prop = fdt_getprop(fdt, node, "bootargs", NULL); > - if (!prop) > - goto out; > + prop = fdt_getprop(fdt, node, "bootargs", NULL); > + if (!prop) > + return NULL; > > - __parse_cmdline(prop, true); > + return strlen(prop) ? prop : NULL; > +} > > - if (!IS_ENABLED(CONFIG_CMDLINE_EXTEND)) > - return; > +static __init void parse_cmdline(void) > +{ > + const u8 *prop = get_bootargs_cmdline(); > + > + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || > + IS_ENABLED(CONFIG_CMDLINE_FORCE) || > + !prop) { The logic hurts, but I think I grok it now. The last term is actually a reduction of (IS_ENABLED(CONFIG_CMDLINE_FROM_BOOTLOADER) && !prop) and we know for sure that if none of the other two terms are true, then CMDLINE_FROM_BOOTLOADER *must* be enabled. > + __parse_cmdline(CONFIG_CMDLINE, true); > } > > -out: > - __parse_cmdline(CONFIG_CMDLINE, true); > + if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && prop) > + __parse_cmdline(prop, true); > } > > /* Keep checkers quiet */ I don't think we need to backport anything to stable for the nokaslr handling, do we? Otherwise, Reviewed-by: Marc Zyngier <maz@kernel.org> Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] arm64: cpufeatures: Fix handling of CONFIG_CMDLINE for idreg overrides 2021-02-25 13:53 ` Marc Zyngier @ 2021-02-25 14:04 ` Will Deacon 0 siblings, 0 replies; 13+ messages in thread From: Will Deacon @ 2021-02-25 14:04 UTC (permalink / raw) To: Marc Zyngier Cc: Rob Herring, kernel-team, Arnd Bergmann, devicetree, Greg Kroah-Hartman, Doug Anderson, linux-kernel, Tyler Hicks, Palmer Dabbelt, Catalin Marinas, Max Uvarov, Frank Rowand, Ard Biesheuvel, linux-arm-kernel On Thu, Feb 25, 2021 at 01:53:56PM +0000, Marc Zyngier wrote: > On Thu, 25 Feb 2021 12:59:20 +0000, > Will Deacon <will@kernel.org> wrote: > > > > The built-in kernel commandline (CONFIG_CMDLINE) can be configured in > > three different ways: > > > > 1. CMDLINE_FORCE: Use CONFIG_CMDLINE instead of any bootloader args > > 2. CMDLINE_EXTEND: Append the bootloader args to CONFIG_CMDLINE > > 3. CMDLINE_FROM_BOOTLOADER: Only use CONFIG_CMDLINE if there aren't > > any bootloader args. > > > > The early cmdline parsing to detect idreg overrides gets (2) and (3) > > slightly wrong: in the case of (2) the bootloader args are parsed first > > and in the case of (3) the CMDLINE is always parsed. > > > > Fix these issues by moving the bootargs parsing out into a helper > > function and following the same logic as that used by the EFI stub. > > > > Cc: Marc Zyngier <maz@kernel.org> > > Fixes: 33200303553d ("arm64: cpufeature: Add an early command-line cpufeature override facility") > > Signed-off-by: Will Deacon <will@kernel.org> > > --- > > arch/arm64/kernel/idreg-override.c | 44 +++++++++++++++++------------- > > 1 file changed, 25 insertions(+), 19 deletions(-) > > > > diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c > > index dffb16682330..cc071712c6f9 100644 > > --- a/arch/arm64/kernel/idreg-override.c > > +++ b/arch/arm64/kernel/idreg-override.c > > @@ -163,33 +163,39 @@ static __init void __parse_cmdline(const char *cmdline, bool parse_aliases) > > } while (1); > > } > > > > -static __init void parse_cmdline(void) > > +static __init const u8 *get_bootargs_cmdline(void) > > { > > - if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) { > > - const u8 *prop; > > - void *fdt; > > - int node; > > + const u8 *prop; > > + void *fdt; > > + int node; > > > > - fdt = get_early_fdt_ptr(); > > - if (!fdt) > > - goto out; > > + fdt = get_early_fdt_ptr(); > > + if (!fdt) > > + return NULL; > > > > - node = fdt_path_offset(fdt, "/chosen"); > > - if (node < 0) > > - goto out; > > + node = fdt_path_offset(fdt, "/chosen"); > > + if (node < 0) > > + return NULL; > > > > - prop = fdt_getprop(fdt, node, "bootargs", NULL); > > - if (!prop) > > - goto out; > > + prop = fdt_getprop(fdt, node, "bootargs", NULL); > > + if (!prop) > > + return NULL; > > > > - __parse_cmdline(prop, true); > > + return strlen(prop) ? prop : NULL; > > +} > > > > - if (!IS_ENABLED(CONFIG_CMDLINE_EXTEND)) > > - return; > > +static __init void parse_cmdline(void) > > +{ > > + const u8 *prop = get_bootargs_cmdline(); > > + > > + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || > > + IS_ENABLED(CONFIG_CMDLINE_FORCE) || > > + !prop) { > > The logic hurts, but I think I grok it now. The last term is actually > a reduction of > > (IS_ENABLED(CONFIG_CMDLINE_FROM_BOOTLOADER) && !prop) > > and we know for sure that if none of the other two terms are true, > then CMDLINE_FROM_BOOTLOADER *must* be enabled. Yes, with one gotcha: when CONFIG_CMDLINE is "", I don't think any of the CONFIG_CMDLINE_* are set, but the behaviour ends up being the same as CMDLINE_FROM_BOOTLOADER. > > > + __parse_cmdline(CONFIG_CMDLINE, true); > > } > > > > -out: > > - __parse_cmdline(CONFIG_CMDLINE, true); > > + if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && prop) > > + __parse_cmdline(prop, true); > > } > > > > /* Keep checkers quiet */ > > I don't think we need to backport anything to stable for the nokaslr > handling, do we? No, I don't think so. There isn't a "kaslr" or "nonokaslr", so the ordering doesn't matter afaict. > Reviewed-by: Marc Zyngier <maz@kernel.org> Cheers! Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] of/fdt: Append bootloader arguments when CMDLINE_EXTEND=y 2021-02-25 12:59 [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" Will Deacon 2021-02-25 12:59 ` [PATCH 1/2] arm64: cpufeatures: Fix handling of CONFIG_CMDLINE for idreg overrides Will Deacon @ 2021-02-25 12:59 ` Will Deacon 2021-02-25 14:08 ` Marc Zyngier 2021-03-01 14:19 ` [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" Rob Herring 2 siblings, 1 reply; 13+ messages in thread From: Will Deacon @ 2021-02-25 12:59 UTC (permalink / raw) To: linux-kernel Cc: Rob Herring, kernel-team, Catalin Marinas, Arnd Bergmann, Frank Rowand, devicetree, Marc Zyngier, Doug Anderson, Tyler Hicks, Palmer Dabbelt, Greg Kroah-Hartman, Max Uvarov, Will Deacon, Ard Biesheuvel, linux-arm-kernel The Kconfig help text for CMDLINE_EXTEND is sadly duplicated across all architectures that implement it (arm, arm64, powerpc, riscv and sh), but they all seem to agree that the bootloader arguments will be appended to the CONFIG_CMDLINE. For example, on arm64: | The command-line arguments provided by the boot loader will be | appended to the default kernel command string. This also matches the behaviour of the EFI stub, which parses the bootloader arguments first if CMDLINE_EXTEND is set, as well as the out-of-tree CMDLINE_EXTEND implementation in Android. However, the behaviour in the upstream fdt code appears to be the other way around: CONFIG_CMDLINE is appended to the bootloader arguments. Fix the code to follow the documentation by moving the cmdline processing out into a new function, early_init_dt_retrieve_cmdline(), and copying CONFIG_CMDLINE to the beginning of the cmdline buffer rather than concatenating it onto the end. Cc: Max Uvarov <muvarov@gmail.com> Cc: Rob Herring <robh@kernel.org> Cc: Ard Biesheuvel <ardb@kernel.org> Cc: Marc Zyngier <maz@kernel.org> Cc: Doug Anderson <dianders@chromium.org> Cc: Tyler Hicks <tyhicks@linux.microsoft.com> Cc: Frank Rowand <frowand.list@gmail.com> Fixes: 34b82026a507 ("fdt: fix extend of cmd line") Signed-off-by: Will Deacon <will@kernel.org> --- drivers/of/fdt.c | 64 +++++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index dcc1dd96911a..83b9d065e58d 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -1033,11 +1033,48 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname, return 0; } +static int __init cmdline_from_bootargs(unsigned long node, void *dst, int sz) +{ + int l; + const char *p = of_get_flat_dt_prop(node, "bootargs", &l); + + if (!p || l <= 0) + return -EINVAL; + + return strlcpy(dst, p, min(l, sz)); +} + +/* dst is a zero-initialised buffer of COMMAND_LINE_SIZE bytes */ +static void __init early_init_dt_retrieve_cmdline(unsigned long node, char *dst) +{ + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) { + /* Copy CONFIG_CMDLINE to the start of destination buffer */ + size_t idx = strlcpy(dst, CONFIG_CMDLINE, COMMAND_LINE_SIZE); + + /* Check that we have enough space to concatenate */ + if (idx + 1 >= COMMAND_LINE_SIZE) + return; + + /* Append the bootloader arguments */ + dst[idx++] = ' '; + cmdline_from_bootargs(node, &dst[idx], COMMAND_LINE_SIZE - idx); + } else if (IS_ENABLED(CONFIG_CMDLINE_FORCE)) { + /* Just use CONFIG_CMDLINE */ + strlcpy(dst, CONFIG_CMDLINE, COMMAND_LINE_SIZE); + } else if (IS_ENABLED(CONFIG_CMDLINE_FROM_BOOTLOADER)) { + /* Use CONFIG_CMDLINE if no arguments from bootloader. */ + if (cmdline_from_bootargs(node, dst, COMMAND_LINE_SIZE) <= 0) + strlcpy(dst, CONFIG_CMDLINE, COMMAND_LINE_SIZE); + } else { + /* Just use bootloader arguments */ + cmdline_from_bootargs(node, dst, COMMAND_LINE_SIZE); + } +} + int __init early_init_dt_scan_chosen(unsigned long node, const char *uname, int depth, void *data) { int l; - const char *p; const void *rng_seed; pr_debug("search \"chosen\", depth: %d, uname: %s\n", depth, uname); @@ -1047,30 +1084,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname, return 0; early_init_dt_check_for_initrd(node); - - /* Retrieve command line */ - p = of_get_flat_dt_prop(node, "bootargs", &l); - if (p != NULL && l > 0) - strlcpy(data, p, min(l, COMMAND_LINE_SIZE)); - - /* - * CONFIG_CMDLINE is meant to be a default in case nothing else - * managed to set the command line, unless CONFIG_CMDLINE_FORCE - * is set in which case we override whatever was found earlier. - */ -#ifdef CONFIG_CMDLINE -#if defined(CONFIG_CMDLINE_EXTEND) - strlcat(data, " ", COMMAND_LINE_SIZE); - strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE); -#elif defined(CONFIG_CMDLINE_FORCE) - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE); -#else - /* No arguments from boot loader, use kernel's cmdl*/ - if (!((char *)data)[0]) - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE); -#endif -#endif /* CONFIG_CMDLINE */ - + early_init_dt_retrieve_cmdline(node, data); pr_debug("Command line is: %s\n", (char *)data); rng_seed = of_get_flat_dt_prop(node, "rng-seed", &l); -- 2.30.1.766.gb4fecdf3b7-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] of/fdt: Append bootloader arguments when CMDLINE_EXTEND=y 2021-02-25 12:59 ` [PATCH 2/2] of/fdt: Append bootloader arguments when CMDLINE_EXTEND=y Will Deacon @ 2021-02-25 14:08 ` Marc Zyngier 0 siblings, 0 replies; 13+ messages in thread From: Marc Zyngier @ 2021-02-25 14:08 UTC (permalink / raw) To: Will Deacon Cc: Rob Herring, kernel-team, Arnd Bergmann, devicetree, Greg Kroah-Hartman, Doug Anderson, linux-kernel, Tyler Hicks, Palmer Dabbelt, Catalin Marinas, Max Uvarov, Frank Rowand, Ard Biesheuvel, linux-arm-kernel On Thu, 25 Feb 2021 12:59:21 +0000, Will Deacon <will@kernel.org> wrote: > > The Kconfig help text for CMDLINE_EXTEND is sadly duplicated across all > architectures that implement it (arm, arm64, powerpc, riscv and sh), but > they all seem to agree that the bootloader arguments will be appended to > the CONFIG_CMDLINE. For example, on arm64: > > | The command-line arguments provided by the boot loader will be > | appended to the default kernel command string. > > This also matches the behaviour of the EFI stub, which parses the > bootloader arguments first if CMDLINE_EXTEND is set, as well as the > out-of-tree CMDLINE_EXTEND implementation in Android. > > However, the behaviour in the upstream fdt code appears to be the other > way around: CONFIG_CMDLINE is appended to the bootloader arguments. > > Fix the code to follow the documentation by moving the cmdline > processing out into a new function, early_init_dt_retrieve_cmdline(), > and copying CONFIG_CMDLINE to the beginning of the cmdline buffer rather > than concatenating it onto the end. > > Cc: Max Uvarov <muvarov@gmail.com> > Cc: Rob Herring <robh@kernel.org> > Cc: Ard Biesheuvel <ardb@kernel.org> > Cc: Marc Zyngier <maz@kernel.org> > Cc: Doug Anderson <dianders@chromium.org> > Cc: Tyler Hicks <tyhicks@linux.microsoft.com> > Cc: Frank Rowand <frowand.list@gmail.com> > Fixes: 34b82026a507 ("fdt: fix extend of cmd line") > Signed-off-by: Will Deacon <will@kernel.org> > --- > drivers/of/fdt.c | 64 +++++++++++++++++++++++++++++------------------- > 1 file changed, 39 insertions(+), 25 deletions(-) > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c > index dcc1dd96911a..83b9d065e58d 100644 > --- a/drivers/of/fdt.c > +++ b/drivers/of/fdt.c > @@ -1033,11 +1033,48 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname, > return 0; > } > > +static int __init cmdline_from_bootargs(unsigned long node, void *dst, int sz) > +{ > + int l; > + const char *p = of_get_flat_dt_prop(node, "bootargs", &l); > + > + if (!p || l <= 0) > + return -EINVAL; > + > + return strlcpy(dst, p, min(l, sz)); > +} > + > +/* dst is a zero-initialised buffer of COMMAND_LINE_SIZE bytes */ > +static void __init early_init_dt_retrieve_cmdline(unsigned long node, char *dst) > +{ > + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) { > + /* Copy CONFIG_CMDLINE to the start of destination buffer */ > + size_t idx = strlcpy(dst, CONFIG_CMDLINE, COMMAND_LINE_SIZE); > + > + /* Check that we have enough space to concatenate */ > + if (idx + 1 >= COMMAND_LINE_SIZE) > + return; > + > + /* Append the bootloader arguments */ > + dst[idx++] = ' '; > + cmdline_from_bootargs(node, &dst[idx], COMMAND_LINE_SIZE - idx); > + } else if (IS_ENABLED(CONFIG_CMDLINE_FORCE)) { > + /* Just use CONFIG_CMDLINE */ > + strlcpy(dst, CONFIG_CMDLINE, COMMAND_LINE_SIZE); > + } else if (IS_ENABLED(CONFIG_CMDLINE_FROM_BOOTLOADER)) { > + /* Use CONFIG_CMDLINE if no arguments from bootloader. */ > + if (cmdline_from_bootargs(node, dst, COMMAND_LINE_SIZE) <= 0) > + strlcpy(dst, CONFIG_CMDLINE, COMMAND_LINE_SIZE); > + } else { Do we have any arch that can end-up not defining any of the 3 above cases? We should be able to just have the above case as the catch-all, and drop the one below. > + /* Just use bootloader arguments */ > + cmdline_from_bootargs(node, dst, COMMAND_LINE_SIZE); > + } > +} > + > int __init early_init_dt_scan_chosen(unsigned long node, const char *uname, > int depth, void *data) > { > int l; > - const char *p; > const void *rng_seed; > > pr_debug("search \"chosen\", depth: %d, uname: %s\n", depth, uname); > @@ -1047,30 +1084,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname, > return 0; > > early_init_dt_check_for_initrd(node); > - > - /* Retrieve command line */ > - p = of_get_flat_dt_prop(node, "bootargs", &l); > - if (p != NULL && l > 0) > - strlcpy(data, p, min(l, COMMAND_LINE_SIZE)); > - > - /* > - * CONFIG_CMDLINE is meant to be a default in case nothing else > - * managed to set the command line, unless CONFIG_CMDLINE_FORCE > - * is set in which case we override whatever was found earlier. > - */ > -#ifdef CONFIG_CMDLINE > -#if defined(CONFIG_CMDLINE_EXTEND) > - strlcat(data, " ", COMMAND_LINE_SIZE); > - strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE); > -#elif defined(CONFIG_CMDLINE_FORCE) > - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE); > -#else > - /* No arguments from boot loader, use kernel's cmdl*/ > - if (!((char *)data)[0]) > - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE); > -#endif > -#endif /* CONFIG_CMDLINE */ > - > + early_init_dt_retrieve_cmdline(node, data); > pr_debug("Command line is: %s\n", (char *)data); > > rng_seed = of_get_flat_dt_prop(node, "rng-seed", &l); > -- > 2.30.1.766.gb4fecdf3b7-goog > > Other than the above nit: Reviewed-by: Marc Zyngier <maz@kernel.org> Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" 2021-02-25 12:59 [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" Will Deacon 2021-02-25 12:59 ` [PATCH 1/2] arm64: cpufeatures: Fix handling of CONFIG_CMDLINE for idreg overrides Will Deacon 2021-02-25 12:59 ` [PATCH 2/2] of/fdt: Append bootloader arguments when CMDLINE_EXTEND=y Will Deacon @ 2021-03-01 14:19 ` Rob Herring 2021-03-01 14:41 ` Will Deacon 2 siblings, 1 reply; 13+ messages in thread From: Rob Herring @ 2021-03-01 14:19 UTC (permalink / raw) To: Will Deacon Cc: devicetree, Android Kernel Team, Catalin Marinas, Arnd Bergmann, Marc Zyngier, Doug Anderson, linux-kernel, Tyler Hicks, Palmer Dabbelt, Greg Kroah-Hartman, Max Uvarov, Frank Rowand, Ard Biesheuvel, linux-arm-kernel On Thu, Feb 25, 2021 at 6:59 AM Will Deacon <will@kernel.org> wrote: > > Hi folks, > > We recently [1] enabled support for CMDLINE_EXTEND on arm64, however > when I started looking at replacing Android's out-of-tree implementation [2] Did anyone go read the common, reworked version of all this I referenced that supports prepend and append. Here it is again[1]. Maybe I should have been more assertive there and said 'extend' is ambiguous. > with the upstream version, I noticed that the two behave significantly > differently: Android follows the Kconfig help text of appending the > bootloader arguments to the kernel command line, whereas upstream appends > the kernel command line to the bootloader arguments. That is, except for > the EFI stub, which follows the documented behaviour. > > I think the documented behaviour is more useful, so this patch series > reworks the FDT code to follow that and updates the very recently merged > arm64 idreg early command-line parsing as well. I can just as easily argue that the kernel having the last say makes sense. Regardless, I'm pretty sure there's someone out there relying on current behavior. What is the impact of this change to other arches? Rob [1] https://lore.kernel.org/lkml/20190319232448.45964-2-danielwa@cisco.com/ _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" 2021-03-01 14:19 ` [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" Rob Herring @ 2021-03-01 14:41 ` Will Deacon 2021-03-01 17:26 ` Rob Herring 0 siblings, 1 reply; 13+ messages in thread From: Will Deacon @ 2021-03-01 14:41 UTC (permalink / raw) To: Rob Herring Cc: devicetree, Android Kernel Team, Catalin Marinas, Arnd Bergmann, Marc Zyngier, Doug Anderson, linux-kernel, Tyler Hicks, Palmer Dabbelt, Greg Kroah-Hartman, Max Uvarov, Frank Rowand, Ard Biesheuvel, linux-arm-kernel On Mon, Mar 01, 2021 at 08:19:32AM -0600, Rob Herring wrote: > On Thu, Feb 25, 2021 at 6:59 AM Will Deacon <will@kernel.org> wrote: > > We recently [1] enabled support for CMDLINE_EXTEND on arm64, however > > when I started looking at replacing Android's out-of-tree implementation [2] > > Did anyone go read the common, reworked version of all this I > referenced that supports prepend and append. Here it is again[1]. > Maybe I should have been more assertive there and said 'extend' is > ambiguous. I tried reading that, but (a) most of the series is not in the mailing list archives and (b) the patch that _is_ doesn't touch CMDLINE_EXTEND at all. Right now the code in mainline does the opposite of what it's documented to do. > > with the upstream version, I noticed that the two behave significantly > > differently: Android follows the Kconfig help text of appending the > > bootloader arguments to the kernel command line, whereas upstream appends > > the kernel command line to the bootloader arguments. That is, except for > > the EFI stub, which follows the documented behaviour. > > > > I think the documented behaviour is more useful, so this patch series > > reworks the FDT code to follow that and updates the very recently merged > > arm64 idreg early command-line parsing as well. > > I can just as easily argue that the kernel having the last say makes > sense. Dunno, I'd say that's what CMDLINE_FORCE is for. Plus you'd be arguing against both the documentation and the EFI stub implementation. > Regardless, I'm pretty sure there's someone out there relying on current > behavior. What is the impact of this change to other arches? On arm64, I doubt it, as Android is the main user of this (where it's been supported for 9 years with the documented behaviour). The other option, then, is reverting CMDLINE_EXTEND from arm64 until this is figured out. I think that's preferable to having divergent behaviour. As for other architectures, I think the ATAGs-based solution on arch/arm/ gets it right: static int __init parse_tag_cmdline(const struct tag *tag) { #if defined(CONFIG_CMDLINE_EXTEND) strlcat(default_command_line, " ", COMMAND_LINE_SIZE); strlcat(default_command_line, tag->u.cmdline.cmdline, COMMAND_LINE_SIZE); For now I think we have two options for arm64: either fix the fdt code, or revert CMDLINE_EXTEND until the PREPEND/APPEND series is merged. Which do you prefer? Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" 2021-03-01 14:41 ` Will Deacon @ 2021-03-01 17:26 ` Rob Herring 2021-03-01 17:45 ` Christophe Leroy 2021-03-02 17:12 ` Daniel Walker 0 siblings, 2 replies; 13+ messages in thread From: Rob Herring @ 2021-03-01 17:26 UTC (permalink / raw) To: Will Deacon, Chris Packham, Daniel Walker Cc: devicetree, Android Kernel Team, Catalin Marinas, Arnd Bergmann, Marc Zyngier, linuxppc-dev, Doug Anderson, linux-kernel, Tyler Hicks, Palmer Dabbelt, Michael Ellerman, Greg Kroah-Hartman, Max Uvarov, Frank Rowand, Ard Biesheuvel, linux-arm-kernel +PPC folks and Daniel W On Mon, Mar 1, 2021 at 8:42 AM Will Deacon <will@kernel.org> wrote: > > On Mon, Mar 01, 2021 at 08:19:32AM -0600, Rob Herring wrote: > > On Thu, Feb 25, 2021 at 6:59 AM Will Deacon <will@kernel.org> wrote: > > > We recently [1] enabled support for CMDLINE_EXTEND on arm64, however > > > when I started looking at replacing Android's out-of-tree implementation [2] > > > > Did anyone go read the common, reworked version of all this I > > referenced that supports prepend and append. Here it is again[1]. > > Maybe I should have been more assertive there and said 'extend' is > > ambiguous. > > I tried reading that, but (a) most of the series is not in the mailing list > archives and (b) the patch that _is_ doesn't touch CMDLINE_EXTEND at all. > Right now the code in mainline does the opposite of what it's documented to > do. Actually, there is a newer version I found: https://lore.kernel.org/linuxppc-dev/1551469472-53043-1-git-send-email-danielwa@cisco.com/ https://lore.kernel.org/linuxppc-dev/1551469472-53043-2-git-send-email-danielwa@cisco.com/ https://lore.kernel.org/linuxppc-dev/1551469472-53043-3-git-send-email-danielwa@cisco.com/ (Once again, there's some weird threading going on) > > > with the upstream version, I noticed that the two behave significantly > > > differently: Android follows the Kconfig help text of appending the > > > bootloader arguments to the kernel command line, whereas upstream appends > > > the kernel command line to the bootloader arguments. That is, except for > > > the EFI stub, which follows the documented behaviour. > > > > > > I think the documented behaviour is more useful, so this patch series > > > reworks the FDT code to follow that and updates the very recently merged > > > arm64 idreg early command-line parsing as well. > > > > I can just as easily argue that the kernel having the last say makes > > sense. > > Dunno, I'd say that's what CMDLINE_FORCE is for. Plus you'd be arguing > against both the documentation and the EFI stub implementation. CMDLINE_FORCE is a complete override, not a merging of command lines. > > Regardless, I'm pretty sure there's someone out there relying on current > > behavior. What is the impact of this change to other arches? > > On arm64, I doubt it, as Android is the main user of this (where it's been > supported for 9 years with the documented behaviour). > > The other option, then, is reverting CMDLINE_EXTEND from arm64 until this is > figured out. I think that's preferable to having divergent behaviour. > > As for other architectures, I think the ATAGs-based solution on arch/arm/ > gets it right: > > static int __init parse_tag_cmdline(const struct tag *tag) > { > #if defined(CONFIG_CMDLINE_EXTEND) > strlcat(default_command_line, " ", COMMAND_LINE_SIZE); > strlcat(default_command_line, tag->u.cmdline.cmdline, > COMMAND_LINE_SIZE); The question is really whether any arm32 DT based platform depends on the current behavior. RiscV could also be relying on current behavior. Powerpc also uses the current behavior (and the documentation is also wrong there). Changing the behavior in the FDT code means the powerpc early PROM code and the FDT code do the opposite. Arm32 has had current behaviour for 5 years. Powerpc for 1.5 years and Risc-V for 2 years. Then there's MIPS which has its own Kconfig symbols for this and is its own kind of mess. Either we assume existing users didn't really care about the order or we have to support both prepend and append. > For now I think we have two options for arm64: either fix the fdt code, > or revert CMDLINE_EXTEND until the PREPEND/APPEND series is merged. Which > do you prefer? Like anything copied across arches, I want someone to look at this across all architectures and make this common instead of just copying to new arches. The prepend/append series is the closest we've come. Rob _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" 2021-03-01 17:26 ` Rob Herring @ 2021-03-01 17:45 ` Christophe Leroy 2021-03-02 14:56 ` Rob Herring 2021-03-02 17:12 ` Daniel Walker 1 sibling, 1 reply; 13+ messages in thread From: Christophe Leroy @ 2021-03-01 17:45 UTC (permalink / raw) To: Rob Herring, Will Deacon, Chris Packham, Daniel Walker Cc: devicetree, Arnd Bergmann, Greg Kroah-Hartman, Marc Zyngier, Frank Rowand, linuxppc-dev, Doug Anderson, Tyler Hicks, Palmer Dabbelt, linux-arm-kernel, Catalin Marinas, Max Uvarov, Android Kernel Team, Ard Biesheuvel, linux-kernel Le 01/03/2021 à 18:26, Rob Herring a écrit : > +PPC folks and Daniel W > > On Mon, Mar 1, 2021 at 8:42 AM Will Deacon <will@kernel.org> wrote: >> >> On Mon, Mar 01, 2021 at 08:19:32AM -0600, Rob Herring wrote: >>> On Thu, Feb 25, 2021 at 6:59 AM Will Deacon <will@kernel.org> wrote: >>>> We recently [1] enabled support for CMDLINE_EXTEND on arm64, however >>>> when I started looking at replacing Android's out-of-tree implementation [2] >>> >>> Did anyone go read the common, reworked version of all this I >>> referenced that supports prepend and append. Here it is again[1]. >>> Maybe I should have been more assertive there and said 'extend' is >>> ambiguous. >> >> I tried reading that, but (a) most of the series is not in the mailing list >> archives and (b) the patch that _is_ doesn't touch CMDLINE_EXTEND at all. >> Right now the code in mainline does the opposite of what it's documented to >> do. > > Actually, there is a newer version I found: > > https://lore.kernel.org/linuxppc-dev/1551469472-53043-1-git-send-email-danielwa@cisco.com/ > https://lore.kernel.org/linuxppc-dev/1551469472-53043-2-git-send-email-danielwa@cisco.com/ > https://lore.kernel.org/linuxppc-dev/1551469472-53043-3-git-send-email-danielwa@cisco.com/ This was seen as too much intrusive into powerpc. I proposed an alternative at https://patchwork.ozlabs.org/project/linuxppc-dev/cover/cover.1554195798.git.christophe.leroy@c-s.fr/ but never got any feedback. > > (Once again, there's some weird threading going on) > >>>> with the upstream version, I noticed that the two behave significantly >>>> differently: Android follows the Kconfig help text of appending the >>>> bootloader arguments to the kernel command line, whereas upstream appends >>>> the kernel command line to the bootloader arguments. That is, except for >>>> the EFI stub, which follows the documented behaviour. >>>> >>>> I think the documented behaviour is more useful, so this patch series >>>> reworks the FDT code to follow that and updates the very recently merged >>>> arm64 idreg early command-line parsing as well. >>> >>> I can just as easily argue that the kernel having the last say makes >>> sense. >> >> Dunno, I'd say that's what CMDLINE_FORCE is for. Plus you'd be arguing >> against both the documentation and the EFI stub implementation. > > CMDLINE_FORCE is a complete override, not a merging of command lines. > >>> Regardless, I'm pretty sure there's someone out there relying on current >>> behavior. What is the impact of this change to other arches? >> >> On arm64, I doubt it, as Android is the main user of this (where it's been >> supported for 9 years with the documented behaviour). >> >> The other option, then, is reverting CMDLINE_EXTEND from arm64 until this is >> figured out. I think that's preferable to having divergent behaviour. >> >> As for other architectures, I think the ATAGs-based solution on arch/arm/ >> gets it right: >> >> static int __init parse_tag_cmdline(const struct tag *tag) >> { >> #if defined(CONFIG_CMDLINE_EXTEND) >> strlcat(default_command_line, " ", COMMAND_LINE_SIZE); >> strlcat(default_command_line, tag->u.cmdline.cmdline, >> COMMAND_LINE_SIZE); > > The question is really whether any arm32 DT based platform depends on > the current behavior. RiscV could also be relying on current behavior. > Powerpc also uses the current behavior (and the documentation is also > wrong there). Changing the behavior in the FDT code means the powerpc > early PROM code and the FDT code do the opposite. > > Arm32 has had current behaviour for 5 years. Powerpc for 1.5 years and > Risc-V for 2 years. Then there's MIPS which has its own Kconfig > symbols for this and is its own kind of mess. Either we assume > existing users didn't really care about the order or we have to > support both prepend and append. > >> For now I think we have two options for arm64: either fix the fdt code, >> or revert CMDLINE_EXTEND until the PREPEND/APPEND series is merged. Which >> do you prefer? > > Like anything copied across arches, I want someone to look at this > across all architectures and make this common instead of just copying > to new arches. The prepend/append series is the closest we've come. > > Rob > Christophe _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" 2021-03-01 17:45 ` Christophe Leroy @ 2021-03-02 14:56 ` Rob Herring 2021-03-02 15:16 ` Christophe Leroy 0 siblings, 1 reply; 13+ messages in thread From: Rob Herring @ 2021-03-02 14:56 UTC (permalink / raw) To: Christophe Leroy Cc: Will Deacon, Chris Packham, Daniel Walker, devicetree, Android Kernel Team, Catalin Marinas, Arnd Bergmann, Marc Zyngier, linuxppc-dev, Doug Anderson, linux-kernel, Tyler Hicks, Palmer Dabbelt, Greg Kroah-Hartman, Max Uvarov, Frank Rowand, Ard Biesheuvel, linux-arm-kernel On Mon, Mar 1, 2021 at 11:45 AM Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > > > > Le 01/03/2021 à 18:26, Rob Herring a écrit : > > +PPC folks and Daniel W > > > > On Mon, Mar 1, 2021 at 8:42 AM Will Deacon <will@kernel.org> wrote: > >> > >> On Mon, Mar 01, 2021 at 08:19:32AM -0600, Rob Herring wrote: > >>> On Thu, Feb 25, 2021 at 6:59 AM Will Deacon <will@kernel.org> wrote: > >>>> We recently [1] enabled support for CMDLINE_EXTEND on arm64, however > >>>> when I started looking at replacing Android's out-of-tree implementation [2] > >>> > >>> Did anyone go read the common, reworked version of all this I > >>> referenced that supports prepend and append. Here it is again[1]. > >>> Maybe I should have been more assertive there and said 'extend' is > >>> ambiguous. > >> > >> I tried reading that, but (a) most of the series is not in the mailing list > >> archives and (b) the patch that _is_ doesn't touch CMDLINE_EXTEND at all. > >> Right now the code in mainline does the opposite of what it's documented to > >> do. > > > > Actually, there is a newer version I found: > > > > https://lore.kernel.org/linuxppc-dev/1551469472-53043-1-git-send-email-danielwa@cisco.com/ > > https://lore.kernel.org/linuxppc-dev/1551469472-53043-2-git-send-email-danielwa@cisco.com/ > > https://lore.kernel.org/linuxppc-dev/1551469472-53043-3-git-send-email-danielwa@cisco.com/ > > This was seen as too much intrusive into powerpc. It looked like the main issue was string functions for KASAN? As far as being too complex, I think that will be needed if you look at all architectures and non-DT cases. > I proposed an alternative at > https://patchwork.ozlabs.org/project/linuxppc-dev/cover/cover.1554195798.git.christophe.leroy@c-s.fr/ but > never got any feedback. Didn't go to a list I subscribe to. In particular, if it had gone to DT list and into PW you would have gotten a reply from me. Rob _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" 2021-03-02 14:56 ` Rob Herring @ 2021-03-02 15:16 ` Christophe Leroy 0 siblings, 0 replies; 13+ messages in thread From: Christophe Leroy @ 2021-03-02 15:16 UTC (permalink / raw) To: Rob Herring Cc: Will Deacon, Chris Packham, Daniel Walker, devicetree, Android Kernel Team, Catalin Marinas, Arnd Bergmann, Marc Zyngier, linuxppc-dev, Doug Anderson, linux-kernel, Tyler Hicks, Palmer Dabbelt, Greg Kroah-Hartman, Max Uvarov, Frank Rowand, Ard Biesheuvel, linux-arm-kernel Le 02/03/2021 à 15:56, Rob Herring a écrit : > On Mon, Mar 1, 2021 at 11:45 AM Christophe Leroy > <christophe.leroy@csgroup.eu> wrote: >> >> >> >> Le 01/03/2021 à 18:26, Rob Herring a écrit : >>> +PPC folks and Daniel W >>> >>> On Mon, Mar 1, 2021 at 8:42 AM Will Deacon <will@kernel.org> wrote: >>>> >>>> On Mon, Mar 01, 2021 at 08:19:32AM -0600, Rob Herring wrote: >>>>> On Thu, Feb 25, 2021 at 6:59 AM Will Deacon <will@kernel.org> wrote: >>>>>> We recently [1] enabled support for CMDLINE_EXTEND on arm64, however >>>>>> when I started looking at replacing Android's out-of-tree implementation [2] >>>>> >>>>> Did anyone go read the common, reworked version of all this I >>>>> referenced that supports prepend and append. Here it is again[1]. >>>>> Maybe I should have been more assertive there and said 'extend' is >>>>> ambiguous. >>>> >>>> I tried reading that, but (a) most of the series is not in the mailing list >>>> archives and (b) the patch that _is_ doesn't touch CMDLINE_EXTEND at all. >>>> Right now the code in mainline does the opposite of what it's documented to >>>> do. >>> >>> Actually, there is a newer version I found: >>> >>> https://lore.kernel.org/linuxppc-dev/1551469472-53043-1-git-send-email-danielwa@cisco.com/ >>> https://lore.kernel.org/linuxppc-dev/1551469472-53043-2-git-send-email-danielwa@cisco.com/ >>> https://lore.kernel.org/linuxppc-dev/1551469472-53043-3-git-send-email-danielwa@cisco.com/ >> >> This was seen as too much intrusive into powerpc. > > It looked like the main issue was string functions for KASAN? This is one issue yes, > > As far as being too complex, I think that will be needed if you look > at all architectures and non-DT cases. As far as I remember, I could't understand why we absolutely need to define the command line string in the common part of the code, leading to being obliged to use macros in order to allow the architecture to specify in which section it wants the string. Why not leave the definition of the string to the architecture and just declare it in the common code, allowing the architecture to put it where it suits it and reducing opacity and allowing use of standard static inline functions instead of uggly macros. > >> I proposed an alternative at >> https://patchwork.ozlabs.org/project/linuxppc-dev/cover/cover.1554195798.git.christophe.leroy@c-s.fr/ but >> never got any feedback. > > Didn't go to a list I subscribe to. In particular, if it had gone to > DT list and into PW you would have gotten a reply from me. > Sorry for that. Original series from Daniel (https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20190319232448.45964-2-danielwa@cisco.com/) was sent only to linuxppc-dev list, and Michael suggested to also send it to linux-arch list, and I also always copy linux-kernel. If there is new interest for that functionnality, I can try and rebase my series. Christophe _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" 2021-03-01 17:26 ` Rob Herring 2021-03-01 17:45 ` Christophe Leroy @ 2021-03-02 17:12 ` Daniel Walker 1 sibling, 0 replies; 13+ messages in thread From: Daniel Walker @ 2021-03-02 17:12 UTC (permalink / raw) To: Rob Herring Cc: Will Deacon, Chris Packham, linux-kernel, Max Uvarov, Ard Biesheuvel, Marc Zyngier, Doug Anderson, Tyler Hicks, Frank Rowand, Arnd Bergmann, Palmer Dabbelt, Greg Kroah-Hartman, Catalin Marinas, Android Kernel Team, linux-arm-kernel, devicetree, linuxppc-dev, Michael Ellerman On Mon, Mar 01, 2021 at 11:26:14AM -0600, Rob Herring wrote: > +PPC folks and Daniel W > > On Mon, Mar 1, 2021 at 8:42 AM Will Deacon <will@kernel.org> wrote: > > > > On Mon, Mar 01, 2021 at 08:19:32AM -0600, Rob Herring wrote: > > > On Thu, Feb 25, 2021 at 6:59 AM Will Deacon <will@kernel.org> wrote: > > > > We recently [1] enabled support for CMDLINE_EXTEND on arm64, however > > > > when I started looking at replacing Android's out-of-tree implementation [2] > > > > > > Did anyone go read the common, reworked version of all this I > > > referenced that supports prepend and append. Here it is again[1]. > > > Maybe I should have been more assertive there and said 'extend' is > > > ambiguous. > > > > I tried reading that, but (a) most of the series is not in the mailing list > > archives and (b) the patch that _is_ doesn't touch CMDLINE_EXTEND at all. > > Right now the code in mainline does the opposite of what it's documented to > > do. > > Actually, there is a newer version I found: > > https://lore.kernel.org/linuxppc-dev/1551469472-53043-1-git-send-email-danielwa@cisco.com/ > https://lore.kernel.org/linuxppc-dev/1551469472-53043-2-git-send-email-danielwa@cisco.com/ > https://lore.kernel.org/linuxppc-dev/1551469472-53043-3-git-send-email-danielwa@cisco.com/ > > (Once again, there's some weird threading going on) > I'm happy to work with anyone to resubmit the changes. We currently use the changes in Cisco, and we have used them for many years. I was planning to update and resubmit since someone has recently inquired about why it wasn't upstream. Daniel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-03-03 21:26 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-25 12:59 [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" Will Deacon 2021-02-25 12:59 ` [PATCH 1/2] arm64: cpufeatures: Fix handling of CONFIG_CMDLINE for idreg overrides Will Deacon 2021-02-25 13:53 ` Marc Zyngier 2021-02-25 14:04 ` Will Deacon 2021-02-25 12:59 ` [PATCH 2/2] of/fdt: Append bootloader arguments when CMDLINE_EXTEND=y Will Deacon 2021-02-25 14:08 ` Marc Zyngier 2021-03-01 14:19 ` [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" Rob Herring 2021-03-01 14:41 ` Will Deacon 2021-03-01 17:26 ` Rob Herring 2021-03-01 17:45 ` Christophe Leroy 2021-03-02 14:56 ` Rob Herring 2021-03-02 15:16 ` Christophe Leroy 2021-03-02 17:12 ` Daniel Walker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).