qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCHv2] dma/i82374: avoid double creation of i82374 device
@ 2017-09-29 14:05 Eduardo Otubo
  2017-09-29 14:11 ` no-reply
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Eduardo Otubo @ 2017-09-29 14:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-trivial, Michael Tokarev, ehabkost, Alexander Graf,
	Markus Armbruster, pbonzini

v2:
    Removed user_creatable=false and replaced by error handling using
    Error **errp and error_propagate();

QEMU fails when used with the following command line:

    ./ppc64-softmmu/qemu-system-ppc64 -S -machine 40p,accel=tcg -device i82374
    qemu-system-ppc64: hw/isa/isa-bus.c:110: isa_bus_dma: Assertion `!bus->dma[0] && !bus->dma[1]' failed.
    Aborted (core dumped)

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. One
way to avoid this problem is to set user_creatable=false.

A possible fix in a near future would be making
isa_bus_dma()/DMA_init()/i82374_realize() return an error instead of asserting
as well.

Signed-off-by: Eduardo Otubo <otubo@redhat.com>
---
 hw/dma/i82374.c      | 7 +++++--
 hw/dma/i8257.c       | 7 +++++--
 hw/isa/isa-bus.c     | 9 +++++++--
 include/hw/isa/isa.h | 4 ++--
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
index 6c0f975df0..385a42cb57 100644
--- a/hw/dma/i82374.c
+++ b/hw/dma/i82374.c
@@ -24,6 +24,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/isa/isa.h"
+#include "qapi/error.h"
 
 #define TYPE_I82374 "i82374"
 #define I82374(obj) OBJECT_CHECK(I82374State, (obj), TYPE_I82374)
@@ -117,14 +118,16 @@ static const MemoryRegionPortio i82374_portio_list[] = {
 static void i82374_realize(DeviceState *dev, Error **errp)
 {
     I82374State *s = I82374(dev);
+    Error *local_err = NULL;
 
     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);
 
-    DMA_init(isa_bus_from_device(ISA_DEVICE(dev)), 1);
+    DMA_init(isa_bus_from_device(ISA_DEVICE(dev)), 1, &local_err);
     memset(s->commands, 0, sizeof(s->commands));
+    error_propagate(errp, local_err);
 }
 
 static Property i82374_properties[] = {
@@ -135,7 +138,7 @@ static Property i82374_properties[] = {
 static void i82374_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    
+
     dc->realize = i82374_realize;
     dc->vmsd = &vmstate_i82374;
     dc->props = i82374_properties;
diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
index bd23e893bf..94b1ae7533 100644
--- a/hw/dma/i8257.c
+++ b/hw/dma/i8257.c
@@ -27,6 +27,7 @@
 #include "hw/isa/i8257.h"
 #include "qemu/main-loop.h"
 #include "trace.h"
+#include "qapi/error.h"
 
 #define I8257(obj) \
     OBJECT_CHECK(I8257State, (obj), TYPE_I8257)
@@ -622,10 +623,11 @@ static void i8257_register_types(void)
 
 type_init(i8257_register_types)
 
-void DMA_init(ISABus *bus, int high_page_enable)
+void DMA_init(ISABus *bus, int high_page_enable, Error **errp)
 {
     ISADevice *isa1, *isa2;
     DeviceState *d;
+    Error *local_err = NULL;
 
     isa1 = isa_create(bus, TYPE_I8257);
     d = DEVICE(isa1);
@@ -643,5 +645,6 @@ void DMA_init(ISABus *bus, int high_page_enable)
     qdev_prop_set_int32(d, "dshift", 1);
     qdev_init_nofail(d);
 
-    isa_bus_dma(bus, ISADMA(isa1), ISADMA(isa2));
+    isa_bus_dma(bus, ISADMA(isa1), ISADMA(isa2), &local_err);
+    error_propagate(errp, local_err);
 }
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 348e0eab9d..aa7d8de856 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -104,12 +104,17 @@ void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, int isairq)
     qdev_connect_gpio_out(DEVICE(isadev), gpioirq, irq);
 }
 
-void isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16)
+void isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16, Error **errp)
 {
+    Error *local_err = NULL;
     assert(bus && dma8 && dma16);
-    assert(!bus->dma[0] && !bus->dma[1]);
+    if (bus->dma[0] && bus->dma[1]) {
+        error_setg(errp, "isa dma device i82374 already created");
+        return;
+    }
     bus->dma[0] = dma8;
     bus->dma[1] = dma16;
+    error_propagate(errp, local_err);
 }
 
 IsaDma *isa_get_dma(ISABus *bus, int nchan)
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 95593408ef..5cf4ceaf39 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -103,7 +103,7 @@ void isa_bus_irqs(ISABus *bus, qemu_irq *irqs);
 qemu_irq isa_get_irq(ISADevice *dev, int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq);
 void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, int isairq);
-void isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16);
+void isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16, Error **errp);
 IsaDma *isa_get_dma(ISABus *bus, int nchan);
 MemoryRegion *isa_address_space(ISADevice *dev);
 MemoryRegion *isa_address_space_io(ISADevice *dev);
@@ -152,5 +152,5 @@ static inline ISABus *isa_bus_from_device(ISADevice *d)
 }
 
 /* i8257.c */
-void DMA_init(ISABus *bus, int high_page_enable);
+void DMA_init(ISABus *bus, int high_page_enable, Error **errp);
 #endif
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-10-04  4:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-29 14:05 [Qemu-devel] [PATCHv2] dma/i82374: avoid double creation of i82374 device Eduardo Otubo
2017-09-29 14:11 ` no-reply
2017-09-29 14:12 ` no-reply
2017-09-29 19:31 ` Eduardo Habkost
2017-10-02 12:50   ` Paolo Bonzini
2017-10-03 13:37     ` Eduardo Habkost
2017-10-04  4:50       ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).