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 <zhiyong.yang@intel.com>
Acked-by: Jianfeng Tan <jianfeng.tan@intel.com>
Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d2576d5..87ac2be 100644 (file)
--- 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 55b717c..9574498 100644 (file)
--- 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 36d452c..3c5ce66 100644 (file)
--- 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;