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

[PATCH] [RFC] vpci: allow BAR write while mapped


  • To: <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
  • Date: Wed, 12 Mar 2025 15:50:17 -0400
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=C9CuwYoZPsmM0ysqioe6mQ4s5vfv2OuUyv0T6a636ig=; b=GDMS6AqwW4gHLt8THCb9OVAWUui0k26kN6j1a9guLrWplqPbqFsFz2GJzH2ZoozkPEZZnIGhIgfwutlnnAB2GsOF6IdMMJs8wkfvBlUxsWQt4GXpnF2iEG72V1kpDHfTr2lixi3RfwY4PFfgZrlp5U8QaBrUFNR+N1GuQ5+EJR7G6E7Bn9AUe/FIpHCGw9UbDVaN+L6bAlqdYnIxKvv4hEmWFxXuupeijKXfa4igp/QcZEfPAaCWcwpX7+H0LJ6p14f0KKazgvI4XUKgPuvDDypHmZHJqCjDWc0zuFqowwBXKPRRoLu+QOwos01MelronxTMkQmtLRmOAG8oremK9Q==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=cbVZvsQFwnDLfYVWUeiWvjdX2S/mMmpZRDKV4XR8oTiyAg3ChqWcHKCDfgoq8ovVDjKQnsKyoQYEp8LgwM8nXePixUnOu+MnVtnjjPOSqRP8PBM+Q5QwqcqIQ0ON8EEnjwI4DqncXEQ2XDaHGQTkqaoxWmHwCJd1Gfm4PzPQVPOMod0G12wpNe/9jLjDuL8RMxAYiAas5AbPj6nrdkgN3hdgMMlCDn+Y8+02Xu/es5A/gSqD2piRzLRoE3ydV6Tk01hW/AAAeywRX04XZH9HE0Y0fjFDTAH07xtHveZ27nMv/CO11DxaTZru2HKQUPEjjkcYIQrHY6K/djygZe9fUw==
  • Cc: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Wed, 12 Mar 2025 19:50:45 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Xen vPCI refuses BAR writes if the BAR is mapped in p2m. If firmware
initialized the BAR to a bad address, Linux will try to write a new
address to the BAR without disabling memory decoding. Allow the write
by updating p2m right away in the vPCI BAR write handler.

Resolves: https://gitlab.com/xen-project/xen/-/issues/197
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
---
RFC: Currently the deferred mapping machinery supports only map or
     unmap, not both. It might be better to rework the mapping machinery
     to support unmap-then-map operations, but please let me know your
     thoughts.
RFC: This patch has not yet made an attempt to distinguish between
     32-bit and 64-bit writes. It probably should.
---
 xen/drivers/vpci/header.c | 65 +++++++++++++++++++++++++++++++--------
 1 file changed, 53 insertions(+), 12 deletions(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index ef6c965c081c..66adb2183cfe 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -173,7 +173,7 @@ static void modify_decoding(const struct pci_dev *pdev, 
uint16_t cmd,
         ASSERT_UNREACHABLE();
 }
 
-bool vpci_process_pending(struct vcpu *v)
+static bool process_pending(struct vcpu *v, bool need_lock)
 {
     struct pci_dev *pdev = v->vpci.pdev;
     struct vpci_header *header = NULL;
@@ -182,12 +182,14 @@ bool vpci_process_pending(struct vcpu *v)
     if ( !pdev )
         return false;
 
-    read_lock(&v->domain->pci_lock);
+    if ( need_lock )
+        read_lock(&v->domain->pci_lock);
 
     if ( !pdev->vpci || (v->domain != pdev->domain) )
     {
         v->vpci.pdev = NULL;
-        read_unlock(&v->domain->pci_lock);
+        if ( need_lock )
+            read_unlock(&v->domain->pci_lock);
         return false;
     }
 
@@ -209,17 +211,20 @@ bool vpci_process_pending(struct vcpu *v)
 
         if ( rc == -ERESTART )
         {
-            read_unlock(&v->domain->pci_lock);
+            if ( need_lock )
+                read_unlock(&v->domain->pci_lock);
             return true;
         }
 
         if ( rc )
         {
-            spin_lock(&pdev->vpci->lock);
+            if ( need_lock )
+                spin_lock(&pdev->vpci->lock);
             /* Disable memory decoding unconditionally on failure. */
             modify_decoding(pdev, v->vpci.cmd & ~PCI_COMMAND_MEMORY,
                             false);
-            spin_unlock(&pdev->vpci->lock);
+            if ( need_lock )
+                spin_unlock(&pdev->vpci->lock);
 
             /* Clean all the rangesets */
             for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
@@ -228,7 +233,8 @@ bool vpci_process_pending(struct vcpu *v)
 
             v->vpci.pdev = NULL;
 
-            read_unlock(&v->domain->pci_lock);
+            if ( need_lock )
+                read_unlock(&v->domain->pci_lock);
 
             if ( !is_hardware_domain(v->domain) )
                 domain_crash(v->domain);
@@ -238,15 +244,23 @@ bool vpci_process_pending(struct vcpu *v)
     }
     v->vpci.pdev = NULL;
 
-    spin_lock(&pdev->vpci->lock);
+    if ( need_lock )
+        spin_lock(&pdev->vpci->lock);
     modify_decoding(pdev, v->vpci.cmd, v->vpci.rom_only);
-    spin_unlock(&pdev->vpci->lock);
+    if ( need_lock )
+        spin_unlock(&pdev->vpci->lock);
 
-    read_unlock(&v->domain->pci_lock);
+    if ( need_lock )
+        read_unlock(&v->domain->pci_lock);
 
     return false;
 }
 
+bool vpci_process_pending(struct vcpu *v)
+{
+    return process_pending(v, true);
+}
+
 static int __init apply_map(struct domain *d, const struct pci_dev *pdev,
                             uint16_t cmd)
 {
@@ -565,6 +579,8 @@ static void cf_check bar_write(
 {
     struct vpci_bar *bar = data;
     bool hi = false;
+    bool reenable = false;
+    uint32_t cmd = 0;
 
     ASSERT(is_hardware_domain(pdev->domain));
 
@@ -585,10 +601,31 @@ static void cf_check bar_write(
     {
         /* If the value written is the current one avoid printing a warning. */
         if ( val != (uint32_t)(bar->addr >> (hi ? 32 : 0)) )
+        {
             gprintk(XENLOG_WARNING,
-                    "%pp: ignored BAR %zu write while mapped\n",
+                    "%pp: allowing BAR %zu write while mapped\n",
                     &pdev->sbdf, bar - pdev->vpci->header.bars + hi);
-        return;
+            ASSERT(rw_is_write_locked(&pdev->domain->pci_lock));
+            ASSERT(spin_is_locked(&pdev->vpci->lock));
+            reenable = true;
+            cmd = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
+            /*
+             * Write-while-mapped: unmap the old BAR in p2m. We want this to
+             * finish right away since the deferral machinery only supports
+             * unmap OR map, not unmap-then-remap. Ultimately, it probably 
would
+             * be better to defer the write-while-mapped case just like regular
+             * BAR writes (but still only allow it for 32-bit BAR writes).
+             */
+            /* Disable memory decoding */
+            modify_bars(pdev, cmd & ~PCI_COMMAND_MEMORY, false);
+            /* Call process pending here to ensure P2M operations are done */
+            while ( process_pending(current, false) )
+            {
+                /* Pre-empted, try again */
+            }
+        }
+        else
+            return;
     }
 
 
@@ -610,6 +647,10 @@ static void cf_check bar_write(
     }
 
     pci_conf_write32(pdev->sbdf, reg, val);
+
+    if ( reenable )
+        /* Write-while-mapped: map the new BAR in p2m. OK to defer. */
+        modify_bars(pdev, cmd, false);
 }
 
 static void cf_check guest_mem_bar_write(const struct pci_dev *pdev,

base-commit: 8e60d47cf0112c145b6b0e454d102b04c857db8c
-- 
2.48.1




 


Rackspace

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