[xen master] PCI: simplify (and thus correct) pci_get_pdev{,_by_domain}()

[patchbot@xxxxxxx]

commit 8cf6e0738906fc269af40135ed82a07815dd3b9c
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Fri Aug 12 08:34:33 2022 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Fri Aug 12 08:34:33 2022 +0200

    PCI: simplify (and thus correct) pci_get_pdev{,_by_domain}()
    
    The last "wildcard" use of either function went away with f591755823a7
    ("IOMMU/PCI: don't let domain cleanup continue when device de-assignment
    failed"). Don't allow them to be called this way anymore. Besides
    simplifying the code this also fixes two bugs:
    
    1) When seg != -1, the outer loops should have been terminated after the
       first iteration, or else a device with the same BDF but on another
       segment could be found / returned.
    
    Reported-by: Rahul Singh <rahul.singh@xxxxxxx>
    
    2) When seg == -1 calling get_pseg() is bogus. The function (taking a
       u16) would look for segment 0xffff, which might exist. If it exists,
       we might then find / return a wrong device.
    
    In pci_get_pdev_by_domain() also switch from using the per-segment list
    to using the per-domain one, with the exception of the hardware domain
    (see the code comment there).
    
    While there also constify "pseg" and drop "pdev"'s already previously
    unnecessary initializer.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Rahul Singh <rahul.singh@xxxxxxx>
    Tested-by: Rahul Singh <rahul.singh@xxxxxxx>
---
 xen/drivers/passthrough/pci.c | 61 ++++++++++++++++++-------------------------
 xen/include/xen/pci.h         |  6 ++---
 2 files changed, 29 insertions(+), 38 deletions(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 938821e593..085cd82941 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -576,30 +576,19 @@ int __init pci_ro_device(int seg, int bus, int devfn)
     return 0;
 }
 
-struct pci_dev *pci_get_pdev(int seg, int bus, int devfn)
+struct pci_dev *pci_get_pdev(uint16_t seg, uint8_t bus, uint8_t devfn)
 {
-    struct pci_seg *pseg = get_pseg(seg);
-    struct pci_dev *pdev = NULL;
+    const struct pci_seg *pseg = get_pseg(seg);
+    struct pci_dev *pdev;
 
     ASSERT(pcidevs_locked());
-    ASSERT(seg != -1 || bus == -1);
-    ASSERT(bus != -1 || devfn == -1);
 
     if ( !pseg )
-    {
-        if ( seg == -1 )
-            radix_tree_gang_lookup(&pci_segments, (void **)&pseg, 0, 1);
-        if ( !pseg )
-            return NULL;
-    }
+        return NULL;
 
-    do {
-        list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
-            if ( (pdev->bus == bus || bus == -1) &&
-                 (pdev->devfn == devfn || devfn == -1) )
-                return pdev;
-    } while ( radix_tree_gang_lookup(&pci_segments, (void **)&pseg,
-                                     pseg->nr + 1, 1) );
+    list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
+        if ( pdev->bus == bus && pdev->devfn == devfn )
+            return pdev;
 
     return NULL;
 }
@@ -625,31 +614,33 @@ struct pci_dev *pci_get_real_pdev(int seg, int bus, int 
devfn)
     return pdev;
 }
 
-struct pci_dev *pci_get_pdev_by_domain(const struct domain *d, int seg,
-                                       int bus, int devfn)
+struct pci_dev *pci_get_pdev_by_domain(const struct domain *d, uint16_t seg,
+                                       uint8_t bus, uint8_t devfn)
 {
-    struct pci_seg *pseg = get_pseg(seg);
-    struct pci_dev *pdev = NULL;
-
-    ASSERT(seg != -1 || bus == -1);
-    ASSERT(bus != -1 || devfn == -1);
+    struct pci_dev *pdev;
 
-    if ( !pseg )
+    /*
+     * The hardware domain owns the majority of the devices in the system.
+     * When there are multiple segments, traversing the per-segment list is
+     * likely going to be faster, whereas for a single segment the difference
+     * shouldn't be that large.
+     */
+    if ( is_hardware_domain(d) )
     {
-        if ( seg == -1 )
-            radix_tree_gang_lookup(&pci_segments, (void **)&pseg, 0, 1);
+        const struct pci_seg *pseg = get_pseg(seg);
+
         if ( !pseg )
             return NULL;
-    }
 
-    do {
         list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
-            if ( (pdev->bus == bus || bus == -1) &&
-                 (pdev->devfn == devfn || devfn == -1) &&
-                 (pdev->domain == d) )
+            if ( pdev->bus == bus && pdev->devfn == devfn &&
+                 pdev->domain == d )
+                return pdev;
+    }
+    else
+        list_for_each_entry ( pdev, &d->pdev_list, domain_list )
+            if ( pdev->bus == bus && pdev->devfn == devfn )
                 return pdev;
-    } while ( radix_tree_gang_lookup(&pci_segments, (void **)&pseg,
-                                     pseg->nr + 1, 1) );
 
     return NULL;
 }
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index f34368643c..0eaa5f3938 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -177,10 +177,10 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
 int pci_remove_device(u16 seg, u8 bus, u8 devfn);
 int pci_ro_device(int seg, int bus, int devfn);
 int pci_hide_device(unsigned int seg, unsigned int bus, unsigned int devfn);
-struct pci_dev *pci_get_pdev(int seg, int bus, int devfn);
+struct pci_dev *pci_get_pdev(uint16_t seg, uint8_t bus, uint8_t devfn);
 struct pci_dev *pci_get_real_pdev(int seg, int bus, int devfn);
-struct pci_dev *pci_get_pdev_by_domain(const struct domain *, int seg,
-                                       int bus, int devfn);
+struct pci_dev *pci_get_pdev_by_domain(const struct domain *, uint16_t seg,
+                                       uint8_t bus, uint8_t devfn);
 void pci_check_disable_device(u16 seg, u8 bus, u8 devfn);
 
 uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg);
--
generated by git-patchbot for /home/xen/git/xen.git#master