Re: [Xen-devel] [PATCH v6] VT-d: fix VF of RC integrated PF matched to wrong VT-d unit

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Gao, Chao
> Sent: Wednesday, August 16, 2017 1:12 PM
> 
> The problem is for a VF of RC integrated PF (e.g. PF's BDF is
> 00:02.0), we would wrongly use 00:00.0 to search VT-d unit.
> 
> If a PF is an extended function, the BDF of a traditional function
> within the same device should be used to search VT-d unit. Otherwise,
> the real BDF of PF should be used. According PCI-e spec, an extended
> function is a function within an ARI device and Function Number is
> greater than 7. The original code tried to tell apart Extended
> Function and non-Extended Function through checking PCI_SLOT(),
> missing counterpart of pci_ari_enabled() (this function exists in
> linux kernel) compared to linux kernel. Without checking whether ARI
> is enabled, it incurs a RC integrated PF with PCI_SLOT() >0 is wrongly
> classified to an extended function. Note that a RC integrated function
> isn't within an ARI device and thus cannot be extended function and in
> this case the real BDF should be used.
> 
> This patch introduces a new field, pf_is_extfn, in struct
> pci_dev_info, to indicate whether the physical function is an extended
> function. The new field helps to generate correct BDF to search VT-d
> unit.
> 
> Reported-by: Crawford, Eric R <Eric.R.Crawford@xxxxxxxxx>
> Tested-by: Crawford, Eric R <Eric.R.Crawford@xxxxxxxxx>
> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> ---
>  xen/drivers/passthrough/pci.c      | 6 +++++-
>  xen/drivers/passthrough/vtd/dmar.c | 2 +-
>  xen/include/xen/pci.h              | 1 +
>  3 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index 27bdb71..8c2ba33 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";
> @@ -609,7 +610,9 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>          pcidevs_lock();
>          pdev = pci_get_pdev(seg, info->physfn.bus, info->physfn.devfn);
>          pcidevs_unlock();
> -        if ( !pdev )
> +        if ( pdev )
> +            pf_is_extfn = pdev->info.is_extfn;

besides Roger's comment, can you move above 2 lines inside lock
protection?

> +        else
>              pci_add_device(seg, info->physfn.bus, info->physfn.devfn,
>                             NULL, node);
>          pdev_type = "virtual function";
> @@ -707,6 +710,7 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>                     seg, bus, slot, func, ctrl);
>      }
> 
> +    pdev->info.pf_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..a96558f 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -219,7 +219,7 @@ struct acpi_drhd_unit
> *acpi_find_matched_drhd_unit(const struct pci_dev *pdev)
>      else if ( pdev->info.is_virtfn )
>      {
>          bus = pdev->info.physfn.bus;
> -        devfn = PCI_SLOT(pdev->info.physfn.devfn) ? 0 : pdev-
> >info.physfn.devfn;
> +        devfn = pdev->info.pf_is_extfn ? 0 : pdev->info.physfn.devfn;
>      }
>      else
>      {
> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
> index 59b6e8a..9e76aa0 100644
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -40,6 +40,7 @@
> 
>  struct pci_dev_info {
>      bool_t is_extfn;
> +    bool_t pf_is_extfn; /* Only valid for virtual function */
>      bool_t is_virtfn;
>      struct {
>          u8 bus;
> --
> 1.8.3.1


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