[libvirt] [PATCH 06/10] hostdev: Check for safety before detaching VFIO devices

[Andrea Bolognani]

The newly-introduced virHostdevIsPCIDeviceSafeForDetach() function
checks against all problematic situations that can arise when detaching
a PCI device from the host for VFIO device assignment.

Some of the checks are simply reintroduced after being removed when
reworking virHostdevIsPCIDeviceUsed() earlier, but some are new and
allow us to detect an unsafe operation and abort immediately, instead of
calling QEMU and waiting for it to error out, which is nicer from the
user's point of view.
---
 src/util/virhostdev.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 77757c5..9174525 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -94,6 +94,67 @@ virHostdevIsPCIDeviceUsed(virPCIDevicePtr dev,
     return ret;
 }
 
+typedef struct {
+    const char *domname;
+    virPCIDeviceListPtr activeDevices;
+    virPCIDeviceListPtr inactiveDevices;
+    virPCIDeviceListPtr pendingDevices;
+} virHostdevIsPCIDeviceSafeData;
+
+/**
+ * virHostdevIsPCIDeviceSafe:
+ * @addr: PCI device address
+ * @opaque: user data (virHostdevIsPCIDeviceSafeData*)
+ *
+ * Checks whether a PCI device can be safely detached from the host for device
+ * assignment.
+ *
+ * It is meant to be called from virPCIDeviceAddressIOMMUGroupIterate() or
+ * virPCIDeviceIOMMUGroupIterate(), and the checks it performs only make
+ * sense when using VFIO device assignment.
+ *
+ * Returns: 0 if the device can be safely detached from the host, <0 otherwise
+ */
+static int
+virHostdevIsPCIDeviceSafeForDetach(virPCIDeviceAddressPtr addr,
+                                   void *opaque)
+{
+    virHostdevIsPCIDeviceSafeData *data = opaque;
+    virPCIDevicePtr other = NULL;
+
+    /* Pending devices are safe: they're the ones we're handling right now */
+    if (data->pendingDevices &&
+        virPCIDeviceListFindByIDs(data->pendingDevices,
+                                  addr->domain, addr->bus,
+                                  addr->slot, addr->function))
+        return 0;
+
+    /* Inactive devices are safe: they're already bound to a stub driver */
+    if (data->inactiveDevices &&
+        virPCIDeviceListFindByIDs(data->inactiveDevices,
+                                  addr->domain, addr->bus,
+                                  addr->slot, addr->function))
+        return 0;
+
+    /* Active devices are safe as long as they're used by the same domain */
+    if (data->activeDevices &&
+        (other = virPCIDeviceListFindByIDs(data->activeDevices,
+                                           addr->domain, addr->bus,
+                                           addr->slot, addr->function))) {
+        const char *other_domname = NULL;
+        const char *other_drvname = NULL;
+
+        virPCIDeviceGetUsedBy(other, &other_drvname, &other_domname);
+
+        if (STREQ_NULLABLE(data->domname, other_domname))
+            return 0;
+    }
+
+    /* If we got this far it means that the device is used by either another
+     * domain or the host, so it shouldn't be considered safe for assignment */
+    return -1;
+}
+
 static int virHostdevManagerOnceInit(void)
 {
     if (!(virHostdevManagerClass = virClassNew(virClassForObject(),
@@ -554,6 +615,10 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
 
     for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
         virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
+        virHostdevIsPCIDeviceSafeData data = { dom_name,
+                                               hostdev_mgr->activePCIHostdevs,
+                                               hostdev_mgr->inactivePCIHostdevs,
+                                               pcidevs };
         bool strict_acs_check = !!(flags & VIR_HOSTDEV_STRICT_ACS_CHECK);
         bool usesVfio = (virPCIDeviceGetStubDriver(dev) == VIR_PCI_STUB_DRIVER_VFIO);
 
@@ -567,6 +632,22 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
         /* Host devices can't be shared between domains */
         if (virHostdevIsPCIDeviceUsed(dev, hostdev_mgr))
             goto cleanup;
+
+        /* When using VFIO, we have to take care not to end up in a situation
+         * where some of the devices in a IOMMU group are in use by the host
+         * and some other are assigned to a domain. KVM will actually prevent
+         * that from happening, but we want to bail earlier and with a better
+         * error message */
+        if (usesVfio &&
+            virPCIDeviceIOMMUGroupIterate(dev,
+                                          virHostdevIsPCIDeviceSafeForDetach,
+                                          &data) < 0) {
+            virReportError(VIR_ERR_OPERATION_INVALID,
+                           _("PCI device %s is not assignable: "
+                             "attempting to share IOMMU ownership"),
+                           virPCIDeviceGetName(dev));
+            goto cleanup;
+        }
     }
 
     /* Loop 2: detach managed devices (i.e. bind to appropriate stub driver) */
-- 
2.5.0