[Xen-devel] [PATCH v4 1/5] vpci: fix updating the command register

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

When switching the memory decoding bit in the command register the
rest of the changes where dropped, leading to only the memory decoding
bit being updated.

Fix this by writing the command register once the guest physmap
manipulations are done if there are changes to the memory decoding
bit.

Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxx>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Tim Deegan <tim@xxxxxxx>
Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
---
Changes since v3:
 - Only update the command register once after the physmap changes are
   done.
---
 xen/drivers/vpci/header.c | 35 +++++++++++++++++------------------
 xen/include/xen/vpci.h    |  2 +-
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 4573ccadf0..162d51f7e2 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -81,11 +81,12 @@ static int map_range(unsigned long s, unsigned long e, void 
*data,
  * BAR's enable bit has changed with the memory decoding bit already enabled.
  * If rom_only is not set then it's the memory decoding bit that changed.
  */
-static void modify_decoding(const struct pci_dev *pdev, bool map, bool 
rom_only)
+static void modify_decoding(const struct pci_dev *pdev, uint16_t cmd,
+                            bool rom_only)
 {
     struct vpci_header *header = &pdev->vpci->header;
     uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
-    uint16_t cmd;
+    bool map = cmd & PCI_COMMAND_MEMORY;
     unsigned int i;
 
     for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
@@ -111,9 +112,6 @@ static void modify_decoding(const struct pci_dev *pdev, 
bool map, bool rom_only)
     }
 
     ASSERT(!rom_only);
-    cmd = pci_conf_read16(pdev->seg, pdev->bus, slot, func, PCI_COMMAND);
-    cmd &= ~PCI_COMMAND_MEMORY;
-    cmd |= map ? PCI_COMMAND_MEMORY : 0;
     pci_conf_write16(pdev->seg, pdev->bus, slot, func, PCI_COMMAND,
                      cmd);
 }
@@ -124,7 +122,7 @@ bool vpci_process_pending(struct vcpu *v)
     {
         struct map_data data = {
             .d = v->domain,
-            .map = v->vpci.map,
+            .map = v->vpci.cmd & PCI_COMMAND_MEMORY,
         };
         int rc = rangeset_consume_ranges(v->vpci.mem, map_range, &data);
 
@@ -133,7 +131,8 @@ bool vpci_process_pending(struct vcpu *v)
 
         spin_lock(&v->vpci.pdev->vpci->lock);
         /* Disable memory decoding unconditionally on failure. */
-        modify_decoding(v->vpci.pdev, !rc && v->vpci.map,
+        modify_decoding(v->vpci.pdev,
+                        rc ? v->vpci.cmd & ~PCI_COMMAND_MEMORY : v->vpci.cmd,
                         !rc && v->vpci.rom_only);
         spin_unlock(&v->vpci.pdev->vpci->lock);
 
@@ -154,7 +153,7 @@ bool vpci_process_pending(struct vcpu *v)
 }
 
 static int __init apply_map(struct domain *d, const struct pci_dev *pdev,
-                            struct rangeset *mem)
+                            struct rangeset *mem, uint16_t cmd)
 {
     struct map_data data = { .d = d, .map = true };
     int rc;
@@ -163,13 +162,13 @@ static int __init apply_map(struct domain *d, const 
struct pci_dev *pdev,
         process_pending_softirqs();
     rangeset_destroy(mem);
     if ( !rc )
-        modify_decoding(pdev, true, false);
+        modify_decoding(pdev, cmd, false);
 
     return rc;
 }
 
 static void defer_map(struct domain *d, struct pci_dev *pdev,
-                      struct rangeset *mem, bool map, bool rom_only)
+                      struct rangeset *mem, uint16_t cmd, bool rom_only)
 {
     struct vcpu *curr = current;
 
@@ -181,11 +180,11 @@ static void defer_map(struct domain *d, struct pci_dev 
*pdev,
      */
     curr->vpci.pdev = pdev;
     curr->vpci.mem = mem;
-    curr->vpci.map = map;
+    curr->vpci.cmd = cmd;
     curr->vpci.rom_only = rom_only;
 }
 
-static int modify_bars(const struct pci_dev *pdev, bool map, bool rom_only)
+static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool rom_only)
 {
     struct vpci_header *header = &pdev->vpci->header;
     struct rangeset *mem = rangeset_new(NULL, NULL, 0);
@@ -305,11 +304,11 @@ static int modify_bars(const struct pci_dev *pdev, bool 
map, bool rom_only)
          * be called iff the memory decoding bit is enabled, thus the operation
          * will always be to establish mappings and process all the BARs.
          */
-        ASSERT(map && !rom_only);
-        return apply_map(pdev->domain, pdev, mem);
+        ASSERT((cmd & PCI_COMMAND_MEMORY) && !rom_only);
+        return apply_map(pdev->domain, pdev, mem, cmd);
     }
 
-    defer_map(dev->domain, dev, mem, map, rom_only);
+    defer_map(dev->domain, dev, mem, cmd, rom_only);
 
     return 0;
 }
@@ -332,7 +331,7 @@ static void cmd_write(const struct pci_dev *pdev, unsigned 
int reg,
          * memory decoding bit has not been changed, so leave everything as-is,
          * hoping the guest will realize and try again.
          */
-        modify_bars(pdev, cmd & PCI_COMMAND_MEMORY, false);
+        modify_bars(pdev, cmd, false);
     else
         pci_conf_write16(pdev->seg, pdev->bus, slot, func, reg, cmd);
 }
@@ -413,7 +412,7 @@ static void rom_write(const struct pci_dev *pdev, unsigned 
int reg,
         header->rom_enabled = new_enabled;
         pci_conf_write32(pdev->seg, pdev->bus, slot, func, reg, val);
     }
-    else if ( modify_bars(pdev, new_enabled, true) )
+    else if ( modify_bars(pdev, new_enabled ? PCI_COMMAND_MEMORY : 0, true) )
         /*
          * No memory has been added or removed from the p2m (because the actual
          * p2m changes are deferred in defer_map) and the ROM enable bit has
@@ -549,7 +548,7 @@ static int init_bars(struct pci_dev *pdev)
             rom->type = VPCI_BAR_EMPTY;
     }
 
-    return (cmd & PCI_COMMAND_MEMORY) ? modify_bars(pdev, true, false) : 0;
+    return (cmd & PCI_COMMAND_MEMORY) ? modify_bars(pdev, cmd, false) : 0;
 }
 REGISTER_VPCI_INIT(init_bars, VPCI_PRIORITY_MIDDLE);
 
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index af2b8580ee..44104b75b6 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -145,7 +145,7 @@ struct vpci_vcpu {
     /* Per-vcpu structure to store state while {un}mapping of PCI BARs. */
     struct rangeset *mem;
     struct pci_dev *pdev;
-    bool map      : 1;
+    uint16_t cmd;
     bool rom_only : 1;
 };
 
-- 
2.19.1


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