[Qemu-devel] [PATCH 0/5] dma/i82374: avoid double creation of i82374 device

[Qemu-devel] [PATCH 0/5] dma/i82374: avoid double creation of i82374 device

[Philippe Mathieu-Daudé]

Hi,

This series intend to fix: https://bugs.launchpad.net/qemu/+bug/1721224

Patch #1 is the fix for 2.12, following patches are just refactors for 2.13.

The 8257 only has 4 DMA channels. To have 8 channels, the IBM PC/AT
implementation uses 2x 8257, the second cascaded onto the first.
The i8257_dma_init() name is misleading since this function creates two
8257 to register a total of 8 channels on the ISA bus.

The refactor is to enforce that 2 controllers are used (cascaded) - no
logical change.

Regards,

Phil.

Philippe Mathieu-Daudé (5):
  hw/dma/i82374: Avoid double creation of the 82374 controller
  hw/dma/i8257: Define I8257_CHANNEL_COUNT
  hw/dma/i8257: Split i8257_dma_init() by master/slave
  hw/dma/i8257: Rename i8257_dma_init() -> i8257_dma_init_cascaded()
  hw/dma/i8257: Rename i8257_dma_init(false) -> i8257_dma_init_pc_at()

 include/hw/dma/i8257.h  | 23 +++++++++++++++++++++--
 hw/dma/i82374.c         |  9 ++++++++-
 hw/dma/i8257.c          | 38 ++++++++++++++++++++++++++++----------
 hw/i386/pc.c            |  2 +-
 hw/mips/mips_fulong2e.c |  2 +-
 hw/mips/mips_jazz.c     |  2 +-
 hw/mips/mips_malta.c    |  2 +-
 7 files changed, 61 insertions(+), 17 deletions(-)

-- 
2.16.3

[Qemu-devel] [PATCH for-2.12 1/5] hw/dma/i82374: Avoid double creation of the 82374 controller

[Philippe Mathieu-Daudé]

QEMU fails when used with the following command line:

    ./ppc64-softmmu/qemu-system-ppc64 -S -machine 40p -device i82374
    qemu-system-ppc64: hw/isa/isa-bus.c:110: isa_bus_dma: Assertion `!bus->dma[0] && !bus->dma[1]' failed.

The 40p machine type already creates the device i82374. If specified in the
command line, it will try to create it again, hence generating the error. The
function isa_bus_dma() isn't supposed to be called twice for the same bus.
Check the bus doesn't already have a DMA controller registered before creating
the device.

Fixes: https://bugs.launchpad.net/qemu/+bug/1721224
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/dma/i82374.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
index 83c87d92e0..892f655a7e 100644
--- a/hw/dma/i82374.c
+++ b/hw/dma/i82374.c
@@ -23,6 +23,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "hw/isa/isa.h"
 #include "hw/dma/i8257.h"
 
@@ -118,13 +119,19 @@ static const MemoryRegionPortio i82374_portio_list[] = {
 static void i82374_realize(DeviceState *dev, Error **errp)
 {
     I82374State *s = I82374(dev);
+    ISABus *isa_bus = isa_bus_from_device(ISA_DEVICE(dev));
+
+    if (isa_get_dma(isa_bus, 0)) {
+        error_setg(errp, "DMA already initialized on ISA bus");
+        return;
+    }
+    i8257_dma_init(isa_bus, true);
 
     portio_list_init(&s->port_list, OBJECT(s), i82374_portio_list, s,
                      "i82374");
     portio_list_add(&s->port_list, isa_address_space_io(&s->parent_obj),
                     s->iobase);
 
-    i8257_dma_init(isa_bus_from_device(ISA_DEVICE(dev)), true);
     memset(s->commands, 0, sizeof(s->commands));
 }
 
-- 
2.16.3

[Qemu-devel] [PATCH for-2.13 2/5] hw/dma/i8257: Define I8257_CHANNEL_COUNT

[Philippe Mathieu-Daudé]

The 8257 has 4 DMA channels, reflect that to denote than when 8 channels
are used, this is not a single 8257 (but probably two cascaded).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/dma/i8257.h | 4 +++-
 hw/dma/i8257.c         | 6 +++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/hw/dma/i8257.h b/include/hw/dma/i8257.h
index 2cab50bb6c..3053f18797 100644
--- a/include/hw/dma/i8257.h
+++ b/include/hw/dma/i8257.h
@@ -7,6 +7,8 @@
 
 #define TYPE_I8257 "i8257"
 
+#define I8257_CHANNEL_COUNT 4
+
 typedef struct I8257Regs {
     int now[2];
     uint16_t base[2];
@@ -33,7 +35,7 @@ typedef struct I8257State {
     uint8_t command;
     uint8_t mask;
     uint8_t flip_flop;
-    I8257Regs regs[4];
+    I8257Regs regs[I8257_CHANNEL_COUNT];
     MemoryRegion channel_io;
     MemoryRegion cont_io;
 
diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
index 52675e97c9..df030f934c 100644
--- a/hw/dma/i8257.c
+++ b/hw/dma/i8257.c
@@ -361,7 +361,7 @@ static void i8257_dma_run(void *opaque)
         d->running = 1;
     }
 
-    for (ichan = 0; ichan < 4; ichan++) {
+    for (ichan = 0; ichan < I8257_CHANNEL_COUNT; ichan++) {
         int mask;
 
         mask = 1 << ichan;
@@ -536,8 +536,8 @@ static const VMStateDescription vmstate_i8257 = {
         VMSTATE_UINT8(mask, I8257State),
         VMSTATE_UINT8(flip_flop, I8257State),
         VMSTATE_INT32(dshift, I8257State),
-        VMSTATE_STRUCT_ARRAY(regs, I8257State, 4, 1, vmstate_i8257_regs,
-                             I8257Regs),
+        VMSTATE_STRUCT_ARRAY(regs, I8257State, I8257_CHANNEL_COUNT, 1,
+                             vmstate_i8257_regs, I8257Regs),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
2.16.3

[Qemu-devel] [PATCH for-2.13 3/5] hw/dma/i8257: Split i8257_dma_init() by master/slave

[Philippe Mathieu-Daudé]

This emphasises than two controller are created (in master/slave configuration).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/dma/i8257.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
index df030f934c..72f8893b9e 100644
--- a/hw/dma/i8257.c
+++ b/hw/dma/i8257.c
@@ -622,26 +622,44 @@ static void i8257_register_types(void)
 
 type_init(i8257_register_types)
 
-void i8257_dma_init(ISABus *bus, bool high_page_enable)
+static ISADevice *i8257_dma_init_master(ISABus *bus, bool high_page_enable)
 {
-    ISADevice *isa1, *isa2;
+    ISADevice *isa;
     DeviceState *d;
 
-    isa1 = isa_create(bus, TYPE_I8257);
-    d = DEVICE(isa1);
+    isa = isa_create(bus, TYPE_I8257);
+    d = DEVICE(isa);
     qdev_prop_set_int32(d, "base", 0x00);
     qdev_prop_set_int32(d, "page-base", 0x80);
     qdev_prop_set_int32(d, "pageh-base", high_page_enable ? 0x480 : -1);
     qdev_prop_set_int32(d, "dshift", 0);
     qdev_init_nofail(d);
 
-    isa2 = isa_create(bus, TYPE_I8257);
-    d = DEVICE(isa2);
+    return isa;
+}
+
+static ISADevice *i8257_dma_init_slave(ISABus *bus, bool high_page_enable)
+{
+    ISADevice *isa;
+    DeviceState *d;
+
+    isa = isa_create(bus, TYPE_I8257);
+    d = DEVICE(isa);
     qdev_prop_set_int32(d, "base", 0xc0);
     qdev_prop_set_int32(d, "page-base", 0x88);
     qdev_prop_set_int32(d, "pageh-base", high_page_enable ? 0x488 : -1);
     qdev_prop_set_int32(d, "dshift", 1);
     qdev_init_nofail(d);
 
-    isa_bus_dma(bus, ISADMA(isa1), ISADMA(isa2));
+    return isa;
+}
+
+void i8257_dma_init(ISABus *bus, bool high_page_enable)
+{
+    ISADevice *master, *slave;
+
+    master = i8257_dma_init_master(bus, high_page_enable);
+    slave = i8257_dma_init_slave(bus, high_page_enable);
+
+    isa_bus_dma(bus, ISADMA(master), ISADMA(slave));
 }
-- 
2.16.3

[Qemu-devel] [PATCH for-2.13 /5] hw/dma/i8257: Rename i8257_dma_init() -> i8257_dma_init_cascaded()

[Philippe Mathieu-Daudé]

To keep the patch diff simple, an inline function is used (then removed
in the next commit).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/dma/i8257.h | 6 +++++-
 hw/dma/i82374.c        | 2 +-
 hw/dma/i8257.c         | 2 +-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/hw/dma/i8257.h b/include/hw/dma/i8257.h
index 3053f18797..986319e4e3 100644
--- a/include/hw/dma/i8257.h
+++ b/include/hw/dma/i8257.h
@@ -46,6 +46,10 @@ typedef struct I8257State {
     PortioList portio_pageh;
 } I8257State;
 
-void i8257_dma_init(ISABus *bus, bool high_page_enable);
+void i8257_dma_init_cascaded(ISABus *bus, bool high_page_enable);
+static inline void i8257_dma_init(ISABus *bus, bool high_page_enable)
+{
+    i8257_dma_init_cascaded(bus, high_page_enable);
+}
 
 #endif
diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
index 892f655a7e..b95edd7b98 100644
--- a/hw/dma/i82374.c
+++ b/hw/dma/i82374.c
@@ -125,7 +125,7 @@ static void i82374_realize(DeviceState *dev, Error **errp)
         error_setg(errp, "DMA already initialized on ISA bus");
         return;
     }
-    i8257_dma_init(isa_bus, true);
+    i8257_dma_init_cascaded(isa_bus, true);
 
     portio_list_init(&s->port_list, OBJECT(s), i82374_portio_list, s,
                      "i82374");
diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
index 72f8893b9e..c930c4c531 100644
--- a/hw/dma/i8257.c
+++ b/hw/dma/i8257.c
@@ -654,7 +654,7 @@ static ISADevice *i8257_dma_init_slave(ISABus *bus, bool high_page_enable)
     return isa;
 }
 
-void i8257_dma_init(ISABus *bus, bool high_page_enable)
+void i8257_dma_init_cascaded(ISABus *bus, bool high_page_enable)
 {
     ISADevice *master, *slave;
 
-- 
2.16.3

[Qemu-devel] [PATCH for-2.13 5/5] hw/dma/i8257: Rename i8257_dma_init(false) -> i8257_dma_init_pc_at()

[Philippe Mathieu-Daudé]

Reflect that the PC/AT implementation is used.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/dma/i8257.h  | 17 +++++++++++++++--
 hw/i386/pc.c            |  2 +-
 hw/mips/mips_fulong2e.c |  2 +-
 hw/mips/mips_jazz.c     |  2 +-
 hw/mips/mips_malta.c    |  2 +-
 5 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/include/hw/dma/i8257.h b/include/hw/dma/i8257.h
index 986319e4e3..15db8b4d29 100644
--- a/include/hw/dma/i8257.h
+++ b/include/hw/dma/i8257.h
@@ -47,9 +47,22 @@ typedef struct I8257State {
 } I8257State;
 
 void i8257_dma_init_cascaded(ISABus *bus, bool high_page_enable);
-static inline void i8257_dma_init(ISABus *bus, bool high_page_enable)
+
+/**
+ * i8257_dma_init_pc_at: Install 8 DMA channels on the ISA bus.
+ *
+ * This is the PC/AT DMA implementation:
+ *
+ * Two i8257 controllers are created.
+ * The primary controller register channels [0..3] on the bus,
+ * the secondary controller (slave) is cascaded on the primary (master),
+ * registering channels [4..7].
+ *
+ * @bus: the #ISABus against which these are created.
+ */
+static inline void i8257_dma_init_pc_at(ISABus *bus)
 {
-    i8257_dma_init_cascaded(bus, high_page_enable);
+    i8257_dma_init_cascaded(bus, false);
 }
 
 #endif
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d36bac8c89..baba079d7f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1624,7 +1624,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
         pcspk_init(isa_bus, pit);
     }
 
-    i8257_dma_init(isa_bus, 0);
+    i8257_dma_init_pc_at(isa_bus);
 
     /* Super I/O */
     pc_superio_init(isa_bus, create_fdctrl, no_vmport);
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 02fb2fdcc4..8168e6cf16 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -243,7 +243,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
     isa_bus_irqs(isa_bus, i8259);
     /* init other devices */
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
-    i8257_dma_init(isa_bus, 0);
+    i8257_dma_init_pc_at(isa_bus);
     /* Super I/O */
     isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
 
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 7223085547..2577e8383d 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -222,7 +222,7 @@ static void mips_jazz_init(MachineState *machine,
     /* ISA devices */
     i8259 = i8259_init(isa_bus, env->irq[4]);
     isa_bus_irqs(isa_bus, i8259);
-    i8257_dma_init(isa_bus, 0);
+    i8257_dma_init_pc_at(isa_bus);
     pit = i8254_pit_init(isa_bus, 0x40, 0, NULL);
     pcspk_init(isa_bus, pit);
 
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index f6513a4fd5..9d0409bc36 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1198,7 +1198,7 @@ void mips_malta_init(MachineState *machine)
     smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100,
                           isa_get_irq(NULL, 9), NULL, 0, NULL);
     pit = i8254_pit_init(isa_bus, 0x40, 0, NULL);
-    i8257_dma_init(isa_bus, 0);
+    i8257_dma_init_pc_at(isa_bus);
     mc146818_rtc_init(isa_bus, 2000, NULL);
 
     /* generate SPD EEPROM data */
-- 
2.16.3

Re: [Qemu-devel] [PATCH for-2.13 5/5] hw/dma/i8257: Rename i8257_dma_init(false) -> i8257_dma_init_pc_at()

[Marcel Apfelbaum]

On 26/03/2018 18:34, Philippe Mathieu-Daudé wrote:
> Reflect that the PC/AT implementation is used.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/dma/i8257.h  | 17 +++++++++++++++--
>  hw/i386/pc.c            |  2 +-
>  hw/mips/mips_fulong2e.c |  2 +-
>  hw/mips/mips_jazz.c     |  2 +-
>  hw/mips/mips_malta.c    |  2 +-
>  5 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/dma/i8257.h b/include/hw/dma/i8257.h
> index 986319e4e3..15db8b4d29 100644
> --- a/include/hw/dma/i8257.h
> +++ b/include/hw/dma/i8257.h
> @@ -47,9 +47,22 @@ typedef struct I8257State {
>  } I8257State;
>  
>  void i8257_dma_init_cascaded(ISABus *bus, bool high_page_enable);
> -static inline void i8257_dma_init(ISABus *bus, bool high_page_enable)
> +
> +/**
> + * i8257_dma_init_pc_at: Install 8 DMA channels on the ISA bus.
> + *
> + * This is the PC/AT DMA implementation:
> + *
> + * Two i8257 controllers are created.
> + * The primary controller register channels [0..3] on the bus,
> + * the secondary controller (slave) is cascaded on the primary (master),
> + * registering channels [4..7].
> + *
> + * @bus: the #ISABus against which these are created.
> + */
> +static inline void i8257_dma_init_pc_at(ISABus *bus)
>  {
> -    i8257_dma_init_cascaded(bus, high_page_enable);
> +    i8257_dma_init_cascaded(bus, false);
>  }
>  
>  #endif
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index d36bac8c89..baba079d7f 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1624,7 +1624,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
>          pcspk_init(isa_bus, pit);
>      }
>  
> -    i8257_dma_init(isa_bus, 0);
> +    i8257_dma_init_pc_at(isa_bus);
>  
>      /* Super I/O */
>      pc_superio_init(isa_bus, create_fdctrl, no_vmport);
> diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
> index 02fb2fdcc4..8168e6cf16 100644
> --- a/hw/mips/mips_fulong2e.c
> +++ b/hw/mips/mips_fulong2e.c
> @@ -243,7 +243,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
>      isa_bus_irqs(isa_bus, i8259);
>      /* init other devices */
>      i8254_pit_init(isa_bus, 0x40, 0, NULL);
> -    i8257_dma_init(isa_bus, 0);
> +    i8257_dma_init_pc_at(isa_bus);
>      /* Super I/O */
>      isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
>  
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index 7223085547..2577e8383d 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -222,7 +222,7 @@ static void mips_jazz_init(MachineState *machine,
>      /* ISA devices */
>      i8259 = i8259_init(isa_bus, env->irq[4]);
>      isa_bus_irqs(isa_bus, i8259);
> -    i8257_dma_init(isa_bus, 0);
> +    i8257_dma_init_pc_at(isa_bus);
>      pit = i8254_pit_init(isa_bus, 0x40, 0, NULL);
>      pcspk_init(isa_bus, pit);
>  
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index f6513a4fd5..9d0409bc36 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1198,7 +1198,7 @@ void mips_malta_init(MachineState *machine)
>      smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100,
>                            isa_get_irq(NULL, 9), NULL, 0, NULL);
>      pit = i8254_pit_init(isa_bus, 0x40, 0, NULL);
> -    i8257_dma_init(isa_bus, 0);
> +    i8257_dma_init_pc_at(isa_bus);
>      mc146818_rtc_init(isa_bus, 2000, NULL);
>  
>      /* generate SPD EEPROM data */
> 

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel

[Qemu-devel] [PATCH for-2.13 4/5] hw/dma/i8257: Rename i8257_dma_init() -> i8257_dma_init_cascaded()

[Philippe Mathieu-Daudé]

To keep the patch diff simple, an inline function is used (then removed
in the next commit).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/dma/i8257.h | 6 +++++-
 hw/dma/i82374.c        | 2 +-
 hw/dma/i8257.c         | 2 +-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/hw/dma/i8257.h b/include/hw/dma/i8257.h
index 3053f18797..986319e4e3 100644
--- a/include/hw/dma/i8257.h
+++ b/include/hw/dma/i8257.h
@@ -46,6 +46,10 @@ typedef struct I8257State {
     PortioList portio_pageh;
 } I8257State;
 
-void i8257_dma_init(ISABus *bus, bool high_page_enable);
+void i8257_dma_init_cascaded(ISABus *bus, bool high_page_enable);
+static inline void i8257_dma_init(ISABus *bus, bool high_page_enable)
+{
+    i8257_dma_init_cascaded(bus, high_page_enable);
+}
 
 #endif
diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
index 892f655a7e..b95edd7b98 100644
--- a/hw/dma/i82374.c
+++ b/hw/dma/i82374.c
@@ -125,7 +125,7 @@ static void i82374_realize(DeviceState *dev, Error **errp)
         error_setg(errp, "DMA already initialized on ISA bus");
         return;
     }
-    i8257_dma_init(isa_bus, true);
+    i8257_dma_init_cascaded(isa_bus, true);
 
     portio_list_init(&s->port_list, OBJECT(s), i82374_portio_list, s,
                      "i82374");
diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
index 72f8893b9e..c930c4c531 100644
--- a/hw/dma/i8257.c
+++ b/hw/dma/i8257.c
@@ -654,7 +654,7 @@ static ISADevice *i8257_dma_init_slave(ISABus *bus, bool high_page_enable)
     return isa;
 }
 
-void i8257_dma_init(ISABus *bus, bool high_page_enable)
+void i8257_dma_init_cascaded(ISABus *bus, bool high_page_enable)
 {
     ISADevice *master, *slave;
 
-- 
2.16.3

Re: [Qemu-devel] [PATCH 0/5] dma/i82374: avoid double creation of i82374 device

[Eduardo Otubo]

On 26/03/2018 - 12:34:36, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> This series intend to fix: https://bugs.launchpad.net/qemu/+bug/1721224
> 
> Patch #1 is the fix for 2.12, following patches are just refactors for 2.13.
> 
> The 8257 only has 4 DMA channels. To have 8 channels, the IBM PC/AT
> implementation uses 2x 8257, the second cascaded onto the first.
> The i8257_dma_init() name is misleading since this function creates two
> 8257 to register a total of 8 channels on the ISA bus.
> 
> The refactor is to enforce that 2 controllers are used (cascaded) - no
> logical change.
> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (5):
>   hw/dma/i82374: Avoid double creation of the 82374 controller
>   hw/dma/i8257: Define I8257_CHANNEL_COUNT
>   hw/dma/i8257: Split i8257_dma_init() by master/slave
>   hw/dma/i8257: Rename i8257_dma_init() -> i8257_dma_init_cascaded()
>   hw/dma/i8257: Rename i8257_dma_init(false) -> i8257_dma_init_pc_at()
> 
>  include/hw/dma/i8257.h  | 23 +++++++++++++++++++++--
>  hw/dma/i82374.c         |  9 ++++++++-
>  hw/dma/i8257.c          | 38 ++++++++++++++++++++++++++++----------
>  hw/i386/pc.c            |  2 +-
>  hw/mips/mips_fulong2e.c |  2 +-
>  hw/mips/mips_jazz.c     |  2 +-
>  hw/mips/mips_malta.c    |  2 +-
>  7 files changed, 61 insertions(+), 17 deletions(-)
> 
> -- 
> 2.16.3
> 
> 
Reviewed-by: Eduardo Otubo <otubo@redhat.com>

-- 
Eduardo Otubo

Re: [Qemu-devel] [PATCH for-2.12 1/5] hw/dma/i82374: Avoid double creation of the 82374 controller

[Thomas Huth]

On 26.03.2018 17:34, Philippe Mathieu-Daudé wrote:
> QEMU fails when used with the following command line:
> 
>     ./ppc64-softmmu/qemu-system-ppc64 -S -machine 40p -device i82374
>     qemu-system-ppc64: hw/isa/isa-bus.c:110: isa_bus_dma: Assertion `!bus->dma[0] && !bus->dma[1]' failed.
> 
> The 40p machine type already creates the device i82374. If specified in the
> command line, it will try to create it again, hence generating the error. The
> function isa_bus_dma() isn't supposed to be called twice for the same bus.
> Check the bus doesn't already have a DMA controller registered before creating
> the device.
> 
> Fixes: https://bugs.launchpad.net/qemu/+bug/1721224
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/dma/i82374.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
> index 83c87d92e0..892f655a7e 100644
> --- a/hw/dma/i82374.c
> +++ b/hw/dma/i82374.c
> @@ -23,6 +23,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qapi/error.h"
>  #include "hw/isa/isa.h"
>  #include "hw/dma/i8257.h"
>  
> @@ -118,13 +119,19 @@ static const MemoryRegionPortio i82374_portio_list[] = {
>  static void i82374_realize(DeviceState *dev, Error **errp)
>  {
>      I82374State *s = I82374(dev);
> +    ISABus *isa_bus = isa_bus_from_device(ISA_DEVICE(dev));
> +
> +    if (isa_get_dma(isa_bus, 0)) {
> +        error_setg(errp, "DMA already initialized on ISA bus");
> +        return;
> +    }
> +    i8257_dma_init(isa_bus, true);
>  
>      portio_list_init(&s->port_list, OBJECT(s), i82374_portio_list, s,
>                       "i82374");
>      portio_list_add(&s->port_list, isa_address_space_io(&s->parent_obj),
>                      s->iobase);
>  
> -    i8257_dma_init(isa_bus_from_device(ISA_DEVICE(dev)), true);
>      memset(s->commands, 0, sizeof(s->commands));
>  }
>  
> 

Thanks, looks like a good way to fix this issue for QEMU 2.12 indeed.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>