All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: Support CMDLINE_EXTEND
@ 2023-03-20 21:14 ` Chris Packham
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Packham @ 2023-03-20 21:14 UTC (permalink / raw)
  To: catalin.marinas, will; +Cc: linux-arm-kernel, linux-kernel, Chris Packham

Support extending the bootloader provided command line for arm64
targets. This support is already present via generic DT/EFI code the
only thing required is for the architecture to make it selectable.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm64/Kconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1023e896d46b..3c837b085f21 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -2228,6 +2228,12 @@ config CMDLINE_FROM_BOOTLOADER
 	  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"
 	help
-- 
2.40.0


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

* [PATCH] arm64: Support CMDLINE_EXTEND
@ 2023-03-20 21:14 ` Chris Packham
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Packham @ 2023-03-20 21:14 UTC (permalink / raw)
  To: catalin.marinas, will; +Cc: linux-arm-kernel, linux-kernel, Chris Packham

Support extending the bootloader provided command line for arm64
targets. This support is already present via generic DT/EFI code the
only thing required is for the architecture to make it selectable.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm64/Kconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1023e896d46b..3c837b085f21 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -2228,6 +2228,12 @@ config CMDLINE_FROM_BOOTLOADER
 	  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"
 	help
-- 
2.40.0


_______________________________________________
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] 14+ messages in thread

* Re: [PATCH] arm64: Support CMDLINE_EXTEND
  2023-03-20 21:14 ` Chris Packham
@ 2023-03-21  3:54   ` Anshuman Khandual
  -1 siblings, 0 replies; 14+ messages in thread
From: Anshuman Khandual @ 2023-03-21  3:54 UTC (permalink / raw)
  To: Chris Packham, catalin.marinas, will; +Cc: linux-arm-kernel, linux-kernel

Hello Chris,

On 3/21/23 02:44, Chris Packham wrote:
> Support extending the bootloader provided command line for arm64
> targets. This support is already present via generic DT/EFI code the
> only thing required is for the architecture to make it selectable.

Does this config really depend on given platform's active support or
it is just matter of selecting this for interested platforms ? Could
this config definition be unified in a single place i.e arch/Kconfig
and be selected (unconditionally or conditionally) on all subscribing
platforms. There seems to be a redundancy in defining the exact same
config the same way, on multiple platforms.

$git grep "config CMDLINE_EXTEND"

arch/arm/Kconfig:config CMDLINE_EXTEND
arch/loongarch/Kconfig:config CMDLINE_EXTEND
arch/powerpc/Kconfig:config CMDLINE_EXTEND
arch/riscv/Kconfig:config CMDLINE_EXTEND
arch/sh/Kconfig:config CMDLINE_EXTEND   

I guess this redundancy should be removed as a pre-requisite, before
enabling it on arm64 as proposed here, which in itself seems alright.

> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  arch/arm64/Kconfig | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1023e896d46b..3c837b085f21 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -2228,6 +2228,12 @@ config CMDLINE_FROM_BOOTLOADER
>  	  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"
>  	help

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

* Re: [PATCH] arm64: Support CMDLINE_EXTEND
@ 2023-03-21  3:54   ` Anshuman Khandual
  0 siblings, 0 replies; 14+ messages in thread
From: Anshuman Khandual @ 2023-03-21  3:54 UTC (permalink / raw)
  To: Chris Packham, catalin.marinas, will; +Cc: linux-arm-kernel, linux-kernel

Hello Chris,

On 3/21/23 02:44, Chris Packham wrote:
> Support extending the bootloader provided command line for arm64
> targets. This support is already present via generic DT/EFI code the
> only thing required is for the architecture to make it selectable.

Does this config really depend on given platform's active support or
it is just matter of selecting this for interested platforms ? Could
this config definition be unified in a single place i.e arch/Kconfig
and be selected (unconditionally or conditionally) on all subscribing
platforms. There seems to be a redundancy in defining the exact same
config the same way, on multiple platforms.

$git grep "config CMDLINE_EXTEND"

arch/arm/Kconfig:config CMDLINE_EXTEND
arch/loongarch/Kconfig:config CMDLINE_EXTEND
arch/powerpc/Kconfig:config CMDLINE_EXTEND
arch/riscv/Kconfig:config CMDLINE_EXTEND
arch/sh/Kconfig:config CMDLINE_EXTEND   

I guess this redundancy should be removed as a pre-requisite, before
enabling it on arm64 as proposed here, which in itself seems alright.

> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  arch/arm64/Kconfig | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1023e896d46b..3c837b085f21 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -2228,6 +2228,12 @@ config CMDLINE_FROM_BOOTLOADER
>  	  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"
>  	help

_______________________________________________
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] 14+ messages in thread

* Re: [PATCH] arm64: Support CMDLINE_EXTEND
  2023-03-21  3:54   ` Anshuman Khandual
@ 2023-03-21  4:15     ` Chris Packham
  -1 siblings, 0 replies; 14+ messages in thread
From: Chris Packham @ 2023-03-21  4:15 UTC (permalink / raw)
  To: Anshuman Khandual, catalin.marinas, will; +Cc: linux-arm-kernel, linux-kernel

Hi,

On 21/03/23 16:54, Anshuman Khandual wrote:
> Hello Chris,
>
> On 3/21/23 02:44, Chris Packham wrote:
>> Support extending the bootloader provided command line for arm64
>> targets. This support is already present via generic DT/EFI code the
>> only thing required is for the architecture to make it selectable.
> Does this config really depend on given platform's active support or
> it is just matter of selecting this for interested platforms ?
Most modern platforms using DT or UEFI should be able to use the common 
code. There are some older ones (arm and powerpc) that have other/legacy 
methods of passing the kernel command line so retaining support for that 
while at the same time providing support for the generic methods as well 
could get tricky. It looks as though it won't be too bad as the code 
seems to use the same Kconfig options.
> Could
> this config definition be unified in a single place i.e arch/Kconfig
> and be selected (unconditionally or conditionally) on all subscribing
> platforms. There seems to be a redundancy in defining the exact same
> config the same way, on multiple platforms.
>
> $git grep "config CMDLINE_EXTEND"
>
> arch/arm/Kconfig:config CMDLINE_EXTEND
> arch/loongarch/Kconfig:config CMDLINE_EXTEND
> arch/powerpc/Kconfig:config CMDLINE_EXTEND
> arch/riscv/Kconfig:config CMDLINE_EXTEND
> arch/sh/Kconfig:config CMDLINE_EXTEND

Same applies to CMDLINE_FROM_BOOTLOADER/CMDLINE_FORCE. Although sh uses 
CMDLINE_OVERWRITE instead of CMDLINE_FORCE. I guess it'd be possible to 
move it to some common place and have the arches source it like they do 
for things like power management.

> I guess this redundancy should be removed as a pre-requisite, before
> enabling it on arm64 as proposed here, which in itself seems alright.
I was kind of hoping it wouldn't be a pre-requisite mainly because it 
turns a quick patch I can actually test on hardware I have in front of 
me to something that I can't test on half the platforms that are 
affected. I also haven't had to co-ordinate a change across 6 maintainer 
trees before. But I guess I can give it a try if that's the only way of 
getting it in.
>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>   arch/arm64/Kconfig | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 1023e896d46b..3c837b085f21 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -2228,6 +2228,12 @@ config CMDLINE_FROM_BOOTLOADER
>>   	  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"
>>   	help

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

* Re: [PATCH] arm64: Support CMDLINE_EXTEND
@ 2023-03-21  4:15     ` Chris Packham
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Packham @ 2023-03-21  4:15 UTC (permalink / raw)
  To: Anshuman Khandual, catalin.marinas, will; +Cc: linux-arm-kernel, linux-kernel

Hi,

On 21/03/23 16:54, Anshuman Khandual wrote:
> Hello Chris,
>
> On 3/21/23 02:44, Chris Packham wrote:
>> Support extending the bootloader provided command line for arm64
>> targets. This support is already present via generic DT/EFI code the
>> only thing required is for the architecture to make it selectable.
> Does this config really depend on given platform's active support or
> it is just matter of selecting this for interested platforms ?
Most modern platforms using DT or UEFI should be able to use the common 
code. There are some older ones (arm and powerpc) that have other/legacy 
methods of passing the kernel command line so retaining support for that 
while at the same time providing support for the generic methods as well 
could get tricky. It looks as though it won't be too bad as the code 
seems to use the same Kconfig options.
> Could
> this config definition be unified in a single place i.e arch/Kconfig
> and be selected (unconditionally or conditionally) on all subscribing
> platforms. There seems to be a redundancy in defining the exact same
> config the same way, on multiple platforms.
>
> $git grep "config CMDLINE_EXTEND"
>
> arch/arm/Kconfig:config CMDLINE_EXTEND
> arch/loongarch/Kconfig:config CMDLINE_EXTEND
> arch/powerpc/Kconfig:config CMDLINE_EXTEND
> arch/riscv/Kconfig:config CMDLINE_EXTEND
> arch/sh/Kconfig:config CMDLINE_EXTEND

Same applies to CMDLINE_FROM_BOOTLOADER/CMDLINE_FORCE. Although sh uses 
CMDLINE_OVERWRITE instead of CMDLINE_FORCE. I guess it'd be possible to 
move it to some common place and have the arches source it like they do 
for things like power management.

> I guess this redundancy should be removed as a pre-requisite, before
> enabling it on arm64 as proposed here, which in itself seems alright.
I was kind of hoping it wouldn't be a pre-requisite mainly because it 
turns a quick patch I can actually test on hardware I have in front of 
me to something that I can't test on half the platforms that are 
affected. I also haven't had to co-ordinate a change across 6 maintainer 
trees before. But I guess I can give it a try if that's the only way of 
getting it in.
>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>   arch/arm64/Kconfig | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 1023e896d46b..3c837b085f21 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -2228,6 +2228,12 @@ config CMDLINE_FROM_BOOTLOADER
>>   	  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"
>>   	help
_______________________________________________
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] 14+ messages in thread

* Re: [PATCH] arm64: Support CMDLINE_EXTEND
  2023-03-20 21:14 ` Chris Packham
@ 2023-03-21 10:45   ` Anshuman Khandual
  -1 siblings, 0 replies; 14+ messages in thread
From: Anshuman Khandual @ 2023-03-21 10:45 UTC (permalink / raw)
  To: Chris Packham, catalin.marinas, will; +Cc: linux-arm-kernel, linux-kernel



On 3/21/23 02:44, Chris Packham wrote:
> Support extending the bootloader provided command line for arm64
> targets. This support is already present via generic DT/EFI code the
> only thing required is for the architecture to make it selectable.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  arch/arm64/Kconfig | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1023e896d46b..3c837b085f21 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -2228,6 +2228,12 @@ config CMDLINE_FROM_BOOTLOADER
>  	  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"
>  	help

LGTM, FWIW

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

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

* Re: [PATCH] arm64: Support CMDLINE_EXTEND
@ 2023-03-21 10:45   ` Anshuman Khandual
  0 siblings, 0 replies; 14+ messages in thread
From: Anshuman Khandual @ 2023-03-21 10:45 UTC (permalink / raw)
  To: Chris Packham, catalin.marinas, will; +Cc: linux-arm-kernel, linux-kernel



On 3/21/23 02:44, Chris Packham wrote:
> Support extending the bootloader provided command line for arm64
> targets. This support is already present via generic DT/EFI code the
> only thing required is for the architecture to make it selectable.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  arch/arm64/Kconfig | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1023e896d46b..3c837b085f21 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -2228,6 +2228,12 @@ config CMDLINE_FROM_BOOTLOADER
>  	  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"
>  	help

LGTM, FWIW

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.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] 14+ messages in thread

* Re: [PATCH] arm64: Support CMDLINE_EXTEND
  2023-03-20 21:14 ` Chris Packham
@ 2023-03-21 11:19   ` Mark Rutland
  -1 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2023-03-21 11:19 UTC (permalink / raw)
  To: Chris Packham; +Cc: catalin.marinas, will, linux-arm-kernel, linux-kernel

On Tue, Mar 21, 2023 at 10:14:51AM +1300, Chris Packham wrote:
> Support extending the bootloader provided command line for arm64
> targets. This support is already present via generic DT/EFI code the
> only thing required is for the architecture to make it selectable.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

We deliberately dropped support for CMDLINE_EXTEND in commit:

  cae118b6acc3 ("arm64: Drop support for CMDLINE_EXTEND")

... which was mentioned the last time somone tried to re-add it:

  https://lore.kernel.org/linux-arm-kernel/ZAh8dWvbNkVQT11C@arm.com/

Has something changes such that those issues no longer apply? If so, please
call that out explicitly in the commit message. If not, I do not think we
should take this patch.

Thanks,
Mark.

> ---
>  arch/arm64/Kconfig | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1023e896d46b..3c837b085f21 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -2228,6 +2228,12 @@ config CMDLINE_FROM_BOOTLOADER
>  	  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"
>  	help
> -- 
> 2.40.0
> 

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

* Re: [PATCH] arm64: Support CMDLINE_EXTEND
@ 2023-03-21 11:19   ` Mark Rutland
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2023-03-21 11:19 UTC (permalink / raw)
  To: Chris Packham; +Cc: catalin.marinas, will, linux-arm-kernel, linux-kernel

On Tue, Mar 21, 2023 at 10:14:51AM +1300, Chris Packham wrote:
> Support extending the bootloader provided command line for arm64
> targets. This support is already present via generic DT/EFI code the
> only thing required is for the architecture to make it selectable.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

We deliberately dropped support for CMDLINE_EXTEND in commit:

  cae118b6acc3 ("arm64: Drop support for CMDLINE_EXTEND")

... which was mentioned the last time somone tried to re-add it:

  https://lore.kernel.org/linux-arm-kernel/ZAh8dWvbNkVQT11C@arm.com/

Has something changes such that those issues no longer apply? If so, please
call that out explicitly in the commit message. If not, I do not think we
should take this patch.

Thanks,
Mark.

> ---
>  arch/arm64/Kconfig | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1023e896d46b..3c837b085f21 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -2228,6 +2228,12 @@ config CMDLINE_FROM_BOOTLOADER
>  	  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"
>  	help
> -- 
> 2.40.0
> 

_______________________________________________
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] 14+ messages in thread

* Re: [PATCH] arm64: Support CMDLINE_EXTEND
  2023-03-21 11:19   ` Mark Rutland
@ 2023-03-21 19:06     ` Mark Brown
  -1 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2023-03-21 19:06 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Chris Packham, catalin.marinas, will, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 609 bytes --]

On Tue, Mar 21, 2023 at 11:19:55AM +0000, Mark Rutland wrote:

> We deliberately dropped support for CMDLINE_EXTEND in commit:

>   cae118b6acc3 ("arm64: Drop support for CMDLINE_EXTEND")

> ... which was mentioned the last time somone tried to re-add it:

>   https://lore.kernel.org/linux-arm-kernel/ZAh8dWvbNkVQT11C@arm.com/

> Has something changes such that those issues no longer apply? If so, please
> call that out explicitly in the commit message. If not, I do not think we
> should take this patch.

Given that there have been multiple attempts to readd it is it worth
documenting this in the code?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH] arm64: Support CMDLINE_EXTEND
@ 2023-03-21 19:06     ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2023-03-21 19:06 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Chris Packham, catalin.marinas, will, linux-arm-kernel, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 609 bytes --]

On Tue, Mar 21, 2023 at 11:19:55AM +0000, Mark Rutland wrote:

> We deliberately dropped support for CMDLINE_EXTEND in commit:

>   cae118b6acc3 ("arm64: Drop support for CMDLINE_EXTEND")

> ... which was mentioned the last time somone tried to re-add it:

>   https://lore.kernel.org/linux-arm-kernel/ZAh8dWvbNkVQT11C@arm.com/

> Has something changes such that those issues no longer apply? If so, please
> call that out explicitly in the commit message. If not, I do not think we
> should take this patch.

Given that there have been multiple attempts to readd it is it worth
documenting this in the code?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
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] 14+ messages in thread

* Re: [PATCH] arm64: Support CMDLINE_EXTEND
  2023-03-21 19:06     ` Mark Brown
@ 2023-03-21 20:16       ` Chris Packham
  -1 siblings, 0 replies; 14+ messages in thread
From: Chris Packham @ 2023-03-21 20:16 UTC (permalink / raw)
  To: Mark Brown, Mark Rutland
  Cc: catalin.marinas, will, linux-arm-kernel, linux-kernel


On 22/03/23 08:06, Mark Brown wrote:
> On Tue, Mar 21, 2023 at 11:19:55AM +0000, Mark Rutland wrote:
>
>> We deliberately dropped support for CMDLINE_EXTEND in commit:
>>    cae118b6acc3 ("arm64: Drop support for CMDLINE_EXTEND")
>> ... which was mentioned the last time somone tried to re-add it:
>>    https://lore.kernel.org/linux-arm-kernel/ZAh8dWvbNkVQT11C@arm.com/
>> Has something changes such that those issues no longer apply?

I'm not sure I see any "issues" (although perhaps that's just me not 
being able to find the rest of the series on lore).

My specific use case is that I want to have the  sbsa_gwdt watchdog 
driver built-in to the kernel but I also want it to produce panic output 
instead of silently resetting the board so I want to have 
sbsa_gwdt.action=1 in my kernel command line. Either appending or 
prepending that on the command line would work for me.

>>   If so, please
>> call that out explicitly in the commit message. If not, I do not think we
>> should take this patch.

To save a click here is the relevant part from cae118b6acc3

     The documented behaviour for CMDLINE_EXTEND is that the arguments from
     the bootloader are appended to the built-in kernel command line. This
     also matches the option parsing behaviour for the EFI stub and early ID
     register overrides.

     Bizarrely, the fdt behaviour is the other way around: appending the
     built-in command line to the bootloader arguments, resulting in a
     command-line that doesn't necessarily line-up with the parsing 
order and
     definitely doesn't line-up with the documented behaviour.

This appears to be a current problem for arm an powerpc. If it weren't 
for the fact that EFI version had different behaviour I'd be suggesting 
just updating the help text. Maybe that should be done anyway regardless 
of any unification so that the documentation reflects reality.

> Given that there have been multiple attempts to readd it is it worth
> documenting this in the code?

The proposed CMDLINE_APPEND would work well for my use-case but two 
attempts at landing such generic support seem to have failed. I could 
attempt to resurrect [1] or the alternative [2] but I'm worried I'll end 
up with the same road blocks. Also looks like I just found a 3rd [3].

I hope I haven't kicked a hornets nest here.

--

[1] - 
https://lore.kernel.org/lkml/20190319232448.45964-2-danielwa@cisco.com/
[2] - 
https://patchwork.ozlabs.org/project/linuxppc-dev/cover/cover.1554195798.git.christophe.leroy@c-s.fr/
[3] - 
https://lore.kernel.org/lkml/20210416040924.2882771-1-danielwa@cisco.com/

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

* Re: [PATCH] arm64: Support CMDLINE_EXTEND
@ 2023-03-21 20:16       ` Chris Packham
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Packham @ 2023-03-21 20:16 UTC (permalink / raw)
  To: Mark Brown, Mark Rutland
  Cc: catalin.marinas, will, linux-arm-kernel, linux-kernel


On 22/03/23 08:06, Mark Brown wrote:
> On Tue, Mar 21, 2023 at 11:19:55AM +0000, Mark Rutland wrote:
>
>> We deliberately dropped support for CMDLINE_EXTEND in commit:
>>    cae118b6acc3 ("arm64: Drop support for CMDLINE_EXTEND")
>> ... which was mentioned the last time somone tried to re-add it:
>>    https://lore.kernel.org/linux-arm-kernel/ZAh8dWvbNkVQT11C@arm.com/
>> Has something changes such that those issues no longer apply?

I'm not sure I see any "issues" (although perhaps that's just me not 
being able to find the rest of the series on lore).

My specific use case is that I want to have the  sbsa_gwdt watchdog 
driver built-in to the kernel but I also want it to produce panic output 
instead of silently resetting the board so I want to have 
sbsa_gwdt.action=1 in my kernel command line. Either appending or 
prepending that on the command line would work for me.

>>   If so, please
>> call that out explicitly in the commit message. If not, I do not think we
>> should take this patch.

To save a click here is the relevant part from cae118b6acc3

     The documented behaviour for CMDLINE_EXTEND is that the arguments from
     the bootloader are appended to the built-in kernel command line. This
     also matches the option parsing behaviour for the EFI stub and early ID
     register overrides.

     Bizarrely, the fdt behaviour is the other way around: appending the
     built-in command line to the bootloader arguments, resulting in a
     command-line that doesn't necessarily line-up with the parsing 
order and
     definitely doesn't line-up with the documented behaviour.

This appears to be a current problem for arm an powerpc. If it weren't 
for the fact that EFI version had different behaviour I'd be suggesting 
just updating the help text. Maybe that should be done anyway regardless 
of any unification so that the documentation reflects reality.

> Given that there have been multiple attempts to readd it is it worth
> documenting this in the code?

The proposed CMDLINE_APPEND would work well for my use-case but two 
attempts at landing such generic support seem to have failed. I could 
attempt to resurrect [1] or the alternative [2] but I'm worried I'll end 
up with the same road blocks. Also looks like I just found a 3rd [3].

I hope I haven't kicked a hornets nest here.

--

[1] - 
https://lore.kernel.org/lkml/20190319232448.45964-2-danielwa@cisco.com/
[2] - 
https://patchwork.ozlabs.org/project/linuxppc-dev/cover/cover.1554195798.git.christophe.leroy@c-s.fr/
[3] - 
https://lore.kernel.org/lkml/20210416040924.2882771-1-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] 14+ messages in thread

end of thread, other threads:[~2023-03-21 20:18 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-20 21:14 [PATCH] arm64: Support CMDLINE_EXTEND Chris Packham
2023-03-20 21:14 ` Chris Packham
2023-03-21  3:54 ` Anshuman Khandual
2023-03-21  3:54   ` Anshuman Khandual
2023-03-21  4:15   ` Chris Packham
2023-03-21  4:15     ` Chris Packham
2023-03-21 10:45 ` Anshuman Khandual
2023-03-21 10:45   ` Anshuman Khandual
2023-03-21 11:19 ` Mark Rutland
2023-03-21 11:19   ` Mark Rutland
2023-03-21 19:06   ` Mark Brown
2023-03-21 19:06     ` Mark Brown
2023-03-21 20:16     ` Chris Packham
2023-03-21 20:16       ` Chris Packham

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.