8:23 a.m.
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(a)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