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

[Chao Gao <chao.gao@xxxxxxxxx>]

On Thu, Aug 17, 2017 at 03:43:07PM +0800, Tian, Kevin wrote:
>> From: Gao, Chao
>> Sent: Wednesday, August 16, 2017 1:12 PM
>> 
>> 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, 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 is
>> greater than 7. The original code tried to tell apart Extended
>> Function and non-Extended Function through checking PCI_SLOT(),
>> missing counterpart of pci_ari_enabled() (this function exists in
>> linux kernel) compared to linux kernel. Without checking whether ARI
>> is enabled, it incurs a RC integrated PF with PCI_SLOT() >0 is wrongly
>> classified to an extended function. Note that a RC integrated function
>> isn't within an ARI device and thus cannot be extended function and in
>> this case the real BDF should be used.
>> 
>> This patch introduces a new field, pf_is_extfn, in struct
>> pci_dev_info, to indicate whether the physical function is an extended
>> function. The new field helps to generate correct BDF to search VT-d
>> unit.
>> 
>> Reported-by: Crawford, Eric R <Eric.R.Crawford@xxxxxxxxx>
>> Tested-by: Crawford, Eric R <Eric.R.Crawford@xxxxxxxxx>
>> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
>> ---
>>  xen/drivers/passthrough/pci.c      | 6 +++++-
>>  xen/drivers/passthrough/vtd/dmar.c | 2 +-
>>  xen/include/xen/pci.h              | 1 +
>>  3 files changed, 7 insertions(+), 2 deletions(-)
>> 
>> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
>> index 27bdb71..8c2ba33 100644
>> --- a/xen/drivers/passthrough/pci.c
>> +++ b/xen/drivers/passthrough/pci.c
>> @@ -599,6 +599,7 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>>      unsigned int slot = PCI_SLOT(devfn), func = PCI_FUNC(devfn);
>>      const char *pdev_type;
>>      int ret;
>> +    bool pf_is_extfn = false;
>> 
>>      if (!info)
>>          pdev_type = "device";
>> @@ -609,7 +610,9 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>>          pcidevs_lock();
>>          pdev = pci_get_pdev(seg, info->physfn.bus, info->physfn.devfn);
>>          pcidevs_unlock();
>> -        if ( !pdev )
>> +        if ( pdev )
>> +            pf_is_extfn = pdev->info.is_extfn;
>
>besides Roger's comment, can you move above 2 lines inside lock
>protection?
>

Hi, Kevin and Roger.

I sent out a new version recently. The new version adopts all your
suggestions. Please review it.

Thanks
Chao

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