[PATCH] x86/vpci: fix handling of BAR overlaps with non-hole regions

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

For once the message printed when a BAR overlaps with a non-hole regions is
not accurate on x86.  While the BAR won't be mapped by the vPCI logic, it
is quite likely overlapping with a reserved region in the memory map, and
already mapped as by default all reserved regions are identity mapped in
the p2m.  Fix the message so it just warns about the overlap, without
mentioning that the BAR won't be mapped, as this has caused confusion in
the past.

Secondly, when an overlap is detected the BAR 'enabled' field is not set,
hence other vPCI code that depends on it like vPCI MSI-X handling won't
function properly, as it sees the BAR as disabled, even when memory
decoding is enabled for the device and the BAR is likely mapped in the
p2m.  Change the handling of BARs that overlap non-hole regions to instead
remove any overlapped regions from the rangeset, so the resulting ranges to
map just contain the hole regions.  This requires introducing a new
pci_sanitize_bar_memory() that's implemented per-arch and sanitizes the
address range to add to the p2m.

For x86 pci_sanitize_bar_memory() removes any regions present in the host
memory map, for ARM this is currently left as a dummy handler to not change
existing behavior.

Ultimately the above changes should fix the vPCI MSI-X handlers not working
correctly when the BAR that contains the MSI-X table overlaps with a
non-hole region, as then the 'enabled' BAR bit won't be set and the MSI-X
traps won't handle accesses as expected.

Reported-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
Fixes: 53d9133638c3 ('pci: do not disable memory decoding for devices')
Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 xen/arch/arm/include/asm/pci.h |  3 ++
 xen/arch/x86/include/asm/pci.h | 12 ++------
 xen/arch/x86/pci.c             | 50 ++++++++++++++++++++++++++++++++++
 xen/drivers/vpci/header.c      |  9 ++++++
 4 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/asm/pci.h
index 7f77226c9bbf..1605ec660d0b 100644
--- a/xen/arch/arm/include/asm/pci.h
+++ b/xen/arch/arm/include/asm/pci.h
@@ -128,6 +128,9 @@ int pci_host_bridge_mappings(struct domain *d);
 
 bool pci_check_bar(const struct pci_dev *pdev, mfn_t start, mfn_t end);
 
+static inline int pci_sanitize_bar_memory(struct rangeset *r)
+{ return 0; }
+
 #else   /*!CONFIG_HAS_PCI*/
 
 struct pci_dev;
diff --git a/xen/arch/x86/include/asm/pci.h b/xen/arch/x86/include/asm/pci.h
index fd5480d67d43..d2701f2deb62 100644
--- a/xen/arch/x86/include/asm/pci.h
+++ b/xen/arch/x86/include/asm/pci.h
@@ -2,6 +2,7 @@
 #define __X86_PCI_H__
 
 #include <xen/mm.h>
+#include <xen/rangeset.h>
 
 #define CF8_BDF(cf8)     (  ((cf8) & 0x00ffff00U) >> 8)
 #define CF8_ADDR_LO(cf8) (   (cf8) & 0x000000fcU)
@@ -57,14 +58,7 @@ static always_inline bool is_pci_passthrough_enabled(void)
 
 void arch_pci_init_pdev(struct pci_dev *pdev);
 
-static inline bool pci_check_bar(const struct pci_dev *pdev,
-                                 mfn_t start, mfn_t end)
-{
-    /*
-     * Check if BAR is not overlapping with any memory region defined
-     * in the memory map.
-     */
-    return is_memory_hole(start, end);
-}
+bool pci_check_bar(const struct pci_dev *pdev, mfn_t start, mfn_t end);
+int pci_sanitize_bar_memory(struct rangeset *r);
 
 #endif /* __X86_PCI_H__ */
diff --git a/xen/arch/x86/pci.c b/xen/arch/x86/pci.c
index 97b792e578f1..afaf9fe1c053 100644
--- a/xen/arch/x86/pci.c
+++ b/xen/arch/x86/pci.c
@@ -98,3 +98,53 @@ int pci_conf_write_intercept(unsigned int seg, unsigned int 
bdf,
 
     return rc;
 }
+
+bool pci_check_bar(const struct pci_dev *pdev, mfn_t start, mfn_t end)
+{
+    /*
+     * Check if BAR is not overlapping with any memory region defined
+     * in the memory map.
+     */
+    if ( !is_memory_hole(start, end) )
+        gdprintk(XENLOG_WARNING,
+                 "%pp: BAR at [%"PRI_mfn", %"PRI_mfn"] not in memory map 
hole\n",
+                 &pdev->sbdf, mfn_x(start), mfn_x(end));
+
+    /*
+     * Unconditionally return true, pci_sanitize_bar_memory() will remove any
+     * non-hole regions.
+     */
+    return true;
+}
+
+/* Remove overlaps with any ranges defined in the host memory map. */
+int pci_sanitize_bar_memory(struct rangeset *r)
+{
+    unsigned int i;
+
+    for ( i = 0; i < e820.nr_map; i++ )
+    {
+        const struct e820entry *entry = &e820.map[i];
+        int rc;
+
+        if ( !entry->size )
+            continue;
+
+        rc = rangeset_remove_range(r, PFN_DOWN(entry->addr),
+                                   PFN_UP(entry->addr + entry->size - 1));
+        if ( rc )
+            return rc;
+    }
+
+    return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index ef6c965c081c..1f48f2aac64e 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -394,6 +394,15 @@ static int modify_bars(const struct pci_dev *pdev, 
uint16_t cmd, bool rom_only)
                 return rc;
             }
         }
+
+        rc = pci_sanitize_bar_memory(bar->mem);
+        if ( rc )
+        {
+            gprintk(XENLOG_WARNING,
+                    "%pp: failed to sanitize BAR#%u memory: %d\n",
+                    &pdev->sbdf, i, rc);
+            return rc;
+        }
     }
 
     /* Remove any MSIX regions if present. */
-- 
2.48.1