[PATCH v8] VT-d: use correct BDF for VF to search VT-d unit

[PATCH v8] VT-d: use correct BDF for VF to search VT-d unit

[Chao Gao]

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, the BDF of PF or the BDF of a traditional function may be used. And
it depends on whether the PF is an extended function or not.

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 endpoint to a wrong VT-d unit.

This patch reuses 'is_extfn' field in VF's struct pci_dev_info 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@intel.com>
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
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      |  6 ++++++
 xen/drivers/passthrough/vtd/dmar.c | 12 ++++++------
 xen/include/xen/pci.h              |  4 ++++
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 27bdb71..2a91320 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -599,6 +599,7 @@ 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)
         pdev_type = "device";
@@ -608,6 +609,8 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
     {
         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,
@@ -707,6 +710,9 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
                    seg, bus, slot, func, ctrl);
     }
 
+    /* VF's 'is_extfn' is used to indicate whether PF is an extended function */
+    if ( pdev->info.is_virtfn )
+        pdev->info.is_extfn = pf_is_extfn;
     check_pdev(pdev);
 
     ret = 0;
diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
index 82040dd..75c9c92 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 ? 0 : pdev->info.physfn.devfn;
     }
-    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..ea86f9f 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 {
+    /*
+     * Considering VF's 'is_extfn' field isn't used, we reuse VF's 'is_extfn'
+     * field to show whether the PF of this VF is an extended function.
+     */
     bool_t is_extfn;
     bool_t is_virtfn;
     struct {
-- 
1.8.3.1


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

Re: [PATCH v8] VT-d: use correct BDF for VF to search VT-d unit

[Chao Gao]

I have sent out a new version, let's skip this one.

Thanks
Chao

On Fri, Aug 25, 2017 at 12:17:15PM +0800, Chao Gao wrote:
>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, the BDF of PF or the BDF of a traditional function may be used. And
>it depends on whether the PF is an extended function or not.
>
>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 endpoint to a wrong VT-d unit.
>
>This patch reuses 'is_extfn' field in VF's struct pci_dev_info 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@intel.com>
>Signed-off-by: Chao Gao <chao.gao@intel.com>
>---
>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      |  6 ++++++
> xen/drivers/passthrough/vtd/dmar.c | 12 ++++++------
> xen/include/xen/pci.h              |  4 ++++
> 3 files changed, 16 insertions(+), 6 deletions(-)
>
>diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
>index 27bdb71..2a91320 100644
>--- a/xen/drivers/passthrough/pci.c
>+++ b/xen/drivers/passthrough/pci.c
>@@ -599,6 +599,7 @@ 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)
>         pdev_type = "device";
>@@ -608,6 +609,8 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>     {
>         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,
>@@ -707,6 +710,9 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>                    seg, bus, slot, func, ctrl);
>     }
> 
>+    /* VF's 'is_extfn' is used to indicate whether PF is an extended function */
>+    if ( pdev->info.is_virtfn )
>+        pdev->info.is_extfn = pf_is_extfn;
>     check_pdev(pdev);
> 
>     ret = 0;
>diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
>index 82040dd..75c9c92 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 ? 0 : pdev->info.physfn.devfn;
>     }
>-    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..ea86f9f 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 {
>+    /*
>+     * Considering VF's 'is_extfn' field isn't used, we reuse VF's 'is_extfn'
>+     * field to show whether the PF of this VF is an extended function.
>+     */
>     bool_t is_extfn;
>     bool_t is_virtfn;
>     struct {
>-- 
>1.8.3.1
>

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