vpci/msix: fix PBA accesses

Map the PBA in order to access it from the MSI-X read and write
handlers. Note that previously the handlers would pass the physical
host address into the {read,write}{l,q} handlers, which is wrong as
those expect a linear address.

Map the PBA using ioremap when the first access is performed. Note
that 32bit arches might want to abstract the call to ioremap into a
vPCI arch handler, so they can use a fixmap range to map the PBA.

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Alex Olson <Alex.Olson@starlab.io>
master commit: b4f21160601155762a4d014db9623af921fec959
master date: 2022-03-09 16:21:01 +0100

diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index 846f1b8d70383f15a35e4ef243c64b5e966acbe7..ac5de98f6dadf5558bb57643ad8a2805a1cb36d9 100644 (file)
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -182,6 +182,38 @@ static struct vpci_msix_entry *get_entry(struct vpci_msix *msix,
     return &msix->entries[(addr - start) / PCI_MSIX_ENTRY_SIZE];
 }
 
+static void __iomem *get_pba(struct vpci *vpci)
+{
+    struct vpci_msix *msix = vpci->msix;
+    /*
+     * PBA will only be unmapped when the device is deassigned, so access it
+     * without holding the vpci lock.
+     */
+    void __iomem *pba = read_atomic(&msix->pba);
+
+    if ( likely(pba) )
+        return pba;
+
+    pba = ioremap(vmsix_table_addr(vpci, VPCI_MSIX_PBA),
+                  vmsix_table_size(vpci, VPCI_MSIX_PBA));
+    if ( !pba )
+        return read_atomic(&msix->pba);
+
+    spin_lock(&vpci->lock);
+    if ( !msix->pba )
+    {
+        write_atomic(&msix->pba, pba);
+        spin_unlock(&vpci->lock);
+    }
+    else
+    {
+        spin_unlock(&vpci->lock);
+        iounmap(pba);
+    }
+
+    return read_atomic(&msix->pba);
+}
+
 static int msix_read(struct vcpu *v, unsigned long addr, unsigned int len,
                      unsigned long *data)
 {
@@ -200,6 +232,10 @@ static int msix_read(struct vcpu *v, unsigned long addr, unsigned int len,
 
     if ( VMSIX_ADDR_IN_RANGE(addr, msix->pdev->vpci, VPCI_MSIX_PBA) )
     {
+        struct vpci *vpci = msix->pdev->vpci;
+        unsigned int idx = addr - vmsix_table_addr(vpci, VPCI_MSIX_PBA);
+        const void __iomem *pba = get_pba(vpci);
+
         /*
          * Access to PBA.
          *
@@ -207,14 +243,22 @@ static int msix_read(struct vcpu *v, unsigned long addr, unsigned int len,
          * guest address space. If this changes the address will need to be
          * translated.
          */
+        if ( !pba )
+        {
+            gprintk(XENLOG_WARNING,
+                    "%pp: unable to map MSI-X PBA, report all pending\n",
+                    msix->pdev);
+            return X86EMUL_OKAY;
+        }
+
         switch ( len )
         {
         case 4:
-            *data = readl(addr);
+            *data = readl(pba + idx);
             break;
 
         case 8:
-            *data = readq(addr);
+            *data = readq(pba + idx);
             break;
 
         default:
@@ -278,14 +322,27 @@ static int msix_write(struct vcpu *v, unsigned long addr, unsigned int len,
         /* Ignore writes to PBA for DomUs, it's behavior is undefined. */
         if ( is_hardware_domain(d) )
         {
+            struct vpci *vpci = msix->pdev->vpci;
+            unsigned int idx = addr - vmsix_table_addr(vpci, VPCI_MSIX_PBA);
+            const void __iomem *pba = get_pba(vpci);
+
+            if ( !pba )
+            {
+                /* Unable to map the PBA, ignore write. */
+                gprintk(XENLOG_WARNING,
+                        "%pp: unable to map MSI-X PBA, write ignored\n",
+                        msix->pdev);
+                return X86EMUL_OKAY;
+            }
+
             switch ( len )
             {
             case 4:
-                writel(data, addr);
+                writel(data, pba + idx);
                 break;
 
             case 8:
-                writeq(data, addr);
+                writeq(data, pba + idx);
                 break;
 
             default:

diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 657697fe3406f70c228bdb127909e92489c0d9da..dfc8136ffb95f51445d1df1c865b5cb7d948f5ef 100644 (file)
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -51,6 +51,8 @@ void vpci_remove_device(struct pci_dev *pdev)
         xfree(r);
     }
     spin_unlock(&pdev->vpci->lock);
+    if ( pdev->vpci->msix && pdev->vpci->msix->pba )
+        iounmap(pdev->vpci->msix->pba);
     xfree(pdev->vpci->msix);
     xfree(pdev->vpci->msi);
     xfree(pdev->vpci);

diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index 9ea66e033f117858e2a0400ac8f51e4f0a6c8a03..755b4fd5c81cb199d260bc15201b1087db688b8c 100644 (file)
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -129,6 +129,8 @@ struct vpci {
         bool enabled         : 1;
         /* Masked? */
         bool masked          : 1;
+        /* PBA map */
+        void __iomem *pba;
         /* Entries. */
         struct vpci_msix_entry {
             uint64_t addr;