[PATCH v2 10/16] LoongArch: Boot Image bits

[Simon Glass]

Hi Yao,

On 2026-07-01T11:17:53, Yao Zi <me at ziyao.cc> wrote:
> LoongArch: Boot Image bits
>
> Implement loading and booting functions for LoongArch
> standard kernel image as per spec.
>
> LoongArch kernel do expect us to fake a efi systemtable
> for passing fdt to kernel, we don't need to implement any
> EFI functions for kernel because it won't look into anything
> beside devicetree from that table if we tell kernel we are
> not efi compatible by setting a0 boot argument to zero.
>
> Link: https://docs.kernel.org/arch/loongarch/booting.html
> Signed-off-by: Jiaxun Yang <jiaxun.yang at flygoat.com>
> Signed-off-by: Yao Zi <me at ziyao.cc>
>
> arch/loongarch/lib/Makefile |   2 +
>  arch/loongarch/lib/bootm.c  | 163 ++++++++++++++++++++++++++++++++++++++++++++
>  arch/loongarch/lib/image.c  |  66 ++++++++++++++++++
>  cmd/Kconfig                 |   2 +-
>  4 files changed, 232 insertions(+), 1 deletion(-)

> diff --git a/arch/loongarch/lib/bootm.c b/arch/loongarch/lib/bootm.c
> @@ -0,0 +1,163 @@
> +#include <bootstage.h>
> +#include <bootm.h>
> +#include <command.h>
> +#include <dm.h>
> +#include <efi.h>
> +#include <efi_api.h>
> +#include <fdt_support.h>
> +#include <hang.h>
> +#include <log.h>
> +#include <linux/sizes.h>
> +#include <memalign.h>
> +#include <asm/global_data.h>
> +#include <dm/root.h>
> +#include <image.h>
> +#include <asm/byteorder.h>
> +#include <dm/device.h>
> +#include <dm/root.h>
> +#include <u-boot/zlib.h>

dm/root.h is listed twice - please drop the duplicate, and sort the
block while you are here.

https://docs.u-boot.org/en/latest/develop/codingstyle.html#include-files

> diff --git a/arch/loongarch/lib/bootm.c b/arch/loongarch/lib/bootm.c
> @@ -0,0 +1,163 @@
> +/* LoongArch do expect a EFI style systab */
> +static int generate_systab(struct bootm_headers *images)
> +{
> +     const int nr_cfgtab = 1;
> +     struct bd_info *kbd = images->kbd;
> +     struct efi_system_table *systab;
> +     struct efi_configuration_table *cfgtab;
> +     size_t table_size = sizeof(struct efi_system_table) +
> +                         nr_cfgtab * sizeof(struct efi_configuration_table);
> +
> +     systab = memalign(SZ_64K, table_size);

Please add a comment on why 64K alignment is required.

> diff --git a/arch/loongarch/lib/bootm.c b/arch/loongarch/lib/bootm.c
> @@ -0,0 +1,163 @@
> +static void boot_jump_linux(struct bootm_headers *images, int flag)
> +{
> +     void (*kernel)(ulong efi_boot, char *argc, void *dtb);
> +     int fake = (flag & BOOTM_STATE_OS_FAKE_GO);
> +
> +     kernel = (void (*)(ulong efi_boot, char *argc, void *dtb))images->ep;

The second argument is the kernel command line, not argc - please
rename to cmdline. The third is the fake systab pointer, not a raw
dtb; please rename that too.

> diff --git a/arch/loongarch/lib/bootm.c b/arch/loongarch/lib/bootm.c
> @@ -0,0 +1,163 @@
> +     if (!fake) {
> +             if (CONFIG_IS_ENABLED(OF_LIBFDT) && images->ft_len)
> +                     kernel(0, NULL, (void *)images->kbd->bi_boot_params);
> +     }

Per the LoongArch booting spec, a1 is the kernel command line.
Passing NULL means the bootargs env var never reaches the kernel
via that path - it will only be seen in /chosen/bootargs

So don't you wan to pass images->cmdline_start here?

Also, if OF_LIBFDT is disabled or ft_len is zero we drop through
without jumping *after* announce_and_cleanup() has torn devices
down. Please panic() / hang() in that case rather than silently return.

> diff --git a/arch/loongarch/lib/image.c b/arch/loongarch/lib/image.c
> @@ -0,0 +1,66 @@
> +struct linux_image_h {
> +     uint32_t        code0;                  /* Executable code */
> +     uint32_t        code1;                  /* Executable code */
> +     uint64_t        kernel_entry;   /* Kernel entry point */
> +     uint64_t        image_size;             /* Effective Image size */
> +     uint64_t        load_offset;    /* load offset */
> +     uint64_t        res1;                   /* reserved */
> +     uint64_t        res2;                   /* reserved */
> +     uint64_t        res3;                   /* reserved */
> +     uint32_t        magic;                  /* Magic number */
> +     uint32_t        pe_header;              /* Offset to the PE header */
> +};

Comment tabs after kernel_entry/image_size/load_offset are
inconsistent with the first two members - please tidy so the
comments line up. Also use u32/u64 for consistency with the rest
of U-Boot.

I have to wonder whether it would be OK to share this struct with
RISC-V in a header file?

> diff --git a/arch/loongarch/lib/image.c b/arch/loongarch/lib/image.c
> @@ -0,0 +1,66 @@
> +     /* To workaround kernel supplying DMW based virtual address */
> +     ep_phys = TO_PHYS(lhdr->kernel_entry);
> +     *ep = *relocated_addr + (ep_phys - lhdr->load_offset);

Please expand this comment - from context it looks like kernel_entry
is a DMW virtual address, and you are converting to a physical
offset within the image and re-basing onto the relocated load
address. Spell that out, with a pointer to the relevant paragraph
of the booting spec.

Regards,
Simon