Re: [Qemu-devel] [PULL 01/29] SiFive RISC-V GPIO Device

2019-06-17 Thread Fabien Chouteau
On 14/06/2019 14:10, Palmer Dabbelt wrote:
> Sorry this took a while to fix, I've just sent a patch to fix the memory leak.

Thank you for taking care of this!



Re: [Qemu-devel] [PULL 01/29] SiFive RISC-V GPIO Device

2019-06-14 Thread Palmer Dabbelt

On Thu, 30 May 2019 03:57:12 PDT (-0700), Peter Maydell wrote:

On Sun, 26 May 2019 at 02:10, Palmer Dabbelt  wrote:


From: Fabien Chouteau 

QEMU model of the GPIO device on the SiFive E300 series SOCs.

The pins are not used by a board definition yet, however this
implementation can already be used to trigger GPIO interrupts from the
software by configuring a pin as both output and input.

Signed-off-by: Fabien Chouteau 
Reviewed-by: Palmer Dabbelt 
Signed-off-by: Palmer Dabbelt 


Hi; this patch causes Coverity to complain about a memory
leak (CID 1401707):


 static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp)
 {
 const struct MemmapEntry *memmap = sifive_e_memmap;
+Error *err = NULL;

 SiFiveESoCState *s = RISCV_E_SOC(dev);
 MemoryRegion *sys_mem = get_system_memory();
@@ -184,8 +188,28 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, 
Error **errp)
 sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon",
 memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size);
 sifive_prci_create(memmap[SIFIVE_E_PRCI].base);
-sifive_mmio_emulate(sys_mem, "riscv.sifive.e.gpio0",
-memmap[SIFIVE_E_GPIO0].base, memmap[SIFIVE_E_GPIO0].size);
+
+/* GPIO */
+
+object_property_set_bool(OBJECT(>gpio), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}


This function allocated xip_mem and mask_rom via g_new() but
then this error-exit doesn't free them.

The best way to fix this is to stop doing separate memory
allocations at all -- instead just have fields in the
SiFiveESoCState struct
   MemoryRegion xip_mem;
   Memory_Region mask_rom;

and pass >xip_mem etc where currently the code uses xip_mem.


Sorry this took a while to fix, I've just sent a patch to fix the memory leak.



Re: [Qemu-devel] [PULL 01/29] SiFive RISC-V GPIO Device

2019-05-30 Thread Peter Maydell
On Sun, 26 May 2019 at 02:10, Palmer Dabbelt  wrote:
>
> From: Fabien Chouteau 
>
> QEMU model of the GPIO device on the SiFive E300 series SOCs.
>
> The pins are not used by a board definition yet, however this
> implementation can already be used to trigger GPIO interrupts from the
> software by configuring a pin as both output and input.
>
> Signed-off-by: Fabien Chouteau 
> Reviewed-by: Palmer Dabbelt 
> Signed-off-by: Palmer Dabbelt 

Hi; this patch causes Coverity to complain about a memory
leak (CID 1401707):

>  static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp)
>  {
>  const struct MemmapEntry *memmap = sifive_e_memmap;
> +Error *err = NULL;
>
>  SiFiveESoCState *s = RISCV_E_SOC(dev);
>  MemoryRegion *sys_mem = get_system_memory();
> @@ -184,8 +188,28 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, 
> Error **errp)
>  sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon",
>  memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size);
>  sifive_prci_create(memmap[SIFIVE_E_PRCI].base);
> -sifive_mmio_emulate(sys_mem, "riscv.sifive.e.gpio0",
> -memmap[SIFIVE_E_GPIO0].base, memmap[SIFIVE_E_GPIO0].size);
> +
> +/* GPIO */
> +
> +object_property_set_bool(OBJECT(>gpio), true, "realized", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}

This function allocated xip_mem and mask_rom via g_new() but
then this error-exit doesn't free them.

The best way to fix this is to stop doing separate memory
allocations at all -- instead just have fields in the
SiFiveESoCState struct
   MemoryRegion xip_mem;
   Memory_Region mask_rom;

and pass >xip_mem etc where currently the code uses xip_mem.

thanks
-- PMM



[Qemu-devel] [PULL 01/29] SiFive RISC-V GPIO Device

2019-05-25 Thread Palmer Dabbelt
From: Fabien Chouteau 

QEMU model of the GPIO device on the SiFive E300 series SOCs.

The pins are not used by a board definition yet, however this
implementation can already be used to trigger GPIO interrupts from the
software by configuring a pin as both output and input.

Signed-off-by: Fabien Chouteau 
Reviewed-by: Palmer Dabbelt 
Signed-off-by: Palmer Dabbelt 
---
 Makefile.objs  |   1 +
 hw/riscv/Makefile.objs |   1 +
 hw/riscv/sifive_e.c|  28 ++-
 hw/riscv/sifive_gpio.c | 388 +
 hw/riscv/trace-events  |   7 +
 include/hw/riscv/sifive_e.h|   8 +-
 include/hw/riscv/sifive_gpio.h |  72 ++
 7 files changed, 501 insertions(+), 4 deletions(-)
 create mode 100644 hw/riscv/sifive_gpio.c
 create mode 100644 hw/riscv/trace-events
 create mode 100644 include/hw/riscv/sifive_gpio.h

diff --git a/Makefile.objs b/Makefile.objs
index d7491413c1c1..b61568ad0610 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -182,6 +182,7 @@ trace-events-subdirs += hw/virtio
 trace-events-subdirs += hw/watchdog
 trace-events-subdirs += hw/xen
 trace-events-subdirs += hw/gpio
+trace-events-subdirs += hw/riscv
 trace-events-subdirs += migration
 trace-events-subdirs += net
 trace-events-subdirs += ui
diff --git a/hw/riscv/Makefile.objs b/hw/riscv/Makefile.objs
index 79bfb3abf90a..a65027304a2c 100644
--- a/hw/riscv/Makefile.objs
+++ b/hw/riscv/Makefile.objs
@@ -2,6 +2,7 @@ obj-$(CONFIG_SPIKE) += riscv_htif.o
 obj-$(CONFIG_HART) += riscv_hart.o
 obj-$(CONFIG_SIFIVE_E) += sifive_e.o
 obj-$(CONFIG_SIFIVE) += sifive_clint.o
+obj-$(CONFIG_SIFIVE) += sifive_gpio.o
 obj-$(CONFIG_SIFIVE) += sifive_prci.o
 obj-$(CONFIG_SIFIVE) += sifive_plic.o
 obj-$(CONFIG_SIFIVE) += sifive_test.o
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index b1cd11363c93..80ac56fa7d5e 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -146,11 +146,15 @@ static void riscv_sifive_e_soc_init(Object *obj)
 _abort);
 object_property_set_int(OBJECT(>cpus), smp_cpus, "num-harts",
 _abort);
+sysbus_init_child_obj(obj, "riscv.sifive.e.gpio0",
+  >gpio, sizeof(s->gpio),
+  TYPE_SIFIVE_GPIO);
 }
 
 static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp)
 {
 const struct MemmapEntry *memmap = sifive_e_memmap;
+Error *err = NULL;
 
 SiFiveESoCState *s = RISCV_E_SOC(dev);
 MemoryRegion *sys_mem = get_system_memory();
@@ -184,8 +188,28 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, 
Error **errp)
 sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon",
 memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size);
 sifive_prci_create(memmap[SIFIVE_E_PRCI].base);
-sifive_mmio_emulate(sys_mem, "riscv.sifive.e.gpio0",
-memmap[SIFIVE_E_GPIO0].base, memmap[SIFIVE_E_GPIO0].size);
+
+/* GPIO */
+
+object_property_set_bool(OBJECT(>gpio), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+/* Map GPIO registers */
+sysbus_mmio_map(SYS_BUS_DEVICE(>gpio), 0, memmap[SIFIVE_E_GPIO0].base);
+
+/* Pass all GPIOs to the SOC layer so they are available to the board */
+qdev_pass_gpios(DEVICE(>gpio), dev, NULL);
+
+/* Connect GPIO interrupts to the PLIC */
+for (int i = 0; i < 32; i++) {
+sysbus_connect_irq(SYS_BUS_DEVICE(>gpio), i,
+   qdev_get_gpio_in(DEVICE(s->plic),
+SIFIVE_E_GPIO0_IRQ0 + i));
+}
+
 sifive_uart_create(sys_mem, memmap[SIFIVE_E_UART0].base,
 serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_E_UART0_IRQ));
 sifive_mmio_emulate(sys_mem, "riscv.sifive.e.qspi0",
diff --git a/hw/riscv/sifive_gpio.c b/hw/riscv/sifive_gpio.c
new file mode 100644
index ..06bd8112d7e7
--- /dev/null
+++ b/hw/riscv/sifive_gpio.c
@@ -0,0 +1,388 @@
+/*
+ * sifive System-on-Chip general purpose input/output register definition
+ *
+ * Copyright 2019 AdaCore
+ *
+ * Base on nrf51_gpio.c:
+ *
+ * Copyright 2018 Steffen Görtz 
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/riscv/sifive_gpio.h"
+#include "trace.h"
+
+static void update_output_irq(SIFIVEGPIOState *s)
+{
+
+uint32_t pending;
+uint32_t pin;
+
+pending = s->high_ip & s->high_ie;
+pending |= s->low_ip & s->low_ie;
+pending |= s->rise_ip & s->rise_ie;
+pending |= s->fall_ip & s->fall_ie;
+
+for (int i = 0; i < SIFIVE_GPIO_PINS; i++) {
+pin = 1 << i;
+qemu_set_irq(s->irq[i], (pending & pin) != 0);
+trace_sifive_gpio_update_output_irq(i, (pending & pin) != 0);
+}
+}
+
+static void update_state(SIFIVEGPIOState *s)
+{
+size_t i;
+bool prev_ival, in, in_mask, port, out_xor, pull,