[libvirt] [PATCH] pci: Save and restore each devices/functions behind the bus

[Osier Yang]

Previously it refuses to do the secondary bus reset as long as
there is(are) devices/functions behind the same bus, regardless
of whether the devices/functions are being used or not. And it
only save and restore the device itself's PCI config space.

But later it was changed to allow the secondary bus reset as long
as the devices/functions behind the same bus are not being
used. Unfortunately, it still just saves and restores the device
itself's PCI config space. It means we will lose the PCI config
space for the devices share same bus when doing passthrough.

Also, (hope my guess is right) as it assumes the secondary reset
is allowed unless the device doesn't have devices/functions behind
the same bus, so it only reads the bridge control register from the
device, but not the parent.

This patch fixes the problem by finding out all the devices/functions
behind the same bus of the device to be reset, and save/restore
PCI config space for all of them. And read the bridge control register
from the device's parent (bridge) before resetting.

* src/util/pci.c:
  - New helper pciSharesBus to check if two devices share same bus.

  - New helper pciDevicesShareBus to return a list containg all of
    the devices/functions which share same bus with the device

  - pciTrySecondaryBusReset: Save and restore PCI config space for
    all the devices/functions behind the same bus; Read the bridge
    control register from the device's parent instead before resetting.
---
 src/util/pci.c |   80 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/src/util/pci.c b/src/util/pci.c
index 0742d07..1a9777a 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -517,6 +517,39 @@ pciBusContainsActiveDevices(pciDevice *dev,
     return active;
 }
 
+/*
+ * Check if the @dev and @check share bus.
+ */
+static int
+pciSharesBus(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED)
+{
+    if ((dev->domain == check->domain) &&
+        (dev->bus == check->bus) &&
+        (dev->slot == check->slot))
+        return 1;
+
+    return 0;
+}
+
+/*
+ * Return all the devices/functions share same bus with @dev
+ * as a list.
+ */
+static pciDeviceList *
+pciDevicesShareBus(pciDevice *dev)
+{
+    pciDevice *match = NULL;
+    pciDeviceList *pcis = NULL;
+
+    if (!(pcis = pciDeviceListNew()))
+        return NULL;
+
+    if (pciIterDevices(pciSharesBus, dev, &match, NULL))
+        pciDeviceListAdd(pcis, match);
+
+    return pcis;
+}
+
 /* Is @check the parent of @dev ? */
 static int
 pciIsParent(pciDevice *dev, pciDevice *check, void *data)
@@ -604,6 +637,9 @@ pciTrySecondaryBusReset(pciDevice *dev,
     uint8_t config_space[PCI_CONF_LEN];
     uint16_t ctl;
     int ret = -1;
+    pciDeviceList *list = NULL;
+    uint8_t (*config_spaces)[PCI_CONF_LEN];
+    int i;
 
     /* Refuse to do a secondary bus reset if there are other
      * devices/functions behind the bus are used by the host
@@ -628,10 +664,7 @@ pciTrySecondaryBusReset(pciDevice *dev,
 
     VIR_DEBUG("%s %s: doing a secondary bus reset", dev->id, dev->name);
 
-    /* Save and restore the device's config space; we only do this
-     * for the supplied device since we refuse to do a reset if there
-     * are multiple devices/functions
-     */
+    /* Save the device's config space */
     if (pciRead(dev, 0, config_space, PCI_CONF_LEN) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Failed to read PCI config space for %s"),
@@ -639,10 +672,29 @@ pciTrySecondaryBusReset(pciDevice *dev,
         goto out;
     }
 
+    /* Save the config space of devices behind the same bus  */
+    if ((list = pciDevicesShareBus(dev))) {
+        if (VIR_ALLOC_N(config_spaces, list->count) < 0) {
+            virReportOOMError();
+            goto out;
+        }
+
+        for (i = 0; i < list->count; i++) {
+            pciDevice *pci = list->devs[i];
+
+            if (pciRead(pci, 0, config_spaces[i], PCI_CONF_LEN) < 0) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Failed to read PCI config space for %s"),
+                               pci->name);
+                goto out;
+            }
+        }
+    }
+
     /* Read the control register, set the reset flag, wait 200ms,
      * unset the reset flag and wait 200ms.
      */
-    ctl = pciRead16(dev, PCI_BRIDGE_CONTROL);
+    ctl = pciRead16(parent, PCI_BRIDGE_CONTROL);
 
     pciWrite16(parent, PCI_BRIDGE_CONTROL, ctl | PCI_BRIDGE_CTL_RESET);
 
@@ -652,14 +704,32 @@ pciTrySecondaryBusReset(pciDevice *dev,
 
     usleep(200 * 1000); /* sleep 200ms */
 
+    /* Restore the device's config space */
     if (pciWrite(dev, 0, config_space, PCI_CONF_LEN) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Failed to restore PCI config space for %s"),
                        dev->name);
         goto out;
     }
+
+    /* Restore the config space of devices behind the same bus */
+    if (list) {
+        for (i = 0; i < list->count; i++) {
+            pciDevice *pci = list->devs[i];
+
+            if (pciWrite(pci, 0, config_spaces[i], PCI_CONF_LEN) < 0) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Failed to restore PCI config space for %s"),
+                               pci->name);
+                goto out;
+            }
+        }
+    }
+
     ret = 0;
 out:
+    pciDeviceListFree(list);
+    VIR_FREE(config_spaces);
     pciFreeDevice(parent);
     return ret;
 }
-- 
1.7.7.3