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

[Chao Gao <chao.gao@xxxxxxxxx>]

On Wed, Jul 05, 2017 at 10:46:39AM +0800, Tian, Kevin wrote:
>> From: Gao, Chao
>> Sent: Monday, July 3, 2017 12:37 PM
>> 
>> On Fri, Jun 30, 2017 at 05:19:52PM +0800, Tian, Kevin wrote:
>> >> From: Gao, Chao
>> >> Sent: Friday, June 30, 2017 9:17 AM
>> >>
>> >> 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.
>> >
>> >Above description looks like the bug is caused by ARI problem. But
>> >if you look at the original code (and the problem you described), it's
>> >not related to ARI. ARI comes just when adding a clean fix, so please
>> >revise the description to make that part clear
>> >
>> 
>> How about this:
>> 
>> 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, a traditional function's BDF should be
>> used to search VT-d unit. Previous code only checks whether Function
>> Number is greater than 7, without checking the prerequisite that the
>
>where did above check come from in original code? 
>
>-        devfn = PCI_SLOT(pdev->info.physfn.devfn) ? 0 : 
>pdev->info.physfn.devfn;
>

Yes. It is the check I described. This line assigns 0 to 'devfn' if PF's
function number > 7. Otherwise, use PF's real devfn.

Thanks
Chao

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