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(-)
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.0On Wed, Feb 25, 2026 at 09:57:38PM -0500, Stewart Hildebrand wrote:
> 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>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Thanks.
On 3/4/26 10:23, Roger Pau Monné wrote:
> On Wed, Feb 25, 2026 at 09:57:38PM -0500, Stewart Hildebrand wrote:
>> 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>
>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>
> Thanks.
Thanks!
I would like to point out that this now needs a rebase:
The helper vmsix_table_bar_valid() should be moved to the new private.h.
I'd be happy to send v4, assuming I can retain your R-b.
On Wed, Mar 04, 2026 at 10:39:56AM -0500, Stewart Hildebrand wrote:
> On 3/4/26 10:23, Roger Pau Monné wrote:
> > On Wed, Feb 25, 2026 at 09:57:38PM -0500, Stewart Hildebrand wrote:
> >> 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>
> >
> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> >
> > Thanks.
>
> Thanks!
>
> I would like to point out that this now needs a rebase:
> The helper vmsix_table_bar_valid() should be moved to the new private.h.
> I'd be happy to send v4, assuming I can retain your R-b.
Sure, please keep the RB.
Thanks, Roger.
On 04.03.2026 17:54, Roger Pau Monné wrote:
> On Wed, Mar 04, 2026 at 10:39:56AM -0500, Stewart Hildebrand wrote:
>> On 3/4/26 10:23, Roger Pau Monné wrote:
>>> On Wed, Feb 25, 2026 at 09:57:38PM -0500, Stewart Hildebrand wrote:
>>>> 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>
>>>
>>> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>>>
>>> Thanks.
>>
>> Thanks!
>>
>> I would like to point out that this now needs a rebase:
>> The helper vmsix_table_bar_valid() should be moved to the new private.h.
>> I'd be happy to send v4, assuming I can retain your R-b.
>
> Sure, please keep the RB.
I was actually going to see about doing the rebasing when doing my next sweep
(pretty soon, i.e. after finishing going through email).
Jan
© 2016 - 2026 Red Hat, Inc.