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 Wed, Aug 16, 2017 at 09:17:46AM +0100, Roger Pau Monné wrote:
>on wed, aug 16, 2017 at 01:12:24pm +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.
>> 
>> 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.
>
>AFAIK, extended functions simply remove the slot and extend the
>function number to [0, 255], so it seems correct to expect that the
>VT-d unit search should be done using the bus and extended function
>parameters, and assume slot is 0. Is this some kind of limitation of
>VT-d?

VT-d spec makes such provision for VT-d unit search without any
explaination. But I think it isn't. Whether we can find the right VT-d unit
depends on DMAR. So I would rather regard it as firmware doesn't prepare
entries for extended functions in DMAR.

>
>> 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.
>
>[...]
>> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
>> index 59b6e8a..9e76aa0 100644
>> --- a/xen/include/xen/pci.h
>> +++ b/xen/include/xen/pci.h
>> @@ -40,6 +40,7 @@
>>  
>>  struct pci_dev_info {
>>      bool_t is_extfn;
>> +    bool_t pf_is_extfn; /* Only valid for virtual function */
>
>Can't you just re-use is_virtfn and is_extfn, and when both are true
>it means the pf where this vf belongs is an extended function?

Yes. Reuse vf's is_extfn field is possible.

Thanks
Chao

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