| Message ID | 20230111170948.316276-8-dbarboza@ventanamicro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: create_fdt() related cleanups | expand |
On 11/1/23 18:09, Daniel Henrique Barboza wrote: > All callers of riscv_load_fdt() are using machine->ram_size as > 'mem_size' and the fdt is always retrievable via machine->fdt. > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > hw/riscv/boot.c | 4 +++- > hw/riscv/microchip_pfsoc.c | 4 ++-- > hw/riscv/sifive_u.c | 3 +-- > hw/riscv/spike.c | 3 +-- > hw/riscv/virt.c | 3 +-- > include/hw/riscv/boot.h | 2 +- > 6 files changed, 9 insertions(+), 10 deletions(-) > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > index e868fb6ade..21dea7eac2 100644 > --- a/hw/riscv/boot.c > +++ b/hw/riscv/boot.c > @@ -265,10 +265,12 @@ out: > return kernel_entry; > } > > -uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt) > +uint64_t riscv_load_fdt(MachineState *ms, hwaddr dram_base) > { > uint64_t temp, fdt_addr; > + uint64_t mem_size = ms->ram_size; > hwaddr dram_end = dram_base + mem_size; What about sparse memory ...? > if (fdtsize <= 0) { > diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c > index c45023a2b1..6bb08f66bd 100644 > --- a/hw/riscv/microchip_pfsoc.c > +++ b/hw/riscv/microchip_pfsoc.c > @@ -633,8 +633,8 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) > true, NULL); > > /* Compute the fdt load address in dram */ > - fdt_load_addr = riscv_load_fdt(memmap[MICROCHIP_PFSOC_DRAM_LO].base, > - machine->ram_size, machine->fdt); > + fdt_load_addr = riscv_load_fdt(machine, > + memmap[MICROCHIP_PFSOC_DRAM_LO].base); ... i.e: hw/riscv/microchip_pfsoc.c: [MICROCHIP_PFSOC_DRAM_LO] = { 0x80000000, 0x40000000 }, hw/riscv/microchip_pfsoc.c: [MICROCHIP_PFSOC_DRAM_HI] = { 0x1000000000, 0x0 }, IIUC FDT is always loaded into a contiguous region, so maybe simply 'dram_base' is a bad name for '[fdt_]load_address'? 'mem_size' is used to calculate 'dram_end'... This function seems buggy. We should pass either start_addr/end_addr or base/size, but the range passed must be contiguous.
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c index e868fb6ade..21dea7eac2 100644 --- a/hw/riscv/boot.c +++ b/hw/riscv/boot.c @@ -265,10 +265,12 @@ out: return kernel_entry; } -uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt) +uint64_t riscv_load_fdt(MachineState *ms, hwaddr dram_base) { uint64_t temp, fdt_addr; + uint64_t mem_size = ms->ram_size; hwaddr dram_end = dram_base + mem_size; + void *fdt = ms->fdt; int ret, fdtsize = fdt_totalsize(fdt); if (fdtsize <= 0) { diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c index c45023a2b1..6bb08f66bd 100644 --- a/hw/riscv/microchip_pfsoc.c +++ b/hw/riscv/microchip_pfsoc.c @@ -633,8 +633,8 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) true, NULL); /* Compute the fdt load address in dram */ - fdt_load_addr = riscv_load_fdt(memmap[MICROCHIP_PFSOC_DRAM_LO].base, - machine->ram_size, machine->fdt); + fdt_load_addr = riscv_load_fdt(machine, + memmap[MICROCHIP_PFSOC_DRAM_LO].base); /* Load the reset vector */ riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, firmware_load_addr, memmap[MICROCHIP_PFSOC_ENVM_DATA].base, diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c index ccad386920..fc2a8a7af4 100644 --- a/hw/riscv/sifive_u.c +++ b/hw/riscv/sifive_u.c @@ -609,8 +609,7 @@ static void sifive_u_machine_init(MachineState *machine) } /* Compute the fdt load address in dram */ - fdt_load_addr = riscv_load_fdt(memmap[SIFIVE_U_DEV_DRAM].base, - machine->ram_size, machine->fdt); + fdt_load_addr = riscv_load_fdt(machine, memmap[SIFIVE_U_DEV_DRAM].base); if (!riscv_is_32bit(&s->soc.u_cpus)) { start_addr_hi32 = (uint64_t)start_addr >> 32; } diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c index 91bf194ec1..82093dd2cb 100644 --- a/hw/riscv/spike.c +++ b/hw/riscv/spike.c @@ -316,8 +316,7 @@ static void spike_board_init(MachineState *machine) } /* Compute the fdt load address in dram */ - fdt_load_addr = riscv_load_fdt(memmap[SPIKE_DRAM].base, - machine->ram_size, machine->fdt); + fdt_load_addr = riscv_load_fdt(machine, memmap[SPIKE_DRAM].base); /* load the reset vector */ riscv_setup_rom_reset_vec(machine, &s->soc[0], memmap[SPIKE_DRAM].base, diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index e374b58f89..0a0252368e 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -1300,8 +1300,7 @@ static void virt_machine_done(Notifier *notifier, void *data) } /* Compute the fdt load address in dram */ - fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base, - machine->ram_size, machine->fdt); + fdt_load_addr = riscv_load_fdt(machine, memmap[VIRT_DRAM].base); /* load the reset vector */ riscv_setup_rom_reset_vec(machine, &s->soc[0], start_addr, virt_memmap[VIRT_MROM].base, diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h index cbd131bad7..3581bbe447 100644 --- a/include/hw/riscv/boot.h +++ b/include/hw/riscv/boot.h @@ -47,7 +47,7 @@ target_ulong riscv_load_kernel(MachineState *machine, target_ulong firmware_end_addr, bool load_initrd, symbol_fn_t sym_cb); -uint64_t riscv_load_fdt(hwaddr dram_start, uint64_t dram_size, void *fdt); +uint64_t riscv_load_fdt(MachineState *ms, hwaddr dram_start); void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState *harts, hwaddr saddr, hwaddr rom_base, hwaddr rom_size,
All callers of riscv_load_fdt() are using machine->ram_size as 'mem_size' and the fdt is always retrievable via machine->fdt. Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- hw/riscv/boot.c | 4 +++- hw/riscv/microchip_pfsoc.c | 4 ++-- hw/riscv/sifive_u.c | 3 +-- hw/riscv/spike.c | 3 +-- hw/riscv/virt.c | 3 +-- include/hw/riscv/boot.h | 2 +- 6 files changed, 9 insertions(+), 10 deletions(-)