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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Jun 29, 2017 at 11:21:53AM +0800, Chao Gao wrote:
> 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.
> 
> From SRIOV spec REV 1.0 section 3.7.3, it says:
> "ARI is not applicable to Root Complex integrated Endpoints; all other
> SR-IOV Capable Devices (Devices that include at least one PF) shall
> implement the ARI Capability in each Function.". So PFs can be classified to
> two kinds: one is RC integrated PF and the other is non-RC integrated PF. The
> former can't support ARI and the latter shall support ARI. For Extended
> Functions, one traditional function's BDF should be used to search VT-d unit.
> And according to PCIe spec, Extened Function means within an ARI device, a
> Function whose Function Number is greater than 7. Thus, the former can't be an
> extended function, while the latter is as long as its devfn > 7, this check is
> exactly what the original code did; The original code wasn't aware the former.
> 
> This patch directly looks up the 'is_extfn' field of PF's struct pci_dev
> to decide whether the PF is a extended function.
> 
> Reported-by: Crawford, Eric R <Eric.R.Crawford@xxxxxxxxx>
> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> ---
>  xen/drivers/passthrough/vtd/dmar.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/passthrough/vtd/dmar.c 
> b/xen/drivers/passthrough/vtd/dmar.c
> index 82040dd..27ff471 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -218,8 +218,17 @@ struct acpi_drhd_unit *acpi_find_matched_drhd_unit(const 
> struct pci_dev *pdev)
>      }
>      else if ( pdev->info.is_virtfn )
>      {
> +        struct pci_dev *physfn;
> +
>          bus = pdev->info.physfn.bus;
> -        devfn = PCI_SLOT(pdev->info.physfn.devfn) ? 0 : 
> pdev->info.physfn.devfn;
> +        /*
> +         * Use 0 as 'devfn' to search VT-d unit when the physical function
> +         * is an Extended Function.
> +         */
> +        pcidevs_lock();
> +        physfn = pci_get_pdev(pdev->seg, bus, pdev->info.physfn.devfn);
> +        pcidevs_unlock();
> +        devfn = (physfn && physfn->info.is_extfn) ? 0 : 
> pdev->info.physfn.devfn;

AFAICT you should only release the pcidevs lock when you are done with
the device, so that a concurrent call to pci_remove_device doesn't
free the device while you are poking at it.

Roger.

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