[Xen-changelog] [xen staging] vpci: fix updating the command register

[patchbot@xxxxxxx]

commit 0290d553e0d5ae395c88bd82301251d579e15ea7
Author:     Roger Pau Monné <roger.pau@xxxxxxxxxx>
AuthorDate: Mon Nov 26 15:40:06 2018 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Mon Nov 26 15:40:06 2018 +0100

    vpci: fix updating the command register
    
    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.
    
    Note that when only mapping/unmapping the ROM BAR a fabricated command
    register value is passed to modify_bars which is only used to signal
    whether the action is a mapping or unmapping, but the value is never
    written to the device command register. Turn the maodify_decoding
    ASSERT into an ASSERT_UNREACHABLE and make sure that non-debug builds
    won't end up writing to the command register if only modifying the ROM
    BAR.
    
    Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/drivers/vpci/header.c | 47 ++++++++++++++++++++++++++---------------------
 xen/include/xen/vpci.h    |  2 +-
 2 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 4573ccadf0..39dffb21fb 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++ )
@@ -110,12 +111,11 @@ static void modify_decoding(const struct pci_dev *pdev, 
bool map, bool rom_only)
             header->bars[i].enabled = map;
     }
 
-    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);
+    if ( !rom_only )
+        pci_conf_write16(pdev->seg, pdev->bus, slot, func, PCI_COMMAND,
+                         cmd);
+    else
+        ASSERT_UNREACHABLE();
 }
 
 bool vpci_process_pending(struct vcpu *v)
@@ -124,7 +124,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 +133,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 +155,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 +164,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 +182,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 +306,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 +333,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 +414,11 @@ 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) )
+    /*
+     * Pass PCI_COMMAND_MEMORY or 0 to signal a map/unmap request, note that
+     * this fabricated command is never going to be written to the register.
+     */
+    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 +554,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;
 };
 
--
generated by git-patchbot for /home/xen/git/xen.git#staging

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog