[PATCH 2/2] LoongArch: Refactor command line processing

[Dave Young]

On Thu, 25 Sept 2025 at 17:52, Youling Tang <youling.tang at linux.dev> wrote:
>
> On 9/25/25 17:22, Dave Young wrote:
>
> On Thu, 25 Sept 2025 at 14:33, Youling Tang <youling.tang at linux.dev> wrote:
>
> From: Youling Tang <tangyouling at kylinos.cn>
>
> Refactor the cmdline_add_xxx code flow, and simultaneously display
> the content of parameters such as initrd in hexadecimal format to
> improve readability.
>
> Signed-off-by: Youling Tang <tangyouling at kylinos.cn>
> ---
>  kexec/arch/loongarch/kexec-loongarch.c | 138 ++++++++++---------------
>  1 file changed, 55 insertions(+), 83 deletions(-)
>
> diff --git a/kexec/arch/loongarch/kexec-loongarch.c b/kexec/arch/loongarch/kexec-loongarch.c
> index 240202f..c2503de 100644
> --- a/kexec/arch/loongarch/kexec-loongarch.c
> +++ b/kexec/arch/loongarch/kexec-loongarch.c
> @@ -35,83 +35,49 @@
>  #define _O_BINARY 0
>  #endif
>
> -#define CMDLINE_PREFIX "kexec "
> -static char cmdline[COMMAND_LINE_SIZE] = CMDLINE_PREFIX;
> +/* Add the "kexec" command line parameter to command line. */
> +static void cmdline_add_loader(unsigned long *cmdline_tmplen, char *modified_cmdline)
> +{
> +       int loader_strlen;
> +
> +       loader_strlen = sprintf(modified_cmdline + (*cmdline_tmplen), "kexec ");
> +       *cmdline_tmplen += loader_strlen;
> +}
>
> Not sure why this is needed,  I guess it is to distinguish the new
> kernel and original kernel?   As replied in another reply I would
> suggest adding an extra cmdline in scripts instead of hard coded here,
>   you need to remove the fake param each time otherwise it will make
> the cmdline longer and longer after many kexec reboot cycles.
>
>
> In arch_process_options(), the "kexec" parameter will be removed when
> reusing the current command line.
>
> -/* Adds "initrd=start,size" parameters to command line. */
> -static int cmdline_add_initrd(char *cmdline, unsigned long addr,
> -               unsigned long size)
> +/* Add the "initrd=start,size" command line parameter to command line. */
> +static void cmdline_add_initrd(unsigned long *cmdline_tmplen, char *modified_cmdline,
> +                              unsigned long initrd_base, unsigned long initrd_size)
>  {
> -       int cmdlen, len;
> -       char str[50], *ptr;
> -
> -       ptr = str;
> -       strcpy(str, " initrd=");
> -       ptr += strlen(str);
> -       ultoa(addr, ptr);
> -       strcat(str, ",");
> -       ptr = str + strlen(str);
> -       ultoa(size, ptr);
> -       len = strlen(str);
> -       cmdlen = strlen(cmdline) + len;
> -       if (cmdlen > (COMMAND_LINE_SIZE - 1))
> -               die("Command line overflow\n");
> -       strcat(cmdline, str);
> +       int initrd_strlen;
>
> -       return 0;
> +       initrd_strlen = sprintf(modified_cmdline + (*cmdline_tmplen), "initrd=0x%lx,0x%lx ",
> +                               initrd_base, initrd_size);
> +       *cmdline_tmplen += initrd_strlen;
>  }
>
> -/* Adds the appropriate "mem=size at start" options to command line, indicating the
> - * memory region the new kernel can use to boot into. */
> -static int cmdline_add_mem(char *cmdline, unsigned long addr,
> -               unsigned long size)
> +/*
> + * Add the "mem=size at start" command line parameter to command line, indicating the
> + * memory region the new kernel can use to boot into.
> + */
> +static void cmdline_add_mem(unsigned long *cmdline_tmplen, char *modified_cmdline,
> +                           unsigned long mem_start, unsigned long mem_sz)
>  {
> -       int cmdlen, len;
> -       char str[50], *ptr;
> -
> -       addr = addr/1024;
> -       size = size/1024;
> -       ptr = str;
> -       strcpy(str, " mem=");
> -       ptr += strlen(str);
> -       ultoa(size, ptr);
> -       strcat(str, "K@");
> -       ptr = str + strlen(str);
> -       ultoa(addr, ptr);
> -       strcat(str, "K");
> -       len = strlen(str);
> -       cmdlen = strlen(cmdline) + len;
> -       if (cmdlen > (COMMAND_LINE_SIZE - 1))
> -               die("Command line overflow\n");
> -       strcat(cmdline, str);
> +       int mem_strlen = 0;
>
> -       return 0;
> +       mem_strlen = sprintf(modified_cmdline + (*cmdline_tmplen), "mem=0x%lx at 0x%lx ",
> +                            mem_sz, mem_start);
> +       *cmdline_tmplen += mem_strlen;
>  }
>
> Ditto for the mem= param and other similar cases, can this be done out
> of the kexec-tools c code? it will be more flexible.
>
>
> Currently, we will maintain passing this parameter through the command line, not
> via FDT like ARM64. In the future, we may consider whether it can be passed through
> the FDT table in efisystab (but that approach may not be friendly to ELF kernels).
>

If the kexec boot depends on the customized mem layout, ideally it
should be passed with fdt or other method.
it is reasonable to keep it for the time being.  But the "kexec" extra
cmdline should not be hard coded in my opinion.

Thanks
Dave