Kexec Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Yao Zi <ziyao@disroot.org>
To: Youling Tang <youling.tang@linux.dev>,
	Huacai Chen <chenhuacai@kernel.org>
Cc: WANG Xuerui <kernel@xen0n.name>, Baoquan He <bhe@redhat.com>,
	kexec@lists.infradead.org, loongarch@lists.linux.dev,
	linux-kernel@vger.kernel.org,
	Youling Tang <tangyouling@kylinos.cn>
Subject: Re: [PATCH 3/6] LoongArch/kexec_file: Add initrd loading
Date: Tue, 12 Aug 2025 06:25:57 +0000	[thread overview]
Message-ID: <aJredf2InXhHY7ek@pie> (raw)
In-Reply-To: <0767b8fe-7c04-4e73-9235-ee326ee058cc@linux.dev>

On Tue, Aug 12, 2025 at 12:05:30PM +0800, Youling Tang wrote:
> Hi, Yao
> On 2025/8/12 01:58, Yao Zi wrote:
> > On Mon, Aug 11, 2025 at 05:26:56PM +0800, Youling Tang wrote:
> > > From: Youling Tang <tangyouling@kylinos.cn>
> > > 
> > > Add inird loading support and pass it to the second kernel via the
> > > cmdline 'initrd=start,size'.
> > I think This won't work if the exec'ed kernel enables
> > CONFIG_CMDLINE_FORCE. Is it possible to mimic libstub's behavior of
> > installing a configuration table LINUX_EFI_INITRD_MEDIA_GUID?
> The command line passed by kexec to the second kernel has no effect if
> CONFIG_CMDLINE_FORCE is enabled, which is not quite suitable for the
> kexec scenario.
> 
> Currently, the initrd, elfcorehdr, and mem parameters will all be passed
> through the command line to maintain consistency with the implementation
> behavior of kexec-tools. It is possible that the content of systab will
> be modified in the future and some parts will be integrated into systab
> (the current cmdline mode will be better compatible with the elf kernel).
> > 
> > > Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
> > > ---
> > >   arch/loongarch/kernel/machine_kexec_file.c | 71 ++++++++++++++++++++++
> > >   1 file changed, 71 insertions(+)
> > > 
> > > diff --git a/arch/loongarch/kernel/machine_kexec_file.c b/arch/loongarch/kernel/machine_kexec_file.c
> > > index bc91ae0afa4c..e1240644f529 100644
> > > --- a/arch/loongarch/kernel/machine_kexec_file.c
> > > +++ b/arch/loongarch/kernel/machine_kexec_file.c
> > > @@ -34,13 +34,84 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
> > >   	return kexec_image_post_load_cleanup_default(image);
> > >   }
> > > +/* Adds the "initrd=start,size" command line parameter to command line. */
> > > +static void cmdline_add_initrd(struct kimage *image, unsigned long *cmdline_tmplen,
> > > +				char *modified_cmdline, unsigned long initrd)
> > > +{
> > > +	int initrd_strlen;
> > > +
> > > +	initrd_strlen = sprintf(modified_cmdline + (*cmdline_tmplen), "initrd=0x%lx,0x%lx ",
> > modified_cmdline is allocated as COMMAND_LINE_SIZE bytes, thus I think
> > it's possible to overflow the buffer.
> At this point, modified_cmdline can clearly know that it only stores
> the additional commands we add (initrd,mem,elfcorehdr), and will not
> exceed COMMAND_LINE_SIZE.
> > 
> > > +		initrd, image->initrd_buf_len);
> > > +	*cmdline_tmplen += initrd_strlen;
> > > +}
> > > +
> > > +/*
> > > + * Tries to add the initrd to the image. If it is not possible to find
> > > + * valid locations, this function will undo changes to the image and return non
> > > + * zero.
> > > + */
> > >   int load_other_segments(struct kimage *image,
> > >   			unsigned long kernel_load_addr,
> > >   			unsigned long kernel_size,
> > >   			char *initrd, unsigned long initrd_len,
> > >   			char *cmdline, unsigned long cmdline_len)
> > >   {
> > > +	struct kexec_buf kbuf;
> > > +	unsigned long orig_segments = image->nr_segments;
> > > +	char *modified_cmdline = NULL;
> > > +	unsigned long cmdline_tmplen = 0;
> > > +	unsigned long initrd_load_addr = 0;
> > > +	int ret = 0;
> > > +
> > > +
> > > +	kbuf.image = image;
> > > +	/* not allocate anything below the kernel */
> > > +	kbuf.buf_min = kernel_load_addr + kernel_size;
> > > +
> > > +	modified_cmdline = kzalloc(COMMAND_LINE_SIZE, GFP_KERNEL);
> > > +	if (!modified_cmdline)
> > > +		return -EINVAL;
> > > +
> > > +	/* Ensure it's nul terminated */
> > > +	modified_cmdline[COMMAND_LINE_SIZE - 1] = '\0';
> > > +
> > > +	/* load initrd */
> > > +	if (initrd) {
> > > +		kbuf.buffer = initrd;
> > > +		kbuf.bufsz = initrd_len;
> > > +		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> > > +		kbuf.memsz = initrd_len;
> > > +		kbuf.buf_align = 0;
> > > +		/* within 1GB-aligned window of up to 32GB in size */
> > > +		kbuf.buf_max = round_down(kernel_load_addr, SZ_1G)
> > > +						+ (unsigned long)SZ_1G * 32;
> > > +		kbuf.top_down = false;
> > > +
> > > +		ret = kexec_add_buffer(&kbuf);
> > > +		if (ret)
> > > +			goto out_err;
> > > +		initrd_load_addr = kbuf.mem;
> > > +
> > > +		kexec_dprintk("Loaded initrd at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> > > +			      initrd_load_addr, kbuf.bufsz, kbuf.memsz);
> > > +
> > > +		/* Add the initrd=start,size parameter to the command line */
> > > +		cmdline_add_initrd(image, &cmdline_tmplen, modified_cmdline, initrd_load_addr);
> > > +	}
> > > +
> > > +	if (cmdline_len + cmdline_tmplen > COMMAND_LINE_SIZE) {
> > It's too later to check for overflowing here, where the data after
> > modified_cmdline may already be overwritten.
> At this point, we append the original command line to modified_cmdline,
> so it is appropriate to determine whether the command line length exceeds
> the limit.

Thanks, I misunderstood the code logic: before cmdline_add_initrd
appends new arguments, modified_cmdline is empty instead of containing
a copy of cmdline, so this is fine. Sorry for the noise.

Best regards,
Yao Zi


  reply	other threads:[~2025-08-12  6:26 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-11  9:26 [PATCH 0/6] Add kexec_file support for LoongArch Youling Tang
2025-08-11  9:26 ` [PATCH 1/6] LoongArch: Add struct loongarch_image_header for kernel image Youling Tang
2025-08-11  9:26 ` [PATCH 2/6] LoongArch: Add kexec_file support Youling Tang
2025-08-11 14:07   ` Huacai Chen
2025-08-12  1:21     ` Youling Tang
2025-08-12  1:53       ` Yanteng Si
2025-08-12  9:32         ` Youling Tang
2025-08-13  1:15           ` Yanteng Si
2025-08-12  2:53       ` Huacai Chen
2025-08-11 17:06   ` Yao Zi
2025-08-12  2:39     ` Huacai Chen
2025-08-12  7:56       ` Yao Zi
2025-08-12  6:15     ` Youling Tang
2025-08-12  7:06       ` Youling Tang
2025-08-12  9:43         ` Yao Zi
2025-08-13  2:18           ` Youling Tang
2025-08-13  3:24             ` Yao Zi
2025-08-16  5:37   ` kernel test robot
2025-08-11  9:26 ` [PATCH 3/6] LoongArch/kexec_file: Add initrd loading Youling Tang
2025-08-11 14:12   ` Huacai Chen
2025-08-12  2:38     ` Youling Tang
2025-08-12  3:03       ` Huacai Chen
2025-08-11 17:58   ` Yao Zi
2025-08-12  4:05     ` Youling Tang
2025-08-12  6:25       ` Yao Zi [this message]
2025-08-11  9:26 ` [PATCH 4/6] LoongArch/kexec_file: Add crash dump support Youling Tang
2025-08-11  9:26 ` [PATCH 5/6] LoongArch/kexec_file: Add "mem" parameter to limit memory usage of kdump kernel Youling Tang
2025-08-11  9:26 ` [PATCH 6/6] LoongArch: Enable CONFIG_KEXEC_FILE Youling Tang
2025-08-11 16:20 ` [PATCH 0/6] Add kexec_file support for LoongArch Vincent Li
2025-08-12  6:21   ` Youling Tang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aJredf2InXhHY7ek@pie \
    --to=ziyao@disroot.org \
    --cc=bhe@redhat.com \
    --cc=chenhuacai@kernel.org \
    --cc=kernel@xen0n.name \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=loongarch@lists.linux.dev \
    --cc=tangyouling@kylinos.cn \
    --cc=youling.tang@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox