yeahuh / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone

Blame SOURCES/kvm-s390x-pci-Introduce-unplug-requests-and-split-unplug.patch

b38b0f
From db9158c9e7905a77c854b178140be79f5295bcdd Mon Sep 17 00:00:00 2001
b38b0f
From: Cornelia Huck <cohuck@redhat.com>
b38b0f
Date: Wed, 17 Apr 2019 13:57:33 +0100
b38b0f
Subject: [PATCH 16/24] s390x/pci: Introduce unplug requests and split unplug
b38b0f
 handler
b38b0f
b38b0f
RH-Author: Cornelia Huck <cohuck@redhat.com>
b38b0f
Message-id: <20190417135741.25297-17-cohuck@redhat.com>
b38b0f
Patchwork-id: 85799
b38b0f
O-Subject: [RHEL-8.1.0 qemu-kvm PATCH v2 16/24] s390x/pci: Introduce unplug requests and split unplug handler
b38b0f
Bugzilla: 1699070
b38b0f
RH-Acked-by: David Hildenbrand <david@redhat.com>
b38b0f
RH-Acked-by: Thomas Huth <thuth@redhat.com>
b38b0f
RH-Acked-by: Jens Freimann <jfreimann@redhat.com>
b38b0f
b38b0f
From: David Hildenbrand <david@redhat.com>
b38b0f
b38b0f
PCI on s390x is really weird and how it was modeled in QEMU might not have
b38b0f
been the right choice. Anyhow, right now it is the case that:
b38b0f
- Hotplugging a PCI device will silently create a zPCI device
b38b0f
  (if none is provided)
b38b0f
- Hotunplugging a zPCI device will unplug the PCI device (if any)
b38b0f
- Hotunplugging a PCI device will unplug also the zPCI device
b38b0f
As far as I can see, we can no longer change this behavior. But we
b38b0f
should fix it.
b38b0f
b38b0f
Both device types are handled via a single hotplug handler call. This
b38b0f
is problematic for various reasons:
b38b0f
1. Unplugging via the zPCI device allows to unplug devices that are not
b38b0f
   hot removable. (check performed in qdev_unplug()) - bad.
b38b0f
2. Hotplug handler chains are not possible for the unplug case. In the
b38b0f
   future, the machine might want to override hotplug handlers, to
b38b0f
   process device specific stuff and to then branch off to the actual
b38b0f
   hotplug handler. We need separate hotplug handler calls for both the
b38b0f
   PCI and zPCI device to make this work reliably. All other PCI
b38b0f
   implementations are already prepared to handle this correctly, only
b38b0f
   s390x is missing.
b38b0f
b38b0f
Therefore, introduce the unplug_request handler and properly perform
b38b0f
unplug checks by redirecting to the separate unplug_request handlers.
b38b0f
When finally unplugging, perform two separate hotplug_handler_unplug()
b38b0f
calls, first for the PCI device, followed by the zPCI device. This now
b38b0f
nicely splits unplugging paths for both devices.
b38b0f
b38b0f
The redirect part is a little hairy, as the user is allowed to trigger
b38b0f
unplug either via the PCI or the zPCI device. So redirect always to the
b38b0f
PCI unplug request handler first and remember if that check has been
b38b0f
performed in the zPCI device. Redirect then to the zPCI device unplug
b38b0f
request handler to perform the magic. Remembering that we already
b38b0f
checked the PCI device breaks the redirect loop.
b38b0f
b38b0f
Signed-off-by: David Hildenbrand <david@redhat.com>
b38b0f
Message-Id: <20190130155733.32742-5-david@redhat.com>
b38b0f
Reviewed-by: Collin Walling <walling@linux.ibm.com>
b38b0f
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
b38b0f
(cherry picked from commit e0998fe8910435f0e818e5c9ac58d4d2d9144a98)
b38b0f
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
b38b0f
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
b38b0f
b38b0f
Conflicts:
b38b0f
	hw/s390x/s390-pci-bus.c
b38b0f
  We don't have 6e92c70c3754 ("s390x/pci: add common function measurement
b38b0f
  block") downstream.
b38b0f
b38b0f
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
b38b0f
---
b38b0f
 hw/s390x/s390-pci-bus.c | 165 ++++++++++++++++++++++++++++++++----------------
b38b0f
 hw/s390x/s390-pci-bus.h |   1 +
b38b0f
 2 files changed, 113 insertions(+), 53 deletions(-)
b38b0f
b38b0f
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
b38b0f
index e3f576a..21419df 100644
b38b0f
--- a/hw/s390x/s390-pci-bus.c
b38b0f
+++ b/hw/s390x/s390-pci-bus.c
b38b0f
@@ -148,6 +148,22 @@ out:
b38b0f
     psccb->header.response_code = cpu_to_be16(rc);
b38b0f
 }
b38b0f
 
b38b0f
+static void s390_pci_perform_unplug(S390PCIBusDevice *pbdev)
b38b0f
+{
b38b0f
+    HotplugHandler *hotplug_ctrl;
b38b0f
+
b38b0f
+    /* Unplug the PCI device */
b38b0f
+    if (pbdev->pdev) {
b38b0f
+        hotplug_ctrl = qdev_get_hotplug_handler(DEVICE(pbdev->pdev));
b38b0f
+        hotplug_handler_unplug(hotplug_ctrl, DEVICE(pbdev->pdev),
b38b0f
+                               &error_abort);
b38b0f
+    }
b38b0f
+
b38b0f
+    /* Unplug the zPCI device */
b38b0f
+    hotplug_ctrl = qdev_get_hotplug_handler(DEVICE(pbdev));
b38b0f
+    hotplug_handler_unplug(hotplug_ctrl, DEVICE(pbdev), &error_abort);
b38b0f
+}
b38b0f
+
b38b0f
 void s390_pci_sclp_deconfigure(SCCB *sccb)
b38b0f
 {
b38b0f
     IoaCfgSccb *psccb = (IoaCfgSccb *)sccb;
b38b0f
@@ -179,7 +195,7 @@ void s390_pci_sclp_deconfigure(SCCB *sccb)
b38b0f
         rc = SCLP_RC_NORMAL_COMPLETION;
b38b0f
 
b38b0f
         if (pbdev->release_timer) {
b38b0f
-            qdev_unplug(DEVICE(pbdev->pdev), NULL);
b38b0f
+            s390_pci_perform_unplug(pbdev);
b38b0f
         }
b38b0f
     }
b38b0f
 out:
b38b0f
@@ -217,6 +233,24 @@ S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,
b38b0f
     return NULL;
b38b0f
 }
b38b0f
 
b38b0f
+static S390PCIBusDevice *s390_pci_find_dev_by_pci(S390pciState *s,
b38b0f
+                                                  PCIDevice *pci_dev)
b38b0f
+{
b38b0f
+    S390PCIBusDevice *pbdev;
b38b0f
+
b38b0f
+    if (!pci_dev) {
b38b0f
+        return NULL;
b38b0f
+    }
b38b0f
+
b38b0f
+    QTAILQ_FOREACH(pbdev, &s->zpci_devs, link) {
b38b0f
+        if (pbdev->pdev == pci_dev) {
b38b0f
+            return pbdev;
b38b0f
+        }
b38b0f
+    }
b38b0f
+
b38b0f
+    return NULL;
b38b0f
+}
b38b0f
+
b38b0f
 S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx)
b38b0f
 {
b38b0f
     return g_hash_table_lookup(s->zpci_table, &idx);
b38b0f
@@ -943,76 +977,100 @@ static void s390_pcihost_timer_cb(void *opaque)
b38b0f
     pbdev->state = ZPCI_FS_STANDBY;
b38b0f
     s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
b38b0f
                                  pbdev->fh, pbdev->fid);
b38b0f
-    qdev_unplug(DEVICE(pbdev), NULL);
b38b0f
+    s390_pci_perform_unplug(pbdev);
b38b0f
 }
b38b0f
 
b38b0f
 static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
b38b0f
                                 Error **errp)
b38b0f
 {
b38b0f
     S390pciState *s = S390_PCI_HOST_BRIDGE(hotplug_dev);
b38b0f
-    PCIDevice *pci_dev = NULL;
b38b0f
-    PCIBus *bus;
b38b0f
-    int32_t devfn;
b38b0f
     S390PCIBusDevice *pbdev = NULL;
b38b0f
 
b38b0f
+    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
b38b0f
+        PCIDevice *pci_dev = PCI_DEVICE(dev);
b38b0f
+        PCIBus *bus;
b38b0f
+        int32_t devfn;
b38b0f
+
b38b0f
+        pbdev = s390_pci_find_dev_by_pci(s, PCI_DEVICE(dev));
b38b0f
+        g_assert(pbdev);
b38b0f
+
b38b0f
+        s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
b38b0f
+                                     pbdev->fh, pbdev->fid);
b38b0f
+        bus = pci_get_bus(pci_dev);
b38b0f
+        devfn = pci_dev->devfn;
b38b0f
+        object_unparent(OBJECT(pci_dev));
b38b0f
+
b38b0f
+        s390_pci_msix_free(pbdev);
b38b0f
+        s390_pci_iommu_free(s, bus, devfn);
b38b0f
+        pbdev->pdev = NULL;
b38b0f
+        pbdev->state = ZPCI_FS_RESERVED;
b38b0f
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) {
b38b0f
+        pbdev = S390_PCI_DEVICE(dev);
b38b0f
+
b38b0f
+        if (pbdev->release_timer) {
b38b0f
+            timer_del(pbdev->release_timer);
b38b0f
+            timer_free(pbdev->release_timer);
b38b0f
+            pbdev->release_timer = NULL;
b38b0f
+        }
b38b0f
+        pbdev->fid = 0;
b38b0f
+        QTAILQ_REMOVE(&s->zpci_devs, pbdev, link);
b38b0f
+        g_hash_table_remove(s->zpci_table, &pbdev->idx);
b38b0f
+        object_unparent(OBJECT(pbdev));
b38b0f
+    }
b38b0f
+}
b38b0f
+
b38b0f
+static void s390_pcihost_unplug_request(HotplugHandler *hotplug_dev,
b38b0f
+                                        DeviceState *dev,
b38b0f
+                                        Error **errp)
b38b0f
+{
b38b0f
+    S390pciState *s = S390_PCI_HOST_BRIDGE(hotplug_dev);
b38b0f
+    S390PCIBusDevice *pbdev;
b38b0f
+
b38b0f
     if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) {
b38b0f
         error_setg(errp, "PCI bridge hot unplug currently not supported");
b38b0f
-        return;
b38b0f
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
b38b0f
-        pci_dev = PCI_DEVICE(dev);
b38b0f
-
b38b0f
-        QTAILQ_FOREACH(pbdev, &s->zpci_devs, link) {
b38b0f
-            if (pbdev->pdev == pci_dev) {
b38b0f
-                break;
b38b0f
-            }
b38b0f
-        }
b38b0f
-        assert(pbdev != NULL);
b38b0f
+        /*
b38b0f
+         * Redirect the unplug request to the zPCI device and remember that
b38b0f
+         * we've checked the PCI device already (to prevent endless recursion).
b38b0f
+         */
b38b0f
+        pbdev = s390_pci_find_dev_by_pci(s, PCI_DEVICE(dev));
b38b0f
+        g_assert(pbdev);
b38b0f
+        pbdev->pci_unplug_request_processed = true;
b38b0f
+        qdev_unplug(DEVICE(pbdev), errp);
b38b0f
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) {
b38b0f
         pbdev = S390_PCI_DEVICE(dev);
b38b0f
-        pci_dev = pbdev->pdev;
b38b0f
-    } else {
b38b0f
-        g_assert_not_reached();
b38b0f
-    }
b38b0f
 
b38b0f
-    switch (pbdev->state) {
b38b0f
-    case ZPCI_FS_RESERVED:
b38b0f
-        goto out;
b38b0f
-    case ZPCI_FS_STANDBY:
b38b0f
-        break;
b38b0f
-    default:
b38b0f
-        if (pbdev->release_timer) {
b38b0f
+        /*
b38b0f
+         * If unplug was initially requested for the zPCI device, we
b38b0f
+         * first have to redirect to the PCI device, which will in return
b38b0f
+         * redirect back to us after performing its checks (if the request
b38b0f
+         * is not blocked, e.g. because it's a PCI bridge).
b38b0f
+         */
b38b0f
+        if (pbdev->pdev && !pbdev->pci_unplug_request_processed) {
b38b0f
+            qdev_unplug(DEVICE(pbdev->pdev), errp);
b38b0f
             return;
b38b0f
         }
b38b0f
-        s390_pci_generate_plug_event(HP_EVENT_DECONFIGURE_REQUEST,
b38b0f
-                                     pbdev->fh, pbdev->fid);
b38b0f
-        pbdev->release_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
b38b0f
-                                            s390_pcihost_timer_cb,
b38b0f
-                                            pbdev);
b38b0f
-        timer_mod(pbdev->release_timer,
b38b0f
-                  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + HOT_UNPLUG_TIMEOUT);
b38b0f
-        return;
b38b0f
-    }
b38b0f
+        pbdev->pci_unplug_request_processed = false;
b38b0f
 
b38b0f
-    if (pbdev->release_timer) {
b38b0f
-        timer_del(pbdev->release_timer);
b38b0f
-        timer_free(pbdev->release_timer);
b38b0f
-        pbdev->release_timer = NULL;
b38b0f
+        switch (pbdev->state) {
b38b0f
+        case ZPCI_FS_STANDBY:
b38b0f
+        case ZPCI_FS_RESERVED:
b38b0f
+            s390_pci_perform_unplug(pbdev);
b38b0f
+            break;
b38b0f
+        default:
b38b0f
+            if (pbdev->release_timer) {
b38b0f
+                return;
b38b0f
+            }
b38b0f
+            s390_pci_generate_plug_event(HP_EVENT_DECONFIGURE_REQUEST,
b38b0f
+                                         pbdev->fh, pbdev->fid);
b38b0f
+            pbdev->release_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
b38b0f
+                                                s390_pcihost_timer_cb, pbdev);
b38b0f
+            timer_mod(pbdev->release_timer,
b38b0f
+                    qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + HOT_UNPLUG_TIMEOUT);
b38b0f
+        }
b38b0f
+    } else {
b38b0f
+        g_assert_not_reached();
b38b0f
     }
b38b0f
-
b38b0f
-    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
b38b0f
-                                 pbdev->fh, pbdev->fid);
b38b0f
-    bus = pci_get_bus(pci_dev);
b38b0f
-    devfn = pci_dev->devfn;
b38b0f
-    object_unparent(OBJECT(pci_dev));
b38b0f
-    s390_pci_msix_free(pbdev);
b38b0f
-    s390_pci_iommu_free(s, bus, devfn);
b38b0f
-    pbdev->pdev = NULL;
b38b0f
-    pbdev->state = ZPCI_FS_RESERVED;
b38b0f
-out:
b38b0f
-    pbdev->fid = 0;
b38b0f
-    QTAILQ_REMOVE(&s->zpci_devs, pbdev, link);
b38b0f
-    g_hash_table_remove(s->zpci_table, &pbdev->idx);
b38b0f
-    object_unparent(OBJECT(pbdev));
b38b0f
 }
b38b0f
 
b38b0f
 static void s390_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev,
b38b0f
@@ -1062,6 +1120,7 @@ static void s390_pcihost_class_init(ObjectClass *klass, void *data)
b38b0f
     dc->realize = s390_pcihost_realize;
b38b0f
     hc->pre_plug = s390_pcihost_pre_plug;
b38b0f
     hc->plug = s390_pcihost_plug;
b38b0f
+    hc->unplug_request = s390_pcihost_unplug_request;
b38b0f
     hc->unplug = s390_pcihost_unplug;
b38b0f
     msi_nonbroken = true;
b38b0f
 }
b38b0f
diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
b38b0f
index 1f7f9b5..7684658 100644
b38b0f
--- a/hw/s390x/s390-pci-bus.h
b38b0f
+++ b/hw/s390x/s390-pci-bus.h
b38b0f
@@ -308,6 +308,7 @@ struct S390PCIBusDevice {
b38b0f
     IndAddr *summary_ind;
b38b0f
     IndAddr *indicator;
b38b0f
     QEMUTimer *release_timer;
b38b0f
+    bool pci_unplug_request_processed;
b38b0f
     QTAILQ_ENTRY(S390PCIBusDevice) link;
b38b0f
 };
b38b0f
 
b38b0f
-- 
b38b0f
1.8.3.1
b38b0f