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