[PATCH] loongarch: Make CONFIG_CMDLINE work with CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER

[PATCH] loongarch: Make CONFIG_CMDLINE work with CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER

[Dong Zhihong]

This patch tends to fix broken CONFIG_CMDLINE, which is not working with
CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER. Some code is absent in
arch/loongarch/kernel/setup.c, which should handle built-in command lines
in CONFIG_CMDLINE. The absence of such code causes CONFIG_CMDLINE not
working if CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER is set to Y.
CONFIG_CMDLINE_FORCE is not affected.

Signed-off-by: Dong Zhihong <donmor3000@hotmail.com>
---
 arch/loongarch/kernel/setup.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
index 78a00359bde3..504218b94879 100644
--- a/arch/loongarch/kernel/setup.c
+++ b/arch/loongarch/kernel/setup.c
@@ -335,6 +335,22 @@ static void __init bootcmdline_init(char **cmdline_p)
 	}
 #endif
 
+#ifdef CONFIG_CMDLINE
+	/*
+	 * If CONFIG_CMDLINE_BOOTLOADER is enabled then we use thei built-in
+	 * command line if no command line given, or we append given command
+	 * line to the built-in one if CONFIG_CMDLINE_EXTEND is enabled.
+	 */
+	if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) {
+		strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+		strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
+		strlcat(boot_command_line, init_command_line, COMMAND_LINE_SIZE);
+	}
+
+	if (IS_ENABLED(CONFIG_CMDLINE_BOOTLOADER) && !boot_command_line[0])
+		strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+#endif
+
 out:
 	*cmdline_p = boot_command_line;
 }
-- 
2.25.1

Re: [PATCH] loongarch: Make CONFIG_CMDLINE work with CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER

[Randy Dunlap]

On 7/9/23 06:32, Dong Zhihong wrote:
> This patch tends to fix broken CONFIG_CMDLINE, which is not working with
> CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER. Some code is absent in
> arch/loongarch/kernel/setup.c, which should handle built-in command lines
> in CONFIG_CMDLINE. The absence of such code causes CONFIG_CMDLINE not
> working if CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER is set to Y.
> CONFIG_CMDLINE_FORCE is not affected.
> 
> Signed-off-by: Dong Zhihong <donmor3000@hotmail.com>
> ---
>  arch/loongarch/kernel/setup.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> index 78a00359bde3..504218b94879 100644
> --- a/arch/loongarch/kernel/setup.c
> +++ b/arch/loongarch/kernel/setup.c
> @@ -335,6 +335,22 @@ static void __init bootcmdline_init(char **cmdline_p)
>  	}
>  #endif
>  
> +#ifdef CONFIG_CMDLINE
> +	/*
> +	 * If CONFIG_CMDLINE_BOOTLOADER is enabled then we use thei built-in

	                                                       the

> +	 * command line if no command line given, or we append given command
> +	 * line to the built-in one if CONFIG_CMDLINE_EXTEND is enabled.
> +	 */
> +	if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) {
> +		strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> +		strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> +		strlcat(boot_command_line, init_command_line, COMMAND_LINE_SIZE);
> +	}
> +
> +	if (IS_ENABLED(CONFIG_CMDLINE_BOOTLOADER) && !boot_command_line[0])
> +		strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> +#endif
> +
>  out:
>  	*cmdline_p = boot_command_line;
>  }

-- 
~Randy

Re: [PATCH] loongarch: Make CONFIG_CMDLINE work with CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER

[Huacai Chen]

On Sun, Jul 9, 2023 at 9:33 PM Dong Zhihong <donmor3000@hotmail.com> wrote:
>
> This patch tends to fix broken CONFIG_CMDLINE, which is not working with
> CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER. Some code is absent in
> arch/loongarch/kernel/setup.c, which should handle built-in command lines
> in CONFIG_CMDLINE. The absence of such code causes CONFIG_CMDLINE not
> working if CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER is set to Y.
> CONFIG_CMDLINE_FORCE is not affected.
How to reproduce? If you use UEFI firmware, this is handled in
drivers/firmware/efi/libstub/efi-stub.c; if you use non-UEFI firmware,
this is handled in drivers/of/fdt.c.

Huacai
>
> Signed-off-by: Dong Zhihong <donmor3000@hotmail.com>
> ---
>  arch/loongarch/kernel/setup.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> index 78a00359bde3..504218b94879 100644
> --- a/arch/loongarch/kernel/setup.c
> +++ b/arch/loongarch/kernel/setup.c
> @@ -335,6 +335,22 @@ static void __init bootcmdline_init(char **cmdline_p)
>         }
>  #endif
>
> +#ifdef CONFIG_CMDLINE
> +       /*
> +        * If CONFIG_CMDLINE_BOOTLOADER is enabled then we use thei built-in
> +        * command line if no command line given, or we append given command
> +        * line to the built-in one if CONFIG_CMDLINE_EXTEND is enabled.
> +        */
> +       if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) {
> +               strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> +               strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> +               strlcat(boot_command_line, init_command_line, COMMAND_LINE_SIZE);
> +       }
> +
> +       if (IS_ENABLED(CONFIG_CMDLINE_BOOTLOADER) && !boot_command_line[0])
> +               strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> +#endif
> +
>  out:
>         *cmdline_p = boot_command_line;
>  }
> --
> 2.25.1
>
>

Re: [PATCH] loongarch: Make CONFIG_CMDLINE work with CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER

[‎ donmor]

在 2023-07-09星期日的 23:30 +0800，Huacai Chen写道：
> How to reproduce? If you use UEFI firmware, this is handled in
> drivers/firmware/efi/libstub/efi-stub.c; if you use non-UEFI firmware,
> this is handled in drivers/of/fdt.c.

In fact it 's not command line retrieved from efi or fdt, but the built-in
command line to be concerned. CONFIG_CMDLINE was never been touched during
boot on loongarch, unless CONFIG_CMDLINE_FORCE is set because there is code
handling this. There should be code handling CONFIG_CMDLINE_FORCE and
CONFIG_CMDLINE_EXTEND as well but is absent by now.

Re: [PATCH] loongarch: Make CONFIG_CMDLINE work with CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER

[Huacai Chen]

On Mon, Jul 10, 2023 at 1:35 AM ‎ donmor <donmor3000@hotmail.com> wrote:
>
> 在 2023-07-09星期日的 23:30 +0800，Huacai Chen写道：
> > How to reproduce? If you use UEFI firmware, this is handled in
> > drivers/firmware/efi/libstub/efi-stub.c; if you use non-UEFI firmware,
> > this is handled in drivers/of/fdt.c.
>
> In fact it 's not command line retrieved from efi or fdt, but the built-in
> command line to be concerned. CONFIG_CMDLINE was never been touched during
> boot on loongarch, unless CONFIG_CMDLINE_FORCE is set because there is code
> handling this. There should be code handling CONFIG_CMDLINE_FORCE and
> CONFIG_CMDLINE_EXTEND as well but is absent by now.

I mean the code in drivers/of/fdt.c already handles
CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_FORCE, understand?

On the other hand, I found that for non-FDT system
CONFIG_CMDLINE_FORCE indeed doesn't work, but
CONFIG_CMDLINE_BOOTLOADER works, so you needn't fix
CONFIG_CMDLINE_BOOTLOADER, and you need add "goto out" in the "if
(initial_boot_params)" condition.

Huacai

Re: [PATCH] loongarch: Make CONFIG_CMDLINE work with CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER

[‎ donmor]

在 2023-07-10星期一的 15:53 +0800，Huacai Chen写道：
> On Mon, Jul 10, 2023 at 1:35 AM ‎ donmor <donmor3000@hotmail.com> wrote:
> > 在 2023-07-09星期日的 23:30 +0800，Huacai Chen写道：
> > > How to reproduce? If you use UEFI firmware, this is handled in
> > > drivers/firmware/efi/libstub/efi-stub.c; if you use non-UEFI firmware,
> > > this is handled in drivers/of/fdt.c.
> > 
> > In fact it 's not command line retrieved from efi or fdt, but the built-in
> > command line to be concerned. CONFIG_CMDLINE was never been touched during
> > boot on loongarch, unless CONFIG_CMDLINE_FORCE is set because there is code
> > handling this. There should be code handling CONFIG_CMDLINE_FORCE and
> > CONFIG_CMDLINE_EXTEND as well but is absent by now.
> 
> I mean the code in drivers/of/fdt.c already handles
> CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_FORCE, understand?
> 
> On the other hand, I found that for non-FDT system
> CONFIG_CMDLINE_FORCE indeed doesn't work, but
> CONFIG_CMDLINE_BOOTLOADER works, so you needn't fix
> CONFIG_CMDLINE_BOOTLOADER, and you need add "goto out" in the "if
> (initial_boot_params)" condition.
> 
> Huacai

Got it. I'll make a v2 asap.

donmor

Re: [PATCH] loongarch: Make CONFIG_CMDLINE work with CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER

[‎ donmor]

在 2023-07-10星期一的 15:53 +0800，Huacai Chen写道：
> 
> On the other hand, I found that for non-FDT system
> CONFIG_CMDLINE_FORCE indeed doesn't work, but
> CONFIG_CMDLINE_BOOTLOADER works, so you needn't fix
> CONFIG_CMDLINE_BOOTLOADER, and you need add "goto out" in the "if
> (initial_boot_params)" condition.
> 
> Huacai

And It is CONFIG_CMDLINE_FORCE but not CONFIG_CMDLINE_BOOTLOADER
that does work.

donmor

Re: [PATCH] loongarch: Make CONFIG_CMDLINE work with CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER

[Markus Elfring]

> This patch tends to fix broken CONFIG_CMDLINE, which is not working with
> CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER. Some code is absent in
> arch/loongarch/kernel/setup.c, which should handle built-in command lines
> in CONFIG_CMDLINE. The absence of such code causes CONFIG_CMDLINE not
> working if CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER is set to Y.
…

Which circumstances did hinder you to choose another imperative change suggestion?

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94

Regards,
Markus

Re: [PATCH] loongarch: Make CONFIG_CMDLINE work with CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER

[‎ donmor]

在 2023-07-09星期日的 17:43 +0200，Markus Elfring写道：
> > This patch tends to fix broken CONFIG_CMDLINE, which is not working with
> > CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER. Some code is absent in
> > arch/loongarch/kernel/setup.c, which should handle built-in command lines
> > in CONFIG_CMDLINE. The absence of such code causes CONFIG_CMDLINE not
> > working if CONFIG_CMDLINE_EXTEND or CONFIG_CMDLINE_BOOTLOADER is set to Y.
> …
> 
> Which circumstances did hinder you to choose another imperative change suggestion?
> 
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94
> 
> Regards,
> Markus

I'm trying boot the system with a single bootloongarch64.efi and got stuck on mounting
rootfs. It turns out to be the malfunction of kernel which dropped my built-in command
line. It's quiet annoying to be lacking a feature compared to other architectures.
Also, a feature shown in config should be functioning, or be considered buggy and get
fixed.

Re: loongarch: Make CONFIG_CMDLINE work with CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER

[Markus Elfring]

> Also, a feature shown in config should be functioning,

We may hope so.


> or be considered buggy

This can happen.


> and get fixed.

Such an action can occasionally become desirable.

Under which circumstances would you get into the mood to enhance descriptions
another bit (also according to the published Linux development requirements)
for the presented change possibilities?

Regards,
Markus