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 at 6wind.com>
Suggested-by: David Marchand <david.marchand at 6wind.com>
Signed-off-by: Jianfeng Tan <jianfeng.tan at intel.com>
---
 v2: RTE_PCI_DRV_INTR_LSC -> RTE_ETH_DEV_INTR_LSC.

 drivers/net/virtio/virtio_ethdev.c | 25 ++++++++++++++-----------
 drivers/net/virtio/virtio_pci.c    | 13 +++++++------
 drivers/net/virtio/virtio_pci.h    |  3 ++-
 3 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c 
b/drivers/net/virtio/virtio_ethdev.c
index 63a368a..1fe90ae 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -59,7 +59,6 @@
 #include "virtqueue.h"
 #include "virtio_rxtx.h"

-
 static int eth_virtio_dev_init(struct rte_eth_dev *eth_dev);
 static int eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev);
 static int  virtio_dev_configure(struct rte_eth_dev *dev);
@@ -491,7 +490,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 +497,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_ETH_DEV_INTR_LSC)
                vtpci_irq_config(hw, VIRTIO_MSI_NO_VECTOR);
        vtpci_reset(hw);
        virtio_dev_free_mbufs(dev);
@@ -1034,6 +1032,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 = RTE_ETH_DEV_DETACHABLE;
        int ret;

        RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr));
@@ -1057,7 +1056,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 +1073,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_ETH_DEV_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 +1160,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_ETH_DEV_INTR_LSC)
                rte_intr_callback_register(&pci_dev->intr_handle,
                                   virtio_interrupt_handler, eth_dev);

@@ -1190,7 +1195,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_ETH_DEV_INTR_LSC)
                rte_intr_callback_unregister(&pci_dev->intr_handle,
                                                virtio_interrupt_handler,
                                                eth_dev);
@@ -1240,7 +1245,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 +1262,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_ETH_DEV_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 +1277,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_ETH_DEV_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..9cdca06 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_ETH_DEV_INTR_LSC;
        else
-               pci_dev->driver->drv_flags &= ~RTE_PCI_DRV_INTR_LSC;
+               *dev_flags &= ~RTE_ETH_DEV_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_ETH_DEV_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 via email to