[Xen-devel] [PATCH] pci: a workaround for nonstandard PCI devices whose PBA shares

[Chao Gao <chao.gao@xxxxxxxxx>]

... the same page with other registers which are not relevant to MSI-X. Xen
marks pages where PBA resides as read-only. When assigning such devices to
guest, device driver writes MSI-X irrelevant registers on those pages would
lead to an EPT violation and the guest is destroyed because no handler is
registered for those address range. In order to make guest capable to use such
kind of devices, trapping very frequent write accesses is not a good idea for
it would significantly impact the performance.

This patch provides a workaround with caveat. Specifically, an option is
introduced to specify a list of devices. For those devices, Xen doesn't
control the access right to pages where PBA resides. Hence, guest device
driver is able to write those pages and functions well. Note that adding an
untrusted device to this option may endanger security of the entire system.

Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
---
 docs/misc/xen-command-line.markdown | 10 +++++++++
 xen/arch/x86/msi.c                  |  7 ++++--
 xen/drivers/passthrough/pci.c       | 45 +++++++++++++++++++++++++++++++++++--
 xen/include/asm-x86/msi.h           |  1 +
 4 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index b353352..e382513 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1423,6 +1423,16 @@ Defaults to booting secondary processors.
 
 > Default: `on`
 
+### pba\_quirk
+> `= List of [<seg>:]<bus>:<device>.<function>
+
+Specify a list of SBDF of devices. When assigning devices in this list to
+guest, reading or writing the page where MSI-X PBA resides are allowed.
+This option provides a workaround for nonstandard PCI devices whose
+MSI-X PBA shares the same 4K-byte page with other registers. Note that
+adding an untrusted device to this option would undermine security of
+the entire system.
+
 ### pci
 > `= {no-}serr | {no-}perr`
 
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 5567990..2abf2cf 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -992,7 +992,9 @@ static int msix_capability_init(struct pci_dev *dev,
         if ( rangeset_add_range(mmio_ro_ranges, msix->table.first,
                                 msix->table.last) )
             WARN();
-        if ( rangeset_add_range(mmio_ro_ranges, msix->pba.first,
+
+        if ( !msix->pba_quirk_enabled &&
+             rangeset_add_range(mmio_ro_ranges, msix->pba.first,
                                 msix->pba.last) )
             WARN();
 
@@ -1139,7 +1141,8 @@ static void _pci_cleanup_msix(struct arch_msix *msix)
         if ( rangeset_remove_range(mmio_ro_ranges, msix->table.first,
                                    msix->table.last) )
             WARN();
-        if ( rangeset_remove_range(mmio_ro_ranges, msix->pba.first,
+        if ( !msix->pba_quirk_enabled &&
+             rangeset_remove_range(mmio_ro_ranges, msix->pba.first,
                                    msix->pba.last) )
             WARN();
     }
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 1db69d5..cd765ef 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -184,6 +184,38 @@ static int __init parse_phantom_dev(const char *str)
 }
 custom_param("pci-phantom", parse_phantom_dev);
 
+static struct pba_quirk_dev {
+    uint32_t sbdf;
+} pba_quirk_devs[8];
+static unsigned int nr_pba_quirk_devs;
+
+static int __init parse_pba_quirk(const char *str)
+{
+    unsigned int seg, bus, dev, func;
+
+    for ( ; ; )
+    {
+        if ( nr_pba_quirk_devs >= ARRAY_SIZE(pba_quirk_devs) )
+            return -E2BIG;
+
+        str = parse_pci(str, &seg, &bus, &dev, &func);
+        if ( !str )
+            return -EINVAL;
+
+        pba_quirk_devs[nr_pba_quirk_devs++].sbdf = PCI_SBDF(seg, bus, dev,
+                                                            func);
+        if ( *str == ',' )
+            str++;
+        else if ( !*str )
+            break;
+        else
+            return -EINVAL;
+    }
+
+    return 0;
+}
+custom_param("pba_quirk", parse_pba_quirk);
+
 static u16 __read_mostly command_mask;
 static u16 __read_mostly bridge_ctl_mask;
 
@@ -300,6 +332,7 @@ static void check_pdev(const struct pci_dev *pdev)
 
 static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
 {
+    unsigned int i;
     struct pci_dev *pdev;
 
     list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
@@ -328,6 +361,16 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 
bus, u8 devfn)
         }
         spin_lock_init(&msix->table_lock);
         pdev->msix = msix;
+
+        for ( i = 0; i < nr_pba_quirk_devs; i++ )
+        {
+            if ( pba_quirk_devs[i].sbdf == PCI_SBDF3(pseg->nr, bus, devfn) )
+            {
+                pdev->msix->pba_quirk_enabled = true;
+                printk(XENLOG_WARNING "Enable PBA quirk for 
%04x:%02x:%02x.%u\n",
+                       pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+            }
+        }
     }
 
     list_add(&pdev->alldevs_list, &pseg->alldevs_list);
@@ -371,8 +414,6 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 
bus, u8 devfn)
             }
             else
             {
-                unsigned int i;
-
                 for ( i = 0; i < nr_phantom_devs; ++i )
                     if ( phantom_devs[i].seg == pseg->nr &&
                          phantom_devs[i].bus == bus &&
diff --git a/xen/include/asm-x86/msi.h b/xen/include/asm-x86/msi.h
index 10387dc..1645d1b 100644
--- a/xen/include/asm-x86/msi.h
+++ b/xen/include/asm-x86/msi.h
@@ -239,6 +239,7 @@ struct arch_msix {
     int table_idx[MAX_MSIX_TABLE_PAGES];
     spinlock_t table_lock;
     bool host_maskall, guest_maskall;
+    bool pba_quirk_enabled;
     domid_t warned;
 };
 
-- 
1.8.3.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel