From nobody Fri Nov 7 06:50:11 2025 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 1547462710174776.700568158338; Mon, 14 Jan 2019 02:45:10 -0800 (PST) Received: from localhost ([127.0.0.1]:41803 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gizjx-000703-0m for importer@patchew.org; Mon, 14 Jan 2019 05:45:09 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51094) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gizWc-0004Id-A0 for qemu-devel@nongnu.org; Mon, 14 Jan 2019 05:31:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gizWb-0000nT-KZ for qemu-devel@nongnu.org; Mon, 14 Jan 2019 05:31:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37810) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gizWb-0000hb-A3; Mon, 14 Jan 2019 05:31:21 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4EB1F637EE; Mon, 14 Jan 2019 10:31:19 +0000 (UTC) Received: from t460s.redhat.com (unknown [10.36.118.18]) by smtp.corp.redhat.com (Postfix) with ESMTP id 93AF31810B; Mon, 14 Jan 2019 10:31:17 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Mon, 14 Jan 2019 11:31:05 +0100 Message-Id: <20190114103110.10909-2-david@redhat.com> In-Reply-To: <20190114103110.10909-1-david@redhat.com> References: <20190114103110.10909-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Mon, 14 Jan 2019 10:31:19 +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 v2 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 Reviewed-by: Collin Walling --- 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 7f911b216a..86dda831f9 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 Fri Nov 7 06:50:11 2025 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 1547462880106963.277588878339; Mon, 14 Jan 2019 02:48:00 -0800 (PST) Received: from localhost ([127.0.0.1]:42397 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gizmc-0000af-8N for importer@patchew.org; Mon, 14 Jan 2019 05:47:54 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51120) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gizWk-0004Of-HC for qemu-devel@nongnu.org; Mon, 14 Jan 2019 05:31:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gizWc-0000pe-Kp for qemu-devel@nongnu.org; Mon, 14 Jan 2019 05:31:25 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49538) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gizWc-0000nx-Cg; Mon, 14 Jan 2019 05:31:22 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6119889AD0; Mon, 14 Jan 2019 10:31:21 +0000 (UTC) Received: from t460s.redhat.com (unknown [10.36.118.18]) by smtp.corp.redhat.com (Postfix) with ESMTP id 942F960CCF; Mon, 14 Jan 2019 10:31:19 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Mon, 14 Jan 2019 11:31:06 +0100 Message-Id: <20190114103110.10909-3-david@redhat.com> In-Reply-To: <20190114103110.10909-1-david@redhat.com> References: <20190114103110.10909-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Mon, 14 Jan 2019 10:31:21 +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 v2 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 86dda831f9..1775388524 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 Fri Nov 7 06:50:11 2025 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 1547463108170877.9363104724204; Mon, 14 Jan 2019 02:51:48 -0800 (PST) Received: from localhost ([127.0.0.1]:43167 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gizqN-0003r9-3S for importer@patchew.org; Mon, 14 Jan 2019 05:51:47 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51174) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gizWu-0004am-R4 for qemu-devel@nongnu.org; Mon, 14 Jan 2019 05:31:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gizWt-0001JA-Sx for qemu-devel@nongnu.org; Mon, 14 Jan 2019 05:31:40 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37896) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gizWl-0000sB-B3; Mon, 14 Jan 2019 05:31:36 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5D4AF58E27; Mon, 14 Jan 2019 10:31:23 +0000 (UTC) Received: from t460s.redhat.com (unknown [10.36.118.18]) by smtp.corp.redhat.com (Postfix) with ESMTP id A76D260CCF; Mon, 14 Jan 2019 10:31:21 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Mon, 14 Jan 2019 11:31:07 +0100 Message-Id: <20190114103110.10909-4-david@redhat.com> In-Reply-To: <20190114103110.10909-1-david@redhat.com> References: <20190114103110.10909-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 14 Jan 2019 10:31:23 +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 v2 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 Reviewed-by: Collin Walling --- 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 1775388524..59325cae3b 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 Fri Nov 7 06:50:11 2025 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 1547463288185715.7681760142838; Mon, 14 Jan 2019 02:54:48 -0800 (PST) Received: from localhost ([127.0.0.1]:43857 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1giztH-0006Tb-7M for importer@patchew.org; Mon, 14 Jan 2019 05:54:47 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51207) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gizWv-0004ba-Ia for qemu-devel@nongnu.org; Mon, 14 Jan 2019 05:31:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gizWu-0001Kf-I2 for qemu-devel@nongnu.org; Mon, 14 Jan 2019 05:31:41 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47854) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gizWu-00010K-Am; Mon, 14 Jan 2019 05:31:40 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4CC1A811DB; Mon, 14 Jan 2019 10:31:28 +0000 (UTC) Received: from t460s.redhat.com (unknown [10.36.118.18]) by smtp.corp.redhat.com (Postfix) with ESMTP id A3E605E9C4; Mon, 14 Jan 2019 10:31:23 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Mon, 14 Jan 2019 11:31:08 +0100 Message-Id: <20190114103110.10909-5-david@redhat.com> In-Reply-To: <20190114103110.10909-1-david@redhat.com> References: <20190114103110.10909-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Mon, 14 Jan 2019 10:31:28 +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 v2 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 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 Reviewed-by: Collin Walling --- 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 59325cae3b..34a9cb2a80 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 Fri Nov 7 06:50:11 2025 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 154746347231311.329573193879355; Mon, 14 Jan 2019 02:57:52 -0800 (PST) Received: from localhost ([127.0.0.1]:44547 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gizwA-0000Zy-9q for importer@patchew.org; Mon, 14 Jan 2019 05:57:46 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51251) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gizWw-0004cu-Ow for qemu-devel@nongnu.org; Mon, 14 Jan 2019 05:31:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gizWu-0001KS-Gv for qemu-devel@nongnu.org; Mon, 14 Jan 2019 05:31:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42306) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gizWu-00013V-7e; Mon, 14 Jan 2019 05:31:40 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 472F6DC8E0; Mon, 14 Jan 2019 10:31:30 +0000 (UTC) Received: from t460s.redhat.com (unknown [10.36.118.18]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9421F60C73; Mon, 14 Jan 2019 10:31:28 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Mon, 14 Jan 2019 11:31:09 +0100 Message-Id: <20190114103110.10909-6-david@redhat.com> In-Reply-To: <20190114103110.10909-1-david@redhat.com> References: <20190114103110.10909-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 14 Jan 2019 10:31:30 +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 v2 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 As far as I can see, we can no longer change this behavior. But we should fix it. Both device types 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. Remembering that we already checked the PCI device breaks the redirect loop. Signed-off-by: David Hildenbrand --- hw/s390x/s390-pci-bus.c | 161 +++++++++++++++++++++++++++------------- hw/s390x/s390-pci-bus.h | 1 + 2 files changed, 111 insertions(+), 51 deletions(-) diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index 34a9cb2a80..bfd7fc1d2d 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -148,6 +148,22 @@ 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->pdev) { + hotplug_ctrl =3D qdev_get_hotplug_handler(DEVICE(pbdev->pdev)); + hotplug_handler_unplug(hotplug_ctrl, DEVICE(pbdev->pdev), + &error_abort); + } + + /* 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 +195,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 +233,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 +971,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_TIM= EOUT); + } } - - 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 +1112,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 Fri Nov 7 06:50:11 2025 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 15474633096531.512611231595315; Mon, 14 Jan 2019 02:55:09 -0800 (PST) Received: from localhost ([127.0.0.1]:43917 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1giztc-0006sk-Ju for importer@patchew.org; Mon, 14 Jan 2019 05:55:08 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51194) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gizWv-0004bH-90 for qemu-devel@nongnu.org; Mon, 14 Jan 2019 05:31:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gizWu-0001K4-Ba for qemu-devel@nongnu.org; Mon, 14 Jan 2019 05:31:41 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42352) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gizWu-000174-54; Mon, 14 Jan 2019 05:31:40 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4302A12F900; Mon, 14 Jan 2019 10:31:32 +0000 (UTC) Received: from t460s.redhat.com (unknown [10.36.118.18]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8DE3B60C73; Mon, 14 Jan 2019 10:31:30 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Mon, 14 Jan 2019 11:31:10 +0100 Message-Id: <20190114103110.10909-7-david@redhat.com> In-Reply-To: <20190114103110.10909-1-david@redhat.com> References: <20190114103110.10909-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 14 Jan 2019 10:31:32 +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 v2 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(). But I have the feeling that this should not be done for all reset types. This approach is similar to what's done for acpi PCI hotplug in acpi_pcihp_reset() -> acpi_pcihp_update() -> acpi_pcihp_update_hotplug_bus() -> acpi_pcihp_eject_slot(). s390_pci_generate_plug_event()'s will still be generated, I guess this is not an issue (same thing could happen right now if the timer expires just after reset). 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 bfd7fc1d2d..f399eeede6 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -1098,6 +1098,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