[RFC PATCH] iommu: make no-quarantine mean no-quarantine

[Scott Davis <scottwd@xxxxxxxxx>]

This patch modifies Xen's behavior when making devices assignable while the
iommu=no-quarantine command line option is in effect. Currently this option
only affects device deassignment, causing devices to get immediately assigned
back to Dom0 instead of to the quarantine dom_io domain. This patch extends
no-quarantine to device assignment as well, preventing devices from being
assigned to dom_io when they are made assignable while no-quarantine is in
effect.

Signed-off-by: Scott Davis <scott.davis@xxxxxxxxxx>
---
First patch submission, apologies in advance for any formatting or other errors.

Background: I am setting up a QEMU-based development and testing environment for
the Crucible team at Star Lab that includes emulated PCIe devices for
passthrough and hotplug. I encountered an issue with `xl pci-assignable-add`
that causes the host QEMU to rapidly allocate memory until getting OOM-killed.
I then found that the issue could be worked around either by using manual sysfs
commands to rebind devices to pciback or by skipping over the quarantine logic
in `libxl__device_pci_assignable_add`, producing a working system. I hoped that
setting iommu=no-quarantine on the command line would have the same effect, only
to be surprised that it did not.

This patch fixes the issue I encountered and, in my opinion, makes the behavior
of no-quarantine more intuitive. While I intend to root-cause whatever
interaction between Xen and QEMU's VT-d vIOMMU is causing the problem, I plan to
carry this patch, or one like it, in the meantime. I'd welcome any feedback on
its design or intent. Note that this RFC version is based and tested on
Crucible's 4.14.1 branch of Xen, but I will rebase and test on staging if it's
desired upstream in some form.
---
 docs/misc/xen-command-line.pandoc | 8 ++++----
 xen/drivers/passthrough/pci.c     | 9 +++++++--
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc 
b/docs/misc/xen-command-line.pandoc
index 4ae9391fcd..1015f95dd5 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1317,10 +1317,10 @@ boolean (e.g. `iommu=no`) can override this and leave 
the IOMMUs disabled.
     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.
+    making devices assignable to guests and de-assigning devices from guests.
+    If enabled (the default), Xen will quarantine such devices; they must be
+    explicitly assigned back to Dom0 before they can be used there again.  If
+    disabled, Xen will not quarantine devices.

 *   The `sharept` boolean controls whether the IOMMU pagetables are shared
     with the CPU-side HAP pagetables, or allocated separately.  Sharing
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 2d6238a5bb..4c37f2d272 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -935,9 +935,14 @@ static int deassign_device(struct domain *d, uint16_t seg, 
uint8_t bus,
         return -EINVAL;

     ASSERT(pcidevs_locked());
-    pdev = pci_get_pdev_by_domain(d, seg, bus, devfn);
+    pdev = pci_get_pdev(seg, bus, devfn);
     if ( !pdev )
         return -ENODEV;
+    else if ( d == dom_io && pdev->domain != dom_io )
+        /* OK: Request to de-quarantine a device that is not quarantined */
+        return 0;
+    else if ( pdev->domain != d )
+        return -ENODEV;

     /* De-assignment from dom_io should de-quarantine the device */
     target = ((pdev->quarantine || iommu_quarantine) &&
@@ -1510,7 +1515,7 @@ static int assign_device(struct domain *d, u16 seg, u8 
bus, u8 devfn, u32 flag)
     struct pci_dev *pdev;
     int rc = 0;

-    if ( !is_iommu_enabled(d) )
+    if ( !is_iommu_enabled(d) || (d == dom_io && !iommu_quarantine) )
         return 0;

     /* Prevent device assign if mem paging or mem sharing have been
--
2.25.1