[PATCH 0/3] Some fixes on virtio LSC

[PATCH 0/3] Some fixes on virtio LSC

[Jianfeng Tan]

Patch 1: fix wrong MSI-X for modern devices so that LSC is always not
         available.
Patch 2: clean up LSC setting.
Patch 3: fix link status always being down.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>

Jianfeng Tan (3):
  net/virtio: fix wrong MSI-X for modern devices
  net/virtio: clean up LSC setting
  net/virtio: fix link status always being down

 drivers/net/virtio/virtio_ethdev.c | 14 +++++-----
 drivers/net/virtio/virtio_pci.c    | 52 +++++---------------------------------
 drivers/net/virtio/virtio_pci.h    |  3 +--
 3 files changed, 13 insertions(+), 56 deletions(-)

-- 
2.7.4

[PATCH 1/3] net/virtio: fix wrong MSI-X for modern devices

[Jianfeng Tan]

The field, use_msix, in struct virtio_hw is not updated for modern
device, and is always zero. And now we depend on the status feature
and MSI-X to report LSC support (which is also not a correct
behavior). As a result, LSC is always disabled for modern devices.

Te fix this, we just recognize MSI-X capability when going through
capability list, and update the info in virtio.

Fixes: 6ba1f63b5ab0 ("virtio: support specification 1.0")
Cc: stable@dpdk.org

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_pci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index b767c03..ecad46e 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -50,6 +50,7 @@
  */
 #define PCI_CAPABILITY_LIST	0x34
 #define PCI_CAP_ID_VNDR		0x09
+#define PCI_CAP_ID_MSIX		0x11
 
 /*
  * The remaining space is defined by each driver as the per-driver
@@ -650,6 +651,9 @@ virtio_read_caps(struct rte_pci_device *dev, struct virtio_hw *hw)
 			break;
 		}
 
+		if (cap.cap_vndr == PCI_CAP_ID_MSIX)
+			hw->use_msix = 1;
+
 		if (cap.cap_vndr != PCI_CAP_ID_VNDR) {
 			PMD_INIT_LOG(DEBUG,
 				"[%2x] skipping non VNDR cap id: %02x",
-- 
2.7.4

[PATCH 2/3] net/virtio: clean up LSC setting

[Jianfeng Tan]

Clean up LSC setting:
  - LSC flag is set in several places, but only the last one takes
    effect; so we just keep the last one.
  - As we already change to use capability list to detect MSI-X, remove
    the redundant MSI-X detection in legacy devices.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c |  7 ++----
 drivers/net/virtio/virtio_pci.c    | 48 ++------------------------------------
 drivers/net/virtio/virtio_pci.h    |  3 +--
 3 files changed, 5 insertions(+), 53 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index e6c57b3..e79748e 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1353,6 +1353,7 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 		rte_eth_copy_pci_info(eth_dev, pci_dev);
 	}
 
+	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 	/* If host does not support both status and MSI-X then disable LSC */
 	if (vtpci_with_feature(hw, VIRTIO_NET_F_STATUS) && hw->use_msix)
 		eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
@@ -1521,7 +1522,6 @@ int
 eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 {
 	struct virtio_hw *hw = eth_dev->data->dev_private;
-	uint32_t dev_flags = RTE_ETH_DEV_DETACHABLE;
 	int ret;
 
 	RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr_mrg_rxbuf));
@@ -1561,14 +1561,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 	 * virtio_user_eth_dev_alloc() before eth_virtio_dev_init() is called.
 	 */
 	if (!hw->virtio_user_dev) {
-		ret = vtpci_init(RTE_DEV_TO_PCI(eth_dev->device), hw,
-				 &dev_flags);
+		ret = vtpci_init(RTE_DEV_TO_PCI(eth_dev->device), hw);
 		if (ret)
 			return ret;
 	}
 
-	eth_dev->data->dev_flags = dev_flags;
-
 	/* reset device and negotiate default features */
 	ret = virtio_init_device(eth_dev, VIRTIO_PMD_DEFAULT_GUEST_FEATURES);
 	if (ret < 0)
diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index ecad46e..1df26a6 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -279,47 +279,6 @@ legacy_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
 			 VIRTIO_PCI_QUEUE_NOTIFY);
 }
 
-#ifdef RTE_EXEC_ENV_LINUXAPP
-static int
-legacy_virtio_has_msix(const struct rte_pci_addr *loc)
-{
-	DIR *d;
-	char dirname[PATH_MAX];
-
-	snprintf(dirname, sizeof(dirname),
-		     "%s/" PCI_PRI_FMT "/msi_irqs", pci_get_sysfs_path(),
-		     loc->domain, loc->bus, loc->devid, loc->function);
-
-	d = opendir(dirname);
-	if (d)
-		closedir(d);
-
-	return d != NULL;
-}
-#else
-static int
-legacy_virtio_has_msix(const struct rte_pci_addr *loc __rte_unused)
-{
-	/* nic_uio does not enable interrupts, return 0 (false). */
-	return 0;
-}
-#endif
-
-static int
-legacy_virtio_resource_init(struct rte_pci_device *pci_dev,
-			    struct virtio_hw *hw, uint32_t *dev_flags)
-{
-	if (rte_eal_pci_ioport_map(pci_dev, 0, VTPCI_IO(hw)) < 0)
-		return -1;
-
-	if (pci_dev->intr_handle.type != RTE_INTR_HANDLE_UNKNOWN)
-		*dev_flags |= RTE_ETH_DEV_INTR_LSC;
-	else
-		*dev_flags &= ~RTE_ETH_DEV_INTR_LSC;
-
-	return 0;
-}
-
 const struct virtio_pci_ops legacy_ops = {
 	.read_dev_cfg	= legacy_read_dev_config,
 	.write_dev_cfg	= legacy_write_dev_config,
@@ -712,8 +671,7 @@ virtio_read_caps(struct rte_pci_device *dev, struct virtio_hw *hw)
  * Return 0 on success.
  */
 int
-vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw,
-	   uint32_t *dev_flags)
+vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw)
 {
 	/*
 	 * Try if we can succeed reading virtio pci caps, which exists
@@ -724,12 +682,11 @@ vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw,
 		PMD_INIT_LOG(INFO, "modern virtio pci detected.");
 		virtio_hw_internal[hw->port_id].vtpci_ops = &modern_ops;
 		hw->modern = 1;
-		*dev_flags |= RTE_ETH_DEV_INTR_LSC;
 		return 0;
 	}
 
 	PMD_INIT_LOG(INFO, "trying with legacy virtio pci.");
-	if (legacy_virtio_resource_init(dev, hw, dev_flags) < 0) {
+	if (rte_eal_pci_ioport_map(dev, 0, VTPCI_IO(hw)) < 0) {
 		if (dev->kdrv == RTE_KDRV_UNKNOWN &&
 		    (!dev->device.devargs ||
 		     dev->device.devargs->type !=
@@ -742,7 +699,6 @@ vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw,
 	}
 
 	virtio_hw_internal[hw->port_id].vtpci_ops = &legacy_ops;
-	hw->use_msix = legacy_virtio_has_msix(&dev->addr);
 	hw->modern   = 0;
 
 	return 0;
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index e7290d7..5651eb5 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -321,8 +321,7 @@ vtpci_with_feature(struct virtio_hw *hw, uint64_t bit)
 /*
  * Function declaration from virtio_pci.c
  */
-int vtpci_init(struct rte_pci_device *, struct virtio_hw *,
-	       uint32_t *dev_flags);
+int vtpci_init(struct rte_pci_device *, struct virtio_hw *);
 void vtpci_reset(struct virtio_hw *);
 
 void vtpci_reinit_complete(struct virtio_hw *);
-- 
2.7.4

Re: [PATCH 2/3] net/virtio: clean up LSC setting

[Yuanhan Liu]

On Wed, Apr 26, 2017 at 04:52:50AM +0000, Jianfeng Tan wrote:
> Clean up LSC setting:

Firstly, this patch does two things. There should be two patches.

>   - LSC flag is set in several places, but only the last one takes
>     effect; so we just keep the last one.
>   - As we already change to use capability list to detect MSI-X, remove
>     the redundant MSI-X detection in legacy devices.

However, there is no capability list for legacy device.

	--yliu

Re: [PATCH 2/3] net/virtio: clean up LSC setting

[Tan, Jianfeng]

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, April 26, 2017 1:33 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; maxime.coquelin@redhat.com; thomas@monjalon.net
> Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
> 
> On Wed, Apr 26, 2017 at 04:52:50AM +0000, Jianfeng Tan wrote:
> > Clean up LSC setting:
> 
> Firstly, this patch does two things. There should be two patches.

I just merged them into one to adapt to the name of "clean up" :-)

> 
> >   - LSC flag is set in several places, but only the last one takes
> >     effect; so we just keep the last one.
> >   - As we already change to use capability list to detect MSI-X, remove
> >     the redundant MSI-X detection in legacy devices.
> 
> However, there is no capability list for legacy device.

When I try legacy device on QEMU (disable-modern=true), I did detect MSI-X capability. So does virtio spec mean legacy devices do not have vendor-specific capability list?

Thanks,
Jianfeng

Re: [PATCH 2/3] net/virtio: clean up LSC setting

[Yuanhan Liu]

On Wed, Apr 26, 2017 at 05:44:05AM +0000, Tan, Jianfeng wrote:
> 
> 
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > Sent: Wednesday, April 26, 2017 1:33 PM
> > To: Tan, Jianfeng
> > Cc: dev@dpdk.org; maxime.coquelin@redhat.com; thomas@monjalon.net
> > Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
> > 
> > On Wed, Apr 26, 2017 at 04:52:50AM +0000, Jianfeng Tan wrote:
> > > Clean up LSC setting:
> > 
> > Firstly, this patch does two things. There should be two patches.
> 
> I just merged them into one to adapt to the name of "clean up" :-)

I then could merge all bug fix patches into one and name it "fix buges"?
No, please don't do that. It hurts review.

> > 
> > >   - LSC flag is set in several places, but only the last one takes
> > >     effect; so we just keep the last one.
> > >   - As we already change to use capability list to detect MSI-X, remove
> > >     the redundant MSI-X detection in legacy devices.
> > 
> > However, there is no capability list for legacy device.
> 
> When I try legacy device on QEMU (disable-modern=true), I did detect MSI-X capability. So does virtio spec mean legacy devices do not have vendor-specific capability list?

Oh, right. I mixed that up.

	--yliu

Re: [PATCH 2/3] net/virtio: clean up LSC setting

[Tan, Jianfeng]

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, April 26, 2017 1:52 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; maxime.coquelin@redhat.com; thomas@monjalon.net;
> Michael S. Tsirkin
> Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
> 
> On Wed, Apr 26, 2017 at 05:44:05AM +0000, Tan, Jianfeng wrote:
> >
> >
> > > -----Original Message-----
> > > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > > Sent: Wednesday, April 26, 2017 1:33 PM
> > > To: Tan, Jianfeng
> > > Cc: dev@dpdk.org; maxime.coquelin@redhat.com;
> thomas@monjalon.net
> > > Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
> > >
> > > On Wed, Apr 26, 2017 at 04:52:50AM +0000, Jianfeng Tan wrote:
> > > > Clean up LSC setting:
> > >
> > > Firstly, this patch does two things. There should be two patches.
> >
> > I just merged them into one to adapt to the name of "clean up" :-)
> 
> I then could merge all bug fix patches into one and name it "fix buges"?
> No, please don't do that. It hurts review.

OK, will fix this in next version.

Thanks,
Jianfeng

Re: [PATCH 2/3] net/virtio: clean up LSC setting

[Michael S. Tsirkin]

On Wed, Apr 26, 2017 at 05:44:05AM +0000, Tan, Jianfeng wrote:
> 
> 
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > Sent: Wednesday, April 26, 2017 1:33 PM
> > To: Tan, Jianfeng
> > Cc: dev@dpdk.org; maxime.coquelin@redhat.com; thomas@monjalon.net
> > Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
> > 
> > On Wed, Apr 26, 2017 at 04:52:50AM +0000, Jianfeng Tan wrote:
> > > Clean up LSC setting:
> > 
> > Firstly, this patch does two things. There should be two patches.
> 
> I just merged them into one to adapt to the name of "clean up" :-)
> 
> > 
> > >   - LSC flag is set in several places, but only the last one takes
> > >     effect; so we just keep the last one.
> > >   - As we already change to use capability list to detect MSI-X, remove
> > >     the redundant MSI-X detection in legacy devices.
> > 
> > However, there is no capability list for legacy device.
> 
> When I try legacy device on QEMU (disable-modern=true), I did detect MSI-X capability. So does virtio spec mean legacy devices do not have vendor-specific capability list?
> 
> Thanks,
> Jianfeng

Yes, as far as I know legacy devices don't have vendor-specific
capability lists.

-- 
MST

Re: [PATCH 2/3] net/virtio: clean up LSC setting

[Tan, Jianfeng]

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Wednesday, April 26, 2017 11:11 PM
> To: Tan, Jianfeng
> Cc: Yuanhan Liu; dev@dpdk.org; maxime.coquelin@redhat.com;
> thomas@monjalon.net
> Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
> 
> On Wed, Apr 26, 2017 at 05:44:05AM +0000, Tan, Jianfeng wrote:
> >
> >
> > > -----Original Message-----
> > > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > > Sent: Wednesday, April 26, 2017 1:33 PM
> > > To: Tan, Jianfeng
> > > Cc: dev@dpdk.org; maxime.coquelin@redhat.com;
> thomas@monjalon.net
> > > Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
> > >
> > > On Wed, Apr 26, 2017 at 04:52:50AM +0000, Jianfeng Tan wrote:
> > > > Clean up LSC setting:
> > >
> > > Firstly, this patch does two things. There should be two patches.
> >
> > I just merged them into one to adapt to the name of "clean up" :-)
> >
> > >
> > > >   - LSC flag is set in several places, but only the last one takes
> > > >     effect; so we just keep the last one.
> > > >   - As we already change to use capability list to detect MSI-X, remove
> > > >     the redundant MSI-X detection in legacy devices.
> > >
> > > However, there is no capability list for legacy device.
> >
> > When I try legacy device on QEMU (disable-modern=true), I did detect
> MSI-X capability. So does virtio spec mean legacy devices do not have
> vendor-specific capability list?
> >
> > Thanks,
> > Jianfeng
> 
> Yes, as far as I know legacy devices don't have vendor-specific
> capability lists.
> 
> --
> MST

Thank you to confirm this, Michael.

Thanks,
Jianfeng

[PATCH 3/3] net/virtio: fix link status always being down

[Jianfeng Tan]

The virtio port link status will always be DOWN:

The commit aa9f06061765 ("net/virtio: fix link status always being up")
introduces a flag to help checking the status. If this flag is not set,
status will be always down. However, in dev start, this flag is set
after link status update, then we miss the chance to change the status
to UP in dev start.

To fix this bug, we simply move the link status update after the flag
setting so that the status can be correctly updated.

Fixes: aa9f06061765 ("net/virtio: fix link status always being up")
Cc: stable@dpdk.org

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index e79748e..cd87c0e 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1742,9 +1742,6 @@ virtio_dev_start(struct rte_eth_dev *dev)
 		}
 	}
 
-	/* Initialize Link state */
-	virtio_dev_link_update(dev, 0);
-
 	/*Notify the backend
 	 *Otherwise the tap backend might already stop its queue due to fullness.
 	 *vhost backend will have no chance to be waked up
@@ -1773,8 +1770,12 @@ virtio_dev_start(struct rte_eth_dev *dev)
 		txvq = dev->data->tx_queues[i];
 		VIRTQUEUE_DUMP(txvq->vq);
 	}
+
 	hw->started = 1;
 
+	/* Initialize Link state */
+	virtio_dev_link_update(dev, 0);
+
 	return 0;
 }
 
-- 
2.7.4

[PATCH v2 0/4] fixes and cleanup on virtio LSC

[Jianfeng Tan]

v2:
  - Split 2nd patch into two patches.

Patch 1: fix wrong MSI-X for modern devices so that LSC is always not
         available.
Patch 2: clean up LSC setting.
Patch 3: remove redundant MSI-X detection
Patch 4: fix link status always being down.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>

Jianfeng Tan (4):
  net/virtio: fix wrong MSI-X for modern devices
  net/virtio: clean up LSC setting
  net/virtio: remove redundant MSI-X detection
  net/virtio: fix link status always being down

 drivers/net/virtio/virtio_ethdev.c | 14 +++++-----
 drivers/net/virtio/virtio_pci.c    | 52 +++++---------------------------------
 drivers/net/virtio/virtio_pci.h    |  3 +--
 3 files changed, 13 insertions(+), 56 deletions(-)

-- 
2.7.4

[PATCH v2 1/4] net/virtio: fix wrong MSI-X for modern devices

[Jianfeng Tan]

The field, use_msix, in struct virtio_hw is not updated for modern
device, and is always zero. And now we depend on the status feature
and MSI-X to report LSC support (which is also not a correct
behavior). As a result, LSC is always disabled for modern devices.

Te fix this, we just recognize MSI-X capability when going through
capability list, and update the info in virtio.

Fixes: 6ba1f63b5ab0 ("virtio: support specification 1.0")
Cc: stable@dpdk.org

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_pci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index b767c03..ecad46e 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -50,6 +50,7 @@
  */
 #define PCI_CAPABILITY_LIST	0x34
 #define PCI_CAP_ID_VNDR		0x09
+#define PCI_CAP_ID_MSIX		0x11
 
 /*
  * The remaining space is defined by each driver as the per-driver
@@ -650,6 +651,9 @@ virtio_read_caps(struct rte_pci_device *dev, struct virtio_hw *hw)
 			break;
 		}
 
+		if (cap.cap_vndr == PCI_CAP_ID_MSIX)
+			hw->use_msix = 1;
+
 		if (cap.cap_vndr != PCI_CAP_ID_VNDR) {
 			PMD_INIT_LOG(DEBUG,
 				"[%2x] skipping non VNDR cap id: %02x",
-- 
2.7.4

[PATCH v2 2/4] net/virtio: clean up LSC setting

[Jianfeng Tan]

LSC flag is set in several places, but only the last one takes effect;
so we remove the redundant ones and just keep the last one.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c |  7 ++-----
 drivers/net/virtio/virtio_pci.c    | 21 ++-------------------
 drivers/net/virtio/virtio_pci.h    |  3 +--
 3 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index e6c57b3..e79748e 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1353,6 +1353,7 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 		rte_eth_copy_pci_info(eth_dev, pci_dev);
 	}
 
+	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 	/* If host does not support both status and MSI-X then disable LSC */
 	if (vtpci_with_feature(hw, VIRTIO_NET_F_STATUS) && hw->use_msix)
 		eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
@@ -1521,7 +1522,6 @@ int
 eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 {
 	struct virtio_hw *hw = eth_dev->data->dev_private;
-	uint32_t dev_flags = RTE_ETH_DEV_DETACHABLE;
 	int ret;
 
 	RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr_mrg_rxbuf));
@@ -1561,14 +1561,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 	 * virtio_user_eth_dev_alloc() before eth_virtio_dev_init() is called.
 	 */
 	if (!hw->virtio_user_dev) {
-		ret = vtpci_init(RTE_DEV_TO_PCI(eth_dev->device), hw,
-				 &dev_flags);
+		ret = vtpci_init(RTE_DEV_TO_PCI(eth_dev->device), hw);
 		if (ret)
 			return ret;
 	}
 
-	eth_dev->data->dev_flags = dev_flags;
-
 	/* reset device and negotiate default features */
 	ret = virtio_init_device(eth_dev, VIRTIO_PMD_DEFAULT_GUEST_FEATURES);
 	if (ret < 0)
diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index ecad46e..1e17757 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -305,21 +305,6 @@ legacy_virtio_has_msix(const struct rte_pci_addr *loc __rte_unused)
 }
 #endif
 
-static int
-legacy_virtio_resource_init(struct rte_pci_device *pci_dev,
-			    struct virtio_hw *hw, uint32_t *dev_flags)
-{
-	if (rte_eal_pci_ioport_map(pci_dev, 0, VTPCI_IO(hw)) < 0)
-		return -1;
-
-	if (pci_dev->intr_handle.type != RTE_INTR_HANDLE_UNKNOWN)
-		*dev_flags |= RTE_ETH_DEV_INTR_LSC;
-	else
-		*dev_flags &= ~RTE_ETH_DEV_INTR_LSC;
-
-	return 0;
-}
-
 const struct virtio_pci_ops legacy_ops = {
 	.read_dev_cfg	= legacy_read_dev_config,
 	.write_dev_cfg	= legacy_write_dev_config,
@@ -712,8 +697,7 @@ virtio_read_caps(struct rte_pci_device *dev, struct virtio_hw *hw)
  * Return 0 on success.
  */
 int
-vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw,
-	   uint32_t *dev_flags)
+vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw)
 {
 	/*
 	 * Try if we can succeed reading virtio pci caps, which exists
@@ -724,12 +708,11 @@ vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw,
 		PMD_INIT_LOG(INFO, "modern virtio pci detected.");
 		virtio_hw_internal[hw->port_id].vtpci_ops = &modern_ops;
 		hw->modern = 1;
-		*dev_flags |= RTE_ETH_DEV_INTR_LSC;
 		return 0;
 	}
 
 	PMD_INIT_LOG(INFO, "trying with legacy virtio pci.");
-	if (legacy_virtio_resource_init(dev, hw, dev_flags) < 0) {
+	if (rte_eal_pci_ioport_map(dev, 0, VTPCI_IO(hw)) < 0) {
 		if (dev->kdrv == RTE_KDRV_UNKNOWN &&
 		    (!dev->device.devargs ||
 		     dev->device.devargs->type !=
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index e7290d7..5651eb5 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -321,8 +321,7 @@ vtpci_with_feature(struct virtio_hw *hw, uint64_t bit)
 /*
  * Function declaration from virtio_pci.c
  */
-int vtpci_init(struct rte_pci_device *, struct virtio_hw *,
-	       uint32_t *dev_flags);
+int vtpci_init(struct rte_pci_device *, struct virtio_hw *);
 void vtpci_reset(struct virtio_hw *);
 
 void vtpci_reinit_complete(struct virtio_hw *);
-- 
2.7.4

Re: [PATCH v2 2/4] net/virtio: clean up LSC setting

[Yuanhan Liu]

On Thu, Apr 27, 2017 at 07:35:37AM +0000, Jianfeng Tan wrote:
> LSC flag is set in several places, but only the last one takes effect;
> so we remove the redundant ones and just keep the last one.

I think this patch would also fix the issue reported by:
    http://dpdk.org/dev/patchwork/patch/20552/

You patch is better, I will take yours.

	--yliu

Re: [PATCH v2 2/4] net/virtio: clean up LSC setting

[Yuanhan Liu]

On Thu, Apr 27, 2017 at 07:35:37AM +0000, Jianfeng Tan wrote:
> LSC flag is set in several places, but only the last one takes effect;
> so we remove the redundant ones and just keep the last one.

Note that I modified this commit log a bit. It actually fixed a bug: the
dev_flags is being overwritten by rte_eth_copy_pci_info(), which resets
it to 0 unconditionally.

Thus, "Cc: stable@dpdk.org" is also added.

Thanks.

	--yliu

[PATCH v2 3/4] net/virtio: remove redundant MSI-X detection

[Jianfeng Tan]

As we already change to use capability list to detect MSI-X, remove
the redundant MSI-X detection in legacy devices.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_pci.c | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index 1e17757..1df26a6 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -279,32 +279,6 @@ legacy_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
 			 VIRTIO_PCI_QUEUE_NOTIFY);
 }
 
-#ifdef RTE_EXEC_ENV_LINUXAPP
-static int
-legacy_virtio_has_msix(const struct rte_pci_addr *loc)
-{
-	DIR *d;
-	char dirname[PATH_MAX];
-
-	snprintf(dirname, sizeof(dirname),
-		     "%s/" PCI_PRI_FMT "/msi_irqs", pci_get_sysfs_path(),
-		     loc->domain, loc->bus, loc->devid, loc->function);
-
-	d = opendir(dirname);
-	if (d)
-		closedir(d);
-
-	return d != NULL;
-}
-#else
-static int
-legacy_virtio_has_msix(const struct rte_pci_addr *loc __rte_unused)
-{
-	/* nic_uio does not enable interrupts, return 0 (false). */
-	return 0;
-}
-#endif
-
 const struct virtio_pci_ops legacy_ops = {
 	.read_dev_cfg	= legacy_read_dev_config,
 	.write_dev_cfg	= legacy_write_dev_config,
@@ -725,7 +699,6 @@ vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw)
 	}
 
 	virtio_hw_internal[hw->port_id].vtpci_ops = &legacy_ops;
-	hw->use_msix = legacy_virtio_has_msix(&dev->addr);
 	hw->modern   = 0;
 
 	return 0;
-- 
2.7.4

[PATCH v2 4/4] net/virtio: fix link status always being down

[Jianfeng Tan]

The virtio port link status will always be DOWN:

The commit aa9f06061765 ("net/virtio: fix link status always being up")
introduces a flag to help checking the status. If this flag is not set,
status will be always down. However, in dev start, this flag is set
after link status update, then we miss the chance to change the status
to UP in dev start.

To fix this bug, we simply move the link status update after the flag
setting so that the status can be correctly updated.

Fixes: aa9f06061765 ("net/virtio: fix link status always being up")
Cc: stable@dpdk.org

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index e79748e..cd87c0e 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1742,9 +1742,6 @@ virtio_dev_start(struct rte_eth_dev *dev)
 		}
 	}
 
-	/* Initialize Link state */
-	virtio_dev_link_update(dev, 0);
-
 	/*Notify the backend
 	 *Otherwise the tap backend might already stop its queue due to fullness.
 	 *vhost backend will have no chance to be waked up
@@ -1773,8 +1770,12 @@ virtio_dev_start(struct rte_eth_dev *dev)
 		txvq = dev->data->tx_queues[i];
 		VIRTQUEUE_DUMP(txvq->vq);
 	}
+
 	hw->started = 1;
 
+	/* Initialize Link state */
+	virtio_dev_link_update(dev, 0);
+
 	return 0;
 }
 
-- 
2.7.4

Re: [PATCH v2 0/4] fixes and cleanup on virtio LSC

[Yuanhan Liu]

On Thu, Apr 27, 2017 at 07:35:35AM +0000, Jianfeng Tan wrote:
> v2:
>   - Split 2nd patch into two patches.
> 
> Patch 1: fix wrong MSI-X for modern devices so that LSC is always not
>          available.
> Patch 2: clean up LSC setting.
> Patch 3: remove redundant MSI-X detection
> Patch 4: fix link status always being down.
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>

FYI, no need to add SoB in cover letter.

Series applied to dpdk-next-virtio. Thanks.

	--yliu


> 
> Jianfeng Tan (4):
>   net/virtio: fix wrong MSI-X for modern devices
>   net/virtio: clean up LSC setting
>   net/virtio: remove redundant MSI-X detection
>   net/virtio: fix link status always being down
> 
>  drivers/net/virtio/virtio_ethdev.c | 14 +++++-----
>  drivers/net/virtio/virtio_pci.c    | 52 +++++---------------------------------
>  drivers/net/virtio/virtio_pci.h    |  3 +--
>  3 files changed, 13 insertions(+), 56 deletions(-)
> 
> -- 
> 2.7.4