Commit 53859596c0d3 depends on the behavioral change introduced in
ee2eb6849d50 that inverted the initialization of the MSI-X and header vPCI
handlers.  53859596c0d3 requires the header to be initialized before MSI-X.

Change the order of initialization and adjust init_msix() to carve holes
for the MSI-X tables if necessary, since it will now run after the BARs
have been possibly mapped into the domain p2m.

Fixes: f9aea52a9747 ("vpci/msix: improve handling of bogus MSI-X capabilities")
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: jbeulich@suse.com
Cc: stewart.hildebrand@amd.com
---
 xen/drivers/vpci/header.c | 2 +-
 xen/drivers/vpci/msix.c   | 9 +++++++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 1f48f2aac64e..b002eb207243 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -1016,7 +1016,7 @@ static int cf_check init_header(struct pci_dev *pdev)
     pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
     return rc;
 }
-REGISTER_VPCI_INIT(init_header, VPCI_PRIORITY_MIDDLE);
+REGISTER_VPCI_INIT(init_header, VPCI_PRIORITY_HIGH);
 
 /*
  * Local variables:
diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index faa7c1cd494a..94fb645377e5 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -792,9 +792,14 @@ static int cf_check init_msix(struct pci_dev *pdev)
     pdev->vpci->msix = msix;
     list_add(&msix->next, &d->arch.hvm.msix_tables);
 
-    return 0;
+    /*
+     * vPCI header initialization will have mapped the whole BAR into the
+     * p2m, as MSI-X capability was not yet initialized.  Crave a hole for
+     * the MSI-X table here, so that Xen can trap accesses.
+     */
+    return vpci_make_msix_hole(pdev);
 }
-REGISTER_VPCI_INIT(init_msix, VPCI_PRIORITY_HIGH);
+REGISTER_VPCI_INIT(init_msix, VPCI_PRIORITY_MIDDLE);
 
 /*
  * Local variables:
-- 
2.51.0

On 22.10.2025 10:59, Roger Pau Monne wrote:
> Commit 53859596c0d3 depends on the behavioral change introduced in
> ee2eb6849d50 that inverted the initialization of the MSI-X and header vPCI
> handlers.  53859596c0d3 requires the header to be initialized before MSI-X.
> 
> Change the order of initialization and adjust init_msix() to carve holes
> for the MSI-X tables if necessary, since it will now run after the BARs
> have been possibly mapped into the domain p2m.
> 
> Fixes: f9aea52a9747 ("vpci/msix: improve handling of bogus MSI-X capabilities")
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -1016,7 +1016,7 @@ static int cf_check init_header(struct pci_dev *pdev)
>      pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
>      return rc;
>  }
> -REGISTER_VPCI_INIT(init_header, VPCI_PRIORITY_MIDDLE);
> +REGISTER_VPCI_INIT(init_header, VPCI_PRIORITY_HIGH);
>  
>  /*
>   * Local variables:
> --- a/xen/drivers/vpci/msix.c
> +++ b/xen/drivers/vpci/msix.c
> @@ -792,9 +792,14 @@ static int cf_check init_msix(struct pci_dev *pdev)
>      pdev->vpci->msix = msix;
>      list_add(&msix->next, &d->arch.hvm.msix_tables);
>  
> -    return 0;
> +    /*
> +     * vPCI header initialization will have mapped the whole BAR into the
> +     * p2m, as MSI-X capability was not yet initialized.  Crave a hole for
> +     * the MSI-X table here, so that Xen can trap accesses.
> +     */
> +    return vpci_make_msix_hole(pdev);
>  }
> -REGISTER_VPCI_INIT(init_msix, VPCI_PRIORITY_HIGH);
> +REGISTER_VPCI_INIT(init_msix, VPCI_PRIORITY_MIDDLE);

Aiui this could as well be VPCI_PRIORITY_LOW now, much like in staging init
order between MSI, MSI-X, and ReBar isn't enforced anymore?

Jan

On Wed, Oct 22, 2025 at 11:28:42AM +0200, Jan Beulich wrote:
> On 22.10.2025 10:59, Roger Pau Monne wrote:
> > Commit 53859596c0d3 depends on the behavioral change introduced in
> > ee2eb6849d50 that inverted the initialization of the MSI-X and header vPCI
> > handlers.  53859596c0d3 requires the header to be initialized before MSI-X.
> > 
> > Change the order of initialization and adjust init_msix() to carve holes
> > for the MSI-X tables if necessary, since it will now run after the BARs
> > have been possibly mapped into the domain p2m.
> > 
> > Fixes: f9aea52a9747 ("vpci/msix: improve handling of bogus MSI-X capabilities")
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

> > --- a/xen/drivers/vpci/header.c
> > +++ b/xen/drivers/vpci/header.c
> > @@ -1016,7 +1016,7 @@ static int cf_check init_header(struct pci_dev *pdev)
> >      pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
> >      return rc;
> >  }
> > -REGISTER_VPCI_INIT(init_header, VPCI_PRIORITY_MIDDLE);
> > +REGISTER_VPCI_INIT(init_header, VPCI_PRIORITY_HIGH);
> >  
> >  /*
> >   * Local variables:
> > --- a/xen/drivers/vpci/msix.c
> > +++ b/xen/drivers/vpci/msix.c
> > @@ -792,9 +792,14 @@ static int cf_check init_msix(struct pci_dev *pdev)
> >      pdev->vpci->msix = msix;
> >      list_add(&msix->next, &d->arch.hvm.msix_tables);
> >  
> > -    return 0;
> > +    /*
> > +     * vPCI header initialization will have mapped the whole BAR into the
> > +     * p2m, as MSI-X capability was not yet initialized.  Crave a hole for
> > +     * the MSI-X table here, so that Xen can trap accesses.
> > +     */
> > +    return vpci_make_msix_hole(pdev);
> >  }
> > -REGISTER_VPCI_INIT(init_msix, VPCI_PRIORITY_HIGH);
> > +REGISTER_VPCI_INIT(init_msix, VPCI_PRIORITY_MIDDLE);
> 
> Aiui this could as well be VPCI_PRIORITY_LOW now, much like in staging init
> order between MSI, MSI-X, and ReBar isn't enforced anymore?

Yes, that's my understating also, I however didn't want to make that
change to avoid any surprises.  This way MSI-X and header are still
the first ones to initialize.  Also that would render
VPCI_PRIORITY_MIDDLE effectively unused in the code, which might be
confusing for readers.

Roger.