linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND
@ 2020-09-21 19:15 Tyler Hicks
  2020-09-21 19:15 ` [PATCH 1/2] arm64: kaslr: Refactor early init command line parsing Tyler Hicks
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Tyler Hicks @ 2020-09-21 19:15 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: Pavel Tatashin, Rob Herring, linux-arm-kernel, linux-kernel

Provide the CONFIG_CMDLINE_EXTEND config option for arm64 kernels. This
config option can be used to extend the kernel command line parameters,
specified by the bootloader, with additional command line parameters
specified in the kernel configuration.

This option addresses the following use cases:

1) Switching between stable and development kernel versions, where one
   of the versions benefits from additional command line parameters,
   such as debugging options.
2) Specifying additional command line parameters, for additional tuning
   or debugging, when the bootloader does not offer an interactive mode.

After implementing these patches, I noticed that a previous attempt has
been made to upstream CONFIG_CMDLINE_EXTEND support in arm64:

 https://lore.kernel.org/linux-arm-kernel/1447838885-9888-1-git-send-email-p.fedin@samsung.com/

I don't believe that the previous objection still holds as the generic
command line parsing series hasn't been revised in over a year.

This series is based on commit f322010a08da ("Merge branch
'for-next/mte' into for-next/core") of the for-next/core branch of the
arm64 tree.

Below is a summary of testing that I performed.

Upgrade testing:

* CONFIG_CMDLINE unset
  - oldconfig target doesn't prompt, 
* CONFIG_CMDLINE set, CONFIG_CMDLINE_FORCE unset
  - oldconfig target prompts for command line type with default choice
    set to CONFIG_CMDLINE_FROM_BOOTLOADER
* CONFIG_CMDLINE set, CONFIG_CMDLINE_FORCE set
  - oldconfig target prompts for command line type with default choice
    set to CONFIG_CMDLINE_FORCE

Functional testing:

* Set CONFIG_CMDLINE="nokaslr apparmor=0" and CONFIG_CMDLINE_EXTEND=y to
  test early init parsing and regular parsing
  - /proc/cmdline shows that "nokaslr apparmor=0" was appended to the
    end of the bootloader supplied command line
  - "KASLR disabled on command line" found in dmesg
  - AppArmor is disabled. /sys/kernel/security/apparmor does not exist
    and aa-status prints:
     apparmor module is loaded.
     apparmor filesystem is not mounted.
* Set CONFIG_CMDLINE="nokaslr apparmor=0",
  CONFIG_CMDLINE_FROM_BOOTLOADER=y, and have the bootloader specify a
  command line without those options
  - The bootloader's command line is used and does not contain
    CONFIG_CMDLINE's value
  - AppArmor and KASLR are enabled
* Set CONFIG_CMDLINE="nokaslr apparmor=0" and CONFIG_CMDLINE_FORCE=y
  - The CONFIG_CMDLINE value is used and does not contain the
    bootloader's command line
  - AppArmor and KASLR are disabled

Tyler

Tyler Hicks (2):
  arm64: kaslr: Refactor early init command line parsing
  arm64: Extend the kernel command line from the bootloader

 arch/arm64/Kconfig        | 23 ++++++++++++++++++++++-
 arch/arm64/kernel/kaslr.c | 26 ++++++++++++++++++--------
 2 files changed, 40 insertions(+), 9 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] arm64: kaslr: Refactor early init command line parsing
  2020-09-21 19:15 [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND Tyler Hicks
@ 2020-09-21 19:15 ` Tyler Hicks
  2020-09-21 19:15 ` [PATCH 2/2] arm64: Extend the kernel command line from the bootloader Tyler Hicks
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Tyler Hicks @ 2020-09-21 19:15 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: Pavel Tatashin, Rob Herring, linux-arm-kernel, linux-kernel

Don't ask for *the* command line string to search for "nokaslr" in
kaslr_early_init(). Instead, tell a helper function to search all the
appropriate command line strings for "nokaslr" and return the result.

This paves the way for searching multiple command line strings without
having to concatenate the strings in early init.

Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
---
 arch/arm64/kernel/kaslr.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
index b181e0544b79..4c779a67c2a6 100644
--- a/arch/arm64/kernel/kaslr.c
+++ b/arch/arm64/kernel/kaslr.c
@@ -50,10 +50,16 @@ static __init u64 get_kaslr_seed(void *fdt)
 	return ret;
 }
 
-static __init const u8 *kaslr_get_cmdline(void *fdt)
+static __init bool cmdline_contains_nokaslr(const u8 *cmdline)
 {
-	static __initconst const u8 default_cmdline[] = CONFIG_CMDLINE;
+	const u8 *str;
 
+	str = strstr(cmdline, "nokaslr");
+	return str == cmdline || (str > cmdline && *(str - 1) == ' ');
+}
+
+static __init bool is_kaslr_disabled_cmdline(void *fdt)
+{
 	if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
 		int node;
 		const u8 *prop;
@@ -65,10 +71,10 @@ static __init const u8 *kaslr_get_cmdline(void *fdt)
 		prop = fdt_getprop(fdt, node, "bootargs", NULL);
 		if (!prop)
 			goto out;
-		return prop;
+		return cmdline_contains_nokaslr(prop);
 	}
 out:
-	return default_cmdline;
+	return cmdline_contains_nokaslr(CONFIG_CMDLINE);
 }
 
 /*
@@ -83,7 +89,6 @@ u64 __init kaslr_early_init(u64 dt_phys)
 {
 	void *fdt;
 	u64 seed, offset, mask, module_range;
-	const u8 *cmdline, *str;
 	unsigned long raw;
 	int size;
 
@@ -115,9 +120,7 @@ u64 __init kaslr_early_init(u64 dt_phys)
 	 * Check if 'nokaslr' appears on the command line, and
 	 * return 0 if that is the case.
 	 */
-	cmdline = kaslr_get_cmdline(fdt);
-	str = strstr(cmdline, "nokaslr");
-	if (str == cmdline || (str > cmdline && *(str - 1) == ' ')) {
+	if (is_kaslr_disabled_cmdline(fdt)) {
 		kaslr_status = KASLR_DISABLED_CMDLINE;
 		return 0;
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] arm64: Extend the kernel command line from the bootloader
  2020-09-21 19:15 [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND Tyler Hicks
  2020-09-21 19:15 ` [PATCH 1/2] arm64: kaslr: Refactor early init command line parsing Tyler Hicks
@ 2020-09-21 19:15 ` Tyler Hicks
  2020-11-03 15:59 ` [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND Tyler Hicks
  2020-11-27 19:12 ` Catalin Marinas
  3 siblings, 0 replies; 12+ messages in thread
From: Tyler Hicks @ 2020-09-21 19:15 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: Pavel Tatashin, Rob Herring, linux-arm-kernel, linux-kernel

Provide support for additional kernel command line parameters to be
concatenated onto the end of the command line provided by the
bootloader. Additional parameters are specified in the CONFIG_CMDLINE
option when CONFIG_CMDLINE_EXTEND is selected, matching other
architectures and leveraging existing support in the FDT and EFI stub
code.

Special care must be taken for the arch-specific nokaslr parsing. Search
the bootargs FDT property and the CONFIG_CMDLINE when
CONFIG_CMDLINE_EXTEND is in use.

There are a couple of known use cases for this feature:

1) Switching between stable and development kernel versions, where one
   of the versions benefits from additional command line parameters,
   such as debugging options.
2) Specifying additional command line parameters, for additional tuning
   or debugging, when the bootloader does not offer an interactive mode.

Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
---
 arch/arm64/Kconfig        | 23 ++++++++++++++++++++++-
 arch/arm64/kernel/kaslr.c |  9 ++++++++-
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index e5eb6a69b1e3..466df3415fff 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1864,15 +1864,36 @@ config CMDLINE
 	  entering them here. As a minimum, you should specify the the
 	  root device (e.g. root=/dev/nfs).
 
+choice
+	prompt "Kernel command line type" if CMDLINE != ""
+	default CMDLINE_FROM_BOOTLOADER
+	help
+	  Choose how the kernel will handle the provided default kernel
+	  command line string.
+
+config CMDLINE_FROM_BOOTLOADER
+	bool "Use bootloader kernel arguments if available"
+	help
+	  Uses the command-line options passed by the boot loader. If
+	  the boot loader doesn't provide any, the default kernel command
+	  string provided in CMDLINE will be used.
+
+config CMDLINE_EXTEND
+	bool "Extend bootloader kernel arguments"
+	help
+	  The command-line arguments provided by the boot loader will be
+	  appended to the default kernel command string.
+
 config CMDLINE_FORCE
 	bool "Always use the default kernel command string"
-	depends on CMDLINE != ""
 	help
 	  Always use the default kernel command string, even if the boot
 	  loader passes other arguments to the kernel.
 	  This is useful if you cannot or don't want to change the
 	  command-line options your boot loader passes to the kernel.
 
+endchoice
+
 config EFI_STUB
 	bool
 
diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
index 4c779a67c2a6..0921aa1520b0 100644
--- a/arch/arm64/kernel/kaslr.c
+++ b/arch/arm64/kernel/kaslr.c
@@ -71,7 +71,14 @@ static __init bool is_kaslr_disabled_cmdline(void *fdt)
 		prop = fdt_getprop(fdt, node, "bootargs", NULL);
 		if (!prop)
 			goto out;
-		return cmdline_contains_nokaslr(prop);
+
+		if (cmdline_contains_nokaslr(prop))
+			return true;
+
+		if (IS_ENABLED(CONFIG_CMDLINE_EXTEND))
+			goto out;
+
+		return false;
 	}
 out:
 	return cmdline_contains_nokaslr(CONFIG_CMDLINE);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND
  2020-09-21 19:15 [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND Tyler Hicks
  2020-09-21 19:15 ` [PATCH 1/2] arm64: kaslr: Refactor early init command line parsing Tyler Hicks
  2020-09-21 19:15 ` [PATCH 2/2] arm64: Extend the kernel command line from the bootloader Tyler Hicks
@ 2020-11-03 15:59 ` Tyler Hicks
  2020-11-04 12:08   ` Will Deacon
  2020-11-27 19:12 ` Catalin Marinas
  3 siblings, 1 reply; 12+ messages in thread
From: Tyler Hicks @ 2020-11-03 15:59 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: Pavel Tatashin, Rob Herring, linux-arm-kernel, linux-kernel

On 2020-09-21 14:15:55, Tyler Hicks wrote:
> Provide the CONFIG_CMDLINE_EXTEND config option for arm64 kernels. This
> config option can be used to extend the kernel command line parameters,
> specified by the bootloader, with additional command line parameters
> specified in the kernel configuration.

Hi Catalin and Will - Friendly ping on this series now that we're
on the other side of the 5.10 merge window. I hope it can be considered
for 5.10+1. Let me know if I need to rebase/resubmit. Thanks!

Tyler

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND
  2020-11-03 15:59 ` [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND Tyler Hicks
@ 2020-11-04 12:08   ` Will Deacon
  2020-11-05  5:40     ` Tyler Hicks
  0 siblings, 1 reply; 12+ messages in thread
From: Will Deacon @ 2020-11-04 12:08 UTC (permalink / raw)
  To: Tyler Hicks
  Cc: Catalin Marinas, Pavel Tatashin, Rob Herring, linux-arm-kernel,
	linux-kernel

On Tue, Nov 03, 2020 at 09:59:52AM -0600, Tyler Hicks wrote:
> On 2020-09-21 14:15:55, Tyler Hicks wrote:
> > Provide the CONFIG_CMDLINE_EXTEND config option for arm64 kernels. This
> > config option can be used to extend the kernel command line parameters,
> > specified by the bootloader, with additional command line parameters
> > specified in the kernel configuration.
> 
> Hi Catalin and Will - Friendly ping on this series now that we're
> on the other side of the 5.10 merge window. I hope it can be considered
> for 5.10+1. Let me know if I need to rebase/resubmit. Thanks!

Can you use bootconfig to achieve what you need?

Will

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND
  2020-11-04 12:08   ` Will Deacon
@ 2020-11-05  5:40     ` Tyler Hicks
  2020-11-05  9:58       ` Will Deacon
  0 siblings, 1 reply; 12+ messages in thread
From: Tyler Hicks @ 2020-11-05  5:40 UTC (permalink / raw)
  To: Will Deacon
  Cc: Catalin Marinas, Pavel Tatashin, Rob Herring, linux-arm-kernel,
	linux-kernel

On 2020-11-04 12:08:12, Will Deacon wrote:
> On Tue, Nov 03, 2020 at 09:59:52AM -0600, Tyler Hicks wrote:
> > On 2020-09-21 14:15:55, Tyler Hicks wrote:
> > > Provide the CONFIG_CMDLINE_EXTEND config option for arm64 kernels. This
> > > config option can be used to extend the kernel command line parameters,
> > > specified by the bootloader, with additional command line parameters
> > > specified in the kernel configuration.
> > 
> > Hi Catalin and Will - Friendly ping on this series now that we're
> > on the other side of the 5.10 merge window. I hope it can be considered
> > for 5.10+1. Let me know if I need to rebase/resubmit. Thanks!
> 
> Can you use bootconfig to achieve what you need?

Thanks for mentioning bootconfig. I hadn't considered it.

After reading the docs and code, I see a few reasons why I can't use it
out of the box:

 1) It requires "bootconfig" to be appended to the kernel command line.
    My proposed patch series makes it possible to append new options to
    the kernel command line in situations where the bootloader is not
    interactive. This presents a circular dependency problem for my use
    case.

    A new config option could be added to force the enablement of
    bootconfig but that would sort of be a single-use duplicate of
    CONFIG_CMDLINE_EXTEND's functionality.

 2) Not all kernel command line options can be configured using
    bootconfig. For example, the "nokaslr" and "crashkernel=" parameters
    are parsed/handled before setup_boot_config() is called. KASLR can
    be disabled via a kernel config change but there's no config option
    equivalent for "crashkernel=". Changing the "crashkernel=" command
    line option is something that I need to support because a
    development/debug kernel build often requires a larger reservation
    and we find ourselves adjusting the "crashkernel=" value fairly
    often.

 3) External FIT image build systems do not yet support bootconfig since
    it is so new. It is completely fair if you file this away in your
    not-my-problem folder but simple kernel config modifications, as
    needed for CONFIG_CMDLINE_EXTEND, are something that every image
    build system is likely to support today.

All that said, I do really like the look of bootconfig. Unfortunately,
it doesn't let me achieve everything I need.

Tyler

> 
> Will
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND
  2020-11-05  5:40     ` Tyler Hicks
@ 2020-11-05  9:58       ` Will Deacon
  2020-11-05 15:28         ` Tyler Hicks
  0 siblings, 1 reply; 12+ messages in thread
From: Will Deacon @ 2020-11-05  9:58 UTC (permalink / raw)
  To: Tyler Hicks
  Cc: Catalin Marinas, Pavel Tatashin, Rob Herring, linux-arm-kernel,
	linux-kernel

On Wed, Nov 04, 2020 at 11:40:09PM -0600, Tyler Hicks wrote:
> On 2020-11-04 12:08:12, Will Deacon wrote:
> > On Tue, Nov 03, 2020 at 09:59:52AM -0600, Tyler Hicks wrote:
> > > On 2020-09-21 14:15:55, Tyler Hicks wrote:
> > > > Provide the CONFIG_CMDLINE_EXTEND config option for arm64 kernels. This
> > > > config option can be used to extend the kernel command line parameters,
> > > > specified by the bootloader, with additional command line parameters
> > > > specified in the kernel configuration.
> > > 
> > > Hi Catalin and Will - Friendly ping on this series now that we're
> > > on the other side of the 5.10 merge window. I hope it can be considered
> > > for 5.10+1. Let me know if I need to rebase/resubmit. Thanks!
> > 
> > Can you use bootconfig to achieve what you need?
> 
> Thanks for mentioning bootconfig. I hadn't considered it.
> 
> After reading the docs and code, I see a few reasons why I can't use it
> out of the box:
> 
>  1) It requires "bootconfig" to be appended to the kernel command line.
>     My proposed patch series makes it possible to append new options to
>     the kernel command line in situations where the bootloader is not
>     interactive. This presents a circular dependency problem for my use
>     case.
> 
>     A new config option could be added to force the enablement of
>     bootconfig but that would sort of be a single-use duplicate of
>     CONFIG_CMDLINE_EXTEND's functionality.
> 
>  2) Not all kernel command line options can be configured using
>     bootconfig. For example, the "nokaslr" and "crashkernel=" parameters
>     are parsed/handled before setup_boot_config() is called. KASLR can
>     be disabled via a kernel config change but there's no config option
>     equivalent for "crashkernel=". Changing the "crashkernel=" command
>     line option is something that I need to support because a
>     development/debug kernel build often requires a larger reservation
>     and we find ourselves adjusting the "crashkernel=" value fairly
>     often.
> 
>  3) External FIT image build systems do not yet support bootconfig since
>     it is so new. It is completely fair if you file this away in your
>     not-my-problem folder but simple kernel config modifications, as
>     needed for CONFIG_CMDLINE_EXTEND, are something that every image
>     build system is likely to support today.
> 
> All that said, I do really like the look of bootconfig. Unfortunately,
> it doesn't let me achieve everything I need.

Ok, well thanks for having a look. A follow-up question I have is how is
this handled on x86? They don't appear to have CONFIG_CMDLINE_EXTEND either
afaict. Is it because their bootloader story tends to be more uniform?

Will

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND
  2020-11-05  9:58       ` Will Deacon
@ 2020-11-05 15:28         ` Tyler Hicks
  2020-11-19 16:56           ` Tyler Hicks
  2020-11-20 13:50           ` Rob Herring
  0 siblings, 2 replies; 12+ messages in thread
From: Tyler Hicks @ 2020-11-05 15:28 UTC (permalink / raw)
  To: Will Deacon
  Cc: Catalin Marinas, Pavel Tatashin, Rob Herring, linux-arm-kernel,
	linux-kernel

On 2020-11-05 09:58:54, Will Deacon wrote:
> On Wed, Nov 04, 2020 at 11:40:09PM -0600, Tyler Hicks wrote:
> > On 2020-11-04 12:08:12, Will Deacon wrote:
> > > On Tue, Nov 03, 2020 at 09:59:52AM -0600, Tyler Hicks wrote:
> > > > On 2020-09-21 14:15:55, Tyler Hicks wrote:
> > > > > Provide the CONFIG_CMDLINE_EXTEND config option for arm64 kernels. This
> > > > > config option can be used to extend the kernel command line parameters,
> > > > > specified by the bootloader, with additional command line parameters
> > > > > specified in the kernel configuration.
> > > > 
> > > > Hi Catalin and Will - Friendly ping on this series now that we're
> > > > on the other side of the 5.10 merge window. I hope it can be considered
> > > > for 5.10+1. Let me know if I need to rebase/resubmit. Thanks!
> > > 
> > > Can you use bootconfig to achieve what you need?
> > 
> > Thanks for mentioning bootconfig. I hadn't considered it.
> > 
> > After reading the docs and code, I see a few reasons why I can't use it
> > out of the box:
> > 
> >  1) It requires "bootconfig" to be appended to the kernel command line.
> >     My proposed patch series makes it possible to append new options to
> >     the kernel command line in situations where the bootloader is not
> >     interactive. This presents a circular dependency problem for my use
> >     case.
> > 
> >     A new config option could be added to force the enablement of
> >     bootconfig but that would sort of be a single-use duplicate of
> >     CONFIG_CMDLINE_EXTEND's functionality.
> > 
> >  2) Not all kernel command line options can be configured using
> >     bootconfig. For example, the "nokaslr" and "crashkernel=" parameters
> >     are parsed/handled before setup_boot_config() is called. KASLR can
> >     be disabled via a kernel config change but there's no config option
> >     equivalent for "crashkernel=". Changing the "crashkernel=" command
> >     line option is something that I need to support because a
> >     development/debug kernel build often requires a larger reservation
> >     and we find ourselves adjusting the "crashkernel=" value fairly
> >     often.
> > 
> >  3) External FIT image build systems do not yet support bootconfig since
> >     it is so new. It is completely fair if you file this away in your
> >     not-my-problem folder but simple kernel config modifications, as
> >     needed for CONFIG_CMDLINE_EXTEND, are something that every image
> >     build system is likely to support today.
> > 
> > All that said, I do really like the look of bootconfig. Unfortunately,
> > it doesn't let me achieve everything I need.
> 
> Ok, well thanks for having a look. A follow-up question I have is how is
> this handled on x86? They don't appear to have CONFIG_CMDLINE_EXTEND either
> afaict. Is it because their bootloader story tends to be more uniform?

x86's equivalent was implemented by commit 516cbf3730c4 ("x86, bootup:
add built-in kernel command line for x86 (v2)"). To summarize, you have
to enable CONFIG_CMDLINE_BOOL and then that lets you define built-in
command line parameters in CONFIG_CMDLINE. However, it is backwards in
that the command line provided by the bootloader is appended onto the
end of CONFIG_CMDLINE.

This doesn't seem as useful to me because, using the crashkernel=
example from above, the bootloader provided crashkernel= value may need
to be overridden by the built-in command line to provide a different
crashkernel= value for the particular kernel build being booted. Most
kernel command line parameter parsers are implemented in a way that
supports multiple instances of the parameter while only honoring the
last instance.

Tyler

> 
> Will
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND
  2020-11-05 15:28         ` Tyler Hicks
@ 2020-11-19 16:56           ` Tyler Hicks
  2020-11-19 19:25             ` Will Deacon
  2020-11-20 13:50           ` Rob Herring
  1 sibling, 1 reply; 12+ messages in thread
From: Tyler Hicks @ 2020-11-19 16:56 UTC (permalink / raw)
  To: Will Deacon
  Cc: Catalin Marinas, Pavel Tatashin, Rob Herring, linux-arm-kernel,
	linux-kernel

On 2020-11-05 09:28:38, Tyler Hicks wrote:
> On 2020-11-05 09:58:54, Will Deacon wrote:
> > On Wed, Nov 04, 2020 at 11:40:09PM -0600, Tyler Hicks wrote:
> > > On 2020-11-04 12:08:12, Will Deacon wrote:
> > > > On Tue, Nov 03, 2020 at 09:59:52AM -0600, Tyler Hicks wrote:
> > > > > On 2020-09-21 14:15:55, Tyler Hicks wrote:
> > > > > > Provide the CONFIG_CMDLINE_EXTEND config option for arm64 kernels. This
> > > > > > config option can be used to extend the kernel command line parameters,
> > > > > > specified by the bootloader, with additional command line parameters
> > > > > > specified in the kernel configuration.
> > > > > 
> > > > > Hi Catalin and Will - Friendly ping on this series now that we're
> > > > > on the other side of the 5.10 merge window. I hope it can be considered
> > > > > for 5.10+1. Let me know if I need to rebase/resubmit. Thanks!
> > > > 
> > > > Can you use bootconfig to achieve what you need?
> > > 
> > > Thanks for mentioning bootconfig. I hadn't considered it.
> > > 
> > > After reading the docs and code, I see a few reasons why I can't use it
> > > out of the box:
> > > 
> > >  1) It requires "bootconfig" to be appended to the kernel command line.
> > >     My proposed patch series makes it possible to append new options to
> > >     the kernel command line in situations where the bootloader is not
> > >     interactive. This presents a circular dependency problem for my use
> > >     case.
> > > 
> > >     A new config option could be added to force the enablement of
> > >     bootconfig but that would sort of be a single-use duplicate of
> > >     CONFIG_CMDLINE_EXTEND's functionality.
> > > 
> > >  2) Not all kernel command line options can be configured using
> > >     bootconfig. For example, the "nokaslr" and "crashkernel=" parameters
> > >     are parsed/handled before setup_boot_config() is called. KASLR can
> > >     be disabled via a kernel config change but there's no config option
> > >     equivalent for "crashkernel=". Changing the "crashkernel=" command
> > >     line option is something that I need to support because a
> > >     development/debug kernel build often requires a larger reservation
> > >     and we find ourselves adjusting the "crashkernel=" value fairly
> > >     often.
> > > 
> > >  3) External FIT image build systems do not yet support bootconfig since
> > >     it is so new. It is completely fair if you file this away in your
> > >     not-my-problem folder but simple kernel config modifications, as
> > >     needed for CONFIG_CMDLINE_EXTEND, are something that every image
> > >     build system is likely to support today.
> > > 
> > > All that said, I do really like the look of bootconfig. Unfortunately,
> > > it doesn't let me achieve everything I need.
> > 
> > Ok, well thanks for having a look. A follow-up question I have is how is
> > this handled on x86? They don't appear to have CONFIG_CMDLINE_EXTEND either
> > afaict. Is it because their bootloader story tends to be more uniform?
> 
> x86's equivalent was implemented by commit 516cbf3730c4 ("x86, bootup:
> add built-in kernel command line for x86 (v2)"). To summarize, you have
> to enable CONFIG_CMDLINE_BOOL and then that lets you define built-in
> command line parameters in CONFIG_CMDLINE. However, it is backwards in
> that the command line provided by the bootloader is appended onto the
> end of CONFIG_CMDLINE.
> 
> This doesn't seem as useful to me because, using the crashkernel=
> example from above, the bootloader provided crashkernel= value may need
> to be overridden by the built-in command line to provide a different
> crashkernel= value for the particular kernel build being booted. Most
> kernel command line parameter parsers are implemented in a way that
> supports multiple instances of the parameter while only honoring the
> last instance.

Hey Will - Do you any additional concerns that I should look into?

Thanks!

Tyler

> 
> Tyler
> 
> > 
> > Will
> > 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND
  2020-11-19 16:56           ` Tyler Hicks
@ 2020-11-19 19:25             ` Will Deacon
  0 siblings, 0 replies; 12+ messages in thread
From: Will Deacon @ 2020-11-19 19:25 UTC (permalink / raw)
  To: Tyler Hicks
  Cc: Catalin Marinas, Pavel Tatashin, Rob Herring, linux-arm-kernel,
	linux-kernel

On Thu, Nov 19, 2020 at 10:56:12AM -0600, Tyler Hicks wrote:
> On 2020-11-05 09:28:38, Tyler Hicks wrote:
> > On 2020-11-05 09:58:54, Will Deacon wrote:
> > > On Wed, Nov 04, 2020 at 11:40:09PM -0600, Tyler Hicks wrote:
> > > > On 2020-11-04 12:08:12, Will Deacon wrote:
> > > > > On Tue, Nov 03, 2020 at 09:59:52AM -0600, Tyler Hicks wrote:
> > > > > > On 2020-09-21 14:15:55, Tyler Hicks wrote:
> > > > > > > Provide the CONFIG_CMDLINE_EXTEND config option for arm64 kernels. This
> > > > > > > config option can be used to extend the kernel command line parameters,
> > > > > > > specified by the bootloader, with additional command line parameters
> > > > > > > specified in the kernel configuration.
> > > > > > 
> > > > > > Hi Catalin and Will - Friendly ping on this series now that we're
> > > > > > on the other side of the 5.10 merge window. I hope it can be considered
> > > > > > for 5.10+1. Let me know if I need to rebase/resubmit. Thanks!
> > > > > 
> > > > > Can you use bootconfig to achieve what you need?
> > > > 
> > > > Thanks for mentioning bootconfig. I hadn't considered it.
> > > > 
> > > > After reading the docs and code, I see a few reasons why I can't use it
> > > > out of the box:
> > > > 
> > > >  1) It requires "bootconfig" to be appended to the kernel command line.
> > > >     My proposed patch series makes it possible to append new options to
> > > >     the kernel command line in situations where the bootloader is not
> > > >     interactive. This presents a circular dependency problem for my use
> > > >     case.
> > > > 
> > > >     A new config option could be added to force the enablement of
> > > >     bootconfig but that would sort of be a single-use duplicate of
> > > >     CONFIG_CMDLINE_EXTEND's functionality.
> > > > 
> > > >  2) Not all kernel command line options can be configured using
> > > >     bootconfig. For example, the "nokaslr" and "crashkernel=" parameters
> > > >     are parsed/handled before setup_boot_config() is called. KASLR can
> > > >     be disabled via a kernel config change but there's no config option
> > > >     equivalent for "crashkernel=". Changing the "crashkernel=" command
> > > >     line option is something that I need to support because a
> > > >     development/debug kernel build often requires a larger reservation
> > > >     and we find ourselves adjusting the "crashkernel=" value fairly
> > > >     often.
> > > > 
> > > >  3) External FIT image build systems do not yet support bootconfig since
> > > >     it is so new. It is completely fair if you file this away in your
> > > >     not-my-problem folder but simple kernel config modifications, as
> > > >     needed for CONFIG_CMDLINE_EXTEND, are something that every image
> > > >     build system is likely to support today.
> > > > 
> > > > All that said, I do really like the look of bootconfig. Unfortunately,
> > > > it doesn't let me achieve everything I need.
> > > 
> > > Ok, well thanks for having a look. A follow-up question I have is how is
> > > this handled on x86? They don't appear to have CONFIG_CMDLINE_EXTEND either
> > > afaict. Is it because their bootloader story tends to be more uniform?
> > 
> > x86's equivalent was implemented by commit 516cbf3730c4 ("x86, bootup:
> > add built-in kernel command line for x86 (v2)"). To summarize, you have
> > to enable CONFIG_CMDLINE_BOOL and then that lets you define built-in
> > command line parameters in CONFIG_CMDLINE. However, it is backwards in
> > that the command line provided by the bootloader is appended onto the
> > end of CONFIG_CMDLINE.
> > 
> > This doesn't seem as useful to me because, using the crashkernel=
> > example from above, the bootloader provided crashkernel= value may need
> > to be overridden by the built-in command line to provide a different
> > crashkernel= value for the particular kernel build being booted. Most
> > kernel command line parameter parsers are implemented in a way that
> > supports multiple instances of the parameter while only honoring the
> > last instance.
> 
> Hey Will - Do you any additional concerns that I should look into?

No, you convinced me it's useful. I just haven't found time to look at the
implementation yet!

Will

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND
  2020-11-05 15:28         ` Tyler Hicks
  2020-11-19 16:56           ` Tyler Hicks
@ 2020-11-20 13:50           ` Rob Herring
  1 sibling, 0 replies; 12+ messages in thread
From: Rob Herring @ 2020-11-20 13:50 UTC (permalink / raw)
  To: Tyler Hicks
  Cc: Will Deacon, Catalin Marinas, Pavel Tatashin, linux-arm-kernel,
	linux-kernel

On Thu, Nov 5, 2020 at 9:28 AM Tyler Hicks <tyhicks@linux.microsoft.com> wrote:
>
> On 2020-11-05 09:58:54, Will Deacon wrote:
> > On Wed, Nov 04, 2020 at 11:40:09PM -0600, Tyler Hicks wrote:
> > > On 2020-11-04 12:08:12, Will Deacon wrote:
> > > > On Tue, Nov 03, 2020 at 09:59:52AM -0600, Tyler Hicks wrote:
> > > > > On 2020-09-21 14:15:55, Tyler Hicks wrote:
> > > > > > Provide the CONFIG_CMDLINE_EXTEND config option for arm64 kernels. This
> > > > > > config option can be used to extend the kernel command line parameters,
> > > > > > specified by the bootloader, with additional command line parameters
> > > > > > specified in the kernel configuration.
> > > > >
> > > > > Hi Catalin and Will - Friendly ping on this series now that we're
> > > > > on the other side of the 5.10 merge window. I hope it can be considered
> > > > > for 5.10+1. Let me know if I need to rebase/resubmit. Thanks!
> > > >
> > > > Can you use bootconfig to achieve what you need?
> > >
> > > Thanks for mentioning bootconfig. I hadn't considered it.
> > >
> > > After reading the docs and code, I see a few reasons why I can't use it
> > > out of the box:
> > >
> > >  1) It requires "bootconfig" to be appended to the kernel command line.
> > >     My proposed patch series makes it possible to append new options to
> > >     the kernel command line in situations where the bootloader is not
> > >     interactive. This presents a circular dependency problem for my use
> > >     case.
> > >
> > >     A new config option could be added to force the enablement of
> > >     bootconfig but that would sort of be a single-use duplicate of
> > >     CONFIG_CMDLINE_EXTEND's functionality.
> > >
> > >  2) Not all kernel command line options can be configured using
> > >     bootconfig. For example, the "nokaslr" and "crashkernel=" parameters
> > >     are parsed/handled before setup_boot_config() is called. KASLR can
> > >     be disabled via a kernel config change but there's no config option
> > >     equivalent for "crashkernel=". Changing the "crashkernel=" command
> > >     line option is something that I need to support because a
> > >     development/debug kernel build often requires a larger reservation
> > >     and we find ourselves adjusting the "crashkernel=" value fairly
> > >     often.
> > >
> > >  3) External FIT image build systems do not yet support bootconfig since
> > >     it is so new. It is completely fair if you file this away in your
> > >     not-my-problem folder but simple kernel config modifications, as
> > >     needed for CONFIG_CMDLINE_EXTEND, are something that every image
> > >     build system is likely to support today.
> > >
> > > All that said, I do really like the look of bootconfig. Unfortunately,
> > > it doesn't let me achieve everything I need.
> >
> > Ok, well thanks for having a look. A follow-up question I have is how is
> > this handled on x86? They don't appear to have CONFIG_CMDLINE_EXTEND either
> > afaict. Is it because their bootloader story tends to be more uniform?
>
> x86's equivalent was implemented by commit 516cbf3730c4 ("x86, bootup:
> add built-in kernel command line for x86 (v2)"). To summarize, you have
> to enable CONFIG_CMDLINE_BOOL and then that lets you define built-in
> command line parameters in CONFIG_CMDLINE. However, it is backwards in
> that the command line provided by the bootloader is appended onto the
> end of CONFIG_CMDLINE.
>
> This doesn't seem as useful to me because, using the crashkernel=
> example from above, the bootloader provided crashkernel= value may need
> to be overridden by the built-in command line to provide a different
> crashkernel= value for the particular kernel build being booted. Most
> kernel command line parameter parsers are implemented in a way that
> supports multiple instances of the parameter while only honoring the
> last instance.

These config options should be common IMO. It's a minefield though.
You might want to look at this prior attempt[1].

Rob

[1] https://lore.kernel.org/lkml/20190319232448.45964-2-danielwa@cisco.com/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND
  2020-09-21 19:15 [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND Tyler Hicks
                   ` (2 preceding siblings ...)
  2020-11-03 15:59 ` [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND Tyler Hicks
@ 2020-11-27 19:12 ` Catalin Marinas
  3 siblings, 0 replies; 12+ messages in thread
From: Catalin Marinas @ 2020-11-27 19:12 UTC (permalink / raw)
  To: Will Deacon, Tyler Hicks
  Cc: linux-arm-kernel, Rob Herring, linux-kernel, Pavel Tatashin

On Mon, 21 Sep 2020 14:15:55 -0500, Tyler Hicks wrote:
> Provide the CONFIG_CMDLINE_EXTEND config option for arm64 kernels. This
> config option can be used to extend the kernel command line parameters,
> specified by the bootloader, with additional command line parameters
> specified in the kernel configuration.
> 
> This option addresses the following use cases:
> 
> [...]

Applied to arm64 (for-next/cmdline-extended), thanks!

[1/2] arm64: kaslr: Refactor early init command line parsing
      https://git.kernel.org/arm64/c/52ec03f75d59
[2/2] arm64: Extend the kernel command line from the bootloader
      https://git.kernel.org/arm64/c/1e40d105dae5

-- 
Catalin


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-11-27 19:27 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-21 19:15 [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND Tyler Hicks
2020-09-21 19:15 ` [PATCH 1/2] arm64: kaslr: Refactor early init command line parsing Tyler Hicks
2020-09-21 19:15 ` [PATCH 2/2] arm64: Extend the kernel command line from the bootloader Tyler Hicks
2020-11-03 15:59 ` [PATCH 0/2] arm64: Implement CONFIG_CMDLINE_EXTEND Tyler Hicks
2020-11-04 12:08   ` Will Deacon
2020-11-05  5:40     ` Tyler Hicks
2020-11-05  9:58       ` Will Deacon
2020-11-05 15:28         ` Tyler Hicks
2020-11-19 16:56           ` Tyler Hicks
2020-11-19 19:25             ` Will Deacon
2020-11-20 13:50           ` Rob Herring
2020-11-27 19:12 ` Catalin Marinas

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).