virtio: fix overwritten driver flags

The "drv_flags" is set with device as the input, which means different
device (say, modern vs legacy) could end up with a different value. And
the fact that "drv_flags" is shared by all devices means that every time
we add a new device, it simply overwrites the value configured from the
last device.

Therefore, when two virtio devices have different flags, it may lead to
wrong result, such as virtio would set irq config when it's not supported.

Making the flag per device (using "dev->data->dev_flags") could let us
have different value for each device, which would avoid the above issue.

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>
Acked-by: David Marchand <david.marchand@6wind.com>
Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 1115d454104e39e09c14b2556fa8773e905342ec..995618a53c1d81aaf3cb168e8392304941c91ca4 100644 (file)
--- 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_mrg_rxbuf));
@@ -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,10 @@ 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);
+       eth_dev->data->dev_flags = dev_flags;
 
        rx_func_get(eth_dev);
 
@@ -1155,7 +1155,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 +1190,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 +1240,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 +1257,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 +1272,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 c007959f20f5e5cc94fa650ee25eabe11a3ddf95..9cdca068de90f5b6a05fa90c894194c5ea2de855 100644 (file)
--- 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 b69785ea1b0140c5d113713eca0d56a86d9be3cc..554efea71fc663c84716d4b251d1007ca3f8451c 100644 (file)
--- 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 *);