From patchwork Fri Nov 24 13:46:44 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eduardo Otubo X-Patchwork-Id: 10074059 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 05B7960375 for ; Fri, 24 Nov 2017 13:47:43 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E806B297E2 for ; Fri, 24 Nov 2017 13:47:42 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DB4772A2F2; Fri, 24 Nov 2017 13:47:42 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 4454F297E2 for ; Fri, 24 Nov 2017 13:47:42 +0000 (UTC) Received: from localhost ([::1]:49368 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eIEKT-0003my-8g for patchwork-qemu-devel@patchwork.kernel.org; Fri, 24 Nov 2017 08:47:41 -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+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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=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 created 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 --- 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)); } +void qdev_cleanup_nofail(DeviceState *dev) +{ + Error *err = 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 @@ #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) @@ -118,13 +119,13 @@ static void i82374_realize(DeviceState *dev, Error **errp) { I82374State *s = I82374(dev); - 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) == 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)); + } } static Property i82374_properties[] = { 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) type_init(i8257_register_types) -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; isa1 = isa_create(bus, TYPE_I8257); - d = 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 = 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); isa2 = isa_create(bus, TYPE_I8257); - d = 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 = 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 gpioirq, int isairq) qdev_connect_gpio_out(DEVICE(isadev), gpioirq, irq); } -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] = dma8; bus->dma[1] = dma16; + return 0; } 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) } /* 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);