@@ -333,6 +333,8 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
*((u8*) &pdev->devfn) = devfn;
pdev->domain = NULL;
+ INIT_LIST_HEAD(&pdev->vf_list);
+
arch_pci_init_pdev(pdev);
rc = pdev_msi_init(pdev);
@@ -449,6 +451,10 @@ static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev)
list_del(&pdev->alldevs_list);
pdev_msi_deinit(pdev);
+
+ if ( pdev->info.is_virtfn )
+ list_del(&pdev->vf_list);
+
xfree(pdev);
}
@@ -656,24 +662,11 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
unsigned int slot = PCI_SLOT(devfn), func = PCI_FUNC(devfn);
const char *type;
int ret;
- bool pf_is_extfn = false;
if ( !info )
type = "device";
else if ( info->is_virtfn )
- {
- pcidevs_lock();
- pdev = pci_get_pdev(NULL,
- PCI_SBDF(seg, info->physfn.bus,
- info->physfn.devfn));
- if ( pdev )
- pf_is_extfn = pdev->info.is_extfn;
- pcidevs_unlock();
- if ( !pdev )
- pci_add_device(seg, info->physfn.bus, info->physfn.devfn,
- NULL, node);
type = "virtual function";
- }
else if ( info->is_extfn )
type = "extended function";
else
@@ -698,12 +691,48 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
if ( info )
{
pdev->info = *info;
- /*
- * VF's 'is_extfn' field is used to indicate whether its PF is an
- * extended function.
- */
if ( pdev->info.is_virtfn )
- pdev->info.is_extfn = pf_is_extfn;
+ {
+ struct pci_dev *pf_pdev =
+ pci_get_pdev(NULL, PCI_SBDF(seg, info->physfn.bus,
+ info->physfn.devfn));
+
+ if ( !pf_pdev )
+ {
+ ret = pci_add_device(seg, info->physfn.bus, info->physfn.devfn,
+ NULL, node);
+ if ( ret )
+ {
+ printk(XENLOG_WARNING
+ "Failed to add SR-IOV device PF %pp for VF %pp\n",
+ &PCI_SBDF(seg, info->physfn.bus, info->physfn.devfn),
+ &pdev->sbdf);
+ free_pdev(pseg, pdev);
+ goto out;
+ }
+ pf_pdev = pci_get_pdev(NULL, PCI_SBDF(seg, info->physfn.bus,
+ info->physfn.devfn));
+ if ( !pf_pdev )
+ {
+ printk(XENLOG_ERR
+ "Inconsistent PCI state: failed to find newly added PF %pp for VF %pp\n",
+ &PCI_SBDF(seg, info->physfn.bus, info->physfn.devfn),
+ &pdev->sbdf);
+ ASSERT_UNREACHABLE();
+ free_pdev(pseg, pdev);
+ ret = -EILSEQ;
+ goto out;
+ }
+ }
+
+ if ( !pdev->pf_pdev )
+ {
+ /* VF inherits its 'is_extfn' from PF */
+ pdev->info.is_extfn = pf_pdev->info.is_extfn;
+ list_add(&pdev->vf_list, &pf_pdev->vf_list);
+ pdev->pf_pdev = pf_pdev;
+ }
+ }
}
if ( !pdev->info.is_virtfn && !pdev->vf_rlen[0] )
@@ -821,6 +850,28 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
if ( pdev->bus == bus && pdev->devfn == devfn )
{
+ if ( !pdev->info.is_virtfn && !list_empty(&pdev->vf_list) )
+ {
+ struct pci_dev *vf_pdev;
+
+ /*
+ * Linux Dom0 has been observed to not respect an error code
+ * returned from PHYSDEVOP_pci_device_remove. Mark VFs and PF
+ * broken.
+ */
+ list_for_each_entry(vf_pdev, &pdev->vf_list, vf_list)
+ vf_pdev->broken = true;
+
+ pdev->broken = true;
+
+ printk(XENLOG_WARNING
+ "Attempted to remove PCI SR-IOV PF %pp with VFs still present\n",
+ &pdev->sbdf);
+
+ ret = -EBUSY;
+ break;
+ }
+
if ( pdev->domain )
{
write_lock(&pdev->domain->pci_lock);
@@ -153,7 +153,15 @@ struct pci_dev {
unsigned int count;
#define PT_FAULT_THRESHOLD 10
} fault;
+
+ /*
+ * List head if PF.
+ * 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;
/* Data for vPCI. */
struct vpci *vpci;
Add links between a VF's struct pci_dev and its associated PF struct pci_dev. The hardware domain is expected to remove the associated VFs before removing the PF. If removal happens out of order, print a warning and return an error. This means that VFs can only exist with an associated PF. Additionally, if the hardware domain attempts to remove a PF with VFs still present, mark the PF and VFs broken, because Linux Dom0 has been observed to not respect the error returned. Move the calls to pci_get_pdev() and pci_add_device() down to avoid dropping and re-acquiring the pcidevs_lock(). Check !pdev->pf_pdev before adding the VF to the list to guard against adding it multiple times. Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com> --- Candidate for backport to 4.19 (the next patch depends on this one) v7->v8: * re-add logic catering to adding VFs without PF for backporting (split removal to separate patch) * move & reword comment about is_extfn * check pdev->pf_pdev to guard against multiple list addition * re-indent pf_pdev initialization v6->v7: * cope with multiple invocations of pci_add_device for VFs * get rid of enclosing struct for single member * during PF removal attempt with VFs still present: * keep PF * mark broken * don't unlink * return error * during VF add: * initialize pf_pdev in declaration * remove logic catering to adding VFs without PF v5->v6: * move printk() before ASSERT_UNREACHABLE() * warn about PF removal with VFs still present * clarify commit message v4->v5: * new patch, split from ("x86/msi: fix locking for SR-IOV devices") * move INIT_LIST_HEAD(&pdev->vf_list); earlier * collapse struct list_head instances * retain error code from pci_add_device() * unlink (and mark broken) VFs instead of removing them * const-ify VF->PF link --- xen/drivers/passthrough/pci.c | 87 +++++++++++++++++++++++++++-------- xen/include/xen/pci.h | 8 ++++ 2 files changed, 77 insertions(+), 18 deletions(-)