A hotplugged PCI device may be added uninitialized. In particular,
memory decoding might be disabled and the BARs might be zeroed. In this
case, the BARs will not be mapped in p2m. However, vpci_make_msix_hole()
unconditionally attempts to punch holes in p2m, leading to init_msix()
failing:

(XEN) d0v0 0000:01:00.0: existing mapping (mfn: 1c1880 type: 0) at 0 clobbers MSIX MMIO area
(XEN) d0 0000:01:00.0: init legacy cap 17 fail rc=-17, mask it

vpci_make_msix_hole() should only attempt to punch holes if the BARs
containing the MSI-X/PBA tables are mapped in p2m. Introduce a helper
for checking if a BAR is enabled, and add a check for the situation
inside vpci_make_msix_hole().

As a result of the newly introduced checks in vpci_make_msix_hole(),
move the call to vpci_make_msix_hole() within modify_decoding() to after
setting ->enabled.

Fixes: ee2eb6849d50 ("vpci: Refactor REGISTER_VPCI_INIT")
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
Pipeline: https://gitlab.com/xen-project/people/stewarthildebrand/xen/-/pipelines/2347521688

v2->v3:
* move check inside loop
* slightly reword commit message

v2: https://lore.kernel.org/xen-devel/20260224025626.26909-1-stewart.hildebrand@amd.com/T/#t

v1->v2:
* new title, was ("vpci/msix: conditionally invoke vpci_make_msix_hole")
* move BAR enabled check to inside vpci_make_msix_hole()
* introduce vmsix_table_bar_valid() helper
* move vpci_make_msix_hole() call within modify_decoding() to after
  setting ->enabled
* split typo fixup to separate patch

v1: https://lore.kernel.org/xen-devel/20250812151744.460953-1-stewart.hildebrand@amd.com/T/#t
---
 xen/drivers/vpci/header.c | 26 +++++++++++++-------------
 xen/drivers/vpci/msix.c   |  3 +++
 xen/include/xen/vpci.h    |  6 ++++++
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 739a5f610e91..6a28e07a625b 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -122,19 +122,6 @@ static void modify_decoding(const struct pci_dev *pdev, uint16_t cmd,
     bool map = cmd & PCI_COMMAND_MEMORY;
     unsigned int i;
 
-    /*
-     * Make sure there are no mappings in the MSIX MMIO areas, so that accesses
-     * can be trapped (and emulated) by Xen when the memory decoding bit is
-     * enabled.
-     *
-     * FIXME: punching holes after the p2m has been set up might be racy for
-     * DomU usage, needs to be revisited.
-     */
-#ifdef CONFIG_HAS_PCI_MSI
-    if ( map && !rom_only && vpci_make_msix_hole(pdev) )
-        return;
-#endif
-
     for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
     {
         struct vpci_bar *bar = &header->bars[i];
@@ -164,6 +151,19 @@ static void modify_decoding(const struct pci_dev *pdev, uint16_t cmd,
             bar->enabled = map;
     }
 
+    /*
+     * Make sure there are no mappings in the MSIX MMIO areas, so that accesses
+     * can be trapped (and emulated) by Xen when the memory decoding bit is
+     * enabled.
+     *
+     * FIXME: punching holes after the p2m has been set up might be racy for
+     * DomU usage, needs to be revisited.
+     */
+#ifdef CONFIG_HAS_PCI_MSI
+    if ( map && !rom_only && vpci_make_msix_hole(pdev) )
+        return;
+#endif
+
     if ( !rom_only )
     {
         pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index eaf8fae970f8..3efdd520e4bd 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -605,6 +605,9 @@ int vpci_make_msix_hole(const struct pci_dev *pdev)
         unsigned long end = PFN_DOWN(vmsix_table_addr(pdev->vpci, i) +
                                      vmsix_table_size(pdev->vpci, i) - 1);
 
+        if ( !vmsix_table_bar_valid(pdev->vpci, i) )
+            continue;
+
         for ( ; start <= end; start++ )
         {
             p2m_type_t t;
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index d6310104ea17..8ce730791080 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -267,6 +267,12 @@ static inline paddr_t vmsix_table_addr(const struct vpci *vpci, unsigned int nr)
            (vpci->msix->tables[nr] & ~PCI_MSIX_BIRMASK);
 }
 
+static inline bool vmsix_table_bar_valid(const struct vpci *vpci,
+                                         unsigned int nr)
+{
+    return vpci->header.bars[vpci->msix->tables[nr] & PCI_MSIX_BIRMASK].enabled;
+}
+
 /*
  * Note regarding the size calculation of the PBA: the spec mentions "The last
  * QWORD will not necessarily be fully populated", so it implies that the PBA

base-commit: 70c47ec5d65fa985ef7becd74b2d7b6d744b4c97
-- 
2.53.0