From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id D67C12BC6 for ; Tue, 26 Apr 2016 04:24:35 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga101.fm.intel.com with ESMTP; 25 Apr 2016 19:24:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,535,1455004800"; d="scan'208";a="91865700" Received: from dpdk06.sh.intel.com ([10.239.128.225]) by fmsmga004.fm.intel.com with ESMTP; 25 Apr 2016 19:24:33 -0700 From: Jianfeng Tan To: dev@dpdk.org Cc: huawei.xie@intel.com, yuanhan.liu@intel.com, david.marchand@6wind.com, Jianfeng Tan Date: Tue, 26 Apr 2016 02:24:34 +0000 Message-Id: <1461637474-110602-1-git-send-email-jianfeng.tan@intel.com> X-Mailer: git-send-email 2.1.4 Subject: [dpdk-dev] [PATCH] virtio: fix modify drv_flags for specific device X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Apr 2016 02:24:36 -0000 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 Suggested-by: David Marchand Signed-off-by: Jianfeng Tan --- 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