DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jianfeng Tan <jianfeng.tan@intel.com>
To: dev@dpdk.org
Cc: huawei.xie@intel.com, yuanhan.liu@intel.com,
	david.marchand@6wind.com, Jianfeng Tan <jianfeng.tan@intel.com>
Subject: [dpdk-dev] [PATCH] virtio: fix modify drv_flags for specific device
Date: Tue, 26 Apr 2016 02:24:34 +0000	[thread overview]
Message-ID: <1461637474-110602-1-git-send-email-jianfeng.tan@intel.com> (raw)

Issue: virtio's drv_flags are decided by devices types (modern vs legacy),
and which kernel driver is used, and the negotiated features (especially
VIRTIO_NET_STATUS) with backend, which makes it possible to multiple
virtio devices have different versions of drv_flags, but this variable
is currently shared by each virtio device.

How to fix: dev_flags is a device-specific variable to store this info.

Fixes: da978dfdc43 ("virtio: use port IO to get PCI resource")

Reported-by: David Marchand <david.marchand@6wind.com>
Suggested-by: David Marchand <david.marchand@6wind.com>
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c | 27 ++++++++++++++++-----------
 drivers/net/virtio/virtio_pci.c    | 13 +++++++------
 drivers/net/virtio/virtio_pci.h    |  3 ++-
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 63a368a..b144a58 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -59,6 +59,7 @@
 #include "virtqueue.h"
 #include "virtio_rxtx.h"
 
+#define VIRTIO_DRV_FLAGS	RTE_PCI_DRV_DETACHABLE
 
 static int eth_virtio_dev_init(struct rte_eth_dev *eth_dev);
 static int eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev);
@@ -491,7 +492,6 @@ static void
 virtio_dev_close(struct rte_eth_dev *dev)
 {
 	struct virtio_hw *hw = dev->data->dev_private;
-	struct rte_pci_device *pci_dev = dev->pci_dev;
 
 	PMD_INIT_LOG(DEBUG, "virtio_dev_close");
 
@@ -499,7 +499,7 @@ virtio_dev_close(struct rte_eth_dev *dev)
 		virtio_dev_stop(dev);
 
 	/* reset the NIC */
-	if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
+	if (dev->data->dev_flags & RTE_PCI_DRV_INTR_LSC)
 		vtpci_irq_config(hw, VIRTIO_MSI_NO_VECTOR);
 	vtpci_reset(hw);
 	virtio_dev_free_mbufs(dev);
@@ -1034,6 +1034,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 	struct virtio_net_config *config;
 	struct virtio_net_config local_config;
 	struct rte_pci_device *pci_dev;
+	uint32_t dev_flags = VIRTIO_DRV_FLAGS;
 	int ret;
 
 	RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr));
@@ -1057,7 +1058,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 
 	pci_dev = eth_dev->pci_dev;
 
-	ret = vtpci_init(pci_dev, hw);
+	ret = vtpci_init(pci_dev, hw, &dev_flags);
 	if (ret)
 		return ret;
 
@@ -1074,9 +1075,15 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 
 	/* If host does not support status then disable LSC */
 	if (!vtpci_with_feature(hw, VIRTIO_NET_F_STATUS))
-		pci_dev->driver->drv_flags &= ~RTE_PCI_DRV_INTR_LSC;
+		dev_flags &= ~RTE_PCI_DRV_INTR_LSC;
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
+	/* For virtio devices, dev_flags are decided according to feature
+	 * negotiation, aka if VIRTIO_NET_F_STATUS is set, and which kernel
+	 * driver is used, dynamically. And we should keep drv_flags shared
+	 * and unvaried.
+	 */
+	eth_dev->data->dev_flags = dev_flags;
 
 	rx_func_get(eth_dev);
 
@@ -1155,7 +1162,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 			pci_dev->id.device_id);
 
 	/* Setup interrupt callback  */
-	if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
+	if (eth_dev->data->dev_flags & RTE_PCI_DRV_INTR_LSC)
 		rte_intr_callback_register(&pci_dev->intr_handle,
 				   virtio_interrupt_handler, eth_dev);
 
@@ -1190,7 +1197,7 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
 	eth_dev->data->mac_addrs = NULL;
 
 	/* reset interrupt callback  */
-	if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
+	if (eth_dev->data->dev_flags & RTE_PCI_DRV_INTR_LSC)
 		rte_intr_callback_unregister(&pci_dev->intr_handle,
 						virtio_interrupt_handler,
 						eth_dev);
@@ -1205,7 +1212,7 @@ static struct eth_driver rte_virtio_pmd = {
 	.pci_drv = {
 		.name = "rte_virtio_pmd",
 		.id_table = pci_id_virtio_map,
-		.drv_flags = RTE_PCI_DRV_DETACHABLE,
+		.drv_flags = VIRTIO_DRV_FLAGS,
 	},
 	.eth_dev_init = eth_virtio_dev_init,
 	.eth_dev_uninit = eth_virtio_dev_uninit,
@@ -1240,7 +1247,6 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 {
 	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 	struct virtio_hw *hw = dev->data->dev_private;
-	struct rte_pci_device *pci_dev = dev->pci_dev;
 
 	PMD_INIT_LOG(DEBUG, "configure");
 
@@ -1258,7 +1264,7 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 		return -ENOTSUP;
 	}
 
-	if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
+	if (dev->data->dev_flags & RTE_PCI_DRV_INTR_LSC)
 		if (vtpci_irq_config(hw, 0) == VIRTIO_MSI_NO_VECTOR) {
 			PMD_DRV_LOG(ERR, "failed to set config vector");
 			return -EBUSY;
@@ -1273,11 +1279,10 @@ virtio_dev_start(struct rte_eth_dev *dev)
 {
 	uint16_t nb_queues, i;
 	struct virtio_hw *hw = dev->data->dev_private;
-	struct rte_pci_device *pci_dev = dev->pci_dev;
 
 	/* check if lsc interrupt feature is enabled */
 	if (dev->data->dev_conf.intr_conf.lsc) {
-		if (!(pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)) {
+		if (!(dev->data->dev_flags & RTE_PCI_DRV_INTR_LSC)) {
 			PMD_DRV_LOG(ERR, "link status not supported by host");
 			return -ENOTSUP;
 		}
diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index c007959..78afe94 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -199,15 +199,15 @@ legacy_virtio_has_msix(const struct rte_pci_addr *loc __rte_unused)
 
 static int
 legacy_virtio_resource_init(struct rte_pci_device *pci_dev,
-			    struct virtio_hw *hw)
+			    struct virtio_hw *hw, uint32_t *dev_flags)
 {
 	if (rte_eal_pci_ioport_map(pci_dev, 0, &hw->io) < 0)
 		return -1;
 
 	if (pci_dev->intr_handle.type != RTE_INTR_HANDLE_UNKNOWN)
-		pci_dev->driver->drv_flags |= RTE_PCI_DRV_INTR_LSC;
+		*dev_flags |= RTE_PCI_DRV_INTR_LSC;
 	else
-		pci_dev->driver->drv_flags &= ~RTE_PCI_DRV_INTR_LSC;
+		*dev_flags &= ~RTE_PCI_DRV_INTR_LSC;
 
 	return 0;
 }
@@ -630,7 +630,8 @@ next:
  * Return 0 on success.
  */
 int
-vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw)
+vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw,
+	   uint32_t *dev_flags)
 {
 	hw->dev = dev;
 
@@ -643,12 +644,12 @@ vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw)
 		PMD_INIT_LOG(INFO, "modern virtio pci detected.");
 		hw->vtpci_ops = &modern_ops;
 		hw->modern    = 1;
-		dev->driver->drv_flags |= RTE_PCI_DRV_INTR_LSC;
+		*dev_flags |= RTE_PCI_DRV_INTR_LSC;
 		return 0;
 	}
 
 	PMD_INIT_LOG(INFO, "trying with legacy virtio pci.");
-	if (legacy_virtio_resource_init(dev, hw) < 0) {
+	if (legacy_virtio_resource_init(dev, hw, dev_flags) < 0) {
 		if (dev->kdrv == RTE_KDRV_UNKNOWN &&
 		    dev->devargs->type != RTE_DEVTYPE_WHITELISTED_PCI) {
 			PMD_INIT_LOG(INFO,
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index b69785e..554efea 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -293,7 +293,8 @@ 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 *);
+int vtpci_init(struct rte_pci_device *, struct virtio_hw *,
+	       uint32_t *dev_flags);
 void vtpci_reset(struct virtio_hw *);
 
 void vtpci_reinit_complete(struct virtio_hw *);
-- 
2.1.4

             reply	other threads:[~2016-04-26  2:24 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-26  2:24 Jianfeng Tan [this message]
2016-04-26 11:53 ` David Marchand
2016-04-27  5:10   ` Tan, Jianfeng
2016-04-27  5:20     ` David Marchand
2016-04-28 18:08 ` [dpdk-dev] [PATCH v2] " Jianfeng Tan
2016-05-02 17:55   ` Yuanhan Liu
2016-05-03  8:05   ` David Marchand
2016-05-04 23:18     ` Yuanhan Liu
2016-05-09  9:14       ` Tan, Jianfeng
2016-05-09 18:01         ` Yuanhan Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1461637474-110602-1-git-send-email-jianfeng.tan@intel.com \
    --to=jianfeng.tan@intel.com \
    --cc=david.marchand@6wind.com \
    --cc=dev@dpdk.org \
    --cc=huawei.xie@intel.com \
    --cc=yuanhan.liu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).