Re: [PATCH v4 03/13] hw/riscv/boot: Account for discontiguous memory when loading firmware

[Nicholas Piggin <npiggin@gmail.com>]

On Thu, Apr 30, 2026 at 09:34:24AM +1000, Alistair Francis wrote:
> On Sat, Apr 25, 2026 at 11:20 PM Joel Stanley <joel@jms.id.au> wrote:
> >
> > From: Nicholas Piggin <npiggin@gmail.com>
> >
> > This loads firmware into the first (low) memory range,
> > accounting for machines having discontiguous memory regions.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> > v4: Make RISCVBootInfo *info const in riscv_load_firmware
> > v3: Call riscv_boot_info_init before riscv_find_and_load_firmware in
> >     sifive_u
> > ---
> >  include/hw/riscv/boot.h    |  5 ++++-
> >  hw/riscv/boot.c            | 18 ++++++++++++------
> >  hw/riscv/microchip_pfsoc.c |  6 ++++--
> >  hw/riscv/opentitan.c       |  6 ++++--
> >  hw/riscv/shakti_c.c        |  6 +++++-
> >  hw/riscv/sifive_u.c        |  6 ++++--
> >  hw/riscv/spike.c           |  6 ++++--
> >  hw/riscv/virt.c            |  7 ++++---
> >  hw/riscv/xiangshan_kmh.c   |  6 +++++-
> >  9 files changed, 46 insertions(+), 20 deletions(-)
> >
> > diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> > index 115e3222174f..b2ef64fb1d14 100644
> > --- a/include/hw/riscv/boot.h
> > +++ b/include/hw/riscv/boot.h
> > @@ -53,13 +53,16 @@ void riscv_boot_info_init_discontig_mem(RISCVBootInfo *info,
> >  vaddr riscv_calc_kernel_start_addr(RISCVBootInfo *info,
> >                                     hwaddr firmware_end_addr);
> >  hwaddr riscv_find_and_load_firmware(MachineState *machine,
> > +                                    RISCVBootInfo *info,
> >                                      const char *default_machine_firmware,
> >                                      hwaddr *firmware_load_addr,
> >                                      symbol_fn_t sym_cb);
> >  const char *riscv_default_firmware_name(RISCVHartArrayState *harts);
> >  char *riscv_find_firmware(const char *firmware_filename,
> >                            const char *default_machine_firmware);
> > -hwaddr riscv_load_firmware(const char *firmware_filename,
> > +hwaddr riscv_load_firmware(MachineState *machine,
> > +                           const RISCVBootInfo *info,
> > +                           const char *firmware_filename,
> >                             hwaddr *firmware_load_addr,
> >                             symbol_fn_t sym_cb);
> >  void riscv_load_kernel(MachineState *machine,
> > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> > index 5c9547429a36..4fbc778263cf 100644
> > --- a/hw/riscv/boot.c
> > +++ b/hw/riscv/boot.c
> > @@ -145,6 +145,7 @@ char *riscv_find_firmware(const char *firmware_filename,
> >  }
> >
> >  hwaddr riscv_find_and_load_firmware(MachineState *machine,
> > +                                    RISCVBootInfo *info,
> >                                      const char *default_machine_firmware,
> >                                      hwaddr *firmware_load_addr,
> >                                      symbol_fn_t sym_cb)
> > @@ -157,7 +158,8 @@ hwaddr riscv_find_and_load_firmware(MachineState *machine,
> >
> >      if (firmware_filename) {
> >          /* If not "none" load the firmware */
> > -        firmware_end_addr = riscv_load_firmware(firmware_filename,
> > +        firmware_end_addr = riscv_load_firmware(machine, info,
> > +                                                firmware_filename,
> >                                                  firmware_load_addr, sym_cb);
> >          g_free(firmware_filename);
> >      }
> > @@ -165,10 +167,13 @@ hwaddr riscv_find_and_load_firmware(MachineState *machine,
> >      return firmware_end_addr;
> >  }
> >
> > -hwaddr riscv_load_firmware(const char *firmware_filename,
> > +hwaddr riscv_load_firmware(MachineState *machine,
> > +                           const RISCVBootInfo *info,
> > +                           const char *firmware_filename,
> >                             hwaddr *firmware_load_addr,
> >                             symbol_fn_t sym_cb)
> >  {
> > +    uint64_t mem_size = info->ram_low_size ?: machine->ram_size;
> >      uint64_t firmware_entry, firmware_end;
> >      ssize_t firmware_size;
> >
> > @@ -197,7 +202,7 @@ hwaddr riscv_load_firmware(const char *firmware_filename,
> >
> >      firmware_size = load_image_targphys_as(firmware_filename,
> >                                             *firmware_load_addr,
> > -                                           current_machine->ram_size, NULL,
> > +                                           mem_size, NULL,
> >                                             NULL);
> >
> >      if (firmware_size > 0) {
> > @@ -212,7 +217,7 @@ hwaddr riscv_load_firmware(const char *firmware_filename,
> >  static void riscv_load_initrd(MachineState *machine, RISCVBootInfo *info)
> >  {
> >      const char *filename = machine->initrd_filename;
> > -    uint64_t mem_size = machine->ram_size;
> > +    uint64_t mem_size = info->ram_low_size ?: machine->ram_size;
> >      void *fdt = machine->fdt;
> >      hwaddr start, end;
> >      ssize_t size;
> > @@ -258,6 +263,7 @@ void riscv_load_kernel(MachineState *machine,
> >                         bool load_initrd,
> >                         symbol_fn_t sym_cb)
> >  {
> > +    uint64_t mem_size = info->ram_low_size ?: machine->ram_size;
> >      const char *kernel_filename = machine->kernel_filename;
> >      ssize_t kernel_size;
> >      void *fdt = machine->fdt;
> > @@ -289,7 +295,7 @@ void riscv_load_kernel(MachineState *machine,
> >      }
> >
> >      kernel_size = load_image_targphys_as(kernel_filename, kernel_start_addr,
> > -                                         current_machine->ram_size, NULL, NULL);
> > +                                         mem_size, NULL, NULL);
> >      if (kernel_size > 0) {
> >          info->kernel_size = kernel_size;
> >          info->image_low_addr = kernel_start_addr;
> > @@ -385,7 +391,7 @@ uint64_t riscv_compute_fdt_addr(hwaddr dram_base, hwaddr dram_size,
> >      dtb_start = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
> >
> >      if (dtb_start_limit && (dtb_start < dtb_start_limit)) {
> > -        error_report("No enough memory to place DTB after kernel/initrd");
> > +        error_report("Not enough memory to place DTB after kernel/initrd");
> >          exit(1);
> >      }
> >
> > diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> > index 743f31f00578..1d1ddb05a882 100644
> > --- a/hw/riscv/microchip_pfsoc.c
> > +++ b/hw/riscv/microchip_pfsoc.c
> > @@ -618,18 +618,20 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
> >          firmware_load_addr = RESET_VECTOR;
> >      }
> >
> > +    riscv_boot_info_init(&boot_info, &s->soc.u_cpus);
> 
> We should be able to use the new riscv_boot_info_init_discontig_mem() here

Good catch, something like this.

    riscv_boot_info_init_discontig_mem(&boot_info, &s->soc.u_cpus,
                                       memmap[MICROCHIP_PFSOC_DRAM_LO].base,
                                       mem_low_size);

I'll change that.

Thanks,
Nick