[Xen-devel] [PATCH v10] VT-d: use correct BDF for VF to search VT-d unit

[Chao Gao <chao.gao@xxxxxxxxx>]

When SR-IOV is enabled, 'Virtual Functions' of a 'Physical Function'
are under the scope of the same VT-d unit as the 'Physical Function'.
A 'Physical Function' can be a 'Traditional Function' or an ARI
'Extended Function'. And furthermore, 'Extended Functions' on an
endpoint are under the scope of the same VT-d unit as the 'Traditional
Functions' on the endpoint. To search VT-d unit for a VF, if its PF
isn't an extended function, the BDF of PF should be used. Otherwise
the BDF of a traditional function in the same device with the PF
should be used.

Current code uses PCI_SLOT() to recognize an ARI 'Extended Funcion'.
But it is conceptually wrong w/o checking whether PF is an extended
function and would lead to match VFs of a RC integrated PF to a wrong
VT-d unit.

This patch overrides VF 'is_extfn' field and uses this field to
indicate whether the PF of this VF is an extended function. The field
helps to use correct BDF to search VT-d unit.

Reported-by: Crawford, Eric R <Eric.R.Crawford@xxxxxxxxx>
Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
---
v10:
 - move setting vf's is_extfn closer to the place where we set other fields.
 - reverse the conditional expression in acpi_find_matched_drhd_unit()

v9:
 - check 'is_virtfn' first in pci_add_device() to avoid potential error if
 linux side sets VF's 'is_extfn'
 - comments changes to make it clear that we use VF's 'is_extfn' intentionally
 otherwise the patch seems like a workaround.

v8:
 - use "conceptually wrong", instead of "a corner case" in commit message
 - check 'is_virtfn' first in acpi_find_matched_drhd_unit()

v7:
 - Drop Eric's tested-by
 - Change commit message to be clearer
 - Re-use VF's is_extfn field
 - access PF's is_extfn field in locked area

---
 xen/drivers/passthrough/pci.c      | 19 +++++++++++++++----
 xen/drivers/passthrough/vtd/dmar.c | 12 ++++++------
 xen/include/xen/pci.h              |  4 ++++
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 27bdb71..187a9e7 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -599,21 +599,24 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
     unsigned int slot = PCI_SLOT(devfn), func = PCI_FUNC(devfn);
     const char *pdev_type;
     int ret;
+    bool pf_is_extfn = false;
 
-    if (!info)
+    if ( !info )
         pdev_type = "device";
-    else if (info->is_extfn)
-        pdev_type = "extended function";
-    else if (info->is_virtfn)
+    else if ( info->is_virtfn )
     {
         pcidevs_lock();
         pdev = pci_get_pdev(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);
         pdev_type = "virtual function";
     }
+    else if ( info->is_extfn )
+        pdev_type = "extended function";
     else
     {
         info = NULL;
@@ -637,7 +640,15 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
     pdev->node = node;
 
     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;
+    }
     else if ( !pdev->vf_rlen[0] )
     {
         unsigned int pos = pci_find_ext_capability(seg, bus, devfn,
diff --git a/xen/drivers/passthrough/vtd/dmar.c 
b/xen/drivers/passthrough/vtd/dmar.c
index 82040dd..9676471 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -211,15 +211,15 @@ struct acpi_drhd_unit *acpi_find_matched_drhd_unit(const 
struct pci_dev *pdev)
     if ( pdev == NULL )
         return NULL;
 
-    if ( pdev->info.is_extfn )
+    if ( pdev->info.is_virtfn )
     {
-        bus = pdev->bus;
-        devfn = 0;
+        bus = pdev->info.physfn.bus;
+        devfn = (!pdev->info.is_extfn) ? pdev->info.physfn.devfn : 0;
     }
-    else if ( pdev->info.is_virtfn )
+    else if ( pdev->info.is_extfn )
     {
-        bus = pdev->info.physfn.bus;
-        devfn = PCI_SLOT(pdev->info.physfn.devfn) ? 0 : 
pdev->info.physfn.devfn;
+        bus = pdev->bus;
+        devfn = 0;
     }
     else
     {
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 59b6e8a..da1bd22 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -39,6 +39,10 @@
 #define PCI_SBDF3(s,b,df) ((((s) & 0xffff) << 16) | PCI_BDF2(b, df))
 
 struct pci_dev_info {
+    /*
+     * VF's 'is_extfn' field is used to indicate whether its PF is an extended
+     * function.
+     */
     bool_t is_extfn;
     bool_t is_virtfn;
     struct {
-- 
1.8.3.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel