[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] vpci/msix: fix PBA accesses


  • To: <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
  • Date: Fri, 25 Feb 2022 12:46:47 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=yx4r1lSZIl1pe2SrW52IVtMxwtt6xWeMduyLUDwhOFc=; b=ZycM94aGaz/GDb7qsf3y60aUp6NrJPKDTjYQbG4o3sneLguMlLzr7GxSxz7P99s+NietpOAnEAkPcFpTqzptNRdazSihZsjWjIsoptK5vA9RrDkJrEhQ8n4aWALKQw2XjNpbxk/dz+m6X7nWog9WALzY/kEanTY1wg6aHn+N7t6llwcrx1LyyTjg2h2aTXNy0u8kpOb0QGAZcPA9DqL5S2WduD68BAmiMp4CiNuD+qK415CdMpAlAQ98hyhDduUHzuqYYZBh4QB4YI1PHFPZn6kHBRQ3skKdEAhwFKi31FTxm+Y6uqOns4U2xCTZqnM42AG9Tj/tbiB72JxilPk33g==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RQ7B3ukD7yRi8vctcmCEY/+ppIS1yXl5Ew9TypzlYcorf73Zft2gzwEGAr2TNj1XOVBprx+fw+rmMA6wUpSAeMkzwwLcFi88/Ri3NRkfS5Ikz6rWaoPHx6Oykaq3CGHkVGgCgzXgTmD1WGABxbC73pNd9QAD+APcHp5H8ERclYZNJxJrgX7t2Zg8JwmccefXii5ceA1ps0IA3MOcWPvR4n+dKI6E0y6ppbAn0R+pubTJSeFdePQx01X2Jo70qhhY90Y0hH0oBhPHIXitd/YcBYJ6zZdCScH3Nfqnw/eucQdbObB5zgQ3dvPwTrrdMwlOs8C0cP7L9vg3BF0QV5zQRg==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>
  • Delivery-date: Fri, 25 Feb 2022 11:47:27 +0000
  • Ironport-data: A9a23:O3EBhKKs4Al757lWFE+R05UlxSXFcZb7ZxGr2PjKsXjdYENShGdTn DFKDG7QbKveamX0c90jYdzk808GvcXQm4VkHFFlqX01Q3x08seUXt7xwmUcns+xwm8vaGo9s q3yv/GZdJhcokf0/0vrav67xZVF/fngqoDUUYYoAQgsA148IMsdoUg7wbRh2NQ12YLR7z6l4 rseneWOYDdJ5BYsWo4kw/rrRMRH5amaVJsw5zTSVNgT1LPsvyB94KE3fMldG0DQUIhMdtNWc s6YpF2PEsE1yD92Yj+tuu6TnkTn2dc+NyDW4pZdc/DKbhSvOkXee0v0XRYRQR4/ttmHozx+4 OpLm6ybYwgzBOrRhO03fB1BIyZzAZQTrdcrIVDn2SCS50jPcn+qyPRyFkAme4Yf/46bA0kXq 6ZecmpUKEne2aTmm9pXScE17ignBNPsM44F/Glp0BnSDOo8QICFSKLPjTNd9Gls15ATRquHD yYfQSg0agnpbiQeBnEGUo4TvP+4vXfUVgQN/Tp5ooJoujOOnWSdyoPFMtDYZ9iLTsV9hVuDq yTN+GGRKgEXMpmTxCSI9lqoh/TThmXrVYQKDrq6+/V2xlqJyQQu5AY+DAXh56Pj0wjnBowZe xd8FjcSQbYay0qFfvLXUAeC52OYtzQzX8NgFOAG0VTYokbL2DqxCm8BRz9HTdUpss4qWDAnv mO0c8PV6S9H6+PMFy/EnluAhXbrYHVOczdeDcMRZVZdu7HeTJcPYgUjpzqJOIq8lZXLFD752 FhmRwBu1uxI3abnO0hWlG0rYg5ARLCVH2bZBS2NBwpJCz+Vgqb/NuREDnCBsJ59wH6xFAXpg ZT9s5H2ABoyJZ+MjjeRZ+4GAauk4f2IWBWF3wIyQMV4q2n1oyb7FWy13N2YDB04WirjUWW0C HI/RCsLvMMDVJdURfUfj32N5zQCkvG7SIWNugH8ZdtSeJlhHDJrDwk1DXN8K1vFyRB2+YlmY M/zWZ/1UR4yVPQ2pBLrFrx1+eJ6mUgDKZb7GMmTI+KPiuHFOhZ4iN4tbTOzUwzOxPjc8VWNr o0GbJPiJtc2eLSWXxQ7OLU7dDgiBXM6GYr3u4pQcOuCKRBhA2YvF7naxrZJRmCvt/09ejvgl p1lZnJl9Q==
  • Ironport-hdrordr: A9a23:HNVGa60VKQdNEG4iYLxt8AqjBLYkLtp133Aq2lEZdPUzSL3+qy nOpoV+6faQsl0ssR4b9exoVJPufZq+z/5ICOsqU4tKNTOO0AHEEGgI1+rf6gylNyri9vNMkY dMGpIObeEY1GIK7voSNjPIceod/A==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Map the PBA in order to access it from the MSI-X read handler. Note
that previously the handler will pass the physical host address into
the read{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@xxxxxxxx>
Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
I don't seem to have a box with a driver that will try to access the
PBA, so I would consider this specific code path only build tested. At
least it doesn't seem to regress the current state of vPCI.
---
 xen/drivers/vpci/msix.c | 28 +++++++++++++++++++++++++---
 xen/drivers/vpci/vpci.c |  2 ++
 xen/include/xen/vpci.h  |  2 ++
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index 846f1b8d70..70085e98b9 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -198,8 +198,13 @@ static int msix_read(struct vcpu *v, unsigned long addr, 
unsigned int len,
     if ( !access_allowed(msix->pdev, addr, len) )
         return X86EMUL_OKAY;
 
+    spin_lock(&msix->pdev->vpci->lock);
     if ( VMSIX_ADDR_IN_RANGE(addr, msix->pdev->vpci, VPCI_MSIX_PBA) )
     {
+        struct vpci *vpci = msix->pdev->vpci;
+        paddr_t base = vmsix_table_addr(vpci, VPCI_MSIX_PBA);
+        unsigned int idx = addr - base;
+
         /*
          * Access to PBA.
          *
@@ -207,25 +212,42 @@ 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 ( !msix->pba )
+        {
+            msix->pba = ioremap(base, vmsix_table_size(vpci, VPCI_MSIX_PBA));
+            if ( !msix->pba )
+            {
+                /*
+                 * If unable to map the PBA return all 1s (all pending): it's
+                 * likely better to trigger spurious events than drop them.
+                 */
+                spin_unlock(&vpci->lock);
+                gprintk(XENLOG_WARNING, "%pp: unable to map MSI-X PBA\n",
+                        msix->pdev);
+                return X86EMUL_OKAY;
+           }
+        }
+
         switch ( len )
         {
         case 4:
-            *data = readl(addr);
+            *data = readl(msix->pba + idx);
             break;
 
         case 8:
-            *data = readq(addr);
+            *data = readq(msix->pba + idx);
             break;
 
         default:
             ASSERT_UNREACHABLE();
             break;
         }
+        spin_unlock(&vpci->lock);
 
         return X86EMUL_OKAY;
     }
 
-    spin_lock(&msix->pdev->vpci->lock);
     entry = get_entry(msix, addr);
     offset = addr & (PCI_MSIX_ENTRY_SIZE - 1);
 
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index fb0947179b..f8674f3ea8 100644
--- 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 e8ac1eb395..bf93fc8d22 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -127,6 +127,8 @@ struct vpci {
         bool enabled         : 1;
         /* Masked? */
         bool masked          : 1;
+        /* PBA map */
+        void *pba;
         /* Entries. */
         struct vpci_msix_entry {
             uint64_t addr;
-- 
2.34.1




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.