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

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

[Chao Gao]

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 > 7. Apparently, it treats all
PFs which has devfn > 7 as extended function. However, it is wrong for
a RC integrated PF, which is not ARI-capable but may have devfn > 7.

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 | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
index 82040dd..3ba33c7 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -218,8 +218,18 @@ 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();
+        ASSERT(physfn);
+        devfn = 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

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

[Jan Beulich]

>>> On 21.06.17 at 12:47, <chao.gao@intel.com> 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 > 7. Apparently, it treats all
> PFs which has devfn > 7 as extended function. However, it is wrong for
> a RC integrated PF, which is not ARI-capable but may have devfn > 7.

I'm again having trouble with you talking about ARI and RC
integrated here, but not checking for either in any way in the
new code. Please make sure you establish the full connection
in the description.

> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -218,8 +218,18 @@ struct acpi_drhd_unit *acpi_find_matched_drhd_unit(const 
> struct pci_dev *pdev)
>      }
>      else if ( pdev->info.is_virtfn )
>      {
> +        struct pci_dev *physfn;

const

>          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();
> +        ASSERT(physfn);
> +        devfn = physfn->info.is_extfn ? 0 : pdev->info.physfn.devfn;

This change looks to be fine is we assume that is_extfn is always
set correctly. Looking at the Linux code setting it, I'm not sure
though: I can't see any connection to the PF needing to be RC
integrated there.

I'd also suggest doing error handling not by ASSERT(), but by
checking physfn in the conditional expression.

Jan


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

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

[Chao Gao]

On Thu, Jun 22, 2017 at 03:26:04AM -0600, Jan Beulich wrote:
>>>> On 21.06.17 at 12:47, <chao.gao@intel.com> 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 > 7. Apparently, it treats all
>> PFs which has devfn > 7 as extended function. However, it is wrong for
>> a RC integrated PF, which is not ARI-capable but may have devfn > 7.
>
>I'm again having trouble with you talking about ARI and RC
>integrated here, but not checking for either in any way in the
>new code. Please make sure you establish the full connection
>in the description.

Sorry for this. Let me explain this again.

From SRIOV spec 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 I _think_ 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.
The latter shall support ARI. Only for extended functions, one
traditional function's BDF should be used to search VT-d unit. And
according to PCIE spec, Extended function means within an ARI Device, a
Function whose Function Number is greater than 7. So the former
can't be an extended function. The latter is an extended function as
long as PF's devfn > 7, this check is exactly what the original code
did. So I think the original code didn't aware the former
(aka, RC integrated endpoints.). This patch checks the is_extfn
directly. All of this is only my understanding. I need you and Kevin's
help to decide it's right or not.

>
>> --- a/xen/drivers/passthrough/vtd/dmar.c
>> +++ b/xen/drivers/passthrough/vtd/dmar.c
>> @@ -218,8 +218,18 @@ struct acpi_drhd_unit *acpi_find_matched_drhd_unit(const 
>> struct pci_dev *pdev)
>>      }
>>      else if ( pdev->info.is_virtfn )
>>      {
>> +        struct pci_dev *physfn;
>
>const
>
>>          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();
>> +        ASSERT(physfn);
>> +        devfn = physfn->info.is_extfn ? 0 : pdev->info.physfn.devfn;
>
>This change looks to be fine is we assume that is_extfn is always
>set correctly. Looking at the Linux code setting it, I'm not sure
>though: I can't see any connection to the PF needing to be RC
>integrated there.

Linux code sets it when
 pci_ari_enabled(pci_dev->bus) && PCI_SLOT(pci_dev->devfn)

 I _think_ pci_ari_enabled(pci_dev->bus) means ARIforwarding is enabled
 in the immediatedly upstream Downstream port. Thus, I think the pci_dev
 is an ARI-capable device for PCIe spec 6.13 says:

It is strongly recommended that software in general Set the ARI
Forwarding Enable bit in a 5 Downstream Port only if software is certain
that the device immediately below the Downstream Port is an ARI Device.
If the bit is Set when a non-ARI Device is present, the non-ARI Device
can respond to Configuration Space accesses under what it interprets as
being different Device Numbers, and its Functions can be aliased under
multiple Device Numbers, generally leading to undesired behavior.

and the pci_dev can't be a RC integrated endpoints. From another side, it
also means the is_extfn won't be set for RC integrated PF. Is that
right?

>
>I'd also suggest doing error handling not by ASSERT(), but by
>checking physfn in the conditional expression.

do you mean this：
devfn = (physfn && physfn->info.is_extfn) ? 0 : pdev->info.physfn.devfn;

Thanks
Chao

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

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

[Jan Beulich]

>>> On 22.06.17 at 16:21, <chao.gao@intel.com> wrote:
> On Thu, Jun 22, 2017 at 03:26:04AM -0600, Jan Beulich wrote:
>>>>> On 21.06.17 at 12:47, <chao.gao@intel.com> 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 > 7. Apparently, it treats all
>>> PFs which has devfn > 7 as extended function. However, it is wrong for
>>> a RC integrated PF, which is not ARI-capable but may have devfn > 7.
>>
>>I'm again having trouble with you talking about ARI and RC
>>integrated here, but not checking for either in any way in the
>>new code. Please make sure you establish the full connection
>>in the description.
> 
> Sorry for this. Let me explain this again.
> 
> From SRIOV spec 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 I _think_ 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.
> The latter shall support ARI. Only for extended functions, one
> traditional function's BDF should be used to search VT-d unit. And
> according to PCIE spec, Extended function means within an ARI Device, a
> Function whose Function Number is greater than 7. So the former
> can't be an extended function. The latter is an extended function as
> long as PF's devfn > 7, this check is exactly what the original code
> did. So I think the original code didn't aware the former
> (aka, RC integrated endpoints.). This patch checks the is_extfn
> directly. All of this is only my understanding. I need you and Kevin's
> help to decide it's right or not.

This makes sense to me, but as said, the patch description will need
to include this in some form.

>>> --- a/xen/drivers/passthrough/vtd/dmar.c
>>> +++ b/xen/drivers/passthrough/vtd/dmar.c
>>> @@ -218,8 +218,18 @@ struct acpi_drhd_unit *acpi_find_matched_drhd_unit(const 
>>> struct pci_dev *pdev)
>>>      }
>>>      else if ( pdev->info.is_virtfn )
>>>      {
>>> +        struct pci_dev *physfn;
>>
>>const
>>
>>>          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();
>>> +        ASSERT(physfn);
>>> +        devfn = physfn->info.is_extfn ? 0 : pdev->info.physfn.devfn;
>>
>>This change looks to be fine is we assume that is_extfn is always
>>set correctly. Looking at the Linux code setting it, I'm not sure
>>though: I can't see any connection to the PF needing to be RC
>>integrated there.
> 
> Linux code sets it when
>  pci_ari_enabled(pci_dev->bus) && PCI_SLOT(pci_dev->devfn)
> 
>  I _think_ pci_ari_enabled(pci_dev->bus) means ARIforwarding is enabled
>  in the immediatedly upstream Downstream port. Thus, I think the pci_dev
>  is an ARI-capable device for PCIe spec 6.13 says:
> 
> It is strongly recommended that software in general Set the ARI
> Forwarding Enable bit in a 5 Downstream Port only if software is certain
> that the device immediately below the Downstream Port is an ARI Device.
> If the bit is Set when a non-ARI Device is present, the non-ARI Device
> can respond to Configuration Space accesses under what it interprets as
> being different Device Numbers, and its Functions can be aliased under
> multiple Device Numbers, generally leading to undesired behavior.
> 
> and the pci_dev can't be a RC integrated endpoints. From another side, it
> also means the is_extfn won't be set for RC integrated PF. Is that
> right?

Well, I'm not sure about the Linux parts here? Konrad, do you
happen to know? Or do you know someone who does?

>>I'd also suggest doing error handling not by ASSERT(), but by
>>checking physfn in the conditional expression.
> 
> do you mean this：
> devfn = (physfn && physfn->info.is_extfn) ? 0 : pdev->info.physfn.devfn;

Yes.

Jan

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

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

[Konrad Rzeszutek Wilk]

On Thu, Jun 22, 2017 at 09:31:50AM -0600, Jan Beulich wrote:
> >>> On 22.06.17 at 16:21, <chao.gao@intel.com> wrote:
> > On Thu, Jun 22, 2017 at 03:26:04AM -0600, Jan Beulich wrote:
> >>>>> On 21.06.17 at 12:47, <chao.gao@intel.com> 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 > 7. Apparently, it treats all
> >>> PFs which has devfn > 7 as extended function. However, it is wrong for
> >>> a RC integrated PF, which is not ARI-capable but may have devfn > 7.
> >>
> >>I'm again having trouble with you talking about ARI and RC
> >>integrated here, but not checking for either in any way in the
> >>new code. Please make sure you establish the full connection
> >>in the description.
> > 
> > Sorry for this. Let me explain this again.
> > 
> > From SRIOV spec 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 I _think_ 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.
> > The latter shall support ARI. Only for extended functions, one
> > traditional function's BDF should be used to search VT-d unit. And
> > according to PCIE spec, Extended function means within an ARI Device, a
> > Function whose Function Number is greater than 7. So the former
> > can't be an extended function. The latter is an extended function as
> > long as PF's devfn > 7, this check is exactly what the original code
> > did. So I think the original code didn't aware the former
> > (aka, RC integrated endpoints.). This patch checks the is_extfn
> > directly. All of this is only my understanding. I need you and Kevin's
> > help to decide it's right or not.
> 
> This makes sense to me, but as said, the patch description will need
> to include this in some form.
> 
> >>> --- a/xen/drivers/passthrough/vtd/dmar.c
> >>> +++ b/xen/drivers/passthrough/vtd/dmar.c
> >>> @@ -218,8 +218,18 @@ struct acpi_drhd_unit *acpi_find_matched_drhd_unit(const 
> >>> struct pci_dev *pdev)
> >>>      }
> >>>      else if ( pdev->info.is_virtfn )
> >>>      {
> >>> +        struct pci_dev *physfn;
> >>
> >>const
> >>
> >>>          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();
> >>> +        ASSERT(physfn);
> >>> +        devfn = physfn->info.is_extfn ? 0 : pdev->info.physfn.devfn;
> >>
> >>This change looks to be fine is we assume that is_extfn is always
> >>set correctly. Looking at the Linux code setting it, I'm not sure
> >>though: I can't see any connection to the PF needing to be RC
> >>integrated there.
> > 
> > Linux code sets it when
> >  pci_ari_enabled(pci_dev->bus) && PCI_SLOT(pci_dev->devfn)
> > 
> >  I _think_ pci_ari_enabled(pci_dev->bus) means ARIforwarding is enabled
> >  in the immediatedly upstream Downstream port. Thus, I think the pci_dev
> >  is an ARI-capable device for PCIe spec 6.13 says:
> > 
> > It is strongly recommended that software in general Set the ARI
> > Forwarding Enable bit in a 5 Downstream Port only if software is certain
> > that the device immediately below the Downstream Port is an ARI Device.
> > If the bit is Set when a non-ARI Device is present, the non-ARI Device
> > can respond to Configuration Space accesses under what it interprets as
> > being different Device Numbers, and its Functions can be aliased under
> > multiple Device Numbers, generally leading to undesired behavior.
> > 
> > and the pci_dev can't be a RC integrated endpoints. From another side, it
> > also means the is_extfn won't be set for RC integrated PF. Is that
> > right?
> 
> Well, I'm not sure about the Linux parts here? Konrad, do you
> happen to know? Or do you know someone who does?

Including Govinda and Venu,

> 
> >>I'd also suggest doing error handling not by ASSERT(), but by
> >>checking physfn in the conditional expression.
> > 
> > do you mean this：
> > devfn = (physfn && physfn->info.is_extfn) ? 0 : pdev->info.physfn.devfn;
> 
> Yes.
> 
> Jan

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

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

[Venu Busireddy]

On 2017-06-22 11:52:50 -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Jun 22, 2017 at 09:31:50AM -0600, Jan Beulich wrote:
> > >>> On 22.06.17 at 16:21, <chao.gao@intel.com> wrote:
> > > On Thu, Jun 22, 2017 at 03:26:04AM -0600, Jan Beulich wrote:
> > >>>>> On 21.06.17 at 12:47, <chao.gao@intel.com> 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 > 7. Apparently, it treats all
> > >>> PFs which has devfn > 7 as extended function. However, it is wrong for
> > >>> a RC integrated PF, which is not ARI-capable but may have devfn > 7.
> > >>
> > >>I'm again having trouble with you talking about ARI and RC
> > >>integrated here, but not checking for either in any way in the
> > >>new code. Please make sure you establish the full connection
> > >>in the description.
> > > 
> > > Sorry for this. Let me explain this again.
> > > 
> > > From SRIOV spec 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 I _think_ 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.
> > > The latter shall support ARI. Only for extended functions, one
> > > traditional function's BDF should be used to search VT-d unit. And
> > > according to PCIE spec, Extended function means within an ARI Device, a
> > > Function whose Function Number is greater than 7. So the former
> > > can't be an extended function. The latter is an extended function as
> > > long as PF's devfn > 7, this check is exactly what the original code
> > > did. So I think the original code didn't aware the former
> > > (aka, RC integrated endpoints.). This patch checks the is_extfn
> > > directly. All of this is only my understanding. I need you and Kevin's
> > > help to decide it's right or not.
> > 
> > This makes sense to me, but as said, the patch description will need
> > to include this in some form.
> > 
> > >>> --- a/xen/drivers/passthrough/vtd/dmar.c
> > >>> +++ b/xen/drivers/passthrough/vtd/dmar.c
> > >>> @@ -218,8 +218,18 @@ struct acpi_drhd_unit *acpi_find_matched_drhd_unit(const 
> > >>> struct pci_dev *pdev)
> > >>>      }
> > >>>      else if ( pdev->info.is_virtfn )
> > >>>      {
> > >>> +        struct pci_dev *physfn;
> > >>
> > >>const
> > >>
> > >>>          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();
> > >>> +        ASSERT(physfn);
> > >>> +        devfn = physfn->info.is_extfn ? 0 : pdev->info.physfn.devfn;
> > >>
> > >>This change looks to be fine is we assume that is_extfn is always
> > >>set correctly. Looking at the Linux code setting it, I'm not sure
> > >>though: I can't see any connection to the PF needing to be RC
> > >>integrated there.
> > > 
> > > Linux code sets it when
> > >  pci_ari_enabled(pci_dev->bus) && PCI_SLOT(pci_dev->devfn)
> > > 
> > >  I _think_ pci_ari_enabled(pci_dev->bus) means ARIforwarding is enabled
> > >  in the immediatedly upstream Downstream port. Thus, I think the pci_dev
> > >  is an ARI-capable device for PCIe spec 6.13 says:
> > > 
> > > It is strongly recommended that software in general Set the ARI
> > > Forwarding Enable bit in a 5 Downstream Port only if software is certain
> > > that the device immediately below the Downstream Port is an ARI Device.
> > > If the bit is Set when a non-ARI Device is present, the non-ARI Device
> > > can respond to Configuration Space accesses under what it interprets as
> > > being different Device Numbers, and its Functions can be aliased under
> > > multiple Device Numbers, generally leading to undesired behavior.
> > > 
> > > and the pci_dev can't be a RC integrated endpoints. From another side, it
> > > also means the is_extfn won't be set for RC integrated PF. Is that
> > > right?
> > 
> > Well, I'm not sure about the Linux parts here? Konrad, do you
> > happen to know? Or do you know someone who does?

pci_ari_enabled() and related code trusts that an RC integrated endpoint
does not present the PCI_EXT_CAP_ID_ARI capability. As long as we do
not have rogue endpoints that don't follow the spec, this code works fine.

> 
> Including Govinda and Venu,
> 
> > 
> > >>I'd also suggest doing error handling not by ASSERT(), but by
> > >>checking physfn in the conditional expression.
> > > 
> > > do you mean this???
> > > devfn = (physfn && physfn->info.is_extfn) ? 0 : pdev->info.physfn.devfn;
> > 
> > Yes.
> > 
> > Jan

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