[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH v7 2/2] x86/msi: fix locking for SR-IOV devices
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> --- Candidate for backport to 4.19 Patch #1 ("xen/pci: introduce PF<->VF links") is pre-requisite v6->v7: * turn NULL-check into ASSERT * adjust commentary in struct pf_info v5->v6: * add Jan's R-b v4->v5: * split the PF<->VF links to a pre-requisite patch * pass pci_sbdf_t to read_pci_mem_bar() * use stdint.h types on changed lines * re-add NULL check for pf_info in read_pci_mem_bar(), as pf_info could be NULL v3->v4: * handle case when PF is removed with VFs enabled, then re-added with VFs disabled v2->v3: * link from VF to PF's struct pci_dev * v1->v2: * remove call to pci_get_pdev() --- xen/arch/x86/msi.c | 36 ++++++++++++++++++++--------------- xen/drivers/passthrough/pci.c | 8 +++++--- xen/include/xen/pci.h | 11 ++++++++--- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c index fa724911928e..e2360579deda 100644 --- a/xen/arch/x86/msi.c +++ b/xen/arch/x86/msi.c @@ -663,34 +663,34 @@ static int msi_capability_init(struct pci_dev *dev, return 0; } -static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 func, u8 bir, int vf) +static uint64_t read_pci_mem_bar(pci_sbdf_t sbdf, uint8_t bir, int vf, + const struct pf_info *pf_info) { + uint16_t seg = sbdf.seg; + uint8_t bus = sbdf.bus, slot = sbdf.dev, func = sbdf.fn; u8 limit; u32 addr, base = PCI_BASE_ADDRESS_0; u64 disp = 0; if ( vf >= 0 ) { - struct pci_dev *pdev = pci_get_pdev(NULL, - PCI_SBDF(seg, bus, slot, func)); unsigned int pos; uint16_t ctrl, num_vf, offset, stride; - if ( !pdev ) - return 0; + ASSERT(pf_info); - pos = pci_find_ext_capability(pdev->sbdf, PCI_EXT_CAP_ID_SRIOV); - ctrl = pci_conf_read16(pdev->sbdf, pos + PCI_SRIOV_CTRL); - num_vf = pci_conf_read16(pdev->sbdf, pos + PCI_SRIOV_NUM_VF); - offset = pci_conf_read16(pdev->sbdf, pos + PCI_SRIOV_VF_OFFSET); - stride = pci_conf_read16(pdev->sbdf, pos + PCI_SRIOV_VF_STRIDE); + pos = pci_find_ext_capability(sbdf, PCI_EXT_CAP_ID_SRIOV); + ctrl = pci_conf_read16(sbdf, pos + PCI_SRIOV_CTRL); + num_vf = pci_conf_read16(sbdf, pos + PCI_SRIOV_NUM_VF); + offset = pci_conf_read16(sbdf, pos + PCI_SRIOV_VF_OFFSET); + stride = pci_conf_read16(sbdf, pos + PCI_SRIOV_VF_STRIDE); if ( !pos || !(ctrl & PCI_SRIOV_CTRL_VFE) || !(ctrl & PCI_SRIOV_CTRL_MSE) || !num_vf || !offset || (num_vf > 1 && !stride) || bir >= PCI_SRIOV_NUM_BARS || - !pdev->vf_rlen[bir] ) + !pf_info->vf_rlen[bir] ) return 0; base = pos + PCI_SRIOV_BAR; vf -= PCI_BDF(bus, slot, func) + offset; @@ -704,8 +704,8 @@ static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, u8 func, u8 bir, int vf) } if ( vf >= num_vf ) return 0; - BUILD_BUG_ON(ARRAY_SIZE(pdev->vf_rlen) != PCI_SRIOV_NUM_BARS); - disp = vf * pdev->vf_rlen[bir]; + BUILD_BUG_ON(ARRAY_SIZE(pf_info->vf_rlen) != PCI_SRIOV_NUM_BARS); + disp = vf * pf_info->vf_rlen[bir]; limit = PCI_SRIOV_NUM_BARS; } else switch ( pci_conf_read8(PCI_SBDF(seg, bus, slot, func), @@ -814,6 +814,7 @@ static int msix_capability_init(struct pci_dev *dev, int vf; paddr_t pba_paddr; unsigned int pba_offset; + const struct pf_info *pf_info; if ( !dev->info.is_virtfn ) { @@ -821,6 +822,7 @@ static int msix_capability_init(struct pci_dev *dev, pslot = slot; pfunc = func; vf = -1; + pf_info = NULL; } else { @@ -828,9 +830,12 @@ static int msix_capability_init(struct pci_dev *dev, pslot = PCI_SLOT(dev->info.physfn.devfn); pfunc = PCI_FUNC(dev->info.physfn.devfn); vf = dev->sbdf.bdf; + ASSERT(dev->pf_pdev); + pf_info = &dev->pf_pdev->physfn; } - table_paddr = read_pci_mem_bar(seg, pbus, pslot, pfunc, bir, vf); + table_paddr = read_pci_mem_bar(PCI_SBDF(seg, pbus, pslot, pfunc), bir, + vf, pf_info); WARN_ON(msi && msi->table_base != table_paddr); if ( !table_paddr ) { @@ -853,7 +858,8 @@ static int msix_capability_init(struct pci_dev *dev, pba_offset = pci_conf_read32(dev->sbdf, msix_pba_offset_reg(pos)); bir = (u8)(pba_offset & PCI_MSIX_BIRMASK); - pba_paddr = read_pci_mem_bar(seg, pbus, pslot, pfunc, bir, vf); + pba_paddr = read_pci_mem_bar(PCI_SBDF(seg, pbus, pslot, pfunc), bir, vf, + pf_info); WARN_ON(!pba_paddr); pba_paddr += pba_offset & ~PCI_MSIX_BIRMASK; diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index d4167cea09c0..2cd595f75ebf 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -730,7 +730,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] ) { unsigned int pos = pci_find_ext_capability(pdev->sbdf, PCI_EXT_CAP_ID_SRIOV); @@ -742,7 +742,9 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn, { unsigned int i; - BUILD_BUG_ON(ARRAY_SIZE(pdev->vf_rlen) != PCI_SRIOV_NUM_BARS); + BUILD_BUG_ON(ARRAY_SIZE(pdev->physfn.vf_rlen) != + PCI_SRIOV_NUM_BARS); + for ( i = 0; i < PCI_SRIOV_NUM_BARS; ) { unsigned int idx = pos + PCI_SRIOV_BAR + i * 4; @@ -757,7 +759,7 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn, continue; } ret = pci_size_mem_bar(pdev->sbdf, idx, NULL, - &pdev->vf_rlen[i], + &pdev->physfn.vf_rlen[i], PCI_BAR_VF | ((i == PCI_SRIOV_NUM_BARS - 1) ? PCI_BAR_LAST : 0)); diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h index 977c0d08f78a..f784e9116059 100644 --- a/xen/include/xen/pci.h +++ b/xen/include/xen/pci.h @@ -159,9 +159,14 @@ struct pci_dev { * List entry if VF. */ struct list_head vf_list; - u64 vf_rlen[6]; - /* Link from VF to PF. Only populated for VFs. */ - const struct pci_dev *pf_pdev; + union { + struct pf_info { + /* Only populated for PFs. */ + uint64_t vf_rlen[PCI_SRIOV_NUM_BARS]; + } physfn; + /* Link from VF to PF. Only populated for VFs. */ + const struct pci_dev *pf_pdev; + }; /* Data for vPCI. */ struct vpci *vpci; -- 2.47.0
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |