[Qemu-devel] [PATCHv4 00/13] sun4m: sparc32_dma tidy-ups

[Qemu-devel] [PATCHv4 00/13] sun4m: sparc32_dma tidy-ups

[Mark Cave-Ayland]

This patchset aims to tidy-up the sparc32_dma code by improving the
modelling of the espdma/ledma devices using both QOM and the memory
API which didn't exist when the code was first written.

The result is that it is now possible to remove both the iommu_opaque
and is_ledma workarounds from the code, and the code for wiring up
the espdma/ledma and respective devices is also a lot more readable.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

v4:
- Rebase onto git master
- Update patch 9 to move lance QOM macros/SysBusPCNetState from sun4m.h to lance.h as suggested by Peter
- Add Reviewed-by from Peter for patch 7

v3:
- Add missing sysbus.h include to esp.h in patch 7

v2:
- Make esp/lance devices children of espdma/ledma devices respectively
- Add len parameter to ledma/espdma tracepoints


Mark Cave-Ayland (13):
  sparc32_dma: rename SPARC32_DMA type to SPARC32_DMA_DEVICE
  sparc32_dma: split esp and le into separate DMA devices
  sparc32_dma: move type declarations from sparc32_dma.c to
    sparc32_dma.h
  sun4m: move DMA device wiring from sparc32_dma_init() to
    sun4m_hw_init()
  sun4m_iommu: move TYPE_SUN4M_IOMMU declaration to sun4m.h
  sparc32_dma: use object link instead of qdev property to pass IOMMU
    reference
  esp: move TYPE_ESP and SysBusESPState from esp.c to esp.h
  sparc32_dma: make esp device child of espdma device
  lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h
  sparc32_dma: make lance device child of ledma device
  sparc32_dma: introduce new SPARC32_DMA type container object
  sparc32_dma: remove is_ledma hack and replace with memory region
    alias
  sparc32_dma: add len to esp/le DMA memory tracing

 hw/dma/sparc32_dma.c           |  235 +++++++++++++++++++++++++++++-----------
 hw/dma/sun4m_iommu.c           |   14 ---
 hw/dma/trace-events            |    8 +-
 hw/net/lance.c                 |   11 +-
 hw/scsi/esp.c                  |   13 ---
 hw/sparc/sun4m.c               |   82 ++++++--------
 include/hw/net/lance.h         |   41 +++++++
 include/hw/scsi/esp.h          |   14 +++
 include/hw/sparc/sparc32_dma.h |   55 ++++++++++
 include/hw/sparc/sun4m.h       |   16 +++
 10 files changed, 336 insertions(+), 153 deletions(-)
 create mode 100644 include/hw/net/lance.h

-- 
1.7.10.4

[Qemu-devel] [PATCHv4 01/13] sparc32_dma: rename SPARC32_DMA type to SPARC32_DMA_DEVICE

[Mark Cave-Ayland]

Also update the function names to match as appropriate. While we're
here rename the type from sparc32_dma to sparc32-dma in order to
match the current QOM convention.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
 hw/dma/sparc32_dma.c |   67 +++++++++++++++++++++++++-------------------------
 hw/sparc/sun4m.c     |    2 +-
 2 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index eb491b5..a8d31c1 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -61,12 +61,13 @@
 /* XXX SCSI and ethernet should have different read-only bit masks */
 #define DMA_CSR_RO_MASK 0xfe000007
 
-#define TYPE_SPARC32_DMA "sparc32_dma"
-#define SPARC32_DMA(obj) OBJECT_CHECK(DMAState, (obj), TYPE_SPARC32_DMA)
+#define TYPE_SPARC32_DMA_DEVICE "sparc32-dma-device"
+#define SPARC32_DMA_DEVICE(obj) OBJECT_CHECK(DMADeviceState, (obj), \
+                                             TYPE_SPARC32_DMA_DEVICE)
 
-typedef struct DMAState DMAState;
+typedef struct DMADeviceState DMADeviceState;
 
-struct DMAState {
+struct DMADeviceState {
     SysBusDevice parent_obj;
 
     MemoryRegion iomem;
@@ -86,7 +87,7 @@ enum {
 void ledma_memory_read(void *opaque, hwaddr addr,
                        uint8_t *buf, int len, int do_bswap)
 {
-    DMAState *s = opaque;
+    DMADeviceState *s = opaque;
     int i;
 
     addr |= s->dmaregs[3];
@@ -106,7 +107,7 @@ void ledma_memory_read(void *opaque, hwaddr addr,
 void ledma_memory_write(void *opaque, hwaddr addr,
                         uint8_t *buf, int len, int do_bswap)
 {
-    DMAState *s = opaque;
+    DMADeviceState *s = opaque;
     int l, i;
     uint16_t tmp_buf[32];
 
@@ -134,7 +135,7 @@ void ledma_memory_write(void *opaque, hwaddr addr,
 
 static void dma_set_irq(void *opaque, int irq, int level)
 {
-    DMAState *s = opaque;
+    DMADeviceState *s = opaque;
     if (level) {
         s->dmaregs[0] |= DMA_INTR;
         if (s->dmaregs[0] & DMA_INTREN) {
@@ -154,7 +155,7 @@ static void dma_set_irq(void *opaque, int irq, int level)
 
 void espdma_memory_read(void *opaque, uint8_t *buf, int len)
 {
-    DMAState *s = opaque;
+    DMADeviceState *s = opaque;
 
     trace_espdma_memory_read(s->dmaregs[1]);
     sparc_iommu_memory_read(s->iommu, s->dmaregs[1], buf, len);
@@ -163,7 +164,7 @@ void espdma_memory_read(void *opaque, uint8_t *buf, int len)
 
 void espdma_memory_write(void *opaque, uint8_t *buf, int len)
 {
-    DMAState *s = opaque;
+    DMADeviceState *s = opaque;
 
     trace_espdma_memory_write(s->dmaregs[1]);
     sparc_iommu_memory_write(s->iommu, s->dmaregs[1], buf, len);
@@ -173,7 +174,7 @@ void espdma_memory_write(void *opaque, uint8_t *buf, int len)
 static uint64_t dma_mem_read(void *opaque, hwaddr addr,
                              unsigned size)
 {
-    DMAState *s = opaque;
+    DMADeviceState *s = opaque;
     uint32_t saddr;
 
     if (s->is_ledma && (addr > DMA_MAX_REG_OFFSET)) {
@@ -190,7 +191,7 @@ static uint64_t dma_mem_read(void *opaque, hwaddr addr,
 static void dma_mem_write(void *opaque, hwaddr addr,
                           uint64_t val, unsigned size)
 {
-    DMAState *s = opaque;
+    DMADeviceState *s = opaque;
     uint32_t saddr;
 
     if (s->is_ledma && (addr > DMA_MAX_REG_OFFSET)) {
@@ -252,28 +253,28 @@ static const MemoryRegionOps dma_mem_ops = {
     },
 };
 
-static void dma_reset(DeviceState *d)
+static void sparc32_dma_device_reset(DeviceState *d)
 {
-    DMAState *s = SPARC32_DMA(d);
+    DMADeviceState *s = SPARC32_DMA_DEVICE(d);
 
     memset(s->dmaregs, 0, DMA_SIZE);
     s->dmaregs[0] = DMA_VER;
 }
 
-static const VMStateDescription vmstate_dma = {
+static const VMStateDescription vmstate_sparc32_dma_device = {
     .name ="sparc32_dma",
     .version_id = 2,
     .minimum_version_id = 2,
     .fields = (VMStateField[]) {
-        VMSTATE_UINT32_ARRAY(dmaregs, DMAState, DMA_REGS),
+        VMSTATE_UINT32_ARRAY(dmaregs, DMADeviceState, DMA_REGS),
         VMSTATE_END_OF_LIST()
     }
 };
 
-static void sparc32_dma_init(Object *obj)
+static void sparc32_dma_device_init(Object *obj)
 {
     DeviceState *dev = DEVICE(obj);
-    DMAState *s = SPARC32_DMA(obj);
+    DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
     sysbus_init_irq(sbd, &s->irq);
@@ -284,9 +285,9 @@ static void sparc32_dma_init(Object *obj)
     qdev_init_gpio_out(dev, s->gpio, 2);
 }
 
-static void sparc32_dma_realize(DeviceState *dev, Error **errp)
+static void sparc32_dma_device_realize(DeviceState *dev, Error **errp)
 {
-    DMAState *s = SPARC32_DMA(dev);
+    DMADeviceState *s = SPARC32_DMA_DEVICE(dev);
     int reg_size;
 
     reg_size = s->is_ledma ? DMA_ETH_SIZE : DMA_SIZE;
@@ -294,35 +295,35 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
                           "dma", reg_size);
 }
 
-static Property sparc32_dma_properties[] = {
-    DEFINE_PROP_PTR("iommu_opaque", DMAState, iommu),
-    DEFINE_PROP_UINT32("is_ledma", DMAState, is_ledma, 0),
+static Property sparc32_dma_device_properties[] = {
+    DEFINE_PROP_PTR("iommu_opaque", DMADeviceState, iommu),
+    DEFINE_PROP_UINT32("is_ledma", DMADeviceState, is_ledma, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void sparc32_dma_class_init(ObjectClass *klass, void *data)
+static void sparc32_dma_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    dc->reset = dma_reset;
-    dc->vmsd = &vmstate_dma;
-    dc->props = sparc32_dma_properties;
-    dc->realize = sparc32_dma_realize;
+    dc->reset = sparc32_dma_device_reset;
+    dc->vmsd = &vmstate_sparc32_dma_device;
+    dc->props = sparc32_dma_device_properties;
+    dc->realize = sparc32_dma_device_realize;
     /* Reason: pointer property "iommu_opaque" */
     dc->user_creatable = false;
 }
 
-static const TypeInfo sparc32_dma_info = {
-    .name          = TYPE_SPARC32_DMA,
+static const TypeInfo sparc32_dma_device_info = {
+    .name          = TYPE_SPARC32_DMA_DEVICE,
     .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(DMAState),
-    .instance_init = sparc32_dma_init,
-    .class_init    = sparc32_dma_class_init,
+    .instance_size = sizeof(DMADeviceState),
+    .instance_init = sparc32_dma_device_init,
+    .class_init    = sparc32_dma_device_class_init,
 };
 
 static void sparc32_dma_register_types(void)
 {
-    type_register_static(&sparc32_dma_info);
+    type_register_static(&sparc32_dma_device_info);
 }
 
 type_init(sparc32_dma_register_types)
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index e1bdd48..82c553c 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -313,7 +313,7 @@ static void *sparc32_dma_init(hwaddr daddr, qemu_irq parent_irq,
     DeviceState *dev;
     SysBusDevice *s;
 
-    dev = qdev_create(NULL, "sparc32_dma");
+    dev = qdev_create(NULL, "sparc32-dma-device");
     qdev_prop_set_ptr(dev, "iommu_opaque", iommu);
     qdev_prop_set_uint32(dev, "is_ledma", is_ledma);
     qdev_init_nofail(dev);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv4 01/13] sparc32_dma: rename SPARC32_DMA type to SPARC32_DMA_DEVICE

[Philippe Mathieu-Daudé]

On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
> Also update the function names to match as appropriate. While we're
> here rename the type from sparc32_dma to sparc32-dma in order to
> match the current QOM convention.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

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

> ---
>  hw/dma/sparc32_dma.c |   67 +++++++++++++++++++++++++-------------------------
>  hw/sparc/sun4m.c     |    2 +-
>  2 files changed, 35 insertions(+), 34 deletions(-)
> 
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index eb491b5..a8d31c1 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -61,12 +61,13 @@
>  /* XXX SCSI and ethernet should have different read-only bit masks */
>  #define DMA_CSR_RO_MASK 0xfe000007
>  
> -#define TYPE_SPARC32_DMA "sparc32_dma"
> -#define SPARC32_DMA(obj) OBJECT_CHECK(DMAState, (obj), TYPE_SPARC32_DMA)
> +#define TYPE_SPARC32_DMA_DEVICE "sparc32-dma-device"
> +#define SPARC32_DMA_DEVICE(obj) OBJECT_CHECK(DMADeviceState, (obj), \
> +                                             TYPE_SPARC32_DMA_DEVICE)
>  
> -typedef struct DMAState DMAState;
> +typedef struct DMADeviceState DMADeviceState;
>  
> -struct DMAState {
> +struct DMADeviceState {
>      SysBusDevice parent_obj;
>  
>      MemoryRegion iomem;
> @@ -86,7 +87,7 @@ enum {
>  void ledma_memory_read(void *opaque, hwaddr addr,
>                         uint8_t *buf, int len, int do_bswap)
>  {
> -    DMAState *s = opaque;
> +    DMADeviceState *s = opaque;
>      int i;
>  
>      addr |= s->dmaregs[3];
> @@ -106,7 +107,7 @@ void ledma_memory_read(void *opaque, hwaddr addr,
>  void ledma_memory_write(void *opaque, hwaddr addr,
>                          uint8_t *buf, int len, int do_bswap)
>  {
> -    DMAState *s = opaque;
> +    DMADeviceState *s = opaque;
>      int l, i;
>      uint16_t tmp_buf[32];
>  
> @@ -134,7 +135,7 @@ void ledma_memory_write(void *opaque, hwaddr addr,
>  
>  static void dma_set_irq(void *opaque, int irq, int level)
>  {
> -    DMAState *s = opaque;
> +    DMADeviceState *s = opaque;
>      if (level) {
>          s->dmaregs[0] |= DMA_INTR;
>          if (s->dmaregs[0] & DMA_INTREN) {
> @@ -154,7 +155,7 @@ static void dma_set_irq(void *opaque, int irq, int level)
>  
>  void espdma_memory_read(void *opaque, uint8_t *buf, int len)
>  {
> -    DMAState *s = opaque;
> +    DMADeviceState *s = opaque;
>  
>      trace_espdma_memory_read(s->dmaregs[1]);
>      sparc_iommu_memory_read(s->iommu, s->dmaregs[1], buf, len);
> @@ -163,7 +164,7 @@ void espdma_memory_read(void *opaque, uint8_t *buf, int len)
>  
>  void espdma_memory_write(void *opaque, uint8_t *buf, int len)
>  {
> -    DMAState *s = opaque;
> +    DMADeviceState *s = opaque;
>  
>      trace_espdma_memory_write(s->dmaregs[1]);
>      sparc_iommu_memory_write(s->iommu, s->dmaregs[1], buf, len);
> @@ -173,7 +174,7 @@ void espdma_memory_write(void *opaque, uint8_t *buf, int len)
>  static uint64_t dma_mem_read(void *opaque, hwaddr addr,
>                               unsigned size)
>  {
> -    DMAState *s = opaque;
> +    DMADeviceState *s = opaque;
>      uint32_t saddr;
>  
>      if (s->is_ledma && (addr > DMA_MAX_REG_OFFSET)) {
> @@ -190,7 +191,7 @@ static uint64_t dma_mem_read(void *opaque, hwaddr addr,
>  static void dma_mem_write(void *opaque, hwaddr addr,
>                            uint64_t val, unsigned size)
>  {
> -    DMAState *s = opaque;
> +    DMADeviceState *s = opaque;
>      uint32_t saddr;
>  
>      if (s->is_ledma && (addr > DMA_MAX_REG_OFFSET)) {
> @@ -252,28 +253,28 @@ static const MemoryRegionOps dma_mem_ops = {
>      },
>  };
>  
> -static void dma_reset(DeviceState *d)
> +static void sparc32_dma_device_reset(DeviceState *d)
>  {
> -    DMAState *s = SPARC32_DMA(d);
> +    DMADeviceState *s = SPARC32_DMA_DEVICE(d);
>  
>      memset(s->dmaregs, 0, DMA_SIZE);
>      s->dmaregs[0] = DMA_VER;
>  }
>  
> -static const VMStateDescription vmstate_dma = {
> +static const VMStateDescription vmstate_sparc32_dma_device = {
>      .name ="sparc32_dma",
>      .version_id = 2,
>      .minimum_version_id = 2,
>      .fields = (VMStateField[]) {
> -        VMSTATE_UINT32_ARRAY(dmaregs, DMAState, DMA_REGS),
> +        VMSTATE_UINT32_ARRAY(dmaregs, DMADeviceState, DMA_REGS),
>          VMSTATE_END_OF_LIST()
>      }
>  };
>  
> -static void sparc32_dma_init(Object *obj)
> +static void sparc32_dma_device_init(Object *obj)
>  {
>      DeviceState *dev = DEVICE(obj);
> -    DMAState *s = SPARC32_DMA(obj);
> +    DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>  
>      sysbus_init_irq(sbd, &s->irq);
> @@ -284,9 +285,9 @@ static void sparc32_dma_init(Object *obj)
>      qdev_init_gpio_out(dev, s->gpio, 2);
>  }
>  
> -static void sparc32_dma_realize(DeviceState *dev, Error **errp)
> +static void sparc32_dma_device_realize(DeviceState *dev, Error **errp)
>  {
> -    DMAState *s = SPARC32_DMA(dev);
> +    DMADeviceState *s = SPARC32_DMA_DEVICE(dev);
>      int reg_size;
>  
>      reg_size = s->is_ledma ? DMA_ETH_SIZE : DMA_SIZE;
> @@ -294,35 +295,35 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
>                            "dma", reg_size);
>  }
>  
> -static Property sparc32_dma_properties[] = {
> -    DEFINE_PROP_PTR("iommu_opaque", DMAState, iommu),
> -    DEFINE_PROP_UINT32("is_ledma", DMAState, is_ledma, 0),
> +static Property sparc32_dma_device_properties[] = {
> +    DEFINE_PROP_PTR("iommu_opaque", DMADeviceState, iommu),
> +    DEFINE_PROP_UINT32("is_ledma", DMADeviceState, is_ledma, 0),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -static void sparc32_dma_class_init(ObjectClass *klass, void *data)
> +static void sparc32_dma_device_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> -    dc->reset = dma_reset;
> -    dc->vmsd = &vmstate_dma;
> -    dc->props = sparc32_dma_properties;
> -    dc->realize = sparc32_dma_realize;
> +    dc->reset = sparc32_dma_device_reset;
> +    dc->vmsd = &vmstate_sparc32_dma_device;
> +    dc->props = sparc32_dma_device_properties;
> +    dc->realize = sparc32_dma_device_realize;
>      /* Reason: pointer property "iommu_opaque" */
>      dc->user_creatable = false;
>  }
>  
> -static const TypeInfo sparc32_dma_info = {
> -    .name          = TYPE_SPARC32_DMA,
> +static const TypeInfo sparc32_dma_device_info = {
> +    .name          = TYPE_SPARC32_DMA_DEVICE,
>      .parent        = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(DMAState),
> -    .instance_init = sparc32_dma_init,
> -    .class_init    = sparc32_dma_class_init,
> +    .instance_size = sizeof(DMADeviceState),
> +    .instance_init = sparc32_dma_device_init,
> +    .class_init    = sparc32_dma_device_class_init,
>  };
>  
>  static void sparc32_dma_register_types(void)
>  {
> -    type_register_static(&sparc32_dma_info);
> +    type_register_static(&sparc32_dma_device_info);
>  }
>  
>  type_init(sparc32_dma_register_types)
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index e1bdd48..82c553c 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -313,7 +313,7 @@ static void *sparc32_dma_init(hwaddr daddr, qemu_irq parent_irq,
>      DeviceState *dev;
>      SysBusDevice *s;
>  
> -    dev = qdev_create(NULL, "sparc32_dma");
> +    dev = qdev_create(NULL, "sparc32-dma-device");
>      qdev_prop_set_ptr(dev, "iommu_opaque", iommu);
>      qdev_prop_set_uint32(dev, "is_ledma", is_ledma);
>      qdev_init_nofail(dev);
> 

[Qemu-devel] [PATCHv4 02/13] sparc32_dma: split esp and le into separate DMA devices

[Mark Cave-Ayland]

Due to slight differences in behaviour accessing the registers for the
esp and le devices, create two separate SPARC32_DMA_DEVICE types and
update the sun4m machine to use.

Note that by using different device types we already know the size of
the register block and the value of is_ledma at init time, allowing us to
drop the SPARC32_DMA_DEVICE realize function and the is_ledma device
property.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
 hw/dma/sparc32_dma.c |   63 ++++++++++++++++++++++++++++++++++++++++----------
 hw/sparc/sun4m.c     |    3 +--
 2 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index a8d31c1..e4ff4a8 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -78,6 +78,22 @@ struct DMADeviceState {
     uint32_t is_ledma;
 };
 
+#define TYPE_SPARC32_ESPDMA_DEVICE "sparc32-espdma"
+#define SPARC32_ESPDMA_DEVICE(obj) OBJECT_CHECK(ESPDMADeviceState, (obj), \
+                                                TYPE_SPARC32_ESPDMA_DEVICE)
+
+typedef struct ESPDMADeviceState {
+    DMADeviceState parent_obj;
+} ESPDMADeviceState;
+
+#define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
+#define SPARC32_LEDMA_DEVICE(obj) OBJECT_CHECK(LEDMADeviceState, (obj), \
+                                               TYPE_SPARC32_LEDMA_DEVICE)
+
+typedef struct LEDMADeviceState {
+    DMADeviceState parent_obj;
+} LEDMADeviceState;
+
 enum {
     GPIO_RESET = 0,
     GPIO_DMA,
@@ -285,19 +301,8 @@ static void sparc32_dma_device_init(Object *obj)
     qdev_init_gpio_out(dev, s->gpio, 2);
 }
 
-static void sparc32_dma_device_realize(DeviceState *dev, Error **errp)
-{
-    DMADeviceState *s = SPARC32_DMA_DEVICE(dev);
-    int reg_size;
-
-    reg_size = s->is_ledma ? DMA_ETH_SIZE : DMA_SIZE;
-    memory_region_init_io(&s->iomem, OBJECT(dev), &dma_mem_ops, s,
-                          "dma", reg_size);
-}
-
 static Property sparc32_dma_device_properties[] = {
     DEFINE_PROP_PTR("iommu_opaque", DMADeviceState, iommu),
-    DEFINE_PROP_UINT32("is_ledma", DMADeviceState, is_ledma, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -308,7 +313,6 @@ static void sparc32_dma_device_class_init(ObjectClass *klass, void *data)
     dc->reset = sparc32_dma_device_reset;
     dc->vmsd = &vmstate_sparc32_dma_device;
     dc->props = sparc32_dma_device_properties;
-    dc->realize = sparc32_dma_device_realize;
     /* Reason: pointer property "iommu_opaque" */
     dc->user_creatable = false;
 }
@@ -316,14 +320,49 @@ static void sparc32_dma_device_class_init(ObjectClass *klass, void *data)
 static const TypeInfo sparc32_dma_device_info = {
     .name          = TYPE_SPARC32_DMA_DEVICE,
     .parent        = TYPE_SYS_BUS_DEVICE,
+    .abstract      = true,
     .instance_size = sizeof(DMADeviceState),
     .instance_init = sparc32_dma_device_init,
     .class_init    = sparc32_dma_device_class_init,
 };
 
+static void sparc32_espdma_device_init(Object *obj)
+{
+    DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
+
+    memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
+                          "espdma-mmio", DMA_SIZE);
+    s->is_ledma = 0;
+}
+
+static const TypeInfo sparc32_espdma_device_info = {
+    .name          = TYPE_SPARC32_ESPDMA_DEVICE,
+    .parent        = TYPE_SPARC32_DMA_DEVICE,
+    .instance_size = sizeof(ESPDMADeviceState),
+    .instance_init = sparc32_espdma_device_init,
+};
+
+static void sparc32_ledma_device_init(Object *obj)
+{
+    DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
+
+    memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
+                          "ledma-mmio", DMA_ETH_SIZE);
+    s->is_ledma = 1;
+}
+
+static const TypeInfo sparc32_ledma_device_info = {
+    .name          = TYPE_SPARC32_LEDMA_DEVICE,
+    .parent        = TYPE_SPARC32_DMA_DEVICE,
+    .instance_size = sizeof(LEDMADeviceState),
+    .instance_init = sparc32_ledma_device_init,
+};
+
 static void sparc32_dma_register_types(void)
 {
     type_register_static(&sparc32_dma_device_info);
+    type_register_static(&sparc32_espdma_device_info);
+    type_register_static(&sparc32_ledma_device_info);
 }
 
 type_init(sparc32_dma_register_types)
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 82c553c..88a9752 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -313,9 +313,8 @@ static void *sparc32_dma_init(hwaddr daddr, qemu_irq parent_irq,
     DeviceState *dev;
     SysBusDevice *s;
 
-    dev = qdev_create(NULL, "sparc32-dma-device");
+    dev = qdev_create(NULL, is_ledma ? "sparc32-ledma" : "sparc32-espdma");
     qdev_prop_set_ptr(dev, "iommu_opaque", iommu);
-    qdev_prop_set_uint32(dev, "is_ledma", is_ledma);
     qdev_init_nofail(dev);
     s = SYS_BUS_DEVICE(dev);
     sysbus_connect_irq(s, 0, parent_irq);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv4 02/13] sparc32_dma: split esp and le into separate DMA devices

[Philippe Mathieu-Daudé]

On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
> Due to slight differences in behaviour accessing the registers for the
> esp and le devices, create two separate SPARC32_DMA_DEVICE types and
> update the sun4m machine to use.
> 
> Note that by using different device types we already know the size of
> the register block and the value of is_ledma at init time, allowing us to
> drop the SPARC32_DMA_DEVICE realize function and the is_ledma device
> property.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

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

> ---
>  hw/dma/sparc32_dma.c |   63 ++++++++++++++++++++++++++++++++++++++++----------
>  hw/sparc/sun4m.c     |    3 +--
>  2 files changed, 52 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index a8d31c1..e4ff4a8 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -78,6 +78,22 @@ struct DMADeviceState {
>      uint32_t is_ledma;
>  };
>  
> +#define TYPE_SPARC32_ESPDMA_DEVICE "sparc32-espdma"
> +#define SPARC32_ESPDMA_DEVICE(obj) OBJECT_CHECK(ESPDMADeviceState, (obj), \
> +                                                TYPE_SPARC32_ESPDMA_DEVICE)
> +
> +typedef struct ESPDMADeviceState {
> +    DMADeviceState parent_obj;
> +} ESPDMADeviceState;
> +
> +#define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
> +#define SPARC32_LEDMA_DEVICE(obj) OBJECT_CHECK(LEDMADeviceState, (obj), \
> +                                               TYPE_SPARC32_LEDMA_DEVICE)
> +
> +typedef struct LEDMADeviceState {
> +    DMADeviceState parent_obj;
> +} LEDMADeviceState;
> +
>  enum {
>      GPIO_RESET = 0,
>      GPIO_DMA,
> @@ -285,19 +301,8 @@ static void sparc32_dma_device_init(Object *obj)
>      qdev_init_gpio_out(dev, s->gpio, 2);
>  }
>  
> -static void sparc32_dma_device_realize(DeviceState *dev, Error **errp)
> -{
> -    DMADeviceState *s = SPARC32_DMA_DEVICE(dev);
> -    int reg_size;
> -
> -    reg_size = s->is_ledma ? DMA_ETH_SIZE : DMA_SIZE;
> -    memory_region_init_io(&s->iomem, OBJECT(dev), &dma_mem_ops, s,
> -                          "dma", reg_size);
> -}
> -
>  static Property sparc32_dma_device_properties[] = {
>      DEFINE_PROP_PTR("iommu_opaque", DMADeviceState, iommu),
> -    DEFINE_PROP_UINT32("is_ledma", DMADeviceState, is_ledma, 0),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -308,7 +313,6 @@ static void sparc32_dma_device_class_init(ObjectClass *klass, void *data)
>      dc->reset = sparc32_dma_device_reset;
>      dc->vmsd = &vmstate_sparc32_dma_device;
>      dc->props = sparc32_dma_device_properties;
> -    dc->realize = sparc32_dma_device_realize;
>      /* Reason: pointer property "iommu_opaque" */
>      dc->user_creatable = false;
>  }
> @@ -316,14 +320,49 @@ static void sparc32_dma_device_class_init(ObjectClass *klass, void *data)
>  static const TypeInfo sparc32_dma_device_info = {
>      .name          = TYPE_SPARC32_DMA_DEVICE,
>      .parent        = TYPE_SYS_BUS_DEVICE,
> +    .abstract      = true,
>      .instance_size = sizeof(DMADeviceState),
>      .instance_init = sparc32_dma_device_init,
>      .class_init    = sparc32_dma_device_class_init,
>  };
>  
> +static void sparc32_espdma_device_init(Object *obj)
> +{
> +    DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
> +
> +    memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
> +                          "espdma-mmio", DMA_SIZE);
> +    s->is_ledma = 0;
> +}
> +
> +static const TypeInfo sparc32_espdma_device_info = {
> +    .name          = TYPE_SPARC32_ESPDMA_DEVICE,
> +    .parent        = TYPE_SPARC32_DMA_DEVICE,
> +    .instance_size = sizeof(ESPDMADeviceState),
> +    .instance_init = sparc32_espdma_device_init,
> +};
> +
> +static void sparc32_ledma_device_init(Object *obj)
> +{
> +    DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
> +
> +    memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
> +                          "ledma-mmio", DMA_ETH_SIZE);
> +    s->is_ledma = 1;
> +}
> +
> +static const TypeInfo sparc32_ledma_device_info = {
> +    .name          = TYPE_SPARC32_LEDMA_DEVICE,
> +    .parent        = TYPE_SPARC32_DMA_DEVICE,
> +    .instance_size = sizeof(LEDMADeviceState),
> +    .instance_init = sparc32_ledma_device_init,
> +};
> +
>  static void sparc32_dma_register_types(void)
>  {
>      type_register_static(&sparc32_dma_device_info);
> +    type_register_static(&sparc32_espdma_device_info);
> +    type_register_static(&sparc32_ledma_device_info);
>  }
>  
>  type_init(sparc32_dma_register_types)
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 82c553c..88a9752 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -313,9 +313,8 @@ static void *sparc32_dma_init(hwaddr daddr, qemu_irq parent_irq,
>      DeviceState *dev;
>      SysBusDevice *s;
>  
> -    dev = qdev_create(NULL, "sparc32-dma-device");
> +    dev = qdev_create(NULL, is_ledma ? "sparc32-ledma" : "sparc32-espdma");
>      qdev_prop_set_ptr(dev, "iommu_opaque", iommu);
> -    qdev_prop_set_uint32(dev, "is_ledma", is_ledma);
>      qdev_init_nofail(dev);
>      s = SYS_BUS_DEVICE(dev);
>      sysbus_connect_irq(s, 0, parent_irq);
> 

[Qemu-devel] [PATCHv4 03/13] sparc32_dma: move type declarations from sparc32_dma.c to sparc32_dma.h

[Mark Cave-Ayland]

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
 hw/dma/sparc32_dma.c           |   34 ----------------------------------
 include/hw/sparc/sparc32_dma.h |   37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 34 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index e4ff4a8..ae8fa06 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -40,7 +40,6 @@
  * http://www.ibiblio.org/pub/historic-linux/early-ports/Sparc/NCR/DMA2.txt
  */
 
-#define DMA_REGS 4
 #define DMA_SIZE (4 * sizeof(uint32_t))
 /* We need the mask, because one instance of the device is not page
    aligned (ledma, start address 0x0010) */
@@ -61,39 +60,6 @@
 /* XXX SCSI and ethernet should have different read-only bit masks */
 #define DMA_CSR_RO_MASK 0xfe000007
 
-#define TYPE_SPARC32_DMA_DEVICE "sparc32-dma-device"
-#define SPARC32_DMA_DEVICE(obj) OBJECT_CHECK(DMADeviceState, (obj), \
-                                             TYPE_SPARC32_DMA_DEVICE)
-
-typedef struct DMADeviceState DMADeviceState;
-
-struct DMADeviceState {
-    SysBusDevice parent_obj;
-
-    MemoryRegion iomem;
-    uint32_t dmaregs[DMA_REGS];
-    qemu_irq irq;
-    void *iommu;
-    qemu_irq gpio[2];
-    uint32_t is_ledma;
-};
-
-#define TYPE_SPARC32_ESPDMA_DEVICE "sparc32-espdma"
-#define SPARC32_ESPDMA_DEVICE(obj) OBJECT_CHECK(ESPDMADeviceState, (obj), \
-                                                TYPE_SPARC32_ESPDMA_DEVICE)
-
-typedef struct ESPDMADeviceState {
-    DMADeviceState parent_obj;
-} ESPDMADeviceState;
-
-#define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
-#define SPARC32_LEDMA_DEVICE(obj) OBJECT_CHECK(LEDMADeviceState, (obj), \
-                                               TYPE_SPARC32_LEDMA_DEVICE)
-
-typedef struct LEDMADeviceState {
-    DMADeviceState parent_obj;
-} LEDMADeviceState;
-
 enum {
     GPIO_RESET = 0,
     GPIO_DMA,
diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index 9497b13..df7491d 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -1,6 +1,43 @@
 #ifndef SPARC32_DMA_H
 #define SPARC32_DMA_H
 
+#include "hw/sysbus.h"
+
+#define DMA_REGS 4
+
+#define TYPE_SPARC32_DMA_DEVICE "sparc32-dma-device"
+#define SPARC32_DMA_DEVICE(obj) OBJECT_CHECK(DMADeviceState, (obj), \
+                                             TYPE_SPARC32_DMA_DEVICE)
+
+typedef struct DMADeviceState DMADeviceState;
+
+struct DMADeviceState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    uint32_t dmaregs[DMA_REGS];
+    qemu_irq irq;
+    void *iommu;
+    qemu_irq gpio[2];
+    uint32_t is_ledma;
+};
+
+#define TYPE_SPARC32_ESPDMA_DEVICE "sparc32-espdma"
+#define SPARC32_ESPDMA_DEVICE(obj) OBJECT_CHECK(ESPDMADeviceState, (obj), \
+                                                TYPE_SPARC32_ESPDMA_DEVICE)
+
+typedef struct ESPDMADeviceState {
+    DMADeviceState parent_obj;
+} ESPDMADeviceState;
+
+#define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
+#define SPARC32_LEDMA_DEVICE(obj) OBJECT_CHECK(LEDMADeviceState, (obj), \
+                                               TYPE_SPARC32_LEDMA_DEVICE)
+
+typedef struct LEDMADeviceState {
+    DMADeviceState parent_obj;
+} LEDMADeviceState;
+
 /* sparc32_dma.c */
 void ledma_memory_read(void *opaque, hwaddr addr,
                        uint8_t *buf, int len, int do_bswap);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv4 03/13] sparc32_dma: move type declarations from sparc32_dma.c to sparc32_dma.h

[Philippe Mathieu-Daudé]

Hi Mark,

On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
> ---
>  hw/dma/sparc32_dma.c           |   34 ----------------------------------
>  include/hw/sparc/sparc32_dma.h |   37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 34 deletions(-)
> 
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index e4ff4a8..ae8fa06 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -40,7 +40,6 @@
>   * http://www.ibiblio.org/pub/historic-linux/early-ports/Sparc/NCR/DMA2.txt
>   */
>  
> -#define DMA_REGS 4
>  #define DMA_SIZE (4 * sizeof(uint32_t))

Since you move this out, can you replace with:

   #define DMA_SIZE (DMA_REGS * sizeof(uint32_t))

>  /* We need the mask, because one instance of the device is not page
>     aligned (ledma, start address 0x0010) */
> @@ -61,39 +60,6 @@
>  /* XXX SCSI and ethernet should have different read-only bit masks */
>  #define DMA_CSR_RO_MASK 0xfe000007
>  
> -#define TYPE_SPARC32_DMA_DEVICE "sparc32-dma-device"
> -#define SPARC32_DMA_DEVICE(obj) OBJECT_CHECK(DMADeviceState, (obj), \
> -                                             TYPE_SPARC32_DMA_DEVICE)
> -
> -typedef struct DMADeviceState DMADeviceState;
> -
> -struct DMADeviceState {
> -    SysBusDevice parent_obj;
> -
> -    MemoryRegion iomem;
> -    uint32_t dmaregs[DMA_REGS];
> -    qemu_irq irq;
> -    void *iommu;
> -    qemu_irq gpio[2];

OK to move until here to sparc32_dma.h

However I don't like having now network/scsi fields mixed in
sparc32_dma.h which is supposed to be device agnostic IMHO.

So I'd rather keep this part here

(from here ...

> -    uint32_t is_ledma;
> -};
> -
> -#define TYPE_SPARC32_ESPDMA_DEVICE "sparc32-espdma"
> -#define SPARC32_ESPDMA_DEVICE(obj) OBJECT_CHECK(ESPDMADeviceState, (obj), \
> -                                                TYPE_SPARC32_ESPDMA_DEVICE)
> -
> -typedef struct ESPDMADeviceState {
> -    DMADeviceState parent_obj;
> -} ESPDMADeviceState;
> -
> -#define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
> -#define SPARC32_LEDMA_DEVICE(obj) OBJECT_CHECK(LEDMADeviceState, (obj), \
> -                                               TYPE_SPARC32_LEDMA_DEVICE)
> -
> -typedef struct LEDMADeviceState {
> -    DMADeviceState parent_obj;
> -} LEDMADeviceState;

... until here)

Or move it to hw/sparc/sun4m.c (or maybe clever in "hw/sparc/sun4m_dma.h").

> -
>  enum {
>      GPIO_RESET = 0,
>      GPIO_DMA,
> diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
> index 9497b13..df7491d 100644
> --- a/include/hw/sparc/sparc32_dma.h
> +++ b/include/hw/sparc/sparc32_dma.h
> @@ -1,6 +1,43 @@
>  #ifndef SPARC32_DMA_H
>  #define SPARC32_DMA_H
>  
> +#include "hw/sysbus.h"
> +
> +#define DMA_REGS 4
> +
> +#define TYPE_SPARC32_DMA_DEVICE "sparc32-dma-device"
> +#define SPARC32_DMA_DEVICE(obj) OBJECT_CHECK(DMADeviceState, (obj), \
> +                                             TYPE_SPARC32_DMA_DEVICE)
> +
> +typedef struct DMADeviceState DMADeviceState;
> +
> +struct DMADeviceState {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion iomem;
> +    uint32_t dmaregs[DMA_REGS];
> +    qemu_irq irq;
> +    void *iommu;
> +    qemu_irq gpio[2];

};

[no ...

> +    uint32_t is_ledma;
> +};
> +
> +#define TYPE_SPARC32_ESPDMA_DEVICE "sparc32-espdma"
> +#define SPARC32_ESPDMA_DEVICE(obj) OBJECT_CHECK(ESPDMADeviceState, (obj), \
> +                                                TYPE_SPARC32_ESPDMA_DEVICE)
> +
> +typedef struct ESPDMADeviceState {
> +    DMADeviceState parent_obj;
> +} ESPDMADeviceState;
> +
> +#define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
> +#define SPARC32_LEDMA_DEVICE(obj) OBJECT_CHECK(LEDMADeviceState, (obj), \
> +                                               TYPE_SPARC32_LEDMA_DEVICE)
> +
> +typedef struct LEDMADeviceState {
> +    DMADeviceState parent_obj;
> +} LEDMADeviceState;
> +

... no]

>  /* sparc32_dma.c */
>  void ledma_memory_read(void *opaque, hwaddr addr,
>                         uint8_t *buf, int len, int do_bswap);
> 

What do you think?

Regards,

Phil.

Re: [Qemu-devel] [PATCHv4 03/13] sparc32_dma: move type declarations from sparc32_dma.c to sparc32_dma.h

[Philippe Mathieu-Daudé]

> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index e4ff4a8..ae8fa06 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -40,7 +40,6 @@
>   * http://www.ibiblio.org/pub/historic-linux/early-ports/Sparc/NCR/DMA2.txt
>   */
>  
> -#define DMA_REGS 4
>  #define DMA_SIZE (4 * sizeof(uint32_t))
>  /* We need the mask, because one instance of the device is not page
>     aligned (ledma, start address 0x0010) */

#define DMA_MAX_REG_OFFSET (2 * DMA_SIZE - 1)

^ You can also remove this

> @@ -61,39 +60,6 @@
>  /* XXX SCSI and ethernet should have different read-only bit masks */
>  #define DMA_CSR_RO_MASK 0xfe000007

[Qemu-devel] [PATCHv4 04/13] sun4m: move DMA device wiring from sparc32_dma_init() to sun4m_hw_init()

[Mark Cave-Ayland]

By using the sysbus interface it is possible to wire up the esp/le devices
to the sun4m DMA controller directly during sun4m_hw_init() instead of
passing qemu_irqs into the sparc32_dma_init() function.

This is an intermediate step to allow further reorganisation as more logic
is moved into the relevant SPARC32 DMA devices; there will be a final
refactoring of sparc32_dma_init() once this work is complete.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
 hw/sparc/sun4m.c |   29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 88a9752..4f2ed4b 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -307,8 +307,7 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
     return s;
 }
 
-static void *sparc32_dma_init(hwaddr daddr, qemu_irq parent_irq,
-                              void *iommu, qemu_irq *dev_irq, int is_ledma)
+static void *sparc32_dma_init(hwaddr daddr, void *iommu, int is_ledma)
 {
     DeviceState *dev;
     SysBusDevice *s;
@@ -317,8 +316,6 @@ static void *sparc32_dma_init(hwaddr daddr, qemu_irq parent_irq,
     qdev_prop_set_ptr(dev, "iommu_opaque", iommu);
     qdev_init_nofail(dev);
     s = SYS_BUS_DEVICE(dev);
-    sysbus_connect_irq(s, 0, parent_irq);
-    *dev_irq = qdev_get_gpio_in(dev, 0);
     sysbus_mmio_map(s, 0, daddr);
 
     return s;
@@ -821,9 +818,10 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     DeviceState *slavio_intctl;
     const char *cpu_model = machine->cpu_model;
     unsigned int i;
-    void *iommu, *espdma, *ledma, *nvram;
-    qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS],
-        espdma_irq, ledma_irq;
+    void *iommu, *nvram;
+    DeviceState *espdma, *ledma;
+    SysBusDevice *sbd;
+    qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS];
     qemu_irq esp_reset, dma_enable;
     qemu_irq fdc_tc;
     unsigned long kernel_size;
@@ -882,11 +880,13 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
         empty_slot_init(hwdef->iommu_pad_base,hwdef->iommu_pad_len);
     }
 
-    espdma = sparc32_dma_init(hwdef->dma_base, slavio_irq[18],
-                              iommu, &espdma_irq, 0);
+    espdma = sparc32_dma_init(hwdef->dma_base, iommu, 0);
+    sbd = SYS_BUS_DEVICE(espdma);
+    sysbus_connect_irq(sbd, 0, slavio_irq[18]);
 
-    ledma = sparc32_dma_init(hwdef->dma_base + 16ULL,
-                             slavio_irq[16], iommu, &ledma_irq, 1);
+    ledma = sparc32_dma_init(hwdef->dma_base + 16ULL, iommu, 1);
+    sbd = SYS_BUS_DEVICE(ledma);
+    sysbus_connect_irq(sbd, 0, slavio_irq[16]);
 
     if (graphic_depth != 8 && graphic_depth != 24) {
         error_report("Unsupported depth: %d", graphic_depth);
@@ -939,7 +939,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
         empty_slot_init(hwdef->sx_base, 0x2000);
     }
 
-    lance_init(&nd_table[0], hwdef->le_base, ledma, ledma_irq);
+    lance_init(&nd_table[0], hwdef->le_base, ledma,
+               qdev_get_gpio_in(ledma, 0));
 
     nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
 
@@ -971,7 +972,9 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
 
     esp_init(hwdef->esp_base, 2,
              espdma_memory_read, espdma_memory_write,
-             espdma, espdma_irq, &esp_reset, &dma_enable);
+             espdma,
+             qdev_get_gpio_in(espdma, 0),
+             &esp_reset, &dma_enable);
 
     qdev_connect_gpio_out(espdma, 0, esp_reset);
     qdev_connect_gpio_out(espdma, 1, dma_enable);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv4 04/13] sun4m: move DMA device wiring from sparc32_dma_init() to sun4m_hw_init()

[Philippe Mathieu-Daudé]

On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
> By using the sysbus interface it is possible to wire up the esp/le devices
> to the sun4m DMA controller directly during sun4m_hw_init() instead of
> passing qemu_irqs into the sparc32_dma_init() function.
> 
> This is an intermediate step to allow further reorganisation as more logic
> is moved into the relevant SPARC32 DMA devices; there will be a final
> refactoring of sparc32_dma_init() once this work is complete.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

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

> ---
>  hw/sparc/sun4m.c |   29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 88a9752..4f2ed4b 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -307,8 +307,7 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
>      return s;
>  }
>  
> -static void *sparc32_dma_init(hwaddr daddr, qemu_irq parent_irq,
> -                              void *iommu, qemu_irq *dev_irq, int is_ledma)
> +static void *sparc32_dma_init(hwaddr daddr, void *iommu, int is_ledma)
>  {
>      DeviceState *dev;
>      SysBusDevice *s;
> @@ -317,8 +316,6 @@ static void *sparc32_dma_init(hwaddr daddr, qemu_irq parent_irq,
>      qdev_prop_set_ptr(dev, "iommu_opaque", iommu);
>      qdev_init_nofail(dev);
>      s = SYS_BUS_DEVICE(dev);
> -    sysbus_connect_irq(s, 0, parent_irq);
> -    *dev_irq = qdev_get_gpio_in(dev, 0);
>      sysbus_mmio_map(s, 0, daddr);
>  
>      return s;
> @@ -821,9 +818,10 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      DeviceState *slavio_intctl;
>      const char *cpu_model = machine->cpu_model;
>      unsigned int i;
> -    void *iommu, *espdma, *ledma, *nvram;
> -    qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS],
> -        espdma_irq, ledma_irq;
> +    void *iommu, *nvram;
> +    DeviceState *espdma, *ledma;
> +    SysBusDevice *sbd;
> +    qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS];
>      qemu_irq esp_reset, dma_enable;
>      qemu_irq fdc_tc;
>      unsigned long kernel_size;
> @@ -882,11 +880,13 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>          empty_slot_init(hwdef->iommu_pad_base,hwdef->iommu_pad_len);
>      }
>  
> -    espdma = sparc32_dma_init(hwdef->dma_base, slavio_irq[18],
> -                              iommu, &espdma_irq, 0);
> +    espdma = sparc32_dma_init(hwdef->dma_base, iommu, 0);
> +    sbd = SYS_BUS_DEVICE(espdma);
> +    sysbus_connect_irq(sbd, 0, slavio_irq[18]);
>  
> -    ledma = sparc32_dma_init(hwdef->dma_base + 16ULL,
> -                             slavio_irq[16], iommu, &ledma_irq, 1);
> +    ledma = sparc32_dma_init(hwdef->dma_base + 16ULL, iommu, 1);
> +    sbd = SYS_BUS_DEVICE(ledma);
> +    sysbus_connect_irq(sbd, 0, slavio_irq[16]);
>  
>      if (graphic_depth != 8 && graphic_depth != 24) {
>          error_report("Unsupported depth: %d", graphic_depth);
> @@ -939,7 +939,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>          empty_slot_init(hwdef->sx_base, 0x2000);
>      }
>  
> -    lance_init(&nd_table[0], hwdef->le_base, ledma, ledma_irq);
> +    lance_init(&nd_table[0], hwdef->le_base, ledma,
> +               qdev_get_gpio_in(ledma, 0));
>  
>      nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
>  
> @@ -971,7 +972,9 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>  
>      esp_init(hwdef->esp_base, 2,
>               espdma_memory_read, espdma_memory_write,
> -             espdma, espdma_irq, &esp_reset, &dma_enable);
> +             espdma,
> +             qdev_get_gpio_in(espdma, 0),
> +             &esp_reset, &dma_enable);
>  
>      qdev_connect_gpio_out(espdma, 0, esp_reset);
>      qdev_connect_gpio_out(espdma, 1, dma_enable);
> 

[Qemu-devel] [PATCHv4 05/13] sun4m_iommu: move TYPE_SUN4M_IOMMU declaration to sun4m.h

[Mark Cave-Ayland]

This is in preparation to allow the type to be used elsewhere.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/dma/sun4m_iommu.c     |   14 --------------
 include/hw/sparc/sun4m.h |   16 ++++++++++++++++
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/hw/dma/sun4m_iommu.c b/hw/dma/sun4m_iommu.c
index 335ef63..840064b 100644
--- a/hw/dma/sun4m_iommu.c
+++ b/hw/dma/sun4m_iommu.c
@@ -36,7 +36,6 @@
  * http://mediacast.sun.com/users/Barton808/media/Sun4M_SystemArchitecture_edited2.pdf
  */
 
-#define IOMMU_NREGS         (4*4096/4)
 #define IOMMU_CTRL          (0x0000 >> 2)
 #define IOMMU_CTRL_IMPL     0xf0000000 /* Implementation */
 #define IOMMU_CTRL_VERS     0x0f000000 /* Version */
@@ -128,19 +127,6 @@
 #define IOMMU_PAGE_SIZE     (1 << IOMMU_PAGE_SHIFT)
 #define IOMMU_PAGE_MASK     ~(IOMMU_PAGE_SIZE - 1)
 
-#define TYPE_SUN4M_IOMMU "iommu"
-#define SUN4M_IOMMU(obj) OBJECT_CHECK(IOMMUState, (obj), TYPE_SUN4M_IOMMU)
-
-typedef struct IOMMUState {
-    SysBusDevice parent_obj;
-
-    MemoryRegion iomem;
-    uint32_t regs[IOMMU_NREGS];
-    hwaddr iostart;
-    qemu_irq irq;
-    uint32_t version;
-} IOMMUState;
-
 static uint64_t iommu_mem_read(void *opaque, hwaddr addr,
                                unsigned size)
 {
diff --git a/include/hw/sparc/sun4m.h b/include/hw/sparc/sun4m.h
index 580d87b..1f1cf91 100644
--- a/include/hw/sparc/sun4m.h
+++ b/include/hw/sparc/sun4m.h
@@ -4,10 +4,26 @@
 #include "qemu-common.h"
 #include "exec/hwaddr.h"
 #include "qapi/qmp/types.h"
+#include "hw/sysbus.h"
 
 /* Devices used by sparc32 system.  */
 
 /* iommu.c */
+#define TYPE_SUN4M_IOMMU "iommu"
+#define SUN4M_IOMMU(obj) OBJECT_CHECK(IOMMUState, (obj), TYPE_SUN4M_IOMMU)
+
+#define IOMMU_NREGS         (4 * 4096 / 4)
+
+typedef struct IOMMUState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    uint32_t regs[IOMMU_NREGS];
+    hwaddr iostart;
+    qemu_irq irq;
+    uint32_t version;
+} IOMMUState;
+
 void sparc_iommu_memory_rw(void *opaque, hwaddr addr,
                                  uint8_t *buf, int len, int is_write);
 static inline void sparc_iommu_memory_read(void *opaque,
-- 
1.7.10.4

[Qemu-devel] [PATCHv4 06/13] sparc32_dma: use object link instead of qdev property to pass IOMMU reference

[Mark Cave-Ayland]

This enables us to remove the last remaining (opaque) qdev property. Whilst we
are here, also update iommu_init() to use TYPE_SUN4M_IOMMU instead of a
hardcoded string.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/dma/sparc32_dma.c |   13 +++++--------
 hw/sparc/sun4m.c     |    4 ++--
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index ae8fa06..c56a2ba 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -263,24 +263,21 @@ static void sparc32_dma_device_init(Object *obj)
 
     sysbus_init_mmio(sbd, &s->iomem);
 
+    object_property_add_link(OBJECT(dev), "iommu", TYPE_SUN4M_IOMMU,
+                             (Object **) &s->iommu,
+                             qdev_prop_allow_set_link_before_realize,
+                             0, NULL);
+
     qdev_init_gpio_in(dev, dma_set_irq, 1);
     qdev_init_gpio_out(dev, s->gpio, 2);
 }
 
-static Property sparc32_dma_device_properties[] = {
-    DEFINE_PROP_PTR("iommu_opaque", DMADeviceState, iommu),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static void sparc32_dma_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->reset = sparc32_dma_device_reset;
     dc->vmsd = &vmstate_sparc32_dma_device;
-    dc->props = sparc32_dma_device_properties;
-    /* Reason: pointer property "iommu_opaque" */
-    dc->user_creatable = false;
 }
 
 static const TypeInfo sparc32_dma_device_info = {
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 4f2ed4b..12d36b5 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -297,7 +297,7 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
     DeviceState *dev;
     SysBusDevice *s;
 
-    dev = qdev_create(NULL, "iommu");
+    dev = qdev_create(NULL, TYPE_SUN4M_IOMMU);
     qdev_prop_set_uint32(dev, "version", version);
     qdev_init_nofail(dev);
     s = SYS_BUS_DEVICE(dev);
@@ -313,7 +313,7 @@ static void *sparc32_dma_init(hwaddr daddr, void *iommu, int is_ledma)
     SysBusDevice *s;
 
     dev = qdev_create(NULL, is_ledma ? "sparc32-ledma" : "sparc32-espdma");
-    qdev_prop_set_ptr(dev, "iommu_opaque", iommu);
+    object_property_set_link(OBJECT(dev), OBJECT(iommu), "iommu", &error_abort);
     qdev_init_nofail(dev);
     s = SYS_BUS_DEVICE(dev);
     sysbus_mmio_map(s, 0, daddr);
-- 
1.7.10.4

[Qemu-devel] [PATCHv4 07/13] esp: move TYPE_ESP and SysBusESPState from esp.c to esp.h

[Mark Cave-Ayland]

This enables them to be used outside of esp.c.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
CC: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/scsi/esp.c         |   13 -------------
 include/hw/scsi/esp.h |   14 ++++++++++++++
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 22c2d91..ee586e7 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -592,19 +592,6 @@ const VMStateDescription vmstate_esp = {
     }
 };
 
-#define TYPE_ESP "esp"
-#define ESP_STATE(obj) OBJECT_CHECK(SysBusESPState, (obj), TYPE_ESP)
-
-typedef struct {
-    /*< private >*/
-    SysBusDevice parent_obj;
-    /*< public >*/
-
-    MemoryRegion iomem;
-    uint32_t it_shift;
-    ESPState esp;
-} SysBusESPState;
-
 static void sysbus_esp_mem_write(void *opaque, hwaddr addr,
                                  uint64_t val, unsigned int size)
 {
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index d2c4886..3b160f8 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -2,6 +2,7 @@
 #define QEMU_HW_ESP_H
 
 #include "hw/scsi/scsi.h"
+#include "hw/sysbus.h"
 
 /* esp.c */
 #define ESP_MAX_DEVS 7
@@ -52,6 +53,19 @@ struct ESPState {
     void (*dma_cb)(ESPState *s);
 };
 
+#define TYPE_ESP "esp"
+#define ESP_STATE(obj) OBJECT_CHECK(SysBusESPState, (obj), TYPE_ESP)
+
+typedef struct {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    MemoryRegion iomem;
+    uint32_t it_shift;
+    ESPState esp;
+} SysBusESPState;
+
 #define ESP_TCLO   0x0
 #define ESP_TCMID  0x1
 #define ESP_FIFO   0x2
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv4 07/13] esp: move TYPE_ESP and SysBusESPState from esp.c to esp.h

[Philippe Mathieu-Daudé]

On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
> This enables them to be used outside of esp.c.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

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

> ---
>  hw/scsi/esp.c         |   13 -------------
>  include/hw/scsi/esp.h |   14 ++++++++++++++
>  2 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 22c2d91..ee586e7 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -592,19 +592,6 @@ const VMStateDescription vmstate_esp = {
>      }
>  };
>  
> -#define TYPE_ESP "esp"
> -#define ESP_STATE(obj) OBJECT_CHECK(SysBusESPState, (obj), TYPE_ESP)
> -
> -typedef struct {
> -    /*< private >*/
> -    SysBusDevice parent_obj;
> -    /*< public >*/
> -
> -    MemoryRegion iomem;
> -    uint32_t it_shift;
> -    ESPState esp;
> -} SysBusESPState;
> -
>  static void sysbus_esp_mem_write(void *opaque, hwaddr addr,
>                                   uint64_t val, unsigned int size)
>  {
> diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
> index d2c4886..3b160f8 100644
> --- a/include/hw/scsi/esp.h
> +++ b/include/hw/scsi/esp.h
> @@ -2,6 +2,7 @@
>  #define QEMU_HW_ESP_H
>  
>  #include "hw/scsi/scsi.h"
> +#include "hw/sysbus.h"
>  
>  /* esp.c */
>  #define ESP_MAX_DEVS 7
> @@ -52,6 +53,19 @@ struct ESPState {
>      void (*dma_cb)(ESPState *s);
>  };
>  
> +#define TYPE_ESP "esp"
> +#define ESP_STATE(obj) OBJECT_CHECK(SysBusESPState, (obj), TYPE_ESP)
> +
> +typedef struct {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    MemoryRegion iomem;
> +    uint32_t it_shift;
> +    ESPState esp;
> +} SysBusESPState;
> +
>  #define ESP_TCLO   0x0
>  #define ESP_TCMID  0x1
>  #define ESP_FIFO   0x2
> 

[Qemu-devel] [PATCHv4 08/13] sparc32_dma: make esp device child of espdma device

[Mark Cave-Ayland]

This makes it possible to reference the esp device from the espdma device as
required, and by wiring up the device ourselves in sun4m.c we can drop use
of the esp_init() function.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
 hw/dma/sparc32_dma.c           |   26 ++++++++++++++++++++++++++
 hw/sparc/sun4m.c               |   19 ++++++++-----------
 include/hw/sparc/sparc32_dma.h |    3 +++
 3 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index c56a2ba..6009b94 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -298,11 +298,37 @@ static void sparc32_espdma_device_init(Object *obj)
     s->is_ledma = 0;
 }
 
+static void sparc32_espdma_device_realize(DeviceState *dev, Error **errp)
+{
+    DeviceState *d;
+    SysBusESPState *sysbus;
+    ESPState *esp;
+
+    d = qdev_create(NULL, TYPE_ESP);
+    object_property_add_child(OBJECT(dev), "esp", OBJECT(d), errp);
+    sysbus = ESP_STATE(d);
+    esp = &sysbus->esp;
+    esp->dma_memory_read = espdma_memory_read;
+    esp->dma_memory_write = espdma_memory_write;
+    esp->dma_opaque = SPARC32_DMA_DEVICE(dev);
+    sysbus->it_shift = 2;
+    esp->dma_enabled = 1;
+    qdev_init_nofail(d);
+}
+
+static void sparc32_espdma_device_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = sparc32_espdma_device_realize;
+}
+
 static const TypeInfo sparc32_espdma_device_info = {
     .name          = TYPE_SPARC32_ESPDMA_DEVICE,
     .parent        = TYPE_SPARC32_DMA_DEVICE,
     .instance_size = sizeof(ESPDMADeviceState),
     .instance_init = sparc32_espdma_device_init,
+    .class_init    = sparc32_espdma_device_class_init,
 };
 
 static void sparc32_ledma_device_init(Object *obj)
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 12d36b5..4626c85 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -819,10 +819,9 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     const char *cpu_model = machine->cpu_model;
     unsigned int i;
     void *iommu, *nvram;
-    DeviceState *espdma, *ledma;
+    DeviceState *espdma, *esp, *ledma;
     SysBusDevice *sbd;
     qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS];
-    qemu_irq esp_reset, dma_enable;
     qemu_irq fdc_tc;
     unsigned long kernel_size;
     DriveInfo *fd[MAX_FD];
@@ -884,6 +883,13 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     sbd = SYS_BUS_DEVICE(espdma);
     sysbus_connect_irq(sbd, 0, slavio_irq[18]);
 
+    esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));
+    sbd = SYS_BUS_DEVICE(esp);
+    sysbus_mmio_map(sbd, 0, hwdef->esp_base);
+    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(espdma, 0));
+    qdev_connect_gpio_out(espdma, 0, qdev_get_gpio_in(esp, 0));
+    qdev_connect_gpio_out(espdma, 1, qdev_get_gpio_in(esp, 1));
+
     ledma = sparc32_dma_init(hwdef->dma_base + 16ULL, iommu, 1);
     sbd = SYS_BUS_DEVICE(ledma);
     sysbus_connect_irq(sbd, 0, slavio_irq[16]);
@@ -970,15 +976,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     slavio_misc_init(hwdef->slavio_base, hwdef->aux1_base, hwdef->aux2_base,
                      slavio_irq[30], fdc_tc);
 
-    esp_init(hwdef->esp_base, 2,
-             espdma_memory_read, espdma_memory_write,
-             espdma,
-             qdev_get_gpio_in(espdma, 0),
-             &esp_reset, &dma_enable);
-
-    qdev_connect_gpio_out(espdma, 0, esp_reset);
-    qdev_connect_gpio_out(espdma, 1, dma_enable);
-
     if (hwdef->cs_base) {
         sysbus_create_simple("SUNW,CS4231", hwdef->cs_base,
                              slavio_irq[5]);
diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index df7491d..365160f 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -2,6 +2,7 @@
 #define SPARC32_DMA_H
 
 #include "hw/sysbus.h"
+#include "hw/scsi/esp.h"
 
 #define DMA_REGS 4
 
@@ -28,6 +29,8 @@ struct DMADeviceState {
 
 typedef struct ESPDMADeviceState {
     DMADeviceState parent_obj;
+
+    SysBusESPState *esp;
 } ESPDMADeviceState;
 
 #define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv4 08/13] sparc32_dma: make esp device child of espdma device

[Philippe Mathieu-Daudé]

On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
> This makes it possible to reference the esp device from the espdma device as
> required, and by wiring up the device ourselves in sun4m.c we can drop use
> of the esp_init() function.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
> ---
>  hw/dma/sparc32_dma.c           |   26 ++++++++++++++++++++++++++
>  hw/sparc/sun4m.c               |   19 ++++++++-----------
>  include/hw/sparc/sparc32_dma.h |    3 +++
>  3 files changed, 37 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index c56a2ba..6009b94 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -298,11 +298,37 @@ static void sparc32_espdma_device_init(Object *obj)
>      s->is_ledma = 0;
>  }
>  
> +static void sparc32_espdma_device_realize(DeviceState *dev, Error **errp)
> +{
> +    DeviceState *d;
> +    SysBusESPState *sysbus;
> +    ESPState *esp;
> +
> +    d = qdev_create(NULL, TYPE_ESP);
> +    object_property_add_child(OBJECT(dev), "esp", OBJECT(d), errp);

TYPE_ESP ?

> +    sysbus = ESP_STATE(d);
> +    esp = &sysbus->esp;
> +    esp->dma_memory_read = espdma_memory_read;
> +    esp->dma_memory_write = espdma_memory_write;
> +    esp->dma_opaque = SPARC32_DMA_DEVICE(dev);
> +    sysbus->it_shift = 2;
> +    esp->dma_enabled = 1;
> +    qdev_init_nofail(d);
> +}
> +
> +static void sparc32_espdma_device_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = sparc32_espdma_device_realize;
> +}
> +
>  static const TypeInfo sparc32_espdma_device_info = {
>      .name          = TYPE_SPARC32_ESPDMA_DEVICE,
>      .parent        = TYPE_SPARC32_DMA_DEVICE,
>      .instance_size = sizeof(ESPDMADeviceState),
>      .instance_init = sparc32_espdma_device_init,
> +    .class_init    = sparc32_espdma_device_class_init,
>  };
>  
>  static void sparc32_ledma_device_init(Object *obj)
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 12d36b5..4626c85 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -819,10 +819,9 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      const char *cpu_model = machine->cpu_model;
>      unsigned int i;
>      void *iommu, *nvram;
> -    DeviceState *espdma, *ledma;
> +    DeviceState *espdma, *esp, *ledma;
>      SysBusDevice *sbd;
>      qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS];
> -    qemu_irq esp_reset, dma_enable;
>      qemu_irq fdc_tc;
>      unsigned long kernel_size;
>      DriveInfo *fd[MAX_FD];
> @@ -884,6 +883,13 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      sbd = SYS_BUS_DEVICE(espdma);
>      sysbus_connect_irq(sbd, 0, slavio_irq[18]);
>  
> +    esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));
> +    sbd = SYS_BUS_DEVICE(esp);
> +    sysbus_mmio_map(sbd, 0, hwdef->esp_base);
> +    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(espdma, 0));
> +    qdev_connect_gpio_out(espdma, 0, qdev_get_gpio_in(esp, 0));
> +    qdev_connect_gpio_out(espdma, 1, qdev_get_gpio_in(esp, 1));
> +
>      ledma = sparc32_dma_init(hwdef->dma_base + 16ULL, iommu, 1);
>      sbd = SYS_BUS_DEVICE(ledma);
>      sysbus_connect_irq(sbd, 0, slavio_irq[16]);
> @@ -970,15 +976,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      slavio_misc_init(hwdef->slavio_base, hwdef->aux1_base, hwdef->aux2_base,
>                       slavio_irq[30], fdc_tc);
>  
> -    esp_init(hwdef->esp_base, 2,
> -             espdma_memory_read, espdma_memory_write,
> -             espdma,
> -             qdev_get_gpio_in(espdma, 0),
> -             &esp_reset, &dma_enable);
> -
> -    qdev_connect_gpio_out(espdma, 0, esp_reset);
> -    qdev_connect_gpio_out(espdma, 1, dma_enable);
> -
>      if (hwdef->cs_base) {
>          sysbus_create_simple("SUNW,CS4231", hwdef->cs_base,
>                               slavio_irq[5]);
> diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
> index df7491d..365160f 100644
> --- a/include/hw/sparc/sparc32_dma.h
> +++ b/include/hw/sparc/sparc32_dma.h
> @@ -2,6 +2,7 @@
>  #define SPARC32_DMA_H
>  
>  #include "hw/sysbus.h"
> +#include "hw/scsi/esp.h"

I'd keep this in "hw/sparc/sun4m_dma.h"

>  
>  #define DMA_REGS 4
>  
> @@ -28,6 +29,8 @@ struct DMADeviceState {
>  
>  typedef struct ESPDMADeviceState {
>      DMADeviceState parent_obj;
> +
> +    SysBusESPState *esp;
>  } ESPDMADeviceState;
>  
>  #define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
> 

[Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h

[Mark Cave-Ayland]

This enables them to be used outside of lance.c.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
CC: Jason Wang <jasowang@redhat.com>
---
 hw/net/lance.c         |   11 +----------
 include/hw/net/lance.h |   41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 10 deletions(-)
 create mode 100644 include/hw/net/lance.h

diff --git a/hw/net/lance.c b/hw/net/lance.c
index 92b0c68..23929fd 100644
--- a/hw/net/lance.c
+++ b/hw/net/lance.c
@@ -41,19 +41,10 @@
 #include "qemu/timer.h"
 #include "qemu/sockets.h"
 #include "hw/sparc/sun4m.h"
-#include "pcnet.h"
+#include "hw/net/lance.h"
 #include "trace.h"
 #include "sysemu/sysemu.h"
 
-#define TYPE_LANCE "lance"
-#define SYSBUS_PCNET(obj) \
-    OBJECT_CHECK(SysBusPCNetState, (obj), TYPE_LANCE)
-
-typedef struct {
-    SysBusDevice parent_obj;
-
-    PCNetState state;
-} SysBusPCNetState;
 
 static void parent_lance_reset(void *opaque, int irq, int level)
 {
diff --git a/include/hw/net/lance.h b/include/hw/net/lance.h
new file mode 100644
index 0000000..c112fbc
--- /dev/null
+++ b/include/hw/net/lance.h
@@ -0,0 +1,41 @@
+/*
+ * QEMU AMD PC-Net II (Am79C970A) emulation
+ *
+ * Copyright (c) 2004 Antony T Curtis
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef LANCE_H
+#define LANCE_H
+
+#include "net/net.h"
+#include "hw/net/pcnet.h"
+
+#define TYPE_LANCE "lance"
+#define SYSBUS_PCNET(obj) \
+    OBJECT_CHECK(SysBusPCNetState, (obj), TYPE_LANCE)
+
+typedef struct {
+    SysBusDevice parent_obj;
+
+    PCNetState state;
+} SysBusPCNetState;
+
+#endif
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h

[Peter Maydell]

On 25 October 2017 at 16:59, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> This enables them to be used outside of lance.c.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> CC: Jason Wang <jasowang@redhat.com>


> diff --git a/include/hw/net/lance.h b/include/hw/net/lance.h
> new file mode 100644
> index 0000000..c112fbc
> --- /dev/null
> +++ b/include/hw/net/lance.h
> @@ -0,0 +1,41 @@
> +/*
> + * QEMU AMD PC-Net II (Am79C970A) emulation

Filename says this is Lance ethernet, but the comment says it's PC-Net ?

> +#define TYPE_LANCE "lance"
> +#define SYSBUS_PCNET(obj) \
> +    OBJECT_CHECK(SysBusPCNetState, (obj), TYPE_LANCE)

It's a bit weird that the OBJECT_CHECK macro name
and the state structure name don't line up with
the device name, but this is just code motion so
not a problem.

Other than fixing the comment,
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

(In an ideal world pcnet.h would be in include/.)

thanks
-- PMM

Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h

[Mark Cave-Ayland]

On 25/10/17 18:47, Peter Maydell wrote:

> On 25 October 2017 at 16:59, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>> This enables them to be used outside of lance.c.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> CC: Jason Wang <jasowang@redhat.com>
> 
> 
>> diff --git a/include/hw/net/lance.h b/include/hw/net/lance.h
>> new file mode 100644
>> index 0000000..c112fbc
>> --- /dev/null
>> +++ b/include/hw/net/lance.h
>> @@ -0,0 +1,41 @@
>> +/*
>> + * QEMU AMD PC-Net II (Am79C970A) emulation
> 
> Filename says this is Lance ethernet, but the comment says it's PC-Net ?

According to the datasheet for Am79C970A, the original Lance is an
Am7990 device and the Am79C970A aka PCNet-PCI II as emulated by QEMU is
register-compatible with it.

I guess the comment above is more technically correct, but I'm happy to
adjust it in my local tree if you still feel it needs to change?

>> +#define TYPE_LANCE "lance"
>> +#define SYSBUS_PCNET(obj) \
>> +    OBJECT_CHECK(SysBusPCNetState, (obj), TYPE_LANCE)
> 
> It's a bit weird that the OBJECT_CHECK macro name
> and the state structure name don't line up with
> the device name, but this is just code motion so
> not a problem.

Yeah, same issue as above... :/

> Other than fixing the comment,
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> (In an ideal world pcnet.h would be in include/.)
> 
> thanks
> -- PMM


ATB,

Mark.

Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h

[Mark Cave-Ayland]

On 26/10/17 11:12, Mark Cave-Ayland wrote:

> On 25/10/17 18:47, Peter Maydell wrote:
> 
>> On 25 October 2017 at 16:59, Mark Cave-Ayland
>> <mark.cave-ayland@ilande.co.uk> wrote:
>>> This enables them to be used outside of lance.c.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> CC: Jason Wang <jasowang@redhat.com>
>>
>>
>>> diff --git a/include/hw/net/lance.h b/include/hw/net/lance.h
>>> new file mode 100644
>>> index 0000000..c112fbc
>>> --- /dev/null
>>> +++ b/include/hw/net/lance.h
>>> @@ -0,0 +1,41 @@
>>> +/*
>>> + * QEMU AMD PC-Net II (Am79C970A) emulation
>>
>> Filename says this is Lance ethernet, but the comment says it's PC-Net ?
> 
> According to the datasheet for Am79C970A, the original Lance is an
> Am7990 device and the Am79C970A aka PCNet-PCI II as emulated by QEMU is
> register-compatible with it.
> 
> I guess the comment above is more technically correct, but I'm happy to
> adjust it in my local tree if you still feel it needs to change?
> 
>>> +#define TYPE_LANCE "lance"
>>> +#define SYSBUS_PCNET(obj) \
>>> +    OBJECT_CHECK(SysBusPCNetState, (obj), TYPE_LANCE)
>>
>> It's a bit weird that the OBJECT_CHECK macro name
>> and the state structure name don't line up with
>> the device name, but this is just code motion so
>> not a problem.
> 
> Yeah, same issue as above... :/
> 
>> Other than fixing the comment,
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>
>> (In an ideal world pcnet.h would be in include/.)
>>
>> thanks
>> -- PMM

Hi Peter,

Did you have any further comments on the above? If not I can tidy up my
branch for this (and the sun4m IOMMU work) later today and submit a PR
in time for freeze.


ATB,

Mark.

Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h

[Peter Maydell]

On 30 October 2017 at 13:10, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 26/10/17 11:12, Mark Cave-Ayland wrote:
>> On 25/10/17 18:47, Peter Maydell wrote:
>>> On 25 October 2017 at 16:59, Mark Cave-Ayland wrote:
>>>> --- /dev/null
>>>> +++ b/include/hw/net/lance.h
>>>> @@ -0,0 +1,41 @@
>>>> +/*
>>>> + * QEMU AMD PC-Net II (Am79C970A) emulation
>>>
>>> Filename says this is Lance ethernet, but the comment says it's PC-Net ?
>>
>> According to the datasheet for Am79C970A, the original Lance is an
>> Am7990 device and the Am79C970A aka PCNet-PCI II as emulated by QEMU is
>> register-compatible with it.
>>
>> I guess the comment above is more technically correct, but I'm happy to
>> adjust it in my local tree if you still feel it needs to change?

I think we should have a comment that says what the file is
for. Since this is a different file from pcnet.h, we should
have a comment that isn't the same as the pcnet.h one.
I don't particularly mind what it says, as long as it briefly
explains what's in the file (and by implication what distinguishes
things in this file from things in the other).

thanks
-- PMM

Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h

[Mark Cave-Ayland]

On 30/10/17 13:22, Peter Maydell wrote:

> On 30 October 2017 at 13:10, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>> On 26/10/17 11:12, Mark Cave-Ayland wrote:
>>> On 25/10/17 18:47, Peter Maydell wrote:
>>>> On 25 October 2017 at 16:59, Mark Cave-Ayland wrote:
>>>>> --- /dev/null
>>>>> +++ b/include/hw/net/lance.h
>>>>> @@ -0,0 +1,41 @@
>>>>> +/*
>>>>> + * QEMU AMD PC-Net II (Am79C970A) emulation
>>>>
>>>> Filename says this is Lance ethernet, but the comment says it's PC-Net ?
>>>
>>> According to the datasheet for Am79C970A, the original Lance is an
>>> Am7990 device and the Am79C970A aka PCNet-PCI II as emulated by QEMU is
>>> register-compatible with it.
>>>
>>> I guess the comment above is more technically correct, but I'm happy to
>>> adjust it in my local tree if you still feel it needs to change?
> 
> I think we should have a comment that says what the file is
> for. Since this is a different file from pcnet.h, we should
> have a comment that isn't the same as the pcnet.h one.
> I don't particularly mind what it says, as long as it briefly
> explains what's in the file (and by implication what distinguishes
> things in this file from things in the other).

Okay then how about something along the lines of:

 * QEMU Lance (Am7990) device emulation
 *
 * Copyright (c) 2004 Antony T Curtis
 * Copyright (c) 2017 Mark Cave-Ayland
 *
 * This represents the Sparc32 lance (Am7990) ethernet device which is
 * an earlier register-compatible member of the AMD PC-Net II
 * (Am79C970A) family.

In reality pcnet.c/pcnet.h are just the inner workings of the pcnet-pci
and lance devices. The comments in pcnet-pci.c and lance.c are identical
except for the Sparc32 reference and it was the same header from
pcnet-pci.c that I used as the basis for my last patch.


ATB,

Mark.

Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h

[Philippe Mathieu-Daudé]

Hi Mark,

>>>>>> +/*
>>>>>> + * QEMU AMD PC-Net II (Am79C970A) emulation
>>>>>
>>>>> Filename says this is Lance ethernet, but the comment says it's PC-Net ?
>>>>
>>>> According to the datasheet for Am79C970A, the original Lance is an
>>>> Am7990 device and the Am79C970A aka PCNet-PCI II as emulated by QEMU is
>>>> register-compatible with it.
>>>>
>>>> I guess the comment above is more technically correct, but I'm happy to
>>>> adjust it in my local tree if you still feel it needs to change?
>>
>> I think we should have a comment that says what the file is
>> for. Since this is a different file from pcnet.h, we should
>> have a comment that isn't the same as the pcnet.h one.
>> I don't particularly mind what it says, as long as it briefly
>> explains what's in the file (and by implication what distinguishes
>> things in this file from things in the other).
> 
> Okay then how about something along the lines of:
> 
>  * QEMU Lance (Am7990) device emulation
>  *
>  * Copyright (c) 2004 Antony T Curtis
>  * Copyright (c) 2017 Mark Cave-Ayland
>  *
>  * This represents the Sparc32 lance (Am7990) ethernet device which is
>  * an earlier register-compatible member of the AMD PC-Net II
>  * (Am79C970A) family.
> 
> In reality pcnet.c/pcnet.h are just the inner workings of the pcnet-pci
> and lance devices. The comments in pcnet-pci.c and lance.c are identical
> except for the Sparc32 reference and it was the same header from
> pcnet-pci.c that I used as the basis for my last patch.

You are right the PCnet family is based on the Lance one, but it seems
nobody remembers the Lance origin, the "PC-Net" took over.

Maybe we can agree with the different families having an unique
"hw/net/pcnet_lance.h" header with all Lance/PC-Net related XXX_TYPEs,
what do you think?

Regards,

Phil.

Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h

[Mark Cave-Ayland]

On 30/10/17 18:45, Philippe Mathieu-Daudé wrote:

>>>>>>> +/*
>>>>>>> + * QEMU AMD PC-Net II (Am79C970A) emulation
>>>>>>
>>>>>> Filename says this is Lance ethernet, but the comment says it's PC-Net ?
>>>>>
>>>>> According to the datasheet for Am79C970A, the original Lance is an
>>>>> Am7990 device and the Am79C970A aka PCNet-PCI II as emulated by QEMU is
>>>>> register-compatible with it.
>>>>>
>>>>> I guess the comment above is more technically correct, but I'm happy to
>>>>> adjust it in my local tree if you still feel it needs to change?
>>>
>>> I think we should have a comment that says what the file is
>>> for. Since this is a different file from pcnet.h, we should
>>> have a comment that isn't the same as the pcnet.h one.
>>> I don't particularly mind what it says, as long as it briefly
>>> explains what's in the file (and by implication what distinguishes
>>> things in this file from things in the other).
>>
>> Okay then how about something along the lines of:
>>
>>  * QEMU Lance (Am7990) device emulation
>>  *
>>  * Copyright (c) 2004 Antony T Curtis
>>  * Copyright (c) 2017 Mark Cave-Ayland
>>  *
>>  * This represents the Sparc32 lance (Am7990) ethernet device which is
>>  * an earlier register-compatible member of the AMD PC-Net II
>>  * (Am79C970A) family.
>>
>> In reality pcnet.c/pcnet.h are just the inner workings of the pcnet-pci
>> and lance devices. The comments in pcnet-pci.c and lance.c are identical
>> except for the Sparc32 reference and it was the same header from
>> pcnet-pci.c that I used as the basis for my last patch.
> 
> You are right the PCnet family is based on the Lance one, but it seems
> nobody remembers the Lance origin, the "PC-Net" took over.
> 
> Maybe we can agree with the different families having an unique
> "hw/net/pcnet_lance.h" header with all Lance/PC-Net related XXX_TYPEs,
> what do you think?

This is definitely a good idea in principle, however the lance device in
its current form cannot work for anything other than SPARC32 because of
the the word-size byte swaps for FIFO transfers which occur for DMA
transfers (see the code for ledma_memory_read/ledma_memory_write).

I think in its current form the patch allows for someone to potentially
implement this later for other architectures (e.g x86) so while I don't
want to disallow this in future, it isn't really within the scope of
this particular patchset.


ATB,

Mark.

Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h

[Philippe Mathieu-Daudé]

On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
> This enables them to be used outside of lance.c.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

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

> CC: Jason Wang <jasowang@redhat.com>
> ---
>  hw/net/lance.c         |   11 +----------
>  include/hw/net/lance.h |   41 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 10 deletions(-)
>  create mode 100644 include/hw/net/lance.h
> 
> diff --git a/hw/net/lance.c b/hw/net/lance.c
> index 92b0c68..23929fd 100644
> --- a/hw/net/lance.c
> +++ b/hw/net/lance.c
> @@ -41,19 +41,10 @@
>  #include "qemu/timer.h"
>  #include "qemu/sockets.h"
>  #include "hw/sparc/sun4m.h"
> -#include "pcnet.h"
> +#include "hw/net/lance.h"
>  #include "trace.h"
>  #include "sysemu/sysemu.h"
>  
> -#define TYPE_LANCE "lance"
> -#define SYSBUS_PCNET(obj) \
> -    OBJECT_CHECK(SysBusPCNetState, (obj), TYPE_LANCE)
> -
> -typedef struct {
> -    SysBusDevice parent_obj;
> -
> -    PCNetState state;
> -} SysBusPCNetState;
>  
>  static void parent_lance_reset(void *opaque, int irq, int level)
>  {
> diff --git a/include/hw/net/lance.h b/include/hw/net/lance.h
> new file mode 100644
> index 0000000..c112fbc
> --- /dev/null
> +++ b/include/hw/net/lance.h
> @@ -0,0 +1,41 @@
> +/*
> + * QEMU AMD PC-Net II (Am79C970A) emulation
> + *
> + * Copyright (c) 2004 Antony T Curtis
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef LANCE_H
> +#define LANCE_H
> +
> +#include "net/net.h"
> +#include "hw/net/pcnet.h"
> +
> +#define TYPE_LANCE "lance"
> +#define SYSBUS_PCNET(obj) \
> +    OBJECT_CHECK(SysBusPCNetState, (obj), TYPE_LANCE)
> +
> +typedef struct {
> +    SysBusDevice parent_obj;
> +
> +    PCNetState state;
> +} SysBusPCNetState;
> +
> +#endif
> 

[Qemu-devel] [PATCHv4 10/13] sparc32_dma: make lance device child of ledma device

[Mark Cave-Ayland]

This makes it possible to reference the lance device from the ledma device as
required.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
 hw/dma/sparc32_dma.c           |   22 ++++++++++++++++++++++
 hw/sparc/sun4m.c               |   31 +++++++------------------------
 include/hw/sparc/sparc32_dma.h |    3 +++
 3 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index 6009b94..d4cff74 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -340,11 +340,33 @@ static void sparc32_ledma_device_init(Object *obj)
     s->is_ledma = 1;
 }
 
+static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
+{
+    DeviceState *d;
+    NICInfo *nd = &nd_table[0];
+
+    qemu_check_nic_model(nd, TYPE_LANCE);
+
+    d = qdev_create(NULL, TYPE_LANCE);
+    object_property_add_child(OBJECT(dev), "lance", OBJECT(d), errp);
+    qdev_set_nic_properties(d, nd);
+    qdev_prop_set_ptr(d, "dma", dev);
+    qdev_init_nofail(d);
+}
+
+static void sparc32_ledma_device_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = sparc32_ledma_device_realize;
+}
+
 static const TypeInfo sparc32_ledma_device_info = {
     .name          = TYPE_SPARC32_LEDMA_DEVICE,
     .parent        = TYPE_SPARC32_DMA_DEVICE,
     .instance_size = sizeof(LEDMADeviceState),
     .instance_init = sparc32_ledma_device_init,
+    .class_init    = sparc32_ledma_device_class_init,
 };
 
 static void sparc32_dma_register_types(void)
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 4626c85..ae486a4 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -321,26 +321,6 @@ static void *sparc32_dma_init(hwaddr daddr, void *iommu, int is_ledma)
     return s;
 }
 
-static void lance_init(NICInfo *nd, hwaddr leaddr,
-                       void *dma_opaque, qemu_irq irq)
-{
-    DeviceState *dev;
-    SysBusDevice *s;
-    qemu_irq reset;
-
-    qemu_check_nic_model(&nd_table[0], "lance");
-
-    dev = qdev_create(NULL, "lance");
-    qdev_set_nic_properties(dev, nd);
-    qdev_prop_set_ptr(dev, "dma", dma_opaque);
-    qdev_init_nofail(dev);
-    s = SYS_BUS_DEVICE(dev);
-    sysbus_mmio_map(s, 0, leaddr);
-    sysbus_connect_irq(s, 0, irq);
-    reset = qdev_get_gpio_in(dev, 0);
-    qdev_connect_gpio_out(dma_opaque, 0, reset);
-}
-
 static DeviceState *slavio_intctl_init(hwaddr addr,
                                        hwaddr addrg,
                                        qemu_irq **parent_irq)
@@ -819,7 +799,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     const char *cpu_model = machine->cpu_model;
     unsigned int i;
     void *iommu, *nvram;
-    DeviceState *espdma, *esp, *ledma;
+    DeviceState *espdma, *esp, *ledma, *lance;
     SysBusDevice *sbd;
     qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS];
     qemu_irq fdc_tc;
@@ -894,6 +874,12 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     sbd = SYS_BUS_DEVICE(ledma);
     sysbus_connect_irq(sbd, 0, slavio_irq[16]);
 
+    lance = DEVICE(object_resolve_path_component(OBJECT(ledma), "lance"));
+    sbd = SYS_BUS_DEVICE(lance);
+    sysbus_mmio_map(sbd, 0, hwdef->le_base);
+    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(ledma, 0));
+    qdev_connect_gpio_out(ledma, 0, qdev_get_gpio_in(lance, 0));
+
     if (graphic_depth != 8 && graphic_depth != 24) {
         error_report("Unsupported depth: %d", graphic_depth);
         exit (1);
@@ -945,9 +931,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
         empty_slot_init(hwdef->sx_base, 0x2000);
     }
 
-    lance_init(&nd_table[0], hwdef->le_base, ledma,
-               qdev_get_gpio_in(ledma, 0));
-
     nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
 
     slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus);
diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index 365160f..5e39d81 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -3,6 +3,7 @@
 
 #include "hw/sysbus.h"
 #include "hw/scsi/esp.h"
+#include "hw/net/lance.h"
 
 #define DMA_REGS 4
 
@@ -39,6 +40,8 @@ typedef struct ESPDMADeviceState {
 
 typedef struct LEDMADeviceState {
     DMADeviceState parent_obj;
+
+    SysBusPCNetState *lance;
 } LEDMADeviceState;
 
 /* sparc32_dma.c */
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv4 10/13] sparc32_dma: make lance device child of ledma device

[Philippe Mathieu-Daudé]

On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
> This makes it possible to reference the lance device from the ledma device as
> required.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
> ---
>  hw/dma/sparc32_dma.c           |   22 ++++++++++++++++++++++
>  hw/sparc/sun4m.c               |   31 +++++++------------------------
>  include/hw/sparc/sparc32_dma.h |    3 +++
>  3 files changed, 32 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index 6009b94..d4cff74 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -340,11 +340,33 @@ static void sparc32_ledma_device_init(Object *obj)
>      s->is_ledma = 1;
>  }
>  
> +static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
> +{
> +    DeviceState *d;
> +    NICInfo *nd = &nd_table[0];
> +
> +    qemu_check_nic_model(nd, TYPE_LANCE);

It works but it is not the same idea (Type != Model) so I'd rather use
"lance" here.

> +
> +    d = qdev_create(NULL, TYPE_LANCE);
> +    object_property_add_child(OBJECT(dev), "lance", OBJECT(d), errp);

And use TYPE_LANCE here :)

> +    qdev_set_nic_properties(d, nd);
> +    qdev_prop_set_ptr(d, "dma", dev);
> +    qdev_init_nofail(d);
> +}
> +
> +static void sparc32_ledma_device_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = sparc32_ledma_device_realize;
> +}
> +
>  static const TypeInfo sparc32_ledma_device_info = {
>      .name          = TYPE_SPARC32_LEDMA_DEVICE,
>      .parent        = TYPE_SPARC32_DMA_DEVICE,
>      .instance_size = sizeof(LEDMADeviceState),
>      .instance_init = sparc32_ledma_device_init,
> +    .class_init    = sparc32_ledma_device_class_init,
>  };
>  
>  static void sparc32_dma_register_types(void)
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 4626c85..ae486a4 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -321,26 +321,6 @@ static void *sparc32_dma_init(hwaddr daddr, void *iommu, int is_ledma)
>      return s;
>  }
>  
> -static void lance_init(NICInfo *nd, hwaddr leaddr,
> -                       void *dma_opaque, qemu_irq irq)
> -{
> -    DeviceState *dev;
> -    SysBusDevice *s;
> -    qemu_irq reset;
> -
> -    qemu_check_nic_model(&nd_table[0], "lance");
> -
> -    dev = qdev_create(NULL, "lance");
> -    qdev_set_nic_properties(dev, nd);
> -    qdev_prop_set_ptr(dev, "dma", dma_opaque);
> -    qdev_init_nofail(dev);
> -    s = SYS_BUS_DEVICE(dev);
> -    sysbus_mmio_map(s, 0, leaddr);
> -    sysbus_connect_irq(s, 0, irq);
> -    reset = qdev_get_gpio_in(dev, 0);
> -    qdev_connect_gpio_out(dma_opaque, 0, reset);
> -}
> -
>  static DeviceState *slavio_intctl_init(hwaddr addr,
>                                         hwaddr addrg,
>                                         qemu_irq **parent_irq)
> @@ -819,7 +799,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      const char *cpu_model = machine->cpu_model;
>      unsigned int i;
>      void *iommu, *nvram;
> -    DeviceState *espdma, *esp, *ledma;
> +    DeviceState *espdma, *esp, *ledma, *lance;
>      SysBusDevice *sbd;
>      qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS];
>      qemu_irq fdc_tc;
> @@ -894,6 +874,12 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      sbd = SYS_BUS_DEVICE(ledma);
>      sysbus_connect_irq(sbd, 0, slavio_irq[16]);
>  
> +    lance = DEVICE(object_resolve_path_component(OBJECT(ledma), "lance"));

TYPE_LANCE here (or not)

> +    sbd = SYS_BUS_DEVICE(lance);
> +    sysbus_mmio_map(sbd, 0, hwdef->le_base);
> +    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(ledma, 0));
> +    qdev_connect_gpio_out(ledma, 0, qdev_get_gpio_in(lance, 0));
> +
>      if (graphic_depth != 8 && graphic_depth != 24) {
>          error_report("Unsupported depth: %d", graphic_depth);
>          exit (1);
> @@ -945,9 +931,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>          empty_slot_init(hwdef->sx_base, 0x2000);
>      }
>  
> -    lance_init(&nd_table[0], hwdef->le_base, ledma,
> -               qdev_get_gpio_in(ledma, 0));
> -
>      nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
>  
>      slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus);

OK until here.

> diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
> index 365160f..5e39d81 100644
> --- a/include/hw/sparc/sparc32_dma.h
> +++ b/include/hw/sparc/sparc32_dma.h
> @@ -3,6 +3,7 @@
>  
>  #include "hw/sysbus.h"
>  #include "hw/scsi/esp.h"
> +#include "hw/net/lance.h"

Same comment as patch 9, use "hw/sparc/sun4m_dma.h"?
>  
>  #define DMA_REGS 4
>  
> @@ -39,6 +40,8 @@ typedef struct ESPDMADeviceState {
>  
>  typedef struct LEDMADeviceState {
>      DMADeviceState parent_obj;
> +
> +    SysBusPCNetState *lance;
>  } LEDMADeviceState;
>  
>  /* sparc32_dma.c */
> 

[Qemu-devel] [PATCHv4 11/13] sparc32_dma: introduce new SPARC32_DMA type container object

[Mark Cave-Ayland]

Create a new SPARC32_DMA container object (including an appropriate container
memory region) and add instances of the SPARC32_ESPDMA_DEVICE and
SPARC32_LEDMA_DEVICE as child objects. The benefit is that most of the gpio
wiring complexity between esp/espdma and lance/ledma is now hidden within the
SPARC32_DMA realize function.

Since the sun4m IOMMU is already QOMified we can find a reference to
it using object_resolve_path_type() allowing us to completely remove all external
references to the iommu pointer.

Finally we rework sun4m's sparc32_dma_init() to invoke the new SPARC32_DMA object
and wire up the remaining board memory regions/IRQs.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
 hw/dma/sparc32_dma.c           |   70 ++++++++++++++++++++++++++++++++++++++++
 hw/sparc/sun4m.c               |   66 ++++++++++++++++++-------------------
 include/hw/sparc/sparc32_dma.h |   12 +++++++
 3 files changed, 114 insertions(+), 34 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index d4cff74..582b7cc 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -30,6 +30,7 @@
 #include "hw/sparc/sparc32_dma.h"
 #include "hw/sparc/sun4m.h"
 #include "hw/sysbus.h"
+#include "qapi/error.h"
 #include "trace.h"
 
 /*
@@ -369,11 +370,80 @@ static const TypeInfo sparc32_ledma_device_info = {
     .class_init    = sparc32_ledma_device_class_init,
 };
 
+static void sparc32_dma_realize(DeviceState *dev, Error **errp)
+{
+    SPARC32DMAState *s = SPARC32_DMA(dev);
+    DeviceState *espdma, *esp, *ledma, *lance;
+    SysBusDevice *sbd;
+    Object *iommu;
+
+    iommu = object_resolve_path_type("", TYPE_SUN4M_IOMMU, NULL);
+    if (!iommu) {
+        error_setg(errp, "unable to locate sun4m IOMMU device");
+        return;
+    }
+
+    espdma = qdev_create(NULL, TYPE_SPARC32_ESPDMA_DEVICE);
+    object_property_set_link(OBJECT(espdma), iommu, "iommu", errp);
+    object_property_add_child(OBJECT(s), "espdma", OBJECT(espdma), errp);
+    qdev_init_nofail(espdma);
+
+    esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));
+    sbd = SYS_BUS_DEVICE(esp);
+    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(espdma, 0));
+    qdev_connect_gpio_out(espdma, 0, qdev_get_gpio_in(esp, 0));
+    qdev_connect_gpio_out(espdma, 1, qdev_get_gpio_in(esp, 1));
+
+    sbd = SYS_BUS_DEVICE(espdma);
+    memory_region_add_subregion(&s->dmamem, 0x0,
+                                sysbus_mmio_get_region(sbd, 0));
+
+    ledma = qdev_create(NULL, TYPE_SPARC32_LEDMA_DEVICE);
+    object_property_set_link(OBJECT(ledma), iommu, "iommu", errp);
+    object_property_add_child(OBJECT(s), "ledma", OBJECT(ledma), errp);
+    qdev_init_nofail(ledma);
+
+    lance = DEVICE(object_resolve_path_component(OBJECT(ledma), "lance"));
+    sbd = SYS_BUS_DEVICE(lance);
+    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(ledma, 0));
+    qdev_connect_gpio_out(ledma, 0, qdev_get_gpio_in(lance, 0));
+
+    sbd = SYS_BUS_DEVICE(ledma);
+    memory_region_add_subregion(&s->dmamem, 0x10,
+                                sysbus_mmio_get_region(sbd, 0));
+}
+
+static void sparc32_dma_init(Object *obj)
+{
+    SPARC32DMAState *s = SPARC32_DMA(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+
+    memory_region_init(&s->dmamem, OBJECT(s), "dma", DMA_SIZE + DMA_ETH_SIZE);
+    sysbus_init_mmio(sbd, &s->dmamem);
+}
+
+static void sparc32_dma_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = sparc32_dma_realize;
+}
+
+static const TypeInfo sparc32_dma_info = {
+    .name          = TYPE_SPARC32_DMA,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(SPARC32DMAState),
+    .instance_init = sparc32_dma_init,
+    .class_init    = sparc32_dma_class_init,
+};
+
+
 static void sparc32_dma_register_types(void)
 {
     type_register_static(&sparc32_dma_device_info);
     type_register_static(&sparc32_espdma_device_info);
     type_register_static(&sparc32_ledma_device_info);
+    type_register_static(&sparc32_dma_info);
 }
 
 type_init(sparc32_dma_register_types)
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index ae486a4..5017ae5 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -307,18 +307,36 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
     return s;
 }
 
-static void *sparc32_dma_init(hwaddr daddr, void *iommu, int is_ledma)
+static void *sparc32_dma_init(hwaddr dma_base,
+                              hwaddr esp_base, qemu_irq espdma_irq,
+                              hwaddr le_base, qemu_irq ledma_irq)
 {
-    DeviceState *dev;
-    SysBusDevice *s;
+    DeviceState *dma;
+    ESPDMADeviceState *espdma;
+    LEDMADeviceState *ledma;
+    SysBusESPState *esp;
+    SysBusPCNetState *lance;
 
-    dev = qdev_create(NULL, is_ledma ? "sparc32-ledma" : "sparc32-espdma");
-    object_property_set_link(OBJECT(dev), OBJECT(iommu), "iommu", &error_abort);
-    qdev_init_nofail(dev);
-    s = SYS_BUS_DEVICE(dev);
-    sysbus_mmio_map(s, 0, daddr);
+    dma = qdev_create(NULL, TYPE_SPARC32_DMA);
+    qdev_init_nofail(dma);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
 
-    return s;
+    espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
+                                   OBJECT(dma), "espdma"));
+    sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
+
+    esp = ESP_STATE(object_resolve_path_component(OBJECT(espdma), "esp"));
+    sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
+
+    ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(
+                                 OBJECT(dma), "ledma"));
+    sysbus_connect_irq(SYS_BUS_DEVICE(ledma), 0, ledma_irq);
+
+    lance = SYSBUS_PCNET(object_resolve_path_component(
+                         OBJECT(ledma), "lance"));
+    sysbus_mmio_map(SYS_BUS_DEVICE(lance), 0, le_base);
+
+    return dma;
 }
 
 static DeviceState *slavio_intctl_init(hwaddr addr,
@@ -798,9 +816,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     DeviceState *slavio_intctl;
     const char *cpu_model = machine->cpu_model;
     unsigned int i;
-    void *iommu, *nvram;
-    DeviceState *espdma, *esp, *ledma, *lance;
-    SysBusDevice *sbd;
+    void *nvram;
     qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS];
     qemu_irq fdc_tc;
     unsigned long kernel_size;
@@ -848,8 +864,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
         afx_init(hwdef->afx_base);
     }
 
-    iommu = iommu_init(hwdef->iommu_base, hwdef->iommu_version,
-                       slavio_irq[30]);
+    iommu_init(hwdef->iommu_base, hwdef->iommu_version, slavio_irq[30]);
 
     if (hwdef->iommu_pad_base) {
         /* On the real hardware (SS-5, LX) the MMU is not padded, but aliased.
@@ -859,26 +874,9 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
         empty_slot_init(hwdef->iommu_pad_base,hwdef->iommu_pad_len);
     }
 
-    espdma = sparc32_dma_init(hwdef->dma_base, iommu, 0);
-    sbd = SYS_BUS_DEVICE(espdma);
-    sysbus_connect_irq(sbd, 0, slavio_irq[18]);
-
-    esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));
-    sbd = SYS_BUS_DEVICE(esp);
-    sysbus_mmio_map(sbd, 0, hwdef->esp_base);
-    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(espdma, 0));
-    qdev_connect_gpio_out(espdma, 0, qdev_get_gpio_in(esp, 0));
-    qdev_connect_gpio_out(espdma, 1, qdev_get_gpio_in(esp, 1));
-
-    ledma = sparc32_dma_init(hwdef->dma_base + 16ULL, iommu, 1);
-    sbd = SYS_BUS_DEVICE(ledma);
-    sysbus_connect_irq(sbd, 0, slavio_irq[16]);
-
-    lance = DEVICE(object_resolve_path_component(OBJECT(ledma), "lance"));
-    sbd = SYS_BUS_DEVICE(lance);
-    sysbus_mmio_map(sbd, 0, hwdef->le_base);
-    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(ledma, 0));
-    qdev_connect_gpio_out(ledma, 0, qdev_get_gpio_in(lance, 0));
+    sparc32_dma_init(hwdef->dma_base,
+                     hwdef->esp_base, slavio_irq[18],
+                     hwdef->le_base, slavio_irq[16]);
 
     if (graphic_depth != 8 && graphic_depth != 24) {
         error_report("Unsupported depth: %d", graphic_depth);
diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index 5e39d81..5deeca6 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -44,6 +44,18 @@ typedef struct LEDMADeviceState {
     SysBusPCNetState *lance;
 } LEDMADeviceState;
 
+#define TYPE_SPARC32_DMA "sparc32-dma"
+#define SPARC32_DMA(obj) OBJECT_CHECK(SPARC32DMAState, (obj), \
+                                      TYPE_SPARC32_DMA)
+
+typedef struct SPARC32DMAState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion dmamem;
+    ESPDMADeviceState *espdma;
+    LEDMADeviceState *ledma;
+} SPARC32DMAState;
+
 /* sparc32_dma.c */
 void ledma_memory_read(void *opaque, hwaddr addr,
                        uint8_t *buf, int len, int do_bswap);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv4 11/13] sparc32_dma: introduce new SPARC32_DMA type container object

[Philippe Mathieu-Daudé]

On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
> Create a new SPARC32_DMA container object (including an appropriate container
> memory region) and add instances of the SPARC32_ESPDMA_DEVICE and
> SPARC32_LEDMA_DEVICE as child objects. The benefit is that most of the gpio
> wiring complexity between esp/espdma and lance/ledma is now hidden within the
> SPARC32_DMA realize function.
> 
> Since the sun4m IOMMU is already QOMified we can find a reference to
> it using object_resolve_path_type() allowing us to completely remove all external
> references to the iommu pointer.
> 
> Finally we rework sun4m's sparc32_dma_init() to invoke the new SPARC32_DMA object
> and wire up the remaining board memory regions/IRQs.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
> ---
>  hw/dma/sparc32_dma.c           |   70 ++++++++++++++++++++++++++++++++++++++++
>  hw/sparc/sun4m.c               |   66 ++++++++++++++++++-------------------
>  include/hw/sparc/sparc32_dma.h |   12 +++++++
>  3 files changed, 114 insertions(+), 34 deletions(-)
> 
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index d4cff74..582b7cc 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -30,6 +30,7 @@
>  #include "hw/sparc/sparc32_dma.h"
>  #include "hw/sparc/sun4m.h"
>  #include "hw/sysbus.h"
> +#include "qapi/error.h"
>  #include "trace.h"
>  
>  /*
> @@ -369,11 +370,80 @@ static const TypeInfo sparc32_ledma_device_info = {
>      .class_init    = sparc32_ledma_device_class_init,
>  };
>  
> +static void sparc32_dma_realize(DeviceState *dev, Error **errp)
> +{
> +    SPARC32DMAState *s = SPARC32_DMA(dev);
> +    DeviceState *espdma, *esp, *ledma, *lance;
> +    SysBusDevice *sbd;
> +    Object *iommu;
> +
> +    iommu = object_resolve_path_type("", TYPE_SUN4M_IOMMU, NULL);
> +    if (!iommu) {
> +        error_setg(errp, "unable to locate sun4m IOMMU device");
> +        return;
> +    }
> +
> +    espdma = qdev_create(NULL, TYPE_SPARC32_ESPDMA_DEVICE);
> +    object_property_set_link(OBJECT(espdma), iommu, "iommu", errp);
> +    object_property_add_child(OBJECT(s), "espdma", OBJECT(espdma), errp);
> +    qdev_init_nofail(espdma);
> +
> +    esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));

TYPE_ESP?

> +    sbd = SYS_BUS_DEVICE(esp);
> +    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(espdma, 0));
> +    qdev_connect_gpio_out(espdma, 0, qdev_get_gpio_in(esp, 0));
> +    qdev_connect_gpio_out(espdma, 1, qdev_get_gpio_in(esp, 1));
> +
> +    sbd = SYS_BUS_DEVICE(espdma);
> +    memory_region_add_subregion(&s->dmamem, 0x0,
> +                                sysbus_mmio_get_region(sbd, 0));
> +
> +    ledma = qdev_create(NULL, TYPE_SPARC32_LEDMA_DEVICE);
> +    object_property_set_link(OBJECT(ledma), iommu, "iommu", errp);
> +    object_property_add_child(OBJECT(s), "ledma", OBJECT(ledma), errp);
> +    qdev_init_nofail(ledma);
> +
> +    lance = DEVICE(object_resolve_path_component(OBJECT(ledma), "lance"));

TYPE_LANCE?

> +    sbd = SYS_BUS_DEVICE(lance);
> +    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(ledma, 0));
> +    qdev_connect_gpio_out(ledma, 0, qdev_get_gpio_in(lance, 0));
> +
> +    sbd = SYS_BUS_DEVICE(ledma);
> +    memory_region_add_subregion(&s->dmamem, 0x10,
> +                                sysbus_mmio_get_region(sbd, 0));
> +}
> +
> +static void sparc32_dma_init(Object *obj)
> +{
> +    SPARC32DMAState *s = SPARC32_DMA(obj);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +
> +    memory_region_init(&s->dmamem, OBJECT(s), "dma", DMA_SIZE + DMA_ETH_SIZE);
> +    sysbus_init_mmio(sbd, &s->dmamem);
> +}
> +
> +static void sparc32_dma_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = sparc32_dma_realize;
> +}
> +
> +static const TypeInfo sparc32_dma_info = {
> +    .name          = TYPE_SPARC32_DMA,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(SPARC32DMAState),
> +    .instance_init = sparc32_dma_init,
> +    .class_init    = sparc32_dma_class_init,
> +};
> +
> +
>  static void sparc32_dma_register_types(void)
>  {
>      type_register_static(&sparc32_dma_device_info);
>      type_register_static(&sparc32_espdma_device_info);
>      type_register_static(&sparc32_ledma_device_info);
> +    type_register_static(&sparc32_dma_info);
>  }
>  
>  type_init(sparc32_dma_register_types)
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index ae486a4..5017ae5 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -307,18 +307,36 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
>      return s;
>  }
>  
> -static void *sparc32_dma_init(hwaddr daddr, void *iommu, int is_ledma)
> +static void *sparc32_dma_init(hwaddr dma_base,
> +                              hwaddr esp_base, qemu_irq espdma_irq,
> +                              hwaddr le_base, qemu_irq ledma_irq)
>  {
> -    DeviceState *dev;
> -    SysBusDevice *s;
> +    DeviceState *dma;
> +    ESPDMADeviceState *espdma;
> +    LEDMADeviceState *ledma;
> +    SysBusESPState *esp;
> +    SysBusPCNetState *lance;
>  
> -    dev = qdev_create(NULL, is_ledma ? "sparc32-ledma" : "sparc32-espdma");

TYPE_SPARC32_LEDMA_DEVICE, TYPE_SPARC32_ESPDMA_DEVICE

> -    object_property_set_link(OBJECT(dev), OBJECT(iommu), "iommu", &error_abort);
> -    qdev_init_nofail(dev);
> -    s = SYS_BUS_DEVICE(dev);
> -    sysbus_mmio_map(s, 0, daddr);
> +    dma = qdev_create(NULL, TYPE_SPARC32_DMA);
> +    qdev_init_nofail(dma);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
>  
> -    return s;
> +    espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
> +                                   OBJECT(dma), "espdma"));
> +    sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
> +
> +    esp = ESP_STATE(object_resolve_path_component(OBJECT(espdma), "esp"));

TYPE_ESP?

> +    sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
> +
> +    ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(
> +                                 OBJECT(dma), "ledma"));
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ledma), 0, ledma_irq);
> +
> +    lance = SYSBUS_PCNET(object_resolve_path_component(
> +                         OBJECT(ledma), "lance"));

TYPE_LANCE?

> +    sysbus_mmio_map(SYS_BUS_DEVICE(lance), 0, le_base);
> +
> +    return dma;
>  }
>  
>  static DeviceState *slavio_intctl_init(hwaddr addr,
> @@ -798,9 +816,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      DeviceState *slavio_intctl;
>      const char *cpu_model = machine->cpu_model;
>      unsigned int i;
> -    void *iommu, *nvram;
> -    DeviceState *espdma, *esp, *ledma, *lance;
> -    SysBusDevice *sbd;
> +    void *nvram;
>      qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS];
>      qemu_irq fdc_tc;
>      unsigned long kernel_size;
> @@ -848,8 +864,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>          afx_init(hwdef->afx_base);
>      }
>  
> -    iommu = iommu_init(hwdef->iommu_base, hwdef->iommu_version,
> -                       slavio_irq[30]);
> +    iommu_init(hwdef->iommu_base, hwdef->iommu_version, slavio_irq[30]);
>  
>      if (hwdef->iommu_pad_base) {
>          /* On the real hardware (SS-5, LX) the MMU is not padded, but aliased.
> @@ -859,26 +874,9 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>          empty_slot_init(hwdef->iommu_pad_base,hwdef->iommu_pad_len);
>      }
>  
> -    espdma = sparc32_dma_init(hwdef->dma_base, iommu, 0);
> -    sbd = SYS_BUS_DEVICE(espdma);
> -    sysbus_connect_irq(sbd, 0, slavio_irq[18]);
> -
> -    esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));
> -    sbd = SYS_BUS_DEVICE(esp);
> -    sysbus_mmio_map(sbd, 0, hwdef->esp_base);
> -    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(espdma, 0));
> -    qdev_connect_gpio_out(espdma, 0, qdev_get_gpio_in(esp, 0));
> -    qdev_connect_gpio_out(espdma, 1, qdev_get_gpio_in(esp, 1));
> -
> -    ledma = sparc32_dma_init(hwdef->dma_base + 16ULL, iommu, 1);
> -    sbd = SYS_BUS_DEVICE(ledma);
> -    sysbus_connect_irq(sbd, 0, slavio_irq[16]);
> -
> -    lance = DEVICE(object_resolve_path_component(OBJECT(ledma), "lance"));
> -    sbd = SYS_BUS_DEVICE(lance);
> -    sysbus_mmio_map(sbd, 0, hwdef->le_base);
> -    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(ledma, 0));
> -    qdev_connect_gpio_out(ledma, 0, qdev_get_gpio_in(lance, 0));
> +    sparc32_dma_init(hwdef->dma_base,
> +                     hwdef->esp_base, slavio_irq[18],
> +                     hwdef->le_base, slavio_irq[16]);
>  
>      if (graphic_depth != 8 && graphic_depth != 24) {
>          error_report("Unsupported depth: %d", graphic_depth);

OK until here.

> diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
> index 5e39d81..5deeca6 100644
> --- a/include/hw/sparc/sparc32_dma.h
> +++ b/include/hw/sparc/sparc32_dma.h
> @@ -44,6 +44,18 @@ typedef struct LEDMADeviceState {
>      SysBusPCNetState *lance;
>  } LEDMADeviceState;
>  
> +#define TYPE_SPARC32_DMA "sparc32-dma"
> +#define SPARC32_DMA(obj) OBJECT_CHECK(SPARC32DMAState, (obj), \
> +                                      TYPE_SPARC32_DMA)
> +
> +typedef struct SPARC32DMAState {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion dmamem;
> +    ESPDMADeviceState *espdma;
> +    LEDMADeviceState *ledma;
> +} SPARC32DMAState;

This part is "include/hw/sparc/sun4m.h"

> +
>  /* sparc32_dma.c */
>  void ledma_memory_read(void *opaque, hwaddr addr,
>                         uint8_t *buf, int len, int do_bswap);
> 

Re: [Qemu-devel] [PATCHv4 11/13] sparc32_dma: introduce new SPARC32_DMA type container object

[Mark Cave-Ayland]

On 27/10/17 17:18, Philippe Mathieu-Daudé wrote:
> On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
>> Create a new SPARC32_DMA container object (including an appropriate container
>> memory region) and add instances of the SPARC32_ESPDMA_DEVICE and
>> SPARC32_LEDMA_DEVICE as child objects. The benefit is that most of the gpio
>> wiring complexity between esp/espdma and lance/ledma is now hidden within the
>> SPARC32_DMA realize function.
>>
>> Since the sun4m IOMMU is already QOMified we can find a reference to
>> it using object_resolve_path_type() allowing us to completely remove all external
>> references to the iommu pointer.
>>
>> Finally we rework sun4m's sparc32_dma_init() to invoke the new SPARC32_DMA object
>> and wire up the remaining board memory regions/IRQs.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
>> ---
>>  hw/dma/sparc32_dma.c           |   70 ++++++++++++++++++++++++++++++++++++++++
>>  hw/sparc/sun4m.c               |   66 ++++++++++++++++++-------------------
>>  include/hw/sparc/sparc32_dma.h |   12 +++++++
>>  3 files changed, 114 insertions(+), 34 deletions(-)
>>
>> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
>> index d4cff74..582b7cc 100644
>> --- a/hw/dma/sparc32_dma.c
>> +++ b/hw/dma/sparc32_dma.c
>> @@ -30,6 +30,7 @@
>>  #include "hw/sparc/sparc32_dma.h"
>>  #include "hw/sparc/sun4m.h"
>>  #include "hw/sysbus.h"
>> +#include "qapi/error.h"
>>  #include "trace.h"
>>  
>>  /*
>> @@ -369,11 +370,80 @@ static const TypeInfo sparc32_ledma_device_info = {
>>      .class_init    = sparc32_ledma_device_class_init,
>>  };
>>  
>> +static void sparc32_dma_realize(DeviceState *dev, Error **errp)
>> +{
>> +    SPARC32DMAState *s = SPARC32_DMA(dev);
>> +    DeviceState *espdma, *esp, *ledma, *lance;
>> +    SysBusDevice *sbd;
>> +    Object *iommu;
>> +
>> +    iommu = object_resolve_path_type("", TYPE_SUN4M_IOMMU, NULL);
>> +    if (!iommu) {
>> +        error_setg(errp, "unable to locate sun4m IOMMU device");
>> +        return;
>> +    }
>> +
>> +    espdma = qdev_create(NULL, TYPE_SPARC32_ESPDMA_DEVICE);
>> +    object_property_set_link(OBJECT(espdma), iommu, "iommu", errp);
>> +    object_property_add_child(OBJECT(s), "espdma", OBJECT(espdma), errp);
>> +    qdev_init_nofail(espdma);
>> +
>> +    esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));
> 
> TYPE_ESP?
> 
>> +    sbd = SYS_BUS_DEVICE(esp);
>> +    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(espdma, 0));
>> +    qdev_connect_gpio_out(espdma, 0, qdev_get_gpio_in(esp, 0));
>> +    qdev_connect_gpio_out(espdma, 1, qdev_get_gpio_in(esp, 1));
>> +
>> +    sbd = SYS_BUS_DEVICE(espdma);
>> +    memory_region_add_subregion(&s->dmamem, 0x0,
>> +                                sysbus_mmio_get_region(sbd, 0));
>> +
>> +    ledma = qdev_create(NULL, TYPE_SPARC32_LEDMA_DEVICE);
>> +    object_property_set_link(OBJECT(ledma), iommu, "iommu", errp);
>> +    object_property_add_child(OBJECT(s), "ledma", OBJECT(ledma), errp);
>> +    qdev_init_nofail(ledma);
>> +
>> +    lance = DEVICE(object_resolve_path_component(OBJECT(ledma), "lance"));
> 
> TYPE_LANCE?
> 
>> +    sbd = SYS_BUS_DEVICE(lance);
>> +    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(ledma, 0));
>> +    qdev_connect_gpio_out(ledma, 0, qdev_get_gpio_in(lance, 0));
>> +
>> +    sbd = SYS_BUS_DEVICE(ledma);
>> +    memory_region_add_subregion(&s->dmamem, 0x10,
>> +                                sysbus_mmio_get_region(sbd, 0));
>> +}
>> +
>> +static void sparc32_dma_init(Object *obj)
>> +{
>> +    SPARC32DMAState *s = SPARC32_DMA(obj);
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>> +
>> +    memory_region_init(&s->dmamem, OBJECT(s), "dma", DMA_SIZE + DMA_ETH_SIZE);
>> +    sysbus_init_mmio(sbd, &s->dmamem);
>> +}
>> +
>> +static void sparc32_dma_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->realize = sparc32_dma_realize;
>> +}
>> +
>> +static const TypeInfo sparc32_dma_info = {
>> +    .name          = TYPE_SPARC32_DMA,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(SPARC32DMAState),
>> +    .instance_init = sparc32_dma_init,
>> +    .class_init    = sparc32_dma_class_init,
>> +};
>> +
>> +
>>  static void sparc32_dma_register_types(void)
>>  {
>>      type_register_static(&sparc32_dma_device_info);
>>      type_register_static(&sparc32_espdma_device_info);
>>      type_register_static(&sparc32_ledma_device_info);
>> +    type_register_static(&sparc32_dma_info);
>>  }
>>  
>>  type_init(sparc32_dma_register_types)
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index ae486a4..5017ae5 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -307,18 +307,36 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
>>      return s;
>>  }
>>  
>> -static void *sparc32_dma_init(hwaddr daddr, void *iommu, int is_ledma)
>> +static void *sparc32_dma_init(hwaddr dma_base,
>> +                              hwaddr esp_base, qemu_irq espdma_irq,
>> +                              hwaddr le_base, qemu_irq ledma_irq)
>>  {
>> -    DeviceState *dev;
>> -    SysBusDevice *s;
>> +    DeviceState *dma;
>> +    ESPDMADeviceState *espdma;
>> +    LEDMADeviceState *ledma;
>> +    SysBusESPState *esp;
>> +    SysBusPCNetState *lance;
>>  
>> -    dev = qdev_create(NULL, is_ledma ? "sparc32-ledma" : "sparc32-espdma");
> 
> TYPE_SPARC32_LEDMA_DEVICE, TYPE_SPARC32_ESPDMA_DEVICE
> 
>> -    object_property_set_link(OBJECT(dev), OBJECT(iommu), "iommu", &error_abort);
>> -    qdev_init_nofail(dev);
>> -    s = SYS_BUS_DEVICE(dev);
>> -    sysbus_mmio_map(s, 0, daddr);
>> +    dma = qdev_create(NULL, TYPE_SPARC32_DMA);
>> +    qdev_init_nofail(dma);
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
>>  
>> -    return s;
>> +    espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
>> +                                   OBJECT(dma), "espdma"));
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
>> +
>> +    esp = ESP_STATE(object_resolve_path_component(OBJECT(espdma), "esp"));
> 
> TYPE_ESP?
> 
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
>> +
>> +    ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(
>> +                                 OBJECT(dma), "ledma"));
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ledma), 0, ledma_irq);
>> +
>> +    lance = SYSBUS_PCNET(object_resolve_path_component(
>> +                         OBJECT(ledma), "lance"));
> 
> TYPE_LANCE?
> 
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(lance), 0, le_base);
>> +
>> +    return dma;
>>  }
>>  
>>  static DeviceState *slavio_intctl_init(hwaddr addr,
>> @@ -798,9 +816,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>      DeviceState *slavio_intctl;
>>      const char *cpu_model = machine->cpu_model;
>>      unsigned int i;
>> -    void *iommu, *nvram;
>> -    DeviceState *espdma, *esp, *ledma, *lance;
>> -    SysBusDevice *sbd;
>> +    void *nvram;
>>      qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS];
>>      qemu_irq fdc_tc;
>>      unsigned long kernel_size;
>> @@ -848,8 +864,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>          afx_init(hwdef->afx_base);
>>      }
>>  
>> -    iommu = iommu_init(hwdef->iommu_base, hwdef->iommu_version,
>> -                       slavio_irq[30]);
>> +    iommu_init(hwdef->iommu_base, hwdef->iommu_version, slavio_irq[30]);
>>  
>>      if (hwdef->iommu_pad_base) {
>>          /* On the real hardware (SS-5, LX) the MMU is not padded, but aliased.
>> @@ -859,26 +874,9 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>          empty_slot_init(hwdef->iommu_pad_base,hwdef->iommu_pad_len);
>>      }
>>  
>> -    espdma = sparc32_dma_init(hwdef->dma_base, iommu, 0);
>> -    sbd = SYS_BUS_DEVICE(espdma);
>> -    sysbus_connect_irq(sbd, 0, slavio_irq[18]);
>> -
>> -    esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));
>> -    sbd = SYS_BUS_DEVICE(esp);
>> -    sysbus_mmio_map(sbd, 0, hwdef->esp_base);
>> -    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(espdma, 0));
>> -    qdev_connect_gpio_out(espdma, 0, qdev_get_gpio_in(esp, 0));
>> -    qdev_connect_gpio_out(espdma, 1, qdev_get_gpio_in(esp, 1));
>> -
>> -    ledma = sparc32_dma_init(hwdef->dma_base + 16ULL, iommu, 1);
>> -    sbd = SYS_BUS_DEVICE(ledma);
>> -    sysbus_connect_irq(sbd, 0, slavio_irq[16]);
>> -
>> -    lance = DEVICE(object_resolve_path_component(OBJECT(ledma), "lance"));
>> -    sbd = SYS_BUS_DEVICE(lance);
>> -    sysbus_mmio_map(sbd, 0, hwdef->le_base);
>> -    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(ledma, 0));
>> -    qdev_connect_gpio_out(ledma, 0, qdev_get_gpio_in(lance, 0));
>> +    sparc32_dma_init(hwdef->dma_base,
>> +                     hwdef->esp_base, slavio_irq[18],
>> +                     hwdef->le_base, slavio_irq[16]);
>>  
>>      if (graphic_depth != 8 && graphic_depth != 24) {
>>          error_report("Unsupported depth: %d", graphic_depth);

I disagree with using the TYPE_* macros for the link property names
because semantically we aren't referencing the type, it just so happens
that I've used the same name as the type for the property name and I'd
like to keep this distinction.


ATB,

Mark.

Re: [Qemu-devel] [PATCHv4 11/13] sparc32_dma: introduce new SPARC32_DMA type container object

[Philippe Mathieu-Daudé]

On Mon, Oct 30, 2017 at 3:51 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 27/10/17 17:18, Philippe Mathieu-Daudé wrote:
>> On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
>>> Create a new SPARC32_DMA container object (including an appropriate container
>>> memory region) and add instances of the SPARC32_ESPDMA_DEVICE and
>>> SPARC32_LEDMA_DEVICE as child objects. The benefit is that most of the gpio
>>> wiring complexity between esp/espdma and lance/ledma is now hidden within the
>>> SPARC32_DMA realize function.
>>>
>>> Since the sun4m IOMMU is already QOMified we can find a reference to
>>> it using object_resolve_path_type() allowing us to completely remove all external
>>> references to the iommu pointer.
>>>
>>> Finally we rework sun4m's sparc32_dma_init() to invoke the new SPARC32_DMA object
>>> and wire up the remaining board memory regions/IRQs.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
>>> ---
>>>  hw/dma/sparc32_dma.c           |   70 ++++++++++++++++++++++++++++++++++++++++
>>>  hw/sparc/sun4m.c               |   66 ++++++++++++++++++-------------------
>>>  include/hw/sparc/sparc32_dma.h |   12 +++++++
>>>  3 files changed, 114 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
>>> index d4cff74..582b7cc 100644
>>> --- a/hw/dma/sparc32_dma.c
>>> +++ b/hw/dma/sparc32_dma.c
>>> @@ -30,6 +30,7 @@
>>>  #include "hw/sparc/sparc32_dma.h"
>>>  #include "hw/sparc/sun4m.h"
>>>  #include "hw/sysbus.h"
>>> +#include "qapi/error.h"
>>>  #include "trace.h"
>>>
>>>  /*
>>> @@ -369,11 +370,80 @@ static const TypeInfo sparc32_ledma_device_info = {
>>>      .class_init    = sparc32_ledma_device_class_init,
>>>  };
>>>
>>> +static void sparc32_dma_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    SPARC32DMAState *s = SPARC32_DMA(dev);
>>> +    DeviceState *espdma, *esp, *ledma, *lance;
>>> +    SysBusDevice *sbd;
>>> +    Object *iommu;
>>> +
>>> +    iommu = object_resolve_path_type("", TYPE_SUN4M_IOMMU, NULL);
>>> +    if (!iommu) {
>>> +        error_setg(errp, "unable to locate sun4m IOMMU device");
>>> +        return;
>>> +    }
>>> +
>>> +    espdma = qdev_create(NULL, TYPE_SPARC32_ESPDMA_DEVICE);
>>> +    object_property_set_link(OBJECT(espdma), iommu, "iommu", errp);
>>> +    object_property_add_child(OBJECT(s), "espdma", OBJECT(espdma), errp);
>>> +    qdev_init_nofail(espdma);
>>> +
>>> +    esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));
>>
>> TYPE_ESP?
>>
>>> +    sbd = SYS_BUS_DEVICE(esp);
>>> +    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(espdma, 0));
>>> +    qdev_connect_gpio_out(espdma, 0, qdev_get_gpio_in(esp, 0));
>>> +    qdev_connect_gpio_out(espdma, 1, qdev_get_gpio_in(esp, 1));
>>> +
>>> +    sbd = SYS_BUS_DEVICE(espdma);
>>> +    memory_region_add_subregion(&s->dmamem, 0x0,
>>> +                                sysbus_mmio_get_region(sbd, 0));
>>> +
>>> +    ledma = qdev_create(NULL, TYPE_SPARC32_LEDMA_DEVICE);
>>> +    object_property_set_link(OBJECT(ledma), iommu, "iommu", errp);
>>> +    object_property_add_child(OBJECT(s), "ledma", OBJECT(ledma), errp);
>>> +    qdev_init_nofail(ledma);
>>> +
>>> +    lance = DEVICE(object_resolve_path_component(OBJECT(ledma), "lance"));
>>
>> TYPE_LANCE?
>>
>>> +    sbd = SYS_BUS_DEVICE(lance);
>>> +    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(ledma, 0));
>>> +    qdev_connect_gpio_out(ledma, 0, qdev_get_gpio_in(lance, 0));
>>> +
>>> +    sbd = SYS_BUS_DEVICE(ledma);
>>> +    memory_region_add_subregion(&s->dmamem, 0x10,
>>> +                                sysbus_mmio_get_region(sbd, 0));
>>> +}
>>> +
>>> +static void sparc32_dma_init(Object *obj)
>>> +{
>>> +    SPARC32DMAState *s = SPARC32_DMA(obj);
>>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>>> +
>>> +    memory_region_init(&s->dmamem, OBJECT(s), "dma", DMA_SIZE + DMA_ETH_SIZE);
>>> +    sysbus_init_mmio(sbd, &s->dmamem);
>>> +}
>>> +
>>> +static void sparc32_dma_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> +    dc->realize = sparc32_dma_realize;
>>> +}
>>> +
>>> +static const TypeInfo sparc32_dma_info = {
>>> +    .name          = TYPE_SPARC32_DMA,
>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>> +    .instance_size = sizeof(SPARC32DMAState),
>>> +    .instance_init = sparc32_dma_init,
>>> +    .class_init    = sparc32_dma_class_init,
>>> +};
>>> +
>>> +
>>>  static void sparc32_dma_register_types(void)
>>>  {
>>>      type_register_static(&sparc32_dma_device_info);
>>>      type_register_static(&sparc32_espdma_device_info);
>>>      type_register_static(&sparc32_ledma_device_info);
>>> +    type_register_static(&sparc32_dma_info);
>>>  }
>>>
>>>  type_init(sparc32_dma_register_types)
>>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>>> index ae486a4..5017ae5 100644
>>> --- a/hw/sparc/sun4m.c
>>> +++ b/hw/sparc/sun4m.c
>>> @@ -307,18 +307,36 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
>>>      return s;
>>>  }
>>>
>>> -static void *sparc32_dma_init(hwaddr daddr, void *iommu, int is_ledma)
>>> +static void *sparc32_dma_init(hwaddr dma_base,
>>> +                              hwaddr esp_base, qemu_irq espdma_irq,
>>> +                              hwaddr le_base, qemu_irq ledma_irq)
>>>  {
>>> -    DeviceState *dev;
>>> -    SysBusDevice *s;
>>> +    DeviceState *dma;
>>> +    ESPDMADeviceState *espdma;
>>> +    LEDMADeviceState *ledma;
>>> +    SysBusESPState *esp;
>>> +    SysBusPCNetState *lance;
>>>
>>> -    dev = qdev_create(NULL, is_ledma ? "sparc32-ledma" : "sparc32-espdma");
>>
>> TYPE_SPARC32_LEDMA_DEVICE, TYPE_SPARC32_ESPDMA_DEVICE
>>
>>> -    object_property_set_link(OBJECT(dev), OBJECT(iommu), "iommu", &error_abort);
>>> -    qdev_init_nofail(dev);
>>> -    s = SYS_BUS_DEVICE(dev);
>>> -    sysbus_mmio_map(s, 0, daddr);
>>> +    dma = qdev_create(NULL, TYPE_SPARC32_DMA);
>>> +    qdev_init_nofail(dma);
>>> +    sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
>>>
>>> -    return s;
>>> +    espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
>>> +                                   OBJECT(dma), "espdma"));
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
>>> +
>>> +    esp = ESP_STATE(object_resolve_path_component(OBJECT(espdma), "esp"));
>>
>> TYPE_ESP?
>>
>>> +    sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
>>> +
>>> +    ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(
>>> +                                 OBJECT(dma), "ledma"));
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ledma), 0, ledma_irq);
>>> +
>>> +    lance = SYSBUS_PCNET(object_resolve_path_component(
>>> +                         OBJECT(ledma), "lance"));
>>
>> TYPE_LANCE?
>>
>>> +    sysbus_mmio_map(SYS_BUS_DEVICE(lance), 0, le_base);
>>> +
>>> +    return dma;
>>>  }
>>>
>>>  static DeviceState *slavio_intctl_init(hwaddr addr,
>>> @@ -798,9 +816,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>>      DeviceState *slavio_intctl;
>>>      const char *cpu_model = machine->cpu_model;
>>>      unsigned int i;
>>> -    void *iommu, *nvram;
>>> -    DeviceState *espdma, *esp, *ledma, *lance;
>>> -    SysBusDevice *sbd;
>>> +    void *nvram;
>>>      qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS];
>>>      qemu_irq fdc_tc;
>>>      unsigned long kernel_size;
>>> @@ -848,8 +864,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>>          afx_init(hwdef->afx_base);
>>>      }
>>>
>>> -    iommu = iommu_init(hwdef->iommu_base, hwdef->iommu_version,
>>> -                       slavio_irq[30]);
>>> +    iommu_init(hwdef->iommu_base, hwdef->iommu_version, slavio_irq[30]);
>>>
>>>      if (hwdef->iommu_pad_base) {
>>>          /* On the real hardware (SS-5, LX) the MMU is not padded, but aliased.
>>> @@ -859,26 +874,9 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>>          empty_slot_init(hwdef->iommu_pad_base,hwdef->iommu_pad_len);
>>>      }
>>>
>>> -    espdma = sparc32_dma_init(hwdef->dma_base, iommu, 0);
>>> -    sbd = SYS_BUS_DEVICE(espdma);
>>> -    sysbus_connect_irq(sbd, 0, slavio_irq[18]);
>>> -
>>> -    esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));
>>> -    sbd = SYS_BUS_DEVICE(esp);
>>> -    sysbus_mmio_map(sbd, 0, hwdef->esp_base);
>>> -    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(espdma, 0));
>>> -    qdev_connect_gpio_out(espdma, 0, qdev_get_gpio_in(esp, 0));
>>> -    qdev_connect_gpio_out(espdma, 1, qdev_get_gpio_in(esp, 1));
>>> -
>>> -    ledma = sparc32_dma_init(hwdef->dma_base + 16ULL, iommu, 1);
>>> -    sbd = SYS_BUS_DEVICE(ledma);
>>> -    sysbus_connect_irq(sbd, 0, slavio_irq[16]);
>>> -
>>> -    lance = DEVICE(object_resolve_path_component(OBJECT(ledma), "lance"));
>>> -    sbd = SYS_BUS_DEVICE(lance);
>>> -    sysbus_mmio_map(sbd, 0, hwdef->le_base);
>>> -    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(ledma, 0));
>>> -    qdev_connect_gpio_out(ledma, 0, qdev_get_gpio_in(lance, 0));
>>> +    sparc32_dma_init(hwdef->dma_base,
>>> +                     hwdef->esp_base, slavio_irq[18],
>>> +                     hwdef->le_base, slavio_irq[16]);
>>>
>>>      if (graphic_depth != 8 && graphic_depth != 24) {
>>>          error_report("Unsupported depth: %d", graphic_depth);
>
> I disagree with using the TYPE_* macros for the link property names
> because semantically we aren't referencing the type, it just so happens
> that I've used the same name as the type for the property name and I'd
> like to keep this distinction.

OK.

[Qemu-devel] [PATCHv4 12/13] sparc32_dma: remove is_ledma hack and replace with memory region alias

[Mark Cave-Ayland]

This hack originated from before the memory region API was introduced, and
increased the size of the ledma DMA device to capture incorrect accesses
beyond the end of the ledma device. A full analysis can be found on Artyom's
blog at http://tyom.blogspot.co.uk/2010/10/bug-in-all-solaris-versions-after-57.html.

With the memory API we can now simply alias the incorrect access onto its
intended destination allowing us to remove the hack.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
---
 hw/dma/sparc32_dma.c           |   20 ++++++--------------
 include/hw/sparc/sparc32_dma.h |    2 +-
 2 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index 582b7cc..f64787e 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -160,12 +160,6 @@ static uint64_t dma_mem_read(void *opaque, hwaddr addr,
     DMADeviceState *s = opaque;
     uint32_t saddr;
 
-    if (s->is_ledma && (addr > DMA_MAX_REG_OFFSET)) {
-        /* aliased to espdma, but we can't get there from here */
-        /* buggy driver if using undocumented behavior, just return 0 */
-        trace_sparc32_dma_mem_readl(addr, 0);
-        return 0;
-    }
     saddr = (addr & DMA_MASK) >> 2;
     trace_sparc32_dma_mem_readl(addr, s->dmaregs[saddr]);
     return s->dmaregs[saddr];
@@ -177,11 +171,6 @@ static void dma_mem_write(void *opaque, hwaddr addr,
     DMADeviceState *s = opaque;
     uint32_t saddr;
 
-    if (s->is_ledma && (addr > DMA_MAX_REG_OFFSET)) {
-        /* aliased to espdma, but we can't get there from here */
-        trace_sparc32_dma_mem_writel(addr, 0, val);
-        return;
-    }
     saddr = (addr & DMA_MASK) >> 2;
     trace_sparc32_dma_mem_writel(addr, s->dmaregs[saddr], val);
     switch (saddr) {
@@ -296,7 +285,6 @@ static void sparc32_espdma_device_init(Object *obj)
 
     memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
                           "espdma-mmio", DMA_SIZE);
-    s->is_ledma = 0;
 }
 
 static void sparc32_espdma_device_realize(DeviceState *dev, Error **errp)
@@ -337,8 +325,7 @@ static void sparc32_ledma_device_init(Object *obj)
     DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
 
     memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
-                          "ledma-mmio", DMA_ETH_SIZE);
-    s->is_ledma = 1;
+                          "ledma-mmio", DMA_SIZE);
 }
 
 static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
@@ -411,6 +398,11 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
     sbd = SYS_BUS_DEVICE(ledma);
     memory_region_add_subregion(&s->dmamem, 0x10,
                                 sysbus_mmio_get_region(sbd, 0));
+
+    /* Add ledma alias to handle SunOS 5.7 - Solaris 9 invalid access bug */
+    memory_region_init_alias(&s->ledma_alias, OBJECT(dev), "ledma-alias",
+                             sysbus_mmio_get_region(sbd, 0), 0x4, 0x4);
+    memory_region_add_subregion(&s->dmamem, 0x20, &s->ledma_alias);
 }
 
 static void sparc32_dma_init(Object *obj)
diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index 5deeca6..ab42c54 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -21,7 +21,6 @@ struct DMADeviceState {
     qemu_irq irq;
     void *iommu;
     qemu_irq gpio[2];
-    uint32_t is_ledma;
 };
 
 #define TYPE_SPARC32_ESPDMA_DEVICE "sparc32-espdma"
@@ -52,6 +51,7 @@ typedef struct SPARC32DMAState {
     SysBusDevice parent_obj;
 
     MemoryRegion dmamem;
+    MemoryRegion ledma_alias;
     ESPDMADeviceState *espdma;
     LEDMADeviceState *ledma;
 } SPARC32DMAState;
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv4 12/13] sparc32_dma: remove is_ledma hack and replace with memory region alias

[Philippe Mathieu-Daudé]

On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
> This hack originated from before the memory region API was introduced, and
> increased the size of the ledma DMA device to capture incorrect accesses
> beyond the end of the ledma device. A full analysis can be found on Artyom's
> blog at http://tyom.blogspot.co.uk/2010/10/bug-in-all-solaris-versions-after-57.html.
> 
> With the memory API we can now simply alias the incorrect access onto its
> intended destination allowing us to remove the hack.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
> ---
>  hw/dma/sparc32_dma.c           |   20 ++++++--------------
>  include/hw/sparc/sparc32_dma.h |    2 +-
>  2 files changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index 582b7cc..f64787e 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -160,12 +160,6 @@ static uint64_t dma_mem_read(void *opaque, hwaddr addr,
>      DMADeviceState *s = opaque;
>      uint32_t saddr;
>  
> -    if (s->is_ledma && (addr > DMA_MAX_REG_OFFSET)) {
> -        /* aliased to espdma, but we can't get there from here */
> -        /* buggy driver if using undocumented behavior, just return 0 */
> -        trace_sparc32_dma_mem_readl(addr, 0);
> -        return 0;
> -    }
>      saddr = (addr & DMA_MASK) >> 2;
>      trace_sparc32_dma_mem_readl(addr, s->dmaregs[saddr]);
>      return s->dmaregs[saddr];
> @@ -177,11 +171,6 @@ static void dma_mem_write(void *opaque, hwaddr addr,
>      DMADeviceState *s = opaque;
>      uint32_t saddr;
>  
> -    if (s->is_ledma && (addr > DMA_MAX_REG_OFFSET)) {
> -        /* aliased to espdma, but we can't get there from here */
> -        trace_sparc32_dma_mem_writel(addr, 0, val);
> -        return;
> -    }
>      saddr = (addr & DMA_MASK) >> 2;
>      trace_sparc32_dma_mem_writel(addr, s->dmaregs[saddr], val);
>      switch (saddr) {
> @@ -296,7 +285,6 @@ static void sparc32_espdma_device_init(Object *obj)
>  
>      memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
>                            "espdma-mmio", DMA_SIZE);
> -    s->is_ledma = 0;
>  }
>  
>  static void sparc32_espdma_device_realize(DeviceState *dev, Error **errp)
> @@ -337,8 +325,7 @@ static void sparc32_ledma_device_init(Object *obj)
>      DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
>  
>      memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
> -                          "ledma-mmio", DMA_ETH_SIZE);
> -    s->is_ledma = 1;
> +                          "ledma-mmio", DMA_SIZE);
>  }
>  
>  static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
> @@ -411,6 +398,11 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
>      sbd = SYS_BUS_DEVICE(ledma);
>      memory_region_add_subregion(&s->dmamem, 0x10,
>                                  sysbus_mmio_get_region(sbd, 0));
> +
> +    /* Add ledma alias to handle SunOS 5.7 - Solaris 9 invalid access bug */
> +    memory_region_init_alias(&s->ledma_alias, OBJECT(dev), "ledma-alias",
> +                             sysbus_mmio_get_region(sbd, 0), 0x4, 0x4);
> +    memory_region_add_subregion(&s->dmamem, 0x20, &s->ledma_alias);
>  }
>  
>  static void sparc32_dma_init(Object *obj)
> diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
> index 5deeca6..ab42c54 100644
> --- a/include/hw/sparc/sparc32_dma.h
> +++ b/include/hw/sparc/sparc32_dma.h
> @@ -21,7 +21,6 @@ struct DMADeviceState {
>      qemu_irq irq;
>      void *iommu;
>      qemu_irq gpio[2];
> -    uint32_t is_ledma;
>  };
>  
>  #define TYPE_SPARC32_ESPDMA_DEVICE "sparc32-espdma"
> @@ -52,6 +51,7 @@ typedef struct SPARC32DMAState {
>      SysBusDevice parent_obj;
>  
>      MemoryRegion dmamem;
> +    MemoryRegion ledma_alias;
>      ESPDMADeviceState *espdma;
>      LEDMADeviceState *ledma;
>  } SPARC32DMAState;
> 

The same using "include/hw/sparc/sun4m.h":
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

[Qemu-devel] [PATCHv4 13/13] sparc32_dma: add len to esp/le DMA memory tracing

[Mark Cave-Ayland]

This is surprisingly useful when trying to debug DMA issues.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/dma/sparc32_dma.c |    8 ++++----
 hw/dma/trace-events  |    8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index f64787e..7d00f1a 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -74,7 +74,7 @@ void ledma_memory_read(void *opaque, hwaddr addr,
     int i;
 
     addr |= s->dmaregs[3];
-    trace_ledma_memory_read(addr);
+    trace_ledma_memory_read(addr, len);
     if (do_bswap) {
         sparc_iommu_memory_read(s->iommu, addr, buf, len);
     } else {
@@ -95,7 +95,7 @@ void ledma_memory_write(void *opaque, hwaddr addr,
     uint16_t tmp_buf[32];
 
     addr |= s->dmaregs[3];
-    trace_ledma_memory_write(addr);
+    trace_ledma_memory_write(addr, len);
     if (do_bswap) {
         sparc_iommu_memory_write(s->iommu, addr, buf, len);
     } else {
@@ -140,7 +140,7 @@ void espdma_memory_read(void *opaque, uint8_t *buf, int len)
 {
     DMADeviceState *s = opaque;
 
-    trace_espdma_memory_read(s->dmaregs[1]);
+    trace_espdma_memory_read(s->dmaregs[1], len);
     sparc_iommu_memory_read(s->iommu, s->dmaregs[1], buf, len);
     s->dmaregs[1] += len;
 }
@@ -149,7 +149,7 @@ void espdma_memory_write(void *opaque, uint8_t *buf, int len)
 {
     DMADeviceState *s = opaque;
 
-    trace_espdma_memory_write(s->dmaregs[1]);
+    trace_espdma_memory_write(s->dmaregs[1], len);
     sparc_iommu_memory_write(s->iommu, s->dmaregs[1], buf, len);
     s->dmaregs[1] += len;
 }
diff --git a/hw/dma/trace-events b/hw/dma/trace-events
index 428469a..6b367f0 100644
--- a/hw/dma/trace-events
+++ b/hw/dma/trace-events
@@ -7,12 +7,12 @@ rc4030_read(uint64_t addr, uint32_t ret) "read reg[0x%"PRIx64"] = 0x%x"
 rc4030_write(uint64_t addr, uint32_t val) "write reg[0x%"PRIx64"] = 0x%x"
 
 # hw/dma/sparc32_dma.c
-ledma_memory_read(uint64_t addr) "DMA read addr 0x%"PRIx64
-ledma_memory_write(uint64_t addr) "DMA write addr 0x%"PRIx64
+ledma_memory_read(uint64_t addr, int len) "DMA read addr 0x%"PRIx64 " len %d"
+ledma_memory_write(uint64_t addr, int len) "DMA write addr 0x%"PRIx64 " len %d"
 sparc32_dma_set_irq_raise(void) "Raise IRQ"
 sparc32_dma_set_irq_lower(void) "Lower IRQ"
-espdma_memory_read(uint32_t addr) "DMA read addr 0x%08x"
-espdma_memory_write(uint32_t addr) "DMA write addr 0x%08x"
+espdma_memory_read(uint32_t addr, int len) "DMA read addr 0x%08x len %d"
+espdma_memory_write(uint32_t addr, int len) "DMA write addr 0x%08x len %d"
 sparc32_dma_mem_readl(uint64_t addr, uint32_t ret) "read dmareg 0x%"PRIx64": 0x%08x"
 sparc32_dma_mem_writel(uint64_t addr, uint32_t old, uint32_t val) "write dmareg 0x%"PRIx64": 0x%08x -> 0x%08x"
 sparc32_dma_enable_raise(void) "Raise DMA enable"
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv4 00/13] sun4m: sparc32_dma tidy-ups

[Artyom Tarasenko]

On Wed, Oct 25, 2017 at 5:59 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> This patchset aims to tidy-up the sparc32_dma code by improving the
> modelling of the espdma/ledma devices using both QOM and the memory
> API which didn't exist when the code was first written.
>
> The result is that it is now possible to remove both the iommu_opaque
> and is_ledma workarounds from the code, and the code for wiring up
> the espdma/ledma and respective devices is also a lot more readable.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

>
> v4:
> - Rebase onto git master
> - Update patch 9 to move lance QOM macros/SysBusPCNetState from sun4m.h to lance.h as suggested by Peter
> - Add Reviewed-by from Peter for patch 7
>
> v3:
> - Add missing sysbus.h include to esp.h in patch 7
>
> v2:
> - Make esp/lance devices children of espdma/ledma devices respectively
> - Add len parameter to ledma/espdma tracepoints
>
>
> Mark Cave-Ayland (13):
>   sparc32_dma: rename SPARC32_DMA type to SPARC32_DMA_DEVICE
>   sparc32_dma: split esp and le into separate DMA devices
>   sparc32_dma: move type declarations from sparc32_dma.c to
>     sparc32_dma.h
>   sun4m: move DMA device wiring from sparc32_dma_init() to
>     sun4m_hw_init()
>   sun4m_iommu: move TYPE_SUN4M_IOMMU declaration to sun4m.h
>   sparc32_dma: use object link instead of qdev property to pass IOMMU
>     reference
>   esp: move TYPE_ESP and SysBusESPState from esp.c to esp.h
>   sparc32_dma: make esp device child of espdma device
>   lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h
>   sparc32_dma: make lance device child of ledma device
>   sparc32_dma: introduce new SPARC32_DMA type container object
>   sparc32_dma: remove is_ledma hack and replace with memory region
>     alias
>   sparc32_dma: add len to esp/le DMA memory tracing
>
>  hw/dma/sparc32_dma.c           |  235 +++++++++++++++++++++++++++++-----------
>  hw/dma/sun4m_iommu.c           |   14 ---
>  hw/dma/trace-events            |    8 +-
>  hw/net/lance.c                 |   11 +-
>  hw/scsi/esp.c                  |   13 ---
>  hw/sparc/sun4m.c               |   82 ++++++--------
>  include/hw/net/lance.h         |   41 +++++++
>  include/hw/scsi/esp.h          |   14 +++
>  include/hw/sparc/sparc32_dma.h |   55 ++++++++++
>  include/hw/sparc/sun4m.h       |   16 +++
>  10 files changed, 336 insertions(+), 153 deletions(-)
>  create mode 100644 include/hw/net/lance.h
>
> --
> 1.7.10.4
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCHv4 00/13] sun4m: sparc32_dma tidy-ups

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PATCHv4 00/13] sun4m: sparc32_dma tidy-ups
Type: series
Message-id: 1508947167-5304-1-git-send-email-mark.cave-ayland@ilande.co.uk

=== 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

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
 * [new tag]               patchew/1508947167-5304-1-git-send-email-mark.cave-ayland@ilande.co.uk -> patchew/1508947167-5304-1-git-send-email-mark.cave-ayland@ilande.co.uk
Switched to a new branch 'test'
36eda9dd73 sparc32_dma: add len to esp/le DMA memory tracing
ac63079d23 sparc32_dma: remove is_ledma hack and replace with memory region alias
0e813fbc8a sparc32_dma: introduce new SPARC32_DMA type container object
2ac77fa8c0 sparc32_dma: make lance device child of ledma device
67c364cb3a lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h
63a69fb7ff sparc32_dma: make esp device child of espdma device
6aab399e1b esp: move TYPE_ESP and SysBusESPState from esp.c to esp.h
378f5fe6a5 sparc32_dma: use object link instead of qdev property to pass IOMMU reference
a927040955 sun4m_iommu: move TYPE_SUN4M_IOMMU declaration to sun4m.h
8367f3cdd0 sun4m: move DMA device wiring from sparc32_dma_init() to sun4m_hw_init()
1b4868953f sparc32_dma: move type declarations from sparc32_dma.c to sparc32_dma.h
018f7bd135 sparc32_dma: split esp and le into separate DMA devices
2ca43fd787 sparc32_dma: rename SPARC32_DMA type to SPARC32_DMA_DEVICE

=== OUTPUT BEGIN ===
Checking PATCH 1/13: sparc32_dma: rename SPARC32_DMA type to SPARC32_DMA_DEVICE...
Checking PATCH 2/13: sparc32_dma: split esp and le into separate DMA devices...
Checking PATCH 3/13: sparc32_dma: move type declarations from sparc32_dma.c to sparc32_dma.h...
Checking PATCH 4/13: sun4m: move DMA device wiring from sparc32_dma_init() to sun4m_hw_init()...
ERROR: spaces required around that '*' (ctx:WxV)
#52: FILE: hw/sparc/sun4m.c:824:
+    qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS];
              ^

total: 1 errors, 0 warnings, 66 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 5/13: sun4m_iommu: move TYPE_SUN4M_IOMMU declaration to sun4m.h...
Checking PATCH 6/13: sparc32_dma: use object link instead of qdev property to pass IOMMU reference...
Checking PATCH 7/13: esp: move TYPE_ESP and SysBusESPState from esp.c to esp.h...
Checking PATCH 8/13: sparc32_dma: make esp device child of espdma device...
Checking PATCH 9/13: lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h...
Checking PATCH 10/13: sparc32_dma: make lance device child of ledma device...
Checking PATCH 11/13: sparc32_dma: introduce new SPARC32_DMA type container object...
Checking PATCH 12/13: sparc32_dma: remove is_ledma hack and replace with memory region alias...
Checking PATCH 13/13: sparc32_dma: add len to esp/le DMA memory tracing...
=== 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

Re: [Qemu-devel] [PATCHv4 00/13] sun4m: sparc32_dma tidy-ups

[Philippe Mathieu-Daudé]

Hi Mark,

On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
> This patchset aims to tidy-up the sparc32_dma code by improving the
> modelling of the espdma/ledma devices using both QOM and the memory
> API which didn't exist when the code was first written.
> 
> The result is that it is now possible to remove both the iommu_opaque
> and is_ledma workarounds from the code, and the code for wiring up
> the espdma/ledma and respective devices is also a lot more readable.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

The whole series:

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

If you don't accept my comments (or don't have time) about keeping
"hw/sparc/sparc32_dma.h" generic and moving network/scsi parts in
"hw/sparc/sun4m.h" you can still add to your series:

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

Also while testing your series on a Debian image, I noted your series
results faster, I timed:

master: 104s
your series: 85s (>20% faster!)

Regards,

Phil.

> 
> v4:
> - Rebase onto git master
> - Update patch 9 to move lance QOM macros/SysBusPCNetState from sun4m.h to lance.h as suggested by Peter
> - Add Reviewed-by from Peter for patch 7
> 
> v3:
> - Add missing sysbus.h include to esp.h in patch 7
> 
> v2:
> - Make esp/lance devices children of espdma/ledma devices respectively
> - Add len parameter to ledma/espdma tracepoints
> 
> 
> Mark Cave-Ayland (13):
>   sparc32_dma: rename SPARC32_DMA type to SPARC32_DMA_DEVICE
>   sparc32_dma: split esp and le into separate DMA devices
>   sparc32_dma: move type declarations from sparc32_dma.c to
>     sparc32_dma.h
>   sun4m: move DMA device wiring from sparc32_dma_init() to
>     sun4m_hw_init()
>   sun4m_iommu: move TYPE_SUN4M_IOMMU declaration to sun4m.h
>   sparc32_dma: use object link instead of qdev property to pass IOMMU
>     reference
>   esp: move TYPE_ESP and SysBusESPState from esp.c to esp.h
>   sparc32_dma: make esp device child of espdma device
>   lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h
>   sparc32_dma: make lance device child of ledma device
>   sparc32_dma: introduce new SPARC32_DMA type container object
>   sparc32_dma: remove is_ledma hack and replace with memory region
>     alias
>   sparc32_dma: add len to esp/le DMA memory tracing
> 
>  hw/dma/sparc32_dma.c           |  235 +++++++++++++++++++++++++++++-----------
>  hw/dma/sun4m_iommu.c           |   14 ---
>  hw/dma/trace-events            |    8 +-
>  hw/net/lance.c                 |   11 +-
>  hw/scsi/esp.c                  |   13 ---
>  hw/sparc/sun4m.c               |   82 ++++++--------
>  include/hw/net/lance.h         |   41 +++++++
>  include/hw/scsi/esp.h          |   14 +++
>  include/hw/sparc/sparc32_dma.h |   55 ++++++++++
>  include/hw/sparc/sun4m.h       |   16 +++
>  10 files changed, 336 insertions(+), 153 deletions(-)
>  create mode 100644 include/hw/net/lance.h
> 

Re: [Qemu-devel] [PATCHv4 00/13] sun4m: sparc32_dma tidy-ups

[Mark Cave-Ayland]

On 27/10/17 17:42, Philippe Mathieu-Daudé wrote:

> Hi Mark,
> 
> On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
>> This patchset aims to tidy-up the sparc32_dma code by improving the
>> modelling of the espdma/ledma devices using both QOM and the memory
>> API which didn't exist when the code was first written.
>>
>> The result is that it is now possible to remove both the iommu_opaque
>> and is_ledma workarounds from the code, and the code for wiring up
>> the espdma/ledma and respective devices is also a lot more readable.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> The whole series:
> 
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> If you don't accept my comments (or don't have time) about keeping
> "hw/sparc/sparc32_dma.h" generic and moving network/scsi parts in
> "hw/sparc/sun4m.h" you can still add to your series:
> 
> Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thanks for the review, I've added your R-B tags to the individual
patches. Note that while potentially I could move the network/scsi parts
to hw/sparc/sun4m.h I feel that it's a slightly better match for the
SPARC32 DMA container device to remain in sparc32_dma.c. So for these
patches I've just added your A-B tag.

> Also while testing your series on a Debian image, I noted your series
> results faster, I timed:
> 
> master: 104s
> your series: 85s (>20% faster!)

Really? Is that for just this patchset or also with the v2 IOMMU
patchset applied on top? I can't immediately see how moving the logic
into sparc32_dma.c could make a difference here...


ATB,

Mark.

Re: [Qemu-devel] [PATCHv4 00/13] sun4m: sparc32_dma tidy-ups

[Philippe Mathieu-Daudé]

On Mon, Oct 30, 2017 at 4:00 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 27/10/17 17:42, Philippe Mathieu-Daudé wrote:
[...]
>> If you don't accept my comments (or don't have time) about keeping
>> "hw/sparc/sparc32_dma.h" generic and moving network/scsi parts in
>> "hw/sparc/sun4m.h" you can still add to your series:
>>
>> Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> Thanks for the review, I've added your R-B tags to the individual
> patches. Note that while potentially I could move the network/scsi parts
> to hw/sparc/sun4m.h I feel that it's a slightly better match for the
> SPARC32 DMA container device to remain in sparc32_dma.c. So for these
> patches I've just added your A-B tag.

OK.

>> Also while testing your series on a Debian image, I noted your series
>> results faster, I timed:
>>
>> master: 104s
>> your series: 85s (>20% faster!)
>
> Really? Is that for just this patchset or also with the v2 IOMMU
> patchset applied on top? I can't immediately see how moving the logic
> into sparc32_dma.c could make a difference here...

Yes, I was trying with both series applied, so this comment belong to
the other series (IOMMU).

Regards,

Phil.