From d1ce5b3af38aa7d4eb9d3a2dd90a7572a69f7d41 Mon Sep 17 00:00:00 2001

From: Cornelia Huck <cohuck@redhat.com>

Date: Wed, 17 Apr 2019 13:57:34 +0100

Subject: [PATCH 17/24] s390x/pci: Drop release timer and replace it with a

 flag

RH-Author: Cornelia Huck <cohuck@redhat.com>

Message-id: <20190417135741.25297-18-cohuck@redhat.com>

Patchwork-id: 85798

O-Subject: [RHEL-8.1.0 qemu-kvm PATCH v2 17/24] s390x/pci: Drop release timer and replace it with a flag

Bugzilla: 1699070

RH-Acked-by: David Hildenbrand <david@redhat.com>

RH-Acked-by: Thomas Huth <thuth@redhat.com>

RH-Acked-by: Jens Freimann <jfreimann@redhat.com>

From: David Hildenbrand <david@redhat.com>

Let's handle it similar to x86 ACPI PCI code and don't use a timer.

Instead, remember if an unplug request is pending and keep it pending

for eternity. (a follow up patch will process the request on

reboot).

We expect that a guest that is up and running, will process the unplug

request and trigger the unplug. This is normal operation, no timer needed.

If the guest does not react, this usually means something in the guest

is going wrong. Simply removing the device after 30 seconds does not

really sound like a good idea. It might sometimes be wanted, but I

consider this rather an "opt-in" decision as it might harm a guest not

prepared for it.

If we ever actually want a "forced/surprise removal", we will have to

implement something on top of the existing "device_del" framework. E.g.

also x86 might want to do a forced/surprise removal of PCI devices under

some conditions. "device_del X, forced=true" could be an option and will

require changes to the hotplug handler infrastructure.

This will then move the responsibility on when to do a forced removal

to a higher level. Doing a forced removal right now over-complicates

things and doesn't really seem to be required.

Let's allow to send multiple requests.

Signed-off-by: David Hildenbrand <david@redhat.com>

Message-Id: <20190130155733.32742-6-david@redhat.com>

Reviewed-by: Collin Walling <walling@linux.ibm.com>

Signed-off-by: Cornelia Huck <cohuck@redhat.com>

(cherry picked from commit 9f2a46b11139cd21c41f4d97c0416af6f9e76f7b)

Signed-off-by: Cornelia Huck <cohuck@redhat.com>

Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>

---

 hw/s390x/s390-pci-bus.c | 38 +++++++-------------------------------

 hw/s390x/s390-pci-bus.h |  3 +--

 2 files changed, 8 insertions(+), 33 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c

index 21419df..97d3eb8 100644

--- a/hw/s390x/s390-pci-bus.c

+++ b/hw/s390x/s390-pci-bus.c

@@ -194,7 +194,7 @@ void s390_pci_sclp_deconfigure(SCCB *sccb)

         pbdev->state = ZPCI_FS_STANDBY;

         rc = SCLP_RC_NORMAL_COMPLETION;

-        if (pbdev->release_timer) {

+        if (pbdev->unplug_requested) {

             s390_pci_perform_unplug(pbdev);

         }

     }

@@ -963,23 +963,6 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,

     }

 }

-static void s390_pcihost_timer_cb(void *opaque)

-{

-    S390PCIBusDevice *pbdev = opaque;

-

-    if (pbdev->summary_ind) {

-        pci_dereg_irqs(pbdev);

-    }

-    if (pbdev->iommu->enabled) {

-        pci_dereg_ioat(pbdev->iommu);

-    }

-

-    pbdev->state = ZPCI_FS_STANDBY;

-    s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,

-                                 pbdev->fh, pbdev->fid);

-    s390_pci_perform_unplug(pbdev);

-}

-

 static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,

                                 Error **errp)

 {

@@ -1006,12 +989,6 @@ static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,

         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);

@@ -1058,15 +1035,14 @@ static void s390_pcihost_unplug_request(HotplugHandler *hotplug_dev,

             s390_pci_perform_unplug(pbdev);

             break;

         default:

-            if (pbdev->release_timer) {

-                return;

-            }

+            /*

+             * Allow to send multiple requests, e.g. if the guest crashed

+             * before releasing the device, we would not be able to send

+             * another request to the same VM (e.g. fresh OS).

+             */

+            pbdev->unplug_requested = true;

             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();

diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h

index 7684658..3eae782 100644

--- a/hw/s390x/s390-pci-bus.h

+++ b/hw/s390x/s390-pci-bus.h

@@ -35,7 +35,6 @@

 #define ZPCI_MAX_UID 0xffff

 #define UID_UNDEFINED 0

 #define UID_CHECKING_ENABLED 0x01

-#define HOT_UNPLUG_TIMEOUT (NANOSECONDS_PER_SECOND * 60 * 5)

 #define S390_PCI_HOST_BRIDGE(obj) \

     OBJECT_CHECK(S390pciState, (obj), TYPE_S390_PCI_HOST_BRIDGE)

@@ -307,8 +306,8 @@ struct S390PCIBusDevice {

     MemoryRegion msix_notify_mr;

     IndAddr *summary_ind;

     IndAddr *indicator;

-    QEMUTimer *release_timer;

     bool pci_unplug_request_processed;

+    bool unplug_requested;

     QTAILQ_ENTRY(S390PCIBusDevice) link;

 };

-- 

1.8.3.1