* [PATCH v3] VT-d: fix VF of RC integrated PF matched to wrong VT-d unit
@ 2017-06-29 3:21 Chao Gao
2017-06-29 15:42 ` Roger Pau Monné
0 siblings, 1 reply; 3+ messages in thread
From: Chao Gao @ 2017-06-29 3:21 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Venu Busireddy, Crawford Eric R, Jan Beulich, Chao Gao
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@intel.com>
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
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;
}
else
{
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] VT-d: fix VF of RC integrated PF matched to wrong VT-d unit
2017-06-29 3:21 [PATCH v3] VT-d: fix VF of RC integrated PF matched to wrong VT-d unit Chao Gao
@ 2017-06-29 15:42 ` Roger Pau Monné
2017-06-30 1:02 ` Chao Gao
0 siblings, 1 reply; 3+ messages in thread
From: Roger Pau Monné @ 2017-06-29 15:42 UTC (permalink / raw)
To: Chao Gao
Cc: Kevin Tian, Venu Busireddy, Crawford Eric R, Jan Beulich, xen-devel
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@intel.com>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> ---
> 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@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] VT-d: fix VF of RC integrated PF matched to wrong VT-d unit
2017-06-29 15:42 ` Roger Pau Monné
@ 2017-06-30 1:02 ` Chao Gao
0 siblings, 0 replies; 3+ messages in thread
From: Chao Gao @ 2017-06-30 1:02 UTC (permalink / raw)
To: Roger Pau Monné
Cc: Kevin Tian, Venu Busireddy, Crawford Eric R, Jan Beulich, xen-devel
On Thu, Jun 29, 2017 at 04:42:33PM +0100, Roger Pau Monné wrote:
>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@intel.com>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> ---
>> 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.
Yes. Thank you, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-06-30 1:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-29 3:21 [PATCH v3] VT-d: fix VF of RC integrated PF matched to wrong VT-d unit Chao Gao
2017-06-29 15:42 ` Roger Pau Monné
2017-06-30 1:02 ` Chao Gao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.