[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v5 3/3] x86/msi: fix locking for SR-IOV devices
On 11.10.2024 17:27, Stewart Hildebrand wrote: > In commit 4f78438b45e2 ("vpci: use per-domain PCI lock to protect vpci > structure") a lock was moved from allocate_and_map_msi_pirq() to the > caller and changed from pcidevs_lock() to read_lock(&d->pci_lock). > However, one call path wasn't updated to reflect the change, leading to > a failed assertion observed under the following conditions: > > * PV dom0 > * Debug build (CONFIG_DEBUG=y) of Xen > * There is an SR-IOV device in the system with one or more VFs enabled > * Dom0 has loaded the driver for the VF and enabled MSI-X > > (XEN) Assertion 'd || pcidevs_locked()' failed at > drivers/passthrough/pci.c:535 > (XEN) ----[ Xen-4.20-unstable x86_64 debug=y Not tainted ]---- > ... > (XEN) Xen call trace: > (XEN) [<ffff82d040284da8>] R pci_get_pdev+0x4c/0xab > (XEN) [<ffff82d040344f5c>] F arch/x86/msi.c#read_pci_mem_bar+0x58/0x272 > (XEN) [<ffff82d04034530e>] F > arch/x86/msi.c#msix_capability_init+0x198/0x755 > (XEN) [<ffff82d040345dad>] F arch/x86/msi.c#__pci_enable_msix+0x82/0xe8 > (XEN) [<ffff82d0403463e5>] F pci_enable_msi+0x3f/0x78 > (XEN) [<ffff82d04034be2b>] F map_domain_pirq+0x2a4/0x6dc > (XEN) [<ffff82d04034d4d5>] F allocate_and_map_msi_pirq+0x103/0x262 > (XEN) [<ffff82d04035da5d>] F physdev_map_pirq+0x210/0x259 > (XEN) [<ffff82d04035e798>] F do_physdev_op+0x9c3/0x1454 > (XEN) [<ffff82d040329475>] F pv_hypercall+0x5ac/0x6af > (XEN) [<ffff82d0402012d3>] F lstar_enter+0x143/0x150 > > In read_pci_mem_bar(), the VF obtains the struct pci_dev pointer for its > associated PF to access the vf_rlen array. This array is initialized in > pci_add_device() and is only populated in the associated PF's struct > pci_dev. > > Access the vf_rlen array via the link to the PF, and remove the > troublesome call to pci_get_pdev(). > > Fixes: 4f78438b45e2 ("vpci: use per-domain PCI lock to protect vpci > structure") > Reported-by: Teddy Astie <teddy.astie@xxxxxxxxxx> > Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -736,7 +736,7 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn, > } > } > > - if ( !pdev->info.is_virtfn && !pdev->vf_rlen[0] ) > + if ( !pdev->info.is_virtfn && !pdev->physfn.vf_rlen[0] ) Unrelated to your change: Now that I look at this again, it seems slightly wrong to me to use array slot 0 as "have we populated the array already" indicator. If BAR0 was unused, we may end up doing this more than once. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |