[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
On Wed, Jan 27, 2010 at 09:23:52AM -0500, Chris Lalancette wrote:
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.
A domain is only trusted until its guest OS gets exploited at which point this proposed change may let it escape into the host. If you don't have any IOMMU on your host, you can't use PCI device assignment with KVM at all, because it would not be safe in the event of guest exploit / mis- behavior. The same is true of device assignment in this non-ACS + hidden bridge case. Thus I don't see why we should introduce a special "permissive" flag solely for the non-ACS edge case, while at the smae time not allowing the same permissiveness for the far more common non-IOMMU case. NB, I'm not suggesting we allow skipping of the checks for the non-IOMMU case either. Keeping a whitelist of devices up2date wrt new hardware launches is no more troublesome than the existing problem of updating the PCI-IDs databse or actually providing updated kernel releases with new drivers. If we use the permissive attribute, then every admin with a device that is known to be safe has the pain of setting the permissive attribute, every time, on every machine with this hardware. If we have a whitelist, then 99% of the time everything will just work because it will already be known to the whitelist. If the whitelist were an external datafile the admin could even extend it in the rare occasion when a new device were not known. This is a choice of make everyone solve over & over again themselves, or solve it once for everybody. I don't really like the idea of a whitelist, but I like it more than just pushing the problem onto admins via per guest flags. For that matter I don't like the host level flag we have either and would rather we removed it. If only there's a 3rd way that were neither flags or whitelists ... Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 01/27/2010 01:49 PM, Daniel P. Berrange wrote:
On Wed, Jan 27, 2010 at 09:23:52AM -0500, Chris Lalancette wrote:
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.
A domain is only trusted until its guest OS gets exploited at which point this proposed change may let it escape into the host. If you don't have any IOMMU on your host, you can't use PCI device assignment with KVM at all, because it would not be safe in the event of guest exploit / mis- behavior. The same is true of device assignment in this non-ACS + hidden bridge case.
While this is true, I don't think this is the sort of policy we should be enforcing in libvirt. Witness the users who still use Xen PV PCI passthrough, despite the fact that is unsafe. We still support them. If KVM had the ability to do non-IOMMU passthrough, I would think we would want to allow that usage through libvirt. Should we be safe by default? Absolutely. But should we give administrators enough rope to hang themselves? Yes, it's the Unix way.
Thus I don't see why we should introduce a special "permissive" flag solely for the non-ACS edge case, while at the smae time not allowing the same permissiveness for the far more common non-IOMMU case. NB, I'm not suggesting we allow skipping of the checks for the non-IOMMU case either.
Keeping a whitelist of devices up2date wrt new hardware launches is no more troublesome than the existing problem of updating the PCI-IDs databse or actually providing updated kernel releases with new drivers.
If we use the permissive attribute, then every admin with a device that is known to be safe has the pain of setting the permissive attribute, every time, on every machine with this hardware. If we have a whitelist, then 99% of the time everything will just work because it will already be known to the whitelist. If the whitelist were an external datafile the admin could even extend it in the rare occasion when a new device were not known. This is a choice of make everyone solve over & over again themselves, or solve it once for everybody.
I don't really like the idea of a whitelist, but I like it more than just pushing the problem onto admins via per guest flags. For that matter I don't like the host level flag we have either and would rather we removed it. If only there's a 3rd way that were neither flags or whitelists ...
The problem that I tried (and failed) to explain in the commit message is that even with a whitelist, it is not sufficient. The problem is that with multi-function PCI devices, software cannot tell whether it is possible to do P2P traffic. In the case that a manufacturer tells us "no, our device doesn't allow P2P traffic", then great, we can add it to the whitelist. But if a manufacturer can't or won't tell us that, then we have to assume it can do P2P traffic, and block it. But these multi-function PCI devices are *exactly* the devices we want to passthrough to guests (all multi-port NICs we have tested are multi-function). So even with the whitelist, you still have to leave a way for the admin to override the check. -- Chris Lalancette
On Thu, Jan 28, 2010 at 08:36:02AM -0500, Chris Lalancette wrote:
On 01/27/2010 01:49 PM, Daniel P. Berrange wrote:
On Wed, Jan 27, 2010 at 09:23:52AM -0500, Chris Lalancette wrote:
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.
A domain is only trusted until its guest OS gets exploited at which point this proposed change may let it escape into the host. If you don't have any IOMMU on your host, you can't use PCI device assignment with KVM at all, because it would not be safe in the event of guest exploit / mis- behavior. The same is true of device assignment in this non-ACS + hidden bridge case.
While this is true, I don't think this is the sort of policy we should be enforcing in libvirt. Witness the users who still use Xen PV PCI passthrough, despite the fact that is unsafe. We still support them. If KVM had the ability to do non-IOMMU passthrough, I would think we would want to allow that usage through libvirt. Should we be safe by default? Absolutely. But should we give administrators enough rope to hang themselves? Yes, it's the Unix way.
Since KVM does not provide for running without an IOMMU, I don't think we should be allowing bypass of the IOMMU in one small edge case. Particularly since the rules on when it is safe to do so are soo obscure that no sysadmin can realistically make a well informed decision. There are plenty of other constraints in PCI device assignment that libvirt is already checking and enforcing that we don't allow bypass of too.
Thus I don't see why we should introduce a special "permissive" flag solely for the non-ACS edge case, while at the smae time not allowing the same permissiveness for the far more common non-IOMMU case. NB, I'm not suggesting we allow skipping of the checks for the non-IOMMU case either.
Keeping a whitelist of devices up2date wrt new hardware launches is no more troublesome than the existing problem of updating the PCI-IDs databse or actually providing updated kernel releases with new drivers.
If we use the permissive attribute, then every admin with a device that is known to be safe has the pain of setting the permissive attribute, every time, on every machine with this hardware. If we have a whitelist, then 99% of the time everything will just work because it will already be known to the whitelist. If the whitelist were an external datafile the admin could even extend it in the rare occasion when a new device were not known. This is a choice of make everyone solve over & over again themselves, or solve it once for everybody.
I don't really like the idea of a whitelist, but I like it more than just pushing the problem onto admins via per guest flags. For that matter I don't like the host level flag we have either and would rather we removed it. If only there's a 3rd way that were neither flags or whitelists ...
The problem that I tried (and failed) to explain in the commit message is that even with a whitelist, it is not sufficient. The problem is that with multi-function PCI devices, software cannot tell whether it is possible to do P2P traffic. In the case that a manufacturer tells us "no, our device doesn't allow P2P traffic", then great, we can add it to the whitelist. But if a manufacturer can't or won't tell us that, then we have to assume it can do P2P traffic, and block it. But these multi-function PCI devices are *exactly* the devices we want to passthrough to guests (all multi-port NICs we have tested are multi-function). So even with the whitelist, you still have to leave a way for the admin to override the check.
This is all coming down to 'its hard, lets just have the user decide' which is not a satisfactory strategy here. New multi-function devices will have drivers written & submitted and if they want them used it is not unreasonable to expect the company providing drivers to indicate whether the devices allows P2P traffic or now. Or for enterprise OS vendors to determine this, when deciding whether to support the hardware in their distro. I accept it can't be discovered from hardware inquiry, but that doesn't stop this being indicated in other ways, kernel drivers exposing flags, or maintaining a whitelist of PCI IDs. If an admin desperately needs to override this, a whitelist still allows them to add a further PCI ID in a single place, rather than changing *every* guest XML that uses the device. This is fundamentally something that needs to be tracked against the device's PCI ID, and not the guest config. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (2)
-
Chris Lalancette -
Daniel P. Berrange