[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 11/35] RISC-V: Mark ROM read-only after copyi
|
From: |
Alistair Francis |
|
Subject: |
Re: [Qemu-devel] [PATCH v8 11/35] RISC-V: Mark ROM read-only after copying in code |
|
Date: |
Thu, 26 Apr 2018 16:48:13 +0000 |
On Wed, Apr 25, 2018 at 5:03 PM Michael Clark <address@hidden> 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 <address@hidden>
> Cc: Bastian Koppelmann <address@hidden>
> Cc: Palmer Dabbelt <address@hidden>
> Cc: Alistair Francis <address@hidden>
> Signed-off-by: Michael Clark <address@hidden>
> ---
> 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
- Re: [Qemu-devel] [PATCH v8 07/35] RISC-V: Make some header guards more specific, (continued)
- [Qemu-devel] [PATCH v8 08/35] RISC-V: Make virt header comment title consistent, Michael Clark, 2018/04/25
- [Qemu-devel] [PATCH v8 09/35] RISC-V: Remove EM_RISCV ELF_MACHINE indirection, Michael Clark, 2018/04/25
- [Qemu-devel] [PATCH v8 10/35] RISC-V: Remove erroneous comment from translate.c, Michael Clark, 2018/04/25
- [Qemu-devel] [PATCH v8 12/35] RISC-V: Update address bits to support sv39 and sv48, Michael Clark, 2018/04/25
- [Qemu-devel] [PATCH v8 11/35] RISC-V: Mark ROM read-only after copying in code, Michael Clark, 2018/04/25
- Re: [Qemu-devel] [PATCH v8 11/35] RISC-V: Mark ROM read-only after copying in code,
Alistair Francis <=
[Qemu-devel] [PATCH v8 13/35] RISC-V: Improve page table walker spec compliance, Michael Clark, 2018/04/25
[Qemu-devel] [PATCH v8 14/35] RISC-V: Update E order and I extension order, Michael Clark, 2018/04/25
[Qemu-devel] [PATCH v8 15/35] RISC-V: Hardwire satp to 0 for no-mmu case, Michael Clark, 2018/04/25
[Qemu-devel] [PATCH v8 16/35] RISC-V: Make mtvec/stvec ignore vectored traps, Michael Clark, 2018/04/25