[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 06/26] RISC-V: Mark ROM read-only after copyi
From: |
Michael Clark |
Subject: |
Re: [Qemu-devel] [PATCH v6 06/26] RISC-V: Mark ROM read-only after copying in code |
Date: |
Mon, 26 Mar 2018 15:22:07 -0700 |
On Sun, Mar 25, 2018 at 5:47 AM, Peter Maydell <address@hidden>
wrote:
> On 25 March 2018 at 00:23, Michael Clark <address@hidden> wrote:
> >
> >
> > On Sat, Mar 24, 2018 at 2:23 PM, Peter Maydell <address@hidden
> >
> > wrote:
> >>
> >> On 24 March 2018 at 18:13, Michael Clark <address@hidden> wrote:
> >> > The sifive_u machine already marks its ROM readonly. This fixes
> >> > the remaining boards.
> >> >
> >> > Cc: Sagar Karandikar <address@hidden>
> >> > Cc: Bastian Koppelmann <address@hidden>
> >> > Signed-off-by: Michael Clark <address@hidden>
> >> > Signed-off-by: Palmer Dabbelt <address@hidden>
> >> > ---
> >> > hw/riscv/sifive_u.c | 9 +++++----
> >> > hw/riscv/spike.c | 18 ++++++++++--------
> >> > hw/riscv/virt.c | 7 ++++---
> >> > include/hw/riscv/spike.h | 8 --------
> >> > 4 files changed, 19 insertions(+), 23 deletions(-)
> >> >
> >> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> >> > index 6116c38..25df16c 100644
> >> > --- a/hw/riscv/sifive_u.c
> >> > +++ b/hw/riscv/sifive_u.c
> >> > @@ -223,7 +223,7 @@ static void riscv_sifive_u_init(MachineState
> >> > *machine)
> >> > SiFiveUState *s = g_new0(SiFiveUState, 1);
> >> > MemoryRegion *sys_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);
> >> >
> >> > /* Initialize SOC */
> >> > object_initialize(&s->soc, sizeof(s->soc),
> TYPE_RISCV_HART_ARRAY);
> >> > @@ -246,10 +246,10 @@ static void riscv_sifive_u_init(MachineState
> >> > *machine)
> >> > create_fdt(s, memmap, machine->ram_size,
> machine->kernel_cmdline);
> >> >
> >> > /* boot rom */
> >> > - memory_region_init_ram(boot_rom, NULL, "riscv.sifive.u.mrom",
> >> > + memory_region_init_ram(mask_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_set_readonly(mask_rom, true);
> >> > + memory_region_add_subregion(sys_memory, 0x0, mask_rom);
> >>
> >> memory_region_init_ram + memory_region_set_readonly is
> >> equivalent to memory_region_init_rom.
> >>
> >> > if (machine->kernel_filename) {
> >> > load_kernel(machine->kernel_filename);
> >> > @@ -279,6 +279,7 @@ static void riscv_sifive_u_init(MachineState
> >> > *machine)
> >> > 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);
> >> > + memory_region_set_readonly(mask_rom, true);
> >>
> >> Rather than doing this, you should use
> >> rom_add_blob_fixed(). That works even on ROMs which
> >> means you can just create them as read-only from the
> >> start rather than waiting til you've written to them
> >> and then marking them read-only. It also means that
> >> you get the contents correctly reset on reset, even
> >> if the user has been messing with their contents
> >> via the debugger or something.
> >>
> >> hw/arm/boot.c has code which (among a lot of other
> >> things) loads initial kernels and dtb images into
> >> guest memory. You can also find ppc code doing
> >> similar things.
> >
> >
> > I don't mind to make this change, however it is a case of good vs
> perfect.
> > Currently we have some machines with writable ROM sections, this change
> > fixes it and has been tested.
>
> Yes, I would put this on your set of things to
> address for 2.13.
>
Okay. It's on my list...
I will be replying to the other thread with a list of the oustanding
patches, whether they are bug fixes vs cleanups, and whether they are
reviewed.
The mstatus.FS fix is relatively critical and Igor's cpu init actually
fixes a bug with -cpu list. In any case I'll send a list shortly...
- [Qemu-devel] [PATCH v6 11/26] RISC-V: Update E order and I extension order, (continued)
- [Qemu-devel] [PATCH v6 11/26] RISC-V: Update E order and I extension order, Michael Clark, 2018/03/24
- [Qemu-devel] [PATCH v6 14/26] RISC-V: Use memory_region_is_ram in pte update, Michael Clark, 2018/03/24
- [Qemu-devel] [PATCH v6 16/26] RISC-V: Hardwire satp to 0 for no-mmu case, Michael Clark, 2018/03/24
- [Qemu-devel] [PATCH v6 23/26] RISC-V: Clear mtval/stval on exceptions without info, Michael Clark, 2018/03/24
- [Qemu-devel] [PATCH v6 06/26] RISC-V: Mark ROM read-only after copying in code, Michael Clark, 2018/03/24
[Qemu-devel] [PATCH v6 19/26] RISC-V: vectored traps are optional, Michael Clark, 2018/03/24
[Qemu-devel] [PATCH v6 10/26] RISC-V: Improve page table walker spec compliance, Michael Clark, 2018/03/24
[Qemu-devel] [PATCH v6 08/26] RISC-V: Make sure rom has space for fdt, Michael Clark, 2018/03/24
[Qemu-devel] [PATCH v6 26/26] RISC-V: Workaround for critical mstatus.FS MTTCG bug, Michael Clark, 2018/03/24
[Qemu-devel] [PATCH v6 20/26] RISC-V: No traps on writes to misa, minstret, mcycle, Michael Clark, 2018/03/24