From patchwork Mon Jan 21 13:42:48 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Hildenbrand X-Patchwork-Id: 1028687 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43jtBg2kxzz9sD4 for ; Tue, 22 Jan 2019 00:46:51 +1100 (AEDT) Received: from localhost ([127.0.0.1]:53936 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1glZub-0003sK-A9 for incoming@patchwork.ozlabs.org; Mon, 21 Jan 2019 08:46:49 -0500 Received: from eggs.gnu.org ([209.51.188.92]:49961) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1glZqs-0001Hh-Qr for qemu-devel@nongnu.org; Mon, 21 Jan 2019 08:43:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1glZqr-0006uQ-FH for qemu-devel@nongnu.org; Mon, 21 Jan 2019 08:42:58 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40322) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1glZqq-0006ta-MI; Mon, 21 Jan 2019 08:42:57 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DFA06B5BA; Mon, 21 Jan 2019 13:42:55 +0000 (UTC) Received: from t460s.redhat.com (unknown [10.36.118.109]) by smtp.corp.redhat.com (Postfix) with ESMTP id 102F91048125; Mon, 21 Jan 2019 13:42:53 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Mon, 21 Jan 2019 14:42:48 +0100 Message-Id: <20190121134249.16615-2-david@redhat.com> In-Reply-To: <20190121134249.16615-1-david@redhat.com> References: <20190121134249.16615-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 21 Jan 2019 13:42:56 +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 v3 1/2] 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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 | 166 +++++++++++++++++++++++++++------------- hw/s390x/s390-pci-bus.h | 1 + 2 files changed, 113 insertions(+), 54 deletions(-) diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index f017c1ded0..bc17a8cf65 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -148,6 +148,22 @@ out: psccb->header.response_code = cpu_to_be16(rc); } +static void s390_pci_perform_unplug(S390PCIBusDevice *pbdev) +{ + HotplugHandler *hotplug_ctrl; + + /* Unplug the PCI device */ + if (pbdev->pdev) { + hotplug_ctrl = qdev_get_hotplug_handler(DEVICE(pbdev->pdev)); + hotplug_handler_unplug(hotplug_ctrl, DEVICE(pbdev->pdev), + &error_abort); + } + + /* Unplug the zPCI device */ + hotplug_ctrl = qdev_get_hotplug_handler(DEVICE(pbdev)); + hotplug_handler_unplug(hotplug_ctrl, DEVICE(pbdev), &error_abort); +} + void s390_pci_sclp_deconfigure(SCCB *sccb) { IoaCfgSccb *psccb = (IoaCfgSccb *)sccb; @@ -179,7 +195,7 @@ void s390_pci_sclp_deconfigure(SCCB *sccb) rc = SCLP_RC_NORMAL_COMPLETION; 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(S390pciState *s, return NULL; } +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 == 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); @@ -939,77 +973,100 @@ static void s390_pcihost_timer_cb(void *opaque) pbdev->state = 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); } static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { S390pciState *s = S390_PCI_HOST_BRIDGE(hotplug_dev); - PCIDevice *pci_dev = NULL; - PCIBus *bus; - int32_t devfn; S390PCIBusDevice *pbdev = NULL; + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { + PCIDevice *pci_dev = PCI_DEVICE(dev); + PCIBus *bus; + int32_t devfn; + + pbdev = 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 = pci_get_bus(pci_dev); + devfn = pci_dev->devfn; + object_unparent(OBJECT(pci_dev)); + + s390_pci_msix_free(pbdev); + s390_pci_iommu_free(s, bus, devfn); + pbdev->pdev = NULL; + pbdev->state = ZPCI_FS_RESERVED; + } else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) { + pbdev = S390_PCI_DEVICE(dev); + + if (pbdev->release_timer) { + timer_del(pbdev->release_timer); + timer_free(pbdev->release_timer); + pbdev->release_timer = NULL; + } + pbdev->fid = 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 = 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 = PCI_DEVICE(dev); - - QTAILQ_FOREACH(pbdev, &s->zpci_devs, link) { - if (pbdev->pdev == pci_dev) { - break; - } - } - assert(pbdev != NULL); + /* + * Redirect the unplug request to the zPCI device and remember that + * we've checked the PCI device already (to prevent endless recursion). + */ + pbdev = s390_pci_find_dev_by_pci(s, PCI_DEVICE(dev)); + g_assert(pbdev); + pbdev->pci_unplug_request_processed = true; + qdev_unplug(DEVICE(pbdev), errp); } else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) { pbdev = S390_PCI_DEVICE(dev); - pci_dev = pbdev->pdev; - } else { - g_assert_not_reached(); - } - 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 = 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_TIMEOUT); - return; - } + pbdev->pci_unplug_request_processed = false; - if (pbdev->release_timer) { - timer_del(pbdev->release_timer); - timer_free(pbdev->release_timer); - pbdev->release_timer = 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 = 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_TIMEOUT); + } + } else { + g_assert_not_reached(); } - - s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED, - pbdev->fh, pbdev->fid); - bus = pci_get_bus(pci_dev); - devfn = pci_dev->devfn; - object_unparent(OBJECT(pci_dev)); - fmb_timer_free(pbdev); - s390_pci_msix_free(pbdev); - s390_pci_iommu_free(s, bus, devfn); - pbdev->pdev = NULL; - pbdev->state = ZPCI_FS_RESERVED; -out: - pbdev->fid = 0; - QTAILQ_REMOVE(&s->zpci_devs, pbdev, link); - g_hash_table_remove(s->zpci_table, &pbdev->idx); - object_unparent(OBJECT(pbdev)); } static void s390_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, @@ -1059,6 +1116,7 @@ static void s390_pcihost_class_init(ObjectClass *klass, void *data) dc->realize = s390_pcihost_realize; hc->pre_plug = s390_pcihost_pre_plug; hc->plug = s390_pcihost_plug; + hc->unplug_request = s390_pcihost_unplug_request; hc->unplug = s390_pcihost_unplug; msi_nonbroken = true; } diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h index dadad1f758..b1a6bb8296 100644 --- a/hw/s390x/s390-pci-bus.h +++ b/hw/s390x/s390-pci-bus.h @@ -336,6 +336,7 @@ struct S390PCIBusDevice { IndAddr *summary_ind; IndAddr *indicator; QEMUTimer *release_timer; + bool pci_unplug_request_processed; QTAILQ_ENTRY(S390PCIBusDevice) link; }; From patchwork Mon Jan 21 13:42:49 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Hildenbrand X-Patchwork-Id: 1028681 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43jt763WfLz9sBQ for ; Tue, 22 Jan 2019 00:43:46 +1100 (AEDT) Received: from localhost ([127.0.0.1]:53882 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1glZrc-0001KK-C3 for incoming@patchwork.ozlabs.org; Mon, 21 Jan 2019 08:43:44 -0500 Received: from eggs.gnu.org ([209.51.188.92]:49978) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1glZqt-0001IB-Hi for qemu-devel@nongnu.org; Mon, 21 Jan 2019 08:43:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1glZqs-0006vG-Qv for qemu-devel@nongnu.org; Mon, 21 Jan 2019 08:42:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44226) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1glZqs-0006ul-KS; Mon, 21 Jan 2019 08:42:58 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D6393C059B9E; Mon, 21 Jan 2019 13:42:57 +0000 (UTC) Received: from t460s.redhat.com (unknown [10.36.118.109]) by smtp.corp.redhat.com (Postfix) with ESMTP id 322441048125; Mon, 21 Jan 2019 13:42:56 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Mon, 21 Jan 2019 14:42:49 +0100 Message-Id: <20190121134249.16615-3-david@redhat.com> In-Reply-To: <20190121134249.16615-1-david@redhat.com> References: <20190121134249.16615-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Mon, 21 Jan 2019 13:42: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 v3 2/2] 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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=test (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 bc17a8cf65..b70ae25533 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -1102,6 +1102,14 @@ static void s390_pcihost_reset(DeviceState *dev) { S390pciState *s = S390_PCI_HOST_BRIDGE(dev); PCIBus *bus = 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); + } + } s->bus_no = 0; pci_for_each_device(bus, pci_bus_num(bus), s390_pci_enumerate_bridge, s);