IOMMU: default to always quarantining PCI devices

XSA-302 relies on the use of libxl's "assignable-add" feature to prepare
devices to be assigned to untrusted guests.

Unfortunately, this is not considered a strictly required step for
device assignment. The PCI passthrough documentation on the wiki
describes alternate ways of preparing devices for assignment, and
libvirt uses its own ways as well. Hosts where these alternate methods
are used will still leave the system in a vulnerable state after the
device comes back from a guest.

Default to always quarantining PCI devices, but provide a command line
option to revert back to prior behavior (such that people who both
sufficiently trust their guests and want to be able to use devices in
Dom0 again after they had been in use by a guest wouldn't need to
"manually" move such devices back from DomIO to Dom0).

This is XSA-306.

Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Wei Liu <wl@xen.org>

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 0394d669c0cbc6415419f6a5a2a96d08cc971ef0..891d2d439fefa199413b753e8d7c261042ac9070 100644 (file)
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1219,7 +1219,7 @@ detection of systems known to misbehave upon accesses to that port.
 > Default: `new` unless directed-EOI is supported
 
 ### iommu
-    = List of [ <bool>, verbose, debug, force, required,
+    = List of [ <bool>, verbose, debug, force, required, quarantine,
                 sharept, intremap, intpost, crash-disable,
                 snoop, qinval, igfx, amd-iommu-perdev-intremap,
                 dom0-{passthrough,strict} ]
@@ -1257,6 +1257,12 @@ boolean (e.g. `iommu=no`) can override this and leave the IOMMUs disabled.
     will prevent Xen from booting if IOMMUs aren't discovered and enabled
     successfully.
 
+*   The `quarantine` boolean can be used to control Xen's behavior when
+    de-assigning devices from guests.  If enabled (the default), Xen always
+    quarantines such devices; they must be explicitly assigned back to Dom0
+    before they can be used there again.  If disabled, Xen will only
+    quarantine devices the toolstack hass arranged for getting quarantined.
+
 *   The `sharept` boolean controls whether the IOMMU pagetables are shared
     with the CPU-side HAP pagetables, or allocated separately.  Sharing
     reduces the memory overhead, but doesn't work in combination with CPU-side

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 8cbe908fff3318f78f51d1d22983fbe3adcc19ed..656fdeb093f0c0827795e28e373ce2094b686108 100644 (file)
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -30,6 +30,7 @@ bool_t __initdata iommu_enable = 1;
 bool_t __read_mostly iommu_enabled;
 bool_t __read_mostly force_iommu;
 bool_t __read_mostly iommu_verbose;
+bool __read_mostly iommu_quarantine = true;
 bool_t __read_mostly iommu_igfx = 1;
 bool_t __read_mostly iommu_snoop = 1;
 bool_t __read_mostly iommu_qinval = 1;
@@ -78,6 +79,8 @@ static int __init parse_iommu_param(const char *s)
         else if ( (val = parse_boolean("force", s, ss)) >= 0 ||
                   (val = parse_boolean("required", s, ss)) >= 0 )
             force_iommu = val;
+        else if ( (val = parse_boolean("quarantine", s, ss)) >= 0 )
+            iommu_quarantine = val;
         else if ( (val = parse_boolean("igfx", s, ss)) >= 0 )
             iommu_igfx = val;
         else if ( (val = parse_boolean("verbose", s, ss)) >= 0 )

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 8e501a79a88aaf65128154056d9d96a9f0c40287..cbd232c1312418405ab4b7faf0e19d3aa2822895 100644 (file)
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -922,7 +922,8 @@ static int deassign_device(struct domain *d, uint16_t seg, uint8_t bus,
         return -ENODEV;
 
     /* De-assignment from dom_io should de-quarantine the device */
-    target = (pdev->quarantine && pdev->domain != dom_io) ?
+    target = ((pdev->quarantine || iommu_quarantine) &&
+              pdev->domain != dom_io) ?
         dom_io : hardware_domain;
 
     while ( pdev->phantom_stride )

diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 974bd3ffe878c8e338b8de801157d5a356c4ab5b..fd45060e56f97fc4eba81aa7ba227944fd813c79 100644 (file)
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -53,7 +53,7 @@ static inline bool_t dfn_eq(dfn_t x, dfn_t y)
 }
 
 extern bool_t iommu_enable, iommu_enabled;
-extern bool_t force_iommu, iommu_verbose, iommu_igfx;
+extern bool force_iommu, iommu_quarantine, iommu_verbose, iommu_igfx;
 extern bool_t iommu_snoop, iommu_qinval, iommu_intremap, iommu_intpost;
 
 #if defined(CONFIG_IOMMU_FORCE_PT_SHARE)