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

[Chao Gao <chao.gao@xxxxxxxxx>]

On Fri, Jun 16, 2017 at 09:52:11AM -0600, Jan Beulich wrote:
>>>> On 16.06.17 at 08:48, <chao.gao@xxxxxxxxx> wrote:
>> The problem is 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.
>> 
>> To search VT-d unit for a VF, the BDF of the PF is used. And If the
>> PF is an Extended Function, the BDF of one traditional function is
>> used. The following line (from acpi_find_matched_drhd_unit()):
>>     devfn = PCI_SLOT(pdev->info.physfn.devfn) ? 0 : pdev->info.physfn.devfn;
>> sets 'devfn' to 0 if PF's devfn > 8.
>
>Is that really the relevant line? Since you say PF is an Extended
>Function, wouldn't
>
>    if ( pdev->info.is_extfn )
>    {
>        bus = pdev->bus;
>        devfn = 0;
>    }
>
>be the relevant code? Or else - is is_extfn not being set correctly?
>

I think this field is not being set for VF. And here what we want to
know is whether the PF of this VF is an extended functin. We also can add
a new field 'is_extfn' in pdev->info.physfn and change the caller in
linux kernel accordingly. But it will be not compatible with the old kernel.

>
>> --- a/xen/drivers/passthrough/vtd/dmar.c
>> +++ b/xen/drivers/passthrough/vtd/dmar.c
>> @@ -219,7 +219,12 @@ 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;
>> +        /* ARI is not appliable to Root Complex Integrated Endpoints */
>> +        if ( PCI_SLOT(pdev->info.physfn.devfn) &&
>> +             (pdev->type != DEV_TYPE_RC_ENDPOINT) )
>> +            devfn = 0;
>> +        else
>> +            devfn = pdev->info.physfn.devfn;
>>      }
>
>While I think I understand some of PCI, I have to admit that the
>connection to ARI is not at all obvious to me at this place in the
>sources. Hence I'd appreciate if you would extend the comment.

Ok. How about this:

-        devfn = PCI_SLOT(pdev->info.physfn.devfn) ? 0 : 
pdev->info.physfn.devfn;
+        /* 
+         * Use 0 as the devfn to search VT-d unit when the PF is an Extended
+         * Function (means within an ARI Device, a Function whose Function 
Number
+         * is greater than 7).
+         */
+        if ( PCI_SLOT(pdev->info.physfn.devfn) &&
+             (pci_find_ext_capability(pdev->seg, bus,
+                         pdev->info.physfn.devfn, PCI_EXT_CAP_ID_ARI)) )
+            devfn = 0;
+        else
+            devfn = pdev->info.physfn.devfn;

Thanks
Chao

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