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

Philippe Mathieu-Daudé posted 5 patches 7 years, 10 months ago
[Qemu-devel] [PATCH for-2.12 1/5] hw/dma/i82374: Avoid double creation of the 82374 controller
Posted by Philippe Mathieu-Daudé 7 years, 10 months ago
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


Re: [Qemu-devel] [PATCH for-2.12 1/5] hw/dma/i82374: Avoid double creation of the 82374 controller
Posted by Thomas Huth 7 years, 10 months ago
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>