[PATCH 1/2] Remove the "mem=" parameter when using the --reuse-cmdline option in kdump operation

[PATCH 1/2] Remove the "mem=" parameter when using the --reuse-cmdline option in kdump operation

[Youling Tang]

From: Youling Tang <tangyouling@kylinos.cn>

During kdump operations, the capture kernel cannot reuse the "mem="
parameter from the production kernel. The "mem=" parameter is used
to specify the available memory range for the kernel. Reusing the
"mem=" memory range may destroy the production environment.

Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
---
 kexec/kexec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index c9e4bcb..fadf986 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1256,8 +1256,10 @@ char *get_command_line(void)
 		*p = '\0';
 
 	remove_parameter(line, "BOOT_IMAGE");
-	if (kexec_flags & KEXEC_ON_CRASH)
+	if (kexec_flags & KEXEC_ON_CRASH) {
 		remove_parameter(line, "crashkernel");
+		remove_parameter(line, "mem=");
+	}
 
 	return line;
 }
-- 
2.48.1

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

[Youling Tang]

From: Youling Tang <tangyouling@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@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;
+}
 
-/* 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@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@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@0x%lx ",
+			     mem_sz, mem_start);
+	*cmdline_tmplen += mem_strlen;
 }
 
-/* Adds the "elfcorehdr=size@start" command line parameter to command line. */
-static int cmdline_add_elfcorehdr(char *cmdline, unsigned long addr,
-			unsigned long size)
+/* Add the "elfcorehdr=size@start" command line parameter to command line. */
+static void cmdline_add_elfcorehdr(unsigned long *cmdline_tmplen, char *modified_cmdline,
+				   unsigned long elfcorehdr_start, unsigned long elfcorehdr_sz)
 {
-	int cmdlen, len;
-	char str[50], *ptr;
-
-	addr = addr/1024;
-	size = size/1024;
-	ptr = str;
-	strcpy(str, " elfcorehdr=");
-	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 elfcorehdr_strlen = 0;
 
-	return 0;
+	elfcorehdr_strlen = sprintf(modified_cmdline + (*cmdline_tmplen), "elfcorehdr=0x%lx@0x%lx ",
+				    elfcorehdr_sz, elfcorehdr_start);
+	*cmdline_tmplen += elfcorehdr_strlen;
 }
 
 /* Return a sorted list of memory ranges. */
@@ -329,18 +295,17 @@ int loongarch_load_other_segments(struct kexec_info *info, unsigned long hole_mi
 	unsigned long initrd_min, hole_max;
 	char *initrd_buf = NULL;
 	unsigned long pagesize = getpagesize();
+	unsigned long cmdline_tmplen = 0;
+	char *cmdline = NULL;
 	int i;
 
-	if (arch_options.command_line) {
-		if (strlen(arch_options.command_line) >
-		    sizeof(cmdline) - 1) {
-			fprintf(stderr,
-				"Kernel command line too long for kernel!\n");
-			return EFAILED;
-		}
+	cmdline = calloc(1, COMMAND_LINE_SIZE);
+	if (!cmdline)
+		return EFAILED;
 
-		strncat(cmdline, arch_options.command_line, sizeof(cmdline) - 1);
-	}
+	/* Ensure it's null terminated */
+	cmdline_add_loader(&cmdline_tmplen, cmdline);
+	cmdline[COMMAND_LINE_SIZE - 1] = '\0';
 
 	/* Put the other segments after the image. */
 
@@ -360,21 +325,29 @@ int loongarch_load_other_segments(struct kexec_info *info, unsigned long hole_mi
 						pagesize), hole_max, -1);
 		dbgprintf("initrd_base: %lx, initrd_size: %lx\n", initrd_base, initrd_size);
 
-		cmdline_add_initrd(cmdline, initrd_base, initrd_size);
+		cmdline_add_initrd(&cmdline_tmplen, cmdline, initrd_base, initrd_size);
 	}
 
 	if (info->kexec_flags & KEXEC_ON_CRASH) {
-		cmdline_add_elfcorehdr(cmdline, elfcorehdr_mem.start,
-				elfcorehdr_mem.end - elfcorehdr_mem.start + 1);
+		cmdline_add_elfcorehdr(&cmdline_tmplen, cmdline, elfcorehdr_mem.start,
+				       elfcorehdr_mem.end - elfcorehdr_mem.start + 1);
 
 		for(i = 0;i < usablemem_rgns.size; i++) {
-			cmdline_add_mem(cmdline, crash_reserved_mem[i].start,
-			crash_reserved_mem[i].end -
-			crash_reserved_mem[i].start + 1);
+			cmdline_add_mem(&cmdline_tmplen, cmdline, crash_reserved_mem[i].start,
+					crash_reserved_mem[i].end - crash_reserved_mem[i].start + 1);
+		}
+	}
+
+	if (arch_options.command_line) {
+		if (strlen(arch_options.command_line) + cmdline_tmplen > COMMAND_LINE_SIZE) {
+			fprintf(stderr, "Kernel command line too long for kernel!\n");
+			free(cmdline);
+			return EFAILED;
 		}
+		memcpy(cmdline + cmdline_tmplen, arch_options.command_line,
+		       strlen(arch_options.command_line));
 	}
 
-	cmdline[sizeof(cmdline) - 1] = '\0';
 	add_buffer(info, cmdline, sizeof(cmdline), sizeof(cmdline),
 		sizeof(void *), _ALIGN_UP(hole_min, getpagesize()),
 		hole_max, 1);
@@ -382,7 +355,6 @@ int loongarch_load_other_segments(struct kexec_info *info, unsigned long hole_mi
 	dbgprintf("%s:%d: command_line: %s\n", __func__, __LINE__, cmdline);
 
 	return 0;
-
 }
 
 int arch_compat_trampoline(struct kexec_info *UNUSED(info))
-- 
2.48.1

Re: [PATCH 1/2] Remove the "mem=" parameter when using the --reuse-cmdline option in kdump operation

[Dave Young]

Hi Youling,

On Thu, 25 Sept 2025 at 14:33, Youling Tang <youling.tang@linux.dev> wrote:
>
> From: Youling Tang <tangyouling@kylinos.cn>
>
> During kdump operations, the capture kernel cannot reuse the "mem="
> parameter from the production kernel. The "mem=" parameter is used
> to specify the available memory range for the kernel. Reusing the
> "mem=" memory range may destroy the production environment.
>
> Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
> ---
>  kexec/kexec.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index c9e4bcb..fadf986 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1256,8 +1256,10 @@ char *get_command_line(void)
>                 *p = '\0';
>
>         remove_parameter(line, "BOOT_IMAGE");
> -       if (kexec_flags & KEXEC_ON_CRASH)
> +       if (kexec_flags & KEXEC_ON_CRASH) {
>                 remove_parameter(line, "crashkernel");
> +               remove_parameter(line, "mem=");
> +       }

People can remove it from the kdump load scripts instead if it is not
useful. I do not suggest removing other cmdline in c code.  The
crashkernel is a special one which is fine.

>
>         return line;
>  }
> --
> 2.48.1
>
>

Thanks
Dave

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

[Dave Young]

On Thu, 25 Sept 2025 at 14:33, Youling Tang <youling.tang@linux.dev> wrote:
>
> From: Youling Tang <tangyouling@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@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.

>
> -/* 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@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@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@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.

>
> -/* Adds the "elfcorehdr=size@start" command line parameter to command line. */
> -static int cmdline_add_elfcorehdr(char *cmdline, unsigned long addr,
> -                       unsigned long size)
> +/* Add the "elfcorehdr=size@start" command line parameter to command line. */
> +static void cmdline_add_elfcorehdr(unsigned long *cmdline_tmplen, char *modified_cmdline,
> +                                  unsigned long elfcorehdr_start, unsigned long elfcorehdr_sz)
>  {
> -       int cmdlen, len;
> -       char str[50], *ptr;
> -
> -       addr = addr/1024;
> -       size = size/1024;
> -       ptr = str;
> -       strcpy(str, " elfcorehdr=");
> -       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 elfcorehdr_strlen = 0;
>
> -       return 0;
> +       elfcorehdr_strlen = sprintf(modified_cmdline + (*cmdline_tmplen), "elfcorehdr=0x%lx@0x%lx ",
> +                                   elfcorehdr_sz, elfcorehdr_start);
> +       *cmdline_tmplen += elfcorehdr_strlen;
>  }
>
>  /* Return a sorted list of memory ranges. */
> @@ -329,18 +295,17 @@ int loongarch_load_other_segments(struct kexec_info *info, unsigned long hole_mi
>         unsigned long initrd_min, hole_max;
>         char *initrd_buf = NULL;
>         unsigned long pagesize = getpagesize();
> +       unsigned long cmdline_tmplen = 0;
> +       char *cmdline = NULL;
>         int i;
>
> -       if (arch_options.command_line) {
> -               if (strlen(arch_options.command_line) >
> -                   sizeof(cmdline) - 1) {
> -                       fprintf(stderr,
> -                               "Kernel command line too long for kernel!\n");
> -                       return EFAILED;
> -               }
> +       cmdline = calloc(1, COMMAND_LINE_SIZE);
> +       if (!cmdline)
> +               return EFAILED;
>
> -               strncat(cmdline, arch_options.command_line, sizeof(cmdline) - 1);
> -       }
> +       /* Ensure it's null terminated */
> +       cmdline_add_loader(&cmdline_tmplen, cmdline);
> +       cmdline[COMMAND_LINE_SIZE - 1] = '\0';
>
>         /* Put the other segments after the image. */
>
> @@ -360,21 +325,29 @@ int loongarch_load_other_segments(struct kexec_info *info, unsigned long hole_mi
>                                                 pagesize), hole_max, -1);
>                 dbgprintf("initrd_base: %lx, initrd_size: %lx\n", initrd_base, initrd_size);
>
> -               cmdline_add_initrd(cmdline, initrd_base, initrd_size);
> +               cmdline_add_initrd(&cmdline_tmplen, cmdline, initrd_base, initrd_size);
>         }
>
>         if (info->kexec_flags & KEXEC_ON_CRASH) {
> -               cmdline_add_elfcorehdr(cmdline, elfcorehdr_mem.start,
> -                               elfcorehdr_mem.end - elfcorehdr_mem.start + 1);
> +               cmdline_add_elfcorehdr(&cmdline_tmplen, cmdline, elfcorehdr_mem.start,
> +                                      elfcorehdr_mem.end - elfcorehdr_mem.start + 1);
>
>                 for(i = 0;i < usablemem_rgns.size; i++) {
> -                       cmdline_add_mem(cmdline, crash_reserved_mem[i].start,
> -                       crash_reserved_mem[i].end -
> -                       crash_reserved_mem[i].start + 1);
> +                       cmdline_add_mem(&cmdline_tmplen, cmdline, crash_reserved_mem[i].start,
> +                                       crash_reserved_mem[i].end - crash_reserved_mem[i].start + 1);
> +               }
> +       }
> +
> +       if (arch_options.command_line) {
> +               if (strlen(arch_options.command_line) + cmdline_tmplen > COMMAND_LINE_SIZE) {
> +                       fprintf(stderr, "Kernel command line too long for kernel!\n");
> +                       free(cmdline);
> +                       return EFAILED;
>                 }
> +               memcpy(cmdline + cmdline_tmplen, arch_options.command_line,
> +                      strlen(arch_options.command_line));
>         }
>
> -       cmdline[sizeof(cmdline) - 1] = '\0';
>         add_buffer(info, cmdline, sizeof(cmdline), sizeof(cmdline),
>                 sizeof(void *), _ALIGN_UP(hole_min, getpagesize()),
>                 hole_max, 1);
> @@ -382,7 +355,6 @@ int loongarch_load_other_segments(struct kexec_info *info, unsigned long hole_mi
>         dbgprintf("%s:%d: command_line: %s\n", __func__, __LINE__, cmdline);
>
>         return 0;
> -
>  }
>
>  int arch_compat_trampoline(struct kexec_info *UNUSED(info))
> --
> 2.48.1
>
>

Re: [PATCH 1/2] Remove the "mem=" parameter when using the --reuse-cmdline option in kdump operation

[Youling Tang]

Hi, Dave

On 9/25/25 17:11, Dave Young wrote:
> Hi Youling,
>
> On Thu, 25 Sept 2025 at 14:33, Youling Tang <youling.tang@linux.dev> wrote:
>> From: Youling Tang <tangyouling@kylinos.cn>
>>
>> During kdump operations, the capture kernel cannot reuse the "mem="
>> parameter from the production kernel. The "mem=" parameter is used
>> to specify the available memory range for the kernel. Reusing the
>> "mem=" memory range may destroy the production environment.
>>
>> Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
>> ---
>>   kexec/kexec.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/kexec/kexec.c b/kexec/kexec.c
>> index c9e4bcb..fadf986 100644
>> --- a/kexec/kexec.c
>> +++ b/kexec/kexec.c
>> @@ -1256,8 +1256,10 @@ char *get_command_line(void)
>>                  *p = '\0';
>>
>>          remove_parameter(line, "BOOT_IMAGE");
>> -       if (kexec_flags & KEXEC_ON_CRASH)
>> +       if (kexec_flags & KEXEC_ON_CRASH) {
>>                  remove_parameter(line, "crashkernel");
>> +               remove_parameter(line, "mem=");
>> +       }
> People can remove it from the kdump load scripts instead if it is not
> useful. I do not suggest removing other cmdline in c code.  The
> crashkernel is a special one which is fine.
Removing it in scripts is also acceptable. However, reusing the 
production kernel's
"mem=" parameter will inevitably damage the production environment. In 
my view,
modifying it in C code can achieve a permanent solution (avoiding the 
need to
modify kdump services for each distribution, and also applicable to 
non-kdump
services scenarios where commands are executed directly).

Thanks,
Youling.
>>          return line;
>>   }
>> --
>> 2.48.1
>>
>>
> Thanks
> Dave
>

Re: [PATCH 1/2] Remove the "mem=" parameter when using the --reuse-cmdline option in kdump operation

[Dave Young]

On Thu, 25 Sept 2025 at 17:26, Youling Tang <youling.tang@linux.dev> wrote:
>
> Hi, Dave
>
> On 9/25/25 17:11, Dave Young wrote:
> > Hi Youling,
> >
> > On Thu, 25 Sept 2025 at 14:33, Youling Tang <youling.tang@linux.dev> wrote:
> >> From: Youling Tang <tangyouling@kylinos.cn>
> >>
> >> During kdump operations, the capture kernel cannot reuse the "mem="
> >> parameter from the production kernel. The "mem=" parameter is used
> >> to specify the available memory range for the kernel. Reusing the
> >> "mem=" memory range may destroy the production environment.
> >>
> >> Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
> >> ---
> >>   kexec/kexec.c | 4 +++-
> >>   1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/kexec/kexec.c b/kexec/kexec.c
> >> index c9e4bcb..fadf986 100644
> >> --- a/kexec/kexec.c
> >> +++ b/kexec/kexec.c
> >> @@ -1256,8 +1256,10 @@ char *get_command_line(void)
> >>                  *p = '\0';
> >>
> >>          remove_parameter(line, "BOOT_IMAGE");
> >> -       if (kexec_flags & KEXEC_ON_CRASH)
> >> +       if (kexec_flags & KEXEC_ON_CRASH) {
> >>                  remove_parameter(line, "crashkernel");
> >> +               remove_parameter(line, "mem=");
> >> +       }
> > People can remove it from the kdump load scripts instead if it is not
> > useful. I do not suggest removing other cmdline in c code.  The
> > crashkernel is a special one which is fine.
> Removing it in scripts is also acceptable. However, reusing the
> production kernel's
> "mem=" parameter will inevitably damage the production environment. In
> my view,
> modifying it in C code can achieve a permanent solution (avoiding the
> need to
> modify kdump services for each distribution, and also applicable to
> non-kdump
> services scenarios where commands are executed directly).

But the param is a general one, not specific for kexec, people can use
it in their own way, eg. use less memory to boot a machine for debug
purposes.   This is why I suggest doing it in scripts and
distributions.

The two kdump related param, crashkernel and elfcorehdr are special anyway.

>
> Thanks,
> Youling.
> >>          return line;
> >>   }
> >> --
> >> 2.48.1
> >>
> >>
> > Thanks
> > Dave
> >
>

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

[Dave Young]

On Thu, 25 Sept 2025 at 17:52, Youling Tang <youling.tang@linux.dev> wrote:
>
> On 9/25/25 17:22, Dave Young wrote:
>
> On Thu, 25 Sept 2025 at 14:33, Youling Tang <youling.tang@linux.dev> wrote:
>
> From: Youling Tang <tangyouling@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@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@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@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@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

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

[Youling Tang]

Hi, Simon

Currently, it is passed through command-line parameters (fdt has not been
used yet), but the readability of the existing command-line parameters is
too poor. By rewriting it to be consistent with the kernel implementation
and using hexadecimal, the readability will be better.

Can Patch2 be applied alone (please ignore Patch1)?

Thanks,
Youling.
On 9/25/25 18:23, Dave Young wrote:
> On Thu, 25 Sept 2025 at 17:52, Youling Tang <youling.tang@linux.dev> wrote:
>> On 9/25/25 17:22, Dave Young wrote:
>>
>> On Thu, 25 Sept 2025 at 14:33, Youling Tang <youling.tang@linux.dev> wrote:
>>
>> From: Youling Tang <tangyouling@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@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@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@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@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
>