patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Shijith Thotton <shijith.thotton@caviumnetworks.com>
To: Gregory Etelson <gregory@weka.io>
Cc: "Wu, Jingjing" <jingjing.wu@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"Tan, Jianfeng" <jianfeng.tan@intel.com>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	"Yang, Qiming" <qiming.yang@intel.com>,
	"Patil, Harish" <Harish.Patil@cavium.com>,
	"Zhang, Helin" <helin.zhang@intel.com>,
	"Hu, Xuekun" <xuekun.hu@intel.com>,
	"Li, Xiaoyun" <xiaoyun.li@intel.com>,
	"Thotton, Shijith" <Shijith.Thotton@cavium.com>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] igb_uio: remove PCI reset during uio device open
Date: Fri, 13 Oct 2017 20:06:58 +0530	[thread overview]
Message-ID: <20171013143656.GA25827@localhost.localdomain> (raw)
In-Reply-To: <14115180.Lok5QBBSsB@polaris>

On Tue, Oct 03, 2017 at 02:35:38PM +0300, Gregory Etelson wrote:
> Hello,
> 
> Can we hold with revert until proper solution will be introduced ?
> 
> Regards,
> Gregory
> 
> On Monday, 2 October 2017 21:24:19 IDT Shijith Thotton wrote:
> > On Fri, Sep 29, 2017 at 12:57:22PM +0000, Wu, Jingjing wrote:
> > > Hi, Shijith
> > > 
> > > Only removing the PCI reset in uio device open function is not enough.
> > > 
> > > We faced an issue like:
> > > 
> > > 1. Here is a FVL NIC, generate VF on one port, and then pass-through the
> > > VF by vfio-pci to VM: For example:
> > > echo 1 > /sys/bus/pci/devices/0000\:07\:00.1/sriov_numvfs
> > > modprobe vfio-pci
> > > echo "8086 154c" > /sys/bus/pci/drivers/vfio-pci/new_id
> > > echo 0000:07:0a.0 > /sys/bus/pci/devices/0000\:07\:0a.0/driver/unbind
> > > echo 0000:07:0a.0 > /sys/bus/pci/drivers/vfio-pci/bind
> > > 
> > > 2. Start VM (by QEMU) in the VM, and in VM, bind the passthrough VF to
> > > igb_uio driver 3.Check the MSIX status of that VF, you can see the MSIX
> > > is enabled both in guest and host. For example:
> > > root@ubuntu-4:~ # lspci -vv -s 00:04.0 | grep MSI
> > > 
> > >         Capabilities: [70] MSI-X: Enable+ Count=5 Masked-
> > >         Capabilities: [a0] Express (v2) Endpoint, MSI 00
> > > 
> > > [root@dpdk2]# lspci -vv -s 07:0a.0 | grep MSI
> > > 
> > >         Capabilities: [70] MSI-X: Enable+ Count=5 Masked-
> > >         Capabilities: [a0] Express (v2) Endpoint, MSI 00
> > > 
> > > 4. start dpdk example (e.g. testpmd)
> > > 5. quit the dpdk example
> > > 6. Check the MSIX status of that VF, you can see the MSIX is enabled in
> > > Guest, but disabled on host
> > > 
> > > Such like:
> > > root@ubuntu-4:~ # lspci -vv -s 00:04.0 | grep MSI
> > > 
> > >         Capabilities: [70] MSI-X: Enable+ Count=5 Masked-
> > >         Capabilities: [a0] Express (v2) Endpoint, MSI 00
> > > 
> > > [root@dpdk2 dpdk.org]# lspci -vv -s 07:0a.0 | grep MSI
> > > 
> > >        Capabilities: [70] MSI-X: Enable- Count=5 Masked-
> > >        
> > >         Capabilities: [a0] Express (v2) Endpoint, MSI 00
> > > 
> > > 7. if restart dpdk application again, DPDK in VM cannot get any interrupts
> > > on that VF.
> > > 
> > > 
> > > After investigate, I found current Qemu cannot support pci_reset_function
> > > well if the MSI-X is enabled on that VF.. Because when we use
> > > pci_reset_function to reset VF in in VM, the Qemu captures the control
> > > register reading/writing.
> > > 
> > > In pci_reset_function, it first reads the PCI configure and set FLR reset,
> > > and then writes PCI configure as restoration. But not all the writing are
> > > successful to Host. If we look into the vfio-pci driver, you will find
> > > that, for different PCI CAP ID, the read/write functions are different.
> > > For PCI MSI-X, it cannot be write to host VF. I think that is because
> > > vfio already provides ioctl ops to deal with MSI-X cap.
> > > 
> > > So I think it is a common issue, not only for intel NICs.
> > > 
> > > There may be same ways to fix that:
> > > 
> > > 1. fix Qemu to capture the FLR writing, and sync the Qemu's status on
> > > MSIX.
> > > 2. revert the patch in DPDK which introduced "pci_reset_function".
> > > 3. move the pci_reset_function from open/release func to igb_uio
> > > probe/remove func. 4. move the enable/disable MSIX from probe/remove to
> > > open/release func.
> > > 
> > > Any opinions?
> > 
> > Hi Jingjing,
> > 
> > Thanks for finding the root cause. I'm in for reverting the patch (as there
> > are chances of issues in future), even though option 4 can fix the issue
> > for both side. If there are no expert opinion on this, please proceed with
> > the best option.
> > 
> > Shijith
> > 
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shijith Thotton
> > > > Sent: Tuesday, September 19, 2017 6:24 PM
> > > > To: dev@dpdk.org
> > > > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon
> > > > <thomas@monjalon.net>; Yang, Qiming <qiming.yang@intel.com>; Patil,
> > > > Harish <Harish.Patil@cavium.com>; Zhang, Helin <helin.zhang@intel.com>;
> > > > Gregory Etelson <gregory@weka.io>; Tan, Jianfeng
> > > > <jianfeng.tan@intel.com>; Hu, Xuekun <xuekun.hu@intel.com>; Li, Xiaoyun
> > > > <xiaoyun.li@intel.com>; Thotton, Shijith <Shijith.Thotton@cavium.com>;
> > > > stable@dpdk.org
> > > > Subject: [dpdk-dev] [PATCH] igb_uio: remove PCI reset during uio device
> > > > open
> > > > 
> > > > Issuing reset during uio device open caused PMD init failure for some
> > > > NIC VFs (i40, ixgbe, qede) in host. So this initial reset is removed.
> > > > Bus master enable is kept as part of open since we disable it in uio
> > > > device release.
> > > > 
> > > > Fixes: b58eedfc7dd5 ("igb_uio: issue FLR during open and release of
> > > > device file") Cc: stable@dpdk.org
> > > > 
> > > > Signed-off-by: Shijith Thotton <shijith.thotton@caviumnetworks.com>
> > > > ---
> > > > 
> > > >  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 4 +---
> > > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > > 
> > > > diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > > > b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > > > index 07a19a3..a6c2996 100644
> > > > --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > > > +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > > > @@ -179,9 +179,7 @@ struct rte_uio_pci_dev {
> > > > 
> > > >  	struct rte_uio_pci_dev *udev = info->priv;
> > > >  	struct pci_dev *dev = udev->pdev;
> > > > 
> > > > -	pci_reset_function(dev);
> > > > -
> > > > -	/* set bus master, which was cleared by the reset function */
> > > > +	/* enable bus mastering on the device */
> > > > 
> > > >  	pci_set_master(dev);
> > > >  	
> > > >  	return 0;
> > > > 
> > > > --
> > > > 1.8.3.1
> 
>

Jingjing's patch[1] supersedes this patch, updating it in patchwork.

1. http://dpdk.org/dev/patchwork/patch/30022/

Shijith

      reply	other threads:[~2017-10-13 14:37 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <F5DF4F0E3AFEF648ADC1C3C33AD4DBF16F7E0E71@SHSMSX101.ccr.corp.intel.com>
     [not found] ` <D5DED7B9.160793%Harish.Patil@cavium.com>
     [not found]   ` <F5DF4F0E3AFEF648ADC1C3C33AD4DBF16F7EA3EB@SHSMSX151.ccr.corp.intel.com>
2017-09-15  8:42     ` [dpdk-stable] [dpdk-dev] vf init issue with patch igb_uio: issue FLR during open and release of device file Thomas Monjalon
2017-09-15  9:18       ` Yang, Qiming
2017-09-15  9:25         ` Ferruh Yigit
2017-09-15  9:31         ` Shijith Thotton
2017-09-19 10:24 ` [dpdk-stable] [PATCH] igb_uio: remove PCI reset during uio device open Shijith Thotton
2017-09-20 16:52   ` Ferruh Yigit
2017-09-21 10:00   ` [dpdk-stable] [dpdk-dev] " Luca Boccassi
2017-09-22  2:47   ` [dpdk-stable] " Yang, Qiming
2017-09-29 12:57   ` [dpdk-stable] [dpdk-dev] " Wu, Jingjing
2017-10-02 18:24     ` Shijith Thotton
2017-10-03 11:35       ` Gregory Etelson
2017-10-13 14:36         ` Shijith Thotton [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=20171013143656.GA25827@localhost.localdomain \
    --to=shijith.thotton@caviumnetworks.com \
    --cc=Harish.Patil@cavium.com \
    --cc=Shijith.Thotton@cavium.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=gregory@weka.io \
    --cc=helin.zhang@intel.com \
    --cc=jianfeng.tan@intel.com \
    --cc=jingjing.wu@intel.com \
    --cc=qiming.yang@intel.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=xiaoyun.li@intel.com \
    --cc=xuekun.hu@intel.com \
    /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).