All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup
@ 2018-03-09  4:12 Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 01/23] RISC-V: Make virt create_fdt interface consistent Michael Clark
                   ` (23 more replies)
  0 siblings, 24 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, Paolo Bonzini

Apparently there is at least one logic bug in amongst this
set of 23 patches. I'll shout you a beer if you can find it.
I found one myself so there was probably at least two.

Hey, it boots SMP Linux here, but I think it needs more testing.

This is a series of spec conformance bug fixes and code cleanups.
We would like to get this series in after our core changes in v8.2.

* Implements WARL behavior for CSRs that don't support writes
* Improves specification conformance of the page table walker
  * Change access checks from ternary operator to if statements
  * Checks for misaligned PPNs
  * Disallow M-mode or S-mode from fetching from User pages
  * Adds reserved PTE flag check: W or W|X
  * Adds prot read if mode is not U and mstatus.mxr is set
  * Improves page walker comments and general readability 
* Several trivial code cleanups to hw/riscv
  * Replacing hard coded constants with reference to enums
    or the machine memory maps.
* Adds bounds checks when writing device-tree to ROM
* Updates the cpu model to use a more modern interface

v2

- remove unused class boilerplate retains qom parent_obj
- convert cpu definition towards future model
- honor mstatus.mxr flag in page table walker

Michael Clark (23):
  RISC-V: Make virt create_fdt interface consistent
  RISC-V: Replace hardcoded constants with enum values
  RISC-V: Make virt board description match spike
  RISC-V: Use ROM base address and size from memmap
  RISC-V: Remove identity_translate from load_elf
  RISC-V: Mark ROM read-only after copying in code
  RISC-V: Remove unused class definitions
  RISC-V: Make sure rom has space for fdt
  RISC-V: Include intruction hex in disassembly
  RISC-V: Hold rcu_read_lock when accessing memory
  RISC-V: Improve page table walker spec compliance
  RISC-V: Update E order and I extension order
  RISC-V: Make some header guards more specific
  RISC-V: Make virt header comment title consistent
  RISC-V: Use memory_region_is_ram in pte update
  RISC-V: Remove EM_RISCV ELF_MACHINE indirection
  RISC-V: Hardwire satp to 0 for no-mmu case
  RISC-V: Remove braces from satp case statement
  RISC-V: riscv-qemu port supports sv39 and sv48
  RISC-V: vectored traps are optional
  RISC-V: No traps on writes to misa,minstret,mcycle
  RISC-V: Remove support for adhoc X_COP interrupt
  RISC-V: Convert cpu definition towards future model

 disas/riscv.c                   |  39 +++++++------
 hw/riscv/sifive_clint.c         |   9 +--
 hw/riscv/sifive_e.c             |  34 +----------
 hw/riscv/sifive_u.c             |  65 +++++++--------------
 hw/riscv/spike.c                |  65 ++++++++-------------
 hw/riscv/virt.c                 |  77 +++++++++----------------
 include/hw/riscv/sifive_clint.h |   4 ++
 include/hw/riscv/sifive_e.h     |   5 --
 include/hw/riscv/sifive_u.h     |   9 ++-
 include/hw/riscv/spike.h        |  15 ++---
 include/hw/riscv/virt.h         |  17 +++---
 target/riscv/cpu.c              | 125 ++++++++++++++++++++++------------------
 target/riscv/cpu.h              |   6 +-
 target/riscv/cpu_bits.h         |   3 -
 target/riscv/helper.c           |  65 +++++++++++++++------
 target/riscv/op_helper.c        |  52 ++++++++---------
 16 files changed, 263 insertions(+), 327 deletions(-)

-- 
2.7.0

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 01/23] RISC-V: Make virt create_fdt interface consistent
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-10 20:33   ` Philippe Mathieu-Daudé
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 02/23] RISC-V: Replace hardcoded constants with enum values Michael Clark
                   ` (22 subsequent siblings)
  23 siblings, 1 reply; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

create_fdt sets the fdt variable on RISCVVirtState and this is
used to access the fdt. This reverts a change introduced in
https://github.com/riscv/riscv-qemu/pull/109 which introduced
a redundant return value, overlooking the RISCVVirtState
structure member that made create_fdt inconsistent with the
other RISC-V machines. The other alternative is to change
the other boards to return the fdt. Note: the RISCVVirtState
also contains fdt_size.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 hw/riscv/virt.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index e2c214e..37968d2 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -108,7 +108,7 @@ static hwaddr load_initrd(const char *filename, uint64_t mem_size,
     return *start + size;
 }
 
-static void *create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
+static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
     uint64_t mem_size, const char *cmdline)
 {
     void *fdt;
@@ -264,8 +264,6 @@ static void *create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
     qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename);
     qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
     g_free(nodename);
-
-    return fdt;
 }
 
 static void riscv_virt_board_init(MachineState *machine)
@@ -279,7 +277,6 @@ static void riscv_virt_board_init(MachineState *machine)
     char *plic_hart_config;
     size_t plic_hart_config_len;
     int i;
-    void *fdt;
 
     /* Initialize SOC */
     object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
@@ -299,7 +296,7 @@ static void riscv_virt_board_init(MachineState *machine)
         main_mem);
 
     /* create device tree */
-    fdt = create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
+    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
 
     /* boot rom */
     memory_region_init_ram(boot_rom, NULL, "riscv_virt_board.bootrom",
@@ -314,9 +311,9 @@ static void riscv_virt_board_init(MachineState *machine)
             hwaddr end = load_initrd(machine->initrd_filename,
                                      machine->ram_size, kernel_entry,
                                      &start);
-            qemu_fdt_setprop_cell(fdt, "/chosen",
-                                  "linux,initrd-start", start);
-            qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+            qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-start",
+                                  start);
+            qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-end",
                                   end);
         }
     }
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 02/23] RISC-V: Replace hardcoded constants with enum values
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 01/23] RISC-V: Make virt create_fdt interface consistent Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 03/23] RISC-V: Make virt board description match spike Michael Clark
                   ` (21 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

The RISC-V device-tree code has a number of hard-coded
constants and this change moves them into header enums.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/riscv/sifive_clint.c         | 9 +++------
 hw/riscv/sifive_u.c             | 6 ++++--
 hw/riscv/spike.c                | 6 ++++--
 hw/riscv/virt.c                 | 6 ++++--
 include/hw/riscv/sifive_clint.h | 4 ++++
 include/hw/riscv/sifive_u.h     | 4 ++++
 include/hw/riscv/spike.h        | 4 ++++
 include/hw/riscv/virt.h         | 4 ++++
 8 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
index 4893453..7cc606e 100644
--- a/hw/riscv/sifive_clint.c
+++ b/hw/riscv/sifive_clint.c
@@ -26,13 +26,10 @@
 #include "hw/riscv/sifive_clint.h"
 #include "qemu/timer.h"
 
-/* See: riscv-pk/machine/sbi_entry.S and arch/riscv/kernel/time.c */
-#define TIMER_FREQ (10 * 1000 * 1000)
-
 static uint64_t cpu_riscv_read_rtc(void)
 {
-    return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), TIMER_FREQ,
-                    NANOSECONDS_PER_SECOND);
+    return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
+        SIFIVE_CLINT_TIMEBASE_FREQ, NANOSECONDS_PER_SECOND);
 }
 
 /*
@@ -59,7 +56,7 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value)
     diff = cpu->env.timecmp - rtc_r;
     /* back to ns (note args switched in muldiv64) */
     next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-        muldiv64(diff, NANOSECONDS_PER_SECOND, TIMER_FREQ);
+        muldiv64(diff, NANOSECONDS_PER_SECOND, SIFIVE_CLINT_TIMEBASE_FREQ);
     timer_mod(cpu->env.timer, next);
 }
 
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 1c2deef..f3f7615 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -122,7 +122,8 @@ static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
     g_free(nodename);
 
     qemu_fdt_add_subnode(fdt, "/cpus");
-    qemu_fdt_setprop_cell(fdt, "/cpus", "timebase-frequency", 10000000);
+    qemu_fdt_setprop_cell(fdt, "/cpus", "timebase-frequency",
+        SIFIVE_CLINT_TIMEBASE_FREQ);
     qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
     qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
 
@@ -131,7 +132,8 @@ static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
         char *intc = g_strdup_printf("/cpus/cpu@%d/interrupt-controller", cpu);
         char *isa = riscv_isa_string(&s->soc.harts[cpu]);
         qemu_fdt_add_subnode(fdt, nodename);
-        qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency", 1000000000);
+        qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
+                              SIFIVE_U_CLOCK_FREQ);
         qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
         qemu_fdt_setprop_string(fdt, nodename, "riscv,isa", isa);
         qemu_fdt_setprop_string(fdt, nodename, "compatible", "riscv");
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 2d1f114..4c233ec 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -115,7 +115,8 @@ static void create_fdt(SpikeState *s, const struct MemmapEntry *memmap,
     g_free(nodename);
 
     qemu_fdt_add_subnode(fdt, "/cpus");
-    qemu_fdt_setprop_cell(fdt, "/cpus", "timebase-frequency", 10000000);
+    qemu_fdt_setprop_cell(fdt, "/cpus", "timebase-frequency",
+        SIFIVE_CLINT_TIMEBASE_FREQ);
     qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
     qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
 
@@ -124,7 +125,8 @@ static void create_fdt(SpikeState *s, const struct MemmapEntry *memmap,
         char *intc = g_strdup_printf("/cpus/cpu@%d/interrupt-controller", cpu);
         char *isa = riscv_isa_string(&s->soc.harts[cpu]);
         qemu_fdt_add_subnode(fdt, nodename);
-        qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency", 1000000000);
+        qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
+                              SPIKE_CLOCK_FREQ);
         qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
         qemu_fdt_setprop_string(fdt, nodename, "riscv,isa", isa);
         qemu_fdt_setprop_string(fdt, nodename, "compatible", "riscv");
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 37968d2..a402856 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -145,7 +145,8 @@ static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
     g_free(nodename);
 
     qemu_fdt_add_subnode(fdt, "/cpus");
-    qemu_fdt_setprop_cell(fdt, "/cpus", "timebase-frequency", 10000000);
+    qemu_fdt_setprop_cell(fdt, "/cpus", "timebase-frequency",
+                          SIFIVE_CLINT_TIMEBASE_FREQ);
     qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
     qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
 
@@ -155,7 +156,8 @@ static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
         char *intc = g_strdup_printf("/cpus/cpu@%d/interrupt-controller", cpu);
         char *isa = riscv_isa_string(&s->soc.harts[cpu]);
         qemu_fdt_add_subnode(fdt, nodename);
-        qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency", 1000000000);
+        qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
+                              VIRT_CLOCK_FREQ);
         qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
         qemu_fdt_setprop_string(fdt, nodename, "riscv,isa", isa);
         qemu_fdt_setprop_string(fdt, nodename, "compatible", "riscv");
diff --git a/include/hw/riscv/sifive_clint.h b/include/hw/riscv/sifive_clint.h
index aaa2a58..e2865be 100644
--- a/include/hw/riscv/sifive_clint.h
+++ b/include/hw/riscv/sifive_clint.h
@@ -47,4 +47,8 @@ enum {
     SIFIVE_TIME_BASE    = 0xBFF8
 };
 
+enum {
+    SIFIVE_CLINT_TIMEBASE_FREQ = 10000000
+};
+
 #endif
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index 662e8a1..be38aa0 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -50,6 +50,10 @@ enum {
     SIFIVE_U_UART1_IRQ = 4
 };
 
+enum {
+    SIFIVE_U_CLOCK_FREQ = 1000000000
+};
+
 #define SIFIVE_U_PLIC_HART_CONFIG "MS"
 #define SIFIVE_U_PLIC_NUM_SOURCES 127
 #define SIFIVE_U_PLIC_NUM_PRIORITIES 7
diff --git a/include/hw/riscv/spike.h b/include/hw/riscv/spike.h
index cb55a14..d85a64e 100644
--- a/include/hw/riscv/spike.h
+++ b/include/hw/riscv/spike.h
@@ -42,6 +42,10 @@ enum {
     SPIKE_DRAM
 };
 
+enum {
+    SPIKE_CLOCK_FREQ = 1000000000
+};
+
 #if defined(TARGET_RISCV32)
 #define SPIKE_V1_09_1_CPU TYPE_RISCV_CPU_RV32GCSU_V1_09_1
 #define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_RV32GCSU_V1_10_0
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 7525647..2fbe808 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -55,6 +55,10 @@ enum {
     VIRTIO_NDEV = 10
 };
 
+enum {
+    VIRT_CLOCK_FREQ = 1000000000
+};
+
 #define VIRT_PLIC_HART_CONFIG "MS"
 #define VIRT_PLIC_NUM_SOURCES 127
 #define VIRT_PLIC_NUM_PRIORITIES 7
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 03/23] RISC-V: Make virt board description match spike
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 01/23] RISC-V: Make virt create_fdt interface consistent Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 02/23] RISC-V: Replace hardcoded constants with enum values Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-10 20:34   ` Philippe Mathieu-Daudé
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 04/23] RISC-V: Use ROM base address and size from memmap Michael Clark
                   ` (20 subsequent siblings)
  23 siblings, 1 reply; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

This makes 'qemu-system-riscv64 -machine help' output more tidy
and consistent.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 hw/riscv/virt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index a402856..0055439 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -404,7 +404,7 @@ static const TypeInfo riscv_virt_board_device = {
 
 static void riscv_virt_board_machine_init(MachineClass *mc)
 {
-    mc->desc = "RISC-V VirtIO Board (Privileged spec v1.10)";
+    mc->desc = "RISC-V VirtIO Board (Privileged ISA v1.10)";
     mc->init = riscv_virt_board_init;
     mc->max_cpus = 8; /* hardcoded limit in BBL */
 }
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 04/23] RISC-V: Use ROM base address and size from memmap
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (2 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 03/23] RISC-V: Make virt board description match spike Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-10 20:35   ` Philippe Mathieu-Daudé
  2018-03-12 18:24   ` Eric Blake
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 05/23] RISC-V: Remove identity_translate from load_elf Michael Clark
                   ` (19 subsequent siblings)
  23 siblings, 2 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

Another case of replaceing hard coded constants, this time
referring to the definition in the virt machine's memmap.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 hw/riscv/virt.c         | 4 ++--
 include/hw/riscv/virt.h | 2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 0055439..0d101fc 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -338,11 +338,11 @@ static void riscv_virt_board_init(MachineState *machine)
     };
 
     /* copy in the reset vector */
-    copy_le32_to_phys(ROM_BASE, reset_vec, sizeof(reset_vec));
+    copy_le32_to_phys(memmap[VIRT_MROM].base, reset_vec, sizeof(reset_vec));
 
     /* copy in the device tree */
     qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
-    cpu_physical_memory_write(ROM_BASE + sizeof(reset_vec),
+    cpu_physical_memory_write(memmap[VIRT_MROM].base + sizeof(reset_vec),
         s->fdt, s->fdt_size);
 
     /* create PLIC hart topology configuration string */
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 2fbe808..655e85d 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -23,8 +23,6 @@
 #define VIRT(obj) \
     OBJECT_CHECK(RISCVVirtState, (obj), TYPE_RISCV_VIRT_BOARD)
 
-enum { ROM_BASE = 0x1000 };
-
 typedef struct {
     /*< private >*/
     SysBusDevice parent_obj;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 05/23] RISC-V: Remove identity_translate from load_elf
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (3 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 04/23] RISC-V: Use ROM base address and size from memmap Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 06/23] RISC-V: Mark ROM read-only after copying in code Michael Clark
                   ` (18 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

When load_elf is called with NULL as an argument to the
address translate callback, it does an identity translation.
This commit removes the redundant identity_translate callback.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/riscv/sifive_e.c | 7 +------
 hw/riscv/sifive_u.c | 7 +------
 hw/riscv/spike.c    | 7 +------
 hw/riscv/virt.c     | 7 +------
 4 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 19eca36..09c9d49 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -82,16 +82,11 @@ static void copy_le32_to_phys(hwaddr pa, uint32_t *rom, size_t len)
     }
 }
 
-static uint64_t identity_translate(void *opaque, uint64_t addr)
-{
-    return addr;
-}
-
 static uint64_t load_kernel(const char *kernel_filename)
 {
     uint64_t kernel_entry, kernel_high;
 
-    if (load_elf(kernel_filename, identity_translate, NULL,
+    if (load_elf(kernel_filename, NULL, NULL,
                  &kernel_entry, NULL, &kernel_high,
                  0, ELF_MACHINE, 1, 0) < 0) {
         error_report("qemu: could not load kernel '%s'", kernel_filename);
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index f3f7615..6116c38 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -68,16 +68,11 @@ static void copy_le32_to_phys(hwaddr pa, uint32_t *rom, size_t len)
     }
 }
 
-static uint64_t identity_translate(void *opaque, uint64_t addr)
-{
-    return addr;
-}
-
 static uint64_t load_kernel(const char *kernel_filename)
 {
     uint64_t kernel_entry, kernel_high;
 
-    if (load_elf(kernel_filename, identity_translate, NULL,
+    if (load_elf(kernel_filename, NULL, NULL,
                  &kernel_entry, NULL, &kernel_high,
                  0, ELF_MACHINE, 1, 0) < 0) {
         error_report("qemu: could not load kernel '%s'", kernel_filename);
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 4c233ec..7710333 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -59,16 +59,11 @@ static void copy_le32_to_phys(hwaddr pa, uint32_t *rom, size_t len)
     }
 }
 
-static uint64_t identity_translate(void *opaque, uint64_t addr)
-{
-    return addr;
-}
-
 static uint64_t load_kernel(const char *kernel_filename)
 {
     uint64_t kernel_entry, kernel_high;
 
-    if (load_elf_ram_sym(kernel_filename, identity_translate, NULL,
+    if (load_elf_ram_sym(kernel_filename, NULL, NULL,
             &kernel_entry, NULL, &kernel_high, 0, ELF_MACHINE, 1, 0,
             NULL, true, htif_symbol_callback) < 0) {
         error_report("qemu: could not load kernel '%s'", kernel_filename);
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 0d101fc..f8c19b4 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -62,16 +62,11 @@ static void copy_le32_to_phys(hwaddr pa, uint32_t *rom, size_t len)
     }
 }
 
-static uint64_t identity_translate(void *opaque, uint64_t addr)
-{
-    return addr;
-}
-
 static uint64_t load_kernel(const char *kernel_filename)
 {
     uint64_t kernel_entry, kernel_high;
 
-    if (load_elf(kernel_filename, identity_translate, NULL,
+    if (load_elf(kernel_filename, NULL, NULL,
                  &kernel_entry, NULL, &kernel_high,
                  0, ELF_MACHINE, 1, 0) < 0) {
         error_report("qemu: could not load kernel '%s'", kernel_filename);
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 06/23] RISC-V: Mark ROM read-only after copying in code
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (4 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 05/23] RISC-V: Remove identity_translate from load_elf Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 07/23] RISC-V: Remove unused class definitions Michael Clark
                   ` (17 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

The sifive_u machine already marks its ROM readonly. This fixes
the remaining boards.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 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);
 
     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);
 
     /* MMIO */
     s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 7710333..74edf33 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -173,7 +173,7 @@ 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);
 
     /* Initialize SOC */
     object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
@@ -196,9 +196,9 @@ 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",
+    memory_region_init_ram(mask_rom, NULL, "riscv.spike.mrom",
                            s->fdt_size + 0x2000, &error_fatal);
-    memory_region_add_subregion(system_memory, 0x0, boot_rom);
+    memory_region_add_subregion(system_memory, 0x0, mask_rom);
 
     if (machine->kernel_filename) {
         load_kernel(machine->kernel_filename);
@@ -228,9 +228,10 @@ static void spike_v1_10_0_board_init(MachineState *machine)
     qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
     cpu_physical_memory_write(memmap[SPIKE_MROM].base + sizeof(reset_vec),
         s->fdt, s->fdt_size);
+    memory_region_set_readonly(mask_rom, true);
 
     /* 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 +245,7 @@ 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);
 
     /* Initialize SOC */
     object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
@@ -264,9 +265,9 @@ static void spike_v1_09_1_board_init(MachineState *machine)
         main_mem);
 
     /* boot rom */
-    memory_region_init_ram(boot_rom, NULL, "riscv.spike.bootrom",
+    memory_region_init_ram(mask_rom, NULL, "riscv.spike.mrom",
                            0x40000, &error_fatal);
-    memory_region_add_subregion(system_memory, 0x0, boot_rom);
+    memory_region_add_subregion(system_memory, 0x0, mask_rom);
 
     if (machine->kernel_filename) {
         load_kernel(machine->kernel_filename);
@@ -325,9 +326,10 @@ static void spike_v1_09_1_board_init(MachineState *machine)
     /* copy in the config string */
     cpu_physical_memory_write(memmap[SPIKE_MROM].base + sizeof(reset_vec),
         config_string, config_string_len);
+    memory_region_set_readonly(mask_rom, true);
 
     /* 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 f8c19b4..f1e3641 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -270,7 +270,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;
@@ -296,9 +296,9 @@ static void riscv_virt_board_init(MachineState *machine)
     create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
 
     /* boot rom */
-    memory_region_init_ram(boot_rom, NULL, "riscv_virt_board.bootrom",
+    memory_region_init_ram(mask_rom, NULL, "riscv_virt_board.mrom",
                            s->fdt_size + 0x2000, &error_fatal);
-    memory_region_add_subregion(system_memory, 0x0, boot_rom);
+    memory_region_add_subregion(system_memory, 0x0, mask_rom);
 
     if (machine->kernel_filename) {
         uint64_t kernel_entry = load_kernel(machine->kernel_filename);
@@ -339,6 +339,7 @@ static void riscv_virt_board_init(MachineState *machine)
     qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
     cpu_physical_memory_write(memmap[VIRT_MROM].base + sizeof(reset_vec),
         s->fdt, s->fdt_size);
+    memory_region_set_readonly(mask_rom, true);
 
     /* create PLIC hart topology configuration string */
     plic_hart_config_len = (strlen(VIRT_PLIC_HART_CONFIG) + 1) * smp_cpus;
diff --git a/include/hw/riscv/spike.h b/include/hw/riscv/spike.h
index d85a64e..179b6cf 100644
--- a/include/hw/riscv/spike.h
+++ b/include/hw/riscv/spike.h
@@ -22,20 +22,12 @@
 #define TYPE_RISCV_SPIKE_V1_09_1_BOARD "riscv.spike_v1_9_1"
 #define TYPE_RISCV_SPIKE_V1_10_0_BOARD "riscv.spike_v1_10"
 
-#define SPIKE(obj) \
-    OBJECT_CHECK(SpikeState, (obj), TYPE_RISCV_SPIKE_BOARD)
-
 typedef struct {
-    /*< private >*/
-    SysBusDevice parent_obj;
-
-    /*< public >*/
     RISCVHartArrayState soc;
     void *fdt;
     int fdt_size;
 } SpikeState;
 
-
 enum {
     SPIKE_MROM,
     SPIKE_CLINT,
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 07/23] RISC-V: Remove unused class definitions
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (5 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 06/23] RISC-V: Mark ROM read-only after copying in code Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 08/23] RISC-V: Make sure rom has space for fdt Michael Clark
                   ` (16 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

Removes a whole lot of unnecessary boilerplate code. Machines
don't need to be objects. The expansion of the SOC object model
for the RISC-V machines will happen in the future as SiFive
plans to add their FE310 and FU540 SOCs to QEMU. However, it
seems that this present boilerplate is complete unnecessary.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/riscv/sifive_e.c         | 25 -------------------------
 hw/riscv/sifive_u.c         | 25 -------------------------
 hw/riscv/spike.c            | 20 --------------------
 hw/riscv/virt.c             | 25 -------------------------
 include/hw/riscv/sifive_e.h |  5 -----
 include/hw/riscv/sifive_u.h |  5 -----
 include/hw/riscv/spike.h    |  7 ++++---
 include/hw/riscv/virt.h     |  5 -----
 8 files changed, 4 insertions(+), 113 deletions(-)

diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 09c9d49..4872b68 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -194,24 +194,6 @@ static void riscv_sifive_e_init(MachineState *machine)
     }
 }
 
-static int riscv_sifive_e_sysbus_device_init(SysBusDevice *sysbusdev)
-{
-    return 0;
-}
-
-static void riscv_sifive_e_class_init(ObjectClass *klass, void *data)
-{
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-    k->init = riscv_sifive_e_sysbus_device_init;
-}
-
-static const TypeInfo riscv_sifive_e_device = {
-    .name          = TYPE_SIFIVE_E,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(SiFiveEState),
-    .class_init    = riscv_sifive_e_class_init,
-};
-
 static void riscv_sifive_e_machine_init(MachineClass *mc)
 {
     mc->desc = "RISC-V Board compatible with SiFive E SDK";
@@ -220,10 +202,3 @@ static void riscv_sifive_e_machine_init(MachineClass *mc)
 }
 
 DEFINE_MACHINE("sifive_e", riscv_sifive_e_machine_init)
-
-static void riscv_sifive_e_register_types(void)
-{
-    type_register_static(&riscv_sifive_e_device);
-}
-
-type_init(riscv_sifive_e_register_types);
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 25df16c..083043a 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -302,31 +302,6 @@ static void riscv_sifive_u_init(MachineState *machine)
         SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE);
 }
 
-static int riscv_sifive_u_sysbus_device_init(SysBusDevice *sysbusdev)
-{
-    return 0;
-}
-
-static void riscv_sifive_u_class_init(ObjectClass *klass, void *data)
-{
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-    k->init = riscv_sifive_u_sysbus_device_init;
-}
-
-static const TypeInfo riscv_sifive_u_device = {
-    .name          = TYPE_SIFIVE_U,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(SiFiveUState),
-    .class_init    = riscv_sifive_u_class_init,
-};
-
-static void riscv_sifive_u_register_types(void)
-{
-    type_register_static(&riscv_sifive_u_device);
-}
-
-type_init(riscv_sifive_u_register_types);
-
 static void riscv_sifive_u_machine_init(MachineClass *mc)
 {
     mc->desc = "RISC-V Board compatible with SiFive U SDK";
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 74edf33..64e585e 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -336,18 +336,6 @@ static void spike_v1_09_1_board_init(MachineState *machine)
         smp_cpus, SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE);
 }
 
-static const TypeInfo spike_v_1_09_1_device = {
-    .name          = TYPE_RISCV_SPIKE_V1_09_1_BOARD,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(SpikeState),
-};
-
-static const TypeInfo spike_v_1_10_0_device = {
-    .name          = TYPE_RISCV_SPIKE_V1_10_0_BOARD,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(SpikeState),
-};
-
 static void spike_v1_09_1_machine_init(MachineClass *mc)
 {
     mc->desc = "RISC-V Spike Board (Privileged ISA v1.9.1)";
@@ -365,11 +353,3 @@ static void spike_v1_10_0_machine_init(MachineClass *mc)
 
 DEFINE_MACHINE("spike_v1.9.1", spike_v1_09_1_machine_init)
 DEFINE_MACHINE("spike_v1.10", spike_v1_10_0_machine_init)
-
-static void riscv_spike_board_register_types(void)
-{
-    type_register_static(&spike_v_1_09_1_device);
-    type_register_static(&spike_v_1_10_0_device);
-}
-
-type_init(riscv_spike_board_register_types);
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index f1e3641..5913100 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -380,24 +380,6 @@ static void riscv_virt_board_init(MachineState *machine)
         serial_hds[0], DEVICE_LITTLE_ENDIAN);
 }
 
-static int riscv_virt_board_sysbus_device_init(SysBusDevice *sysbusdev)
-{
-    return 0;
-}
-
-static void riscv_virt_board_class_init(ObjectClass *klass, void *data)
-{
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-    k->init = riscv_virt_board_sysbus_device_init;
-}
-
-static const TypeInfo riscv_virt_board_device = {
-    .name          = TYPE_RISCV_VIRT_BOARD,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(RISCVVirtState),
-    .class_init    = riscv_virt_board_class_init,
-};
-
 static void riscv_virt_board_machine_init(MachineClass *mc)
 {
     mc->desc = "RISC-V VirtIO Board (Privileged ISA v1.10)";
@@ -406,10 +388,3 @@ static void riscv_virt_board_machine_init(MachineClass *mc)
 }
 
 DEFINE_MACHINE("virt", riscv_virt_board_machine_init)
-
-static void riscv_virt_board_register_types(void)
-{
-    type_register_static(&riscv_virt_board_device);
-}
-
-type_init(riscv_virt_board_register_types);
diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h
index 0aebc57..12ad6d2 100644
--- a/include/hw/riscv/sifive_e.h
+++ b/include/hw/riscv/sifive_e.h
@@ -19,11 +19,6 @@
 #ifndef HW_SIFIVE_E_H
 #define HW_SIFIVE_E_H
 
-#define TYPE_SIFIVE_E "riscv.sifive_e"
-
-#define SIFIVE_E(obj) \
-    OBJECT_CHECK(SiFiveEState, (obj), TYPE_SIFIVE_E)
-
 typedef struct SiFiveEState {
     /*< private >*/
     SysBusDevice parent_obj;
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index be38aa0..94a3905 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -19,11 +19,6 @@
 #ifndef HW_SIFIVE_U_H
 #define HW_SIFIVE_U_H
 
-#define TYPE_SIFIVE_U "riscv.sifive_u"
-
-#define SIFIVE_U(obj) \
-    OBJECT_CHECK(SiFiveUState, (obj), TYPE_SIFIVE_U)
-
 typedef struct SiFiveUState {
     /*< private >*/
     SysBusDevice parent_obj;
diff --git a/include/hw/riscv/spike.h b/include/hw/riscv/spike.h
index 179b6cf..8410430 100644
--- a/include/hw/riscv/spike.h
+++ b/include/hw/riscv/spike.h
@@ -19,10 +19,11 @@
 #ifndef HW_SPIKE_H
 #define HW_SPIKE_H
 
-#define TYPE_RISCV_SPIKE_V1_09_1_BOARD "riscv.spike_v1_9_1"
-#define TYPE_RISCV_SPIKE_V1_10_0_BOARD "riscv.spike_v1_10"
-
 typedef struct {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
     RISCVHartArrayState soc;
     void *fdt;
     int fdt_size;
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 655e85d..b91a412 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -19,10 +19,6 @@
 #ifndef HW_VIRT_H
 #define HW_VIRT_H
 
-#define TYPE_RISCV_VIRT_BOARD "riscv.virt"
-#define VIRT(obj) \
-    OBJECT_CHECK(RISCVVirtState, (obj), TYPE_RISCV_VIRT_BOARD)
-
 typedef struct {
     /*< private >*/
     SysBusDevice parent_obj;
@@ -45,7 +41,6 @@ enum {
     VIRT_DRAM
 };
 
-
 enum {
     UART0_IRQ = 10,
     VIRTIO_IRQ = 1, /* 1 to 8 */
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 08/23] RISC-V: Make sure rom has space for fdt
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (6 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 07/23] RISC-V: Remove unused class definitions Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 09/23] RISC-V: Include intruction hex in disassembly Michael Clark
                   ` (15 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

Remove a potential buffer overflow (not seen in practice).
Perhaps cpu_physical_memory_write already has bound checks.
This change however makes space for the maximum device tree
size and adds an explicit bounds check and error message.
It doesn't trigger, but it may help in the future if the
device-tree size is exceeded. e.g. large bootargs.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 hw/riscv/sifive_u.c | 20 ++++++++++++--------
 hw/riscv/spike.c    | 16 +++++++++++-----
 hw/riscv/virt.c     | 13 +++++++++----
 3 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 083043a..57b4f4f 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 },
@@ -221,7 +221,7 @@ 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 *mask_rom = g_new(MemoryRegion, 1);
 
@@ -239,7 +239,7 @@ 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 */
@@ -247,9 +247,9 @@ static void riscv_sifive_u_init(MachineState *machine)
 
     /* boot rom */
     memory_region_init_ram(mask_rom, NULL, "riscv.sifive.u.mrom",
-                           memmap[SIFIVE_U_MROM].base, &error_fatal);
-    memory_region_set_readonly(mask_rom, true);
-    memory_region_add_subregion(sys_memory, 0x0, mask_rom);
+                           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);
@@ -276,6 +276,10 @@ static void riscv_sifive_u_init(MachineState *machine)
     copy_le32_to_phys(memmap[SIFIVE_U_MROM].base, reset_vec, sizeof(reset_vec));
 
     /* 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);
@@ -293,9 +297,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 64e585e..c7d937b 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -46,7 +46,7 @@ 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 },
 };
@@ -197,8 +197,9 @@ static void spike_v1_10_0_board_init(MachineState *machine)
 
     /* boot rom */
     memory_region_init_ram(mask_rom, NULL, "riscv.spike.mrom",
-                           s->fdt_size + 0x2000, &error_fatal);
-    memory_region_add_subregion(system_memory, 0x0, mask_rom);
+                           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);
@@ -225,6 +226,10 @@ static void spike_v1_10_0_board_init(MachineState *machine)
     copy_le32_to_phys(memmap[SPIKE_MROM].base, reset_vec, sizeof(reset_vec));
 
     /* 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);
@@ -266,8 +271,9 @@ static void spike_v1_09_1_board_init(MachineState *machine)
 
     /* boot rom */
     memory_region_init_ram(mask_rom, NULL, "riscv.spike.mrom",
-                           0x40000, &error_fatal);
-    memory_region_add_subregion(system_memory, 0x0, mask_rom);
+                           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);
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 5913100..d680cbd 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 },
@@ -297,8 +297,9 @@ static void riscv_virt_board_init(MachineState *machine)
 
     /* boot rom */
     memory_region_init_ram(mask_rom, NULL, "riscv_virt_board.mrom",
-                           s->fdt_size + 0x2000, &error_fatal);
-    memory_region_add_subregion(system_memory, 0x0, mask_rom);
+                           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);
@@ -336,6 +337,10 @@ static void riscv_virt_board_init(MachineState *machine)
     copy_le32_to_phys(memmap[VIRT_MROM].base, reset_vec, sizeof(reset_vec));
 
     /* 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);
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 09/23] RISC-V: Include intruction hex in disassembly
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (7 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 08/23] RISC-V: Make sure rom has space for fdt Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 10/23] RISC-V: Hold rcu_read_lock when accessing memory Michael Clark
                   ` (14 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

This was added to help debug issues using -d in_asm. It is
useful to see the instruction bytes, as one can detect if
one is trying to execute ASCII or device-tree magic.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 disas/riscv.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index 3c17501..4580308 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -2769,25 +2769,6 @@ static void format_inst(char *buf, size_t buflen, size_t tab, rv_decode *dec)
     char tmp[64];
     const char *fmt;
 
-    if (dec->op == rv_op_illegal) {
-        size_t len = inst_length(dec->inst);
-        switch (len) {
-        case 2:
-            snprintf(buf, buflen, "(0x%04" PRIx64 ")", dec->inst);
-            break;
-        case 4:
-            snprintf(buf, buflen, "(0x%08" PRIx64 ")", dec->inst);
-            break;
-        case 6:
-            snprintf(buf, buflen, "(0x%012" PRIx64 ")", dec->inst);
-            break;
-        default:
-            snprintf(buf, buflen, "(0x%016" PRIx64 ")", dec->inst);
-            break;
-        }
-        return;
-    }
-
     fmt = opcode_data[dec->op].format;
     while (*fmt) {
         switch (*fmt) {
@@ -3004,6 +2985,11 @@ disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst)
     format_inst(buf, buflen, 16, &dec);
 }
 
+#define INST_FMT_2 "%04" PRIx64 "              "
+#define INST_FMT_4 "%08" PRIx64 "          "
+#define INST_FMT_6 "%012" PRIx64 "      "
+#define INST_FMT_8 "%016" PRIx64 "  "
+
 static int
 print_insn_riscv(bfd_vma memaddr, struct disassemble_info *info, rv_isa isa)
 {
@@ -3031,6 +3017,21 @@ print_insn_riscv(bfd_vma memaddr, struct disassemble_info *info, rv_isa isa)
         }
     }
 
+    switch (len) {
+    case 2:
+        (*info->fprintf_func)(info->stream, INST_FMT_2, inst);
+        break;
+    case 4:
+        (*info->fprintf_func)(info->stream, INST_FMT_4, inst);
+        break;
+    case 6:
+        (*info->fprintf_func)(info->stream, INST_FMT_6, inst);
+        break;
+    default:
+        (*info->fprintf_func)(info->stream, INST_FMT_8, inst);
+        break;
+    }
+
     disasm_inst(buf, sizeof(buf), isa, memaddr, inst);
     (*info->fprintf_func)(info->stream, "%s", buf);
 
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 10/23] RISC-V: Hold rcu_read_lock when accessing memory
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (8 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 09/23] RISC-V: Include intruction hex in disassembly Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-10 20:45   ` Philippe Mathieu-Daudé
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 11/23] RISC-V: Improve page table walker spec compliance Michael Clark
                   ` (13 subsequent siblings)
  23 siblings, 1 reply; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann,
	Stefan O'Rear, Palmer Dabbelt

>From reading other code that accesses memory regions directly,
it appears that the rcu_read_lock needs to be held. Note: the
original code for accessing RAM directly was added because
there is no other way to use atomic_cmpxchg on guest physical
address space.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
CC: Stefan O'Rear <sorear2@gmail.com>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 target/riscv/helper.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target/riscv/helper.c b/target/riscv/helper.c
index 02cbcea..228933c 100644
--- a/target/riscv/helper.c
+++ b/target/riscv/helper.c
@@ -209,6 +209,7 @@ restart:
                    as the PTE is no longer valid */
                 MemoryRegion *mr;
                 hwaddr l = sizeof(target_ulong), addr1;
+                rcu_read_lock();
                 mr = address_space_translate(cs->as, pte_addr,
                     &addr1, &l, false);
                 if (memory_access_is_direct(mr, true)) {
@@ -222,16 +223,19 @@ restart:
                     target_ulong old_pte =
                         atomic_cmpxchg(pte_pa, pte, updated_pte);
                     if (old_pte != pte) {
+                        rcu_read_unlock();
                         goto restart;
                     } else {
                         pte = updated_pte;
                     }
 #endif
                 } else {
+                    rcu_read_unlock();
                     /* misconfigured PTE in ROM (AD bits are not preset) or
                      * PTE is in IO space and can't be updated atomically */
                     return TRANSLATE_FAIL;
                 }
+                rcu_read_unlock();
             }
 
             /* for superpage mappings, make a fake leaf PTE for the TLB's
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 11/23] RISC-V: Improve page table walker spec compliance
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (9 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 10/23] RISC-V: Hold rcu_read_lock when accessing memory Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 12/23] RISC-V: Update E order and I extension order Michael Clark
                   ` (12 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

- Inline PTE_TABLE check for better readability
- Improve readibility of User page U mode and SUM test
- Disallow non U mode from fetching from User pages
- Add reserved PTE flag check: W or W|X
- Add misaligned PPN check
- Change access checks from ternary operator to if statements
- Improves page walker comments
- No measurable performance impact on dd test

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 target/riscv/cpu_bits.h |  2 --
 target/riscv/helper.c   | 59 ++++++++++++++++++++++++++++++++++---------------
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 64aa097..12b4757 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -407,5 +407,3 @@
 #define PTE_SOFT  0x300 /* Reserved for Software */
 
 #define PTE_PPN_SHIFT 10
-
-#define PTE_TABLE(PTE) (((PTE) & (PTE_V | PTE_R | PTE_W | PTE_X)) == PTE_V)
diff --git a/target/riscv/helper.c b/target/riscv/helper.c
index 228933c..162d5ec 100644
--- a/target/riscv/helper.c
+++ b/target/riscv/helper.c
@@ -185,16 +185,36 @@ restart:
 #endif
         target_ulong ppn = pte >> PTE_PPN_SHIFT;
 
-        if (PTE_TABLE(pte)) { /* next level of page table */
+        if (!(pte & PTE_V)) {
+            /* Invalid PTE */
+            return TRANSLATE_FAIL;
+        } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
+            /* Inner PTE, continue walking */
             base = ppn << PGSHIFT;
-        } else if ((pte & PTE_U) ? (mode == PRV_S) && !sum : !(mode == PRV_S)) {
-            break;
-        } else if (!(pte & PTE_V) || (!(pte & PTE_R) && (pte & PTE_W))) {
-            break;
-        } else if (access_type == MMU_INST_FETCH ? !(pte & PTE_X) :
-                  access_type == MMU_DATA_LOAD ?  !(pte & PTE_R) &&
-                  !(mxr && (pte & PTE_X)) : !((pte & PTE_R) && (pte & PTE_W))) {
-            break;
+        } else if ((pte & (PTE_R | PTE_W | PTE_X)) == PTE_W) {
+            /* Reserved leaf PTE flags: PTE_W */
+            return TRANSLATE_FAIL;
+        } else if ((pte & (PTE_R | PTE_W | PTE_X)) == (PTE_W | PTE_X)) {
+            /* Reserved leaf PTE flags: PTE_W + PTE_X */
+            return TRANSLATE_FAIL;
+        } else if ((pte & PTE_U) && ((mode != PRV_U) &&
+                   (!sum || access_type == MMU_INST_FETCH))) {
+            /* User PTE flags when not U mode and mstatus.SUM is not set,
+               or the access type is an instruction fetch */
+            return TRANSLATE_FAIL;
+        } else if (ppn & ((1ULL << ptshift) - 1)) {
+            /* Misasligned PPN */
+            return TRANSLATE_FAIL;
+        } else if (access_type == MMU_DATA_LOAD && !((pte & PTE_R) ||
+                   (mode != PRV_U && (pte & PTE_X) && mxr))) {
+            /* Read access check failed */
+            return TRANSLATE_FAIL;
+        } else if (access_type == MMU_DATA_STORE && !(pte & PTE_W)) {
+            /* Write access check failed */
+            return TRANSLATE_FAIL;
+        } else if (access_type == MMU_INST_FETCH && !(pte & PTE_X)) {
+            /* Fetch access check failed */
+            return TRANSLATE_FAIL;
         } else {
             /* if necessary, set accessed and dirty bits. */
             target_ulong updated_pte = pte | PTE_A |
@@ -202,11 +222,14 @@ restart:
 
             /* Page table updates need to be atomic with MTTCG enabled */
             if (updated_pte != pte) {
-                /* if accessed or dirty bits need updating, and the PTE is
-                 * in RAM, then we do so atomically with a compare and swap.
-                 * if the PTE is in IO space, then it can't be updated.
-                 * if the PTE changed, then we must re-walk the page table
-                   as the PTE is no longer valid */
+                /*
+                 * - if accessed or dirty bits need updating, and the PTE is
+                 *   in RAM, then we do so atomically with a compare and swap.
+                 * - if the PTE is in IO space or ROM, then it can't be updated
+                 *   and we return TRANSLATE_FAIL.
+                 * - if the PTE changed by the time we went to update it, then
+                 *   it is no longer valid and we must re-walk the page table.
+                 */
                 MemoryRegion *mr;
                 hwaddr l = sizeof(target_ulong), addr1;
                 rcu_read_lock();
@@ -243,15 +266,15 @@ restart:
             target_ulong vpn = addr >> PGSHIFT;
             *physical = (ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT;
 
-            if ((pte & PTE_R)) {
+            /* set permissions on the TLB entry */
+            if ((pte & PTE_R) || (mode != PRV_U && (pte & PTE_X) && mxr)) {
                 *prot |= PAGE_READ;
             }
             if ((pte & PTE_X)) {
                 *prot |= PAGE_EXEC;
             }
-           /* only add write permission on stores or if the page
-              is already dirty, so that we don't miss further
-              page table walks to update the dirty bit */
+            /* add write permission on stores or if the page is already dirty,
+               so that we TLB miss on later writes to update the dirty bit */
             if ((pte & PTE_W) &&
                     (access_type == MMU_DATA_STORE || (pte & PTE_D))) {
                 *prot |= PAGE_WRITE;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 12/23] RISC-V: Update E order and I extension order
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (10 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 11/23] RISC-V: Improve page table walker spec compliance Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 13/23] RISC-V: Make some header guards more specific Michael Clark
                   ` (11 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

Section 22.8 Subset Naming Convention of the RISC-V ISA Specification
defines the canonical order for extensions in the ISA string. It is
silent on the position of the E extension however E is a substitute
for I so it must come early in the extension list order. A comment
is added to state E and I are mutually exclusive, as the E extension
will be added to the RISC-V port in the future.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 target/riscv/cpu.c | 2 +-
 target/riscv/cpu.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4851890..d2ae56a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -26,7 +26,7 @@
 
 /* RISC-V CPU definitions */
 
-static const char riscv_exts[26] = "IMAFDQECLBJTPVNSUHKORWXYZG";
+static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
 
 const char * const riscv_int_regnames[] = {
   "zero", "ra  ", "sp  ", "gp  ", "tp  ", "t0  ", "t1  ", "t2  ",
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index cff02a2..3a0ca2f 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -71,6 +71,7 @@
 #define RV(x) ((target_ulong)1 << (x - 'A'))
 
 #define RVI RV('I')
+#define RVE RV('E') /* E and I are mutually exclusive */
 #define RVM RV('M')
 #define RVA RV('A')
 #define RVF RV('F')
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 13/23] RISC-V: Make some header guards more specific
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (11 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 12/23] RISC-V: Update E order and I extension order Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-10 20:30   ` Philippe Mathieu-Daudé
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 14/23] RISC-V: Make virt header comment title consistent Michael Clark
                   ` (10 subsequent siblings)
  23 siblings, 1 reply; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 include/hw/riscv/spike.h | 4 ++--
 include/hw/riscv/virt.h  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/hw/riscv/spike.h b/include/hw/riscv/spike.h
index 8410430..641b70d 100644
--- a/include/hw/riscv/spike.h
+++ b/include/hw/riscv/spike.h
@@ -16,8 +16,8 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef HW_SPIKE_H
-#define HW_SPIKE_H
+#ifndef HW_RISCV_SPIKE_H
+#define HW_RISCV_SPIKE_H
 
 typedef struct {
     /*< private >*/
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index b91a412..3a4f23e 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -16,8 +16,8 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef HW_VIRT_H
-#define HW_VIRT_H
+#ifndef HW_RISCV_VIRT_H
+#define HW_RISCV_VIRT_H
 
 typedef struct {
     /*< private >*/
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 14/23] RISC-V: Make virt header comment title consistent
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (12 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 13/23] RISC-V: Make some header guards more specific Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-10 20:31   ` Philippe Mathieu-Daudé
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 15/23] RISC-V: Use memory_region_is_ram in pte update Michael Clark
                   ` (9 subsequent siblings)
  23 siblings, 1 reply; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 include/hw/riscv/virt.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 3a4f23e..91163d6 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -1,5 +1,5 @@
 /*
- * SiFive VirtIO Board
+ * QEMU RISC-V VirtIO machine interface
  *
  * Copyright (c) 2017 SiFive, Inc.
  *
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 15/23] RISC-V: Use memory_region_is_ram in pte update
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (13 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 14/23] RISC-V: Make virt header comment title consistent Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-10 20:42   ` Philippe Mathieu-Daudé
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 16/23] RISC-V: Remove EM_RISCV ELF_MACHINE indirection Michael Clark
                   ` (8 subsequent siblings)
  23 siblings, 1 reply; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

After reading cpu_physical_memory_write and friends, it seems
that memory_region_is_ram is a more appropriate interface,
and matches the intent of the code that is calling it.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 target/riscv/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/helper.c b/target/riscv/helper.c
index 162d5ec..fc550d3 100644
--- a/target/riscv/helper.c
+++ b/target/riscv/helper.c
@@ -235,7 +235,7 @@ restart:
                 rcu_read_lock();
                 mr = address_space_translate(cs->as, pte_addr,
                     &addr1, &l, false);
-                if (memory_access_is_direct(mr, true)) {
+                if (memory_region_is_ram(mr)) {
                     target_ulong *pte_pa =
                         qemu_map_ram_ptr(mr->ram_block, addr1);
 #if TCG_OVERSIZED_GUEST
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 16/23] RISC-V: Remove EM_RISCV ELF_MACHINE indirection
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (14 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 15/23] RISC-V: Use memory_region_is_ram in pte update Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-10 20:32   ` Philippe Mathieu-Daudé
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 17/23] RISC-V: Hardwire satp to 0 for no-mmu case Michael Clark
                   ` (7 subsequent siblings)
  23 siblings, 1 reply; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

Pointless indirection. Other ports use EM_ constants directly.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 hw/riscv/sifive_e.c | 2 +-
 hw/riscv/sifive_u.c | 2 +-
 hw/riscv/spike.c    | 2 +-
 hw/riscv/virt.c     | 2 +-
 target/riscv/cpu.h  | 1 -
 5 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 4872b68..39e4cb4 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -88,7 +88,7 @@ static uint64_t load_kernel(const char *kernel_filename)
 
     if (load_elf(kernel_filename, NULL, NULL,
                  &kernel_entry, NULL, &kernel_high,
-                 0, ELF_MACHINE, 1, 0) < 0) {
+                 0, EM_RISCV, 1, 0) < 0) {
         error_report("qemu: could not load kernel '%s'", kernel_filename);
         exit(1);
     }
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 57b4f4f..0e633a0 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -74,7 +74,7 @@ static uint64_t load_kernel(const char *kernel_filename)
 
     if (load_elf(kernel_filename, NULL, NULL,
                  &kernel_entry, NULL, &kernel_high,
-                 0, ELF_MACHINE, 1, 0) < 0) {
+                 0, EM_RISCV, 1, 0) < 0) {
         error_report("qemu: could not load kernel '%s'", kernel_filename);
         exit(1);
     }
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index c7d937b..70e697c 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -64,7 +64,7 @@ static uint64_t load_kernel(const char *kernel_filename)
     uint64_t kernel_entry, kernel_high;
 
     if (load_elf_ram_sym(kernel_filename, NULL, NULL,
-            &kernel_entry, NULL, &kernel_high, 0, ELF_MACHINE, 1, 0,
+            &kernel_entry, NULL, &kernel_high, 0, EM_RISCV, 1, 0,
             NULL, true, htif_symbol_callback) < 0) {
         error_report("qemu: could not load kernel '%s'", kernel_filename);
         exit(1);
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index d680cbd..e3f8bb7 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -68,7 +68,7 @@ static uint64_t load_kernel(const char *kernel_filename)
 
     if (load_elf(kernel_filename, NULL, NULL,
                  &kernel_entry, NULL, &kernel_high,
-                 0, ELF_MACHINE, 1, 0) < 0) {
+                 0, EM_RISCV, 1, 0) < 0) {
         error_report("qemu: could not load kernel '%s'", kernel_filename);
         exit(1);
     }
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 3a0ca2f..7c4482b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -34,7 +34,6 @@
 
 #define TCG_GUEST_DEFAULT_MO 0
 
-#define ELF_MACHINE EM_RISCV
 #define CPUArchState struct CPURISCVState
 
 #include "qemu-common.h"
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 17/23] RISC-V: Hardwire satp to 0 for no-mmu case
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (15 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 16/23] RISC-V: Remove EM_RISCV ELF_MACHINE indirection Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 18/23] RISC-V: Remove braces from satp case statement Michael Clark
                   ` (6 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

satp is WARL so it should not trap on illegal writes, rather
it can be hardwired to zero and silently ignore illegal writes.

It seems the RISC-V WARL behaviour is preferred to having to
trap overhead versus simply reading back the value and checking
if the write took (saves hundreds of cycles and more complex
trap handling code).

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 target/riscv/op_helper.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index e34715d..dd3e417 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -242,7 +242,7 @@ void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
     }
     case CSR_SATP: /* CSR_SPTBR */ {
         if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
-            goto do_illegal;
+            break;
         }
         if (env->priv_ver <= PRIV_VERSION_1_09_1 && (val_to_write ^ env->sptbr))
         {
@@ -452,7 +452,10 @@ target_ulong csr_read_helper(CPURISCVState *env, target_ulong csrno)
         return env->scounteren;
     case CSR_SCAUSE:
         return env->scause;
-    case CSR_SPTBR:
+    case CSR_SATP: /* CSR_SPTBR */
+        if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
+            return 0;
+        }
         if (env->priv_ver >= PRIV_VERSION_1_10_0) {
             return env->satp;
         } else {
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 18/23] RISC-V: Remove braces from satp case statement
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (16 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 17/23] RISC-V: Hardwire satp to 0 for no-mmu case Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 19/23] RISC-V: riscv-qemu port supports sv39 and sv48 Michael Clark
                   ` (5 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/riscv/op_helper.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index dd3e417..f79716a 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -240,7 +240,7 @@ void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
         csr_write_helper(env, next_mie, CSR_MIE);
         break;
     }
-    case CSR_SATP: /* CSR_SPTBR */ {
+    case CSR_SATP: /* CSR_SPTBR */
         if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
             break;
         }
@@ -258,7 +258,6 @@ void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
             env->satp = val_to_write;
         }
         break;
-    }
     case CSR_SEPC:
         env->sepc = val_to_write;
         break;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 19/23] RISC-V: riscv-qemu port supports sv39 and sv48
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (17 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 18/23] RISC-V: Remove braces from satp case statement Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 20/23] RISC-V: vectored traps are optional Michael Clark
                   ` (4 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 target/riscv/cpu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 7c4482b..f47fc9c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -24,8 +24,8 @@
 #define TARGET_PAGE_BITS 12 /* 4 KiB Pages */
 #if defined(TARGET_RISCV64)
 #define TARGET_LONG_BITS 64
-#define TARGET_PHYS_ADDR_SPACE_BITS 50
-#define TARGET_VIRT_ADDR_SPACE_BITS 39
+#define TARGET_PHYS_ADDR_SPACE_BITS 52
+#define TARGET_VIRT_ADDR_SPACE_BITS 48
 #elif defined(TARGET_RISCV32)
 #define TARGET_LONG_BITS 32
 #define TARGET_PHYS_ADDR_SPACE_BITS 34
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 20/23] RISC-V: vectored traps are optional
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (18 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 19/23] RISC-V: riscv-qemu port supports sv39 and sv48 Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 21/23] RISC-V: No traps on writes to misa, minstret, mcycle Michael Clark
                   ` (3 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

Vectored traps for asynchrounous interrupts are optional.
The mtvec/stvec mode field is WARL and hence does not trap
if an illegal value is written. Illegal values are ignored.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 target/riscv/op_helper.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index f79716a..aa101cc 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -262,11 +262,10 @@ void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
         env->sepc = val_to_write;
         break;
     case CSR_STVEC:
-        if (val_to_write & 1) {
-            qemu_log_mask(LOG_UNIMP, "CSR_STVEC: vectored traps not supported");
-            goto do_illegal;
+        /* we do not support vectored traps for asynchrounous interrupts */ 
+        if ((val_to_write & 3) == 0) {
+            env->stvec = val_to_write >> 2 << 2;
         }
-        env->stvec = val_to_write >> 2 << 2;
         break;
     case CSR_SCOUNTEREN:
         env->scounteren = val_to_write;
@@ -284,11 +283,10 @@ void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
         env->mepc = val_to_write;
         break;
     case CSR_MTVEC:
-        if (val_to_write & 1) {
-            qemu_log_mask(LOG_UNIMP, "CSR_MTVEC: vectored traps not supported");
-            goto do_illegal;
+        /* we do not support vectored traps for asynchrounous interrupts */ 
+        if ((val_to_write & 3) == 0) {
+            env->mtvec = val_to_write >> 2 << 2;
         }
-        env->mtvec = val_to_write >> 2 << 2;
         break;
     case CSR_MCOUNTEREN:
         env->mcounteren = val_to_write;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 21/23] RISC-V: No traps on writes to misa, minstret, mcycle
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (19 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 20/23] RISC-V: vectored traps are optional Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 22/23] RISC-V: Remove support for adhoc X_COP interrupt Michael Clark
                   ` (2 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

These fields are marked WARL in the specification so illegal
writes are silently dropped.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 target/riscv/op_helper.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index aa101cc..f8595a6 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -200,17 +200,19 @@ void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
         break;
     }
     case CSR_MINSTRET:
-        qemu_log_mask(LOG_UNIMP, "CSR_MINSTRET: write not implemented");
-        goto do_illegal;
+        /* minstret is WARL so unsupported writes are ignored */
+        break;
     case CSR_MCYCLE:
-        qemu_log_mask(LOG_UNIMP, "CSR_MCYCLE: write not implemented");
-        goto do_illegal;
+        /* mcycle is WARL so unsupported writes are ignored */
+        break;
+#if defined(TARGET_RISCV32)
     case CSR_MINSTRETH:
-        qemu_log_mask(LOG_UNIMP, "CSR_MINSTRETH: write not implemented");
-        goto do_illegal;
+        /* minstreth is WARL so unsupported writes are ignored */
+        break;
     case CSR_MCYCLEH:
-        qemu_log_mask(LOG_UNIMP, "CSR_MCYCLEH: write not implemented");
-        goto do_illegal;
+        /* mcycleh is WARL so unsupported writes are ignored */
+        break;
+#endif
     case CSR_MUCOUNTEREN:
         env->mucounteren = val_to_write;
         break;
@@ -300,10 +302,9 @@ void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
     case CSR_MBADADDR:
         env->mbadaddr = val_to_write;
         break;
-    case CSR_MISA: {
-        qemu_log_mask(LOG_UNIMP, "CSR_MISA: misa writes not supported");
-        goto do_illegal;
-    }
+    case CSR_MISA:
+        /* misa is WARL so unsupported writes are ignored */
+        break;
     case CSR_PMPCFG0:
     case CSR_PMPCFG1:
     case CSR_PMPCFG2:
@@ -328,7 +329,6 @@ void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
     case CSR_PMPADDR15:
        pmpaddr_csr_write(env, csrno - CSR_PMPADDR0, val_to_write);
        break;
-    do_illegal:
 #endif
     default:
         do_raise_exception_err(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 22/23] RISC-V: Remove support for adhoc X_COP interrupt
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (20 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 21/23] RISC-V: No traps on writes to misa, minstret, mcycle Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 23/23] RISC-V: Convert cpu definition towards future model Michael Clark
  2018-03-12 17:34 ` [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup no-reply
  23 siblings, 0 replies; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt

This is essentially dead-code elimination. Support for more
local interrupts will be added in a future revision, as they
will be defined in a future version of the Privileged ISA
specification.

Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 target/riscv/cpu_bits.h  | 1 -
 target/riscv/op_helper.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 12b4757..133e070 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -346,7 +346,6 @@
 #define IRQ_S_EXT       9
 #define IRQ_H_EXT       10 /* until: priv-1.9.1 */
 #define IRQ_M_EXT       11 /* until: priv-1.9.1 */
-#define IRQ_X_COP       12 /* non-standard */
 
 /* Default addresses */
 #define DEFAULT_RSTVEC     0x00001000
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index f8595a6..f543e61 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -90,7 +90,7 @@ void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
         target_ulong csrno)
 {
 #ifndef CONFIG_USER_ONLY
-    uint64_t delegable_ints = MIP_SSIP | MIP_STIP | MIP_SEIP | (1 << IRQ_X_COP);
+    uint64_t delegable_ints = MIP_SSIP | MIP_STIP | MIP_SEIP;
     uint64_t all_ints = delegable_ints | MIP_MSIP | MIP_MTIP;
 #endif
 
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Qemu-devel] [PATCH v2 23/23] RISC-V: Convert cpu definition towards future model
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (21 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 22/23] RISC-V: Remove support for adhoc X_COP interrupt Michael Clark
@ 2018-03-09  4:12 ` Michael Clark
  2018-03-10 20:30   ` Philippe Mathieu-Daudé
  2018-03-12 17:34 ` [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup no-reply
  23 siblings, 1 reply; 35+ messages in thread
From: Michael Clark @ 2018-03-09  4:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Clark, Igor Mammedov, Palmer Dabbelt, Sagar Karandikar,
	Bastian Koppelmann

- Model borrowed from target/sh4/cpu.c
- Rewrote riscv_cpu_list to use object_class_get_list
- Dropped 'struct RISCVCPUInfo' and used TypeInfo array
- Replaced riscv_cpu_register_types with DEFINE_TYPES
- Marked base class as abstract

Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by Michael Clark <mjc@sifive.com>
---
 target/riscv/cpu.c | 123 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 69 insertions(+), 54 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d2ae56a..1f25968 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -115,6 +115,8 @@ static void riscv_any_cpu_init(Object *obj)
     set_resetvec(env, DEFAULT_RSTVEC);
 }
 
+#if defined(TARGET_RISCV32)
+
 static void rv32gcsu_priv1_09_1_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -141,6 +143,8 @@ static void rv32imacu_nommu_cpu_init(Object *obj)
     set_resetvec(env, DEFAULT_RSTVEC);
 }
 
+#elif defined(TARGET_RISCV64)
+
 static void rv64gcsu_priv1_09_1_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -167,20 +171,7 @@ static void rv64imacu_nommu_cpu_init(Object *obj)
     set_resetvec(env, DEFAULT_RSTVEC);
 }
 
-static const RISCVCPUInfo riscv_cpus[] = {
-    { 96, TYPE_RISCV_CPU_ANY,              riscv_any_cpu_init },
-    { 32, TYPE_RISCV_CPU_RV32GCSU_V1_09_1, rv32gcsu_priv1_09_1_cpu_init },
-    { 32, TYPE_RISCV_CPU_RV32GCSU_V1_10_0, rv32gcsu_priv1_10_0_cpu_init },
-    { 32, TYPE_RISCV_CPU_RV32IMACU_NOMMU,  rv32imacu_nommu_cpu_init },
-    { 32, TYPE_RISCV_CPU_SIFIVE_E31,       rv32imacu_nommu_cpu_init },
-    { 32, TYPE_RISCV_CPU_SIFIVE_U34,       rv32gcsu_priv1_10_0_cpu_init },
-    { 64, TYPE_RISCV_CPU_RV64GCSU_V1_09_1, rv64gcsu_priv1_09_1_cpu_init },
-    { 64, TYPE_RISCV_CPU_RV64GCSU_V1_10_0, rv64gcsu_priv1_10_0_cpu_init },
-    { 64, TYPE_RISCV_CPU_RV64IMACU_NOMMU,  rv64imacu_nommu_cpu_init },
-    { 64, TYPE_RISCV_CPU_SIFIVE_E51,       rv64imacu_nommu_cpu_init },
-    { 64, TYPE_RISCV_CPU_SIFIVE_U54,       rv64gcsu_priv1_10_0_cpu_init },
-    { 0, NULL, NULL }
-};
+#endif
 
 static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
 {
@@ -366,28 +357,6 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
     cc->vmsd = &vmstate_riscv_cpu;
 }
 
-static void cpu_register(const RISCVCPUInfo *info)
-{
-    TypeInfo type_info = {
-        .name = info->name,
-        .parent = TYPE_RISCV_CPU,
-        .instance_size = sizeof(RISCVCPU),
-        .instance_init = info->initfn,
-    };
-
-    type_register(&type_info);
-}
-
-static const TypeInfo riscv_cpu_type_info = {
-    .name = TYPE_RISCV_CPU,
-    .parent = TYPE_CPU,
-    .instance_size = sizeof(RISCVCPU),
-    .instance_init = riscv_cpu_init,
-    .abstract = false,
-    .class_size = sizeof(RISCVCPUClass),
-    .class_init = riscv_cpu_class_init,
-};
-
 char *riscv_isa_string(RISCVCPU *cpu)
 {
     int i;
@@ -403,30 +372,76 @@ char *riscv_isa_string(RISCVCPU *cpu)
     return isa_string;
 }
 
-void riscv_cpu_list(FILE *f, fprintf_function cpu_fprintf)
+typedef struct RISCVCPUListState {
+    fprintf_function cpu_fprintf;
+    FILE *file;
+} RISCVCPUListState;
+
+static gint riscv_cpu_list_compare(gconstpointer a, gconstpointer b)
 {
-    const RISCVCPUInfo *info = riscv_cpus;
+    ObjectClass *class_a = (ObjectClass *)a;
+    ObjectClass *class_b = (ObjectClass *)b;
+    const char *name_a, *name_b;
 
-    while (info->name) {
-        if (info->bit_widths & TARGET_LONG_BITS) {
-            (*cpu_fprintf)(f, "%s\n", info->name);
-        }
-        info++;
-    }
+    name_a = object_class_get_name(class_a);
+    name_b = object_class_get_name(class_b);
+    return strcmp(name_a, name_b);
 }
 
-static void riscv_cpu_register_types(void)
+static void riscv_cpu_list_entry(gpointer data, gpointer user_data)
 {
-    const RISCVCPUInfo *info = riscv_cpus;
+    RISCVCPUListState *s = user_data;
+    const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    int len = strlen(typename) - strlen(RISCV_CPU_TYPE_SUFFIX);
 
-    type_register_static(&riscv_cpu_type_info);
+    (*s->cpu_fprintf)(s->file, "%.*s\n", len, typename);
+}
 
-    while (info->name) {
-        if (info->bit_widths & TARGET_LONG_BITS) {
-            cpu_register(info);
-        }
-        info++;
-    }
+void riscv_cpu_list(FILE *f, fprintf_function cpu_fprintf)
+{
+    RISCVCPUListState s = {
+        .cpu_fprintf = cpu_fprintf,
+        .file = f,
+    };
+    GSList *list;
+
+    list = object_class_get_list(TYPE_RISCV_CPU, false);
+    list = g_slist_sort(list, riscv_cpu_list_compare);
+    g_slist_foreach(list, riscv_cpu_list_entry, &s);
+    g_slist_free(list);
 }
 
-type_init(riscv_cpu_register_types)
+#define DEFINE_CPU(type_name, initfn)      \
+    {                                      \
+        .name = type_name,                 \
+        .parent = TYPE_RISCV_CPU,          \
+        .instance_init = initfn            \
+    }
+
+static const TypeInfo riscv_cpu_type_infos[] = {
+    {
+        .name = TYPE_RISCV_CPU,
+        .parent = TYPE_CPU,
+        .instance_size = sizeof(RISCVCPU),
+        .instance_init = riscv_cpu_init,
+        .abstract = true,
+        .class_size = sizeof(RISCVCPUClass),
+        .class_init = riscv_cpu_class_init,
+    },
+    DEFINE_CPU(TYPE_RISCV_CPU_ANY,              riscv_any_cpu_init),
+#if defined(TARGET_RISCV32)
+    DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_09_1, rv32gcsu_priv1_09_1_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_10_0, rv32gcsu_priv1_10_0_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_RV32IMACU_NOMMU,  rv32imacu_nommu_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31,       rv32imacu_nommu_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,       rv32gcsu_priv1_10_0_cpu_init)
+#elif defined(TARGET_RISCV64)
+    DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_09_1, rv64gcsu_priv1_09_1_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_10_0, rv64gcsu_priv1_10_0_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_RV64IMACU_NOMMU,  rv64imacu_nommu_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51,       rv64imacu_nommu_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,       rv64gcsu_priv1_10_0_cpu_init)
+#endif
+};
+
+DEFINE_TYPES(riscv_cpu_type_infos)
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [Qemu-devel] [PATCH v2 23/23] RISC-V: Convert cpu definition towards future model
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 23/23] RISC-V: Convert cpu definition towards future model Michael Clark
@ 2018-03-10 20:30   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-10 20:30 UTC (permalink / raw)
  To: Michael Clark, qemu-devel
  Cc: Igor Mammedov, Palmer Dabbelt, Sagar Karandikar, Bastian Koppelmann

Hi Michael,

On 03/09/2018 05:12 AM, Michael Clark wrote:
> - Model borrowed from target/sh4/cpu.c

> - Rewrote riscv_cpu_list to use object_class_get_list
> - Dropped 'struct RISCVCPUInfo' and used TypeInfo array
> - Replaced riscv_cpu_register_types with DEFINE_TYPES
> - Marked base class as abstract

Those could be 4 patches (easier to bisect bug, cherry-pick or rebase),
I'll let that choice to the maintainer, you :)

> 
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by Michael Clark <mjc@sifive.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/riscv/cpu.c | 123 ++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 69 insertions(+), 54 deletions(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index d2ae56a..1f25968 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -115,6 +115,8 @@ static void riscv_any_cpu_init(Object *obj)
>      set_resetvec(env, DEFAULT_RSTVEC);
>  }
>  
> +#if defined(TARGET_RISCV32)
> +
>  static void rv32gcsu_priv1_09_1_cpu_init(Object *obj)
>  {
>      CPURISCVState *env = &RISCV_CPU(obj)->env;
> @@ -141,6 +143,8 @@ static void rv32imacu_nommu_cpu_init(Object *obj)
>      set_resetvec(env, DEFAULT_RSTVEC);
>  }
>  
> +#elif defined(TARGET_RISCV64)
> +
>  static void rv64gcsu_priv1_09_1_cpu_init(Object *obj)
>  {
>      CPURISCVState *env = &RISCV_CPU(obj)->env;
> @@ -167,20 +171,7 @@ static void rv64imacu_nommu_cpu_init(Object *obj)
>      set_resetvec(env, DEFAULT_RSTVEC);
>  }
>  
> -static const RISCVCPUInfo riscv_cpus[] = {
> -    { 96, TYPE_RISCV_CPU_ANY,              riscv_any_cpu_init },
> -    { 32, TYPE_RISCV_CPU_RV32GCSU_V1_09_1, rv32gcsu_priv1_09_1_cpu_init },
> -    { 32, TYPE_RISCV_CPU_RV32GCSU_V1_10_0, rv32gcsu_priv1_10_0_cpu_init },
> -    { 32, TYPE_RISCV_CPU_RV32IMACU_NOMMU,  rv32imacu_nommu_cpu_init },
> -    { 32, TYPE_RISCV_CPU_SIFIVE_E31,       rv32imacu_nommu_cpu_init },
> -    { 32, TYPE_RISCV_CPU_SIFIVE_U34,       rv32gcsu_priv1_10_0_cpu_init },
> -    { 64, TYPE_RISCV_CPU_RV64GCSU_V1_09_1, rv64gcsu_priv1_09_1_cpu_init },
> -    { 64, TYPE_RISCV_CPU_RV64GCSU_V1_10_0, rv64gcsu_priv1_10_0_cpu_init },
> -    { 64, TYPE_RISCV_CPU_RV64IMACU_NOMMU,  rv64imacu_nommu_cpu_init },
> -    { 64, TYPE_RISCV_CPU_SIFIVE_E51,       rv64imacu_nommu_cpu_init },
> -    { 64, TYPE_RISCV_CPU_SIFIVE_U54,       rv64gcsu_priv1_10_0_cpu_init },
> -    { 0, NULL, NULL }
> -};
> +#endif
>  
>  static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
>  {
> @@ -366,28 +357,6 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
>      cc->vmsd = &vmstate_riscv_cpu;
>  }
>  
> -static void cpu_register(const RISCVCPUInfo *info)
> -{
> -    TypeInfo type_info = {
> -        .name = info->name,
> -        .parent = TYPE_RISCV_CPU,
> -        .instance_size = sizeof(RISCVCPU),
> -        .instance_init = info->initfn,
> -    };
> -
> -    type_register(&type_info);
> -}
> -
> -static const TypeInfo riscv_cpu_type_info = {
> -    .name = TYPE_RISCV_CPU,
> -    .parent = TYPE_CPU,
> -    .instance_size = sizeof(RISCVCPU),
> -    .instance_init = riscv_cpu_init,
> -    .abstract = false,
> -    .class_size = sizeof(RISCVCPUClass),
> -    .class_init = riscv_cpu_class_init,
> -};
> -
>  char *riscv_isa_string(RISCVCPU *cpu)
>  {
>      int i;
> @@ -403,30 +372,76 @@ char *riscv_isa_string(RISCVCPU *cpu)
>      return isa_string;
>  }
>  
> -void riscv_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> +typedef struct RISCVCPUListState {
> +    fprintf_function cpu_fprintf;
> +    FILE *file;
> +} RISCVCPUListState;
> +
> +static gint riscv_cpu_list_compare(gconstpointer a, gconstpointer b)
>  {
> -    const RISCVCPUInfo *info = riscv_cpus;
> +    ObjectClass *class_a = (ObjectClass *)a;
> +    ObjectClass *class_b = (ObjectClass *)b;
> +    const char *name_a, *name_b;
>  
> -    while (info->name) {
> -        if (info->bit_widths & TARGET_LONG_BITS) {
> -            (*cpu_fprintf)(f, "%s\n", info->name);
> -        }
> -        info++;
> -    }
> +    name_a = object_class_get_name(class_a);
> +    name_b = object_class_get_name(class_b);
> +    return strcmp(name_a, name_b);
>  }
>  
> -static void riscv_cpu_register_types(void)
> +static void riscv_cpu_list_entry(gpointer data, gpointer user_data)
>  {
> -    const RISCVCPUInfo *info = riscv_cpus;
> +    RISCVCPUListState *s = user_data;
> +    const char *typename = object_class_get_name(OBJECT_CLASS(data));
> +    int len = strlen(typename) - strlen(RISCV_CPU_TYPE_SUFFIX);
>  
> -    type_register_static(&riscv_cpu_type_info);
> +    (*s->cpu_fprintf)(s->file, "%.*s\n", len, typename);
> +}
>  
> -    while (info->name) {
> -        if (info->bit_widths & TARGET_LONG_BITS) {
> -            cpu_register(info);
> -        }
> -        info++;
> -    }
> +void riscv_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> +{
> +    RISCVCPUListState s = {
> +        .cpu_fprintf = cpu_fprintf,
> +        .file = f,
> +    };
> +    GSList *list;
> +
> +    list = object_class_get_list(TYPE_RISCV_CPU, false);
> +    list = g_slist_sort(list, riscv_cpu_list_compare);
> +    g_slist_foreach(list, riscv_cpu_list_entry, &s);
> +    g_slist_free(list);
>  }
>  
> -type_init(riscv_cpu_register_types)
> +#define DEFINE_CPU(type_name, initfn)      \
> +    {                                      \
> +        .name = type_name,                 \
> +        .parent = TYPE_RISCV_CPU,          \
> +        .instance_init = initfn            \
> +    }
> +
> +static const TypeInfo riscv_cpu_type_infos[] = {
> +    {
> +        .name = TYPE_RISCV_CPU,
> +        .parent = TYPE_CPU,
> +        .instance_size = sizeof(RISCVCPU),
> +        .instance_init = riscv_cpu_init,
> +        .abstract = true,
> +        .class_size = sizeof(RISCVCPUClass),
> +        .class_init = riscv_cpu_class_init,
> +    },
> +    DEFINE_CPU(TYPE_RISCV_CPU_ANY,              riscv_any_cpu_init),
> +#if defined(TARGET_RISCV32)
> +    DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_09_1, rv32gcsu_priv1_09_1_cpu_init),
> +    DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_10_0, rv32gcsu_priv1_10_0_cpu_init),
> +    DEFINE_CPU(TYPE_RISCV_CPU_RV32IMACU_NOMMU,  rv32imacu_nommu_cpu_init),
> +    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31,       rv32imacu_nommu_cpu_init),
> +    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,       rv32gcsu_priv1_10_0_cpu_init)
> +#elif defined(TARGET_RISCV64)
> +    DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_09_1, rv64gcsu_priv1_09_1_cpu_init),
> +    DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_10_0, rv64gcsu_priv1_10_0_cpu_init),
> +    DEFINE_CPU(TYPE_RISCV_CPU_RV64IMACU_NOMMU,  rv64imacu_nommu_cpu_init),
> +    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51,       rv64imacu_nommu_cpu_init),
> +    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,       rv64gcsu_priv1_10_0_cpu_init)
> +#endif
> +};
> +
> +DEFINE_TYPES(riscv_cpu_type_infos)
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [Qemu-devel] [PATCH v2 13/23] RISC-V: Make some header guards more specific
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 13/23] RISC-V: Make some header guards more specific Michael Clark
@ 2018-03-10 20:30   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-10 20:30 UTC (permalink / raw)
  To: Michael Clark, qemu-devel
  Cc: Bastian Koppelmann, Palmer Dabbelt, Sagar Karandikar

On 03/09/2018 05:12 AM, Michael Clark wrote:
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Michael Clark <mjc@sifive.com>
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  include/hw/riscv/spike.h | 4 ++--
>  include/hw/riscv/virt.h  | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/riscv/spike.h b/include/hw/riscv/spike.h
> index 8410430..641b70d 100644
> --- a/include/hw/riscv/spike.h
> +++ b/include/hw/riscv/spike.h
> @@ -16,8 +16,8 @@
>   * this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> -#ifndef HW_SPIKE_H
> -#define HW_SPIKE_H
> +#ifndef HW_RISCV_SPIKE_H
> +#define HW_RISCV_SPIKE_H
>  
>  typedef struct {
>      /*< private >*/
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index b91a412..3a4f23e 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -16,8 +16,8 @@
>   * this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> -#ifndef HW_VIRT_H
> -#define HW_VIRT_H
> +#ifndef HW_RISCV_VIRT_H
> +#define HW_RISCV_VIRT_H
>  
>  typedef struct {
>      /*< private >*/
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [Qemu-devel] [PATCH v2 14/23] RISC-V: Make virt header comment title consistent
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 14/23] RISC-V: Make virt header comment title consistent Michael Clark
@ 2018-03-10 20:31   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-10 20:31 UTC (permalink / raw)
  To: Michael Clark, qemu-devel
  Cc: Bastian Koppelmann, Palmer Dabbelt, Sagar Karandikar

On 03/09/2018 05:12 AM, Michael Clark wrote:
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Michael Clark <mjc@sifive.com>
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  include/hw/riscv/virt.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 3a4f23e..91163d6 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -1,5 +1,5 @@
>  /*
> - * SiFive VirtIO Board
> + * QEMU RISC-V VirtIO machine interface
>   *
>   * Copyright (c) 2017 SiFive, Inc.
>   *
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [Qemu-devel] [PATCH v2 16/23] RISC-V: Remove EM_RISCV ELF_MACHINE indirection
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 16/23] RISC-V: Remove EM_RISCV ELF_MACHINE indirection Michael Clark
@ 2018-03-10 20:32   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-10 20:32 UTC (permalink / raw)
  To: Michael Clark, qemu-devel
  Cc: Bastian Koppelmann, Palmer Dabbelt, Sagar Karandikar

On 03/09/2018 05:12 AM, Michael Clark wrote:
> Pointless indirection. Other ports use EM_ constants directly.
> 
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Michael Clark <mjc@sifive.com>
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/riscv/sifive_e.c | 2 +-
>  hw/riscv/sifive_u.c | 2 +-
>  hw/riscv/spike.c    | 2 +-
>  hw/riscv/virt.c     | 2 +-
>  target/riscv/cpu.h  | 1 -
>  5 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> index 4872b68..39e4cb4 100644
> --- a/hw/riscv/sifive_e.c
> +++ b/hw/riscv/sifive_e.c
> @@ -88,7 +88,7 @@ static uint64_t load_kernel(const char *kernel_filename)
>  
>      if (load_elf(kernel_filename, NULL, NULL,
>                   &kernel_entry, NULL, &kernel_high,
> -                 0, ELF_MACHINE, 1, 0) < 0) {
> +                 0, EM_RISCV, 1, 0) < 0) {
>          error_report("qemu: could not load kernel '%s'", kernel_filename);
>          exit(1);
>      }
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 57b4f4f..0e633a0 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -74,7 +74,7 @@ static uint64_t load_kernel(const char *kernel_filename)
>  
>      if (load_elf(kernel_filename, NULL, NULL,
>                   &kernel_entry, NULL, &kernel_high,
> -                 0, ELF_MACHINE, 1, 0) < 0) {
> +                 0, EM_RISCV, 1, 0) < 0) {
>          error_report("qemu: could not load kernel '%s'", kernel_filename);
>          exit(1);
>      }
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index c7d937b..70e697c 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -64,7 +64,7 @@ static uint64_t load_kernel(const char *kernel_filename)
>      uint64_t kernel_entry, kernel_high;
>  
>      if (load_elf_ram_sym(kernel_filename, NULL, NULL,
> -            &kernel_entry, NULL, &kernel_high, 0, ELF_MACHINE, 1, 0,
> +            &kernel_entry, NULL, &kernel_high, 0, EM_RISCV, 1, 0,
>              NULL, true, htif_symbol_callback) < 0) {
>          error_report("qemu: could not load kernel '%s'", kernel_filename);
>          exit(1);
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index d680cbd..e3f8bb7 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -68,7 +68,7 @@ static uint64_t load_kernel(const char *kernel_filename)
>  
>      if (load_elf(kernel_filename, NULL, NULL,
>                   &kernel_entry, NULL, &kernel_high,
> -                 0, ELF_MACHINE, 1, 0) < 0) {
> +                 0, EM_RISCV, 1, 0) < 0) {
>          error_report("qemu: could not load kernel '%s'", kernel_filename);
>          exit(1);
>      }
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 3a0ca2f..7c4482b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -34,7 +34,6 @@
>  
>  #define TCG_GUEST_DEFAULT_MO 0
>  
> -#define ELF_MACHINE EM_RISCV
>  #define CPUArchState struct CPURISCVState
>  
>  #include "qemu-common.h"
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [Qemu-devel] [PATCH v2 01/23] RISC-V: Make virt create_fdt interface consistent
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 01/23] RISC-V: Make virt create_fdt interface consistent Michael Clark
@ 2018-03-10 20:33   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-10 20:33 UTC (permalink / raw)
  To: Michael Clark, qemu-devel
  Cc: Bastian Koppelmann, Palmer Dabbelt, Sagar Karandikar

On 03/09/2018 05:12 AM, Michael Clark wrote:
> create_fdt sets the fdt variable on RISCVVirtState and this is
> used to access the fdt. This reverts a change introduced in
> https://github.com/riscv/riscv-qemu/pull/109 which introduced
> a redundant return value, overlooking the RISCVVirtState
> structure member that made create_fdt inconsistent with the
> other RISC-V machines. The other alternative is to change
> the other boards to return the fdt. Note: the RISCVVirtState
> also contains fdt_size.
> 
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Michael Clark <mjc@sifive.com>
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/riscv/virt.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index e2c214e..37968d2 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -108,7 +108,7 @@ static hwaddr load_initrd(const char *filename, uint64_t mem_size,
>      return *start + size;
>  }
>  
> -static void *create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
> +static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
>      uint64_t mem_size, const char *cmdline)
>  {
>      void *fdt;
> @@ -264,8 +264,6 @@ static void *create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
>      qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename);
>      qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
>      g_free(nodename);
> -
> -    return fdt;
>  }
>  
>  static void riscv_virt_board_init(MachineState *machine)
> @@ -279,7 +277,6 @@ static void riscv_virt_board_init(MachineState *machine)
>      char *plic_hart_config;
>      size_t plic_hart_config_len;
>      int i;
> -    void *fdt;
>  
>      /* Initialize SOC */
>      object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
> @@ -299,7 +296,7 @@ static void riscv_virt_board_init(MachineState *machine)
>          main_mem);
>  
>      /* create device tree */
> -    fdt = create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
> +    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
>  
>      /* boot rom */
>      memory_region_init_ram(boot_rom, NULL, "riscv_virt_board.bootrom",
> @@ -314,9 +311,9 @@ static void riscv_virt_board_init(MachineState *machine)
>              hwaddr end = load_initrd(machine->initrd_filename,
>                                       machine->ram_size, kernel_entry,
>                                       &start);
> -            qemu_fdt_setprop_cell(fdt, "/chosen",
> -                                  "linux,initrd-start", start);
> -            qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
> +            qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-start",
> +                                  start);
> +            qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-end",
>                                    end);
>          }
>      }
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [Qemu-devel] [PATCH v2 03/23] RISC-V: Make virt board description match spike
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 03/23] RISC-V: Make virt board description match spike Michael Clark
@ 2018-03-10 20:34   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-10 20:34 UTC (permalink / raw)
  To: Michael Clark, qemu-devel
  Cc: Bastian Koppelmann, Palmer Dabbelt, Sagar Karandikar

On 03/09/2018 05:12 AM, Michael Clark wrote:
> This makes 'qemu-system-riscv64 -machine help' output more tidy
> and consistent.
> 
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Michael Clark <mjc@sifive.com>
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/riscv/virt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index a402856..0055439 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -404,7 +404,7 @@ static const TypeInfo riscv_virt_board_device = {
>  
>  static void riscv_virt_board_machine_init(MachineClass *mc)
>  {
> -    mc->desc = "RISC-V VirtIO Board (Privileged spec v1.10)";
> +    mc->desc = "RISC-V VirtIO Board (Privileged ISA v1.10)";
>      mc->init = riscv_virt_board_init;
>      mc->max_cpus = 8; /* hardcoded limit in BBL */
>  }
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [Qemu-devel] [PATCH v2 04/23] RISC-V: Use ROM base address and size from memmap
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 04/23] RISC-V: Use ROM base address and size from memmap Michael Clark
@ 2018-03-10 20:35   ` Philippe Mathieu-Daudé
  2018-03-12 18:24   ` Eric Blake
  1 sibling, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-10 20:35 UTC (permalink / raw)
  To: Michael Clark, qemu-devel
  Cc: Bastian Koppelmann, Palmer Dabbelt, Sagar Karandikar

On 03/09/2018 05:12 AM, Michael Clark wrote:
> Another case of replaceing hard coded constants, this time
> referring to the definition in the virt machine's memmap.
> 
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Michael Clark <mjc@sifive.com>
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/riscv/virt.c         | 4 ++--
>  include/hw/riscv/virt.h | 2 --
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 0055439..0d101fc 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -338,11 +338,11 @@ static void riscv_virt_board_init(MachineState *machine)
>      };
>  
>      /* copy in the reset vector */
> -    copy_le32_to_phys(ROM_BASE, reset_vec, sizeof(reset_vec));
> +    copy_le32_to_phys(memmap[VIRT_MROM].base, reset_vec, sizeof(reset_vec));
>  
>      /* copy in the device tree */
>      qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
> -    cpu_physical_memory_write(ROM_BASE + sizeof(reset_vec),
> +    cpu_physical_memory_write(memmap[VIRT_MROM].base + sizeof(reset_vec),
>          s->fdt, s->fdt_size);
>  
>      /* create PLIC hart topology configuration string */
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 2fbe808..655e85d 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -23,8 +23,6 @@
>  #define VIRT(obj) \
>      OBJECT_CHECK(RISCVVirtState, (obj), TYPE_RISCV_VIRT_BOARD)
>  
> -enum { ROM_BASE = 0x1000 };
> -
>  typedef struct {
>      /*< private >*/
>      SysBusDevice parent_obj;
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [Qemu-devel] [PATCH v2 15/23] RISC-V: Use memory_region_is_ram in pte update
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 15/23] RISC-V: Use memory_region_is_ram in pte update Michael Clark
@ 2018-03-10 20:42   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-10 20:42 UTC (permalink / raw)
  To: Michael Clark, qemu-devel
  Cc: Bastian Koppelmann, Palmer Dabbelt, Sagar Karandikar

On 03/09/2018 05:12 AM, Michael Clark wrote:
> After reading cpu_physical_memory_write and friends, it seems
> that memory_region_is_ram is a more appropriate interface,
> and matches the intent of the code that is calling it.
> 
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Michael Clark <mjc@sifive.com>
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/riscv/helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/riscv/helper.c b/target/riscv/helper.c
> index 162d5ec..fc550d3 100644
> --- a/target/riscv/helper.c
> +++ b/target/riscv/helper.c
> @@ -235,7 +235,7 @@ restart:
>                  rcu_read_lock();
>                  mr = address_space_translate(cs->as, pte_addr,
>                      &addr1, &l, false);
> -                if (memory_access_is_direct(mr, true)) {
> +                if (memory_region_is_ram(mr)) {
>                      target_ulong *pte_pa =
>                          qemu_map_ram_ptr(mr->ram_block, addr1);
>  #if TCG_OVERSIZED_GUEST
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [Qemu-devel] [PATCH v2 10/23] RISC-V: Hold rcu_read_lock when accessing memory
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 10/23] RISC-V: Hold rcu_read_lock when accessing memory Michael Clark
@ 2018-03-10 20:45   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-10 20:45 UTC (permalink / raw)
  To: Michael Clark, qemu-devel
  Cc: Stefan O'Rear, Bastian Koppelmann, Palmer Dabbelt, Sagar Karandikar

Hi Michael,

On 03/09/2018 05:12 AM, Michael Clark wrote:
> From reading other code that accesses memory regions directly,
> it appears that the rcu_read_lock needs to be held. Note: the
> original code for accessing RAM directly was added because
> there is no other way to use atomic_cmpxchg on guest physical
> address space.
> 
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> CC: Stefan O'Rear <sorear2@gmail.com>
> Signed-off-by: Michael Clark <mjc@sifive.com>
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
> ---
>  target/riscv/helper.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/target/riscv/helper.c b/target/riscv/helper.c
> index 02cbcea..228933c 100644
> --- a/target/riscv/helper.c
> +++ b/target/riscv/helper.c
> @@ -209,6 +209,7 @@ restart:
>                     as the PTE is no longer valid */
>                  MemoryRegion *mr;
>                  hwaddr l = sizeof(target_ulong), addr1;
> +                rcu_read_lock();
>                  mr = address_space_translate(cs->as, pte_addr,
>                      &addr1, &l, false);
>                  if (memory_access_is_direct(mr, true)) {
> @@ -222,16 +223,19 @@ restart:
>                      target_ulong old_pte =
>                          atomic_cmpxchg(pte_pa, pte, updated_pte);
>                      if (old_pte != pte) {
> +                        rcu_read_unlock();
>                          goto restart;
>                      } else {
>                          pte = updated_pte;
>                      }
>  #endif
>                  } else {
> +                    rcu_read_unlock();
>                      /* misconfigured PTE in ROM (AD bits are not preset) or
>                       * PTE is in IO space and can't be updated atomically */
>                      return TRANSLATE_FAIL;
>                  }
> +                rcu_read_unlock();

Can you refactor to have a unique pair of lock/unlock?
This would be less bug-prone.

Thanks,

Phil.

>              }
>  
>              /* for superpage mappings, make a fake leaf PTE for the TLB's
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup
  2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
                   ` (22 preceding siblings ...)
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 23/23] RISC-V: Convert cpu definition towards future model Michael Clark
@ 2018-03-12 17:34 ` no-reply
  23 siblings, 0 replies; 35+ messages in thread
From: no-reply @ 2018-03-12 17:34 UTC (permalink / raw)
  To: mjc; +Cc: famz, qemu-devel, kbastian, palmer, sagark, pbonzini

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1520568765-58189-1-git-send-email-mjc@sifive.com
Subject: [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/1520530757-4477-1-git-send-email-ian.jackson@eu.citrix.com -> patchew/1520530757-4477-1-git-send-email-ian.jackson@eu.citrix.com
 * [new tag]               patchew/1520568765-58189-1-git-send-email-mjc@sifive.com -> patchew/1520568765-58189-1-git-send-email-mjc@sifive.com
 t [tag update]            patchew/20180228072558.7434-1-haozhong.zhang@intel.com -> patchew/20180228072558.7434-1-haozhong.zhang@intel.com
 t [tag update]            patchew/20180228093232.24854-1-pbonzini@redhat.com -> patchew/20180228093232.24854-1-pbonzini@redhat.com
 t [tag update]            patchew/20180306203103.25563-1-mark.cave-ayland@ilande.co.uk -> patchew/20180306203103.25563-1-mark.cave-ayland@ilande.co.uk
 t [tag update]            patchew/20180308184636.178534-1-vsementsov@virtuozzo.com -> patchew/20180308184636.178534-1-vsementsov@virtuozzo.com
 t [tag update]            patchew/20180308195811.24894-1-dgilbert@redhat.com -> patchew/20180308195811.24894-1-dgilbert@redhat.com
Switched to a new branch 'test'
1a093fa312 RISC-V: Convert cpu definition towards future model
60f285ab8d RISC-V: Remove support for adhoc X_COP interrupt
533511fbfb RISC-V: No traps on writes to misa, minstret, mcycle
395b5ff5df RISC-V: vectored traps are optional
01f250ec13 RISC-V: riscv-qemu port supports sv39 and sv48
728a31f9ba RISC-V: Remove braces from satp case statement
1eea106c3d RISC-V: Hardwire satp to 0 for no-mmu case
6624b330f8 RISC-V: Remove EM_RISCV ELF_MACHINE indirection
0c41b7c63e RISC-V: Use memory_region_is_ram in pte update
31404ec043 RISC-V: Make virt header comment title consistent
8c2d250fd4 RISC-V: Make some header guards more specific
bca06d8667 RISC-V: Update E order and I extension order
311f569e1a RISC-V: Improve page table walker spec compliance
0e3a08f02f RISC-V: Hold rcu_read_lock when accessing memory
f4b56b6603 RISC-V: Include intruction hex in disassembly
252c5a7b3f RISC-V: Make sure rom has space for fdt
07c1f8bc05 RISC-V: Remove unused class definitions
b55bf136ea RISC-V: Mark ROM read-only after copying in code
68db62e5ae RISC-V: Remove identity_translate from load_elf
089c002a16 RISC-V: Use ROM base address and size from memmap
560b8e3327 RISC-V: Make virt board description match spike
e7af5f102c RISC-V: Replace hardcoded constants with enum values
9f5038bb7b RISC-V: Make virt create_fdt interface consistent

=== OUTPUT BEGIN ===
Checking PATCH 1/23: RISC-V: Make virt create_fdt interface consistent...
Checking PATCH 2/23: RISC-V: Replace hardcoded constants with enum values...
Checking PATCH 3/23: RISC-V: Make virt board description match spike...
Checking PATCH 4/23: RISC-V: Use ROM base address and size from memmap...
Checking PATCH 5/23: RISC-V: Remove identity_translate from load_elf...
Checking PATCH 6/23: RISC-V: Mark ROM read-only after copying in code...
Checking PATCH 7/23: RISC-V: Remove unused class definitions...
Checking PATCH 8/23: RISC-V: Make sure rom has space for fdt...
Checking PATCH 9/23: RISC-V: Include intruction hex in disassembly...
Checking PATCH 10/23: RISC-V: Hold rcu_read_lock when accessing memory...
Checking PATCH 11/23: RISC-V: Improve page table walker spec compliance...
Checking PATCH 12/23: RISC-V: Update E order and I extension order...
Checking PATCH 13/23: RISC-V: Make some header guards more specific...
Checking PATCH 14/23: RISC-V: Make virt header comment title consistent...
Checking PATCH 15/23: RISC-V: Use memory_region_is_ram in pte update...
Checking PATCH 16/23: RISC-V: Remove EM_RISCV ELF_MACHINE indirection...
Checking PATCH 17/23: RISC-V: Hardwire satp to 0 for no-mmu case...
Checking PATCH 18/23: RISC-V: Remove braces from satp case statement...
Checking PATCH 19/23: RISC-V: riscv-qemu port supports sv39 and sv48...
Checking PATCH 20/23: RISC-V: vectored traps are optional...
ERROR: trailing whitespace
#27: FILE: target/riscv/op_helper.c:265:
+        /* we do not support vectored traps for asynchrounous interrupts */ $

ERROR: trailing whitespace
#42: FILE: target/riscv/op_helper.c:286:
+        /* we do not support vectored traps for asynchrounous interrupts */ $

total: 2 errors, 0 warnings, 28 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 21/23: RISC-V: No traps on writes to misa, minstret, mcycle...
Checking PATCH 22/23: RISC-V: Remove support for adhoc X_COP interrupt...
Checking PATCH 23/23: RISC-V: Convert cpu definition towards future model...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [Qemu-devel] [PATCH v2 04/23] RISC-V: Use ROM base address and size from memmap
  2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 04/23] RISC-V: Use ROM base address and size from memmap Michael Clark
  2018-03-10 20:35   ` Philippe Mathieu-Daudé
@ 2018-03-12 18:24   ` Eric Blake
  1 sibling, 0 replies; 35+ messages in thread
From: Eric Blake @ 2018-03-12 18:24 UTC (permalink / raw)
  To: Michael Clark, qemu-devel
  Cc: Bastian Koppelmann, Palmer Dabbelt, Sagar Karandikar

On 03/08/2018 10:12 PM, Michael Clark wrote:
> Another case of replaceing hard coded constants, this time

s/replaceing/replacing/

> referring to the definition in the virt machine's memmap.
> 
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Michael Clark <mjc@sifive.com>
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
> ---

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2018-03-12 18:25 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-09  4:12 [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 01/23] RISC-V: Make virt create_fdt interface consistent Michael Clark
2018-03-10 20:33   ` Philippe Mathieu-Daudé
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 02/23] RISC-V: Replace hardcoded constants with enum values Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 03/23] RISC-V: Make virt board description match spike Michael Clark
2018-03-10 20:34   ` Philippe Mathieu-Daudé
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 04/23] RISC-V: Use ROM base address and size from memmap Michael Clark
2018-03-10 20:35   ` Philippe Mathieu-Daudé
2018-03-12 18:24   ` Eric Blake
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 05/23] RISC-V: Remove identity_translate from load_elf Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 06/23] RISC-V: Mark ROM read-only after copying in code Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 07/23] RISC-V: Remove unused class definitions Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 08/23] RISC-V: Make sure rom has space for fdt Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 09/23] RISC-V: Include intruction hex in disassembly Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 10/23] RISC-V: Hold rcu_read_lock when accessing memory Michael Clark
2018-03-10 20:45   ` Philippe Mathieu-Daudé
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 11/23] RISC-V: Improve page table walker spec compliance Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 12/23] RISC-V: Update E order and I extension order Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 13/23] RISC-V: Make some header guards more specific Michael Clark
2018-03-10 20:30   ` Philippe Mathieu-Daudé
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 14/23] RISC-V: Make virt header comment title consistent Michael Clark
2018-03-10 20:31   ` Philippe Mathieu-Daudé
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 15/23] RISC-V: Use memory_region_is_ram in pte update Michael Clark
2018-03-10 20:42   ` Philippe Mathieu-Daudé
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 16/23] RISC-V: Remove EM_RISCV ELF_MACHINE indirection Michael Clark
2018-03-10 20:32   ` Philippe Mathieu-Daudé
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 17/23] RISC-V: Hardwire satp to 0 for no-mmu case Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 18/23] RISC-V: Remove braces from satp case statement Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 19/23] RISC-V: riscv-qemu port supports sv39 and sv48 Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 20/23] RISC-V: vectored traps are optional Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 21/23] RISC-V: No traps on writes to misa, minstret, mcycle Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 22/23] RISC-V: Remove support for adhoc X_COP interrupt Michael Clark
2018-03-09  4:12 ` [Qemu-devel] [PATCH v2 23/23] RISC-V: Convert cpu definition towards future model Michael Clark
2018-03-10 20:30   ` Philippe Mathieu-Daudé
2018-03-12 17:34 ` [Qemu-devel] [PATCH v2 00/23] RISC-V Post-merge spec conformance and cleanup no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.