Re: [PATCH v8 2/8] vpci: Refactor REGISTER_VPCI_INIT

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

On Fri, Jul 25, 2025 at 03:15:13AM +0000, Chen, Jiqian wrote:
> On 2025/7/24 22:28, Roger Pau Monné wrote:
> > On Thu, Jul 24, 2025 at 01:50:00PM +0800, Jiqian Chen wrote:
> >> diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
> >> index 74211301ba10..eb3e7dcd1068 100644
> >> --- a/xen/drivers/vpci/msix.c
> >> +++ b/xen/drivers/vpci/msix.c
> >> @@ -703,9 +703,18 @@ 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.
> >> +     */
> >> +    spin_lock(&pdev->vpci->lock);
> >> +    rc = vpci_make_msix_hole(pdev);
> >> +    spin_unlock(&pdev->vpci->lock);
> > 
> > I should have asked in the last version, but why do you take the vPCI
> > lock here?
> > 
> > The path that ASSERTs the lock is held should never be taken when
> > called from init_msix().  Is there some path I'm missing in
> > vpci_make_msix_hole() that requires the vCPI lock to be held?
> > 
> > The rest LGTM.
> Sorry to forget to delete this.
> Feel free to change when submit.
> Or I will change by sending a new version.

Can you ensure it also works without the locking?  I think so, but I
haven't tested myself.

Thanks, Roger.