From 62a785a68ebf710eba22b70b46a970a4e4e1cec7 Mon Sep 17 00:00:00 2001 From: Jianfeng Tan Date: Mon, 9 May 2016 09:35:57 -0700 Subject: [PATCH] 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 Suggested-by: David Marchand Signed-off-by: Jianfeng Tan Acked-by: David Marchand Acked-by: Yuanhan Liu --- drivers/net/virtio/virtio_ethdev.c | 20 +++++++++----------- drivers/net/virtio/virtio_pci.c | 13 +++++++------ drivers/net/virtio/virtio_pci.h | 3 ++- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index 1115d45410..995618a53c 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_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 c007959f20..9cdca068de 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 b69785ea1b..554efea71f 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.20.1