From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id CA1D01B61F; Thu, 9 Nov 2017 10:32:42 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Nov 2017 01:32:41 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,369,1505804400"; d="scan'208";a="5625040" Received: from unknown (HELO dpdk5.bj.intel.com) ([172.16.182.182]) by orsmga002.jf.intel.com with ESMTP; 09 Nov 2017 01:32:39 -0800 From: Zhiyong Yang To: dev@dpdk.org Cc: stable@dpdk.org, jianfeng.tan@intel.com, yliu@fridaylinux.org, maxime.coquelin@redhat.com, Zhiyong Yang Date: Thu, 9 Nov 2017 17:21:24 +0800 Message-Id: <20171109092124.54380-1-zhiyong.yang@intel.com> X-Mailer: git-send-email 2.13.3 In-Reply-To: <20171109085519.62567-1-zhiyong.yang@intel.com> References: <20171109085519.62567-1-zhiyong.yang@intel.com> Subject: [dpdk-stable] [PATCH v6] net/virtio: fix rxq intr config fails using vfio-pci X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Nov 2017 09:32:44 -0000 When running l3fwd-power to test virtio rxq interrupt using vfio pci noiommu mode, startup fails. In the function virtio_read_caps, the code if (flags & PCI_MSIX_ENABLE) intends to double check if vfio msix is enabled or not. However, it is not enable at that time. So use_msix is assigned to "0", not "1", which causes the failure of configuring rxq intr in l3fwd-power. This patch adds the function "vtpci_msix_detect" to detect the status of msix when interrupt changes happen. In the meanwhile, virtio_intr_enable/disable are introduced to wrap rte_intr_enable/disable to enhance the ability to detect msix. use_msix can indicate three different msix status by: VIRTIO_MSIX_NONE (0) VIRTIO_MSIX_DISABLED (1) VIRTIO_MSIX_ENABLED (2) CC: stable@dpdk.org CC: jianfeng.tan@intel.com CC: yliu@fridaylinux.org CC: maxime.coquelin@redhat.com Fixes: cb482cb3a305 ("net/virtio: fix MAC address read") Signed-off-by: Zhiyong Yang Acked-by: Jianfeng Tan Acked-by: Maxime Coquelin --- Changes in v6: Change enum type name "VIRTIO_MSIX_STATUS" to "virtio_msix_status". Changes in v5: 1. Introduce new enum data type "VIRTIO_MSIX_STATUS" instead of macros. 2. Remove the unnecessary comments. Changes in v4: Enhance the commit log description. Changes in v3: Simply the code according to jianfeng's comments. Changes in v2: Add the capability to detect msix if virtio intr changes. drivers/net/virtio/virtio_ethdev.c | 46 ++++++++++++++++++++++++++++++++------ drivers/net/virtio/virtio_pci.c | 43 +++++++++++++++++++++++++++++++++-- drivers/net/virtio/virtio_pci.h | 8 +++++++ 3 files changed, 88 insertions(+), 9 deletions(-) diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index d2576d5e0..87ac2bee6 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -97,6 +97,9 @@ static void virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index); static void virtio_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr); +static int virtio_intr_enable(struct rte_eth_dev *dev); +static int virtio_intr_disable(struct rte_eth_dev *dev); + static int virtio_dev_queue_stats_mapping_set( struct rte_eth_dev *eth_dev, uint16_t queue_id, @@ -618,7 +621,7 @@ virtio_dev_close(struct rte_eth_dev *dev) virtio_queues_unbind_intr(dev); if (intr_conf->lsc || intr_conf->rxq) { - rte_intr_disable(dev->intr_handle); + virtio_intr_disable(dev); rte_intr_efd_disable(dev->intr_handle); rte_free(dev->intr_handle->intr_vec); dev->intr_handle->intr_vec = NULL; @@ -1160,6 +1163,34 @@ virtio_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) } static int +virtio_intr_enable(struct rte_eth_dev *dev) +{ + struct virtio_hw *hw = dev->data->dev_private; + + if (rte_intr_enable(dev->intr_handle) < 0) + return -1; + + if (!hw->virtio_user_dev) + hw->use_msix = vtpci_msix_detect(RTE_ETH_DEV_TO_PCI(dev)); + + return 0; +} + +static int +virtio_intr_disable(struct rte_eth_dev *dev) +{ + struct virtio_hw *hw = dev->data->dev_private; + + if (rte_intr_disable(dev->intr_handle) < 0) + return -1; + + if (!hw->virtio_user_dev) + hw->use_msix = vtpci_msix_detect(RTE_ETH_DEV_TO_PCI(dev)); + + return 0; +} + +static int virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features) { uint64_t host_features; @@ -1228,7 +1259,7 @@ virtio_interrupt_handler(void *param) isr = vtpci_isr(hw); PMD_DRV_LOG(INFO, "interrupt status = %#x", isr); - if (rte_intr_enable(dev->intr_handle) < 0) + if (virtio_intr_enable(dev) < 0) PMD_DRV_LOG(ERR, "interrupt enable failed"); if (isr & VIRTIO_PCI_ISR_CONFIG) { @@ -1348,7 +1379,7 @@ virtio_configure_intr(struct rte_eth_dev *dev) * to change the config size from 20 to 24, or VIRTIO_MSI_QUEUE_VECTOR * (22) will be ignored. */ - if (rte_intr_enable(dev->intr_handle) < 0) { + if (virtio_intr_enable(dev) < 0) { PMD_DRV_LOG(ERR, "interrupt enable failed"); return -1; } @@ -1388,7 +1419,8 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features) } /* If host does not support both status and MSI-X then disable LSC */ - if (vtpci_with_feature(hw, VIRTIO_NET_F_STATUS) && hw->use_msix) + if (vtpci_with_feature(hw, VIRTIO_NET_F_STATUS) && + hw->use_msix != VIRTIO_MSIX_NONE) eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC; else eth_dev->data->dev_flags &= ~RTE_ETH_DEV_INTR_LSC; @@ -1801,9 +1833,9 @@ virtio_dev_start(struct rte_eth_dev *dev) */ if (dev->data->dev_conf.intr_conf.lsc || dev->data->dev_conf.intr_conf.rxq) { - rte_intr_disable(dev->intr_handle); + virtio_intr_disable(dev); - if (rte_intr_enable(dev->intr_handle) < 0) { + if (virtio_intr_enable(dev) < 0) { PMD_DRV_LOG(ERR, "interrupt enable failed"); return -EIO; } @@ -1912,7 +1944,7 @@ virtio_dev_stop(struct rte_eth_dev *dev) PMD_INIT_LOG(DEBUG, "stop"); if (intr_conf->lsc || intr_conf->rxq) - rte_intr_disable(dev->intr_handle); + virtio_intr_disable(dev); hw->started = 0; memset(&link, 0, sizeof(link)); diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c index 55b717c03..9574498fb 100644 --- a/drivers/net/virtio/virtio_pci.c +++ b/drivers/net/virtio/virtio_pci.c @@ -57,7 +57,8 @@ * The remaining space is defined by each driver as the per-driver * configuration space. */ -#define VIRTIO_PCI_CONFIG(hw) (((hw)->use_msix) ? 24 : 20) +#define VIRTIO_PCI_CONFIG(hw) \ + (((hw)->use_msix == VIRTIO_MSIX_ENABLED) ? 24 : 20) static inline int check_vq_phys_addr_ok(struct virtqueue *vq) @@ -617,7 +618,9 @@ virtio_read_caps(struct rte_pci_device *dev, struct virtio_hw *hw) uint16_t flags = ((uint16_t *)&cap)[1]; if (flags & PCI_MSIX_ENABLE) - hw->use_msix = 1; + hw->use_msix = VIRTIO_MSIX_ENABLED; + else + hw->use_msix = VIRTIO_MSIX_DISABLED; } if (cap.cap_vndr != PCI_CAP_ID_VNDR) { @@ -710,3 +713,39 @@ vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw) return 0; } + +enum virtio_msix_status +vtpci_msix_detect(struct rte_pci_device *dev) +{ + uint8_t pos; + struct virtio_pci_cap cap; + int ret; + + ret = rte_pci_read_config(dev, &pos, 1, PCI_CAPABILITY_LIST); + if (ret < 0) { + PMD_INIT_LOG(DEBUG, "failed to read pci capability list"); + return VIRTIO_MSIX_NONE; + } + + while (pos) { + ret = rte_pci_read_config(dev, &cap, sizeof(cap), pos); + if (ret < 0) { + PMD_INIT_LOG(ERR, + "failed to read pci cap at pos: %x", pos); + break; + } + + if (cap.cap_vndr == PCI_CAP_ID_MSIX) { + uint16_t flags = ((uint16_t *)&cap)[1]; + + if (flags & PCI_MSIX_ENABLE) + return VIRTIO_MSIX_ENABLED; + else + return VIRTIO_MSIX_DISABLED; + } + + pos = cap.cap_next; + } + + return VIRTIO_MSIX_NONE; +} diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h index 36d452c06..3c5ce66ce 100644 --- a/drivers/net/virtio/virtio_pci.h +++ b/drivers/net/virtio/virtio_pci.h @@ -314,6 +314,12 @@ struct virtio_net_config { /* The alignment to use between consumer and producer parts of vring. */ #define VIRTIO_PCI_VRING_ALIGN 4096 +enum virtio_msix_status { + VIRTIO_MSIX_NONE = 0, + VIRTIO_MSIX_DISABLED = 1, + VIRTIO_MSIX_ENABLED = 2 +}; + static inline int vtpci_with_feature(struct virtio_hw *hw, uint64_t bit) { @@ -339,6 +345,8 @@ void vtpci_read_dev_config(struct virtio_hw *, size_t, void *, int); uint8_t vtpci_isr(struct virtio_hw *); +enum virtio_msix_status vtpci_msix_detect(struct rte_pci_device *dev); + extern const struct virtio_pci_ops legacy_ops; extern const struct virtio_pci_ops modern_ops; extern const struct virtio_pci_ops virtio_user_ops; -- 2.13.3