Re: [dpdk-stable] [dpdk-dev] vf init issue with patch igb_uio: issue FLR during open and release of device file

Re: [dpdk-stable] [dpdk-dev] vf init issue with patch igb_uio: issue FLR during open and release of device file

[Thomas Monjalon]

Hi

15/09/2017 10:04, Yang, Qiming:
> Hi, Thomas
> We meet a urgent issue because of Shijith's patch. 
> When use DPDK version 17.08, DPDK VF driver and kernel PF driver, i40e, ixgbe and qede(Harish said) VF all meet initial failure.

This patch has been integrated in DPDK 17.08-rc2 (2 months ago).
Why a so obvious issue is discovered only now?

> This issue only occurred when use 17.08 igb_uio, vfio-pci and 17.05 igb_uio all works well.
> Considering this issue will block our vf driver develop and test work, could we revert this patch first and find another appropriate way?

We need to be sure the revert is the right solution because
it will be backported to the stable 17.08 release.

Shijith suggests to "try removing pci_reset_function from igbuio_pci_open."
Can we try this first?
Are we sure the bug is not in new firmwares?

Re: [dpdk-stable] [dpdk-dev] vf init issue with patch igb_uio: issue FLR during open and release of device file

[Yang, Qiming]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, September 15, 2017 4:43 PM
> To: Yang, Qiming <qiming.yang@intel.com>
> Cc: dev@dpdk.org; Tan, Jianfeng <jianfeng.tan@intel.com>; Thotton, Shijith
> <Shijith.Thotton@cavium.com>; Hu, Xuekun <xuekun.hu@intel.com>; Yigit,
> Ferruh <ferruh.yigit@intel.com>; Patil, Harish <Harish.Patil@cavium.com>;
> Gregory Etelson <gregory@weka.io>; stable@dpdk.org
> Subject: Re: [dpdk-dev] vf init issue with patch igb_uio: issue FLR during open
> and release of device file
> 
> Hi
> 
> 15/09/2017 10:04, Yang, Qiming:
> > Hi, Thomas
> > We meet a urgent issue because of Shijith's patch.
> > When use DPDK version 17.08, DPDK VF driver and kernel PF driver, i40e, ixgbe
> and qede(Harish said) VF all meet initial failure.
> 
> This patch has been integrated in DPDK 17.08-rc2 (2 months ago).
> Why a so obvious issue is discovered only now?
> 
> > This issue only occurred when use 17.08 igb_uio, vfio-pci and 17.05 igb_uio all
> works well.
> > Considering this issue will block our vf driver develop and test work, could we
> revert this patch first and find another appropriate way?
> 
> We need to be sure the revert is the right solution because it will be backported
> to the stable 17.08 release.
> 
> Shijith suggests to "try removing pci_reset_function from igbuio_pci_open."
> Can we try this first?
> Are we sure the bug is not in new firmwares?

I have tried to remove pci_reset_function, it works in my environment, but it doesn't work in someone's enviroment.
And removing this function won't it changes the original purpose of his patch?  So I'm not sure it is a good solution.
This bug is not a firmware version related issue, we can reproduce stably with older firmware.

Qiming

Re: [dpdk-stable] [dpdk-dev] vf init issue with patch igb_uio: issue FLR during open and release of device file

[Ferruh Yigit]

On 9/15/2017 10:18 AM, Yang, Qiming wrote:
>> -----Original Message-----
>> From: Thomas Monjalon [mailto:thomas@monjalon.net]
>> Sent: Friday, September 15, 2017 4:43 PM
>> To: Yang, Qiming <qiming.yang@intel.com>
>> Cc: dev@dpdk.org; Tan, Jianfeng <jianfeng.tan@intel.com>; Thotton, Shijith
>> <Shijith.Thotton@cavium.com>; Hu, Xuekun <xuekun.hu@intel.com>; Yigit,
>> Ferruh <ferruh.yigit@intel.com>; Patil, Harish <Harish.Patil@cavium.com>;
>> Gregory Etelson <gregory@weka.io>; stable@dpdk.org
>> Subject: Re: [dpdk-dev] vf init issue with patch igb_uio: issue FLR during open
>> and release of device file
>>
>> Hi
>>
>> 15/09/2017 10:04, Yang, Qiming:
>>> Hi, Thomas
>>> We meet a urgent issue because of Shijith's patch.
>>> When use DPDK version 17.08, DPDK VF driver and kernel PF driver, i40e, ixgbe
>> and qede(Harish said) VF all meet initial failure.
>>
>> This patch has been integrated in DPDK 17.08-rc2 (2 months ago).
>> Why a so obvious issue is discovered only now?
>>
>>> This issue only occurred when use 17.08 igb_uio, vfio-pci and 17.05 igb_uio all
>> works well.
>>> Considering this issue will block our vf driver develop and test work, could we
>> revert this patch first and find another appropriate way?
>>
>> We need to be sure the revert is the right solution because it will be backported
>> to the stable 17.08 release.
>>
>> Shijith suggests to "try removing pci_reset_function from igbuio_pci_open."
>> Can we try this first?
>> Are we sure the bug is not in new firmwares?
> 
> I have tried to remove pci_reset_function, it works in my environment, but it doesn't work in someone's enviroment.
> And removing this function won't it changes the original purpose of his patch?  So I'm not sure it is a good solution.

Yes, removing that will be functionally almost same with revert.

> This bug is not a firmware version related issue, we can reproduce stably with older firmware.

I also think this is not related to the firmware.
But I don't see why works with one version of the Linux PF driver, but
not other.

> 
> Qiming
> 

Re: [dpdk-stable] [dpdk-dev] vf init issue with patch igb_uio: issue FLR during open and release of device file

[Shijith Thotton]

On Fri, Sep 15, 2017 at 09:18:00AM +0000, Yang, Qiming wrote:
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Friday, September 15, 2017 4:43 PM
> > To: Yang, Qiming <qiming.yang@intel.com>
> > Cc: dev@dpdk.org; Tan, Jianfeng <jianfeng.tan@intel.com>; Thotton, Shijith
> > <Shijith.Thotton@cavium.com>; Hu, Xuekun <xuekun.hu@intel.com>; Yigit,
> > Ferruh <ferruh.yigit@intel.com>; Patil, Harish <Harish.Patil@cavium.com>;
> > Gregory Etelson <gregory@weka.io>; stable@dpdk.org
> > Subject: Re: [dpdk-dev] vf init issue with patch igb_uio: issue FLR during open
> > and release of device file
> > 
> > Hi
> > 
> > 15/09/2017 10:04, Yang, Qiming:
> > > Hi, Thomas
> > > We meet a urgent issue because of Shijith's patch.
> > > When use DPDK version 17.08, DPDK VF driver and kernel PF driver, i40e, ixgbe
> > and qede(Harish said) VF all meet initial failure.
> > 
> > This patch has been integrated in DPDK 17.08-rc2 (2 months ago).
> > Why a so obvious issue is discovered only now?
> > 
> > > This issue only occurred when use 17.08 igb_uio, vfio-pci and 17.05 igb_uio all
> > works well.
> > > Considering this issue will block our vf driver develop and test work, could we
> > revert this patch first and find another appropriate way?
> > 
> > We need to be sure the revert is the right solution because it will be backported
> > to the stable 17.08 release.
> > 
> > Shijith suggests to "try removing pci_reset_function from igbuio_pci_open."
> > Can we try this first?
> > Are we sure the bug is not in new firmwares?
> 
> I have tried to remove pci_reset_function, it works in my environment, but it doesn't work in someone's enviroment.
> And removing this function won't it changes the original purpose of his patch?  So I'm not sure it is a good solution.
> This bug is not a firmware version related issue, we can reproduce stably with older firmware.
> 

There is a reset during release which was suggested as a fix by Gregory for an
issue he faced with i40e VF[1]. Removing it can cause problems for him.

1. http://dpdk.org/ml/archives/dev/2017-May/066317.html

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

[Shijith Thotton]

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

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

[Ferruh Yigit]

On 9/19/2017 11:24 AM, Shijith Thotton wrote:
> 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>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>


I can't re-produce the issue described by Qiming with this patch.

Hi Qiming,

Can you also please confirm the patch?

Thanks,
ferruh

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

[Luca Boccassi]

On Tue, 2017-09-19 at 15:54 +0530, Shijith Thotton wrote:
> 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;

Tested-by: Luca Boccassi <luca.boccassi@att.com>

Hi, we had the same issue, Chas applied and tested this patch on top of
17.08 and our QA department confirms that it fixes the problem.

-- 
Kind regards,
Luca Boccassi

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

[Yang, Qiming]

> -----Original Message-----
> From: Shijith Thotton [mailto:shijith.thotton@caviumnetworks.com]
> 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: [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

Tested-by: Qiming Yang <qiming.yang@intel.com>

Hi, applied and tested this patch with 17.08 DPDK VF and
2.1.26 kernel PF and it fixes the problem on 10G/25G/40G.

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

[Wu, Jingjing]

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?

Thanks
Jingjing

> -----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

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

[Shijith Thotton]

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
> 

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

[Gregory Etelson]

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

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

[Shijith Thotton]

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