From nobody Thu May 2 13:21:25 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (208.118.235.17 [208.118.235.17]) by mx.zohomail.com with SMTPS id 1511531293536167.73393423014022; Fri, 24 Nov 2017 05:48:13 -0800 (PST) Received: from localhost ([::1]:49369 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eIEKh-0004Sf-Mw for importer@patchew.org; Fri, 24 Nov 2017 08:47:55 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50233) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eIEJq-0003jQ-LT for qemu-devel@nongnu.org; Fri, 24 Nov 2017 08:47:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eIEJk-00050y-JS for qemu-devel@nongnu.org; Fri, 24 Nov 2017 08:47:02 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33320) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eIEJk-00050D-AU for qemu-devel@nongnu.org; Fri, 24 Nov 2017 08:46:56 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A05084E33A; Fri, 24 Nov 2017 13:46:54 +0000 (UTC) Received: from vader.redhat.com (ovpn-116-58.ams2.redhat.com [10.36.116.58]) by smtp.corp.redhat.com (Postfix) with ESMTP id F388260A98; Fri, 24 Nov 2017 13:46:50 +0000 (UTC) From: Eduardo Otubo To: qemu-devel@nongnu.org Date: Fri, 24 Nov 2017 14:46:44 +0100 Message-Id: <20171124134644.490-1-otubo@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Fri, 24 Nov 2017 13:46:54 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCHv3] dma/i82374: avoid double creation of i82374 device X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: armbru@redhat.com, pbonzini@redhat.com, mjt@tls.msk.ru, agraf@suse.de, ehabkost@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" v3: * Removed all unecessary local_err * Change return of isa_bus_dma() and DMA_init() from void to int8_t, returning -EBUSY on error and 0 on success * Added qdev_cleanup_nofail() in case isa_bus_dma() returns error. The cleanup looks safe, but please review if I didn't miss any detail v2: * Removed user_creatable=3Dfalse 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=3Dtcg -device i= 82374 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 created the device i82374. If specified in the command line, it will try to create it again, hence generating the error. T= he 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=3Dfalse. A possible fix in a near future would be making isa_bus_dma()/DMA_init()/i82374_realize() return an error instead of assert= ing as well. Signed-off-by: Eduardo Otubo --- hw/core/qdev.c | 16 ++++++++++++++++ hw/dma/i82374.c | 13 +++++++------ hw/dma/i8257.c | 38 ++++++++++++++++++++++---------------- hw/isa/isa-bus.c | 8 ++++++-- include/hw/isa/isa.h | 4 ++-- include/hw/qdev-core.h | 1 + 6 files changed, 54 insertions(+), 26 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 606ab53c42..0ef03ee461 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -344,6 +344,22 @@ void qdev_init_nofail(DeviceState *dev) object_unref(OBJECT(dev)); } =20 +void qdev_cleanup_nofail(DeviceState *dev) +{ + Error *err =3D NULL; + + assert(dev->realized); + + object_ref(OBJECT(dev)); + object_property_set_bool(OBJECT(dev), false, "realized", &err); + if (err) { + error_reportf_err(err, "Clean up of device %s failed: ", + object_get_typename(OBJECT(dev))); + exit(1); + } + object_unref(OBJECT(dev)); +} + void qdev_machine_creation_done(void) { /* diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c index 6c0f975df0..9b792890b2 100644 --- a/hw/dma/i82374.c +++ b/hw/dma/i82374.c @@ -24,6 +24,7 @@ =20 #include "qemu/osdep.h" #include "hw/isa/isa.h" +#include "qapi/error.h" =20 #define TYPE_I82374 "i82374" #define I82374(obj) OBJECT_CHECK(I82374State, (obj), TYPE_I82374) @@ -118,13 +119,13 @@ static void i82374_realize(DeviceState *dev, Error **= errp) { I82374State *s =3D I82374(dev); =20 - 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), + if (DMA_init(isa_bus_from_device(ISA_DEVICE(dev)), 1, errp) =3D=3D 0) { + 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); - memset(s->commands, 0, sizeof(s->commands)); + memset(s->commands, 0, sizeof(s->commands)); + } } =20 static Property i82374_properties[] =3D { diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c index bd23e893bf..7488e3dd12 100644 --- a/hw/dma/i8257.c +++ b/hw/dma/i8257.c @@ -622,26 +622,32 @@ static void i8257_register_types(void) =20 type_init(i8257_register_types) =20 -void DMA_init(ISABus *bus, int high_page_enable) +int8_t DMA_init(ISABus *bus, int high_page_enable, Error **errp) { ISADevice *isa1, *isa2; - DeviceState *d; + DeviceState *d1, *d2; =20 isa1 =3D isa_create(bus, TYPE_I8257); - d =3D DEVICE(isa1); - 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); + d1 =3D DEVICE(isa1); + qdev_prop_set_int32(d1, "base", 0x00); + qdev_prop_set_int32(d1, "page-base", 0x80); + qdev_prop_set_int32(d1, "pageh-base", high_page_enable ? 0x480 : -1); + qdev_prop_set_int32(d1, "dshift", 0); + qdev_init_nofail(d1); =20 isa2 =3D isa_create(bus, TYPE_I8257); - d =3D DEVICE(isa2); - 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)); + d2 =3D DEVICE(isa2); + qdev_prop_set_int32(d2, "base", 0xc0); + qdev_prop_set_int32(d2, "page-base", 0x88); + qdev_prop_set_int32(d2, "pageh-base", high_page_enable ? 0x488 : -1); + qdev_prop_set_int32(d2, "dshift", 1); + qdev_init_nofail(d2); + + if (isa_bus_dma(bus, ISADMA(isa1), ISADMA(isa2), errp) < 0) { + qdev_cleanup_nofail(d1); + qdev_cleanup_nofail(d2); + return -EBUSY; + } + + return 0; } diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c index 348e0eab9d..4781dddaf9 100644 --- a/hw/isa/isa-bus.c +++ b/hw/isa/isa-bus.c @@ -104,12 +104,16 @@ void isa_connect_gpio_out(ISADevice *isadev, int gpio= irq, int isairq) qdev_connect_gpio_out(DEVICE(isadev), gpioirq, irq); } =20 -void isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16) +int8_t isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16, Error **errp) { assert(bus && dma8 && dma16); - assert(!bus->dma[0] && !bus->dma[1]); + if (bus->dma[0] || bus->dma[1]) { + error_setg(errp, "DMA already initialized on ISA bus"); + return -EBUSY; + } bus->dma[0] =3D dma8; bus->dma[1] =3D dma16; + return 0; } =20 IsaDma *isa_get_dma(ISABus *bus, int nchan) diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h index 95593408ef..2c7f260deb 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); +int8_t 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) } =20 /* i8257.c */ -void DMA_init(ISABus *bus, int high_page_enable); +int8_t DMA_init(ISABus *bus, int high_page_enable, Error **errp); #endif diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 089146197f..77d3b43427 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -283,6 +283,7 @@ typedef struct GlobalProperty { DeviceState *qdev_create(BusState *bus, const char *name); DeviceState *qdev_try_create(BusState *bus, const char *name); void qdev_init_nofail(DeviceState *dev); +void qdev_cleanup_nofail(DeviceState *dev); void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, int required_for_version); HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); --=20 2.13.6