[xen master] vpci: refuse BAR writes only if the BAR is mapped

[patchbot@xxxxxxx]

commit 7abd7bc1626d25ada03c1cff2e8c2ce1a5cc3cbf
Author:     Roger Pau Monné <roger.pau@xxxxxxxxxx>
AuthorDate: Fri Oct 28 11:40:45 2022 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Fri Oct 28 11:40:45 2022 +0200

    vpci: refuse BAR writes only if the BAR is mapped
    
    Writes to the BARs are ignored if memory decoding is enabled for the
    device, and the same happen with ROM BARs if the write is an attempt
    to change the position of the BAR without disabling it first.
    
    The reason of ignoring such writes is a limitation in Xen, as it would
    need to unmap the BAR, change the address, and remap the BAR at the
    new position, which the current logic doesn't support.
    
    Some devices however seem to (wrongly) have the memory decoding bit
    hardcoded to enabled, and attempts to disable it don't get reflected
    on the command register.
    
    This causes issues for well behaved domains that disable memory
    decoding and then try to size the BARs, as vPCI will think memory
    decoding is still enabled and ignore the write.
    
    Since vPCI doesn't explicitly care about whether the memory decoding
    bit is disabled as long as the BAR is not mapped in the domain p2m use
    the information in the vpci_bar to check whether the BAR is mapped,
    and refuse writes only based on that information.  This workarounds
    the issue, and allows domains to size and reposition the BARs properly.
    
    Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
    Release-acked-by: Henry Wang <Henry.Wang@xxxxxxx>
---
 xen/drivers/vpci/header.c | 31 +++++++++++++++++++++----------
 xen/include/xen/vpci.h    |  6 ++++++
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index d272b3f343..ec2e978a4e 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -131,7 +131,10 @@ static void modify_decoding(const struct pci_dev *pdev, 
uint16_t cmd,
     }
 
     if ( !rom_only )
+    {
         pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
+        header->bars_mapped = map;
+    }
     else
         ASSERT_UNREACHABLE();
 }
@@ -352,13 +355,13 @@ static int modify_bars(const struct pci_dev *pdev, 
uint16_t cmd, bool rom_only)
 static void cf_check cmd_write(
     const struct pci_dev *pdev, unsigned int reg, uint32_t cmd, void *data)
 {
-    uint16_t current_cmd = pci_conf_read16(pdev->sbdf, reg);
+    struct vpci_header *header = data;
 
     /*
      * Let Dom0 play with all the bits directly except for the memory
      * decoding one.
      */
-    if ( (cmd ^ current_cmd) & PCI_COMMAND_MEMORY )
+    if ( header->bars_mapped != !!(cmd & PCI_COMMAND_MEMORY) )
         /*
          * Ignore the error. No memory has been added or removed from the p2m
          * (because the actual p2m changes are deferred in defer_map) and the
@@ -385,12 +388,16 @@ static void cf_check bar_write(
     else
         val &= PCI_BASE_ADDRESS_MEM_MASK;
 
-    if ( pci_conf_read16(pdev->sbdf, PCI_COMMAND) & PCI_COMMAND_MEMORY )
+    /*
+     * Xen only cares whether the BAR is mapped into the p2m, so allow BAR
+     * writes as long as the BAR is not mapped into the p2m.
+     */
+    if ( bar->enabled )
     {
         /* 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 with memory decoding 
enabled\n",
+                    "%pp: ignored BAR %zu write while mapped\n",
                     &pdev->sbdf, bar - pdev->vpci->header.bars + hi);
         return;
     }
@@ -419,25 +426,29 @@ static void cf_check rom_write(
 {
     struct vpci_header *header = &pdev->vpci->header;
     struct vpci_bar *rom = data;
-    uint16_t cmd = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
     bool new_enabled = val & PCI_ROM_ADDRESS_ENABLE;
 
-    if ( (cmd & PCI_COMMAND_MEMORY) && header->rom_enabled && new_enabled )
+    /*
+     * See comment in bar_write(). Additionally since the ROM BAR has an enable
+     * bit some writes are allowed while the BAR is mapped, as long as the
+     * write is to unmap the ROM BAR.
+     */
+    if ( rom->enabled && new_enabled )
     {
         gprintk(XENLOG_WARNING,
-                "%pp: ignored ROM BAR write with memory decoding enabled\n",
+                "%pp: ignored ROM BAR write while mapped\n",
                 &pdev->sbdf);
         return;
     }
 
-    if ( !header->rom_enabled )
+    if ( !rom->enabled )
         /*
-         * If the ROM BAR is not enabled update the address field so the
+         * If the ROM BAR is not mapped update the address field so the
          * correct address is mapped into the p2m.
          */
         rom->addr = val & PCI_ROM_ADDRESS_MASK;
 
-    if ( !(cmd & PCI_COMMAND_MEMORY) || header->rom_enabled == new_enabled )
+    if ( !header->bars_mapped || rom->enabled == new_enabled )
     {
         /* Just update the ROM BAR field. */
         header->rom_enabled = new_enabled;
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index 67c9a0c631..d8acfeba8a 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -88,6 +88,12 @@ struct vpci {
          * is mapped into guest p2m) if there's a ROM BAR on the device.
          */
         bool rom_enabled      : 1;
+        /*
+         * Cache whether memory decoding is enabled from our PoV.
+         * Some devices have a sticky memory decoding so that can't be relied
+         * upon to know whether BARs are mapped into the guest p2m.
+         */
+        bool bars_mapped      : 1;
         /* FIXME: currently there's no support for SR-IOV. */
     } header;
 
--
generated by git-patchbot for /home/xen/git/xen.git#master