[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] Allow a per-PCI passthrough device permissive attribute



Currently there is a global tag to let the administrator
turn off system-wide ACS checking when doing PCI device
passthrough.  However, this is too coarse-grained of an
attribute, since it doesn't allow setups where certain
guests are trusted while other ones are untrusted.  Allow
more complicated setups by making the device checking
a per-device setting.

The more detailed explanation of why this is necessary
delves deep into PCIe internals.  Ideally we'd like
to be able to probe devices and figure out whether it
is safe to assign them.  In practice, this isn't possible
because PCIe allows devices to have "hidden" bridges
that software can't discover.  If you were to have
two devices assigned to two different domains behind
one of these hidden bridges, they could do P2P traffic
and bypass all of the VT-d/IOMMU checks.

The next thing we could try to do is to have a whitelist
of devices that we know to be safe.  For instance, instead
of a "hidden" bridge, PCI devices can multiplex functions
instead, which causes all traffic to head to an upstream
bridge before P2P can take place.  Additionally, some
"hidden" PCI bridges may have ACS on-board.  In both of
these cases it's safe to passthrough the device(s), since
they can't P2P without the IOMMU getting involved.

However, even if we did have a whitelist, I think we still
need a permissive attribute.  For one thing, the whitelist
will always be out of date with respect to new hardware,
so we'd need to allow administrators to temporarily
override the whitelist restriction until a new version of
the whitelist came out.  Also, we want to support the case
where the administrator knows it is safe to assign possibly
unsafe devices to a domain he trusts.

Signed-off-by: Chris Lalancette <clalance redhat com>
---
 docs/schemas/domain.rng  |    6 ++++++
 src/conf/domain_conf.c   |   14 +++++++++++---
 src/conf/domain_conf.h   |    1 +
 src/libvirt_private.syms |    2 ++
 src/qemu/qemu_driver.c   |    1 +
 src/util/pci.c           |   13 ++++++++++++-
 src/util/pci.h           |    3 +++
 7 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index 827ff6f..9f90f4d 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -1175,6 +1175,12 @@
             <value>no</value>
           </choice>
         </attribute>
+        <attribute name="permissive">
+          <choice>
+            <value>yes</value>
+            <value>no</value>
+          </choice>
+        </attribute>
       </optional>
       <group>
         <element name="source">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e548d1d..899967d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2929,7 +2929,7 @@ virDomainHostdevDefParseXML(virConnectPtr conn,
 
     xmlNodePtr cur;
     virDomainHostdevDefPtr def;
-    char *mode, *type = NULL, *managed = NULL;
+    char *mode, *type = NULL, *managed = NULL, *permissive = NULL;
 
     if (VIR_ALLOC(def) < 0) {
         virReportOOMError(conn);
@@ -2968,6 +2968,13 @@ virDomainHostdevDefParseXML(virConnectPtr conn,
         VIR_FREE(managed);
     }
 
+    permissive = virXMLPropString(node, "permissive");
+    if (permissive != NULL) {
+        if (STREQ(permissive, "yes"))
+            def->permissive = 1;
+        VIR_FREE(permissive);
+    }
+
     cur = node->children;
     while (cur != NULL) {
         if (cur->type == XML_ELEMENT_NODE) {
@@ -5243,8 +5250,9 @@ virDomainHostdevDefFormat(virConnectPtr conn,
         return -1;
     }
 
-    virBufferVSprintf(buf, "    <hostdev mode='%s' type='%s' managed='%s'>\n",
-                      mode, type, def->managed ? "yes" : "no");
+    virBufferVSprintf(buf, "    <hostdev mode='%s' type='%s' managed='%s' permissive='%s'>\n",
+                      mode, type, def->managed ? "yes" : "no",
+                      def->permissive ? "yes" : "no");
     virBufferAddLit(buf, "      <source>\n");
 
     if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 7be090d..57ab4b4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -475,6 +475,7 @@ typedef virDomainHostdevDef *virDomainHostdevDefPtr;
 struct _virDomainHostdevDef {
     int mode; /* enum virDomainHostdevMode */
     unsigned int managed : 1;
+    unsigned int permissive : 1;
     union {
         struct {
             int type; /* enum virDomainHostdevBusType */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d56fb7d..49e222e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -441,6 +441,8 @@ pciWaitForDeviceCleanup;
 pciResetDevice;
 pciDeviceSetManaged;
 pciDeviceGetManaged;
+pciDeviceSetPermissive;
+pciDeviceGetPermissive;
 pciDeviceListNew;
 pciDeviceListFree;
 pciDeviceListAdd;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5bf6743..9848f1c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2137,6 +2137,7 @@ qemuGetPciHostDeviceList(virConnectPtr conn,
         }
 
         pciDeviceSetManaged(dev, hostdev->managed);
+        pciDeviceSetPermissive(dev, hostdev->permissive);
     }
 
     return list;
diff --git a/src/util/pci.c b/src/util/pci.c
index 0274806..42a0be6 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -64,6 +64,7 @@ struct _pciDevice {
     unsigned      has_flr : 1;
     unsigned      has_pm_reset : 1;
     unsigned      managed : 1;
+    unsigned      permissive : 1;
 };
 
 struct _pciDeviceList {
@@ -1076,6 +1077,16 @@ unsigned pciDeviceGetManaged(pciDevice *dev)
     return dev->managed;
 }
 
+void pciDeviceSetPermissive(pciDevice *dev, unsigned permissive)
+{
+    dev->permissive = !!permissive;
+}
+
+unsigned pciDeviceGetPermissive(pciDevice *dev)
+{
+    return dev->permissive;
+}
+
 pciDeviceList *
 pciDeviceListNew(virConnectPtr conn)
 {
@@ -1353,7 +1364,7 @@ int pciDeviceIsAssignable(virConnectPtr conn,
         return 0;
 
     if (ret) {
-        if (!strict_acs_check) {
+        if (!strict_acs_check || dev->permissive) {
             VIR_DEBUG("%s %s: strict ACS check disabled; device assignment allowed",
                       dev->id, dev->name);
         } else {
diff --git a/src/util/pci.h b/src/util/pci.h
index e6ab137..3f98374 100644
--- a/src/util/pci.h
+++ b/src/util/pci.h
@@ -44,6 +44,9 @@ int        pciResetDevice    (virConnectPtr  conn,
 void      pciDeviceSetManaged(pciDevice     *dev,
                               unsigned       managed);
 unsigned  pciDeviceGetManaged(pciDevice     *dev);
+void      pciDeviceSetPermissive(pciDevice  *dev,
+                                 unsigned    permissive);
+unsigned pciDeviceGetPermissive(pciDevice   *dev);
 
 pciDeviceList *pciDeviceListNew  (virConnectPtr conn);
 void           pciDeviceListFree (virConnectPtr conn,
-- 
1.6.6


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]