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

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

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
>