On Fri, Jun 19, 2020 at 10:11 AM Alexander Richardson <alexander.richard...@cl.cam.ac.uk> wrote: > > On Tue, 16 Jun 2020 at 20:30, Atish Patra <atish.pa...@wdc.com> wrote: > > > > Currently, all riscv machines have identical reset vector code > > implementations with memory addresses being different for all machines. > > They can be easily combined into a single function in common code. > > > > Move it to common function and let all the machines use the common function. > > > > Signed-off-by: Atish Patra <atish.pa...@wdc.com> > > --- > > hw/riscv/boot.c | 46 +++++++++++++++++++++++++++++++++++++++++ > > hw/riscv/sifive_u.c | 38 +++------------------------------- > > hw/riscv/spike.c | 38 +++------------------------------- > > hw/riscv/virt.c | 37 +++------------------------------ > > include/hw/riscv/boot.h | 2 ++ > > 5 files changed, 57 insertions(+), 104 deletions(-) > > > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > > index adb421b91b68..8ed96da600c9 100644 > > --- a/hw/riscv/boot.c > > +++ b/hw/riscv/boot.c > > @@ -22,12 +22,16 @@ > > #include "qemu/units.h" > > #include "qemu/error-report.h" > > #include "exec/cpu-defs.h" > > +#include "exec/address-spaces.h" > > #include "hw/boards.h" > > #include "hw/loader.h" > > #include "hw/riscv/boot.h" > > #include "elf.h" > > +#include "sysemu/device_tree.h" > > #include "sysemu/qtest.h" > > > > +#include <libfdt.h> > > + > > #if defined(TARGET_RISCV32) > > # define KERNEL_BOOT_ADDRESS 0x80400000 > > #else > > @@ -155,3 +159,45 @@ hwaddr riscv_load_initrd(const char *filename, > > uint64_t mem_size, > > > > return *start + size; > > } > > + > > +void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base, > > + hwaddr rom_size, void *fdt) > > +{ > > + int i; > > + /* reset vector */ > > + uint32_t reset_vec[8] = { > > + 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */ > > + 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */ > > + 0xf1402573, /* csrr a0, mhartid */ > > +#if defined(TARGET_RISCV32) > > + 0x0182a283, /* lw t0, 24(t0) */ > > +#elif defined(TARGET_RISCV64) > > + 0x0182b283, /* ld t0, 24(t0) */ > > +#endif > > + 0x00028067, /* jr t0 */ > > + 0x00000000, > > + start_addr, /* start: .dword */ > > + 0x00000000, > > + /* dtb: */ > > + }; > > + > > + /* copy in the reset vector in little_endian byte order */ > > + for (i = 0; i < sizeof(reset_vec) >> 2; i++) { > > + reset_vec[i] = cpu_to_le32(reset_vec[i]); > > + } > Maybe use ARRAY_SIZE(reset_vec) instead of sizeof(reset_vec) >> 2 ? >
Yeah. That's better. Thanks. > > + rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec), > > + rom_base, &address_space_memory); > > + > > + /* copy in the device tree */ > > + if (fdt_pack(fdt) || fdt_totalsize(fdt) > > > + rom_size - sizeof(reset_vec)) { > > + error_report("not enough space to store device-tree"); > > + exit(1); > > + } > > + qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); > > + rom_add_blob_fixed_as("mrom.fdt", fdt, fdt_totalsize(fdt), > > + rom_base + sizeof(reset_vec), > > + &address_space_memory); > > + > > + return; > > +} > > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c > > index f9fef2be9170..c2712570e0d9 100644 > > --- a/hw/riscv/sifive_u.c > > +++ b/hw/riscv/sifive_u.c > > @@ -325,7 +325,6 @@ static void sifive_u_machine_init(MachineState *machine) > > MemoryRegion *main_mem = g_new(MemoryRegion, 1); > > MemoryRegion *flash0 = g_new(MemoryRegion, 1); > > target_ulong start_addr = memmap[SIFIVE_U_DRAM].base; > > - int i; > > > > /* Initialize SoC */ > > object_initialize_child(OBJECT(machine), "soc", &s->soc, > > @@ -374,40 +373,9 @@ static void sifive_u_machine_init(MachineState > > *machine) > > start_addr = memmap[SIFIVE_U_FLASH0].base; > > } > > > > - /* reset vector */ > > - uint32_t reset_vec[8] = { > > - 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */ > > - 0x02028593, /* addi a1, t0, %pcrel_lo(1b) > > */ > > - 0xf1402573, /* csrr a0, mhartid */ > > -#if defined(TARGET_RISCV32) > > - 0x0182a283, /* lw t0, 24(t0) */ > > -#elif defined(TARGET_RISCV64) > > - 0x0182b283, /* ld t0, 24(t0) */ > > -#endif > > - 0x00028067, /* jr t0 */ > > - 0x00000000, > > - start_addr, /* start: .dword */ > > - 0x00000000, > > - /* dtb: */ > > - }; > > - > > - /* copy in the reset vector in little_endian byte order */ > > - for (i = 0; i < sizeof(reset_vec) >> 2; i++) { > > - reset_vec[i] = cpu_to_le32(reset_vec[i]); > > - } > > - rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec), > > - memmap[SIFIVE_U_MROM].base, > > &address_space_memory); > > - > > - /* copy in the device tree */ > > - if (fdt_pack(s->fdt) || fdt_totalsize(s->fdt) > > > - memmap[SIFIVE_U_MROM].size - sizeof(reset_vec)) { > > - error_report("not enough space to store device-tree"); > > - exit(1); > > - } > > - qemu_fdt_dumpdtb(s->fdt, fdt_totalsize(s->fdt)); > > - rom_add_blob_fixed_as("mrom.fdt", s->fdt, fdt_totalsize(s->fdt), > > - memmap[SIFIVE_U_MROM].base + sizeof(reset_vec), > > - &address_space_memory); > > + /* load the reset vector */ > > + riscv_setup_rom_reset_vec(start_addr, memmap[SIFIVE_U_MROM].base, > > + memmap[SIFIVE_U_MROM].size, s->fdt); > > } > > > > static bool sifive_u_machine_get_start_in_flash(Object *obj, Error **errp) > > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c > > index 7bbbdb50363d..238eae48716a 100644 > > --- a/hw/riscv/spike.c > > +++ b/hw/riscv/spike.c > > @@ -165,7 +165,6 @@ static void spike_board_init(MachineState *machine) > > MemoryRegion *system_memory = get_system_memory(); > > MemoryRegion *main_mem = g_new(MemoryRegion, 1); > > MemoryRegion *mask_rom = g_new(MemoryRegion, 1); > > - int i; > > unsigned int smp_cpus = machine->smp.cpus; > > > > /* Initialize SOC */ > > @@ -213,40 +212,9 @@ static void spike_board_init(MachineState *machine) > > } > > } > > > > - /* reset vector */ > > - uint32_t reset_vec[8] = { > > - 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */ > > - 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */ > > - 0xf1402573, /* csrr a0, mhartid */ > > -#if defined(TARGET_RISCV32) > > - 0x0182a283, /* lw t0, 24(t0) */ > > -#elif defined(TARGET_RISCV64) > > - 0x0182b283, /* ld t0, 24(t0) */ > > -#endif > > - 0x00028067, /* jr t0 */ > > - 0x00000000, > > - memmap[SPIKE_DRAM].base, /* start: .dword DRAM_BASE */ > > - 0x00000000, > > - /* dtb: */ > > - }; > > - > > - /* copy in the reset vector in little_endian byte order */ > > - for (i = 0; i < sizeof(reset_vec) >> 2; i++) { > > - reset_vec[i] = cpu_to_le32(reset_vec[i]); > > - } > > - rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec), > > - memmap[SPIKE_MROM].base, &address_space_memory); > > - > > - /* copy in the device tree */ > > - if (fdt_pack(s->fdt) || fdt_totalsize(s->fdt) > > > - memmap[SPIKE_MROM].size - sizeof(reset_vec)) { > > - error_report("not enough space to store device-tree"); > > - exit(1); > > - } > > - qemu_fdt_dumpdtb(s->fdt, fdt_totalsize(s->fdt)); > > - rom_add_blob_fixed_as("mrom.fdt", s->fdt, fdt_totalsize(s->fdt), > > - memmap[SPIKE_MROM].base + sizeof(reset_vec), > > - &address_space_memory); > > + /* load the reset vector */ > > + riscv_setup_rom_reset_vec(memmap[SPIKE_DRAM].base, > > memmap[SPIKE_MROM].base, > > + memmap[SPIKE_MROM].size, s->fdt); > > > > /* initialize HTIF using symbols found in load_kernel */ > > htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env, > > serial_hd(0)); > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > > index 4e4c494a7050..a8e2d58cc067 100644 > > --- a/hw/riscv/virt.c > > +++ b/hw/riscv/virt.c > > @@ -536,40 +536,9 @@ static void virt_machine_init(MachineState *machine) > > start_addr = virt_memmap[VIRT_FLASH].base; > > } > > > > - /* reset vector */ > > - uint32_t reset_vec[8] = { > > - 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */ > > - 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */ > > - 0xf1402573, /* csrr a0, mhartid */ > > -#if defined(TARGET_RISCV32) > > - 0x0182a283, /* lw t0, 24(t0) */ > > -#elif defined(TARGET_RISCV64) > > - 0x0182b283, /* ld t0, 24(t0) */ > > -#endif > > - 0x00028067, /* jr t0 */ > > - 0x00000000, > > - start_addr, /* start: .dword */ > > - 0x00000000, > > - /* dtb: */ > > - }; > > - > > - /* copy in the reset vector in little_endian byte order */ > > - for (i = 0; i < sizeof(reset_vec) >> 2; i++) { > > - reset_vec[i] = cpu_to_le32(reset_vec[i]); > > - } > > - rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec), > > - memmap[VIRT_MROM].base, &address_space_memory); > > - > > - /* copy in the device tree */ > > - if (fdt_pack(s->fdt) || fdt_totalsize(s->fdt) > > > - memmap[VIRT_MROM].size - sizeof(reset_vec)) { > > - error_report("not enough space to store device-tree"); > > - exit(1); > > - } > > - qemu_fdt_dumpdtb(s->fdt, fdt_totalsize(s->fdt)); > > - rom_add_blob_fixed_as("mrom.fdt", s->fdt, fdt_totalsize(s->fdt), > > - memmap[VIRT_MROM].base + sizeof(reset_vec), > > - &address_space_memory); > > + /* load the reset vector */ > > + riscv_setup_rom_reset_vec(start_addr, virt_memmap[VIRT_MROM].base, > > + virt_memmap[VIRT_MROM].size, s->fdt); > > > > /* create PLIC hart topology configuration string */ > > plic_hart_config_len = (strlen(VIRT_PLIC_HART_CONFIG) + 1) * smp_cpus; > > diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h > > index 9daa98da08d7..3e9759c89aa2 100644 > > --- a/include/hw/riscv/boot.h > > +++ b/include/hw/riscv/boot.h > > @@ -35,5 +35,7 @@ target_ulong riscv_load_kernel(const char > > *kernel_filename, > > symbol_fn_t sym_cb); > > hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size, > > uint64_t kernel_entry, hwaddr *start); > > +void riscv_setup_rom_reset_vec(hwaddr saddr, hwaddr rom_base, > > + hwaddr rom_size, void *fdt); > > > > #endif /* RISCV_BOOT_H */ > > -- > > 2.26.2 > > > > > -- Regards, Atish