Re: [PULL v3 3/7] hw/loongarch: Load FDT table into dram memory space

[gaosong]

在 2022/11/8 下午6:41, Richard Henderson 写道:
> On 11/5/22 14:28, Song Gao wrote:
> > From: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> >
> > Load FDT table into dram memory space, and the addr is 2 MiB.
> > Since lowmem region starts from 0, FDT base address is located
> > at 2 MiB to avoid NULL pointer access.
> >
> > Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> > Acked-by: Song Gao <gaosong@loongson.cn>
> > Message-Id: <20221028014007.2718352-2-yangxiaojuan@loongson.cn>
> > Signed-off-by: Song Gao <gaosong@loongson.cn>
> > ---
> >   hw/loongarch/virt.c         | 18 +++++++++++-------
> >   include/hw/loongarch/virt.h |  3 ---
> >   2 files changed, 11 insertions(+), 10 deletions(-)
>
> This breaks make check-tcg:
>
>   TEST    hello on loongarch64
> qemu-system-loongarch64: Some ROM regions are overlapping
> These ROM regions might have been loaded by direct user request or by 
> default.
> They could be BIOS/firmware images, a guest kernel, initrd or some 
> other file loaded into guest memory.
> Check whether you intended to load all this guest code, and whether it 
> has been built to load to the correct addresses.
>
> The following two regions overlap (in the memory address space):
>   hello ELF program header segment 0 (addresses 0x0000000000200000 - 
> 0x0000000000242000)
>   fdt (addresses 0x0000000000200000 - 0x0000000000300000)
> make[1]: *** [Makefile:177: run-hello] Error 1
Thank you,  I had send a patch to fix this.

Thanks.
Song Gao
> > diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> > index 4b595a9ea4..50e9829a94 100644
> > --- a/hw/loongarch/virt.c
> > +++ b/hw/loongarch/virt.c
> > @@ -159,7 +159,6 @@ static void fdt_add_pcie_node(const 
> > LoongArchMachineState *lams)
> >                                    1, FDT_PCI_RANGE_MMIO, 2, base_mmio,
> >                                    2, base_mmio, 2, size_mmio);
> >       g_free(nodename);
> > -    qemu_fdt_dumpdtb(ms->fdt, lams->fdt_size);
> >   }
> >     static void fdt_add_irqchip_node(LoongArchMachineState *lams)
> > @@ -656,6 +655,7 @@ static void loongarch_init(MachineState *machine)
> >       MemoryRegion *address_space_mem = get_system_memory();
> >       LoongArchMachineState *lams = LOONGARCH_MACHINE(machine);
> >       int i;
> > +    hwaddr fdt_base;
> >         if (!cpu_model) {
> >           cpu_model = LOONGARCH_CPU_TYPE_NAME("la464");
> > @@ -760,12 +760,16 @@ static void loongarch_init(MachineState *machine)
> >       lams->machine_done.notify = virt_machine_done;
> > qemu_add_machine_init_done_notifier(&lams->machine_done);
> >       fdt_add_pcie_node(lams);
> > -
> > -    /* load fdt */
> > -    MemoryRegion *fdt_rom = g_new(MemoryRegion, 1);
> > -    memory_region_init_rom(fdt_rom, NULL, "fdt", VIRT_FDT_SIZE, 
> > &error_fatal);
> > -    memory_region_add_subregion(get_system_memory(), VIRT_FDT_BASE, 
> > fdt_rom);
> > -    rom_add_blob_fixed("fdt", machine->fdt, lams->fdt_size, 
> > VIRT_FDT_BASE);
> > +    /*
> > +     * Since lowmem region starts from 0, FDT base address is located
> > +     * at 2 MiB to avoid NULL pointer access.
> > +     *
> > +     * Put the FDT into the memory map as a ROM image: this will ensure
> > +     * the FDT is copied again upon reset, even if addr points into 
> > RAM.
> > +     */
> > +    fdt_base = 2 * MiB;
> > +    qemu_fdt_dumpdtb(machine->fdt, lams->fdt_size);
> > +    rom_add_blob_fixed("fdt", machine->fdt, lams->fdt_size, fdt_base);
> >   }
> >     bool loongarch_is_acpi_enabled(LoongArchMachineState *lams)
> > diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
> > index 09f1c88ee5..45c383f5a7 100644
> > --- a/include/hw/loongarch/virt.h
> > +++ b/include/hw/loongarch/virt.h
> > @@ -28,9 +28,6 @@
> >   #define VIRT_GED_MEM_ADDR       (VIRT_GED_EVT_ADDR + 
> > ACPI_GED_EVT_SEL_LEN)
> >   #define VIRT_GED_REG_ADDR       (VIRT_GED_MEM_ADDR + 
> > MEMORY_HOTPLUG_IO_LEN)
> >   -#define VIRT_FDT_BASE           0x1c400000
> > -#define VIRT_FDT_SIZE           0x100000
> > -
> >   struct LoongArchMachineState {
> >       /*< private >*/
> >       MachineState parent_obj;