From nobody Sat May 4 08:40:13 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.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 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1547053380931102.39119219245345; Wed, 9 Jan 2019 09:03:00 -0800 (PST) Received: from localhost ([127.0.0.1]:39021 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghHFr-0001oN-Ee for importer@patchew.org; Wed, 09 Jan 2019 12:02:59 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51689) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghH7A-0002tE-GS for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:54:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ghH79-0005Bm-C7 for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:54:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:32814) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ghH79-0005BI-5z; Wed, 09 Jan 2019 11:53:59 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C369589AE8; Wed, 9 Jan 2019 16:53:57 +0000 (UTC) Received: from t460s.redhat.com (ovpn-116-123.ams2.redhat.com [10.36.116.123]) by smtp.corp.redhat.com (Postfix) with ESMTP id AC84319940; Wed, 9 Jan 2019 16:53:55 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Wed, 9 Jan 2019 17:53:44 +0100 Message-Id: <20190109165349.23631-2-david@redhat.com> In-Reply-To: <20190109165349.23631-1-david@redhat.com> References: <20190109165349.23631-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 09 Jan 2019 16:53:57 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v1 1/6] s390x/pci: Use hotplug_dev instead of looking up the host bridge 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: Thomas Huth , David Hildenbrand , Cornelia Huck , Collin Walling , Christian Borntraeger , qemu-s390x@nongnu.org, Richard Henderson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" We directly have it in our hands. Signed-off-by: David Hildenbrand --- hw/s390x/s390-pci-bus.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index 15759b6514..de62fc8916 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -826,9 +826,9 @@ static bool s390_pci_alloc_idx(S390pciState *s, S390PCI= BusDevice *pbdev) static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *de= v, Error **errp) { + S390pciState *s =3D S390_PCI_HOST_BRIDGE(hotplug_dev); PCIDevice *pdev =3D NULL; S390PCIBusDevice *pbdev =3D NULL; - S390pciState *s =3D s390_get_phb(); =20 if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) { BusState *bus; @@ -935,11 +935,11 @@ static void s390_pcihost_timer_cb(void *opaque) static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *= dev, Error **errp) { + S390pciState *s =3D S390_PCI_HOST_BRIDGE(hotplug_dev); PCIDevice *pci_dev =3D NULL; PCIBus *bus; int32_t devfn; S390PCIBusDevice *pbdev =3D NULL; - S390pciState *s =3D s390_get_phb(); =20 if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) { error_setg(errp, "PCI bridge hot unplug currently not supported"); --=20 2.17.2 From nobody Sat May 4 08:40:13 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.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 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 15470535842130.8106428194922728; Wed, 9 Jan 2019 09:06:24 -0800 (PST) Received: from localhost ([127.0.0.1]:39815 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghHJ8-0004dY-Jx for importer@patchew.org; Wed, 09 Jan 2019 12:06:22 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51736) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghH7G-0002xB-3C for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:54:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ghH7E-0005Dc-7A for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:54:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:32840) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ghH7D-0005C6-Sd; Wed, 09 Jan 2019 11:54:04 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 22E007E9CF; Wed, 9 Jan 2019 16:54:00 +0000 (UTC) Received: from t460s.redhat.com (ovpn-116-123.ams2.redhat.com [10.36.116.123]) by smtp.corp.redhat.com (Postfix) with ESMTP id 27E921A917; Wed, 9 Jan 2019 16:53:57 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Wed, 9 Jan 2019 17:53:45 +0100 Message-Id: <20190109165349.23631-3-david@redhat.com> In-Reply-To: <20190109165349.23631-1-david@redhat.com> References: <20190109165349.23631-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 09 Jan 2019 16:54:00 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v1 2/6] s390x/pci: Move some hotplug checks to the pre_plug handler 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: Thomas Huth , David Hildenbrand , Cornelia Huck , Collin Walling , Christian Borntraeger , qemu-s390x@nongnu.org, Richard Henderson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Let's move most of the checks to the new pre_plug handler. As a PCI bridge is just a PCI device, we can simplify the code. Notes: We cannot yet move the MSIX check or device ID creation + zPCI device creation to the pre_plug handler as both parts are not fixed before actual device realization (and therefore after pre_plug and before plug). Once that part is factored out, we can move these parts to the pre_plug handler, too and therefore remove all possible errors from the plug handler. Reviewed-by: Collin Walling Signed-off-by: David Hildenbrand --- hw/s390x/s390-pci-bus.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index de62fc8916..936ad14030 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -818,11 +818,31 @@ static bool s390_pci_alloc_idx(S390pciState *s, S390P= CIBusDevice *pbdev) } =20 pbdev->idx =3D idx; - s->next_idx =3D (idx + 1) & FH_MASK_INDEX; - return true; } =20 +static void s390_pcihost_pre_plug(HotplugHandler *hotplug_dev, DeviceState= *dev, + Error **errp) +{ + S390pciState *s =3D S390_PCI_HOST_BRIDGE(hotplug_dev); + + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { + PCIDevice *pdev =3D PCI_DEVICE(dev); + + if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) { + error_setg(errp, "multifunction not supported in s390"); + return; + } + } else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) { + S390PCIBusDevice *pbdev =3D S390_PCI_DEVICE(dev); + + if (!s390_pci_alloc_idx(s, pbdev)) { + error_setg(errp, "no slot for plugging zpci device"); + return; + } + } +} + static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *de= v, Error **errp) { @@ -835,11 +855,6 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_= dev, DeviceState *dev, PCIBridge *pb =3D PCI_BRIDGE(dev); PCIDevice *pdev =3D PCI_DEVICE(dev); =20 - if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) { - error_setg(errp, "multifunction not supported in s390"); - return; - } - pci_bridge_map_irq(pb, dev->id, s390_pci_map_irq); pci_setup_iommu(&pb->sec_bus, s390_pci_dma_iommu, s); =20 @@ -859,11 +874,6 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_= dev, DeviceState *dev, } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { pdev =3D PCI_DEVICE(dev); =20 - if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) { - error_setg(errp, "multifunction not supported in s390"); - return; - } - if (!dev->id) { /* In the case the PCI device does not define an id */ /* we generate one based on the PCI address */ @@ -905,10 +915,8 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_= dev, DeviceState *dev, } else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) { pbdev =3D S390_PCI_DEVICE(dev); =20 - if (!s390_pci_alloc_idx(s, pbdev)) { - error_setg(errp, "no slot for plugging zpci device"); - return; - } + /* the allocated idx is actually getting used */ + s->next_idx =3D (pbdev->idx + 1) & FH_MASK_INDEX; pbdev->fh =3D pbdev->idx; QTAILQ_INSERT_TAIL(&s->zpci_devs, pbdev, link); g_hash_table_insert(s->zpci_table, &pbdev->idx, pbdev); @@ -1041,6 +1049,7 @@ static void s390_pcihost_class_init(ObjectClass *klas= s, void *data) =20 dc->reset =3D s390_pcihost_reset; dc->realize =3D s390_pcihost_realize; + hc->pre_plug =3D s390_pcihost_pre_plug; hc->plug =3D s390_pcihost_plug; hc->unplug =3D s390_pcihost_unplug; msi_nonbroken =3D true; --=20 2.17.2 From nobody Sat May 4 08:40:13 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.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 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1547053260212877.6920515447022; Wed, 9 Jan 2019 09:01:00 -0800 (PST) Received: from localhost ([127.0.0.1]:38497 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghHDs-0008UF-7R for importer@patchew.org; Wed, 09 Jan 2019 12:00:56 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51760) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghH7H-0002zZ-VX for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:54:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ghH7G-0005EJ-4i for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:54:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:24386) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ghH7F-0005D9-4F; Wed, 09 Jan 2019 11:54:06 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7BA6DCD66C; Wed, 9 Jan 2019 16:54:02 +0000 (UTC) Received: from t460s.redhat.com (ovpn-116-123.ams2.redhat.com [10.36.116.123]) by smtp.corp.redhat.com (Postfix) with ESMTP id 83BFB57A8; Wed, 9 Jan 2019 16:54:00 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Wed, 9 Jan 2019 17:53:46 +0100 Message-Id: <20190109165349.23631-4-david@redhat.com> In-Reply-To: <20190109165349.23631-1-david@redhat.com> References: <20190109165349.23631-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Wed, 09 Jan 2019 16:54:02 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v1 3/6] s390x/pci: Always delete and free the release_timer 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: Thomas Huth , David Hildenbrand , Cornelia Huck , Collin Walling , Christian Borntraeger , qemu-s390x@nongnu.org, Richard Henderson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" We should always get rid of it. I don't see a reason to keep the timer alive if the devices are going away. This looks like a memory leak. (hmp) device_add virtio-mouse-pci,id=3Dtest (hmp) device_del test -> guest notified, timer pending. -> guest does not react for some reason (e.g. crash) -> s390_pcihost_timer_cb(). Timer not pending anymore. qmp_unplug(). -> Device deleted. Timer expired (not pending) but not freed. Signed-off-by: David Hildenbrand --- hw/s390x/s390-pci-bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index 936ad14030..1349050838 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -982,7 +982,7 @@ static void s390_pcihost_unplug(HotplugHandler *hotplug= _dev, DeviceState *dev, return; } =20 - if (pbdev->release_timer && timer_pending(pbdev->release_timer)) { + if (pbdev->release_timer) { timer_del(pbdev->release_timer); timer_free(pbdev->release_timer); pbdev->release_timer =3D NULL; --=20 2.17.2 From nobody Sat May 4 08:40:13 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.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 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1547053471653675.1273777347511; Wed, 9 Jan 2019 09:04:31 -0800 (PST) Received: from localhost ([127.0.0.1]:39326 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghHHK-0002qk-4k for importer@patchew.org; Wed, 09 Jan 2019 12:04:30 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51795) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghH7K-00032K-NI for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:54:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ghH7J-0005FV-Ve for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:54:10 -0500 Received: from mx1.redhat.com ([209.132.183.28]:15452) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ghH7H-0005ES-UO; Wed, 09 Jan 2019 11:54:08 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 67244CD66C; Wed, 9 Jan 2019 16:54:06 +0000 (UTC) Received: from t460s.redhat.com (ovpn-116-123.ams2.redhat.com [10.36.116.123]) by smtp.corp.redhat.com (Postfix) with ESMTP id C01491A917; Wed, 9 Jan 2019 16:54:02 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Wed, 9 Jan 2019 17:53:47 +0100 Message-Id: <20190109165349.23631-5-david@redhat.com> In-Reply-To: <20190109165349.23631-1-david@redhat.com> References: <20190109165349.23631-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Wed, 09 Jan 2019 16:54:06 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v1 4/6] s390x/pci: Ignore the unplug call if we already have a release_timer 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: Thomas Huth , David Hildenbrand , Cornelia Huck , Collin Walling , Christian Borntraeger , qemu-s390x@nongnu.org, Richard Henderson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" ... otherwise two successive calls to qdev_unplug() (e.g. by an impatient user) will effectively overwrite pbdev->release_timer, resulting in a memory leak. We are effectively already processing the unplug. If there is already a release_timer, the unplug will be performed after the timeout. Can be easily triggered by (hmp) device_add virtio-mouse-pci,id=3Dtest (hmp) stop (hmp) device_del test (hmp) device_del test Signed-off-by: David Hildenbrand --- hw/s390x/s390-pci-bus.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index 1349050838..f27c5852db 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -972,6 +972,9 @@ static void s390_pcihost_unplug(HotplugHandler *hotplug= _dev, DeviceState *dev, case ZPCI_FS_STANDBY: break; default: + if (pbdev->release_timer) { + return; + } s390_pci_generate_plug_event(HP_EVENT_DECONFIGURE_REQUEST, pbdev->fh, pbdev->fid); pbdev->release_timer =3D timer_new_ns(QEMU_CLOCK_VIRTUAL, --=20 2.17.2 From nobody Sat May 4 08:40:13 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.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 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1547053669545296.8678452567019; Wed, 9 Jan 2019 09:07:49 -0800 (PST) Received: from localhost ([127.0.0.1]:40160 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghHKR-0005gK-NU for importer@patchew.org; Wed, 09 Jan 2019 12:07:43 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51837) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghH7L-000349-R0 for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:54:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ghH7K-0005Fy-9D for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:54:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48570) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ghH7J-0005FD-Vw; Wed, 09 Jan 2019 11:54:10 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BB4E290B16; Wed, 9 Jan 2019 16:54:08 +0000 (UTC) Received: from t460s.redhat.com (ovpn-116-123.ams2.redhat.com [10.36.116.123]) by smtp.corp.redhat.com (Postfix) with ESMTP id BBDD157A8; Wed, 9 Jan 2019 16:54:06 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Wed, 9 Jan 2019 17:53:48 +0100 Message-Id: <20190109165349.23631-6-david@redhat.com> In-Reply-To: <20190109165349.23631-1-david@redhat.com> References: <20190109165349.23631-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 09 Jan 2019 16:54:08 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v1 5/6] s390x/pci: Introduce unplug requests and split unplug handler 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: Thomas Huth , David Hildenbrand , Cornelia Huck , Collin Walling , Christian Borntraeger , qemu-s390x@nongnu.org, Richard Henderson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" PCI on s390x is really weird and how it was modeled in QEMU might not have been the right choice. Anyhow, right now it is the case that: - Hotplugging a PCI device will silently create a zPCI device (if none is provided) - Hotunplugging a zPCI device will unplug the PCI device (if any) - Hotunplugging a PCI device will unplug also the zPCI device However both devices are handled via a single hotplug handler call. This is problematic for various reasons: 1. Unplugging via the zPCI device allows to unplug PCI bridges as checks are not performed - bad. 2. Unplugging via the zPCI device allows to unplug devices that are not hot removable. (check performed in qdev_unplug()) - bad. 3. Hotplug handler chains are not possible for the unplug case. In the future, the machine might want to override hotplug handlers, to process device specific stuff and to then branch off to the actual hotplug handler. We need separate hotplug handler calls for both the PCI and zPCI device to make this work reliably. All other PCI implementations are already prepared to handle this correctly, only s390x is missing. Therefore, introduce the unplug_request handler and properly perform unplug checks by redirecting to the separate unplug_request handlers. When finally unplugging, perform two separate hotplug_handler_unplug() calls, first for the PCI device, followed by the zPCI device. This now nicely splits unplugging paths for both devices. The redirect part is a little hairy, as the user is allowed to trigger unplug either via the PCI or the zPCI device. So redirect always to the PCI unplug request handler first and remember if that check has been performed in the zPCI device. Redirect then to the zPCI device unplug request handler to perform the magic. Otherwise we would have a redirect loop. Signed-off-by: David Hildenbrand --- hw/s390x/s390-pci-bus.c | 162 +++++++++++++++++++++++++++------------- hw/s390x/s390-pci-bus.h | 1 + 2 files changed, 112 insertions(+), 51 deletions(-) diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index f27c5852db..bcd8565a14 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -148,6 +148,23 @@ out: psccb->header.response_code =3D cpu_to_be16(rc); } =20 +static void s390_pci_perform_unplug(S390PCIBusDevice *pbdev) +{ + HotplugHandler *hotplug_ctrl; + + /* Unplug the PCI device */ + if (pbdev->state =3D=3D ZPCI_FS_STANDBY) { + hotplug_ctrl =3D qdev_get_hotplug_handler(DEVICE(pbdev->pdev)); + hotplug_handler_unplug(hotplug_ctrl, DEVICE(pbdev->pdev), + &error_abort); + } + g_assert(pbdev->state =3D=3D ZPCI_FS_RESERVED); + + /* Unplug the zPCI device */ + hotplug_ctrl =3D qdev_get_hotplug_handler(DEVICE(pbdev)); + hotplug_handler_unplug(hotplug_ctrl, DEVICE(pbdev), &error_abort); +} + void s390_pci_sclp_deconfigure(SCCB *sccb) { IoaCfgSccb *psccb =3D (IoaCfgSccb *)sccb; @@ -179,7 +196,7 @@ void s390_pci_sclp_deconfigure(SCCB *sccb) rc =3D SCLP_RC_NORMAL_COMPLETION; =20 if (pbdev->release_timer) { - qdev_unplug(DEVICE(pbdev->pdev), NULL); + s390_pci_perform_unplug(pbdev); } } out: @@ -217,6 +234,24 @@ S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciS= tate *s, return NULL; } =20 +static S390PCIBusDevice *s390_pci_find_dev_by_pci(S390pciState *s, + PCIDevice *pci_dev) +{ + S390PCIBusDevice *pbdev; + + if (!pci_dev) { + return NULL; + } + + QTAILQ_FOREACH(pbdev, &s->zpci_devs, link) { + if (pbdev->pdev =3D=3D pci_dev) { + return pbdev; + } + } + + return NULL; +} + S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx) { return g_hash_table_lookup(s->zpci_table, &idx); @@ -937,74 +972,98 @@ static void s390_pcihost_timer_cb(void *opaque) pbdev->state =3D ZPCI_FS_STANDBY; s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES, pbdev->fh, pbdev->fid); - qdev_unplug(DEVICE(pbdev), NULL); + s390_pci_perform_unplug(pbdev); } =20 static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *= dev, Error **errp) { S390pciState *s =3D S390_PCI_HOST_BRIDGE(hotplug_dev); - PCIDevice *pci_dev =3D NULL; - PCIBus *bus; - int32_t devfn; S390PCIBusDevice *pbdev =3D NULL; =20 + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { + PCIDevice *pci_dev =3D PCI_DEVICE(dev); + PCIBus *bus; + int32_t devfn; + + pbdev =3D s390_pci_find_dev_by_pci(s, PCI_DEVICE(dev)); + g_assert(pbdev); + + s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED, + pbdev->fh, pbdev->fid); + bus =3D pci_get_bus(pci_dev); + devfn =3D pci_dev->devfn; + object_unparent(OBJECT(pci_dev)); + + s390_pci_msix_free(pbdev); + s390_pci_iommu_free(s, bus, devfn); + pbdev->pdev =3D NULL; + pbdev->state =3D ZPCI_FS_RESERVED; + } else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) { + pbdev =3D S390_PCI_DEVICE(dev); + + if (pbdev->release_timer) { + timer_del(pbdev->release_timer); + timer_free(pbdev->release_timer); + pbdev->release_timer =3D NULL; + } + pbdev->fid =3D 0; + QTAILQ_REMOVE(&s->zpci_devs, pbdev, link); + g_hash_table_remove(s->zpci_table, &pbdev->idx); + object_unparent(OBJECT(pbdev)); + } +} + +static void s390_pcihost_unplug_request(HotplugHandler *hotplug_dev, + DeviceState *dev, + Error **errp) +{ + S390pciState *s =3D S390_PCI_HOST_BRIDGE(hotplug_dev); + S390PCIBusDevice *pbdev; + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) { error_setg(errp, "PCI bridge hot unplug currently not supported"); - return; } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { - pci_dev =3D PCI_DEVICE(dev); - - QTAILQ_FOREACH(pbdev, &s->zpci_devs, link) { - if (pbdev->pdev =3D=3D pci_dev) { - break; - } - } - assert(pbdev !=3D NULL); + /* + * Redirect the unplug request to the zPCI device and remember that + * we've checked the PCI device already (to prevent endless recurs= ion). + */ + pbdev =3D s390_pci_find_dev_by_pci(s, PCI_DEVICE(dev)); + g_assert(pbdev); + pbdev->pci_unplug_request_processed =3D true; + qdev_unplug(DEVICE(pbdev), errp); } else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) { pbdev =3D S390_PCI_DEVICE(dev); - pci_dev =3D pbdev->pdev; - } =20 - switch (pbdev->state) { - case ZPCI_FS_RESERVED: - goto out; - case ZPCI_FS_STANDBY: - break; - default: - if (pbdev->release_timer) { + /* + * If unplug was initially requested for the zPCI device, we + * first have to redirect to the PCI device, which will in return + * redirect back to us after performing its checks (if the request + * is not blocked, e.g. because it's a PCI bridge). + */ + if (pbdev->pdev && !pbdev->pci_unplug_request_processed) { + qdev_unplug(DEVICE(pbdev->pdev), errp); return; } - s390_pci_generate_plug_event(HP_EVENT_DECONFIGURE_REQUEST, - pbdev->fh, pbdev->fid); - pbdev->release_timer =3D timer_new_ns(QEMU_CLOCK_VIRTUAL, - s390_pcihost_timer_cb, - pbdev); - timer_mod(pbdev->release_timer, - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + HOT_UNPLUG_TIMEO= UT); - return; - } + pbdev->pci_unplug_request_processed =3D false; =20 - if (pbdev->release_timer) { - timer_del(pbdev->release_timer); - timer_free(pbdev->release_timer); - pbdev->release_timer =3D NULL; + switch (pbdev->state) { + case ZPCI_FS_STANDBY: + case ZPCI_FS_RESERVED: + s390_pci_perform_unplug(pbdev); + break; + default: + if (pbdev->release_timer) { + return; + } + s390_pci_generate_plug_event(HP_EVENT_DECONFIGURE_REQUEST, + pbdev->fh, pbdev->fid); + pbdev->release_timer =3D timer_new_ns(QEMU_CLOCK_VIRTUAL, + s390_pcihost_timer_cb, pbd= ev); + timer_mod(pbdev->release_timer, + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + HOT_UNPLUG_T= IMEOUT); + } } - - s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED, - pbdev->fh, pbdev->fid); - bus =3D pci_get_bus(pci_dev); - devfn =3D pci_dev->devfn; - object_unparent(OBJECT(pci_dev)); - s390_pci_msix_free(pbdev); - s390_pci_iommu_free(s, bus, devfn); - pbdev->pdev =3D NULL; - pbdev->state =3D ZPCI_FS_RESERVED; -out: - pbdev->fid =3D 0; - QTAILQ_REMOVE(&s->zpci_devs, pbdev, link); - g_hash_table_remove(s->zpci_table, &pbdev->idx); - object_unparent(OBJECT(pbdev)); } =20 static void s390_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, @@ -1054,6 +1113,7 @@ static void s390_pcihost_class_init(ObjectClass *klas= s, void *data) dc->realize =3D s390_pcihost_realize; hc->pre_plug =3D s390_pcihost_pre_plug; hc->plug =3D s390_pcihost_plug; + hc->unplug_request =3D s390_pcihost_unplug_request; hc->unplug =3D s390_pcihost_unplug; msi_nonbroken =3D true; } diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h index f47a0f2da5..6987c31df2 100644 --- a/hw/s390x/s390-pci-bus.h +++ b/hw/s390x/s390-pci-bus.h @@ -307,6 +307,7 @@ struct S390PCIBusDevice { IndAddr *summary_ind; IndAddr *indicator; QEMUTimer *release_timer; + bool pci_unplug_request_processed; QTAILQ_ENTRY(S390PCIBusDevice) link; }; =20 --=20 2.17.2 From nobody Sat May 4 08:40:13 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.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 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1547053845985816.7134796642348; Wed, 9 Jan 2019 09:10:45 -0800 (PST) Received: from localhost ([127.0.0.1]:41068 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghHNL-0008C2-7H for importer@patchew.org; Wed, 09 Jan 2019 12:10:43 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51891) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghH7Y-0003Fv-L7 for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:54:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ghH7O-0005IS-VQ for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:54:21 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35474) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ghH7O-0005ID-QE; Wed, 09 Jan 2019 11:54:14 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 11F6E9D51C; Wed, 9 Jan 2019 16:54:14 +0000 (UTC) Received: from t460s.redhat.com (ovpn-116-123.ams2.redhat.com [10.36.116.123]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1E76557A8; Wed, 9 Jan 2019 16:54:08 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Wed, 9 Jan 2019 17:53:49 +0100 Message-Id: <20190109165349.23631-7-david@redhat.com> In-Reply-To: <20190109165349.23631-1-david@redhat.com> References: <20190109165349.23631-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Wed, 09 Jan 2019 16:54:14 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v1 6/6] s390x/pci: unplug remaining devices on pcihost reset 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: Thomas Huth , David Hildenbrand , Cornelia Huck , Collin Walling , Christian Borntraeger , qemu-s390x@nongnu.org, Richard Henderson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" When resetting the guest we should unplug and remove all devices that are still pending. Otherwise the fresh guest will see devices that will suddenly vanish. Can be triggered e.g. via (hmp) device_add virtio-mouse-pci,id=3Dtest (hmp) stop (hmp) device_del test (hmp) system_reset (hmp) c The device will vanish after roughly 5 minutes. With this patch, the device will vanish on reboot (S390_RESET_EXTERNAL and S390_RESET_REIPL, which reset the pcihost bridge via qemu_devices_reset()). If we want these devices to vanish directly on any reset (S390_RESET_MODIFIED_CLEAR and S390_RESET_LOAD_NORMAL), we have to modify s390_machine_reset(). s390_pci_generate_plug_event()'s will still be generated, but that is similar to hotplugging a PCI device before the guest has been started, so I guess it is not problematic. (we could inject such events when hotplugging based on "dev->hotplugged" property, when unplugging we would have to rely on the runstate) Signed-off-by: David Hildenbrand --- hw/s390x/s390-pci-bus.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index bcd8565a14..0212e83069 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -1099,6 +1099,14 @@ static void s390_pcihost_reset(DeviceState *dev) { S390pciState *s =3D S390_PCI_HOST_BRIDGE(dev); PCIBus *bus =3D s->parent_obj.bus; + S390PCIBusDevice *pbdev, *next; + + /* Unplug all pending devices that were requested to be released */ + QTAILQ_FOREACH_SAFE(pbdev, &s->zpci_devs, link, next) { + if (pbdev->release_timer) { + s390_pcihost_timer_cb(pbdev); + } + } =20 s->bus_no =3D 0; pci_for_each_device(bus, pci_bus_num(bus), s390_pci_enumerate_bridge, = s); --=20 2.17.2