All of 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 v2 2/5] LoongArch: Add kexec_file support
Date: Fri, 22 Aug 2025 04:19:09 +0000	[thread overview]
Message-ID: <aKfvvYVyBEkrDp9I@pie> (raw)
In-Reply-To: <a15ad5bd-f54d-466c-8bdd-6f5b5936abee@linux.dev>

On Fri, Aug 22, 2025 at 10:56:18AM +0800, Youling Tang wrote:
> On 2025/8/20 17:13, Youling Tang wrote:
> 
> > Hi, Yao
> > 
> > On 2025/8/20 14:50, Yao Zi wrote:
> > 
> > > On Wed, Aug 20, 2025 at 01:56:57PM +0800, Youling Tang wrote:
> > > > From: Youling Tang <tangyouling@kylinos.cn>
> > > > 
> > > > This patch adds support for kexec_file on LoongArch.
> > > > 
> > > > The efi_kexec_load() as two parts:
> > > > - the first part loads the kernel image (vmlinuz.efi or vmlinux.efi)
> > > > - the second part loads other segments (eg: initrd, cmdline)
> > > > 
> > > > This initrd will be passed to the second kernel via the command line
> > > > 'initrd=start,size'.
> > > > 
> > > > Currently, pez(vmlinuz.efi) and pei(vmlinux.efi) format images
> > > > are supported,
> > > > but ELF format is not supported.
> > > > 
> > > > Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
> > > > ---
> > > >   arch/loongarch/Kconfig                     |   9 ++
> > > >   arch/loongarch/include/asm/image.h         |  17 +++
> > > >   arch/loongarch/include/asm/kexec.h         |  12 +++
> > > >   arch/loongarch/kernel/Makefile             |   1 +
> > > >   arch/loongarch/kernel/kexec_efi.c          | 111 +++++++++++++++++++
> > > >   arch/loongarch/kernel/machine_kexec.c      |  33 ++++--
> > > >   arch/loongarch/kernel/machine_kexec_file.c | 117
> > > > +++++++++++++++++++++
> > > >   7 files changed, 289 insertions(+), 11 deletions(-)
> > > >   create mode 100644 arch/loongarch/kernel/kexec_efi.c
> > > >   create mode 100644 arch/loongarch/kernel/machine_kexec_file.c
> > > ...
> > > 
> > > > diff --git a/arch/loongarch/include/asm/image.h
> > > > b/arch/loongarch/include/asm/image.h
> > > > index 1f090736e71d..655d5836c4e8 100644
> > > > --- a/arch/loongarch/include/asm/image.h
> > > > +++ b/arch/loongarch/include/asm/image.h
> > > > @@ -36,5 +36,22 @@ struct loongarch_image_header {
> > > >       uint32_t pe_header;
> > > >   };
> > > >   +static const uint8_t loongarch_image_pe_sig[2] = {'M', 'Z'};
> > > > +
> > > > +/**
> > > > + * loongarch_header_check_pe_sig - Helper to check the
> > > > loongarch image header.
> > > > + *
> > > > + * Returns non-zero if 'MZ' signature is found.
> > > > + */
> > > > +
> > > > +static inline int loongarch_header_check_pe_sig(const struct
> > > > loongarch_image_header *h)
> > > > +{
> > > > +    if (!h)
> > > > +        return 0;
> > > > +
> > > > +    return (h->pe_sig[0] == loongarch_image_pe_sig[0]
> > > > +        && h->pe_sig[1] == loongarch_image_pe_sig[1]);
> > > > +}
> > > This check is still too weak and doesn't improve comparing to v1.
> > > 
> > > > This could be simplified with a memcmp(). Also, this check isn't
> > > > strict enough: PE files for any architectures, and even legacy MS-DOS
> > > > COM executables all start with "MZ".
> > > I've pointed this out in my previous reply[1].
> > Previously, I had considered adding a specific LoongArch magic
> > number (such as "Loongson") in the loongarch_image_header, but
> > this is incompatible with older versions of the kernel, so it
> > remains the same without further checks.
> > > 
> > > >   #endif /* __ASSEMBLY__ */
> > > >   #endif /* __ASM_IMAGE_H */
> > > ...
> > > 
> > > > diff --git a/arch/loongarch/kernel/kexec_efi.c
> > > > b/arch/loongarch/kernel/kexec_efi.c
> > > > new file mode 100644
> > > > index 000000000000..7741f1139a12
> > > > --- /dev/null
> > > > +++ b/arch/loongarch/kernel/kexec_efi.c
> > > ...
> > > 
> > > > +static void *efi_kexec_load(struct kimage *image,
> > > > +                char *kernel, unsigned long kernel_len,
> > > > +                char *initrd, unsigned long initrd_len,
> > > > +                char *cmdline, unsigned long cmdline_len)
> > > > +{
> > > > +    struct loongarch_image_header *h;
> > > > +    struct kexec_buf kbuf;
> > > > +    unsigned long text_offset, kernel_segment_number;
> > > > +    struct kexec_segment *kernel_segment;
> > > > +    int ret;
> > > > +
> > > > +    h = (struct loongarch_image_header *)kernel;
> > > > +    if (!h->image_size)
> > > > +        return ERR_PTR(-EINVAL);
> > > > +
> > > > +    /* Load the kernel */
> > > > +    kbuf.image = image;
> > > > +    kbuf.buf_max = ULONG_MAX;
> > > > +    kbuf.top_down = false;
> > > > +
> > > > +    kbuf.buffer = kernel;
> > > > +    kbuf.bufsz = kernel_len;
> > > > +    kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> > > > +    kbuf.memsz = le64_to_cpu(h->image_size);
> > > > +    text_offset = le64_to_cpu(h->text_offset);
> > > > +    kbuf.buf_min = text_offset;
> > > > +    kbuf.buf_align = SZ_2M;
> > > > +
> > > > +    kernel_segment_number = image->nr_segments;
> > > > +
> > > > +    /*
> > > > +     * The location of the kernel segment may make it
> > > > impossible to satisfy
> > > > +     * the other segment requirements, so we try repeatedly to find a
> > > > +     * location that will work.
> > > > +     */
> > > > +    while ((ret = kexec_add_buffer(&kbuf)) == 0) {
> > > > +        /* Try to load additional data */
> > > > +        kernel_segment = &image->segment[kernel_segment_number];
> > > > +        ret = load_other_segments(image, kernel_segment->mem,
> > > > +                      kernel_segment->memsz, initrd,
> > > > +                      initrd_len, cmdline, cmdline_len);
> > > > +        if (!ret)
> > > > +            break;
> > > > +
> > > > +        /*
> > > > +         * We couldn't find space for the other segments; erase the
> > > > +         * kernel segment and try the next available hole.
> > > > +         */
> > > > +        image->nr_segments -= 1;
> > > > +        kbuf.buf_min = kernel_segment->mem + kernel_segment->memsz;
> > > > +        kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> > > > +    }
> > > > +
> > > > +    if (ret) {
> > > > +        pr_err("Could not find any suitable kernel location!");
> > > > +        return ERR_PTR(ret);
> > > > +    }
> > > > +
> > > > +    kernel_segment = &image->segment[kernel_segment_number];
> > > > +
> > > > +    /* Make sure the second kernel jumps to the correct
> > > > "kernel_entry". */
> > > > +    image->start = kernel_segment->mem + h->kernel_entry -
> > > > text_offset;
> > > And this still assumes the loaded, secondary kernel is relocatable,
> > > with neither extra check nor comment explaining its limitation.
> > > 
> > > Please see my previous reply[2] that explains why loading a
> > > non-relocatble kernel with kexec_file API is reasonable.
> > LoongArch is a non-position independent (non-PIE) kernel when
> > the RELOCATABLE option is not enabled, the kernel contains certain
> > instructions such as la.abs, which prevent it from being relocated to
> > arbitrary memory addresses for execution. As a result, limitations
> > exist that make features like kdump or kexec_file dependent on
> > the RELOCATABLE option.
> > 
> > Strictly speaking, we need to add additional checks: if the kernel is
> > non-relocatable, the loading operation should fail directly. For a
> > running kernel, we can easily determine this by calling
> > kallsyms_lookup_name("relocate_kernel"). However, for a kernel
> > that is being loaded but has not yet started execution, it is difficult
> > to easily determine whether the currently loaded kernel has the
> > RELOCATABLE configuration option enabled.
> > 
> > For ELF format images, we can determine whether the loaded image
> > contains the ".la_abs" section in the following way:
> > static struct mem_shdr *laabs_section(const struct mem_ehdr *ehdr)
> > {
> >         struct mem_shdr *shdr, *shdr_end;
> >         unsigned char *strtab;
> > 
> >         strtab = (unsigned char *)ehdr->e_shdr[ehdr->e_shstrndx].sh_data;
> >         shdr_end = &ehdr->e_shdr[ehdr->e_shnum];
> >         for (shdr = ehdr->e_shdr; shdr != shdr_end; shdr++) {
> >                 if (shdr->sh_size &&
> >                         strcmp((char *)&strtab[shdr->sh_name],
> > ".la_abs") == 0) {
> >                         return shdr;
> >                 }
> >         }
> > 
> >         return NULL;
> > }
> I attempted to parse the pe header to obtain the sections information
> and found that there were only two sections, '.text' and '.data'. We
> cannot parse whether there is a '.la_abs' section like in the ELF format.

I think it's fine to just leave a comment indicating this doesn't work
with non-relocatable kernels for now.

Best regards,
Yao Zi

> The reason is that when generating vmlinux.efi, when the ELF vmlinux
> is converted to the original binary file through the 'objdump -O binary'
> operation (arch/loongarch/boot/Makefile), the remaining sections are
> merged into the '.text' and '.data' sections.
> 
> Youling.
> > 
> > Thanks,
> > Youling.
> > > 
> > > > +    kexec_dprintk("Loaded kernel at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> > > > +              kernel_segment->mem, kbuf.bufsz,
> > > > +              kernel_segment->memsz);
> > > > +
> > > > +    return NULL;
> > > > +}
> > > > +
> > > > +const struct kexec_file_ops kexec_efi_ops = {
> > > > +    .probe = efi_kexec_probe,
> > > > +    .load = efi_kexec_load,
> > > > +};
> > > Thanks,
> > > Yao Zi
> > > 
> > > [1]: https://lore.kernel.org/all/aJojDiHWi8cgvA2W@pie/
> > > [2]: https://lore.kernel.org/all/aJwFa8x5BQMouB1y@pie/
> 


  reply	other threads:[~2025-08-22 12:49 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-20  5:56 [PATCH v2 0/5] Add kexec_file support for LoongArch Youling Tang
2025-08-20  5:56 ` [PATCH v2 1/5] LoongArch: Add struct loongarch_image_header for kernel image Youling Tang
2025-08-20  5:56 ` [PATCH v2 2/5] LoongArch: Add kexec_file support Youling Tang
2025-08-20  6:50   ` Yao Zi
2025-08-20  9:13     ` Youling Tang
2025-08-20 11:13       ` Yao Zi
2025-08-21  1:19         ` Youling Tang
2025-08-22  4:24           ` Huacai Chen
2025-08-22  2:56       ` Youling Tang
2025-08-22  4:19         ` Yao Zi [this message]
2025-08-22  6:40   ` Huacai Chen
2025-08-26  1:55   ` kernel test robot
2025-08-20  5:56 ` [PATCH v2 3/5] LoongArch/kexec_file: Support loading ELF binary file Youling Tang
2025-08-21  7:14   ` Youling Tang
2025-08-20  5:56 ` [PATCH v2 4/5] LoongArch/kexec_file: Add crash dump support Youling Tang
2025-08-20  5:57 ` [PATCH v2 5/5] LoongArch: Enable CONFIG_KEXEC_FILE Youling Tang
2025-08-22  4:13 ` [PATCH v2 0/5] Add kexec_file support for LoongArch Huacai Chen

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=aKfvvYVyBEkrDp9I@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 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.