On Wed, Apr 25, 2018 at 5:03 PM Michael Clark <m...@sifive.com> wrote:
> The sifive_u machine already marks its ROM readonly. This fixes > the remaining boards. This commit also makes all boards use > mask_rom as the variable name for the ROM. This change also > makes space for the maximum device tree size size and adds > an explicit bounds check and error message. > Cc: Sagar Karandikar <sag...@eecs.berkeley.edu> > Cc: Bastian Koppelmann <kbast...@mail.uni-paderborn.de> > Cc: Palmer Dabbelt <pal...@sifive.com> > Cc: Alistair Francis <alistair.fran...@wdc.com> > Signed-off-by: Michael Clark <m...@sifive.com> > --- > hw/riscv/sifive_e.c | 20 +++++++--------- > hw/riscv/sifive_u.c | 46 ++++++++++++++++++----------------- > hw/riscv/spike.c | 64 ++++++++++++++++++++++++++++--------------------- > hw/riscv/virt.c | 38 +++++++++++++++-------------- > include/hw/riscv/virt.h | 4 ++++ > 5 files changed, 93 insertions(+), 79 deletions(-) > diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c > index 39e4cb4..0c8b8e9 100644 > --- a/hw/riscv/sifive_e.c > +++ b/hw/riscv/sifive_e.c > @@ -74,14 +74,6 @@ static const struct MemmapEntry { > [SIFIVE_E_DTIM] = { 0x80000000, 0x4000 } > }; > -static void copy_le32_to_phys(hwaddr pa, uint32_t *rom, size_t len) > -{ > - int i; > - for (i = 0; i < (len >> 2); i++) { > - stl_phys(&address_space_memory, pa + (i << 2), rom[i]); > - } > -} > - > static uint64_t load_kernel(const char *kernel_filename) > { > uint64_t kernel_entry, kernel_high; > @@ -112,6 +104,7 @@ static void riscv_sifive_e_init(MachineState *machine) > MemoryRegion *main_mem = g_new(MemoryRegion, 1); > MemoryRegion *mask_rom = g_new(MemoryRegion, 1); > MemoryRegion *xip_mem = g_new(MemoryRegion, 1); > + int i; > /* Initialize SOC */ > object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY); > @@ -131,7 +124,7 @@ static void riscv_sifive_e_init(MachineState *machine) > memmap[SIFIVE_E_DTIM].base, main_mem); > /* Mask ROM */ > - memory_region_init_ram(mask_rom, NULL, "riscv.sifive.e.mrom", > + memory_region_init_rom(mask_rom, NULL, "riscv.sifive.e.mrom", > memmap[SIFIVE_E_MROM].size, &error_fatal); > memory_region_add_subregion(sys_mem, > memmap[SIFIVE_E_MROM].base, mask_rom); > @@ -185,9 +178,12 @@ static void riscv_sifive_e_init(MachineState *machine) > 0x00028067, /* 0x1004: jr t0 */ > }; > - /* copy in the reset vector */ > - copy_le32_to_phys(memmap[SIFIVE_E_MROM].base, reset_vec, sizeof(reset_vec)); > - memory_region_set_readonly(mask_rom, true); > + /* 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_E_MROM].base, &address_space_memory); > if (machine->kernel_filename) { > load_kernel(machine->kernel_filename); > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c > index 115618b..11ba4c3 100644 > --- a/hw/riscv/sifive_u.c > +++ b/hw/riscv/sifive_u.c > @@ -52,7 +52,7 @@ static const struct MemmapEntry { > hwaddr size; > } sifive_u_memmap[] = { > [SIFIVE_U_DEBUG] = { 0x0, 0x100 }, > - [SIFIVE_U_MROM] = { 0x1000, 0x2000 }, > + [SIFIVE_U_MROM] = { 0x1000, 0x11000 }, > [SIFIVE_U_CLINT] = { 0x2000000, 0x10000 }, > [SIFIVE_U_PLIC] = { 0xc000000, 0x4000000 }, > [SIFIVE_U_UART0] = { 0x10013000, 0x1000 }, > @@ -60,14 +60,6 @@ static const struct MemmapEntry { > [SIFIVE_U_DRAM] = { 0x80000000, 0x0 }, > }; > -static void copy_le32_to_phys(hwaddr pa, uint32_t *rom, size_t len) > -{ > - int i; > - for (i = 0; i < (len >> 2); i++) { > - stl_phys(&address_space_memory, pa + (i << 2), rom[i]); > - } > -} > - > static uint64_t load_kernel(const char *kernel_filename) > { > uint64_t kernel_entry, kernel_high; > @@ -221,9 +213,10 @@ static void riscv_sifive_u_init(MachineState *machine) > const struct MemmapEntry *memmap = sifive_u_memmap; > SiFiveUState *s = g_new0(SiFiveUState, 1); > - MemoryRegion *sys_memory = get_system_memory(); > + MemoryRegion *system_memory = get_system_memory(); > MemoryRegion *main_mem = g_new(MemoryRegion, 1); > - MemoryRegion *boot_rom = g_new(MemoryRegion, 1); > + MemoryRegion *mask_rom = g_new(MemoryRegion, 1); > + int i; > /* Initialize SOC */ > object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY); > @@ -239,17 +232,17 @@ static void riscv_sifive_u_init(MachineState *machine) > /* register RAM */ > memory_region_init_ram(main_mem, NULL, "riscv.sifive.u.ram", > machine->ram_size, &error_fatal); > - memory_region_add_subregion(sys_memory, memmap[SIFIVE_U_DRAM].base, > + memory_region_add_subregion(system_memory, memmap[SIFIVE_U_DRAM].base, > main_mem); > /* create device tree */ > create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline); > /* boot rom */ > - memory_region_init_ram(boot_rom, NULL, "riscv.sifive.u.mrom", > - memmap[SIFIVE_U_MROM].base, &error_fatal); > - memory_region_set_readonly(boot_rom, true); > - memory_region_add_subregion(sys_memory, 0x0, boot_rom); > + memory_region_init_rom(mask_rom, NULL, "riscv.sifive.u.mrom", > + memmap[SIFIVE_U_MROM].size, &error_fatal); > + memory_region_add_subregion(system_memory, memmap[SIFIVE_U_MROM].base, > + mask_rom); > if (machine->kernel_filename) { > load_kernel(machine->kernel_filename); > @@ -272,13 +265,22 @@ static void riscv_sifive_u_init(MachineState *machine) > /* dtb: */ > }; > - /* copy in the reset vector */ > - copy_le32_to_phys(memmap[SIFIVE_U_MROM].base, reset_vec, sizeof(reset_vec)); > + /* 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 (s->fdt_size >= memmap[SIFIVE_U_MROM].size - sizeof(reset_vec)) { > + error_report("qemu: not enough space to store device-tree"); > + exit(1); > + } > qemu_fdt_dumpdtb(s->fdt, s->fdt_size); > - cpu_physical_memory_write(memmap[SIFIVE_U_MROM].base + > - sizeof(reset_vec), s->fdt, s->fdt_size); > + rom_add_blob_fixed_as("mrom.fdt", s->fdt, s->fdt_size, > + memmap[SIFIVE_U_MROM].base + sizeof(reset_vec), > + &address_space_memory); > /* MMIO */ > s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base, > @@ -292,9 +294,9 @@ static void riscv_sifive_u_init(MachineState *machine) > SIFIVE_U_PLIC_CONTEXT_BASE, > SIFIVE_U_PLIC_CONTEXT_STRIDE, > memmap[SIFIVE_U_PLIC].size); > - sifive_uart_create(sys_memory, memmap[SIFIVE_U_UART0].base, > + sifive_uart_create(system_memory, memmap[SIFIVE_U_UART0].base, > serial_hds[0], SIFIVE_PLIC(s->plic)->irqs[SIFIVE_U_UART0_IRQ]); > - /* sifive_uart_create(sys_memory, memmap[SIFIVE_U_UART1].base, > + /* sifive_uart_create(system_memory, memmap[SIFIVE_U_UART1].base, > serial_hds[1], SIFIVE_PLIC(s->plic)->irqs[SIFIVE_U_UART1_IRQ]); */ > sifive_clint_create(memmap[SIFIVE_U_CLINT].base, > memmap[SIFIVE_U_CLINT].size, smp_cpus, > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c > index 3f6bd0a..d1dbe6b 100644 > --- a/hw/riscv/spike.c > +++ b/hw/riscv/spike.c > @@ -46,19 +46,11 @@ static const struct MemmapEntry { > hwaddr base; > hwaddr size; > } spike_memmap[] = { > - [SPIKE_MROM] = { 0x1000, 0x2000 }, > + [SPIKE_MROM] = { 0x1000, 0x11000 }, > [SPIKE_CLINT] = { 0x2000000, 0x10000 }, > [SPIKE_DRAM] = { 0x80000000, 0x0 }, > }; > -static void copy_le32_to_phys(hwaddr pa, uint32_t *rom, size_t len) > -{ > - int i; > - for (i = 0; i < (len >> 2); i++) { > - stl_phys(&address_space_memory, pa + (i << 2), rom[i]); > - } > -} > - > static uint64_t load_kernel(const char *kernel_filename) > { > uint64_t kernel_entry, kernel_high; > @@ -173,7 +165,8 @@ static void spike_v1_10_0_board_init(MachineState *machine) > SpikeState *s = g_new0(SpikeState, 1); > MemoryRegion *system_memory = get_system_memory(); > MemoryRegion *main_mem = g_new(MemoryRegion, 1); > - MemoryRegion *boot_rom = g_new(MemoryRegion, 1); > + MemoryRegion *mask_rom = g_new(MemoryRegion, 1); > + int i; > /* Initialize SOC */ > object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY); > @@ -196,9 +189,10 @@ static void spike_v1_10_0_board_init(MachineState *machine) > create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline); > /* boot rom */ > - memory_region_init_ram(boot_rom, NULL, "riscv.spike.bootrom", > - s->fdt_size + 0x2000, &error_fatal); > - memory_region_add_subregion(system_memory, 0x0, boot_rom); > + memory_region_init_rom(mask_rom, NULL, "riscv.spike.mrom", > + memmap[SPIKE_MROM].size, &error_fatal); > + memory_region_add_subregion(system_memory, memmap[SPIKE_MROM].base, > + mask_rom); > if (machine->kernel_filename) { > load_kernel(machine->kernel_filename); > @@ -221,16 +215,25 @@ static void spike_v1_10_0_board_init(MachineState *machine) > /* dtb: */ > }; > - /* copy in the reset vector */ > - copy_le32_to_phys(memmap[SPIKE_MROM].base, reset_vec, sizeof(reset_vec)); > + /* 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 (s->fdt_size >= memmap[SPIKE_MROM].size - sizeof(reset_vec)) { > + error_report("qemu: not enough space to store device-tree"); > + exit(1); > + } > qemu_fdt_dumpdtb(s->fdt, s->fdt_size); > - cpu_physical_memory_write(memmap[SPIKE_MROM].base + sizeof(reset_vec), > - s->fdt, s->fdt_size); > + rom_add_blob_fixed_as("mrom.fdt", s->fdt, s->fdt_size, > + memmap[SPIKE_MROM].base + sizeof(reset_vec), > + &address_space_memory); > /* initialize HTIF using symbols found in load_kernel */ > - htif_mm_init(system_memory, boot_rom, &s->soc.harts[0].env, serial_hds[0]); > + htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env, serial_hds[0]); > /* Core Local Interruptor (timer and IPI) */ > sifive_clint_create(memmap[SPIKE_CLINT].base, memmap[SPIKE_CLINT].size, > @@ -244,7 +247,8 @@ static void spike_v1_09_1_board_init(MachineState *machine) > SpikeState *s = g_new0(SpikeState, 1); > MemoryRegion *system_memory = get_system_memory(); > MemoryRegion *main_mem = g_new(MemoryRegion, 1); > - MemoryRegion *boot_rom = g_new(MemoryRegion, 1); > + MemoryRegion *mask_rom = g_new(MemoryRegion, 1); > + int i; > /* Initialize SOC */ > object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY); > @@ -264,9 +268,10 @@ static void spike_v1_09_1_board_init(MachineState *machine) > main_mem); > /* boot rom */ > - memory_region_init_ram(boot_rom, NULL, "riscv.spike.bootrom", > - 0x40000, &error_fatal); > - memory_region_add_subregion(system_memory, 0x0, boot_rom); > + memory_region_init_rom(mask_rom, NULL, "riscv.spike.mrom", > + memmap[SPIKE_MROM].size, &error_fatal); > + memory_region_add_subregion(system_memory, memmap[SPIKE_MROM].base, > + mask_rom); > if (machine->kernel_filename) { > load_kernel(machine->kernel_filename); > @@ -319,15 +324,20 @@ static void spike_v1_09_1_board_init(MachineState *machine) > g_free(isa); > size_t config_string_len = strlen(config_string); > - /* copy in the reset vector */ > - copy_le32_to_phys(memmap[SPIKE_MROM].base, reset_vec, sizeof(reset_vec)); > + /* 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 config string */ > - cpu_physical_memory_write(memmap[SPIKE_MROM].base + sizeof(reset_vec), > - config_string, config_string_len); > + rom_add_blob_fixed_as("mrom.reset", config_string, config_string_len, > + memmap[SPIKE_MROM].base + sizeof(reset_vec), > + &address_space_memory); > /* initialize HTIF using symbols found in load_kernel */ > - htif_mm_init(system_memory, boot_rom, &s->soc.harts[0].env, serial_hds[0]); > + htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env, serial_hds[0]); > /* Core Local Interruptor (timer and IPI) */ > sifive_clint_create(memmap[SPIKE_CLINT].base, memmap[SPIKE_CLINT].size, > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index 090befe..20c509d 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -45,8 +45,8 @@ static const struct MemmapEntry { > hwaddr size; > } virt_memmap[] = { > [VIRT_DEBUG] = { 0x0, 0x100 }, > - [VIRT_MROM] = { 0x1000, 0x2000 }, > - [VIRT_TEST] = { 0x4000, 0x1000 }, > + [VIRT_MROM] = { 0x1000, 0x11000 }, > + [VIRT_TEST] = { 0x100000, 0x1000 }, > [VIRT_CLINT] = { 0x2000000, 0x10000 }, > [VIRT_PLIC] = { 0xc000000, 0x4000000 }, > [VIRT_UART0] = { 0x10000000, 0x100 }, > @@ -54,14 +54,6 @@ static const struct MemmapEntry { > [VIRT_DRAM] = { 0x80000000, 0x0 }, > }; > -static void copy_le32_to_phys(hwaddr pa, uint32_t *rom, size_t len) > -{ > - int i; > - for (i = 0; i < (len >> 2); i++) { > - stl_phys(&address_space_memory, pa + (i << 2), rom[i]); > - } > -} > - > static uint64_t load_kernel(const char *kernel_filename) > { > uint64_t kernel_entry, kernel_high; > @@ -272,7 +264,7 @@ static void riscv_virt_board_init(MachineState *machine) > RISCVVirtState *s = g_new0(RISCVVirtState, 1); > MemoryRegion *system_memory = get_system_memory(); > MemoryRegion *main_mem = g_new(MemoryRegion, 1); > - MemoryRegion *boot_rom = g_new(MemoryRegion, 1); > + MemoryRegion *mask_rom = g_new(MemoryRegion, 1); > char *plic_hart_config; > size_t plic_hart_config_len; > int i; > @@ -299,9 +291,10 @@ static void riscv_virt_board_init(MachineState *machine) > fdt = create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline); > /* boot rom */ > - memory_region_init_ram(boot_rom, NULL, "riscv_virt_board.bootrom", > - s->fdt_size + 0x2000, &error_fatal); > - memory_region_add_subregion(system_memory, 0x0, boot_rom); > + memory_region_init_rom(mask_rom, NULL, "riscv_virt_board.mrom", > + memmap[VIRT_MROM].size, &error_fatal); > + memory_region_add_subregion(system_memory, memmap[VIRT_MROM].base, > + mask_rom); > if (machine->kernel_filename) { > uint64_t kernel_entry = load_kernel(machine->kernel_filename); > @@ -335,13 +328,22 @@ static void riscv_virt_board_init(MachineState *machine) > /* dtb: */ > }; > - /* copy in the reset vector */ > - copy_le32_to_phys(memmap[VIRT_MROM].base, reset_vec, sizeof(reset_vec)); > + /* 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 (s->fdt_size >= memmap[VIRT_MROM].size - sizeof(reset_vec)) { > + error_report("qemu: not enough space to store device-tree"); > + exit(1); > + } > qemu_fdt_dumpdtb(s->fdt, s->fdt_size); > - cpu_physical_memory_write(memmap[VIRT_MROM].base + sizeof(reset_vec), > - s->fdt, s->fdt_size); > + rom_add_blob_fixed_as("mrom.fdt", s->fdt, s->fdt_size, > + memmap[VIRT_MROM].base + sizeof(reset_vec), > + &address_space_memory); > /* create PLIC hart topology configuration string */ > plic_hart_config_len = (strlen(VIRT_PLIC_HART_CONFIG) + 1) * smp_cpus; > diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h > index 91163d6..6f2668e 100644 > --- a/include/hw/riscv/virt.h > +++ b/include/hw/riscv/virt.h > @@ -19,6 +19,10 @@ > #ifndef HW_RISCV_VIRT_H > #define HW_RISCV_VIRT_H > +#define TYPE_RISCV_VIRT_BOARD "riscv.virt" > +#define VIRT(obj) \ > + OBJECT_CHECK(RISCVVirtState, (obj), TYPE_RISCV_VIRT_BOARD) > + This should be in a seperate patch. Alistair > typedef struct { > /*< private >*/ > SysBusDevice parent_obj; > -- > 2.7.0