From fe19d49cb5259aad57c94391b2592367ba882ad6 Mon Sep 17 00:00:00 2001 From: Zhiyong Yang Date: Thu, 9 Nov 2017 17:21:24 +0800 Subject: [PATCH] net/virtio: fix Rx interrupt with VFIO 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) Fixes: cb482cb3a305 ("net/virtio: fix MAC address read") Cc: stable@dpdk.org Signed-off-by: Zhiyong Yang Acked-by: Jianfeng Tan Acked-by: Maxime Coquelin --- 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 d2576d5e03..87ac2bee6d 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; @@ -1159,6 +1162,34 @@ virtio_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) return virtio_send_command(hw->cvq, &ctrl, &len, 1); } +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) { @@ -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 55b717c03e..9574498fb8 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 36d452c067..3c5ce66cec 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.20.1