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

[Chao Gao <chao.gao@intel.com>]

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