Re: [PATCH] igb_uio: remove PCI reset during uio device open

[Shijith Thotton <shijith.thotton@caviumnetworks.com>]

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