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

[Gregory Etelson <gregory@weka.io>]

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