[PATCH v8 1/3] xen/pci: introduce PF<->VF links

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

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@xxxxxxx>
---
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(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 74d3895e1ef6..3fe2e7884f45 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -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);
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 1e4fe68c60fb..977c0d08f78a 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -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;
-- 
2.47.0