I've had the luck to come across a PCI card that exposes a MSI-X capability
where the BIR of the vector and PBA tables points at a BAR that has 0 size.
This doesn't play nice with the code in vpci_make_msix_hole(), as it would
still use the address of such empty BAR (0) and attempt to crave a hole in
the p2m. This leads to errors like the one below being reported by Xen:
d0v0 0000:22:00.0: existing mapping (mfn: 181c4300 type: 0) at 0 clobbers MSIX MMIO area
And the device left unable to enable memory decoding due to the failure
reported by vpci_make_msix_hole().
Introduce checking in init_msix() to ensure the BARs containing the MSI-X
tables are usable. This requires checking that the BIR points to a
non-empty BAR, and the offset and size of the MSI-X tables can fit in the
target BAR.
This fixes booting PVH dom0 on Supermicro AS -2126HS-TN severs with AMD
EPYC 9965 processors. The broken device is:
22:00.0 SATA controller: Advanced Micro Devices, Inc. [AMD] FCH SATA Controller [AHCI mode] (rev 93)
There are multiple of those integrated controllers in the system, all
broken in the same way.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Stewart Hildebrand <stewart.hildebrand@amd.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Oleksii Kurochko <oleksii.kurochko@gmail.com>
While not strictly a bugfix, I consider this a worthy improvement so that
PVH dom0 has a chance to boot on hardware that exposes such broken MSI-X
capabilities. Hence I think this change should be considered for inclusion
into 4.21. There a risk of regressing on hardware that was already working
with PVH, but given enough testing that should be minimal.
---
xen/drivers/vpci/msix.c | 50 ++++++++++++++++++++++++++++++++++++-----
1 file changed, 45 insertions(+), 5 deletions(-)
diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index 54a5070733aa..8458955d5bbb 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -675,6 +675,51 @@ static int cf_check init_msix(struct pci_dev *pdev)
if ( !msix )
return -ENOMEM;
+ msix->tables[VPCI_MSIX_TABLE] =
+ pci_conf_read32(pdev->sbdf, msix_table_offset_reg(msix_offset));
+ msix->tables[VPCI_MSIX_PBA] =
+ pci_conf_read32(pdev->sbdf, msix_pba_offset_reg(msix_offset));
+
+ /* Check that the provided BAR is valid. */
+ for ( i = 0; i < ARRAY_SIZE(msix->tables); i++ )
+ {
+ const char *name = (i == VPCI_MSIX_TABLE) ? "vector" : "PBA";
+ const struct vpci_bar *bars = pdev->vpci->header.bars;
+ unsigned int bir = msix->tables[i] & PCI_MSIX_BIRMASK;
+ unsigned int type;
+ unsigned int offset = msix->tables[i] & ~PCI_MSIX_BIRMASK;
+ unsigned int size =
+ (i == VPCI_MSIX_TABLE) ? max_entries * PCI_MSIX_ENTRY_SIZE
+ : ROUNDUP(DIV_ROUND_UP(max_entries, 8), 8);
+
+ if ( bir >= ARRAY_SIZE(pdev->vpci->header.bars) )
+ {
+ printk(XENLOG_ERR "%pp: MSI-X %s table with out of range BIR %u\n",
+ &pdev->sbdf, name, bir);
+ invalid:
+ xfree(msix);
+ return -ENODEV;
+
+ }
+
+ type = bars[bir].type;
+ if ( type != VPCI_BAR_MEM32 && type != VPCI_BAR_MEM64_LO )
+ {
+ printk(XENLOG_ERR
+ "%pp: MSI-X %s table at invalid BAR%u with type %u\n",
+ &pdev->sbdf, name, bir, type);
+ goto invalid;
+ }
+
+ if ( (uint64_t)offset + size > bars[bir].size )
+ {
+ printk(XENLOG_ERR
+ "%pp: MSI-X %s table offset %#x size %#x outside of BAR%u size %#lx\n",
+ &pdev->sbdf, name, offset, size, bir, bars[bir].size);
+ goto invalid;
+ }
+ }
+
rc = vpci_add_register(pdev->vpci, control_read, control_write,
msix_control_reg(msix_offset), 2, msix);
if ( rc )
@@ -686,11 +731,6 @@ static int cf_check init_msix(struct pci_dev *pdev)
msix->max_entries = max_entries;
msix->pdev = pdev;
- msix->tables[VPCI_MSIX_TABLE] =
- pci_conf_read32(pdev->sbdf, msix_table_offset_reg(msix_offset));
- msix->tables[VPCI_MSIX_PBA] =
- pci_conf_read32(pdev->sbdf, msix_pba_offset_reg(msix_offset));
-
for ( i = 0; i < max_entries; i++)
{
msix->entries[i].masked = true;
--
2.51.0
On 9/29/25 04:41, Roger Pau Monne wrote:
> I've had the luck to come across a PCI card that exposes a MSI-X capability
> where the BIR of the vector and PBA tables points at a BAR that has 0 size.
>
> This doesn't play nice with the code in vpci_make_msix_hole(), as it would
> still use the address of such empty BAR (0) and attempt to crave a hole in
s/crave/carve/
> the p2m. This leads to errors like the one below being reported by Xen:
>
> d0v0 0000:22:00.0: existing mapping (mfn: 181c4300 type: 0) at 0 clobbers MSIX MMIO area
>
> And the device left unable to enable memory decoding due to the failure
> reported by vpci_make_msix_hole().
>
> Introduce checking in init_msix() to ensure the BARs containing the MSI-X
> tables are usable. This requires checking that the BIR points to a
> non-empty BAR, and the offset and size of the MSI-X tables can fit in the
> target BAR.
>
> This fixes booting PVH dom0 on Supermicro AS -2126HS-TN severs with AMD
> EPYC 9965 processors. The broken device is:
>
> 22:00.0 SATA controller: Advanced Micro Devices, Inc. [AMD] FCH SATA Controller [AHCI mode] (rev 93)
>
> There are multiple of those integrated controllers in the system, all
> broken in the same way.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Stewart Hildebrand <stewart.hildebrand@amd.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>
> While not strictly a bugfix, I consider this a worthy improvement so that
> PVH dom0 has a chance to boot on hardware that exposes such broken MSI-X
> capabilities. Hence I think this change should be considered for inclusion
> into 4.21. There a risk of regressing on hardware that was already working
> with PVH, but given enough testing that should be minimal.
> ---
> xen/drivers/vpci/msix.c | 50 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 45 insertions(+), 5 deletions(-)
>
> diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
> index 54a5070733aa..8458955d5bbb 100644
> --- a/xen/drivers/vpci/msix.c
> +++ b/xen/drivers/vpci/msix.c
> @@ -675,6 +675,51 @@ static int cf_check init_msix(struct pci_dev *pdev)
> if ( !msix )
> return -ENOMEM;
>
> + msix->tables[VPCI_MSIX_TABLE] =
> + pci_conf_read32(pdev->sbdf, msix_table_offset_reg(msix_offset));
> + msix->tables[VPCI_MSIX_PBA] =
> + pci_conf_read32(pdev->sbdf, msix_pba_offset_reg(msix_offset));
> +
> + /* Check that the provided BAR is valid. */
> + for ( i = 0; i < ARRAY_SIZE(msix->tables); i++ )
> + {
> + const char *name = (i == VPCI_MSIX_TABLE) ? "vector" : "PBA";
> + const struct vpci_bar *bars = pdev->vpci->header.bars;
> + unsigned int bir = msix->tables[i] & PCI_MSIX_BIRMASK;
> + unsigned int type;
> + unsigned int offset = msix->tables[i] & ~PCI_MSIX_BIRMASK;
> + unsigned int size =
> + (i == VPCI_MSIX_TABLE) ? max_entries * PCI_MSIX_ENTRY_SIZE
> + : ROUNDUP(DIV_ROUND_UP(max_entries, 8), 8);
> +
> + if ( bir >= ARRAY_SIZE(pdev->vpci->header.bars) )
This assumes a type 0 header. For type 1 headers, bir values 2 and up are
also reserved.
> + {
> + printk(XENLOG_ERR "%pp: MSI-X %s table with out of range BIR %u\n",
> + &pdev->sbdf, name, bir);
Nit: placing the cleanup label at the end of the function and using 'rc' would
make it more amenable to future uses.
> + invalid:
> + xfree(msix);
> + return -ENODEV;
> +
Extraneous newline.
> + }
> +
> + type = bars[bir].type;
> + if ( type != VPCI_BAR_MEM32 && type != VPCI_BAR_MEM64_LO )
> + {
> + printk(XENLOG_ERR
> + "%pp: MSI-X %s table at invalid BAR%u with type %u\n",
> + &pdev->sbdf, name, bir, type);
> + goto invalid;
> + }
> +
> + if ( (uint64_t)offset + size > bars[bir].size )
> + {
> + printk(XENLOG_ERR
> + "%pp: MSI-X %s table offset %#x size %#x outside of BAR%u size %#lx\n",
> + &pdev->sbdf, name, offset, size, bir, bars[bir].size);
> + goto invalid;
> + }
> + }
> +
> rc = vpci_add_register(pdev->vpci, control_read, control_write,
> msix_control_reg(msix_offset), 2, msix);
> if ( rc )
> @@ -686,11 +731,6 @@ static int cf_check init_msix(struct pci_dev *pdev)
> msix->max_entries = max_entries;
> msix->pdev = pdev;
>
> - msix->tables[VPCI_MSIX_TABLE] =
> - pci_conf_read32(pdev->sbdf, msix_table_offset_reg(msix_offset));
> - msix->tables[VPCI_MSIX_PBA] =
> - pci_conf_read32(pdev->sbdf, msix_pba_offset_reg(msix_offset));
> -
> for ( i = 0; i < max_entries; i++)
> {
> msix->entries[i].masked = true;
On Mon Sep 29, 2025 at 10:41 AM CEST, Roger Pau Monne wrote:
> I've had the luck to come across a PCI card that exposes a MSI-X capability
> where the BIR of the vector and PBA tables points at a BAR that has 0 size.
>
> This doesn't play nice with the code in vpci_make_msix_hole(), as it would
> still use the address of such empty BAR (0) and attempt to crave a hole in
> the p2m. This leads to errors like the one below being reported by Xen:
>
> d0v0 0000:22:00.0: existing mapping (mfn: 181c4300 type: 0) at 0 clobbers MSIX MMIO area
>
> And the device left unable to enable memory decoding due to the failure
> reported by vpci_make_msix_hole().
>
> Introduce checking in init_msix() to ensure the BARs containing the MSI-X
> tables are usable. This requires checking that the BIR points to a
> non-empty BAR, and the offset and size of the MSI-X tables can fit in the
> target BAR.
>
> This fixes booting PVH dom0 on Supermicro AS -2126HS-TN severs with AMD
> EPYC 9965 processors. The broken device is:
>
> 22:00.0 SATA controller: Advanced Micro Devices, Inc. [AMD] FCH SATA Controller [AHCI mode] (rev 93)
>
> There are multiple of those integrated controllers in the system, all
> broken in the same way.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Stewart Hildebrand <stewart.hildebrand@amd.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>
> While not strictly a bugfix, I consider this a worthy improvement so that
> PVH dom0 has a chance to boot on hardware that exposes such broken MSI-X
> capabilities. Hence I think this change should be considered for inclusion
> into 4.21. There a risk of regressing on hardware that was already working
> with PVH, but given enough testing that should be minimal.
> ---
> xen/drivers/vpci/msix.c | 50 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 45 insertions(+), 5 deletions(-)
>
> diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
> index 54a5070733aa..8458955d5bbb 100644
> --- a/xen/drivers/vpci/msix.c
> +++ b/xen/drivers/vpci/msix.c
> @@ -675,6 +675,51 @@ static int cf_check init_msix(struct pci_dev *pdev)
> if ( !msix )
> return -ENOMEM;
>
> + msix->tables[VPCI_MSIX_TABLE] =
> + pci_conf_read32(pdev->sbdf, msix_table_offset_reg(msix_offset));
> + msix->tables[VPCI_MSIX_PBA] =
> + pci_conf_read32(pdev->sbdf, msix_pba_offset_reg(msix_offset));
> +
> + /* Check that the provided BAR is valid. */
> + for ( i = 0; i < ARRAY_SIZE(msix->tables); i++ )
> + {
> + const char *name = (i == VPCI_MSIX_TABLE) ? "vector" : "PBA";
> + const struct vpci_bar *bars = pdev->vpci->header.bars;
> + unsigned int bir = msix->tables[i] & PCI_MSIX_BIRMASK;
> + unsigned int type;
> + unsigned int offset = msix->tables[i] & ~PCI_MSIX_BIRMASK;
> + unsigned int size =
> + (i == VPCI_MSIX_TABLE) ? max_entries * PCI_MSIX_ENTRY_SIZE
> + : ROUNDUP(DIV_ROUND_UP(max_entries, 8), 8);
> +
> + if ( bir >= ARRAY_SIZE(pdev->vpci->header.bars) )
> + {
> + printk(XENLOG_ERR "%pp: MSI-X %s table with out of range BIR %u\n",
> + &pdev->sbdf, name, bir);
Would it be worth adding something here such that a device vendor testing their
hardware under Xen can trivially grep for device bugs?
Something akin to "[Firmware bug]" on Linux, like "[Device bug]" or some such.
It would also let anyone not very knowledgeable about PCI know that a device
they own is being unreasonable. Same below in the other XENLOG_ERR messages.
> + invalid:
> + xfree(msix);
> + return -ENODEV;
> +
> + }
> +
> + type = bars[bir].type;
> + if ( type != VPCI_BAR_MEM32 && type != VPCI_BAR_MEM64_LO )
> + {
> + printk(XENLOG_ERR
> + "%pp: MSI-X %s table at invalid BAR%u with type %u\n",
> + &pdev->sbdf, name, bir, type);
> + goto invalid;
> + }
> +
> + if ( (uint64_t)offset + size > bars[bir].size )
> + {
> + printk(XENLOG_ERR
> + "%pp: MSI-X %s table offset %#x size %#x outside of BAR%u size %#lx\n",
> + &pdev->sbdf, name, offset, size, bir, bars[bir].size);
> + goto invalid;
> + }
> + }
> +
> rc = vpci_add_register(pdev->vpci, control_read, control_write,
> msix_control_reg(msix_offset), 2, msix);
> if ( rc )
> @@ -686,11 +731,6 @@ static int cf_check init_msix(struct pci_dev *pdev)
> msix->max_entries = max_entries;
> msix->pdev = pdev;
>
> - msix->tables[VPCI_MSIX_TABLE] =
> - pci_conf_read32(pdev->sbdf, msix_table_offset_reg(msix_offset));
> - msix->tables[VPCI_MSIX_PBA] =
> - pci_conf_read32(pdev->sbdf, msix_pba_offset_reg(msix_offset));
> -
> for ( i = 0; i < max_entries; i++)
> {
> msix->entries[i].masked = true;
On Tue, Sep 30, 2025 at 11:15:01AM +0200, Alejandro Vallejo wrote:
> On Mon Sep 29, 2025 at 10:41 AM CEST, Roger Pau Monne wrote:
> > I've had the luck to come across a PCI card that exposes a MSI-X capability
> > where the BIR of the vector and PBA tables points at a BAR that has 0 size.
> >
> > This doesn't play nice with the code in vpci_make_msix_hole(), as it would
> > still use the address of such empty BAR (0) and attempt to crave a hole in
> > the p2m. This leads to errors like the one below being reported by Xen:
> >
> > d0v0 0000:22:00.0: existing mapping (mfn: 181c4300 type: 0) at 0 clobbers MSIX MMIO area
> >
> > And the device left unable to enable memory decoding due to the failure
> > reported by vpci_make_msix_hole().
> >
> > Introduce checking in init_msix() to ensure the BARs containing the MSI-X
> > tables are usable. This requires checking that the BIR points to a
> > non-empty BAR, and the offset and size of the MSI-X tables can fit in the
> > target BAR.
> >
> > This fixes booting PVH dom0 on Supermicro AS -2126HS-TN severs with AMD
> > EPYC 9965 processors. The broken device is:
> >
> > 22:00.0 SATA controller: Advanced Micro Devices, Inc. [AMD] FCH SATA Controller [AHCI mode] (rev 93)
> >
> > There are multiple of those integrated controllers in the system, all
> > broken in the same way.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Stewart Hildebrand <stewart.hildebrand@amd.com>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> >
> > While not strictly a bugfix, I consider this a worthy improvement so that
> > PVH dom0 has a chance to boot on hardware that exposes such broken MSI-X
> > capabilities. Hence I think this change should be considered for inclusion
> > into 4.21. There a risk of regressing on hardware that was already working
> > with PVH, but given enough testing that should be minimal.
> > ---
> > xen/drivers/vpci/msix.c | 50 ++++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 45 insertions(+), 5 deletions(-)
> >
> > diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
> > index 54a5070733aa..8458955d5bbb 100644
> > --- a/xen/drivers/vpci/msix.c
> > +++ b/xen/drivers/vpci/msix.c
> > @@ -675,6 +675,51 @@ static int cf_check init_msix(struct pci_dev *pdev)
> > if ( !msix )
> > return -ENOMEM;
> >
> > + msix->tables[VPCI_MSIX_TABLE] =
> > + pci_conf_read32(pdev->sbdf, msix_table_offset_reg(msix_offset));
> > + msix->tables[VPCI_MSIX_PBA] =
> > + pci_conf_read32(pdev->sbdf, msix_pba_offset_reg(msix_offset));
> > +
> > + /* Check that the provided BAR is valid. */
> > + for ( i = 0; i < ARRAY_SIZE(msix->tables); i++ )
> > + {
> > + const char *name = (i == VPCI_MSIX_TABLE) ? "vector" : "PBA";
> > + const struct vpci_bar *bars = pdev->vpci->header.bars;
> > + unsigned int bir = msix->tables[i] & PCI_MSIX_BIRMASK;
> > + unsigned int type;
> > + unsigned int offset = msix->tables[i] & ~PCI_MSIX_BIRMASK;
> > + unsigned int size =
> > + (i == VPCI_MSIX_TABLE) ? max_entries * PCI_MSIX_ENTRY_SIZE
> > + : ROUNDUP(DIV_ROUND_UP(max_entries, 8), 8);
> > +
> > + if ( bir >= ARRAY_SIZE(pdev->vpci->header.bars) )
> > + {
> > + printk(XENLOG_ERR "%pp: MSI-X %s table with out of range BIR %u\n",
> > + &pdev->sbdf, name, bir);
>
> Would it be worth adding something here such that a device vendor testing their
> hardware under Xen can trivially grep for device bugs?
>
> Something akin to "[Firmware bug]" on Linux, like "[Device bug]" or some such.
>
> It would also let anyone not very knowledgeable about PCI know that a device
> they own is being unreasonable. Same below in the other XENLOG_ERR messages.
We could add indeed. I don't think we haven't done so in the past.
If we go that route I would suggest that I add a:
#define DEVICE_BUG_PREFIX "[Device bug] "
in lib.h or similar, to make sure we use the same prefix uniformly.
TBH I think vendors care little about the output of Xen, as long as it
boots.
The downside of this is that it makes those messages longer, which
will require more time to print if using a slow UART.
Thanks, Roger.
On 9/29/25 10:41 AM, Roger Pau Monne wrote:
> I've had the luck to come across a PCI card that exposes a MSI-X capability
> where the BIR of the vector and PBA tables points at a BAR that has 0 size.
>
> This doesn't play nice with the code in vpci_make_msix_hole(), as it would
> still use the address of such empty BAR (0) and attempt to crave a hole in
> the p2m. This leads to errors like the one below being reported by Xen:
>
> d0v0 0000:22:00.0: existing mapping (mfn: 181c4300 type: 0) at 0 clobbers MSIX MMIO area
>
> And the device left unable to enable memory decoding due to the failure
> reported by vpci_make_msix_hole().
>
> Introduce checking in init_msix() to ensure the BARs containing the MSI-X
> tables are usable. This requires checking that the BIR points to a
> non-empty BAR, and the offset and size of the MSI-X tables can fit in the
> target BAR.
>
> This fixes booting PVH dom0 on Supermicro AS -2126HS-TN severs with AMD
> EPYC 9965 processors. The broken device is:
>
> 22:00.0 SATA controller: Advanced Micro Devices, Inc. [AMD] FCH SATA Controller [AHCI mode] (rev 93)
>
> There are multiple of those integrated controllers in the system, all
> broken in the same way.
>
> Signed-off-by: Roger Pau Monné<roger.pau@citrix.com>
> ---
> Cc: Stewart Hildebrand<stewart.hildebrand@amd.com>
> Cc: Jan Beulich<jbeulich@suse.com>
> Cc: Oleksii Kurochko<oleksii.kurochko@gmail.com>
>
> While not strictly a bugfix, I consider this a worthy improvement so that
> PVH dom0 has a chance to boot on hardware that exposes such broken MSI-X
> capabilities.
Based on your commit description it looks like a bug as without it, for example,
SATA controller can't be used, right?
> Hence I think this change should be considered for inclusion
> into 4.21. There a risk of regressing on hardware that was already working
> with PVH, but given enough testing that should be minimal.
We have some PVH tests in Xen’s GitLab CI, but I assume that isn’t enough?
~ Oleksii
> ---
> xen/drivers/vpci/msix.c | 50 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 45 insertions(+), 5 deletions(-)
>
> diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
> index 54a5070733aa..8458955d5bbb 100644
> --- a/xen/drivers/vpci/msix.c
> +++ b/xen/drivers/vpci/msix.c
> @@ -675,6 +675,51 @@ static int cf_check init_msix(struct pci_dev *pdev)
> if ( !msix )
> return -ENOMEM;
>
> + msix->tables[VPCI_MSIX_TABLE] =
> + pci_conf_read32(pdev->sbdf, msix_table_offset_reg(msix_offset));
> + msix->tables[VPCI_MSIX_PBA] =
> + pci_conf_read32(pdev->sbdf, msix_pba_offset_reg(msix_offset));
> +
> + /* Check that the provided BAR is valid. */
> + for ( i = 0; i < ARRAY_SIZE(msix->tables); i++ )
> + {
> + const char *name = (i == VPCI_MSIX_TABLE) ? "vector" : "PBA";
> + const struct vpci_bar *bars = pdev->vpci->header.bars;
> + unsigned int bir = msix->tables[i] & PCI_MSIX_BIRMASK;
> + unsigned int type;
> + unsigned int offset = msix->tables[i] & ~PCI_MSIX_BIRMASK;
> + unsigned int size =
> + (i == VPCI_MSIX_TABLE) ? max_entries * PCI_MSIX_ENTRY_SIZE
> + : ROUNDUP(DIV_ROUND_UP(max_entries, 8), 8);
> +
> + if ( bir >= ARRAY_SIZE(pdev->vpci->header.bars) )
> + {
> + printk(XENLOG_ERR "%pp: MSI-X %s table with out of range BIR %u\n",
> + &pdev->sbdf, name, bir);
> + invalid:
> + xfree(msix);
> + return -ENODEV;
> +
> + }
> +
> + type = bars[bir].type;
> + if ( type != VPCI_BAR_MEM32 && type != VPCI_BAR_MEM64_LO )
> + {
> + printk(XENLOG_ERR
> + "%pp: MSI-X %s table at invalid BAR%u with type %u\n",
> + &pdev->sbdf, name, bir, type);
> + goto invalid;
> + }
> +
> + if ( (uint64_t)offset + size > bars[bir].size )
> + {
> + printk(XENLOG_ERR
> + "%pp: MSI-X %s table offset %#x size %#x outside of BAR%u size %#lx\n",
> + &pdev->sbdf, name, offset, size, bir, bars[bir].size);
> + goto invalid;
> + }
> + }
> +
> rc = vpci_add_register(pdev->vpci, control_read, control_write,
> msix_control_reg(msix_offset), 2, msix);
> if ( rc )
> @@ -686,11 +731,6 @@ static int cf_check init_msix(struct pci_dev *pdev)
> msix->max_entries = max_entries;
> msix->pdev = pdev;
>
> - msix->tables[VPCI_MSIX_TABLE] =
> - pci_conf_read32(pdev->sbdf, msix_table_offset_reg(msix_offset));
> - msix->tables[VPCI_MSIX_PBA] =
> - pci_conf_read32(pdev->sbdf, msix_pba_offset_reg(msix_offset));
> -
> for ( i = 0; i < max_entries; i++)
> {
> msix->entries[i].masked = true;
On Mon, Sep 29, 2025 at 05:59:00PM +0200, Oleksii Kurochko wrote: > > On 9/29/25 10:41 AM, Roger Pau Monne wrote: > > I've had the luck to come across a PCI card that exposes a MSI-X capability > > where the BIR of the vector and PBA tables points at a BAR that has 0 size. > > > > This doesn't play nice with the code in vpci_make_msix_hole(), as it would > > still use the address of such empty BAR (0) and attempt to crave a hole in > > the p2m. This leads to errors like the one below being reported by Xen: > > > > d0v0 0000:22:00.0: existing mapping (mfn: 181c4300 type: 0) at 0 clobbers MSIX MMIO area > > > > And the device left unable to enable memory decoding due to the failure > > reported by vpci_make_msix_hole(). > > > > Introduce checking in init_msix() to ensure the BARs containing the MSI-X > > tables are usable. This requires checking that the BIR points to a > > non-empty BAR, and the offset and size of the MSI-X tables can fit in the > > target BAR. > > > > This fixes booting PVH dom0 on Supermicro AS -2126HS-TN severs with AMD > > EPYC 9965 processors. The broken device is: > > > > 22:00.0 SATA controller: Advanced Micro Devices, Inc. [AMD] FCH SATA Controller [AHCI mode] (rev 93) > > > > There are multiple of those integrated controllers in the system, all > > broken in the same way. > > > > Signed-off-by: Roger Pau Monné<roger.pau@citrix.com> > > --- > > Cc: Stewart Hildebrand<stewart.hildebrand@amd.com> > > Cc: Jan Beulich<jbeulich@suse.com> > > Cc: Oleksii Kurochko<oleksii.kurochko@gmail.com> > > > > While not strictly a bugfix, I consider this a worthy improvement so that > > PVH dom0 has a chance to boot on hardware that exposes such broken MSI-X > > capabilities. > > Based on your commit description it looks like a bug as without it, for example, > SATA controller can't be used, right? > > > Hence I think this change should be considered for inclusion > > into 4.21. There a risk of regressing on hardware that was already working > > with PVH, but given enough testing that should be minimal. > > We have some PVH tests in Xen’s GitLab CI, but I assume that isn’t enough? It's a very specific controller, which we don't seem to have any examples of in the lab. The model is in the commit message. Without this fix the device doesn't work as expected when used in PVH dom0 mode. Regards, Roger.
On 9/30/25 2:46 PM, Roger Pau Monné wrote: > On Mon, Sep 29, 2025 at 05:59:00PM +0200, Oleksii Kurochko wrote: >> On 9/29/25 10:41 AM, Roger Pau Monne wrote: >>> I've had the luck to come across a PCI card that exposes a MSI-X capability >>> where the BIR of the vector and PBA tables points at a BAR that has 0 size. >>> >>> This doesn't play nice with the code in vpci_make_msix_hole(), as it would >>> still use the address of such empty BAR (0) and attempt to crave a hole in >>> the p2m. This leads to errors like the one below being reported by Xen: >>> >>> d0v0 0000:22:00.0: existing mapping (mfn: 181c4300 type: 0) at 0 clobbers MSIX MMIO area >>> >>> And the device left unable to enable memory decoding due to the failure >>> reported by vpci_make_msix_hole(). >>> >>> Introduce checking in init_msix() to ensure the BARs containing the MSI-X >>> tables are usable. This requires checking that the BIR points to a >>> non-empty BAR, and the offset and size of the MSI-X tables can fit in the >>> target BAR. >>> >>> This fixes booting PVH dom0 on Supermicro AS -2126HS-TN severs with AMD >>> EPYC 9965 processors. The broken device is: >>> >>> 22:00.0 SATA controller: Advanced Micro Devices, Inc. [AMD] FCH SATA Controller [AHCI mode] (rev 93) >>> >>> There are multiple of those integrated controllers in the system, all >>> broken in the same way. >>> >>> Signed-off-by: Roger Pau Monné<roger.pau@citrix.com> >>> --- >>> Cc: Stewart Hildebrand<stewart.hildebrand@amd.com> >>> Cc: Jan Beulich<jbeulich@suse.com> >>> Cc: Oleksii Kurochko<oleksii.kurochko@gmail.com> >>> >>> While not strictly a bugfix, I consider this a worthy improvement so that >>> PVH dom0 has a chance to boot on hardware that exposes such broken MSI-X >>> capabilities. >> Based on your commit description it looks like a bug as without it, for example, >> SATA controller can't be used, right? >> >>> Hence I think this change should be considered for inclusion >>> into 4.21. There a risk of regressing on hardware that was already working >>> with PVH, but given enough testing that should be minimal. >> We have some PVH tests in Xen’s GitLab CI, but I assume that isn’t enough? > It's a very specific controller, which we don't seem to have any > examples of in the lab. The model is in the commit message. Without > this fix the device doesn't work as expected when used in PVH dom0 > mode. Thanks for the explanation. I think we should consider to have this in 4.21 as it is still a fix of bogus behavior: Released-Acked-By: Oleksii Kurochko<oleksii.kurochko@gmail.com> ~ Oleksii
© 2016 - 2025 Red Hat, Inc.