All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v1 0/4] RISC-V: SoCify the SiFive boards and connect the
@ 2018-05-04 20:12 Alistair Francis
  2018-05-04 20:12 ` [Qemu-devel] [PATCH v1 1/4] hw/riscv/sifive_u: Create a U54 SoC object Alistair Francis
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Alistair Francis @ 2018-05-04 20:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis, alistair23, palmer, mjc

This series has two tasks:
 1. To conver the SiFive U and E machines into SoCs and boards
 2. To connect the Cadence GEM device to teh SiFive U board

After this series the SiFive E and U boards have their SoCs split into
seperate QEMU objects, which can be used on future boards if desired.

The RISC-V Virt and Spike boards have not been converted. They haven't
been converted as they aren't physical boards, so it doesn't make a
whole lot of sense to split them into an SoC and board. The only
disadvantage with this is that they now differ to the SiFive boards.

This series also connect the Cadence GEM device to the SiFive U board.
There are some interrupt line changes requried before this is possible.

Based-on: <1524699938-6764-1-git-send-email-mjc@sifive.com>

Alistair Francis (4):
  hw/riscv/sifive_u: Create a U54 SoC object
  hw/riscv/sifive_plic: Use gpios instead of irqs
  hw/riscv/sifive_u: Connect the Cadence GEM Ethernet device
  hw/riscv/sifive_e: Create a E31 SoC object

 default-configs/riscv32-softmmu.mak |   1 +
 default-configs/riscv64-softmmu.mak |   1 +
 hw/riscv/sifive_e.c                 |  97 +++++++++++++++++------
 hw/riscv/sifive_plic.c              |   5 +-
 hw/riscv/sifive_u.c                 | 119 +++++++++++++++++++++++-----
 include/hw/riscv/sifive_e.h         |  16 +++-
 include/hw/riscv/sifive_u.h         |  22 ++++-
 7 files changed, 205 insertions(+), 56 deletions(-)

-- 
2.17.0

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

* [Qemu-devel] [PATCH v1 1/4] hw/riscv/sifive_u: Create a U54 SoC object
  2018-05-04 20:12 [Qemu-devel] [PATCH v1 0/4] RISC-V: SoCify the SiFive boards and connect the Alistair Francis
@ 2018-05-04 20:12 ` Alistair Francis
  2018-05-04 22:55   ` Michael Clark
  2018-05-04 20:13 ` [Qemu-devel] [PATCH v1 2/4] hw/riscv/sifive_plic: Use gpios instead of irqs Alistair Francis
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Alistair Francis @ 2018-05-04 20:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis, alistair23, palmer, mjc

Create a SiFive Unleashed U54 SoC and use that in the sifive_u machine.

We leave the SoC, RAM, device tree and reset/fdt loading as part of the
machine. All the other device creation has been moved to the SoC.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/riscv/sifive_u.c         | 90 ++++++++++++++++++++++++++++---------
 include/hw/riscv/sifive_u.h | 16 ++++++-
 2 files changed, 82 insertions(+), 24 deletions(-)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 9f3d184b72..4924f92262 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -114,10 +114,10 @@ static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
     qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
     qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
 
-    for (cpu = s->soc.num_harts - 1; cpu >= 0; cpu--) {
+    for (cpu = s->soc.cpus.num_harts - 1; cpu >= 0; cpu--) {
         nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
         char *intc = g_strdup_printf("/cpus/cpu@%d/interrupt-controller", cpu);
-        char *isa = riscv_isa_string(&s->soc.harts[cpu]);
+        char *isa = riscv_isa_string(&s->soc.cpus.harts[cpu]);
         qemu_fdt_add_subnode(fdt, nodename);
         qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
                               SIFIVE_U_CLOCK_FREQ);
@@ -138,8 +138,8 @@ static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
         g_free(nodename);
     }
 
-    cells =  g_new0(uint32_t, s->soc.num_harts * 4);
-    for (cpu = 0; cpu < s->soc.num_harts; cpu++) {
+    cells =  g_new0(uint32_t, s->soc.cpus.num_harts * 4);
+    for (cpu = 0; cpu < s->soc.cpus.num_harts; cpu++) {
         nodename =
             g_strdup_printf("/cpus/cpu@%d/interrupt-controller", cpu);
         uint32_t intc_phandle = qemu_fdt_get_phandle(fdt, nodename);
@@ -157,12 +157,12 @@ static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
         0x0, memmap[SIFIVE_U_CLINT].base,
         0x0, memmap[SIFIVE_U_CLINT].size);
     qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
-        cells, s->soc.num_harts * sizeof(uint32_t) * 4);
+        cells, s->soc.cpus.num_harts * sizeof(uint32_t) * 4);
     g_free(cells);
     g_free(nodename);
 
-    cells =  g_new0(uint32_t, s->soc.num_harts * 4);
-    for (cpu = 0; cpu < s->soc.num_harts; cpu++) {
+    cells =  g_new0(uint32_t, s->soc.cpus.num_harts * 4);
+    for (cpu = 0; cpu < s->soc.cpus.num_harts; cpu++) {
         nodename =
             g_strdup_printf("/cpus/cpu@%d/interrupt-controller", cpu);
         uint32_t intc_phandle = qemu_fdt_get_phandle(fdt, nodename);
@@ -179,7 +179,7 @@ static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
     qemu_fdt_setprop_string(fdt, nodename, "compatible", "riscv,plic0");
     qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
     qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
-        cells, s->soc.num_harts * sizeof(uint32_t) * 4);
+        cells, s->soc.cpus.num_harts * sizeof(uint32_t) * 4);
     qemu_fdt_setprop_cells(fdt, nodename, "reg",
         0x0, memmap[SIFIVE_U_PLIC].base,
         0x0, memmap[SIFIVE_U_PLIC].size);
@@ -215,17 +215,12 @@ static void riscv_sifive_u_init(MachineState *machine)
     SiFiveUState *s = g_new0(SiFiveUState, 1);
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *main_mem = g_new(MemoryRegion, 1);
-    MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
     int i;
 
-    /* Initialize SOC */
-    object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
+    /* Initialize SoC */
+    object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_U54_SOC);
     object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
                               &error_abort);
-    object_property_set_str(OBJECT(&s->soc), SIFIVE_U_CPU, "cpu-type",
-                            &error_abort);
-    object_property_set_int(OBJECT(&s->soc), smp_cpus, "num-harts",
-                            &error_abort);
     object_property_set_bool(OBJECT(&s->soc), true, "realized",
                             &error_abort);
 
@@ -233,17 +228,11 @@ static void riscv_sifive_u_init(MachineState *machine)
     memory_region_init_ram(main_mem, NULL, "riscv.sifive.u.ram",
                            machine->ram_size, &error_fatal);
     memory_region_add_subregion(system_memory, memmap[SIFIVE_U_DRAM].base,
-        main_mem);
+                                main_mem);
 
     /* create device tree */
     create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
 
-    /* boot rom */
-    memory_region_init_rom(mask_rom, NULL, "riscv.sifive.u.mrom",
-                           memmap[SIFIVE_U_MROM].size, &error_fatal);
-    memory_region_add_subregion(system_memory, memmap[SIFIVE_U_MROM].base,
-                                mask_rom);
-
     if (machine->kernel_filename) {
         load_kernel(machine->kernel_filename);
     }
@@ -282,6 +271,39 @@ static void riscv_sifive_u_init(MachineState *machine)
     rom_add_blob_fixed_as("mrom.fdt", s->fdt, s->fdt_size,
                           memmap[SIFIVE_U_MROM].base + sizeof(reset_vec),
                           &address_space_memory);
+}
+
+static void riscv_sifive_u54_init(Object *obj)
+{
+    const struct MemmapEntry *memmap = sifive_u_memmap;
+
+    SiFiveU54State *s = RISCV_U54_SOC(obj);
+    MemoryRegion *system_memory = get_system_memory();
+    MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
+
+    object_initialize(&s->cpus, sizeof(s->cpus), TYPE_RISCV_HART_ARRAY);
+    object_property_add_child(obj, "cpus", OBJECT(&s->cpus),
+                              &error_abort);
+    object_property_set_str(OBJECT(&s->cpus), SIFIVE_U_CPU, "cpu-type",
+                            &error_abort);
+    object_property_set_int(OBJECT(&s->cpus), smp_cpus, "num-harts",
+                            &error_abort);
+
+    /* boot rom */
+    memory_region_init_rom(mask_rom, NULL, "riscv.sifive.u.mrom",
+                           memmap[SIFIVE_U_MROM].size, &error_fatal);
+    memory_region_add_subregion(system_memory, memmap[SIFIVE_U_MROM].base,
+                                mask_rom);
+}
+
+static void riscv_sifive_u54_realize(DeviceState *dev, Error **errp)
+{
+    SiFiveU54State *s = RISCV_U54_SOC(dev);
+    const struct MemmapEntry *memmap = sifive_u_memmap;
+    MemoryRegion *system_memory = get_system_memory();
+
+    object_property_set_bool(OBJECT(&s->cpus), true, "realized",
+                             &error_abort);
 
     /* MMIO */
     s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
@@ -312,3 +334,27 @@ static void riscv_sifive_u_machine_init(MachineClass *mc)
 }
 
 DEFINE_MACHINE("sifive_u", riscv_sifive_u_machine_init)
+
+static void riscv_sifive_u54_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = riscv_sifive_u54_realize;
+    /* Reason: Uses serial_hds in realize function, thus can't be used twice */
+    dc->user_creatable = false;
+}
+
+static const TypeInfo riscv_sifive_u54_type_info = {
+    .name = TYPE_RISCV_U54_SOC,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(SiFiveU54State),
+    .instance_init = riscv_sifive_u54_init,
+    .class_init = riscv_sifive_u54_class_init,
+};
+
+static void riscv_sifive_u54_register_types(void)
+{
+    type_register_static(&riscv_sifive_u54_type_info);
+}
+
+type_init(riscv_sifive_u54_register_types)
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index 94a390566e..0f8bdd8fab 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -19,13 +19,25 @@
 #ifndef HW_SIFIVE_U_H
 #define HW_SIFIVE_U_H
 
-typedef struct SiFiveUState {
+#define TYPE_RISCV_U54_SOC "riscv.sifive.u54"
+#define RISCV_U54_SOC(obj) \
+    OBJECT_CHECK(SiFiveU54State, (obj), TYPE_RISCV_U54_SOC)
+
+typedef struct SiFiveU54State {
     /*< private >*/
     SysBusDevice parent_obj;
 
     /*< public >*/
-    RISCVHartArrayState soc;
+    RISCVHartArrayState cpus;
     DeviceState *plic;
+} SiFiveU54State;
+
+typedef struct SiFiveUState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    SiFiveU54State soc;
     void *fdt;
     int fdt_size;
 } SiFiveUState;
-- 
2.17.0

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

* [Qemu-devel] [PATCH v1 2/4] hw/riscv/sifive_plic: Use gpios instead of irqs
  2018-05-04 20:12 [Qemu-devel] [PATCH v1 0/4] RISC-V: SoCify the SiFive boards and connect the Alistair Francis
  2018-05-04 20:12 ` [Qemu-devel] [PATCH v1 1/4] hw/riscv/sifive_u: Create a U54 SoC object Alistair Francis
@ 2018-05-04 20:13 ` Alistair Francis
  2018-05-10  2:33   ` Philippe Mathieu-Daudé
  2018-05-04 20:13 ` [Qemu-devel] [PATCH v1 3/4] hw/riscv/sifive_u: Connect the Cadence GEM Ethernet device Alistair Francis
  2018-05-04 20:13 ` [Qemu-devel] [PATCH v1 4/4] hw/riscv/sifive_e: Create a E31 SoC object Alistair Francis
  3 siblings, 1 reply; 9+ messages in thread
From: Alistair Francis @ 2018-05-04 20:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis, alistair23, palmer, mjc

Instead of creating the interrupt in lines with qemu_allocate_irq() use
qdev_init_gpio_in() as this gives us the ability to use the qdev*gpio*()
helpers later on.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/riscv/sifive_plic.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
index a4ac910ca9..81b6b5245b 100644
--- a/hw/riscv/sifive_plic.c
+++ b/hw/riscv/sifive_plic.c
@@ -431,7 +431,6 @@ static void sifive_plic_irq_request(void *opaque, int irq, int level)
 static void sifive_plic_realize(DeviceState *dev, Error **errp)
 {
     SiFivePLICState *plic = SIFIVE_PLIC(dev);
-    int i;
 
     memory_region_init_io(&plic->mmio, OBJECT(dev), &sifive_plic_ops, plic,
                           TYPE_SIFIVE_PLIC, plic->aperture_size);
@@ -444,9 +443,7 @@ static void sifive_plic_realize(DeviceState *dev, Error **errp)
     plic->enable = g_new0(uint32_t, plic->bitfield_words * plic->num_addrs);
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &plic->mmio);
     plic->irqs = g_new0(qemu_irq, plic->num_sources + 1);
-    for (i = 0; i <= plic->num_sources; i++) {
-        plic->irqs[i] = qemu_allocate_irq(sifive_plic_irq_request, plic, i);
-    }
+    qdev_init_gpio_in(dev, sifive_plic_irq_request, plic->num_sources);
 }
 
 static void sifive_plic_class_init(ObjectClass *klass, void *data)
-- 
2.17.0

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

* [Qemu-devel] [PATCH v1 3/4] hw/riscv/sifive_u: Connect the Cadence GEM Ethernet device
  2018-05-04 20:12 [Qemu-devel] [PATCH v1 0/4] RISC-V: SoCify the SiFive boards and connect the Alistair Francis
  2018-05-04 20:12 ` [Qemu-devel] [PATCH v1 1/4] hw/riscv/sifive_u: Create a U54 SoC object Alistair Francis
  2018-05-04 20:13 ` [Qemu-devel] [PATCH v1 2/4] hw/riscv/sifive_plic: Use gpios instead of irqs Alistair Francis
@ 2018-05-04 20:13 ` Alistair Francis
  2018-05-04 20:13 ` [Qemu-devel] [PATCH v1 4/4] hw/riscv/sifive_e: Create a E31 SoC object Alistair Francis
  3 siblings, 0 replies; 9+ messages in thread
From: Alistair Francis @ 2018-05-04 20:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis, alistair23, palmer, mjc

Connect the Cadence GEM ethernet device. This also requires us to
expose the plic interrupt lines.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 default-configs/riscv32-softmmu.mak |  1 +
 default-configs/riscv64-softmmu.mak |  1 +
 hw/riscv/sifive_u.c                 | 29 +++++++++++++++++++++++++++++
 include/hw/riscv/sifive_u.h         |  6 +++++-
 4 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/default-configs/riscv32-softmmu.mak b/default-configs/riscv32-softmmu.mak
index f9e742120c..9a1c42e8b2 100644
--- a/default-configs/riscv32-softmmu.mak
+++ b/default-configs/riscv32-softmmu.mak
@@ -2,3 +2,4 @@
 
 CONFIG_SERIAL=y
 CONFIG_VIRTIO=y
+CONFIG_CADENCE=y
diff --git a/default-configs/riscv64-softmmu.mak b/default-configs/riscv64-softmmu.mak
index f9e742120c..9a1c42e8b2 100644
--- a/default-configs/riscv64-softmmu.mak
+++ b/default-configs/riscv64-softmmu.mak
@@ -2,3 +2,4 @@
 
 CONFIG_SERIAL=y
 CONFIG_VIRTIO=y
+CONFIG_CADENCE=y
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 4924f92262..e4c7539b89 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -58,8 +58,11 @@ static const struct MemmapEntry {
     [SIFIVE_U_UART0] =    { 0x10013000,     0x1000 },
     [SIFIVE_U_UART1] =    { 0x10023000,     0x1000 },
     [SIFIVE_U_DRAM] =     { 0x80000000,        0x0 },
+    [SIFIVE_U_GEM] =      { 0x100900FC,     0x1000 },
 };
 
+#define GEM_REVISION        0x10070109
+
 static uint64_t load_kernel(const char *kernel_filename)
 {
     uint64_t kernel_entry, kernel_high;
@@ -294,6 +297,9 @@ static void riscv_sifive_u54_init(Object *obj)
                            memmap[SIFIVE_U_MROM].size, &error_fatal);
     memory_region_add_subregion(system_memory, memmap[SIFIVE_U_MROM].base,
                                 mask_rom);
+
+    object_initialize(&s->gem, sizeof(s->gem), TYPE_CADENCE_GEM);
+    qdev_set_parent_bus(DEVICE(&s->gem), sysbus_get_default());
 }
 
 static void riscv_sifive_u54_realize(DeviceState *dev, Error **errp)
@@ -301,6 +307,10 @@ static void riscv_sifive_u54_realize(DeviceState *dev, Error **errp)
     SiFiveU54State *s = RISCV_U54_SOC(dev);
     const struct MemmapEntry *memmap = sifive_u_memmap;
     MemoryRegion *system_memory = get_system_memory();
+    qemu_irq plic_gpios[SIFIVE_U_PLIC_NUM_SOURCES];
+    int i;
+    Error *err = NULL;
+    NICInfo *nd = &nd_table[0];
 
     object_property_set_bool(OBJECT(&s->cpus), true, "realized",
                              &error_abort);
@@ -324,6 +334,25 @@ static void riscv_sifive_u54_realize(DeviceState *dev, Error **errp)
     sifive_clint_create(memmap[SIFIVE_U_CLINT].base,
         memmap[SIFIVE_U_CLINT].size, smp_cpus,
         SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE);
+
+    for (i = 0; i < SIFIVE_U_PLIC_NUM_SOURCES; i++) {
+        plic_gpios[i] = qdev_get_gpio_in(DEVICE(s->plic), i);
+    }
+
+    if (nd->used) {
+        qemu_check_nic_model(nd, TYPE_CADENCE_GEM);
+        qdev_set_nic_properties(DEVICE(&s->gem), nd);
+    }
+    object_property_set_int(OBJECT(&s->gem), GEM_REVISION, "revision",
+                            &error_abort);
+    object_property_set_bool(OBJECT(&s->gem), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gem), 0, memmap[SIFIVE_U_GEM].base);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem), 0,
+                       plic_gpios[53]);
 }
 
 static void riscv_sifive_u_machine_init(MachineClass *mc)
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index 0f8bdd8fab..d40d851999 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -19,6 +19,8 @@
 #ifndef HW_SIFIVE_U_H
 #define HW_SIFIVE_U_H
 
+#include "hw/net/cadence_gem.h"
+
 #define TYPE_RISCV_U54_SOC "riscv.sifive.u54"
 #define RISCV_U54_SOC(obj) \
     OBJECT_CHECK(SiFiveU54State, (obj), TYPE_RISCV_U54_SOC)
@@ -30,6 +32,7 @@ typedef struct SiFiveU54State {
     /*< public >*/
     RISCVHartArrayState cpus;
     DeviceState *plic;
+    CadenceGEMState gem;
 } SiFiveU54State;
 
 typedef struct SiFiveUState {
@@ -49,7 +52,8 @@ enum {
     SIFIVE_U_PLIC,
     SIFIVE_U_UART0,
     SIFIVE_U_UART1,
-    SIFIVE_U_DRAM
+    SIFIVE_U_DRAM,
+    SIFIVE_U_GEM
 };
 
 enum {
-- 
2.17.0

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

* [Qemu-devel] [PATCH v1 4/4] hw/riscv/sifive_e: Create a E31 SoC object
  2018-05-04 20:12 [Qemu-devel] [PATCH v1 0/4] RISC-V: SoCify the SiFive boards and connect the Alistair Francis
                   ` (2 preceding siblings ...)
  2018-05-04 20:13 ` [Qemu-devel] [PATCH v1 3/4] hw/riscv/sifive_u: Connect the Cadence GEM Ethernet device Alistair Francis
@ 2018-05-04 20:13 ` Alistair Francis
  3 siblings, 0 replies; 9+ messages in thread
From: Alistair Francis @ 2018-05-04 20:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis, alistair23, palmer, mjc

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/riscv/sifive_e.c         | 97 +++++++++++++++++++++++++++----------
 include/hw/riscv/sifive_e.h | 16 +++++-
 2 files changed, 86 insertions(+), 27 deletions(-)

diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index e4ecb7aa4b..0ab5e3ca45 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -102,18 +102,12 @@ static void riscv_sifive_e_init(MachineState *machine)
     SiFiveEState *s = g_new0(SiFiveEState, 1);
     MemoryRegion *sys_mem = get_system_memory();
     MemoryRegion *main_mem = g_new(MemoryRegion, 1);
-    MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
-    MemoryRegion *xip_mem = g_new(MemoryRegion, 1);
     int i;
 
-    /* Initialize SOC */
-    object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
+    /* Initialize SoC */
+    object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_E31_SOC);
     object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
                               &error_abort);
-    object_property_set_str(OBJECT(&s->soc), SIFIVE_E_CPU, "cpu-type",
-                            &error_abort);
-    object_property_set_int(OBJECT(&s->soc), smp_cpus, "num-harts",
-                            &error_abort);
     object_property_set_bool(OBJECT(&s->soc), true, "realized",
                             &error_abort);
 
@@ -123,11 +117,57 @@ static void riscv_sifive_e_init(MachineState *machine)
     memory_region_add_subregion(sys_mem,
         memmap[SIFIVE_E_DTIM].base, main_mem);
 
+    /* Mask ROM reset vector */
+    uint32_t reset_vec[2] = {
+        0x204002b7,        /* 0x1000: lui     t0,0x20400 */
+        0x00028067,        /* 0x1004: jr      t0 */
+    };
+
+    /* copy in the reset vector in little_endian byte order */
+    for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
+        reset_vec[i] = cpu_to_le32(reset_vec[i]);
+    }
+    rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
+                          memmap[SIFIVE_E_MROM].base, &address_space_memory);
+
+    if (machine->kernel_filename) {
+        load_kernel(machine->kernel_filename);
+    }
+}
+
+static void riscv_sifive_e31_init(Object *obj)
+{
+    const struct MemmapEntry *memmap = sifive_e_memmap;
+
+    SiFiveE31State *s = RISCV_E31_SOC(obj);
+    MemoryRegion *sys_mem = get_system_memory();
+    MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
+
+    object_initialize(&s->cpus, sizeof(s->cpus), TYPE_RISCV_HART_ARRAY);
+    object_property_add_child(obj, "cpus", OBJECT(&s->cpus),
+                              &error_abort);
+    object_property_set_str(OBJECT(&s->cpus), SIFIVE_E_CPU, "cpu-type",
+                            &error_abort);
+    object_property_set_int(OBJECT(&s->cpus), smp_cpus, "num-harts",
+                            &error_abort);
+
     /* Mask ROM */
     memory_region_init_rom(mask_rom, NULL, "riscv.sifive.e.mrom",
         memmap[SIFIVE_E_MROM].size, &error_fatal);
     memory_region_add_subregion(sys_mem,
         memmap[SIFIVE_E_MROM].base, mask_rom);
+}
+
+static void riscv_sifive_e31_realize(DeviceState *dev, Error **errp)
+{
+    const struct MemmapEntry *memmap = sifive_e_memmap;
+
+    SiFiveE31State *s = RISCV_E31_SOC(dev);
+    MemoryRegion *sys_mem = get_system_memory();
+    MemoryRegion *xip_mem = g_new(MemoryRegion, 1);
+
+    object_property_set_bool(OBJECT(&s->cpus), true, "realized",
+                            &error_abort);
 
     /* MMIO */
     s->plic = sifive_plic_create(memmap[SIFIVE_E_PLIC].base,
@@ -171,23 +211,6 @@ static void riscv_sifive_e_init(MachineState *machine)
         memmap[SIFIVE_E_XIP].size, &error_fatal);
     memory_region_set_readonly(xip_mem, true);
     memory_region_add_subregion(sys_mem, memmap[SIFIVE_E_XIP].base, xip_mem);
-
-    /* Mask ROM reset vector */
-    uint32_t reset_vec[2] = {
-        0x204002b7,        /* 0x1000: lui     t0,0x20400 */
-        0x00028067,        /* 0x1004: jr      t0 */
-    };
-
-    /* copy in the reset vector in little_endian byte order */
-    for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
-        reset_vec[i] = cpu_to_le32(reset_vec[i]);
-    }
-    rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
-                          memmap[SIFIVE_E_MROM].base, &address_space_memory);
-
-    if (machine->kernel_filename) {
-        load_kernel(machine->kernel_filename);
-    }
 }
 
 static void riscv_sifive_e_machine_init(MachineClass *mc)
@@ -198,3 +221,27 @@ static void riscv_sifive_e_machine_init(MachineClass *mc)
 }
 
 DEFINE_MACHINE("sifive_e", riscv_sifive_e_machine_init)
+
+static void riscv_sifive_e31_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = riscv_sifive_e31_realize;
+    /* Reason: Uses serial_hds in realize function, thus can't be used twice */
+    dc->user_creatable = false;
+}
+
+static const TypeInfo riscv_sifive_e31_type_info = {
+    .name = TYPE_RISCV_E31_SOC,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(SiFiveE31State),
+    .instance_init = riscv_sifive_e31_init,
+    .class_init = riscv_sifive_e31_class_init,
+};
+
+static void riscv_sifive_e31_register_types(void)
+{
+    type_register_static(&riscv_sifive_e31_type_info);
+}
+
+type_init(riscv_sifive_e31_register_types)
diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h
index 12ad6d2ebb..5cb19cd564 100644
--- a/include/hw/riscv/sifive_e.h
+++ b/include/hw/riscv/sifive_e.h
@@ -19,13 +19,25 @@
 #ifndef HW_SIFIVE_E_H
 #define HW_SIFIVE_E_H
 
-typedef struct SiFiveEState {
+#define TYPE_RISCV_E31_SOC "riscv.sifive.e31"
+#define RISCV_E31_SOC(obj) \
+    OBJECT_CHECK(SiFiveE31State, (obj), TYPE_RISCV_E31_SOC)
+
+typedef struct SiFiveE31State {
     /*< private >*/
     SysBusDevice parent_obj;
 
     /*< public >*/
-    RISCVHartArrayState soc;
+    RISCVHartArrayState cpus;
     DeviceState *plic;
+} SiFiveE31State;
+
+typedef struct SiFiveEState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    SiFiveE31State soc;
 } SiFiveEState;
 
 enum {
-- 
2.17.0

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

* Re: [Qemu-devel] [PATCH v1 1/4] hw/riscv/sifive_u: Create a U54 SoC object
  2018-05-04 20:12 ` [Qemu-devel] [PATCH v1 1/4] hw/riscv/sifive_u: Create a U54 SoC object Alistair Francis
@ 2018-05-04 22:55   ` Michael Clark
  2018-05-07 20:26     ` Alistair Francis
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Clark @ 2018-05-04 22:55 UTC (permalink / raw)
  To: Alistair Francis; +Cc: QEMU Developers, Alistair Francis, Palmer Dabbelt

On Sat, May 5, 2018 at 8:12 AM, Alistair Francis <alistair.francis@wdc.com>
wrote:

> Create a SiFive Unleashed U54 SoC and use that in the sifive_u machine.
>
> We leave the SoC, RAM, device tree and reset/fdt loading as part of the
> machine. All the other device creation has been moved to the SoC.
>

There is a tiny problem that we will have to resolve with renaming,
otherwise we will end up with lots of SOCs that are the essentially same
with a different CPU.

There is an intention to add a HiFive1 and HiFiveU board in the future
however we were very explicit in renaming sifive_e300 and sifive_u500 to
sifive_e and sifive_u.

If you read the code more closely you'll notice that we instantiate the
sifive_u board with a U34 if 32-bit and a U54 if 64-bit.

#if defined(TARGET_RISCV32)
#define SIFIVE_U_CPU TYPE_RISCV_CPU_SIFIVE_U34
#elif defined(TARGET_RISCV64)
#define SIFIVE_U_CPU TYPE_RISCV_CPU_SIFIVE_U54
#endif

The rationale is that sifive_e and sifive_u can eventually be customized to
represent different configurations of SiFive Core IP. We won't want to end
up hardcoding specific models in sifive_e or sifive_u

SiFive have been talking internally about having sifive_e and sifive_u
reconfigurable, possibly even to the extend where we can configure with a
memory map. This is somewhat consistent with what SiFive do internally as
the FE310 and FU540 are configurations generated by a core generator.

Now comes the question of whether its the right time to add a 'hifive1' or
'hifiveu' machine. I don't think it is immediately necessary. The thought
being that we would be able to give a configuration string of file to sifive_u
e.g. "e51,u54,u54,u54". There is a different between the U54 and the U54-MC
which has an E51 core for management tasks. The PLIC has already been
written with this in mind, and is re-configurable to support the U54-MC
memory layout.

In anycase. The change is simply to use sifive_u_soc or sifive_u without
the 54 (given it configures with a U34 in 32-bit like the sifive_e
configures with an E51 in 64-bit mode, both valid configurations)

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/riscv/sifive_u.c         | 90 ++++++++++++++++++++++++++++---------
>  include/hw/riscv/sifive_u.h | 16 ++++++-
>  2 files changed, 82 insertions(+), 24 deletions(-)
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 9f3d184b72..4924f92262 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -114,10 +114,10 @@ static void create_fdt(SiFiveUState *s, const struct
> MemmapEntry *memmap,
>      qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
>      qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
>
> -    for (cpu = s->soc.num_harts - 1; cpu >= 0; cpu--) {
> +    for (cpu = s->soc.cpus.num_harts - 1; cpu >= 0; cpu--) {
>          nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
>          char *intc = g_strdup_printf("/cpus/cpu@%d/interrupt-controller",
> cpu);
> -        char *isa = riscv_isa_string(&s->soc.harts[cpu]);
> +        char *isa = riscv_isa_string(&s->soc.cpus.harts[cpu]);
>          qemu_fdt_add_subnode(fdt, nodename);
>          qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
>                                SIFIVE_U_CLOCK_FREQ);
> @@ -138,8 +138,8 @@ static void create_fdt(SiFiveUState *s, const struct
> MemmapEntry *memmap,
>          g_free(nodename);
>      }
>
> -    cells =  g_new0(uint32_t, s->soc.num_harts * 4);
> -    for (cpu = 0; cpu < s->soc.num_harts; cpu++) {
> +    cells =  g_new0(uint32_t, s->soc.cpus.num_harts * 4);
> +    for (cpu = 0; cpu < s->soc.cpus.num_harts; cpu++) {
>          nodename =
>              g_strdup_printf("/cpus/cpu@%d/interrupt-controller", cpu);
>          uint32_t intc_phandle = qemu_fdt_get_phandle(fdt, nodename);
> @@ -157,12 +157,12 @@ static void create_fdt(SiFiveUState *s, const struct
> MemmapEntry *memmap,
>          0x0, memmap[SIFIVE_U_CLINT].base,
>          0x0, memmap[SIFIVE_U_CLINT].size);
>      qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
> -        cells, s->soc.num_harts * sizeof(uint32_t) * 4);
> +        cells, s->soc.cpus.num_harts * sizeof(uint32_t) * 4);
>      g_free(cells);
>      g_free(nodename);
>
> -    cells =  g_new0(uint32_t, s->soc.num_harts * 4);
> -    for (cpu = 0; cpu < s->soc.num_harts; cpu++) {
> +    cells =  g_new0(uint32_t, s->soc.cpus.num_harts * 4);
> +    for (cpu = 0; cpu < s->soc.cpus.num_harts; cpu++) {
>          nodename =
>              g_strdup_printf("/cpus/cpu@%d/interrupt-controller", cpu);
>          uint32_t intc_phandle = qemu_fdt_get_phandle(fdt, nodename);
> @@ -179,7 +179,7 @@ static void create_fdt(SiFiveUState *s, const struct
> MemmapEntry *memmap,
>      qemu_fdt_setprop_string(fdt, nodename, "compatible", "riscv,plic0");
>      qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
>      qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
> -        cells, s->soc.num_harts * sizeof(uint32_t) * 4);
> +        cells, s->soc.cpus.num_harts * sizeof(uint32_t) * 4);
>      qemu_fdt_setprop_cells(fdt, nodename, "reg",
>          0x0, memmap[SIFIVE_U_PLIC].base,
>          0x0, memmap[SIFIVE_U_PLIC].size);
> @@ -215,17 +215,12 @@ static void riscv_sifive_u_init(MachineState
> *machine)
>      SiFiveUState *s = g_new0(SiFiveUState, 1);
>      MemoryRegion *system_memory = get_system_memory();
>      MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> -    MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>      int i;
>
> -    /* Initialize SOC */
> -    object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_HART_ARRAY);
> +    /* Initialize SoC */
> +    object_initialize(&s->soc, sizeof(s->soc), TYPE_RISCV_U54_SOC);
>      object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
>                                &error_abort);
> -    object_property_set_str(OBJECT(&s->soc), SIFIVE_U_CPU, "cpu-type",
> -                            &error_abort);
> -    object_property_set_int(OBJECT(&s->soc), smp_cpus, "num-harts",
> -                            &error_abort);
>      object_property_set_bool(OBJECT(&s->soc), true, "realized",
>                              &error_abort);
>
> @@ -233,17 +228,11 @@ static void riscv_sifive_u_init(MachineState
> *machine)
>      memory_region_init_ram(main_mem, NULL, "riscv.sifive.u.ram",
>                             machine->ram_size, &error_fatal);
>      memory_region_add_subregion(system_memory,
> memmap[SIFIVE_U_DRAM].base,
> -        main_mem);
> +                                main_mem);
>
>      /* create device tree */
>      create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
>
> -    /* boot rom */
> -    memory_region_init_rom(mask_rom, NULL, "riscv.sifive.u.mrom",
> -                           memmap[SIFIVE_U_MROM].size, &error_fatal);
> -    memory_region_add_subregion(system_memory,
> memmap[SIFIVE_U_MROM].base,
> -                                mask_rom);
> -
>      if (machine->kernel_filename) {
>          load_kernel(machine->kernel_filename);
>      }
> @@ -282,6 +271,39 @@ static void riscv_sifive_u_init(MachineState
> *machine)
>      rom_add_blob_fixed_as("mrom.fdt", s->fdt, s->fdt_size,
>                            memmap[SIFIVE_U_MROM].base + sizeof(reset_vec),
>                            &address_space_memory);
> +}
> +
> +static void riscv_sifive_u54_init(Object *obj)
> +{
> +    const struct MemmapEntry *memmap = sifive_u_memmap;
> +
> +    SiFiveU54State *s = RISCV_U54_SOC(obj);
> +    MemoryRegion *system_memory = get_system_memory();
> +    MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
> +
> +    object_initialize(&s->cpus, sizeof(s->cpus), TYPE_RISCV_HART_ARRAY);
> +    object_property_add_child(obj, "cpus", OBJECT(&s->cpus),
> +                              &error_abort);
> +    object_property_set_str(OBJECT(&s->cpus), SIFIVE_U_CPU, "cpu-type",
> +                            &error_abort);
> +    object_property_set_int(OBJECT(&s->cpus), smp_cpus, "num-harts",
> +                            &error_abort);
> +
> +    /* boot rom */
> +    memory_region_init_rom(mask_rom, NULL, "riscv.sifive.u.mrom",
> +                           memmap[SIFIVE_U_MROM].size, &error_fatal);
> +    memory_region_add_subregion(system_memory,
> memmap[SIFIVE_U_MROM].base,
> +                                mask_rom);
> +}
> +
> +static void riscv_sifive_u54_realize(DeviceState *dev, Error **errp)
> +{
> +    SiFiveU54State *s = RISCV_U54_SOC(dev);
> +    const struct MemmapEntry *memmap = sifive_u_memmap;
> +    MemoryRegion *system_memory = get_system_memory();
> +
> +    object_property_set_bool(OBJECT(&s->cpus), true, "realized",
> +                             &error_abort);
>
>      /* MMIO */
>      s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
> @@ -312,3 +334,27 @@ static void riscv_sifive_u_machine_init(MachineClass
> *mc)
>  }
>
>  DEFINE_MACHINE("sifive_u", riscv_sifive_u_machine_init)
> +
> +static void riscv_sifive_u54_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +    dc->realize = riscv_sifive_u54_realize;
> +    /* Reason: Uses serial_hds in realize function, thus can't be used
> twice */
> +    dc->user_creatable = false;
> +}
> +
> +static const TypeInfo riscv_sifive_u54_type_info = {
> +    .name = TYPE_RISCV_U54_SOC,
> +    .parent = TYPE_DEVICE,
> +    .instance_size = sizeof(SiFiveU54State),
> +    .instance_init = riscv_sifive_u54_init,
> +    .class_init = riscv_sifive_u54_class_init,
> +};
> +
> +static void riscv_sifive_u54_register_types(void)
> +{
> +    type_register_static(&riscv_sifive_u54_type_info);
> +}
> +
> +type_init(riscv_sifive_u54_register_types)
> diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
> index 94a390566e..0f8bdd8fab 100644
> --- a/include/hw/riscv/sifive_u.h
> +++ b/include/hw/riscv/sifive_u.h
> @@ -19,13 +19,25 @@
>  #ifndef HW_SIFIVE_U_H
>  #define HW_SIFIVE_U_H
>
> -typedef struct SiFiveUState {
> +#define TYPE_RISCV_U54_SOC "riscv.sifive.u54"
> +#define RISCV_U54_SOC(obj) \
> +    OBJECT_CHECK(SiFiveU54State, (obj), TYPE_RISCV_U54_SOC)
> +
> +typedef struct SiFiveU54State {
>      /*< private >*/
>      SysBusDevice parent_obj;
>
>      /*< public >*/
> -    RISCVHartArrayState soc;
> +    RISCVHartArrayState cpus;
>      DeviceState *plic;
> +} SiFiveU54State;
> +
> +typedef struct SiFiveUState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +
> +    /*< public >*/
> +    SiFiveU54State soc;
>      void *fdt;
>      int fdt_size;
>  } SiFiveUState;
> --
> 2.17.0
>
>

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

* Re: [Qemu-devel] [PATCH v1 1/4] hw/riscv/sifive_u: Create a U54 SoC object
  2018-05-04 22:55   ` Michael Clark
@ 2018-05-07 20:26     ` Alistair Francis
  0 siblings, 0 replies; 9+ messages in thread
From: Alistair Francis @ 2018-05-07 20:26 UTC (permalink / raw)
  To: Michael Clark; +Cc: Alistair Francis, QEMU Developers, Palmer Dabbelt

On Fri, May 4, 2018 at 3:55 PM, Michael Clark <mjc@sifive.com> wrote:
>
>
> On Sat, May 5, 2018 at 8:12 AM, Alistair Francis <alistair.francis@wdc.com>
> wrote:
>>
>> Create a SiFive Unleashed U54 SoC and use that in the sifive_u machine.
>>
>> We leave the SoC, RAM, device tree and reset/fdt loading as part of the
>> machine. All the other device creation has been moved to the SoC.
>
>
> There is a tiny problem that we will have to resolve with renaming,
> otherwise we will end up with lots of SOCs that are the essentially same
> with a different CPU.
>
> There is an intention to add a HiFive1 and HiFiveU board in the future
> however we were very explicit in renaming sifive_e300 and sifive_u500 to
> sifive_e and sifive_u.
>
> If you read the code more closely you'll notice that we instantiate the
> sifive_u board with a U34 if 32-bit and a U54 if 64-bit.
>
> #if defined(TARGET_RISCV32)
> #define SIFIVE_U_CPU TYPE_RISCV_CPU_SIFIVE_U34
> #elif defined(TARGET_RISCV64)
> #define SIFIVE_U_CPU TYPE_RISCV_CPU_SIFIVE_U54
> #endif
>
> The rationale is that sifive_e and sifive_u can eventually be customized to
> represent different configurations of SiFive Core IP. We won't want to end
> up hardcoding specific models in sifive_e or sifive_u
>
> SiFive have been talking internally about having sifive_e and sifive_u
> reconfigurable, possibly even to the extend where we can configure with a
> memory map. This is somewhat consistent with what SiFive do internally as
> the FE310 and FU540 are configurations generated by a core generator.
>
> Now comes the question of whether its the right time to add a 'hifive1' or
> 'hifiveu' machine. I don't think it is immediately necessary. The thought
> being that we would be able to give a configuration string of file to
> sifive_u e.g. "e51,u54,u54,u54". There is a different between the U54 and
> the U54-MC which has an E51 core for management tasks. The PLIC has already
> been written with this in mind, and is re-configurable to support the U54-MC
> memory layout.

Aren't the E51, U54, U54-MC, etc. all different SoCs? So doesn't it
make sense to break out the SoCs and then in future the user can
specify the SoC for the machine/board when they start QEMU?

Alistair

>
> In anycase. The change is simply to use sifive_u_soc or sifive_u without the
> 54 (given it configures with a U34 in 32-bit like the sifive_e configures
> with an E51 in 64-bit mode, both valid configurations)
>

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

* Re: [Qemu-devel] [PATCH v1 2/4] hw/riscv/sifive_plic: Use gpios instead of irqs
  2018-05-04 20:13 ` [Qemu-devel] [PATCH v1 2/4] hw/riscv/sifive_plic: Use gpios instead of irqs Alistair Francis
@ 2018-05-10  2:33   ` Philippe Mathieu-Daudé
  2018-05-10 17:40     ` Alistair Francis
  0 siblings, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-10  2:33 UTC (permalink / raw)
  To: Alistair Francis, alistair23; +Cc: qemu-devel, Michael Clark, Palmer Dabbelt

[-- Attachment #1: Type: text/plain, Size: 5403 bytes --]

Hi Alistair,

On 05/04/2018 05:13 PM, Alistair Francis wrote:
> Instead of creating the interrupt in lines with qemu_allocate_irq() use
> qdev_init_gpio_in() as this gives us the ability to use the qdev*gpio*()
> helpers later on.

This is a good idea, but your patch is incomplete:
The devices previously using plic->irqs[] now need to use those
qdev*gpio*() helpers.

> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/riscv/sifive_plic.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
> index a4ac910ca9..81b6b5245b 100644
> --- a/hw/riscv/sifive_plic.c
> +++ b/hw/riscv/sifive_plic.c
> @@ -431,7 +431,6 @@ static void sifive_plic_irq_request(void *opaque, int irq, int level)
>  static void sifive_plic_realize(DeviceState *dev, Error **errp)
>  {
>      SiFivePLICState *plic = SIFIVE_PLIC(dev);
> -    int i;
>  
>      memory_region_init_io(&plic->mmio, OBJECT(dev), &sifive_plic_ops, plic,
>                            TYPE_SIFIVE_PLIC, plic->aperture_size);
> @@ -444,9 +443,7 @@ static void sifive_plic_realize(DeviceState *dev, Error **errp)
>      plic->enable = g_new0(uint32_t, plic->bitfield_words * plic->num_addrs);
>      sysbus_init_mmio(SYS_BUS_DEVICE(dev), &plic->mmio);
>      plic->irqs = g_new0(qemu_irq, plic->num_sources + 1);
> -    for (i = 0; i <= plic->num_sources; i++) {
> -        plic->irqs[i] = qemu_allocate_irq(sifive_plic_irq_request, plic, i);
> -    }
> +    qdev_init_gpio_in(dev, sifive_plic_irq_request, plic->num_sources);
>  }
>  
>  static void sifive_plic_class_init(ObjectClass *klass, void *data)
> 

The following patch completes yours:

- access the irqs with qdev_get_gpio_in()
- remove plic->irqs[] alloc in sifive_plic_realize()
- remove plic->irqs[] from SiFivePLICState struct

-- >8 --
 include/hw/riscv/sifive_plic.h | 1 -
 hw/riscv/sifive_e.c            | 2 +-
 hw/riscv/sifive_plic.c         | 1 -
 hw/riscv/sifive_u.c            | 2 +-
 hw/riscv/virt.c                | 4 ++--
 5 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/hw/riscv/sifive_plic.h b/include/hw/riscv/sifive_plic.h
index 11a5a98df1..2f2af7e686 100644
--- a/include/hw/riscv/sifive_plic.h
+++ b/include/hw/riscv/sifive_plic.h
@@ -56,7 +56,6 @@ typedef struct SiFivePLICState {
     uint32_t *claimed;
     uint32_t *enable;
     QemuMutex lock;
-    qemu_irq *irqs;

     /* config */
     char *hart_config;
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 0ab5e3ca45..7b4f04adcf 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -190,7 +190,7 @@ static void riscv_sifive_e31_realize(DeviceState
*dev, Error **errp)
     sifive_mmio_emulate(sys_mem, "riscv.sifive.e.gpio0",
         memmap[SIFIVE_E_GPIO0].base, memmap[SIFIVE_E_GPIO0].size);
     sifive_uart_create(sys_mem, memmap[SIFIVE_E_UART0].base,
-        serial_hd(0), SIFIVE_PLIC(s->plic)->irqs[SIFIVE_E_UART0_IRQ]);
+        serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic),
SIFIVE_E_UART0_IRQ));
     sifive_mmio_emulate(sys_mem, "riscv.sifive.e.qspi0",
         memmap[SIFIVE_E_QSPI0].base, memmap[SIFIVE_E_QSPI0].size);
     sifive_mmio_emulate(sys_mem, "riscv.sifive.e.pwm0",
diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
index c6ad17057d..a91aeb97ab 100644
--- a/hw/riscv/sifive_plic.c
+++ b/hw/riscv/sifive_plic.c
@@ -447,7 +447,6 @@ static void sifive_plic_realize(DeviceState *dev,
Error **errp)
     plic->claimed = g_new0(uint32_t, plic->bitfield_words);
     plic->enable = g_new0(uint32_t, plic->bitfield_words *
plic->num_addrs);
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &plic->mmio);
-    plic->irqs = g_new0(qemu_irq, plic->num_sources + 1);
     qdev_init_gpio_in(dev, sifive_plic_irq_request, plic->num_sources);
 }

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 54f33c39ae..b7936dcec5 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -330,7 +330,7 @@ static void riscv_sifive_u54_realize(DeviceState
*dev, Error **errp)
         SIFIVE_U_PLIC_CONTEXT_STRIDE,
         memmap[SIFIVE_U_PLIC].size);
     sifive_uart_create(system_memory, memmap[SIFIVE_U_UART0].base,
-        serial_hd(0), SIFIVE_PLIC(s->plic)->irqs[SIFIVE_U_UART0_IRQ]);
+        serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic),
SIFIVE_U_UART0_IRQ));
     /* sifive_uart_create(system_memory, memmap[SIFIVE_U_UART1].base,
         serial_hd(1), SIFIVE_PLIC(s->plic)->irqs[SIFIVE_U_UART1_IRQ]); */
     sifive_clint_create(memmap[SIFIVE_U_CLINT].base,
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index ad03113e0f..bdd75722eb 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -379,11 +379,11 @@ static void riscv_virt_board_init(MachineState
*machine)
     for (i = 0; i < VIRTIO_COUNT; i++) {
         sysbus_create_simple("virtio-mmio",
             memmap[VIRT_VIRTIO].base + i * memmap[VIRT_VIRTIO].size,
-            SIFIVE_PLIC(s->plic)->irqs[VIRTIO_IRQ + i]);
+            qdev_get_gpio_in(DEVICE(s->plic), VIRTIO_IRQ + i));
     }

     serial_mm_init(system_memory, memmap[VIRT_UART0].base,
-        0, SIFIVE_PLIC(s->plic)->irqs[UART0_IRQ], 399193,
+        0, qdev_get_gpio_in(DEVICE(s->plic), UART0_IRQ), 399193,
         serial_hd(0), DEVICE_LITTLE_ENDIAN);
 }

--

Regards,

Phil.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [Qemu-devel] [PATCH v1 2/4] hw/riscv/sifive_plic: Use gpios instead of irqs
  2018-05-10  2:33   ` Philippe Mathieu-Daudé
@ 2018-05-10 17:40     ` Alistair Francis
  0 siblings, 0 replies; 9+ messages in thread
From: Alistair Francis @ 2018-05-10 17:40 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	Michael Clark, Palmer Dabbelt

On Wed, May 9, 2018 at 7:33 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Hi Alistair,
>
> On 05/04/2018 05:13 PM, Alistair Francis wrote:
>> Instead of creating the interrupt in lines with qemu_allocate_irq() use
>> qdev_init_gpio_in() as this gives us the ability to use the qdev*gpio*()
>> helpers later on.
>
> This is a good idea, but your patch is incomplete:
> The devices previously using plic->irqs[] now need to use those
> qdev*gpio*() helpers.
>
>>
>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>  hw/riscv/sifive_plic.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
>> index a4ac910ca9..81b6b5245b 100644
>> --- a/hw/riscv/sifive_plic.c
>> +++ b/hw/riscv/sifive_plic.c
>> @@ -431,7 +431,6 @@ static void sifive_plic_irq_request(void *opaque, int irq, int level)
>>  static void sifive_plic_realize(DeviceState *dev, Error **errp)
>>  {
>>      SiFivePLICState *plic = SIFIVE_PLIC(dev);
>> -    int i;
>>
>>      memory_region_init_io(&plic->mmio, OBJECT(dev), &sifive_plic_ops, plic,
>>                            TYPE_SIFIVE_PLIC, plic->aperture_size);
>> @@ -444,9 +443,7 @@ static void sifive_plic_realize(DeviceState *dev, Error **errp)
>>      plic->enable = g_new0(uint32_t, plic->bitfield_words * plic->num_addrs);
>>      sysbus_init_mmio(SYS_BUS_DEVICE(dev), &plic->mmio);
>>      plic->irqs = g_new0(qemu_irq, plic->num_sources + 1);
>> -    for (i = 0; i <= plic->num_sources; i++) {
>> -        plic->irqs[i] = qemu_allocate_irq(sifive_plic_irq_request, plic, i);
>> -    }
>> +    qdev_init_gpio_in(dev, sifive_plic_irq_request, plic->num_sources);
>>  }
>>
>>  static void sifive_plic_class_init(ObjectClass *klass, void *data)
>>
>
> The following patch completes yours:
>
> - access the irqs with qdev_get_gpio_in()
> - remove plic->irqs[] alloc in sifive_plic_realize()
> - remove plic->irqs[] from SiFivePLICState struct

Thanks for that.

I realised the same thing. This series is also missing some device
tree changes that are required. I'll fix all of that in the next
version.

Alistair

>
> -- >8 --
>  include/hw/riscv/sifive_plic.h | 1 -
>  hw/riscv/sifive_e.c            | 2 +-
>  hw/riscv/sifive_plic.c         | 1 -
>  hw/riscv/sifive_u.c            | 2 +-
>  hw/riscv/virt.c                | 4 ++--
>  5 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/include/hw/riscv/sifive_plic.h b/include/hw/riscv/sifive_plic.h
> index 11a5a98df1..2f2af7e686 100644
> --- a/include/hw/riscv/sifive_plic.h
> +++ b/include/hw/riscv/sifive_plic.h
> @@ -56,7 +56,6 @@ typedef struct SiFivePLICState {
>      uint32_t *claimed;
>      uint32_t *enable;
>      QemuMutex lock;
> -    qemu_irq *irqs;
>
>      /* config */
>      char *hart_config;
> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> index 0ab5e3ca45..7b4f04adcf 100644
> --- a/hw/riscv/sifive_e.c
> +++ b/hw/riscv/sifive_e.c
> @@ -190,7 +190,7 @@ static void riscv_sifive_e31_realize(DeviceState
> *dev, Error **errp)
>      sifive_mmio_emulate(sys_mem, "riscv.sifive.e.gpio0",
>          memmap[SIFIVE_E_GPIO0].base, memmap[SIFIVE_E_GPIO0].size);
>      sifive_uart_create(sys_mem, memmap[SIFIVE_E_UART0].base,
> -        serial_hd(0), SIFIVE_PLIC(s->plic)->irqs[SIFIVE_E_UART0_IRQ]);
> +        serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic),
> SIFIVE_E_UART0_IRQ));
>      sifive_mmio_emulate(sys_mem, "riscv.sifive.e.qspi0",
>          memmap[SIFIVE_E_QSPI0].base, memmap[SIFIVE_E_QSPI0].size);
>      sifive_mmio_emulate(sys_mem, "riscv.sifive.e.pwm0",
> diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
> index c6ad17057d..a91aeb97ab 100644
> --- a/hw/riscv/sifive_plic.c
> +++ b/hw/riscv/sifive_plic.c
> @@ -447,7 +447,6 @@ static void sifive_plic_realize(DeviceState *dev,
> Error **errp)
>      plic->claimed = g_new0(uint32_t, plic->bitfield_words);
>      plic->enable = g_new0(uint32_t, plic->bitfield_words *
> plic->num_addrs);
>      sysbus_init_mmio(SYS_BUS_DEVICE(dev), &plic->mmio);
> -    plic->irqs = g_new0(qemu_irq, plic->num_sources + 1);
>      qdev_init_gpio_in(dev, sifive_plic_irq_request, plic->num_sources);
>  }
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 54f33c39ae..b7936dcec5 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -330,7 +330,7 @@ static void riscv_sifive_u54_realize(DeviceState
> *dev, Error **errp)
>          SIFIVE_U_PLIC_CONTEXT_STRIDE,
>          memmap[SIFIVE_U_PLIC].size);
>      sifive_uart_create(system_memory, memmap[SIFIVE_U_UART0].base,
> -        serial_hd(0), SIFIVE_PLIC(s->plic)->irqs[SIFIVE_U_UART0_IRQ]);
> +        serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic),
> SIFIVE_U_UART0_IRQ));
>      /* sifive_uart_create(system_memory, memmap[SIFIVE_U_UART1].base,
>          serial_hd(1), SIFIVE_PLIC(s->plic)->irqs[SIFIVE_U_UART1_IRQ]); */
>      sifive_clint_create(memmap[SIFIVE_U_CLINT].base,
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index ad03113e0f..bdd75722eb 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -379,11 +379,11 @@ static void riscv_virt_board_init(MachineState
> *machine)
>      for (i = 0; i < VIRTIO_COUNT; i++) {
>          sysbus_create_simple("virtio-mmio",
>              memmap[VIRT_VIRTIO].base + i * memmap[VIRT_VIRTIO].size,
> -            SIFIVE_PLIC(s->plic)->irqs[VIRTIO_IRQ + i]);
> +            qdev_get_gpio_in(DEVICE(s->plic), VIRTIO_IRQ + i));
>      }
>
>      serial_mm_init(system_memory, memmap[VIRT_UART0].base,
> -        0, SIFIVE_PLIC(s->plic)->irqs[UART0_IRQ], 399193,
> +        0, qdev_get_gpio_in(DEVICE(s->plic), UART0_IRQ), 399193,
>          serial_hd(0), DEVICE_LITTLE_ENDIAN);
>  }
>
> --
>
> Regards,
>
> Phil.
>

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

end of thread, other threads:[~2018-05-10 17:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-04 20:12 [Qemu-devel] [PATCH v1 0/4] RISC-V: SoCify the SiFive boards and connect the Alistair Francis
2018-05-04 20:12 ` [Qemu-devel] [PATCH v1 1/4] hw/riscv/sifive_u: Create a U54 SoC object Alistair Francis
2018-05-04 22:55   ` Michael Clark
2018-05-07 20:26     ` Alistair Francis
2018-05-04 20:13 ` [Qemu-devel] [PATCH v1 2/4] hw/riscv/sifive_plic: Use gpios instead of irqs Alistair Francis
2018-05-10  2:33   ` Philippe Mathieu-Daudé
2018-05-10 17:40     ` Alistair Francis
2018-05-04 20:13 ` [Qemu-devel] [PATCH v1 3/4] hw/riscv/sifive_u: Connect the Cadence GEM Ethernet device Alistair Francis
2018-05-04 20:13 ` [Qemu-devel] [PATCH v1 4/4] hw/riscv/sifive_e: Create a E31 SoC object Alistair Francis

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.