Messages in this thread | | | Date | Thu, 4 Mar 2021 09:33:07 +0000 | From | Will Deacon <> | Subject | Re: [PATCH v2 1/2] arm64: cpufeatures: Fix handling of CONFIG_CMDLINE for idreg overrides |
| |
On Wed, Mar 03, 2021 at 04:30:21PM -0600, Rob Herring wrote: > On Wed, Mar 3, 2021 at 7:50 AM 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. > > > > Reviewed-by: 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(-) > > It bothers me a bit having command line handling here. It means how > the command line is assembled in 2 places.
It's more than 2 places, it's also done in the EFI stub; see efi_pe_entry() in drivers/firmware/efi/libstub/efi-stub.c. That's why I'm pushing on this, because at the moment the command-line reported by /proc/cmdline doesn't match the command-line that was parsed there because they stitch it together in opposite orders when CMDLINE_EXTEND is used.
> I guess if we get rid of ambiguous 'extend' then it's better, but perhaps > a better implementation would be an api get a specific command line > parameter. The main downside would be searching the DT again for each > parameter if we can't store any data in between calls, but there's ways > around that. PowerPC also needs similar functionality in > disabled_on_cmdline().
Christophe's patches at least aim to put the assembling all in one place, although we'll have to see whether or not it can be used in the EFI stub environment.
Will
| |