xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Chao Gao <chao.gao@intel.com>
To: "Tian, Kevin" <kevin.tian@intel.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	Jan Beulich <jbeulich@suse.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v6] VT-d: fix VF of RC integrated PF matched to wrong VT-d unit
Date: Tue, 22 Aug 2017 13:03:47 +0800	[thread overview]
Message-ID: <20170822050344.GA15606@op-computing> (raw)
In-Reply-To: <AADFC41AFE54684AB9EE6CBC0274A5D190D76B99@SHSMSX101.ccr.corp.intel.com>

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@intel.com>
>> Tested-by: Crawford, Eric R <Eric.R.Crawford@intel.com>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> ---
>>  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@lists.xen.org
https://lists.xen.org/xen-devel

      reply	other threads:[~2017-08-22  5:03 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-16  5:12 [PATCH v6] VT-d: fix VF of RC integrated PF matched to wrong VT-d unit Chao Gao
2017-08-16  8:17 ` [patch v6] vt-d: fix vf of rc integrated pf matched to wrong vt-d unit Roger Pau Monné
2017-08-16  8:50   ` Chao Gao
2017-08-16  9:10     ` Roger Pau Monné
2017-08-17  7:43 ` [PATCH v6] VT-d: fix VF of RC integrated PF matched to wrong VT-d unit Tian, Kevin
2017-08-22  5:03   ` Chao Gao [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170822050344.GA15606@op-computing \
    --to=chao.gao@intel.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=kevin.tian@intel.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).