Re: [PATCH v2 14/16] board: emulation: Add qemu-loongarch

On Thu, Jul 02, 2026 at 11:27:21AM +0100, Simon Glass wrote:
> Hi Yao,
> 
> On 2026-07-01T11:17:53, Yao Zi <[email protected]> wrote:
> > board: emulation: Add qemu-loongarch
> >
> > Yet another generic QEMU VIRT machine.
> > QEMU placed FDT in memory before launching U-Boot, so we
> > can just obtian FDT here.
> >
> > Signed-off-by: Jiaxun Yang <[email protected]>
> > Signed-off-by: Yao Zi <[email protected]>
> >
> > arch/loongarch/Kconfig                            |  5 ++
> >  arch/loongarch/dts/qemu-loongarch64.dts           |  9 +++
> >  board/emulation/qemu-loongarch/Kconfig            | 68 +++++++++++++++++++++++
> >  board/emulation/qemu-loongarch/MAINTAINERS        |  7 +++
> >  board/emulation/qemu-loongarch/Makefile           |  6 ++
> >  board/emulation/qemu-loongarch/qemu-loongarch.c   | 67 ++++++++++++++++++++++
> >  board/emulation/qemu-loongarch/qemu-loongarch.env |  6 ++
> >  configs/qemu-loongarch64_defconfig                | 36 ++++++++++++
> >  include/configs/qemu-loongarch.h                  | 13 +++++
> >  9 files changed, 217 insertions(+)

...

> > diff --git a/board/emulation/qemu-loongarch/qemu-loongarch.c b/board/emulation/qemu-loongarch/qemu-loongarch.c
> > @@ -0,0 +1,67 @@
> > +phys_addr_t board_get_usable_ram_top(phys_size_t total_size)
> > +{
> > +     /* Limit RAM used by U-Boot to the DDR low region */
> > +     if (gd->ram_top > 0x10000000)
> > +             return 0x10000000;
> > +
> > +     return gd->ram_top;
> > +}
> 
> Please introduce a named constant for 0x10000000 (something like
> SYS_LOWMEM_TOP), and explain in the comment why the low region
> matters - TLB/direct-map restriction, or a QEMU quirk?

This is for keeping compatible with emulated PCI devices that aren't
able to do DMA above the lowest 4GiB RAM. I'll comment on this further.

> > diff --git a/board/emulation/qemu-loongarch/qemu-loongarch.c b/board/emulation/qemu-loongarch/qemu-loongarch.c
> > @@ -0,0 +1,67 @@
> > +int board_init(void)
> > +{
> > +     return 0;
> > +}
> 
> If there is nothing to do, please drop in favour of the weak default.
> 
> > diff --git a/board/emulation/qemu-loongarch/qemu-loongarch.c b/board/emulation/qemu-loongarch/qemu-loongarch.c
> > @@ -0,0 +1,67 @@
> > +int board_fdt_blob_setup(void **fdtp)
> > +{
> > +     /* Stored the DTB address there during our init */
> > +     *fdtp = (void *)(ulong)0x100000;
> > +     return 0;
> > +}
> 
> The comment is misleading - U-Boot does not store the DTB anywhere;
> QEMU (LoongArchQemuPkg) places it at this address. Please rework
> the comment, and use a named constant rather than 0x100000.

Okay.

> It
> would also be worth an fdt_check_header() before returning success,
> so a mis-launch fails cleanly.

However the only call-site of board_fdt_blob_setup(), fdtdec_setup(),
already does this check indirectly through fdtdec_prepare_fdt(), so
I think such a check would be redundant.

...

> > diff --git a/include/configs/qemu-loongarch.h b/include/configs/qemu-loongarch.h
> > @@ -0,0 +1,13 @@
> > +/* Those values are chosen by LoongArchQemuPkg */
> > +#define CFG_SYS_INIT_RAM_ADDR        0x10000
> > +#define CFG_SYS_INIT_RAM_SIZE        0x10000
> 
> 'These values', not 'Those'. Also, since both the DTB address
> (0x100000) and the low-RAM cap (0x10000000) are baked into the C
> file, please pull those into this header so the QEMU/edk2 ABI lives
> in one place.

To make it clear, we just use the same address ranges as EDK2 for RAM
during initialization, and this doesn't mean it's part of the ABI. I'll
probably switch to CUSTOM_SYS_INIT_SP_ADDR in the next version.

And the DTB address is determined by QEMU, not EDK2, thus I don't think
merging them together making anything clearer.

> Regards,
> Simon

Best regards,
Yao Zi