From: Alistair Francis <alistair23@gmail.com>
To: Michael Clark <mjc@sifive.com>
Cc: "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
Sagar Karandikar <sagark@eecs.berkeley.edu>,
Bastian Koppelmann <kbastian@mail.uni-paderborn.de>,
palmer@sifive.com, Alistair Francis <Alistair.Francis@wdc.com>,
patches@groups.riscv.org,
Peter Crosthwaite <crosthwaite.peter@gmail.com>,
Alexander Graf <agraf@suse.de>,
Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [Qemu-devel] [PATCH v8 11/35] RISC-V: Mark ROM read-only after copying in code
Date: Fri, 04 May 2018 23:54:16 +0000 [thread overview]
Message-ID: <CAKmqyKMRQXh+wsMFrXsNwWhHo67ctfRRxNvmLxmM9K_hHptPkA@mail.gmail.com> (raw)
In-Reply-To: <CAKmqyKOe3xEuMNttQOuu=Dn2_tf4qHK39k0Mn-ZE0tQ5B-+N_w@mail.gmail.com>
On Fri, May 4, 2018 at 4:44 PM Alistair Francis <alistair23@gmail.com>
wrote:
> On Thu, May 3, 2018 at 6:45 PM Michael Clark <mjc@sifive.com> wrote:
> > On Sat, Apr 28, 2018 at 4:17 AM, Alistair Francis <alistair23@gmail.com>
> wrote:
> >> On Thu, Apr 26, 2018 at 10:34 PM Michael Clark <mjc@sifive.com> wrote:
> >> > On Fri, Apr 27, 2018 at 5:22 PM, Michael Clark <mjc@sifive.com>
wrote:
> >> >> On Fri, Apr 27, 2018 at 4:48 AM, Alistair Francis <
> alistair23@gmail.com>
> >> wrote:
> >> >>> On Wed, Apr 25, 2018 at 5:03 PM Michael Clark <mjc@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 <sagark@eecs.berkeley.edu>
> >> >>> > Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> >> >>> > Cc: Palmer Dabbelt <palmer@sifive.com>
> >> >>> > Cc: Alistair Francis <Alistair.Francis@wdc.com>
> >> >>> > Signed-off-by: Michael Clark <mjc@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.
> >> >> I'll shift that chunk into "Remove unused class definitions".
> >> > Actually we to need to drop this chunk as the unused check macros
were
> >> removed from machine state structs in "Remove unused class
definitions".
> >> Somehow the chunk made it into this patch. Likely a rebase issue.
> >> > It's probably best that we add what we need back in the QOM SOC
> refactor
> >> on-top of the this series, or at least after the first set of patches
are
> >> merged...
> >> > I think that's what you were planning to do anyway.
> >> Yeah, that works. So just remove it from this series.
> > After rebasing I had to change this patch because of this patch which
> increases the default device tree size to 1MiB. This is not controllable
by
> the user and we don't know how big the resultant device-tree is. It could
> be < 8KiB in our case.
> > -
https://git.qemu.org/?p=qemu.git;a=commit;h=14ec3cbd7c1e31dca4d23f028100c8f43e156573
> > I studied ftd and used public interfaces and a mechanism consistent with
> the fdt resize functions to calculate the size. As far as I can tell it is
> accurate and covers exactly to the end of the uint32 terminator. I needed
> this because our ROMs are currently small.
> > Peter, this is the patch that changes our ROMs from RAM to ROM using
> memory_region_init_rom and rom_add_blob_fixed_as (as per hw/arm/boot.c),
> and it also adds a truncation warning, so that we actually know what size
> the device-tree is, given our ROMs are currently much smaller than 1MiB.
> That is why we needed a method that tells us how big the device tree
> actually is.
> > BTW I'm actually suspicious of 'fdt_resize' here:
> > -
https://git.qemu.org/?p=dtc.git;a=blob;f=libfdt/fdt_sw.c;h=6d33cc29d0224d9fc6307607ef7563df944da2d3
> > as it doesn't check that 'bufsize' has enough space for the header and
> terminator, although that's potentially a dtc bug. I read dtc to make sure
> the method we use to calculate the size was accurate. There probably
should
> be a method in dtc as we rely on some implementation details, however they
> are quite simple and we can get: sizeof(header) + sizeof(structs) +
> sizeof(strings) + terminator using public APIs and basic knowledge of the
> serialised device-tree form.
> > Anyway, here is the rebased version of this patch using the new
> 'qemu_fdt_totalsize' method in the patch I just sent.
> > -
https://github.com/riscv/riscv-qemu/commit/a65f6e0447d6e32d75f64ba31df5f20d529d0489
> > I have a feeling this is the patch that fixes sifive_u. Did you bisect
> which patch in the series fixed sifive_u?
> I agree, I think this is the patch.
> No, I havent' bisected it. I didn't think there was much point, but if we
> want patches backported to stable I guess there is. I'll dig into it.
Yep, just checked this patch is the first patch in which the SiFive U
machine works.
If this patch is rebased onto master if also works, so only this patch is
required.
Alistair
> > I have to send a pull for the reviewed patches which I can do today, but
> this is one of the patches that is early in the series that does not yet
> have Reviewed-by. When I split the series this patch would be in a second
> series that i'll have to link to the pull in patchew (using the method
> Peter mentioned) or wait until the pull is accepted.
> Great, let's get the first part of the series merged. It'd be nice to send
> out the next version of the second part while the PR is being merged.
> Alistair
> > Michael.
next prev parent reply other threads:[~2018-05-04 23:54 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-25 23:45 [Qemu-devel] [PATCH v8 00/35] QEMU 2.13 Privileged ISA emulation updates Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 01/35] RISC-V: Replace hardcoded constants with enum values Michael Clark
2018-04-26 16:37 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 02/35] RISC-V: Make virt board description match spike Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 03/35] RISC-V: Use ROM base address and size from memmap Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 04/35] RISC-V: Remove identity_translate from load_elf Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 05/35] RISC-V: Remove unused class definitions Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 06/35] RISC-V: Include instruction hex in disassembly Michael Clark
2018-04-26 17:05 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 07/35] RISC-V: Make some header guards more specific Michael Clark
2018-04-26 16:43 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 08/35] RISC-V: Make virt header comment title consistent Michael Clark
2018-04-26 16:42 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 09/35] RISC-V: Remove EM_RISCV ELF_MACHINE indirection Michael Clark
2018-04-26 16:42 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 10/35] RISC-V: Remove erroneous comment from translate.c Michael Clark
2018-04-25 23:51 ` [Qemu-devel] [patches] " Palmer Dabbelt
2018-04-26 16:48 ` [Qemu-devel] " Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 11/35] RISC-V: Mark ROM read-only after copying in code Michael Clark
2018-04-26 16:48 ` Alistair Francis
2018-04-27 5:22 ` Michael Clark
2018-04-27 5:34 ` Michael Clark
2018-04-27 16:17 ` Alistair Francis
2018-05-04 1:45 ` Michael Clark
2018-05-04 23:44 ` Alistair Francis
2018-05-04 23:54 ` Alistair Francis [this message]
2018-05-05 2:02 ` Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 12/35] RISC-V: Update address bits to support sv39 and sv48 Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 13/35] RISC-V: Improve page table walker spec compliance Michael Clark
2018-05-03 20:49 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 14/35] RISC-V: Update E order and I extension order Michael Clark
2018-04-26 17:11 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 15/35] RISC-V: Hardwire satp to 0 for no-mmu case Michael Clark
2018-04-26 17:21 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 16/35] RISC-V: Make mtvec/stvec ignore vectored traps Michael Clark
2018-04-26 17:27 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 17/35] RISC-V: No traps on writes to misa, minstret, mcycle Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 18/35] RISC-V: Clear mtval/stval on exceptions without info Michael Clark
2018-04-26 17:36 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 19/35] RISC-V: Allow S-mode mxr access when priv ISA >= v1.10 Michael Clark
2018-04-26 20:02 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 20/35] RISC-V: Use [ms]counteren CSRs " Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 21/35] RISC-V: Add mcycle/minstret support for -icount auto Michael Clark
2018-04-26 20:05 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 22/35] RISC-V: Use atomic_cmpxchg to update PLIC bitmaps Michael Clark
2018-04-27 0:14 ` Richard Henderson
2018-04-27 7:18 ` Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 23/35] RISC-V: Simplify riscv_cpu_local_irqs_pending Michael Clark
2018-04-27 22:33 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 24/35] RISC-V: Allow setting and clearing multiple irqs Michael Clark
2018-05-03 20:54 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 25/35] RISC-V: Move non-ops from op_helper to cpu_helper Michael Clark
2018-04-26 17:42 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 26/35] RISC-V: Update CSR and interrupt definitions Michael Clark
2018-05-03 20:56 ` Alistair Francis
2018-05-04 4:21 ` Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 27/35] RISC-V: Implement modular CSR helper interface Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 28/35] RISC-V: Implement atomic mip/sip CSR updates Michael Clark
2018-05-03 21:11 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 29/35] RISC-V: Implement existential predicates for CSRs Michael Clark
2018-05-03 21:21 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 30/35] RISC-V: Split out mstatus_fs from tb_flags Michael Clark
2018-05-03 21:22 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 31/35] RISC-V: Mark mstatus.fs dirty Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 32/35] RISC-V: Implement mstatus.TSR/TW/TVM Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 33/35] RISC-V: Add public API for the CSR dispatch table Michael Clark
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 34/35] RISC-V: Add hartid and \n to interrupt logging Michael Clark
2018-05-03 21:25 ` Alistair Francis
2018-04-25 23:45 ` [Qemu-devel] [PATCH v8 35/35] RISC-V: Use riscv prefix consistently on cpu helpers Michael Clark
2018-04-26 1:42 ` [Qemu-devel] [PATCH v8 00/35] QEMU 2.13 Privileged ISA emulation updates Michael Clark
2018-04-26 2:01 ` Michael Clark
2018-04-26 18:22 ` Alistair Francis
2018-04-27 0:34 ` Michael Clark
2018-04-27 10:19 ` Peter Maydell
2018-04-27 0:35 ` Richard Henderson
2018-04-27 5:00 ` Michael Clark
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKmqyKMRQXh+wsMFrXsNwWhHo67ctfRRxNvmLxmM9K_hHptPkA@mail.gmail.com \
--to=alistair23@gmail.com \
--cc=Alistair.Francis@wdc.com \
--cc=agraf@suse.de \
--cc=crosthwaite.peter@gmail.com \
--cc=kbastian@mail.uni-paderborn.de \
--cc=mjc@sifive.com \
--cc=palmer@sifive.com \
--cc=patches@groups.riscv.org \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=sagark@eecs.berkeley.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).