[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4] VT-d: fix VF of RC integrated PF matched to wrong VT-d unit
On Wed, Jul 05, 2017 at 01:18:38PM +0800, Tian, Kevin wrote: >> From: Gao, Chao >> Sent: Wednesday, July 5, 2017 12:28 PM >> >> 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. >> > >sorry I overlooked PCI_SLOT. However your description is still about >the wrong behavior if PF is an extended function. You didn't explain >it's also wrong even when PF is not an extended function. > How about changing the second paragraph to: 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 > 7. But the original code only checks the latter requirement, without checking the former requirement. It incurs that a function whose Function Number > 7 but which isn't within an ARI device (such as RC integrated function with Function Number > 7) is wrongly classified to an extended function and then we wrongly use 0 as 'devfn' to search VT-d unit for this case. Thanks Chao _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |