From: Will Deacon <will@kernel.org> To: Rob Herring <robh@kernel.org> Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Max Uvarov <muvarov@gmail.com>, Ard Biesheuvel <ardb@kernel.org>, Marc Zyngier <maz@kernel.org>, Doug Anderson <dianders@chromium.org>, Tyler Hicks <tyhicks@linux.microsoft.com>, Frank Rowand <frowand.list@gmail.com>, Arnd Bergmann <arnd@arndb.de>, Palmer Dabbelt <palmer@dabbelt.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Catalin Marinas <catalin.marinas@arm.com>, Android Kernel Team <kernel-team@android.com>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, devicetree@vger.kernel.org Subject: Re: [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" Date: Mon, 1 Mar 2021 14:41:54 +0000 [thread overview] Message-ID: <20210301144153.GA16716@willie-the-truck> (raw) In-Reply-To: <CAL_JsqJX=TCCs7=gg486r9TN4NYscMTCLNfqJF9crskKPq-bTg@mail.gmail.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will@kernel.org> To: Rob Herring <robh@kernel.org> Cc: devicetree@vger.kernel.org, Android Kernel Team <kernel-team@android.com>, Catalin Marinas <catalin.marinas@arm.com>, Arnd Bergmann <arnd@arndb.de>, Marc Zyngier <maz@kernel.org>, Doug Anderson <dianders@chromium.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Tyler Hicks <tyhicks@linux.microsoft.com>, Palmer Dabbelt <palmer@dabbelt.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Max Uvarov <muvarov@gmail.com>, Frank Rowand <frowand.list@gmail.com>, Ard Biesheuvel <ardb@kernel.org>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" Date: Mon, 1 Mar 2021 14:41:54 +0000 [thread overview] Message-ID: <20210301144153.GA16716@willie-the-truck> (raw) In-Reply-To: <CAL_JsqJX=TCCs7=gg486r9TN4NYscMTCLNfqJF9crskKPq-bTg@mail.gmail.com> 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
next prev parent reply other threads:[~2021-03-01 14:43 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 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 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 13:53 ` Marc Zyngier 2021-02-25 13:53 ` Marc Zyngier 2021-02-25 14:04 ` Will Deacon 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 12:59 ` Will Deacon 2021-02-25 14:08 ` Marc Zyngier 2021-02-25 14:08 ` Marc Zyngier 2021-03-03 19:56 ` kernel test robot 2021-03-01 14:19 ` [PATCH 0/2] Fix CMDLINE_EXTEND handling for FDT "bootargs" Rob Herring 2021-03-01 14:19 ` Rob Herring 2021-03-01 14:41 ` Will Deacon [this message] 2021-03-01 14:41 ` Will Deacon 2021-03-01 17:26 ` Rob Herring 2021-03-01 17:26 ` Rob Herring 2021-03-01 17:26 ` Rob Herring 2021-03-01 17:45 ` Christophe Leroy 2021-03-01 17:45 ` Christophe Leroy 2021-03-01 17:45 ` Christophe Leroy 2021-03-02 14:56 ` Rob Herring 2021-03-02 14:56 ` Rob Herring 2021-03-02 14:56 ` Rob Herring 2021-03-02 15:16 ` Christophe Leroy 2021-03-02 15:16 ` Christophe Leroy 2021-03-02 15:16 ` Christophe Leroy 2021-03-02 17:12 ` Daniel Walker 2021-03-02 17:12 ` Daniel Walker 2021-03-02 17:12 ` 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=20210301144153.GA16716@willie-the-truck \ --to=will@kernel.org \ --cc=ardb@kernel.org \ --cc=arnd@arndb.de \ --cc=catalin.marinas@arm.com \ --cc=devicetree@vger.kernel.org \ --cc=dianders@chromium.org \ --cc=frowand.list@gmail.com \ --cc=gregkh@linuxfoundation.org \ --cc=kernel-team@android.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=maz@kernel.org \ --cc=muvarov@gmail.com \ --cc=palmer@dabbelt.com \ --cc=robh@kernel.org \ --cc=tyhicks@linux.microsoft.com \ /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.