Re: [RFC] xen/x86: allow overlaps with non-RAM regions

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Apr 25, 2025 at 09:47:57AM -0700, Lira, Victor M wrote:
> I can confirm with the patch the NVME drive can be accessed despite the "not
> mapping BAR" warning from Xen.
> Output log attached.

Thanks a lot for the test, and sorry for the delay in getting back.  I
was busy with other stuff and needed some time to figure out the best
way to deal with this.  Can you give a try to the patch below?  I hope
it will still fix the issue.

Thanks, Roger.
---
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..de9ce76e1c8a 100644
--- a/xen/arch/x86/pci.c
+++ b/xen/arch/x86/pci.c
@@ -98,3 +98,56 @@ 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;
+}
+
+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;
+
+        /*
+         * Remove overlaps with any memory ranges defined in the host memory
+         * map.
+         */
+        rc = rangeset_remove_range(r, PFN_DOWN(entry->addr),
+                                   PFN_DOWN(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..533e24ca3674 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -394,6 +394,14 @@ 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 memory: %d\n",
+                    &pdev->sbdf, rc);
+            return rc;
+        }
     }
 
     /* Remove any MSIX regions if present. */