[dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored

[dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored

[Markus Theil]

This patch adds MSI IRQ mode and in a way, that should
also work on older kernel versions. The base for my patch
was an attempt to do this in cf705bc36c which was later reverted in
d8ee82745a. Compilation was tested on Linux 3.2, 4.10 and 4.12.

MSI(X) setup was already using pci_alloc_irq_vectors before,
but calls to pci_free_irq_vectors were missing and added.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/compat.h  |   9 +-
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 175 ++++++++++++++++++++++--------
 2 files changed, 135 insertions(+), 49 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
index b800a53..8674088 100644
--- a/lib/librte_eal/linuxapp/igb_uio/compat.h
+++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
@@ -125,5 +125,12 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
 #endif /* < 3.3.0 */
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(4, 8, 0)
-#define HAVE_PCI_ENABLE_MSIX
+
+#define HAVE_PCI_ENABLE_MSIX 1
+#define HAVE_PCI_ENABLE_MSI 1
+
+#else
+
+#define HAVE_ALLOC_IRQ_VECTORS 1
+
 #endif
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 07a19a3..0a2f993 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -91,6 +91,7 @@ static struct attribute *dev_attrs[] = {
 static const struct attribute_group dev_attr_grp = {
 	.attrs = dev_attrs,
 };
+
 /*
  * It masks the msix on/off of generating MSI-X messages.
  */
@@ -113,6 +114,29 @@ igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state)
 	}
 }
 
+/*
+ * It masks the msi on/off of generating MSI messages.
+ */
+static void
+igbuio_msi_mask_irq(struct pci_dev *pdev, struct msi_desc *desc, int32_t state)
+{
+	u32 mask_bits = desc->masked;
+	u32 offset = desc->irq - pdev->irq;
+	u32 mask = 1 << offset;
+	u32 flag = !!state << offset;
+
+	if (!desc->msi_attrib.maskbit)
+		return;
+
+	mask_bits &= ~mask;
+	mask_bits |= flag;
+
+	if(mask_bits != desc->masked) {
+		pci_write_config_dword(pdev, desc->mask_pos, mask_bits);
+		desc->masked = mask_bits;
+	}
+}
+
 /**
  * This is the irqcontrol callback to be registered to uio_info.
  * It can be used to disable/enable interrupt from user space processes.
@@ -146,6 +170,16 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
 		list_for_each_entry(desc, &pdev->dev.msi_list, list)
 			igbuio_msix_mask_irq(desc, irq_state);
 #endif
+	} else if (udev->mode == RTE_INTR_MODE_MSI) {
+		struct msi_desc *desc;
+
+#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
+		list_for_each_entry(desc, &pdev->msi_list, list)
+			igbuio_msi_mask_irq(pdev, desc, irq_state);
+#else
+		list_for_each_entry(desc, &pdev->dev.msi_list, list)
+			igbuio_msi_mask_irq(pdev, desc, irq_state);
+#endif
 	}
 	pci_cfg_access_unlock(pdev);
 
@@ -309,6 +343,91 @@ igbuio_pci_release_iomem(struct uio_info *info)
 }
 
 static int
+igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
+{
+	int err = 0;
+#ifdef HAVE_PCI_ENABLE_MSIX
+	struct msix_entry msix_entry;
+#endif
+
+	switch (igbuio_intr_mode_preferred) {
+	case RTE_INTR_MODE_MSIX:
+		/* Only 1 msi-x vector needed */
+#ifdef HAVE_PCI_ENABLE_MSIX
+		msix_entry.entry = 0;
+		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = msix_entry.vector;
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#endif
+	case RTE_INTR_MODE_MSI:
+#ifdef HAVE_PCI_ENABLE_MSI
+		if (pci_enable_msi(udev->pdev) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSI) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#endif
+	/* fall back to INTX */
+	case RTE_INTR_MODE_LEGACY:
+		if (pci_intx_mask_supported(udev->pdev)) {
+			dev_dbg(&udev->pdev->dev, "using INTX");
+			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_LEGACY;
+			break;
+		}
+		dev_notice(&udev->pdev->dev, "PCI INTX mask not supported\n");
+		/* fall back to no IRQ */
+	case RTE_INTR_MODE_NONE:
+		udev->mode = RTE_INTR_MODE_NONE;
+		udev->info.irq = 0;
+		break;
+
+	default:
+		dev_err(&udev->pdev->dev, "invalid IRQ mode %u",
+			igbuio_intr_mode_preferred);
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static void
+igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
+{
+#ifndef HAVE_ALLOC_IRQ_VECTORS
+	if (udev->mode == RTE_INTR_MODE_MSIX)
+		pci_disable_msix(udev->pdev);
+	if (udev->mode == RTE_INTR_MODE_MSI)
+		pci_disable_msi(udev->pdev);
+#else
+	if (udev->mode == RTE_INTR_MODE_MSIX ||
+		udev->mode == RTE_INTR_MODE_MSI)
+		pci_free_irq_vectors(udev->pdev);
+#endif
+}
+
+static int
 igbuio_setup_bars(struct pci_dev *dev, struct uio_info *info)
 {
 	int i, iom, iop, ret;
@@ -356,9 +475,6 @@ static int
 igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	struct rte_uio_pci_dev *udev;
-#ifdef HAVE_PCI_ENABLE_MSIX
-	struct msix_entry msix_entry;
-#endif
 	dma_addr_t map_dma_addr;
 	void *map_addr;
 	int err;
@@ -413,48 +529,9 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	udev->info.priv = udev;
 	udev->pdev = dev;
 
-	switch (igbuio_intr_mode_preferred) {
-	case RTE_INTR_MODE_MSIX:
-		/* Only 1 msi-x vector needed */
-#ifdef HAVE_PCI_ENABLE_MSIX
-		msix_entry.entry = 0;
-		if (pci_enable_msix(dev, &msix_entry, 1) == 0) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq_flags = IRQF_NO_THREAD;
-			udev->info.irq = msix_entry.vector;
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#else
-		if (pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_MSIX) == 1) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq = pci_irq_vector(dev, 0);
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#endif
-		/* fall back to INTX */
-	case RTE_INTR_MODE_LEGACY:
-		if (pci_intx_mask_supported(dev)) {
-			dev_dbg(&dev->dev, "using INTX");
-			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
-			udev->info.irq = dev->irq;
-			udev->mode = RTE_INTR_MODE_LEGACY;
-			break;
-		}
-		dev_notice(&dev->dev, "PCI INTX mask not supported\n");
-		/* fall back to no IRQ */
-	case RTE_INTR_MODE_NONE:
-		udev->mode = RTE_INTR_MODE_NONE;
-		udev->info.irq = 0;
-		break;
-
-	default:
-		dev_err(&dev->dev, "invalid IRQ mode %u",
-			igbuio_intr_mode_preferred);
-		err = -EINVAL;
+	err = igbuio_pci_enable_interrupts(udev);
+	if (err != 0)
 		goto fail_release_iomem;
-	}
 
 	err = sysfs_create_group(&dev->dev.kobj, &dev_attr_grp);
 	if (err != 0)
@@ -497,8 +574,7 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 fail_release_iomem:
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(udev->pdev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 fail_free:
 	kfree(udev);
@@ -514,8 +590,7 @@ igbuio_pci_remove(struct pci_dev *dev)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 	uio_unregister_device(&udev->info);
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(dev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 	pci_set_drvdata(dev, NULL);
 	kfree(udev);
@@ -532,6 +607,9 @@ igbuio_config_intr_mode(char *intr_str)
 	if (!strcmp(intr_str, RTE_INTR_MODE_MSIX_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
 		pr_info("Use MSIX interrupt\n");
+	} else if (!strcmp(intr_str, RTE_INTR_MODE_MSI_NAME)) {
+		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSI;
+		pr_info("Use MSI interrupt\n");
 	} else if (!strcmp(intr_str, RTE_INTR_MODE_LEGACY_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_LEGACY;
 		pr_info("Use legacy interrupt\n");
@@ -575,6 +653,7 @@ module_param(intr_mode, charp, S_IRUGO);
 MODULE_PARM_DESC(intr_mode,
 "igb_uio interrupt mode (default=msix):\n"
 "    " RTE_INTR_MODE_MSIX_NAME "       Use MSIX interrupt\n"
+"    " RTE_INTR_MODE_MSI_NAME "        Use MSI interrupt\n"
 "    " RTE_INTR_MODE_LEGACY_NAME "     Use Legacy interrupt\n"
 "\n");
 
-- 
2.7.4

[dpdk-dev] [PATCH v2 1/2] igb_uio: MSI IRQ mode, irq enable/disable refactored

[Markus Theil]

This patch adds MSI IRQ mode and in a way, that should
also work on older kernel versions. The base for my patch
was an attempt to do this in cf705bc36c which was later reverted in
d8ee82745a. Compilation was tested on Linux 3.2, 4.10 and 4.12.

MSI(X) setup was already using pci_alloc_irq_vectors before,
but calls to pci_free_irq_vectors were missing and added.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/compat.h  |   9 +-
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 175 ++++++++++++++++++++++--------
 2 files changed, 135 insertions(+), 49 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
index b800a53..8674088 100644
--- a/lib/librte_eal/linuxapp/igb_uio/compat.h
+++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
@@ -125,5 +125,12 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
 #endif /* < 3.3.0 */
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(4, 8, 0)
-#define HAVE_PCI_ENABLE_MSIX
+
+#define HAVE_PCI_ENABLE_MSIX 1
+#define HAVE_PCI_ENABLE_MSI 1
+
+#else
+
+#define HAVE_ALLOC_IRQ_VECTORS 1
+
 #endif
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 07a19a3..0a2f993 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -91,6 +91,7 @@ static struct attribute *dev_attrs[] = {
 static const struct attribute_group dev_attr_grp = {
 	.attrs = dev_attrs,
 };
+
 /*
  * It masks the msix on/off of generating MSI-X messages.
  */
@@ -113,6 +114,29 @@ igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state)
 	}
 }
 
+/*
+ * It masks the msi on/off of generating MSI messages.
+ */
+static void
+igbuio_msi_mask_irq(struct pci_dev *pdev, struct msi_desc *desc, int32_t state)
+{
+	u32 mask_bits = desc->masked;
+	u32 offset = desc->irq - pdev->irq;
+	u32 mask = 1 << offset;
+	u32 flag = !!state << offset;
+
+	if (!desc->msi_attrib.maskbit)
+		return;
+
+	mask_bits &= ~mask;
+	mask_bits |= flag;
+
+	if(mask_bits != desc->masked) {
+		pci_write_config_dword(pdev, desc->mask_pos, mask_bits);
+		desc->masked = mask_bits;
+	}
+}
+
 /**
  * This is the irqcontrol callback to be registered to uio_info.
  * It can be used to disable/enable interrupt from user space processes.
@@ -146,6 +170,16 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
 		list_for_each_entry(desc, &pdev->dev.msi_list, list)
 			igbuio_msix_mask_irq(desc, irq_state);
 #endif
+	} else if (udev->mode == RTE_INTR_MODE_MSI) {
+		struct msi_desc *desc;
+
+#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
+		list_for_each_entry(desc, &pdev->msi_list, list)
+			igbuio_msi_mask_irq(pdev, desc, irq_state);
+#else
+		list_for_each_entry(desc, &pdev->dev.msi_list, list)
+			igbuio_msi_mask_irq(pdev, desc, irq_state);
+#endif
 	}
 	pci_cfg_access_unlock(pdev);
 
@@ -309,6 +343,91 @@ igbuio_pci_release_iomem(struct uio_info *info)
 }
 
 static int
+igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
+{
+	int err = 0;
+#ifdef HAVE_PCI_ENABLE_MSIX
+	struct msix_entry msix_entry;
+#endif
+
+	switch (igbuio_intr_mode_preferred) {
+	case RTE_INTR_MODE_MSIX:
+		/* Only 1 msi-x vector needed */
+#ifdef HAVE_PCI_ENABLE_MSIX
+		msix_entry.entry = 0;
+		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = msix_entry.vector;
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#endif
+	case RTE_INTR_MODE_MSI:
+#ifdef HAVE_PCI_ENABLE_MSI
+		if (pci_enable_msi(udev->pdev) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSI) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#endif
+	/* fall back to INTX */
+	case RTE_INTR_MODE_LEGACY:
+		if (pci_intx_mask_supported(udev->pdev)) {
+			dev_dbg(&udev->pdev->dev, "using INTX");
+			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_LEGACY;
+			break;
+		}
+		dev_notice(&udev->pdev->dev, "PCI INTX mask not supported\n");
+		/* fall back to no IRQ */
+	case RTE_INTR_MODE_NONE:
+		udev->mode = RTE_INTR_MODE_NONE;
+		udev->info.irq = 0;
+		break;
+
+	default:
+		dev_err(&udev->pdev->dev, "invalid IRQ mode %u",
+			igbuio_intr_mode_preferred);
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static void
+igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
+{
+#ifndef HAVE_ALLOC_IRQ_VECTORS
+	if (udev->mode == RTE_INTR_MODE_MSIX)
+		pci_disable_msix(udev->pdev);
+	if (udev->mode == RTE_INTR_MODE_MSI)
+		pci_disable_msi(udev->pdev);
+#else
+	if (udev->mode == RTE_INTR_MODE_MSIX ||
+		udev->mode == RTE_INTR_MODE_MSI)
+		pci_free_irq_vectors(udev->pdev);
+#endif
+}
+
+static int
 igbuio_setup_bars(struct pci_dev *dev, struct uio_info *info)
 {
 	int i, iom, iop, ret;
@@ -356,9 +475,6 @@ static int
 igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	struct rte_uio_pci_dev *udev;
-#ifdef HAVE_PCI_ENABLE_MSIX
-	struct msix_entry msix_entry;
-#endif
 	dma_addr_t map_dma_addr;
 	void *map_addr;
 	int err;
@@ -413,48 +529,9 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	udev->info.priv = udev;
 	udev->pdev = dev;
 
-	switch (igbuio_intr_mode_preferred) {
-	case RTE_INTR_MODE_MSIX:
-		/* Only 1 msi-x vector needed */
-#ifdef HAVE_PCI_ENABLE_MSIX
-		msix_entry.entry = 0;
-		if (pci_enable_msix(dev, &msix_entry, 1) == 0) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq_flags = IRQF_NO_THREAD;
-			udev->info.irq = msix_entry.vector;
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#else
-		if (pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_MSIX) == 1) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq = pci_irq_vector(dev, 0);
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#endif
-		/* fall back to INTX */
-	case RTE_INTR_MODE_LEGACY:
-		if (pci_intx_mask_supported(dev)) {
-			dev_dbg(&dev->dev, "using INTX");
-			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
-			udev->info.irq = dev->irq;
-			udev->mode = RTE_INTR_MODE_LEGACY;
-			break;
-		}
-		dev_notice(&dev->dev, "PCI INTX mask not supported\n");
-		/* fall back to no IRQ */
-	case RTE_INTR_MODE_NONE:
-		udev->mode = RTE_INTR_MODE_NONE;
-		udev->info.irq = 0;
-		break;
-
-	default:
-		dev_err(&dev->dev, "invalid IRQ mode %u",
-			igbuio_intr_mode_preferred);
-		err = -EINVAL;
+	err = igbuio_pci_enable_interrupts(udev);
+	if (err != 0)
 		goto fail_release_iomem;
-	}
 
 	err = sysfs_create_group(&dev->dev.kobj, &dev_attr_grp);
 	if (err != 0)
@@ -497,8 +574,7 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 fail_release_iomem:
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(udev->pdev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 fail_free:
 	kfree(udev);
@@ -514,8 +590,7 @@ igbuio_pci_remove(struct pci_dev *dev)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 	uio_unregister_device(&udev->info);
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(dev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 	pci_set_drvdata(dev, NULL);
 	kfree(udev);
@@ -532,6 +607,9 @@ igbuio_config_intr_mode(char *intr_str)
 	if (!strcmp(intr_str, RTE_INTR_MODE_MSIX_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
 		pr_info("Use MSIX interrupt\n");
+	} else if (!strcmp(intr_str, RTE_INTR_MODE_MSI_NAME)) {
+		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSI;
+		pr_info("Use MSI interrupt\n");
 	} else if (!strcmp(intr_str, RTE_INTR_MODE_LEGACY_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_LEGACY;
 		pr_info("Use legacy interrupt\n");
@@ -575,6 +653,7 @@ module_param(intr_mode, charp, S_IRUGO);
 MODULE_PARM_DESC(intr_mode,
 "igb_uio interrupt mode (default=msix):\n"
 "    " RTE_INTR_MODE_MSIX_NAME "       Use MSIX interrupt\n"
+"    " RTE_INTR_MODE_MSI_NAME "        Use MSI interrupt\n"
 "    " RTE_INTR_MODE_LEGACY_NAME "     Use Legacy interrupt\n"
 "\n");
 
-- 
2.7.4

[dpdk-dev] [PATCH v2 2/2] igb_uio: conform to coding conventions

[Markus Theil]

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 0a2f993..bd94eb4 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -131,7 +131,7 @@ igbuio_msi_mask_irq(struct pci_dev *pdev, struct msi_desc *desc, int32_t state)
 	mask_bits &= ~mask;
 	mask_bits |= flag;
 
-	if(mask_bits != desc->masked) {
+	if (mask_bits != desc->masked) {
 		pci_write_config_dword(pdev, desc->mask_pos, mask_bits);
 		desc->masked = mask_bits;
 	}
-- 
2.7.4

[dpdk-dev] [PATCH v3] igb_uio: MSI IRQ mode, irq enable/disable refactored

[Markus Theil]

This patch adds MSI IRQ mode and in a way, that should
also work on older kernel versions. The base for my patch
was an attempt to do this in cf705bc36c which was later reverted in
d8ee82745a. Compilation was tested on Linux 3.2, 4.10 and 4.12.

MSI(X) setup was already using pci_alloc_irq_vectors before,
but calls to pci_free_irq_vectors were missing and added.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/compat.h  |   9 +-
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 175 ++++++++++++++++++++++--------
 2 files changed, 135 insertions(+), 49 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
index b800a53..8674088 100644
--- a/lib/librte_eal/linuxapp/igb_uio/compat.h
+++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
@@ -125,5 +125,12 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
 #endif /* < 3.3.0 */
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(4, 8, 0)
-#define HAVE_PCI_ENABLE_MSIX
+
+#define HAVE_PCI_ENABLE_MSIX 1
+#define HAVE_PCI_ENABLE_MSI 1
+
+#else
+
+#define HAVE_ALLOC_IRQ_VECTORS 1
+
 #endif
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 07a19a3..bd94eb4 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -91,6 +91,7 @@ static struct attribute *dev_attrs[] = {
 static const struct attribute_group dev_attr_grp = {
 	.attrs = dev_attrs,
 };
+
 /*
  * It masks the msix on/off of generating MSI-X messages.
  */
@@ -113,6 +114,29 @@ igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state)
 	}
 }
 
+/*
+ * It masks the msi on/off of generating MSI messages.
+ */
+static void
+igbuio_msi_mask_irq(struct pci_dev *pdev, struct msi_desc *desc, int32_t state)
+{
+	u32 mask_bits = desc->masked;
+	u32 offset = desc->irq - pdev->irq;
+	u32 mask = 1 << offset;
+	u32 flag = !!state << offset;
+
+	if (!desc->msi_attrib.maskbit)
+		return;
+
+	mask_bits &= ~mask;
+	mask_bits |= flag;
+
+	if (mask_bits != desc->masked) {
+		pci_write_config_dword(pdev, desc->mask_pos, mask_bits);
+		desc->masked = mask_bits;
+	}
+}
+
 /**
  * This is the irqcontrol callback to be registered to uio_info.
  * It can be used to disable/enable interrupt from user space processes.
@@ -146,6 +170,16 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
 		list_for_each_entry(desc, &pdev->dev.msi_list, list)
 			igbuio_msix_mask_irq(desc, irq_state);
 #endif
+	} else if (udev->mode == RTE_INTR_MODE_MSI) {
+		struct msi_desc *desc;
+
+#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
+		list_for_each_entry(desc, &pdev->msi_list, list)
+			igbuio_msi_mask_irq(pdev, desc, irq_state);
+#else
+		list_for_each_entry(desc, &pdev->dev.msi_list, list)
+			igbuio_msi_mask_irq(pdev, desc, irq_state);
+#endif
 	}
 	pci_cfg_access_unlock(pdev);
 
@@ -309,6 +343,91 @@ igbuio_pci_release_iomem(struct uio_info *info)
 }
 
 static int
+igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
+{
+	int err = 0;
+#ifdef HAVE_PCI_ENABLE_MSIX
+	struct msix_entry msix_entry;
+#endif
+
+	switch (igbuio_intr_mode_preferred) {
+	case RTE_INTR_MODE_MSIX:
+		/* Only 1 msi-x vector needed */
+#ifdef HAVE_PCI_ENABLE_MSIX
+		msix_entry.entry = 0;
+		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = msix_entry.vector;
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#endif
+	case RTE_INTR_MODE_MSI:
+#ifdef HAVE_PCI_ENABLE_MSI
+		if (pci_enable_msi(udev->pdev) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSI) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#endif
+	/* fall back to INTX */
+	case RTE_INTR_MODE_LEGACY:
+		if (pci_intx_mask_supported(udev->pdev)) {
+			dev_dbg(&udev->pdev->dev, "using INTX");
+			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_LEGACY;
+			break;
+		}
+		dev_notice(&udev->pdev->dev, "PCI INTX mask not supported\n");
+		/* fall back to no IRQ */
+	case RTE_INTR_MODE_NONE:
+		udev->mode = RTE_INTR_MODE_NONE;
+		udev->info.irq = 0;
+		break;
+
+	default:
+		dev_err(&udev->pdev->dev, "invalid IRQ mode %u",
+			igbuio_intr_mode_preferred);
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static void
+igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
+{
+#ifndef HAVE_ALLOC_IRQ_VECTORS
+	if (udev->mode == RTE_INTR_MODE_MSIX)
+		pci_disable_msix(udev->pdev);
+	if (udev->mode == RTE_INTR_MODE_MSI)
+		pci_disable_msi(udev->pdev);
+#else
+	if (udev->mode == RTE_INTR_MODE_MSIX ||
+		udev->mode == RTE_INTR_MODE_MSI)
+		pci_free_irq_vectors(udev->pdev);
+#endif
+}
+
+static int
 igbuio_setup_bars(struct pci_dev *dev, struct uio_info *info)
 {
 	int i, iom, iop, ret;
@@ -356,9 +475,6 @@ static int
 igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	struct rte_uio_pci_dev *udev;
-#ifdef HAVE_PCI_ENABLE_MSIX
-	struct msix_entry msix_entry;
-#endif
 	dma_addr_t map_dma_addr;
 	void *map_addr;
 	int err;
@@ -413,48 +529,9 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	udev->info.priv = udev;
 	udev->pdev = dev;
 
-	switch (igbuio_intr_mode_preferred) {
-	case RTE_INTR_MODE_MSIX:
-		/* Only 1 msi-x vector needed */
-#ifdef HAVE_PCI_ENABLE_MSIX
-		msix_entry.entry = 0;
-		if (pci_enable_msix(dev, &msix_entry, 1) == 0) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq_flags = IRQF_NO_THREAD;
-			udev->info.irq = msix_entry.vector;
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#else
-		if (pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_MSIX) == 1) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq = pci_irq_vector(dev, 0);
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#endif
-		/* fall back to INTX */
-	case RTE_INTR_MODE_LEGACY:
-		if (pci_intx_mask_supported(dev)) {
-			dev_dbg(&dev->dev, "using INTX");
-			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
-			udev->info.irq = dev->irq;
-			udev->mode = RTE_INTR_MODE_LEGACY;
-			break;
-		}
-		dev_notice(&dev->dev, "PCI INTX mask not supported\n");
-		/* fall back to no IRQ */
-	case RTE_INTR_MODE_NONE:
-		udev->mode = RTE_INTR_MODE_NONE;
-		udev->info.irq = 0;
-		break;
-
-	default:
-		dev_err(&dev->dev, "invalid IRQ mode %u",
-			igbuio_intr_mode_preferred);
-		err = -EINVAL;
+	err = igbuio_pci_enable_interrupts(udev);
+	if (err != 0)
 		goto fail_release_iomem;
-	}
 
 	err = sysfs_create_group(&dev->dev.kobj, &dev_attr_grp);
 	if (err != 0)
@@ -497,8 +574,7 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 fail_release_iomem:
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(udev->pdev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 fail_free:
 	kfree(udev);
@@ -514,8 +590,7 @@ igbuio_pci_remove(struct pci_dev *dev)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 	uio_unregister_device(&udev->info);
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(dev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 	pci_set_drvdata(dev, NULL);
 	kfree(udev);
@@ -532,6 +607,9 @@ igbuio_config_intr_mode(char *intr_str)
 	if (!strcmp(intr_str, RTE_INTR_MODE_MSIX_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
 		pr_info("Use MSIX interrupt\n");
+	} else if (!strcmp(intr_str, RTE_INTR_MODE_MSI_NAME)) {
+		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSI;
+		pr_info("Use MSI interrupt\n");
 	} else if (!strcmp(intr_str, RTE_INTR_MODE_LEGACY_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_LEGACY;
 		pr_info("Use legacy interrupt\n");
@@ -575,6 +653,7 @@ module_param(intr_mode, charp, S_IRUGO);
 MODULE_PARM_DESC(intr_mode,
 "igb_uio interrupt mode (default=msix):\n"
 "    " RTE_INTR_MODE_MSIX_NAME "       Use MSIX interrupt\n"
+"    " RTE_INTR_MODE_MSI_NAME "        Use MSI interrupt\n"
 "    " RTE_INTR_MODE_LEGACY_NAME "     Use Legacy interrupt\n"
 "\n");
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH v3] igb_uio: MSI IRQ mode, irq enable/disable refactored

[Ferruh Yigit]

On 8/22/2017 2:28 PM, Markus Theil wrote:
> This patch adds MSI IRQ mode and in a way, that should
> also work on older kernel versions. The base for my patch
> was an attempt to do this in cf705bc36c which was later reverted in
> d8ee82745a. Compilation was tested on Linux 3.2, 4.10 and 4.12.
> 
> MSI(X) setup was already using pci_alloc_irq_vectors before,
> but calls to pci_free_irq_vectors were missing and added.

Overall looks good to me, would you mind splitting this into multiple
patches:
1- Create igbuio_pci_enable_interrupts() and
igbuio_pci_disable_interrupts() functions without changing anything

2- Add pci_free_irq_vectors()

3- add MSI support

> 
> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>

<...>

> +/*
> + * It masks the msi on/off of generating MSI messages.
> + */
> +static void
> +igbuio_msi_mask_irq(struct pci_dev *pdev, struct msi_desc *desc, int32_t state> +{
> +	u32 mask_bits = desc->masked;
> +	u32 offset = desc->irq - pdev->irq;
> +	u32 mask = 1 << offset;
> +	u32 flag = !!state << offset;
> +
> +	if (!desc->msi_attrib.maskbit)
> +		return;
> +
> +	mask_bits &= ~mask;
> +	mask_bits |= flag;
> +
> +	if (mask_bits != desc->masked) {
> +		pci_write_config_dword(pdev, desc->mask_pos, mask_bits);
> +		desc->masked = mask_bits;
> +	}

I am not familiar with details of this function, is there a sample usage
in kernel that can help understanding it?

> +}
> +
>  /**
>   * This is the irqcontrol callback to be registered to uio_info.
>   * It can be used to disable/enable interrupt from user space processes.
> @@ -146,6 +170,16 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
>  		list_for_each_entry(desc, &pdev->dev.msi_list, list)
>  			igbuio_msix_mask_irq(desc, irq_state);
>  #endif
> +	} else if (udev->mode == RTE_INTR_MODE_MSI) {
> +		struct msi_desc *desc;

Since two if block is using this, I think this can be moved to function
scope.

> +
> +#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))

Can you please move version check to the compat.h? I am aware this
version check is there for msix, but since we are doubling it, it can be
good time to fix both.

> +		list_for_each_entry(desc, &pdev->msi_list, list)> +			igbuio_msi_mask_irq(pdev, desc, irq_state);

This walks on same list (msi_list) with above msix code. Is there a way
to say if this msi_desc for MSI or MSIx interrupts, how
igbuio_msi_mask_irq() will distinguish them?

> +#else
> +		list_for_each_entry(desc, &pdev->dev.msi_list, list)
> +			igbuio_msi_mask_irq(pdev, desc, irq_state);
> +#endif
>  	}
>  	pci_cfg_access_unlock(pdev);
>  
> @@ -309,6 +343,91 @@ igbuio_pci_release_iomem(struct uio_info *info)
>  }
>  
>  static int
> +igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
> +{
> +	int err = 0;
> +#ifdef HAVE_PCI_ENABLE_MSIX
> +	struct msix_entry msix_entry;
> +#endif
> +
> +	switch (igbuio_intr_mode_preferred) {
> +	case RTE_INTR_MODE_MSIX:
> +		/* Only 1 msi-x vector needed */
> +#ifdef HAVE_PCI_ENABLE_MSIX
> +		msix_entry.entry = 0;
> +		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
> +			dev_dbg(&udev->pdev->dev, "using MSI-X");
> +			udev->info.irq_flags = IRQF_NO_THREAD;
> +			udev->info.irq = msix_entry.vector;
> +			udev->mode = RTE_INTR_MODE_MSIX;
> +			break;
> +		}
> +#else
> +		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
> +			dev_dbg(&udev->pdev->dev, "using MSI-X");
> +			udev->info.irq = pci_irq_vector(udev->pdev, 0);
> +			udev->mode = RTE_INTR_MODE_MSIX;
> +			break;
> +		}
> +#endif

Please add fall back comment: /* fall back to MSI */

And related this fallback, previously flow was:
Try MSIX, if fails try legacy.

Now it is:
Try MSIX, if fails try MSI, if fails try legacy.

Do you think, does this new step included may break anyone's out that
would expect to fallback to legacy? (although I don't expect, I believe
good to sound it)

> +	case RTE_INTR_MODE_MSI:
> +#ifdef HAVE_PCI_ENABLE_MSI
> +		if (pci_enable_msi(udev->pdev) == 0) {
> +			dev_dbg(&udev->pdev->dev, "using MSI");
> +			udev->info.irq_flags = IRQF_NO_THREAD;
> +			udev->info.irq = udev->pdev->irq;
> +			udev->mode = RTE_INTR_MODE_MSI;
> +			break;
> +		}
> +#else
> +		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSI) == 1) {
> +			dev_dbg(&udev->pdev->dev, "using MSI");
> +			udev->info.irq = pci_irq_vector(udev->pdev, 0);
> +			udev->mode = RTE_INTR_MODE_MSI;
> +			break;
> +		}
> +#endif
> +	/* fall back to INTX */
> +	case RTE_INTR_MODE_LEGACY:
> +		if (pci_intx_mask_supported(udev->pdev)) {
> +			dev_dbg(&udev->pdev->dev, "using INTX");
> +			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
> +			udev->info.irq = udev->pdev->irq;
> +			udev->mode = RTE_INTR_MODE_LEGACY;
> +			break;
> +		}
> +		dev_notice(&udev->pdev->dev, "PCI INTX mask not supported\n");
> +		/* fall back to no IRQ */
> +	case RTE_INTR_MODE_NONE:
> +		udev->mode = RTE_INTR_MODE_NONE;
> +		udev->info.irq = 0;
> +		break;
> +
> +	default:
> +		dev_err(&udev->pdev->dev, "invalid IRQ mode %u",
> +			igbuio_intr_mode_preferred);
> +		err = -EINVAL;
> +	}
> +
> +	return err;
> +}

<...>

Re: [dpdk-dev] [PATCH v3] igb_uio: MSI IRQ mode, irq enable/disable refactored

[Markus Theil]

On 30.08.2017 18:32, Ferruh Yigit wrote:
> Please add fall back comment: /* fall back to MSI */
>
> And related this fallback, previously flow was:
> Try MSIX, if fails try legacy.
>
> Now it is:
> Try MSIX, if fails try MSI, if fails try legacy.
>
> Do you think, does this new step included may break anyone's out that
> would expect to fallback to legacy? (although I don't expect, I believe
> good to sound it)
>
I also don't expect it, but can't say it with 100% certainty of course.

[dpdk-dev] [PATCH v4 1/3] igb_uio: refactor irq enable/disable into own functions

[Markus Theil]

Interrupt setup code in igb_uio has to deal with multiple
types of interrupts and kernel versions. This patch moves
the setup and teardown code into own functions, to make
it more readable.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 112 +++++++++++++++++-------------
 1 file changed, 64 insertions(+), 48 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 07a19a3..e2e9263 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -309,6 +309,66 @@ igbuio_pci_release_iomem(struct uio_info *info)
 }
 
 static int
+igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
+{
+	int err = 0;
+#ifdef HAVE_PCI_ENABLE_MSIX
+	struct msix_entry msix_entry;
+#endif
+
+	switch (igbuio_intr_mode_preferred) {
+	case RTE_INTR_MODE_MSIX:
+		/* Only 1 msi-x vector needed */
+#ifdef HAVE_PCI_ENABLE_MSIX
+		msix_entry.entry = 0;
+		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = msix_entry.vector;
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#endif
+	/* fall back to INTX */
+	case RTE_INTR_MODE_LEGACY:
+		if (pci_intx_mask_supported(udev->pdev)) {
+			dev_dbg(&udev->pdev->dev, "using INTX");
+			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_LEGACY;
+			break;
+		}
+		dev_notice(&udev->pdev->dev, "PCI INTX mask not supported\n");
+		/* fall back to no IRQ */
+	case RTE_INTR_MODE_NONE:
+		udev->mode = RTE_INTR_MODE_NONE;
+		udev->info.irq = 0;
+		break;
+
+	default:
+		dev_err(&udev->pdev->dev, "invalid IRQ mode %u",
+			igbuio_intr_mode_preferred);
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static void
+igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
+{
+	if (udev->mode == RTE_INTR_MODE_MSIX)
+		pci_disable_msix(udev->pdev);
+}
+
+static int
 igbuio_setup_bars(struct pci_dev *dev, struct uio_info *info)
 {
 	int i, iom, iop, ret;
@@ -356,9 +416,6 @@ static int
 igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	struct rte_uio_pci_dev *udev;
-#ifdef HAVE_PCI_ENABLE_MSIX
-	struct msix_entry msix_entry;
-#endif
 	dma_addr_t map_dma_addr;
 	void *map_addr;
 	int err;
@@ -413,48 +470,9 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	udev->info.priv = udev;
 	udev->pdev = dev;
 
-	switch (igbuio_intr_mode_preferred) {
-	case RTE_INTR_MODE_MSIX:
-		/* Only 1 msi-x vector needed */
-#ifdef HAVE_PCI_ENABLE_MSIX
-		msix_entry.entry = 0;
-		if (pci_enable_msix(dev, &msix_entry, 1) == 0) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq_flags = IRQF_NO_THREAD;
-			udev->info.irq = msix_entry.vector;
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#else
-		if (pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_MSIX) == 1) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq = pci_irq_vector(dev, 0);
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#endif
-		/* fall back to INTX */
-	case RTE_INTR_MODE_LEGACY:
-		if (pci_intx_mask_supported(dev)) {
-			dev_dbg(&dev->dev, "using INTX");
-			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
-			udev->info.irq = dev->irq;
-			udev->mode = RTE_INTR_MODE_LEGACY;
-			break;
-		}
-		dev_notice(&dev->dev, "PCI INTX mask not supported\n");
-		/* fall back to no IRQ */
-	case RTE_INTR_MODE_NONE:
-		udev->mode = RTE_INTR_MODE_NONE;
-		udev->info.irq = 0;
-		break;
-
-	default:
-		dev_err(&dev->dev, "invalid IRQ mode %u",
-			igbuio_intr_mode_preferred);
-		err = -EINVAL;
+	err = igbuio_pci_enable_interrupts(udev);
+	if (err != 0)
 		goto fail_release_iomem;
-	}
 
 	err = sysfs_create_group(&dev->dev.kobj, &dev_attr_grp);
 	if (err != 0)
@@ -497,8 +515,7 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 fail_release_iomem:
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(udev->pdev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 fail_free:
 	kfree(udev);
@@ -514,8 +531,7 @@ igbuio_pci_remove(struct pci_dev *dev)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 	uio_unregister_device(&udev->info);
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(dev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 	pci_set_drvdata(dev, NULL);
 	kfree(udev);
-- 
2.7.4

[dpdk-dev] [PATCH v4 2/3] igb_uio: fix irq disable on recent kernels

[Markus Theil]

igb_uio already allocates irqs using pci_alloc_irq_vectors on
recent kernels >= 4.8. The interrupt disable code was not
using the corresponding pci_free_irq_vectors, but the also
deprecated pci_disable_msix, before this fix.

Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for MSI-X")
Cc: nicolas.dichtel@6wind.com

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/compat.h  | 4 ++--
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 9 +++++++--
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
index b800a53..3825933 100644
--- a/lib/librte_eal/linuxapp/igb_uio/compat.h
+++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
@@ -124,6 +124,6 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
 
 #endif /* < 3.3.0 */
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 8, 0)
-#define HAVE_PCI_ENABLE_MSIX
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
+#define HAVE_ALLOC_IRQ_VECTORS 1
 #endif
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index e2e9263..93bb71d 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -312,14 +312,14 @@ static int
 igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 {
 	int err = 0;
-#ifdef HAVE_PCI_ENABLE_MSIX
+#ifndef HAVE_ALLOC_IRQ_VECTORS
 	struct msix_entry msix_entry;
 #endif
 
 	switch (igbuio_intr_mode_preferred) {
 	case RTE_INTR_MODE_MSIX:
 		/* Only 1 msi-x vector needed */
-#ifdef HAVE_PCI_ENABLE_MSIX
+#ifndef HAVE_ALLOC_IRQ_VECTORS
 		msix_entry.entry = 0;
 		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
 			dev_dbg(&udev->pdev->dev, "using MSI-X");
@@ -364,8 +364,13 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 static void
 igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
 {
+#ifndef HAVE_ALLOC_IRQ_VECTORS
 	if (udev->mode == RTE_INTR_MODE_MSIX)
 		pci_disable_msix(udev->pdev);
+#else
+	if (udev->mode == RTE_INTR_MODE_MSIX)
+		pci_free_irq_vectors(udev->pdev);
+#endif
 }
 
 static int
-- 
2.7.4

[dpdk-dev] [PATCH v4 3/3] igb_uio: MSI IRQ mode

[Markus Theil]

This patch adds MSI IRQ mode and in a way, that should
also work on older kernel versions. The base for my patch
was an attempt to do this in cf705bc36c which was later reverted in
d8ee82745a. Compilation was tested on Linux 3.2, 4.10 and 4.12.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 60 ++++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 93bb71d..99a085a 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -91,6 +91,7 @@ static struct attribute *dev_attrs[] = {
 static const struct attribute_group dev_attr_grp = {
 	.attrs = dev_attrs,
 };
+
 /*
  * It masks the msix on/off of generating MSI-X messages.
  */
@@ -113,6 +114,29 @@ igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state)
 	}
 }
 
+/*
+ * It masks the msi on/off of generating MSI messages.
+ */
+static void
+igbuio_msi_mask_irq(struct pci_dev *pdev, struct msi_desc *desc, int32_t state)
+{
+	u32 mask_bits = desc->masked;
+	u32 offset = desc->irq - pdev->irq;
+	u32 mask = 1 << offset;
+	u32 flag = !!state << offset;
+
+	if (!desc->msi_attrib.maskbit)
+		return;
+
+	mask_bits &= ~mask;
+	mask_bits |= flag;
+
+	if (mask_bits != desc->masked) {
+		pci_write_config_dword(pdev, desc->mask_pos, mask_bits);
+		desc->masked = mask_bits;
+	}
+}
+
 /**
  * This is the irqcontrol callback to be registered to uio_info.
  * It can be used to disable/enable interrupt from user space processes.
@@ -146,6 +170,16 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
 		list_for_each_entry(desc, &pdev->dev.msi_list, list)
 			igbuio_msix_mask_irq(desc, irq_state);
 #endif
+	} else if (udev->mode == RTE_INTR_MODE_MSI) {
+	struct msi_desc *desc;
+
+#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
+	list_for_each_entry(desc, &pdev->msi_list, list)
+		igbuio_msi_mask_irq(pdev, desc, irq_state);
+#else
+	list_for_each_entry(desc, &pdev->dev.msi_list, list)
+		igbuio_msi_mask_irq(pdev, desc, irq_state);
+#endif
 	}
 	pci_cfg_access_unlock(pdev);
 
@@ -336,6 +370,23 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 			break;
 		}
 #endif
+	case RTE_INTR_MODE_MSI:
+#ifndef HAVE_ALLOC_IRQ_VECTORS
+		if (pci_enable_msi(udev->pdev) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSI) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#endif
 	/* fall back to INTX */
 	case RTE_INTR_MODE_LEGACY:
 		if (pci_intx_mask_supported(udev->pdev)) {
@@ -367,8 +418,11 @@ igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
 #ifndef HAVE_ALLOC_IRQ_VECTORS
 	if (udev->mode == RTE_INTR_MODE_MSIX)
 		pci_disable_msix(udev->pdev);
+	if (udev->mode == RTE_INTR_MODE_MSI)
+		pci_disable_msi(udev->pdev);
 #else
-	if (udev->mode == RTE_INTR_MODE_MSIX)
+	if (udev->mode == RTE_INTR_MODE_MSIX ||
+	    udev->mode == RTE_INTR_MODE_MSI)
 		pci_free_irq_vectors(udev->pdev);
 #endif
 }
@@ -553,6 +607,9 @@ igbuio_config_intr_mode(char *intr_str)
 	if (!strcmp(intr_str, RTE_INTR_MODE_MSIX_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
 		pr_info("Use MSIX interrupt\n");
+	} else if (!strcmp(intr_str, RTE_INTR_MODE_MSI_NAME)) {
+		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSI;
+		pr_info("Use MSI interrupt\n");
 	} else if (!strcmp(intr_str, RTE_INTR_MODE_LEGACY_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_LEGACY;
 		pr_info("Use legacy interrupt\n");
@@ -596,6 +653,7 @@ module_param(intr_mode, charp, S_IRUGO);
 MODULE_PARM_DESC(intr_mode,
 "igb_uio interrupt mode (default=msix):\n"
 "    " RTE_INTR_MODE_MSIX_NAME "       Use MSIX interrupt\n"
+"    " RTE_INTR_MODE_MSI_NAME "        Use MSI interrupt\n"
 "    " RTE_INTR_MODE_LEGACY_NAME "     Use Legacy interrupt\n"
 "\n");
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH v4 3/3] igb_uio: MSI IRQ mode

[Stephen Hemminger]

On Thu, 31 Aug 2017 12:22:29 +0200
Markus Theil <markus.theil@tu-ilmenau.de> wrote:

> +/*
> + * It masks the msi on/off of generating MSI messages.
> + */
> +static void
> +igbuio_msi_mask_irq(struct pci_dev *pdev, struct msi_desc *desc, int32_t state)
> +{
> +	u32 mask_bits = desc->masked;
> +	u32 offset = desc->irq - pdev->irq;
> +	u32 mask = 1 << offset;
> +	u32 flag = !!state << offset;
> +
> +	if (!desc->msi_attrib.maskbit)
> +		return;
> +
> +	mask_bits &= ~mask;
> +	mask_bits |= flag;
> +
> +	if (mask_bits != desc->masked) {
> +		pci_write_config_dword(pdev, desc->mask_pos, mask_bits);
> +		desc->masked = mask_bits;
> +	}
> +}
> +

Why not use the existing kernel API pci_msi_mask_irq()?

Re: [dpdk-dev] [PATCH v4 3/3] igb_uio: MSI IRQ mode

[Markus Theil]

On 31.08.2017 17:32, Stephen Hemminger wrote:
> On Thu, 31 Aug 2017 12:22:29 +0200
> Markus Theil <markus.theil@tu-ilmenau.de> wrote:
>
>> +/*
>> + * It masks the msi on/off of generating MSI messages.
>> + */
>> +static void
>> +igbuio_msi_mask_irq(struct pci_dev *pdev, struct msi_desc *desc, int32_t state)
>> +{
>> +	u32 mask_bits = desc->masked;
>> +	u32 offset = desc->irq - pdev->irq;
>> +	u32 mask = 1 << offset;
>> +	u32 flag = !!state << offset;
>> +
>> +	if (!desc->msi_attrib.maskbit)
>> +		return;
>> +
>> +	mask_bits &= ~mask;
>> +	mask_bits |= flag;
>> +
>> +	if (mask_bits != desc->masked) {
>> +		pci_write_config_dword(pdev, desc->mask_pos, mask_bits);
>> +		desc->masked = mask_bits;
>> +	}
>> +}
>> +
> Why not use the existing kernel API pci_msi_mask_irq()?
Thanks for the advice, I have considered it in v5.

[dpdk-dev] [PATCH v5 1/5] igb_uio: refactor irq enable/disable into own functions

[Markus Theil]

Interrupt setup code in igb_uio has to deal with multiple
types of interrupts and kernel versions. This patch moves
the setup and teardown code into own functions, to make
it more readable.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 112 +++++++++++++++++-------------
 1 file changed, 64 insertions(+), 48 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 07a19a3..e2e9263 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -309,6 +309,66 @@ igbuio_pci_release_iomem(struct uio_info *info)
 }
 
 static int
+igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
+{
+	int err = 0;
+#ifdef HAVE_PCI_ENABLE_MSIX
+	struct msix_entry msix_entry;
+#endif
+
+	switch (igbuio_intr_mode_preferred) {
+	case RTE_INTR_MODE_MSIX:
+		/* Only 1 msi-x vector needed */
+#ifdef HAVE_PCI_ENABLE_MSIX
+		msix_entry.entry = 0;
+		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = msix_entry.vector;
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#endif
+	/* fall back to INTX */
+	case RTE_INTR_MODE_LEGACY:
+		if (pci_intx_mask_supported(udev->pdev)) {
+			dev_dbg(&udev->pdev->dev, "using INTX");
+			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_LEGACY;
+			break;
+		}
+		dev_notice(&udev->pdev->dev, "PCI INTX mask not supported\n");
+		/* fall back to no IRQ */
+	case RTE_INTR_MODE_NONE:
+		udev->mode = RTE_INTR_MODE_NONE;
+		udev->info.irq = 0;
+		break;
+
+	default:
+		dev_err(&udev->pdev->dev, "invalid IRQ mode %u",
+			igbuio_intr_mode_preferred);
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static void
+igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
+{
+	if (udev->mode == RTE_INTR_MODE_MSIX)
+		pci_disable_msix(udev->pdev);
+}
+
+static int
 igbuio_setup_bars(struct pci_dev *dev, struct uio_info *info)
 {
 	int i, iom, iop, ret;
@@ -356,9 +416,6 @@ static int
 igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	struct rte_uio_pci_dev *udev;
-#ifdef HAVE_PCI_ENABLE_MSIX
-	struct msix_entry msix_entry;
-#endif
 	dma_addr_t map_dma_addr;
 	void *map_addr;
 	int err;
@@ -413,48 +470,9 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	udev->info.priv = udev;
 	udev->pdev = dev;
 
-	switch (igbuio_intr_mode_preferred) {
-	case RTE_INTR_MODE_MSIX:
-		/* Only 1 msi-x vector needed */
-#ifdef HAVE_PCI_ENABLE_MSIX
-		msix_entry.entry = 0;
-		if (pci_enable_msix(dev, &msix_entry, 1) == 0) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq_flags = IRQF_NO_THREAD;
-			udev->info.irq = msix_entry.vector;
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#else
-		if (pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_MSIX) == 1) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq = pci_irq_vector(dev, 0);
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#endif
-		/* fall back to INTX */
-	case RTE_INTR_MODE_LEGACY:
-		if (pci_intx_mask_supported(dev)) {
-			dev_dbg(&dev->dev, "using INTX");
-			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
-			udev->info.irq = dev->irq;
-			udev->mode = RTE_INTR_MODE_LEGACY;
-			break;
-		}
-		dev_notice(&dev->dev, "PCI INTX mask not supported\n");
-		/* fall back to no IRQ */
-	case RTE_INTR_MODE_NONE:
-		udev->mode = RTE_INTR_MODE_NONE;
-		udev->info.irq = 0;
-		break;
-
-	default:
-		dev_err(&dev->dev, "invalid IRQ mode %u",
-			igbuio_intr_mode_preferred);
-		err = -EINVAL;
+	err = igbuio_pci_enable_interrupts(udev);
+	if (err != 0)
 		goto fail_release_iomem;
-	}
 
 	err = sysfs_create_group(&dev->dev.kobj, &dev_attr_grp);
 	if (err != 0)
@@ -497,8 +515,7 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 fail_release_iomem:
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(udev->pdev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 fail_free:
 	kfree(udev);
@@ -514,8 +531,7 @@ igbuio_pci_remove(struct pci_dev *dev)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 	uio_unregister_device(&udev->info);
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(dev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 	pci_set_drvdata(dev, NULL);
 	kfree(udev);
-- 
2.7.4

[dpdk-dev] [PATCH v5 2/5] igb_uio: fix irq disable on recent kernels

[Markus Theil]

igb_uio already allocates irqs using pci_alloc_irq_vectors on
recent kernels >= 4.8. The interrupt disable code was not
using the corresponding pci_free_irq_vectors, but the also
deprecated pci_disable_msix, before this fix.

Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for MSI-X")
Cc: nicolas.dichtel@6wind.com

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/compat.h  | 4 ++--
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 9 +++++++--
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
index b800a53..3825933 100644
--- a/lib/librte_eal/linuxapp/igb_uio/compat.h
+++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
@@ -124,6 +124,6 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
 
 #endif /* < 3.3.0 */
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 8, 0)
-#define HAVE_PCI_ENABLE_MSIX
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
+#define HAVE_ALLOC_IRQ_VECTORS 1
 #endif
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index e2e9263..93bb71d 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -312,14 +312,14 @@ static int
 igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 {
 	int err = 0;
-#ifdef HAVE_PCI_ENABLE_MSIX
+#ifndef HAVE_ALLOC_IRQ_VECTORS
 	struct msix_entry msix_entry;
 #endif
 
 	switch (igbuio_intr_mode_preferred) {
 	case RTE_INTR_MODE_MSIX:
 		/* Only 1 msi-x vector needed */
-#ifdef HAVE_PCI_ENABLE_MSIX
+#ifndef HAVE_ALLOC_IRQ_VECTORS
 		msix_entry.entry = 0;
 		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
 			dev_dbg(&udev->pdev->dev, "using MSI-X");
@@ -364,8 +364,13 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 static void
 igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
 {
+#ifndef HAVE_ALLOC_IRQ_VECTORS
 	if (udev->mode == RTE_INTR_MODE_MSIX)
 		pci_disable_msix(udev->pdev);
+#else
+	if (udev->mode == RTE_INTR_MODE_MSIX)
+		pci_free_irq_vectors(udev->pdev);
+#endif
 }
 
 static int
-- 
2.7.4

[dpdk-dev] [PATCH v5 3/5] igb_uio: MSI IRQ mode

[Markus Theil]

This patch adds MSI IRQ mode and in a way, that should
also work on older kernel versions. The base for my patch
was an attempt to do this in cf705bc36c which was later reverted in
d8ee82745a. Compilation was tested on Linux 3.2, 4.10 and 4.12.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 60 ++++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 93bb71d..99a085a 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -91,6 +91,7 @@ static struct attribute *dev_attrs[] = {
 static const struct attribute_group dev_attr_grp = {
 	.attrs = dev_attrs,
 };
+
 /*
  * It masks the msix on/off of generating MSI-X messages.
  */
@@ -113,6 +114,29 @@ igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state)
 	}
 }
 
+/*
+ * It masks the msi on/off of generating MSI messages.
+ */
+static void
+igbuio_msi_mask_irq(struct pci_dev *pdev, struct msi_desc *desc, int32_t state)
+{
+	u32 mask_bits = desc->masked;
+	u32 offset = desc->irq - pdev->irq;
+	u32 mask = 1 << offset;
+	u32 flag = !!state << offset;
+
+	if (!desc->msi_attrib.maskbit)
+		return;
+
+	mask_bits &= ~mask;
+	mask_bits |= flag;
+
+	if (mask_bits != desc->masked) {
+		pci_write_config_dword(pdev, desc->mask_pos, mask_bits);
+		desc->masked = mask_bits;
+	}
+}
+
 /**
  * This is the irqcontrol callback to be registered to uio_info.
  * It can be used to disable/enable interrupt from user space processes.
@@ -146,6 +170,16 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
 		list_for_each_entry(desc, &pdev->dev.msi_list, list)
 			igbuio_msix_mask_irq(desc, irq_state);
 #endif
+	} else if (udev->mode == RTE_INTR_MODE_MSI) {
+	struct msi_desc *desc;
+
+#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
+	list_for_each_entry(desc, &pdev->msi_list, list)
+		igbuio_msi_mask_irq(pdev, desc, irq_state);
+#else
+	list_for_each_entry(desc, &pdev->dev.msi_list, list)
+		igbuio_msi_mask_irq(pdev, desc, irq_state);
+#endif
 	}
 	pci_cfg_access_unlock(pdev);
 
@@ -336,6 +370,23 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 			break;
 		}
 #endif
+	case RTE_INTR_MODE_MSI:
+#ifndef HAVE_ALLOC_IRQ_VECTORS
+		if (pci_enable_msi(udev->pdev) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSI) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#endif
 	/* fall back to INTX */
 	case RTE_INTR_MODE_LEGACY:
 		if (pci_intx_mask_supported(udev->pdev)) {
@@ -367,8 +418,11 @@ igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
 #ifndef HAVE_ALLOC_IRQ_VECTORS
 	if (udev->mode == RTE_INTR_MODE_MSIX)
 		pci_disable_msix(udev->pdev);
+	if (udev->mode == RTE_INTR_MODE_MSI)
+		pci_disable_msi(udev->pdev);
 #else
-	if (udev->mode == RTE_INTR_MODE_MSIX)
+	if (udev->mode == RTE_INTR_MODE_MSIX ||
+	    udev->mode == RTE_INTR_MODE_MSI)
 		pci_free_irq_vectors(udev->pdev);
 #endif
 }
@@ -553,6 +607,9 @@ igbuio_config_intr_mode(char *intr_str)
 	if (!strcmp(intr_str, RTE_INTR_MODE_MSIX_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
 		pr_info("Use MSIX interrupt\n");
+	} else if (!strcmp(intr_str, RTE_INTR_MODE_MSI_NAME)) {
+		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSI;
+		pr_info("Use MSI interrupt\n");
 	} else if (!strcmp(intr_str, RTE_INTR_MODE_LEGACY_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_LEGACY;
 		pr_info("Use legacy interrupt\n");
@@ -596,6 +653,7 @@ module_param(intr_mode, charp, S_IRUGO);
 MODULE_PARM_DESC(intr_mode,
 "igb_uio interrupt mode (default=msix):\n"
 "    " RTE_INTR_MODE_MSIX_NAME "       Use MSIX interrupt\n"
+"    " RTE_INTR_MODE_MSI_NAME "        Use MSI interrupt\n"
 "    " RTE_INTR_MODE_LEGACY_NAME "     Use Legacy interrupt\n"
 "\n");
 
-- 
2.7.4

[dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections

[Markus Theil]

This patch substitutes the custom MSI/MSI-X mask code and uses
already existing kernel APIs. Feedback/small corrections to the previous
patch of this series are also incorporated.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/compat.h  | 26 +++------
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 88 ++++++++-----------------------
 2 files changed, 31 insertions(+), 83 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
index 3825933..67a7ab3 100644
--- a/lib/librte_eal/linuxapp/igb_uio/compat.h
+++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
@@ -15,24 +15,6 @@
 #define HAVE_PTE_MASK_PAGE_IOMAP
 #endif
 
-#ifndef PCI_MSIX_ENTRY_SIZE
-#define PCI_MSIX_ENTRY_SIZE             16
-#define  PCI_MSIX_ENTRY_LOWER_ADDR      0
-#define  PCI_MSIX_ENTRY_UPPER_ADDR      4
-#define  PCI_MSIX_ENTRY_DATA            8
-#define  PCI_MSIX_ENTRY_VECTOR_CTRL     12
-#define   PCI_MSIX_ENTRY_CTRL_MASKBIT   1
-#endif
-
-/*
- * for kernels < 2.6.38 and backported patch that moves MSI-X entry definition
- * to pci_regs.h Those kernels has PCI_MSIX_ENTRY_SIZE defined but not
- * PCI_MSIX_ENTRY_CTRL_MASKBIT
- */
-#ifndef PCI_MSIX_ENTRY_CTRL_MASKBIT
-#define PCI_MSIX_ENTRY_CTRL_MASKBIT    1
-#endif
-
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 34) && \
 	(!(defined(RHEL_RELEASE_CODE) && \
 	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(5, 9)))
@@ -127,3 +109,11 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
 #define HAVE_ALLOC_IRQ_VECTORS 1
 #endif
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 19, 0)
+#define HAVE_PCI_MSI_MASK_IRQ 1
+#endif
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 37)
+#define HAVE_IRQ_DATA 1
+#endif
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 99a085a..5830453 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -92,51 +92,6 @@ static const struct attribute_group dev_attr_grp = {
 	.attrs = dev_attrs,
 };
 
-/*
- * It masks the msix on/off of generating MSI-X messages.
- */
-static void
-igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state)
-{
-	u32 mask_bits = desc->masked;
-	unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
-						PCI_MSIX_ENTRY_VECTOR_CTRL;
-
-	if (state != 0)
-		mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
-	else
-		mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
-
-	if (mask_bits != desc->masked) {
-		writel(mask_bits, desc->mask_base + offset);
-		readl(desc->mask_base);
-		desc->masked = mask_bits;
-	}
-}
-
-/*
- * It masks the msi on/off of generating MSI messages.
- */
-static void
-igbuio_msi_mask_irq(struct pci_dev *pdev, struct msi_desc *desc, int32_t state)
-{
-	u32 mask_bits = desc->masked;
-	u32 offset = desc->irq - pdev->irq;
-	u32 mask = 1 << offset;
-	u32 flag = !!state << offset;
-
-	if (!desc->msi_attrib.maskbit)
-		return;
-
-	mask_bits &= ~mask;
-	mask_bits |= flag;
-
-	if (mask_bits != desc->masked) {
-		pci_write_config_dword(pdev, desc->mask_pos, mask_bits);
-		desc->masked = mask_bits;
-	}
-}
-
 /**
  * This is the irqcontrol callback to be registered to uio_info.
  * It can be used to disable/enable interrupt from user space processes.
@@ -156,31 +111,31 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
 	struct rte_uio_pci_dev *udev = info->priv;
 	struct pci_dev *pdev = udev->pdev;
 
-	pci_cfg_access_lock(pdev);
-	if (udev->mode == RTE_INTR_MODE_LEGACY)
-		pci_intx(pdev, !!irq_state);
-
-	else if (udev->mode == RTE_INTR_MODE_MSIX) {
-		struct msi_desc *desc;
-
-#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
-		list_for_each_entry(desc, &pdev->msi_list, list)
-			igbuio_msix_mask_irq(desc, irq_state);
+#ifdef HAVE_IRQ_DATA
+	struct irq_data *irq = irq_get_irq_data(udev->info.irq);
 #else
-		list_for_each_entry(desc, &pdev->dev.msi_list, list)
-			igbuio_msix_mask_irq(desc, irq_state);
+	unsigned int irq = udev->info.irq;
 #endif
-	} else if (udev->mode == RTE_INTR_MODE_MSI) {
-	struct msi_desc *desc;
 
-#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
-	list_for_each_entry(desc, &pdev->msi_list, list)
-		igbuio_msi_mask_irq(pdev, desc, irq_state);
+	pci_cfg_access_lock(pdev);
+
+	if (udev->mode == RTE_INTR_MODE_MSIX || udev->mode == RTE_INTR_MODE_MSI) {
+#ifdef HAVE_PCI_MSI_MASK_IRQ
+		if (irq_state == 1)
+			pci_msi_unmask_irq(irq);
+		else
+			pci_msi_mask_irq(irq);
 #else
-	list_for_each_entry(desc, &pdev->dev.msi_list, list)
-		igbuio_msi_mask_irq(pdev, desc, irq_state);
+		if (irq_state == 1)
+			unmask_msi_irq(irq);
+		else
+			mask_msi_irq(irq);
 #endif
 	}
+
+	if (udev->mode == RTE_INTR_MODE_LEGACY)
+		pci_intx(pdev, !!irq_state);
+
 	pci_cfg_access_unlock(pdev);
 
 	return 0;
@@ -365,11 +320,13 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 #else
 		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
 			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq_flags = IRQF_NO_THREAD;
 			udev->info.irq = pci_irq_vector(udev->pdev, 0);
 			udev->mode = RTE_INTR_MODE_MSIX;
 			break;
 		}
 #endif
+	/* fall back to MSI */
 	case RTE_INTR_MODE_MSI:
 #ifndef HAVE_ALLOC_IRQ_VECTORS
 		if (pci_enable_msi(udev->pdev) == 0) {
@@ -382,6 +339,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 #else
 		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSI) == 1) {
 			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq_flags = IRQF_NO_THREAD;
 			udev->info.irq = pci_irq_vector(udev->pdev, 0);
 			udev->mode = RTE_INTR_MODE_MSI;
 			break;
@@ -397,7 +355,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 			break;
 		}
 		dev_notice(&udev->pdev->dev, "PCI INTX mask not supported\n");
-		/* fall back to no IRQ */
+	/* fall back to no IRQ */
 	case RTE_INTR_MODE_NONE:
 		udev->mode = RTE_INTR_MODE_NONE;
 		udev->info.irq = 0;
-- 
2.7.4

Re: [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections

[Stephen Hemminger]

On Thu, 31 Aug 2017 23:46:59 +0200
Markus Theil <markus.theil@tu-ilmenau.de> wrote:

> This patch substitutes the custom MSI/MSI-X mask code and uses
> already existing kernel APIs. Feedback/small corrections to the previous
> patch of this series are also incorporated.
> 
> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> ---
>  lib/librte_eal/linuxapp/igb_uio/compat.h  | 26 +++------
>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 88 ++++++++-----------------------
>  2 files changed, 31 insertions(+), 83 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
> index 3825933..67a7ab3 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/compat.h
> +++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
> @@ -15,24 +15,6 @@
>  #define HAVE_PTE_MASK_PAGE_IOMAP
>  #endif
>  
> -#ifndef PCI_MSIX_ENTRY_SIZE
> -#define PCI_MSIX_ENTRY_SIZE             16
> -#define  PCI_MSIX_ENTRY_LOWER_ADDR      0
> -#define  PCI_MSIX_ENTRY_UPPER_ADDR      4
> -#define  PCI_MSIX_ENTRY_DATA            8
> -#define  PCI_MSIX_ENTRY_VECTOR_CTRL     12
> -#define   PCI_MSIX_ENTRY_CTRL_MASKBIT   1
> -#endif
> -
> -/*
> - * for kernels < 2.6.38 and backported patch that moves MSI-X entry definition
> - * to pci_regs.h Those kernels has PCI_MSIX_ENTRY_SIZE defined but not
> - * PCI_MSIX_ENTRY_CTRL_MASKBIT
> - */
> -#ifndef PCI_MSIX_ENTRY_CTRL_MASKBIT
> -#define PCI_MSIX_ENTRY_CTRL_MASKBIT    1
> -#endif
> -
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 34) && \
>  	(!(defined(RHEL_RELEASE_CODE) && \
>  	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(5, 9)))
> @@ -127,3 +109,11 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
>  #define HAVE_ALLOC_IRQ_VECTORS 1
>  #endif
> +
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 19, 0)
> +#define HAVE_PCI_MSI_MASK_IRQ 1
> +#endif
> +
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 37)
> +#define HAVE_IRQ_DATA 1
> +#endif
> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> index 99a085a..5830453 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> @@ -92,51 +92,6 @@ static const struct attribute_group dev_attr_grp = {
>  	.attrs = dev_attrs,
>  };
>  
> -/*
> - * It masks the msix on/off of generating MSI-X messages.
> - */
> -static void
> -igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state)
> -{
> -	u32 mask_bits = desc->masked;
> -	unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
> -						PCI_MSIX_ENTRY_VECTOR_CTRL;
> -
> -	if (state != 0)
> -		mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
> -	else
> -		mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
> -
> -	if (mask_bits != desc->masked) {
> -		writel(mask_bits, desc->mask_base + offset);
> -		readl(desc->mask_base);
> -		desc->masked = mask_bits;
> -	}
> -}
> -
> -/*
> - * It masks the msi on/off of generating MSI messages.
> - */
> -static void
> -igbuio_msi_mask_irq(struct pci_dev *pdev, struct msi_desc *desc, int32_t state)
> -{
> -	u32 mask_bits = desc->masked;
> -	u32 offset = desc->irq - pdev->irq;
> -	u32 mask = 1 << offset;
> -	u32 flag = !!state << offset;
> -
> -	if (!desc->msi_attrib.maskbit)
> -		return;
> -
> -	mask_bits &= ~mask;
> -	mask_bits |= flag;
> -
> -	if (mask_bits != desc->masked) {
> -		pci_write_config_dword(pdev, desc->mask_pos, mask_bits);
> -		desc->masked = mask_bits;
> -	}
> -}
> -
>  /**
>   * This is the irqcontrol callback to be registered to uio_info.
>   * It can be used to disable/enable interrupt from user space processes.
> @@ -156,31 +111,31 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
>  	struct rte_uio_pci_dev *udev = info->priv;
>  	struct pci_dev *pdev = udev->pdev;
>  
> -	pci_cfg_access_lock(pdev);
> -	if (udev->mode == RTE_INTR_MODE_LEGACY)
> -		pci_intx(pdev, !!irq_state);
> -
> -	else if (udev->mode == RTE_INTR_MODE_MSIX) {
> -		struct msi_desc *desc;
> -
> -#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
> -		list_for_each_entry(desc, &pdev->msi_list, list)
> -			igbuio_msix_mask_irq(desc, irq_state);
> +#ifdef HAVE_IRQ_DATA
> +	struct irq_data *irq = irq_get_irq_data(udev->info.irq);
>  #else
> -		list_for_each_entry(desc, &pdev->dev.msi_list, list)
> -			igbuio_msix_mask_irq(desc, irq_state);
> +	unsigned int irq = udev->info.irq;
>  #endif
> -	} else if (udev->mode == RTE_INTR_MODE_MSI) {
> -	struct msi_desc *desc;
>  
> -#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
> -	list_for_each_entry(desc, &pdev->msi_list, list)
> -		igbuio_msi_mask_irq(pdev, desc, irq_state);
> +	pci_cfg_access_lock(pdev);
> +
> +	if (udev->mode == RTE_INTR_MODE_MSIX || udev->mode == RTE_INTR_MODE_MSI) {
> +#ifdef HAVE_PCI_MSI_MASK_IRQ
> +		if (irq_state == 1)
> +			pci_msi_unmask_irq(irq);
> +		else
> +			pci_msi_mask_irq(irq);
>  #else
> -	list_for_each_entry(desc, &pdev->dev.msi_list, list)
> -		igbuio_msi_mask_irq(pdev, desc, irq_state);
> +		if (irq_state == 1)
> +			unmask_msi_irq(irq);
> +		else
> +			mask_msi_irq(irq);
>  #endif
>  	}
> +
> +	if (udev->mode == RTE_INTR_MODE_LEGACY)
> +		pci_intx(pdev, !!irq_state);
> +
>  	pci_cfg_access_unlock(pdev);
>  
>  	return 0;
> @@ -365,11 +320,13 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
>  #else
>  		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
>  			dev_dbg(&udev->pdev->dev, "using MSI-X");
> +			udev->info.irq_flags = IRQF_NO_THREAD;
>  			udev->info.irq = pci_irq_vector(udev->pdev, 0);
>  			udev->mode = RTE_INTR_MODE_MSIX;
>  			break;
>  		}
>  #endif
> +	/* fall back to MSI */
>  	case RTE_INTR_MODE_MSI:
>  #ifndef HAVE_ALLOC_IRQ_VECTORS
>  		if (pci_enable_msi(udev->pdev) == 0) {
> @@ -382,6 +339,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
>  #else
>  		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSI) == 1) {
>  			dev_dbg(&udev->pdev->dev, "using MSI");
> +			udev->info.irq_flags = IRQF_NO_THREAD;
>  			udev->info.irq = pci_irq_vector(udev->pdev, 0);
>  			udev->mode = RTE_INTR_MODE_MSI;
>  			break;
> @@ -397,7 +355,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
>  			break;
>  		}
>  		dev_notice(&udev->pdev->dev, "PCI INTX mask not supported\n");
> -		/* fall back to no IRQ */
> +	/* fall back to no IRQ */
>  	case RTE_INTR_MODE_NONE:
>  		udev->mode = RTE_INTR_MODE_NONE;
>  		udev->info.irq = 0;


Please merge patches 3 and 4. (use  git rebase -i )

Re: [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections

[Burakov, Anatoly]

Hi Markus,

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Markus Theil
> Sent: Thursday, August 31, 2017 10:47 PM
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stephen@networkplumber.org;
> Markus Theil <markus.theil@tu-ilmenau.de>
> Subject: [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from
> kernel, little corrections
> 
> This patch substitutes the custom MSI/MSI-X mask code and uses already
> existing kernel APIs. Feedback/small corrections to the previous patch of this
> series are also incorporated.
> 
> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> ---
>  lib/librte_eal/linuxapp/igb_uio/compat.h  | 26 +++------
> lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 88 ++++++++-----------------------
>  2 files changed, 31 insertions(+), 83 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h
> b/lib/librte_eal/linuxapp/igb_uio/compat.h
> index 3825933..67a7ab3 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/compat.h
> +++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
> @@ -15,24 +15,6 @@
>  #define HAVE_PTE_MASK_PAGE_IOMAP
>  #endif
> 
> -#ifndef PCI_MSIX_ENTRY_SIZE
> -#define PCI_MSIX_ENTRY_SIZE             16
> -#define  PCI_MSIX_ENTRY_LOWER_ADDR      0
> -#define  PCI_MSIX_ENTRY_UPPER_ADDR      4
> -#define  PCI_MSIX_ENTRY_DATA            8
> -#define  PCI_MSIX_ENTRY_VECTOR_CTRL     12
> -#define   PCI_MSIX_ENTRY_CTRL_MASKBIT   1
> -#endif
> -
> -/*
> - * for kernels < 2.6.38 and backported patch that moves MSI-X entry
> definition
> - * to pci_regs.h Those kernels has PCI_MSIX_ENTRY_SIZE defined but not
> - * PCI_MSIX_ENTRY_CTRL_MASKBIT
> - */
> -#ifndef PCI_MSIX_ENTRY_CTRL_MASKBIT
> -#define PCI_MSIX_ENTRY_CTRL_MASKBIT    1
> -#endif
> -
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 34) && \
>  	(!(defined(RHEL_RELEASE_CODE) && \
>  	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(5, 9))) @@ -127,3
> +109,11 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)  #if
> LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)  #define
> HAVE_ALLOC_IRQ_VECTORS 1  #endif
> +
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 19, 0) #define
> +HAVE_PCI_MSI_MASK_IRQ 1 #endif
> +
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 37) #define
> +HAVE_IRQ_DATA 1 #endif
> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> index 99a085a..5830453 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> @@ -92,51 +92,6 @@ static const struct attribute_group dev_attr_grp = {
>  	.attrs = dev_attrs,
>  };
> 
> -/*
> - * It masks the msix on/off of generating MSI-X messages.
> - */
> -static void
> -igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state) -{
> -	u32 mask_bits = desc->masked;
> -	unsigned offset = desc->msi_attrib.entry_nr *
> PCI_MSIX_ENTRY_SIZE +
> -
> 	PCI_MSIX_ENTRY_VECTOR_CTRL;
> -
> -	if (state != 0)
> -		mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
> -	else
> -		mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
> -
> -	if (mask_bits != desc->masked) {
> -		writel(mask_bits, desc->mask_base + offset);
> -		readl(desc->mask_base);
> -		desc->masked = mask_bits;
> -	}
> -}
> -
> -/*
> - * It masks the msi on/off of generating MSI messages.
> - */
> -static void
> -igbuio_msi_mask_irq(struct pci_dev *pdev, struct msi_desc *desc, int32_t
> state) -{
> -	u32 mask_bits = desc->masked;
> -	u32 offset = desc->irq - pdev->irq;
> -	u32 mask = 1 << offset;
> -	u32 flag = !!state << offset;
> -
> -	if (!desc->msi_attrib.maskbit)
> -		return;
> -
> -	mask_bits &= ~mask;
> -	mask_bits |= flag;
> -
> -	if (mask_bits != desc->masked) {
> -		pci_write_config_dword(pdev, desc->mask_pos, mask_bits);
> -		desc->masked = mask_bits;
> -	}
> -}
> -
>  /**
>   * This is the irqcontrol callback to be registered to uio_info.
>   * It can be used to disable/enable interrupt from user space processes.
> @@ -156,31 +111,31 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32
> irq_state)
>  	struct rte_uio_pci_dev *udev = info->priv;
>  	struct pci_dev *pdev = udev->pdev;
> 
> -	pci_cfg_access_lock(pdev);
> -	if (udev->mode == RTE_INTR_MODE_LEGACY)
> -		pci_intx(pdev, !!irq_state);
> -
> -	else if (udev->mode == RTE_INTR_MODE_MSIX) {
> -		struct msi_desc *desc;
> -
> -#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
> -		list_for_each_entry(desc, &pdev->msi_list, list)
> -			igbuio_msix_mask_irq(desc, irq_state);
> +#ifdef HAVE_IRQ_DATA
> +	struct irq_data *irq = irq_get_irq_data(udev->info.irq);
>  #else
> -		list_for_each_entry(desc, &pdev->dev.msi_list, list)
> -			igbuio_msix_mask_irq(desc, irq_state);
> +	unsigned int irq = udev->info.irq;
>  #endif
> -	} else if (udev->mode == RTE_INTR_MODE_MSI) {
> -	struct msi_desc *desc;
> 
> -#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
> -	list_for_each_entry(desc, &pdev->msi_list, list)
> -		igbuio_msi_mask_irq(pdev, desc, irq_state);
> +	pci_cfg_access_lock(pdev);
> +
> +	if (udev->mode == RTE_INTR_MODE_MSIX || udev->mode ==
> +RTE_INTR_MODE_MSI) { #ifdef HAVE_PCI_MSI_MASK_IRQ
> +		if (irq_state == 1)
> +			pci_msi_unmask_irq(irq);
> +		else
> +			pci_msi_mask_irq(irq);
>  #else
> -	list_for_each_entry(desc, &pdev->dev.msi_list, list)
> -		igbuio_msi_mask_irq(pdev, desc, irq_state);
> +		if (irq_state == 1)
> +			unmask_msi_irq(irq);
> +		else
> +			mask_msi_irq(irq);
>  #endif
>  	}
> +
> +	if (udev->mode == RTE_INTR_MODE_LEGACY)
> +		pci_intx(pdev, !!irq_state);
> +
>  	pci_cfg_access_unlock(pdev);
> 
>  	return 0;
> @@ -365,11 +320,13 @@ igbuio_pci_enable_interrupts(struct
> rte_uio_pci_dev *udev)  #else
>  		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) ==
> 1) {
>  			dev_dbg(&udev->pdev->dev, "using MSI-X");
> +			udev->info.irq_flags = IRQF_NO_THREAD;

This (and similar below) looks like unrelated changes. Should probably be in a separate patch, with their own justifications.

Thanks,
Anatoly

>  			udev->info.irq = pci_irq_vector(udev->pdev, 0);
>  			udev->mode = RTE_INTR_MODE_MSIX;
>  			break;
>  		}
>  #endif
> +	/* fall back to MSI */
>  	case RTE_INTR_MODE_MSI:
>  #ifndef HAVE_ALLOC_IRQ_VECTORS
>  		if (pci_enable_msi(udev->pdev) == 0) { @@ -382,6 +339,7
> @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)  #else
>  		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSI) ==
> 1) {
>  			dev_dbg(&udev->pdev->dev, "using MSI");
> +			udev->info.irq_flags = IRQF_NO_THREAD;
>  			udev->info.irq = pci_irq_vector(udev->pdev, 0);
>  			udev->mode = RTE_INTR_MODE_MSI;
>  			break;
> @@ -397,7 +355,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev
> *udev)
>  			break;
>  		}
>  		dev_notice(&udev->pdev->dev, "PCI INTX mask not
> supported\n");
> -		/* fall back to no IRQ */
> +	/* fall back to no IRQ */
>  	case RTE_INTR_MODE_NONE:
>  		udev->mode = RTE_INTR_MODE_NONE;
>  		udev->info.irq = 0;
> --
> 2.7.4

Re: [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections

[Ferruh Yigit]

On 8/31/2017 10:46 PM, Markus Theil wrote:
> This patch substitutes the custom MSI/MSI-X mask code and uses
> already existing kernel APIs. Feedback/small corrections to the previous
> patch of this series are also incorporated.
> 
> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>

<...>

> +	if (udev->mode == RTE_INTR_MODE_MSIX || udev->mode == RTE_INTR_MODE_MSI) {
> +#ifdef HAVE_PCI_MSI_MASK_IRQ
> +		if (irq_state == 1)
> +			pci_msi_unmask_irq(irq);
> +		else
> +			pci_msi_mask_irq(irq);

It is an option to keep using mask_msi_irq() without #ifdef, it seems in
newer version of the kernel mask_msi_irq() is already wrapper to the
pci_msi_mask_irq().

Although this may make the code simpler, it can break it if
mask_msi_irq() wrapper removed.

I would go with using mask_msi_irq() directly, but no strong opinion...

<...>

Re: [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections

[Markus Theil]

On 04.09.2017 14:43, Ferruh Yigit wrote:
> On 8/31/2017 10:46 PM, Markus Theil wrote:
>> This patch substitutes the custom MSI/MSI-X mask code and uses
>> already existing kernel APIs. Feedback/small corrections to the previous
>> patch of this series are also incorporated.
>>
>> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> <...>
>
>> +	if (udev->mode == RTE_INTR_MODE_MSIX || udev->mode == RTE_INTR_MODE_MSI) {
>> +#ifdef HAVE_PCI_MSI_MASK_IRQ
>> +		if (irq_state == 1)
>> +			pci_msi_unmask_irq(irq);
>> +		else
>> +			pci_msi_mask_irq(irq);
> It is an option to keep using mask_msi_irq() without #ifdef, it seems in
> newer version of the kernel mask_msi_irq() is already wrapper to the
> pci_msi_mask_irq().
>
> Although this may make the code simpler, it can break it if
> mask_msi_irq() wrapper removed.
>
> I would go with using mask_msi_irq() directly, but no strong opinion...
>
> <...>
I don't know what the comment "Conversion helpers. Should be removed
after merging" in msi.h really means. If the functions following the
comment are deleted in one of the next versions, I would not change
these lines. Otherwise I second your comment.

Re: [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections

[Ferruh Yigit]

On 9/4/2017 4:16 PM, Markus Theil wrote:
> On 04.09.2017 14:43, Ferruh Yigit wrote:
>> On 8/31/2017 10:46 PM, Markus Theil wrote:
>>> This patch substitutes the custom MSI/MSI-X mask code and uses
>>> already existing kernel APIs. Feedback/small corrections to the previous
>>> patch of this series are also incorporated.
>>>
>>> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
>> <...>
>>
>>> +	if (udev->mode == RTE_INTR_MODE_MSIX || udev->mode == RTE_INTR_MODE_MSI) {
>>> +#ifdef HAVE_PCI_MSI_MASK_IRQ
>>> +		if (irq_state == 1)
>>> +			pci_msi_unmask_irq(irq);
>>> +		else
>>> +			pci_msi_mask_irq(irq);
>> It is an option to keep using mask_msi_irq() without #ifdef, it seems in
>> newer version of the kernel mask_msi_irq() is already wrapper to the
>> pci_msi_mask_irq().
>>
>> Although this may make the code simpler, it can break it if
>> mask_msi_irq() wrapper removed.
>>
>> I would go with using mask_msi_irq() directly, but no strong opinion...
>>
>> <...>
> I don't know what the comment "Conversion helpers. Should be removed
> after merging" in msi.h really means. If the functions following the
> comment are deleted in one of the next versions, I would not change
> these lines. Otherwise I second your comment.

That comment seems there since version 3.19 :)
But it shows the intention to delete the wrappers, so lets stick with
your version.

Thanks,
ferruh

[dpdk-dev] [PATCH v5 5/5] igb_uio: release in exact reverse order

[Markus Theil]

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 5830453..b578c4a 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -493,7 +493,7 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	err = sysfs_create_group(&dev->dev.kobj, &dev_attr_grp);
 	if (err != 0)
-		goto fail_release_iomem;
+		goto fail_disable_interrupts;
 
 	/* register uio driver */
 	err = uio_register_device(&dev->dev, &udev->info);
@@ -530,9 +530,10 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 fail_remove_group:
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
+fail_disable_interrupts:
+	igbuio_pci_disable_interrupts(udev);
 fail_release_iomem:
 	igbuio_pci_release_iomem(&udev->info);
-	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 fail_free:
 	kfree(udev);
@@ -547,8 +548,8 @@ igbuio_pci_remove(struct pci_dev *dev)
 
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 	uio_unregister_device(&udev->info);
-	igbuio_pci_release_iomem(&udev->info);
 	igbuio_pci_disable_interrupts(udev);
+	igbuio_pci_release_iomem(&udev->info);
 	pci_disable_device(dev);
 	pci_set_drvdata(dev, NULL);
 	kfree(udev);
-- 
2.7.4

Re: [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored

[Stephen Hemminger]

On Mon, 21 Aug 2017 19:33:45 +0200
Markus Theil <markus.theil@tu-ilmenau.de> wrote:

> This patch adds MSI IRQ mode and in a way, that should
> also work on older kernel versions. The base for my patch
> was an attempt to do this in cf705bc36c which was later reverted in
> d8ee82745a. Compilation was tested on Linux 3.2, 4.10 and 4.12.
> 
> MSI(X) setup was already using pci_alloc_irq_vectors before,
> but calls to pci_free_irq_vectors were missing and added.
> 
> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>

I wonder if DPDK should only N-1 Long Term Stable kernel.org kernels?
That would mean 4.4.83 or later now, and 4.9 or later starting with 18.XX releases.

If enterprise distro's want to backport more, that is their prerogative but upstream
DPDK shouldn't have to worry about it. The current mess with KNI especially is out
of hand.

Re: [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored

[Markus Theil]

On 22.08.2017 18:55, Stephen Hemminger wrote:
> On Mon, 21 Aug 2017 19:33:45 +0200
> Markus Theil <markus.theil@tu-ilmenau.de> wrote:
>
>> This patch adds MSI IRQ mode and in a way, that should
>> also work on older kernel versions. The base for my patch
>> was an attempt to do this in cf705bc36c which was later reverted in
>> d8ee82745a. Compilation was tested on Linux 3.2, 4.10 and 4.12.
>>
>> MSI(X) setup was already using pci_alloc_irq_vectors before,
>> but calls to pci_free_irq_vectors were missing and added.
>>
>> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> I wonder if DPDK should only N-1 Long Term Stable kernel.org kernels?
> That would mean 4.4.83 or later now, and 4.9 or later starting with 18.XX releases.
>
> If enterprise distro's want to backport more, that is their prerogative but upstream
> DPDK shouldn't have to worry about it. The current mess with KNI especially is out
> of hand.
I don't rely on older kernels than N-1 LTS, but the former MSI patch was
reverted
because of such an issue. If there is consensus about this, adapting
igb_uio
to kernels >= 4.4 will be no problem. I'd then write a little patch
series with that aim.

Re: [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored

[Bruce Richardson]

On Tue, Aug 22, 2017 at 09:55:53AM -0700, Stephen Hemminger wrote:
> On Mon, 21 Aug 2017 19:33:45 +0200
> Markus Theil <markus.theil@tu-ilmenau.de> wrote:
> 
> > This patch adds MSI IRQ mode and in a way, that should
> > also work on older kernel versions. The base for my patch
> > was an attempt to do this in cf705bc36c which was later reverted in
> > d8ee82745a. Compilation was tested on Linux 3.2, 4.10 and 4.12.
> > 
> > MSI(X) setup was already using pci_alloc_irq_vectors before,
> > but calls to pci_free_irq_vectors were missing and added.
> > 
> > Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> 
> I wonder if DPDK should only N-1 Long Term Stable kernel.org kernels?
> That would mean 4.4.83 or later now, and 4.9 or later starting with 18.XX releases.
> 
> If enterprise distro's want to backport more, that is their prerogative but upstream
> DPDK shouldn't have to worry about it. The current mess with KNI especially is out
> of hand.

I agree in principal about limiting ourselves to only supporting a more
limited set of kernel versions in mainline. However, the exact number of
versions probably needs some discussion - my initial impression is that
what you propose is a little too limited.

Re: [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored

[Bruce Richardson]

On Wed, Aug 23, 2017 at 09:47:35AM +0100, Bruce Richardson wrote:
> On Tue, Aug 22, 2017 at 09:55:53AM -0700, Stephen Hemminger wrote:
> > On Mon, 21 Aug 2017 19:33:45 +0200
> > Markus Theil <markus.theil@tu-ilmenau.de> wrote:
> > 
> > > This patch adds MSI IRQ mode and in a way, that should
> > > also work on older kernel versions. The base for my patch
> > > was an attempt to do this in cf705bc36c which was later reverted in
> > > d8ee82745a. Compilation was tested on Linux 3.2, 4.10 and 4.12.
> > > 
> > > MSI(X) setup was already using pci_alloc_irq_vectors before,
> > > but calls to pci_free_irq_vectors were missing and added.
> > > 
> > > Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> > 
> > I wonder if DPDK should only N-1 Long Term Stable kernel.org kernels?
> > That would mean 4.4.83 or later now, and 4.9 or later starting with 18.XX releases.
> > 
> > If enterprise distro's want to backport more, that is their prerogative but upstream
> > DPDK shouldn't have to worry about it. The current mess with KNI especially is out
> > of hand.
> 
> I agree in principal about limiting ourselves to only supporting a more
> limited set of kernel versions in mainline. However, the exact number of
> versions probably needs some discussion - my initial impression is that
> what you propose is a little too limited.

Actually, having had a bit more caffeine, I need to correct myself. :-(

Since these are out of tree modules, it means that most distro's are not
including them in their packaging. This rules out the possibility of
them getting backported by those distros, leaving the maintainence of
them entirely to the DPDK community. Therefore, I don't think we can
limit ourselves to just LTS kernels, but also need to include kernels of
major distros we need to support. In short, pretty much where we are
today, I think. :-(

/Bruce

[dpdk-dev] [PATCH v6 1/5] igb_uio: refactor irq enable/disable into own functions

[Markus Theil]

Interrupt setup code in igb_uio has to deal with multiple
types of interrupts and kernel versions. This patch moves
the setup and teardown code into own functions, to make
it more readable.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 112 +++++++++++++++++-------------
 1 file changed, 64 insertions(+), 48 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 07a19a3..e2e9263 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -309,6 +309,66 @@ igbuio_pci_release_iomem(struct uio_info *info)
 }
 
 static int
+igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
+{
+	int err = 0;
+#ifdef HAVE_PCI_ENABLE_MSIX
+	struct msix_entry msix_entry;
+#endif
+
+	switch (igbuio_intr_mode_preferred) {
+	case RTE_INTR_MODE_MSIX:
+		/* Only 1 msi-x vector needed */
+#ifdef HAVE_PCI_ENABLE_MSIX
+		msix_entry.entry = 0;
+		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = msix_entry.vector;
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#endif
+	/* fall back to INTX */
+	case RTE_INTR_MODE_LEGACY:
+		if (pci_intx_mask_supported(udev->pdev)) {
+			dev_dbg(&udev->pdev->dev, "using INTX");
+			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_LEGACY;
+			break;
+		}
+		dev_notice(&udev->pdev->dev, "PCI INTX mask not supported\n");
+		/* fall back to no IRQ */
+	case RTE_INTR_MODE_NONE:
+		udev->mode = RTE_INTR_MODE_NONE;
+		udev->info.irq = 0;
+		break;
+
+	default:
+		dev_err(&udev->pdev->dev, "invalid IRQ mode %u",
+			igbuio_intr_mode_preferred);
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static void
+igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
+{
+	if (udev->mode == RTE_INTR_MODE_MSIX)
+		pci_disable_msix(udev->pdev);
+}
+
+static int
 igbuio_setup_bars(struct pci_dev *dev, struct uio_info *info)
 {
 	int i, iom, iop, ret;
@@ -356,9 +416,6 @@ static int
 igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	struct rte_uio_pci_dev *udev;
-#ifdef HAVE_PCI_ENABLE_MSIX
-	struct msix_entry msix_entry;
-#endif
 	dma_addr_t map_dma_addr;
 	void *map_addr;
 	int err;
@@ -413,48 +470,9 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	udev->info.priv = udev;
 	udev->pdev = dev;
 
-	switch (igbuio_intr_mode_preferred) {
-	case RTE_INTR_MODE_MSIX:
-		/* Only 1 msi-x vector needed */
-#ifdef HAVE_PCI_ENABLE_MSIX
-		msix_entry.entry = 0;
-		if (pci_enable_msix(dev, &msix_entry, 1) == 0) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq_flags = IRQF_NO_THREAD;
-			udev->info.irq = msix_entry.vector;
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#else
-		if (pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_MSIX) == 1) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq = pci_irq_vector(dev, 0);
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#endif
-		/* fall back to INTX */
-	case RTE_INTR_MODE_LEGACY:
-		if (pci_intx_mask_supported(dev)) {
-			dev_dbg(&dev->dev, "using INTX");
-			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
-			udev->info.irq = dev->irq;
-			udev->mode = RTE_INTR_MODE_LEGACY;
-			break;
-		}
-		dev_notice(&dev->dev, "PCI INTX mask not supported\n");
-		/* fall back to no IRQ */
-	case RTE_INTR_MODE_NONE:
-		udev->mode = RTE_INTR_MODE_NONE;
-		udev->info.irq = 0;
-		break;
-
-	default:
-		dev_err(&dev->dev, "invalid IRQ mode %u",
-			igbuio_intr_mode_preferred);
-		err = -EINVAL;
+	err = igbuio_pci_enable_interrupts(udev);
+	if (err != 0)
 		goto fail_release_iomem;
-	}
 
 	err = sysfs_create_group(&dev->dev.kobj, &dev_attr_grp);
 	if (err != 0)
@@ -497,8 +515,7 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 fail_release_iomem:
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(udev->pdev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 fail_free:
 	kfree(udev);
@@ -514,8 +531,7 @@ igbuio_pci_remove(struct pci_dev *dev)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 	uio_unregister_device(&udev->info);
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(dev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 	pci_set_drvdata(dev, NULL);
 	kfree(udev);
-- 
2.7.4

[dpdk-dev] [PATCH v6 2/5] igb_uio: fix irq disable on recent kernels

[Markus Theil]

igb_uio already allocates irqs using pci_alloc_irq_vectors on
recent kernels >= 4.8. The interrupt disable code was not
using the corresponding pci_free_irq_vectors, but the also
deprecated pci_disable_msix, before this fix.

Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for MSI-X")
Cc: nicolas.dichtel@6wind.com

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/compat.h  |  4 ++--
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 11 ++++++++---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
index b800a53..3825933 100644
--- a/lib/librte_eal/linuxapp/igb_uio/compat.h
+++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
@@ -124,6 +124,6 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
 
 #endif /* < 3.3.0 */
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 8, 0)
-#define HAVE_PCI_ENABLE_MSIX
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
+#define HAVE_ALLOC_IRQ_VECTORS 1
 #endif
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index e2e9263..9bb74b2 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -312,18 +312,17 @@ static int
 igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 {
 	int err = 0;
-#ifdef HAVE_PCI_ENABLE_MSIX
+#ifndef HAVE_ALLOC_IRQ_VECTORS
 	struct msix_entry msix_entry;
 #endif
 
 	switch (igbuio_intr_mode_preferred) {
 	case RTE_INTR_MODE_MSIX:
 		/* Only 1 msi-x vector needed */
-#ifdef HAVE_PCI_ENABLE_MSIX
+#ifndef HAVE_ALLOC_IRQ_VECTORS
 		msix_entry.entry = 0;
 		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
 			dev_dbg(&udev->pdev->dev, "using MSI-X");
-			udev->info.irq_flags = IRQF_NO_THREAD;
 			udev->info.irq = msix_entry.vector;
 			udev->mode = RTE_INTR_MODE_MSIX;
 			break;
@@ -331,6 +330,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 #else
 		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
 			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq_flags = IRQF_NO_THREAD;
 			udev->info.irq = pci_irq_vector(udev->pdev, 0);
 			udev->mode = RTE_INTR_MODE_MSIX;
 			break;
@@ -364,8 +364,13 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 static void
 igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
 {
+#ifndef HAVE_ALLOC_IRQ_VECTORS
 	if (udev->mode == RTE_INTR_MODE_MSIX)
 		pci_disable_msix(udev->pdev);
+#else
+	if (udev->mode == RTE_INTR_MODE_MSIX)
+		pci_free_irq_vectors(udev->pdev);
+#endif
 }
 
 static int
-- 
2.7.4

Re: [dpdk-dev] [PATCH v6 2/5] igb_uio: fix irq disable on recent kernels

[Ferruh Yigit]

On 9/4/2017 7:17 PM, Markus Theil wrote:
> igb_uio already allocates irqs using pci_alloc_irq_vectors on
> recent kernels >= 4.8. The interrupt disable code was not
> using the corresponding pci_free_irq_vectors, but the also
> deprecated pci_disable_msix, before this fix.
> 
> Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for MSI-X")
> Cc: nicolas.dichtel@6wind.com
> 
> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>

<...>

> -#ifdef HAVE_PCI_ENABLE_MSIX
> +#ifndef HAVE_ALLOC_IRQ_VECTORS
>  		msix_entry.entry = 0;
>  		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
>  			dev_dbg(&udev->pdev->dev, "using MSI-X");
> -			udev->info.irq_flags = IRQF_NO_THREAD;

This and below one [1] seems merge artifact, needs to be removed from
this patch.

>  			udev->info.irq = msix_entry.vector;
>  			udev->mode = RTE_INTR_MODE_MSIX;
>  			break;
> @@ -331,6 +330,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
>  #else
>  		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
>  			dev_dbg(&udev->pdev->dev, "using MSI-X");
> +			udev->info.irq_flags = IRQF_NO_THREAD;

[1]

>  			udev->info.irq = pci_irq_vector(udev->pdev, 0);
>  			udev->mode = RTE_INTR_MODE_MSIX;
>  			break;
<...>

[dpdk-dev] [PATCH v6 3/5] igb_uio: fix MSI-X IRQ assignment with new IRQ function

[Markus Theil]

The patch which introduced the usage of pci_alloc_irq_vectors
came after the patch which switched to non-threaded ISR (f0d1896fa1),
but did not use non-threaded ISR, if pci_alloc_irq_vectors
is used.

Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for
MSI-X")
Cc: nicolas.dichtel@6wind.com

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 9bb74b2..6885e72 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -323,6 +323,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 		msix_entry.entry = 0;
 		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
 			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq_flags = IRQF_NO_THREAD;
 			udev->info.irq = msix_entry.vector;
 			udev->mode = RTE_INTR_MODE_MSIX;
 			break;
-- 
2.7.4

Re: [dpdk-dev] [PATCH v6 3/5] igb_uio: fix MSI-X IRQ assignment with new IRQ function

[Ferruh Yigit]

On 9/4/2017 7:17 PM, Markus Theil wrote:
> The patch which introduced the usage of pci_alloc_irq_vectors
> came after the patch which switched to non-threaded ISR (f0d1896fa1),
> but did not use non-threaded ISR, if pci_alloc_irq_vectors
> is used.
> 
> Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for
> MSI-X")
> Cc: nicolas.dichtel@6wind.com
> 
> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> ---
>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> index 9bb74b2..6885e72 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> @@ -323,6 +323,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
>  		msix_entry.entry = 0;
>  		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
>  			dev_dbg(&udev->pdev->dev, "using MSI-X");
> +			udev->info.irq_flags = IRQF_NO_THREAD;

This should be in #else part (pci_alloc_irq_vectors() call one), I guess
when issue in patch 2/5 fixed, this patch also will be correct.

>  			udev->info.irq = msix_entry.vector;
>  			udev->mode = RTE_INTR_MODE_MSIX;
>  			break;
> 

[dpdk-dev] [PATCH v6 4/5] igb_uio: release in exact reverse order

[Markus Theil]

For better readability throughout the module, the destruction
order is changed to the exact inverse construction order.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 6885e72..c570eed 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -482,7 +482,7 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	err = sysfs_create_group(&dev->dev.kobj, &dev_attr_grp);
 	if (err != 0)
-		goto fail_release_iomem;
+		goto fail_disable_interrupts;
 
 	/* register uio driver */
 	err = uio_register_device(&dev->dev, &udev->info);
@@ -519,9 +519,10 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 fail_remove_group:
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
+fail_disable_interrupts:
+	igbuio_pci_disable_interrupts(udev);
 fail_release_iomem:
 	igbuio_pci_release_iomem(&udev->info);
-	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 fail_free:
 	kfree(udev);
@@ -536,8 +537,8 @@ igbuio_pci_remove(struct pci_dev *dev)
 
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 	uio_unregister_device(&udev->info);
-	igbuio_pci_release_iomem(&udev->info);
 	igbuio_pci_disable_interrupts(udev);
+	igbuio_pci_release_iomem(&udev->info);
 	pci_disable_device(dev);
 	pci_set_drvdata(dev, NULL);
 	kfree(udev);
-- 
2.7.4

[dpdk-dev] [PATCH v6 5/5] igb_uio: MSI IRQ mode, use kernel functions for masking MSI and MSI-X

[Markus Theil]

This patch adds MSI IRQ mode and in a way, that should
also work on older kernel versions. The base for my patch
was an attempt to do this in cf705bc36c which was later reverted in
d8ee82745a. Compilation was tested on Linux 3.2, 4.10 and 4.12.

Furthermore the custom MSI-X mask code is ommited and already existing
kernel APIs are used for MSI and MSI-X. Feedback/small corrections
to the previous patch of this series are also incorporated.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/compat.h  | 26 +++-------
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 81 ++++++++++++++++++-------------
 2 files changed, 56 insertions(+), 51 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
index 3825933..67a7ab3 100644
--- a/lib/librte_eal/linuxapp/igb_uio/compat.h
+++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
@@ -15,24 +15,6 @@
 #define HAVE_PTE_MASK_PAGE_IOMAP
 #endif
 
-#ifndef PCI_MSIX_ENTRY_SIZE
-#define PCI_MSIX_ENTRY_SIZE             16
-#define  PCI_MSIX_ENTRY_LOWER_ADDR      0
-#define  PCI_MSIX_ENTRY_UPPER_ADDR      4
-#define  PCI_MSIX_ENTRY_DATA            8
-#define  PCI_MSIX_ENTRY_VECTOR_CTRL     12
-#define   PCI_MSIX_ENTRY_CTRL_MASKBIT   1
-#endif
-
-/*
- * for kernels < 2.6.38 and backported patch that moves MSI-X entry definition
- * to pci_regs.h Those kernels has PCI_MSIX_ENTRY_SIZE defined but not
- * PCI_MSIX_ENTRY_CTRL_MASKBIT
- */
-#ifndef PCI_MSIX_ENTRY_CTRL_MASKBIT
-#define PCI_MSIX_ENTRY_CTRL_MASKBIT    1
-#endif
-
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 34) && \
 	(!(defined(RHEL_RELEASE_CODE) && \
 	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(5, 9)))
@@ -127,3 +109,11 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
 #define HAVE_ALLOC_IRQ_VECTORS 1
 #endif
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 19, 0)
+#define HAVE_PCI_MSI_MASK_IRQ 1
+#endif
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 37)
+#define HAVE_IRQ_DATA 1
+#endif
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index c570eed..b578c4a 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -91,27 +91,6 @@ static struct attribute *dev_attrs[] = {
 static const struct attribute_group dev_attr_grp = {
 	.attrs = dev_attrs,
 };
-/*
- * It masks the msix on/off of generating MSI-X messages.
- */
-static void
-igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state)
-{
-	u32 mask_bits = desc->masked;
-	unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
-						PCI_MSIX_ENTRY_VECTOR_CTRL;
-
-	if (state != 0)
-		mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
-	else
-		mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
-
-	if (mask_bits != desc->masked) {
-		writel(mask_bits, desc->mask_base + offset);
-		readl(desc->mask_base);
-		desc->masked = mask_bits;
-	}
-}
 
 /**
  * This is the irqcontrol callback to be registered to uio_info.
@@ -132,21 +111,31 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
 	struct rte_uio_pci_dev *udev = info->priv;
 	struct pci_dev *pdev = udev->pdev;
 
-	pci_cfg_access_lock(pdev);
-	if (udev->mode == RTE_INTR_MODE_LEGACY)
-		pci_intx(pdev, !!irq_state);
+#ifdef HAVE_IRQ_DATA
+	struct irq_data *irq = irq_get_irq_data(udev->info.irq);
+#else
+	unsigned int irq = udev->info.irq;
+#endif
 
-	else if (udev->mode == RTE_INTR_MODE_MSIX) {
-		struct msi_desc *desc;
+	pci_cfg_access_lock(pdev);
 
-#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
-		list_for_each_entry(desc, &pdev->msi_list, list)
-			igbuio_msix_mask_irq(desc, irq_state);
+	if (udev->mode == RTE_INTR_MODE_MSIX || udev->mode == RTE_INTR_MODE_MSI) {
+#ifdef HAVE_PCI_MSI_MASK_IRQ
+		if (irq_state == 1)
+			pci_msi_unmask_irq(irq);
+		else
+			pci_msi_mask_irq(irq);
 #else
-		list_for_each_entry(desc, &pdev->dev.msi_list, list)
-			igbuio_msix_mask_irq(desc, irq_state);
+		if (irq_state == 1)
+			unmask_msi_irq(irq);
+		else
+			mask_msi_irq(irq);
 #endif
 	}
+
+	if (udev->mode == RTE_INTR_MODE_LEGACY)
+		pci_intx(pdev, !!irq_state);
+
 	pci_cfg_access_unlock(pdev);
 
 	return 0;
@@ -337,6 +326,25 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 			break;
 		}
 #endif
+	/* fall back to MSI */
+	case RTE_INTR_MODE_MSI:
+#ifndef HAVE_ALLOC_IRQ_VECTORS
+		if (pci_enable_msi(udev->pdev) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSI) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#endif
 	/* fall back to INTX */
 	case RTE_INTR_MODE_LEGACY:
 		if (pci_intx_mask_supported(udev->pdev)) {
@@ -347,7 +355,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 			break;
 		}
 		dev_notice(&udev->pdev->dev, "PCI INTX mask not supported\n");
-		/* fall back to no IRQ */
+	/* fall back to no IRQ */
 	case RTE_INTR_MODE_NONE:
 		udev->mode = RTE_INTR_MODE_NONE;
 		udev->info.irq = 0;
@@ -368,8 +376,11 @@ igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
 #ifndef HAVE_ALLOC_IRQ_VECTORS
 	if (udev->mode == RTE_INTR_MODE_MSIX)
 		pci_disable_msix(udev->pdev);
+	if (udev->mode == RTE_INTR_MODE_MSI)
+		pci_disable_msi(udev->pdev);
 #else
-	if (udev->mode == RTE_INTR_MODE_MSIX)
+	if (udev->mode == RTE_INTR_MODE_MSIX ||
+	    udev->mode == RTE_INTR_MODE_MSI)
 		pci_free_irq_vectors(udev->pdev);
 #endif
 }
@@ -555,6 +566,9 @@ igbuio_config_intr_mode(char *intr_str)
 	if (!strcmp(intr_str, RTE_INTR_MODE_MSIX_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
 		pr_info("Use MSIX interrupt\n");
+	} else if (!strcmp(intr_str, RTE_INTR_MODE_MSI_NAME)) {
+		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSI;
+		pr_info("Use MSI interrupt\n");
 	} else if (!strcmp(intr_str, RTE_INTR_MODE_LEGACY_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_LEGACY;
 		pr_info("Use legacy interrupt\n");
@@ -598,6 +612,7 @@ module_param(intr_mode, charp, S_IRUGO);
 MODULE_PARM_DESC(intr_mode,
 "igb_uio interrupt mode (default=msix):\n"
 "    " RTE_INTR_MODE_MSIX_NAME "       Use MSIX interrupt\n"
+"    " RTE_INTR_MODE_MSI_NAME "        Use MSI interrupt\n"
 "    " RTE_INTR_MODE_LEGACY_NAME "     Use Legacy interrupt\n"
 "\n");
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH v6 5/5] igb_uio: MSI IRQ mode, use kernel functions for masking MSI and MSI-X

[Ferruh Yigit]

On 9/4/2017 7:17 PM, Markus Theil wrote:
> This patch adds MSI IRQ mode and in a way, that should
> also work on older kernel versions. The base for my patch
> was an attempt to do this in cf705bc36c which was later reverted in
> d8ee82745a. Compilation was tested on Linux 3.2, 4.10 and 4.12.
> 
> Furthermore the custom MSI-X mask code is ommited and already existing
> kernel APIs are used for MSI and MSI-X. Feedback/small corrections
> to the previous patch of this series are also incorporated.
> 
> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> ---
>  lib/librte_eal/linuxapp/igb_uio/compat.h  | 26 +++-------
>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 81 ++++++++++++++++++-------------
>  2 files changed, 56 insertions(+), 51 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
> index 3825933..67a7ab3 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/compat.h
> +++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
> @@ -15,24 +15,6 @@
>  #define HAVE_PTE_MASK_PAGE_IOMAP
>  #endif
>  
> -#ifndef PCI_MSIX_ENTRY_SIZE
> -#define PCI_MSIX_ENTRY_SIZE             16
> -#define  PCI_MSIX_ENTRY_LOWER_ADDR      0
> -#define  PCI_MSIX_ENTRY_UPPER_ADDR      4
> -#define  PCI_MSIX_ENTRY_DATA            8
> -#define  PCI_MSIX_ENTRY_VECTOR_CTRL     12
> -#define   PCI_MSIX_ENTRY_CTRL_MASKBIT   1
> -#endif
> -
> -/*
> - * for kernels < 2.6.38 and backported patch that moves MSI-X entry definition
> - * to pci_regs.h Those kernels has PCI_MSIX_ENTRY_SIZE defined but not
> - * PCI_MSIX_ENTRY_CTRL_MASKBIT
> - */
> -#ifndef PCI_MSIX_ENTRY_CTRL_MASKBIT
> -#define PCI_MSIX_ENTRY_CTRL_MASKBIT    1
> -#endif
> -
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 34) && \
>  	(!(defined(RHEL_RELEASE_CODE) && \
>  	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(5, 9)))
> @@ -127,3 +109,11 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
>  #define HAVE_ALLOC_IRQ_VECTORS 1
>  #endif
> +
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 19, 0)
> +#define HAVE_PCI_MSI_MASK_IRQ 1
> +#endif
> +
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 37)
> +#define HAVE_IRQ_DATA 1
> +#endif
> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> index c570eed..b578c4a 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> @@ -91,27 +91,6 @@ static struct attribute *dev_attrs[] = {
>  static const struct attribute_group dev_attr_grp = {
>  	.attrs = dev_attrs,
>  };
> -/*
> - * It masks the msix on/off of generating MSI-X messages.
> - */
> -static void
> -igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state)
> -{
> -	u32 mask_bits = desc->masked;
> -	unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
> -						PCI_MSIX_ENTRY_VECTOR_CTRL;
> -
> -	if (state != 0)
> -		mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
> -	else
> -		mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
> -
> -	if (mask_bits != desc->masked) {
> -		writel(mask_bits, desc->mask_base + offset);
> -		readl(desc->mask_base);
> -		desc->masked = mask_bits;
> -	}
> -}
>  
>  /**
>   * This is the irqcontrol callback to be registered to uio_info.
> @@ -132,21 +111,31 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
>  	struct rte_uio_pci_dev *udev = info->priv;
>  	struct pci_dev *pdev = udev->pdev;
>  
> -	pci_cfg_access_lock(pdev);
> -	if (udev->mode == RTE_INTR_MODE_LEGACY)
> -		pci_intx(pdev, !!irq_state);
> +#ifdef HAVE_IRQ_DATA
> +	struct irq_data *irq = irq_get_irq_data(udev->info.irq);
> +#else
> +	unsigned int irq = udev->info.irq;
> +#endif
>  
> -	else if (udev->mode == RTE_INTR_MODE_MSIX) {
> -		struct msi_desc *desc;
> +	pci_cfg_access_lock(pdev);
>  
> -#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
> -		list_for_each_entry(desc, &pdev->msi_list, list)
> -			igbuio_msix_mask_irq(desc, irq_state);
> +	if (udev->mode == RTE_INTR_MODE_MSIX || udev->mode == RTE_INTR_MODE_MSI) {
> +#ifdef HAVE_PCI_MSI_MASK_IRQ
> +		if (irq_state == 1)
> +			pci_msi_unmask_irq(irq);
> +		else
> +			pci_msi_mask_irq(irq);
>  #else
> -		list_for_each_entry(desc, &pdev->dev.msi_list, list)
> -			igbuio_msix_mask_irq(desc, irq_state);
> +		if (irq_state == 1)
> +			unmask_msi_irq(irq);
> +		else
> +			mask_msi_irq(irq);

Can you please split this patch into two:
- first converts igbuio_msix_mask_irq() into Linux functions, with
HAVE_IRQ_DATA and HAVE_PCI_MSI_MASK_IRQ checks
- second add MSI support, as it is in this patch

Thanks,
ferruh

[dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions

[Markus Theil]

Interrupt setup code in igb_uio has to deal with multiple
types of interrupts and kernel versions. This patch moves
the setup and teardown code into own functions, to make
it more readable.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 112 +++++++++++++++++-------------
 1 file changed, 64 insertions(+), 48 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 07a19a3..e2e9263 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -309,6 +309,66 @@ igbuio_pci_release_iomem(struct uio_info *info)
 }
 
 static int
+igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
+{
+	int err = 0;
+#ifdef HAVE_PCI_ENABLE_MSIX
+	struct msix_entry msix_entry;
+#endif
+
+	switch (igbuio_intr_mode_preferred) {
+	case RTE_INTR_MODE_MSIX:
+		/* Only 1 msi-x vector needed */
+#ifdef HAVE_PCI_ENABLE_MSIX
+		msix_entry.entry = 0;
+		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = msix_entry.vector;
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSIX;
+			break;
+		}
+#endif
+	/* fall back to INTX */
+	case RTE_INTR_MODE_LEGACY:
+		if (pci_intx_mask_supported(udev->pdev)) {
+			dev_dbg(&udev->pdev->dev, "using INTX");
+			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_LEGACY;
+			break;
+		}
+		dev_notice(&udev->pdev->dev, "PCI INTX mask not supported\n");
+		/* fall back to no IRQ */
+	case RTE_INTR_MODE_NONE:
+		udev->mode = RTE_INTR_MODE_NONE;
+		udev->info.irq = 0;
+		break;
+
+	default:
+		dev_err(&udev->pdev->dev, "invalid IRQ mode %u",
+			igbuio_intr_mode_preferred);
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static void
+igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
+{
+	if (udev->mode == RTE_INTR_MODE_MSIX)
+		pci_disable_msix(udev->pdev);
+}
+
+static int
 igbuio_setup_bars(struct pci_dev *dev, struct uio_info *info)
 {
 	int i, iom, iop, ret;
@@ -356,9 +416,6 @@ static int
 igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	struct rte_uio_pci_dev *udev;
-#ifdef HAVE_PCI_ENABLE_MSIX
-	struct msix_entry msix_entry;
-#endif
 	dma_addr_t map_dma_addr;
 	void *map_addr;
 	int err;
@@ -413,48 +470,9 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	udev->info.priv = udev;
 	udev->pdev = dev;
 
-	switch (igbuio_intr_mode_preferred) {
-	case RTE_INTR_MODE_MSIX:
-		/* Only 1 msi-x vector needed */
-#ifdef HAVE_PCI_ENABLE_MSIX
-		msix_entry.entry = 0;
-		if (pci_enable_msix(dev, &msix_entry, 1) == 0) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq_flags = IRQF_NO_THREAD;
-			udev->info.irq = msix_entry.vector;
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#else
-		if (pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_MSIX) == 1) {
-			dev_dbg(&dev->dev, "using MSI-X");
-			udev->info.irq = pci_irq_vector(dev, 0);
-			udev->mode = RTE_INTR_MODE_MSIX;
-			break;
-		}
-#endif
-		/* fall back to INTX */
-	case RTE_INTR_MODE_LEGACY:
-		if (pci_intx_mask_supported(dev)) {
-			dev_dbg(&dev->dev, "using INTX");
-			udev->info.irq_flags = IRQF_SHARED | IRQF_NO_THREAD;
-			udev->info.irq = dev->irq;
-			udev->mode = RTE_INTR_MODE_LEGACY;
-			break;
-		}
-		dev_notice(&dev->dev, "PCI INTX mask not supported\n");
-		/* fall back to no IRQ */
-	case RTE_INTR_MODE_NONE:
-		udev->mode = RTE_INTR_MODE_NONE;
-		udev->info.irq = 0;
-		break;
-
-	default:
-		dev_err(&dev->dev, "invalid IRQ mode %u",
-			igbuio_intr_mode_preferred);
-		err = -EINVAL;
+	err = igbuio_pci_enable_interrupts(udev);
+	if (err != 0)
 		goto fail_release_iomem;
-	}
 
 	err = sysfs_create_group(&dev->dev.kobj, &dev_attr_grp);
 	if (err != 0)
@@ -497,8 +515,7 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 fail_release_iomem:
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(udev->pdev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 fail_free:
 	kfree(udev);
@@ -514,8 +531,7 @@ igbuio_pci_remove(struct pci_dev *dev)
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 	uio_unregister_device(&udev->info);
 	igbuio_pci_release_iomem(&udev->info);
-	if (udev->mode == RTE_INTR_MODE_MSIX)
-		pci_disable_msix(dev);
+	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 	pci_set_drvdata(dev, NULL);
 	kfree(udev);
-- 
2.7.4

[dpdk-dev] [PATCH v7 2/6] igb_uio: fix irq disable on recent kernels

[Markus Theil]

igb_uio already allocates irqs using pci_alloc_irq_vectors on
recent kernels >= 4.8. The interrupt disable code was not
using the corresponding pci_free_irq_vectors, but the also
deprecated pci_disable_msix, before this fix.

Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for MSI-X")
Cc: nicolas.dichtel@6wind.com

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/compat.h  | 4 ++--
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 9 +++++++--
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
index b800a53..3825933 100644
--- a/lib/librte_eal/linuxapp/igb_uio/compat.h
+++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
@@ -124,6 +124,6 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
 
 #endif /* < 3.3.0 */
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 8, 0)
-#define HAVE_PCI_ENABLE_MSIX
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
+#define HAVE_ALLOC_IRQ_VECTORS 1
 #endif
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index e2e9263..93bb71d 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -312,14 +312,14 @@ static int
 igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 {
 	int err = 0;
-#ifdef HAVE_PCI_ENABLE_MSIX
+#ifndef HAVE_ALLOC_IRQ_VECTORS
 	struct msix_entry msix_entry;
 #endif
 
 	switch (igbuio_intr_mode_preferred) {
 	case RTE_INTR_MODE_MSIX:
 		/* Only 1 msi-x vector needed */
-#ifdef HAVE_PCI_ENABLE_MSIX
+#ifndef HAVE_ALLOC_IRQ_VECTORS
 		msix_entry.entry = 0;
 		if (pci_enable_msix(udev->pdev, &msix_entry, 1) == 0) {
 			dev_dbg(&udev->pdev->dev, "using MSI-X");
@@ -364,8 +364,13 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 static void
 igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
 {
+#ifndef HAVE_ALLOC_IRQ_VECTORS
 	if (udev->mode == RTE_INTR_MODE_MSIX)
 		pci_disable_msix(udev->pdev);
+#else
+	if (udev->mode == RTE_INTR_MODE_MSIX)
+		pci_free_irq_vectors(udev->pdev);
+#endif
 }
 
 static int
-- 
2.7.4

[dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function

[Markus Theil]

The patch which introduced the usage of pci_alloc_irq_vectors
came after the patch which switched to non-threaded ISR (f0d1896fa1),
but did not use non-threaded ISR, if pci_alloc_irq_vectors
is used.

Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for
MSI-X")
Cc: nicolas.dichtel@6wind.com

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 93bb71d..6885e72 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -331,6 +331,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 #else
 		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
 			dev_dbg(&udev->pdev->dev, "using MSI-X");
+			udev->info.irq_flags = IRQF_NO_THREAD;
 			udev->info.irq = pci_irq_vector(udev->pdev, 0);
 			udev->mode = RTE_INTR_MODE_MSIX;
 			break;
-- 
2.7.4

Re: [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function

[Ferruh Yigit]

On 9/5/2017 1:04 PM, Markus Theil wrote:
> The patch which introduced the usage of pci_alloc_irq_vectors
> came after the patch which switched to non-threaded ISR (f0d1896fa1),
> but did not use non-threaded ISR, if pci_alloc_irq_vectors
> is used.
> 
> Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for
> MSI-X")
> Cc: nicolas.dichtel@6wind.com
> 
> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> ---
>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> index 93bb71d..6885e72 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> @@ -331,6 +331,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
>  #else
>  		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
>  			dev_dbg(&udev->pdev->dev, "using MSI-X");
> +			udev->info.irq_flags = IRQF_NO_THREAD;

IRQF_NO_THREAD seems has been introduced in 2.6.39, so using this flag
causing build error for kernel versions < 2.6.39.

btw, the flag is already in use, so issue is not related to this patch.

In DPDK documentation supported Linux kernel version is >= 2.6.34 [1].

We should either increase supported version to 2.6.39, or update igb_uio
code.

I am for increasing minimum supported kernel version to 2.6.39, any
objection / comment?

2.6.39 released on May 2011
2.6.34 released on May 2010


[1]
http://dpdk.org/doc/guides/linux_gsg/sys_reqs.html#system-software

>  			udev->info.irq = pci_irq_vector(udev->pdev, 0);
>  			udev->mode = RTE_INTR_MODE_MSIX;
>  			break;
> 

Re: [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function

[Stephen Hemminger]

I wonder if it is time to move the bar forward to oldest LTS which is
3.2.92


On Sep 11, 2017 10:56 AM, "Ferruh Yigit" <ferruh.yigit@intel.com> wrote:

> On 9/5/2017 1:04 PM, Markus Theil wrote:
> > The patch which introduced the usage of pci_alloc_irq_vectors
> > came after the patch which switched to non-threaded ISR (f0d1896fa1),
> > but did not use non-threaded ISR, if pci_alloc_irq_vectors
> > is used.
> >
> > Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for
> > MSI-X")
> > Cc: nicolas.dichtel@6wind.com
> >
> > Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> > ---
> >  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > index 93bb71d..6885e72 100644
> > --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > @@ -331,6 +331,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev
> *udev)
> >  #else
> >               if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX)
> == 1) {
> >                       dev_dbg(&udev->pdev->dev, "using MSI-X");
> > +                     udev->info.irq_flags = IRQF_NO_THREAD;
>
> IRQF_NO_THREAD seems has been introduced in 2.6.39, so using this flag
> causing build error for kernel versions < 2.6.39.
>
> btw, the flag is already in use, so issue is not related to this patch.
>
> In DPDK documentation supported Linux kernel version is >= 2.6.34 [1].
>
> We should either increase supported version to 2.6.39, or update igb_uio
> code.
>
> I am for increasing minimum supported kernel version to 2.6.39, any
> objection / comment?
>
> 2.6.39 released on May 2011
> 2.6.34 released on May 2010
>
>
> [1]
> http://dpdk.org/doc/guides/linux_gsg/sys_reqs.html#system-software
>
> >                       udev->info.irq = pci_irq_vector(udev->pdev, 0);
> >                       udev->mode = RTE_INTR_MODE_MSIX;
> >                       break;
> >
>
>

Re: [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function

[Bruce Richardson]

On Mon, Sep 11, 2017 at 03:04:01PM -0700, Stephen Hemminger wrote:
> I wonder if it is time to move the bar forward to oldest LTS which is
> 3.2.92
> 

That seems reasonable. Probably best to do a deprecation notice for it
in 17.11 and move the bar in 18.02.

/Bruce

> 
> On Sep 11, 2017 10:56 AM, "Ferruh Yigit" <ferruh.yigit@intel.com> wrote:
> 
> > On 9/5/2017 1:04 PM, Markus Theil wrote:
> > > The patch which introduced the usage of pci_alloc_irq_vectors
> > > came after the patch which switched to non-threaded ISR (f0d1896fa1),
> > > but did not use non-threaded ISR, if pci_alloc_irq_vectors
> > > is used.
> > >
> > > Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for
> > > MSI-X")
> > > Cc: nicolas.dichtel@6wind.com
> > >
> > > Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> > > ---
> > >  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > > index 93bb71d..6885e72 100644
> > > --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > > +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > > @@ -331,6 +331,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev
> > *udev)
> > >  #else
> > >               if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX)
> > == 1) {
> > >                       dev_dbg(&udev->pdev->dev, "using MSI-X");
> > > +                     udev->info.irq_flags = IRQF_NO_THREAD;
> >
> > IRQF_NO_THREAD seems has been introduced in 2.6.39, so using this flag
> > causing build error for kernel versions < 2.6.39.
> >
> > btw, the flag is already in use, so issue is not related to this patch.
> >
> > In DPDK documentation supported Linux kernel version is >= 2.6.34 [1].
> >
> > We should either increase supported version to 2.6.39, or update igb_uio
> > code.
> >
> > I am for increasing minimum supported kernel version to 2.6.39, any
> > objection / comment?
> >
> > 2.6.39 released on May 2011
> > 2.6.34 released on May 2010
> >
> >
> > [1]
> > http://dpdk.org/doc/guides/linux_gsg/sys_reqs.html#system-software
> >
> > >                       udev->info.irq = pci_irq_vector(udev->pdev, 0);
> > >                       udev->mode = RTE_INTR_MODE_MSIX;
> > >                       break;
> > >
> >
> >

Re: [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function

[Stephen Hemminger]

Why not have an ongoing sliding window policy? So each release it is set

On Sep 12, 2017 1:14 AM, "Bruce Richardson" <bruce.richardson@intel.com>
wrote:

> On Mon, Sep 11, 2017 at 03:04:01PM -0700, Stephen Hemminger wrote:
> > I wonder if it is time to move the bar forward to oldest LTS which is
> > 3.2.92
> >
>
> That seems reasonable. Probably best to do a deprecation notice for it
> in 17.11 and move the bar in 18.02.
>
> /Bruce
>
> >
> > On Sep 11, 2017 10:56 AM, "Ferruh Yigit" <ferruh.yigit@intel.com> wrote:
> >
> > > On 9/5/2017 1:04 PM, Markus Theil wrote:
> > > > The patch which introduced the usage of pci_alloc_irq_vectors
> > > > came after the patch which switched to non-threaded ISR (f0d1896fa1),
> > > > but did not use non-threaded ISR, if pci_alloc_irq_vectors
> > > > is used.
> > > >
> > > > Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for
> > > > MSI-X")
> > > > Cc: nicolas.dichtel@6wind.com
> > > >
> > > > Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> > > > ---
> > > >  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > > b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > > > index 93bb71d..6885e72 100644
> > > > --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > > > +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > > > @@ -331,6 +331,7 @@ igbuio_pci_enable_interrupts(struct
> rte_uio_pci_dev
> > > *udev)
> > > >  #else
> > > >               if (pci_alloc_irq_vectors(udev->pdev, 1, 1,
> PCI_IRQ_MSIX)
> > > == 1) {
> > > >                       dev_dbg(&udev->pdev->dev, "using MSI-X");
> > > > +                     udev->info.irq_flags = IRQF_NO_THREAD;
> > >
> > > IRQF_NO_THREAD seems has been introduced in 2.6.39, so using this flag
> > > causing build error for kernel versions < 2.6.39.
> > >
> > > btw, the flag is already in use, so issue is not related to this patch.
> > >
> > > In DPDK documentation supported Linux kernel version is >= 2.6.34 [1].
> > >
> > > We should either increase supported version to 2.6.39, or update
> igb_uio
> > > code.
> > >
> > > I am for increasing minimum supported kernel version to 2.6.39, any
> > > objection / comment?
> > >
> > > 2.6.39 released on May 2011
> > > 2.6.34 released on May 2010
> > >
> > >
> > > [1]
> > > http://dpdk.org/doc/guides/linux_gsg/sys_reqs.html#system-software
> > >
> > > >                       udev->info.irq = pci_irq_vector(udev->pdev, 0);
> > > >                       udev->mode = RTE_INTR_MODE_MSIX;
> > > >                       break;
> > > >
> > >
> > >
>

Re: [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function

[Bruce Richardson]

On Mon, Sep 11, 2017 at 06:56:39PM +0100, Ferruh Yigit wrote:
> On 9/5/2017 1:04 PM, Markus Theil wrote:
> > The patch which introduced the usage of pci_alloc_irq_vectors
> > came after the patch which switched to non-threaded ISR (f0d1896fa1),
> > but did not use non-threaded ISR, if pci_alloc_irq_vectors
> > is used.
> > 
> > Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for
> > MSI-X")
> > Cc: nicolas.dichtel@6wind.com
> > 
> > Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> > ---
> >  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > index 93bb71d..6885e72 100644
> > --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > @@ -331,6 +331,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
> >  #else
> >  		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
> >  			dev_dbg(&udev->pdev->dev, "using MSI-X");
> > +			udev->info.irq_flags = IRQF_NO_THREAD;
> 
> IRQF_NO_THREAD seems has been introduced in 2.6.39, so using this flag
> causing build error for kernel versions < 2.6.39.
> 
> btw, the flag is already in use, so issue is not related to this patch.
> 
> In DPDK documentation supported Linux kernel version is >= 2.6.34 [1].
> 
> We should either increase supported version to 2.6.39, or update igb_uio
> code.
> 
> I am for increasing minimum supported kernel version to 2.6.39, any
> objection / comment?
> 
> 2.6.39 released on May 2011
> 2.6.34 released on May 2010
> 

Only thing I can think of here is:
* is the necessary support for this in RHEL 6.x series
* anyone still care about RHEL 6 support?

/Bruce

Re: [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function

[Kevin Traynor]

On 09/12/2017 09:16 AM, Bruce Richardson wrote:
> On Mon, Sep 11, 2017 at 06:56:39PM +0100, Ferruh Yigit wrote:
>> On 9/5/2017 1:04 PM, Markus Theil wrote:
>>> The patch which introduced the usage of pci_alloc_irq_vectors
>>> came after the patch which switched to non-threaded ISR (f0d1896fa1),
>>> but did not use non-threaded ISR, if pci_alloc_irq_vectors
>>> is used.
>>>
>>> Fixes: 99bb58f3adc7 ("igb_uio: switch to new irq function for
>>> MSI-X")
>>> Cc: nicolas.dichtel@6wind.com
>>>
>>> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
>>> ---
>>>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>> index 93bb71d..6885e72 100644
>>> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>> @@ -331,6 +331,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
>>>  #else
>>>  		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSIX) == 1) {
>>>  			dev_dbg(&udev->pdev->dev, "using MSI-X");
>>> +			udev->info.irq_flags = IRQF_NO_THREAD;
>>
>> IRQF_NO_THREAD seems has been introduced in 2.6.39, so using this flag
>> causing build error for kernel versions < 2.6.39.
>>
>> btw, the flag is already in use, so issue is not related to this patch.
>>
>> In DPDK documentation supported Linux kernel version is >= 2.6.34 [1].
>>
>> We should either increase supported version to 2.6.39, or update igb_uio
>> code.
>>
>> I am for increasing minimum supported kernel version to 2.6.39, any
>> objection / comment?
>>
>> 2.6.39 released on May 2011
>> 2.6.34 released on May 2010
>>
> 
> Only thing I can think of here is:
> * is the necessary support for this in RHEL 6.x series

It doesn't look to be there

> * anyone still care about RHEL 6 support?
> 

Not an issue with DPDK packages as igb_uio is not included. If someone
was using RHEL6/CentOS6 and igb_uio from DPDK source then seems like it
would be an issue for them. In general, a deprecation notice would be a
good idea if the minimum kernel version is changing.

> /Bruce
> 

[dpdk-dev] [PATCH v7 4/6] igb_uio: release in exact reverse order

[Markus Theil]

For better readability throughout the module, the destruction
order is changed to the exact inverse construction order.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 6885e72..c570eed 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -482,7 +482,7 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	err = sysfs_create_group(&dev->dev.kobj, &dev_attr_grp);
 	if (err != 0)
-		goto fail_release_iomem;
+		goto fail_disable_interrupts;
 
 	/* register uio driver */
 	err = uio_register_device(&dev->dev, &udev->info);
@@ -519,9 +519,10 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 fail_remove_group:
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
+fail_disable_interrupts:
+	igbuio_pci_disable_interrupts(udev);
 fail_release_iomem:
 	igbuio_pci_release_iomem(&udev->info);
-	igbuio_pci_disable_interrupts(udev);
 	pci_disable_device(dev);
 fail_free:
 	kfree(udev);
@@ -536,8 +537,8 @@ igbuio_pci_remove(struct pci_dev *dev)
 
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
 	uio_unregister_device(&udev->info);
-	igbuio_pci_release_iomem(&udev->info);
 	igbuio_pci_disable_interrupts(udev);
+	igbuio_pci_release_iomem(&udev->info);
 	pci_disable_device(dev);
 	pci_set_drvdata(dev, NULL);
 	kfree(udev);
-- 
2.7.4

[dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X

[Markus Theil]

This patch removes the custom MSI-X mask/unmask code and
uses already existing kernel functions.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/compat.h  | 26 +++++-----------
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 51 ++++++++++++-------------------
 2 files changed, 28 insertions(+), 49 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
index 3825933..67a7ab3 100644
--- a/lib/librte_eal/linuxapp/igb_uio/compat.h
+++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
@@ -15,24 +15,6 @@
 #define HAVE_PTE_MASK_PAGE_IOMAP
 #endif
 
-#ifndef PCI_MSIX_ENTRY_SIZE
-#define PCI_MSIX_ENTRY_SIZE             16
-#define  PCI_MSIX_ENTRY_LOWER_ADDR      0
-#define  PCI_MSIX_ENTRY_UPPER_ADDR      4
-#define  PCI_MSIX_ENTRY_DATA            8
-#define  PCI_MSIX_ENTRY_VECTOR_CTRL     12
-#define   PCI_MSIX_ENTRY_CTRL_MASKBIT   1
-#endif
-
-/*
- * for kernels < 2.6.38 and backported patch that moves MSI-X entry definition
- * to pci_regs.h Those kernels has PCI_MSIX_ENTRY_SIZE defined but not
- * PCI_MSIX_ENTRY_CTRL_MASKBIT
- */
-#ifndef PCI_MSIX_ENTRY_CTRL_MASKBIT
-#define PCI_MSIX_ENTRY_CTRL_MASKBIT    1
-#endif
-
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 34) && \
 	(!(defined(RHEL_RELEASE_CODE) && \
 	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(5, 9)))
@@ -127,3 +109,11 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
 #define HAVE_ALLOC_IRQ_VECTORS 1
 #endif
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 19, 0)
+#define HAVE_PCI_MSI_MASK_IRQ 1
+#endif
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 37)
+#define HAVE_IRQ_DATA 1
+#endif
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index c570eed..e4ef817 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -91,27 +91,6 @@ static struct attribute *dev_attrs[] = {
 static const struct attribute_group dev_attr_grp = {
 	.attrs = dev_attrs,
 };
-/*
- * It masks the msix on/off of generating MSI-X messages.
- */
-static void
-igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state)
-{
-	u32 mask_bits = desc->masked;
-	unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
-						PCI_MSIX_ENTRY_VECTOR_CTRL;
-
-	if (state != 0)
-		mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
-	else
-		mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
-
-	if (mask_bits != desc->masked) {
-		writel(mask_bits, desc->mask_base + offset);
-		readl(desc->mask_base);
-		desc->masked = mask_bits;
-	}
-}
 
 /**
  * This is the irqcontrol callback to be registered to uio_info.
@@ -132,21 +111,31 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
 	struct rte_uio_pci_dev *udev = info->priv;
 	struct pci_dev *pdev = udev->pdev;
 
-	pci_cfg_access_lock(pdev);
-	if (udev->mode == RTE_INTR_MODE_LEGACY)
-		pci_intx(pdev, !!irq_state);
+#ifdef HAVE_IRQ_DATA
+	struct irq_data *irq = irq_get_irq_data(udev->info.irq);
+#else
+	unsigned int irq = udev->info.irq;
+#endif
 
-	else if (udev->mode == RTE_INTR_MODE_MSIX) {
-		struct msi_desc *desc;
+	pci_cfg_access_lock(pdev);
 
-#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
-		list_for_each_entry(desc, &pdev->msi_list, list)
-			igbuio_msix_mask_irq(desc, irq_state);
+	if (udev->mode == RTE_INTR_MODE_MSIX) {
+#ifdef HAVE_PCI_MSI_MASK_IRQ
+		if (irq_state == 1)
+			pci_msi_unmask_irq(irq);
+		else
+			pci_msi_mask_irq(irq);
 #else
-		list_for_each_entry(desc, &pdev->dev.msi_list, list)
-			igbuio_msix_mask_irq(desc, irq_state);
+		if (irq_state == 1)
+			unmask_msi_irq(irq);
+		else
+			mask_msi_irq(irq);
 #endif
 	}
+
+	if (udev->mode == RTE_INTR_MODE_LEGACY)
+		pci_intx(pdev, !!irq_state);
+
 	pci_cfg_access_unlock(pdev);
 
 	return 0;
-- 
2.7.4

Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X

[Patrick MacArthur]

Hi, Markus,

This commit appears to cause a regression on CentOS 7.4 with the in-box 
Linux 3.10.0-639-2.2.el7.x86_64 kernel. Although the kernel module 
appears to build correctly, when I attempt to load the module with 
insmod, it fails and I see the following errors in dmesg:

> [620323.805125] igb_uio: Unknown symbol unmask_msi_irq (err 0)
> [620323.805163] igb_uio: Unknown symbol mask_msi_irq (err 0)

It also fails with the same dmesg errors if I copy it into 
/lib/modules/$(uname -r)/extra, run depmod, and try to modprobe it.

Running git bisect points to this commit as the root cause.

This issue was identified as part of setting up the DPDK performance 
test lab CI environment at the University of New Hampshire 
InterOperability Laboratory.

Thanks,
Patrick

On 09/05/2017 08:04 AM, Markus Theil wrote:
> This patch removes the custom MSI-X mask/unmask code and
> uses already existing kernel functions.
> 
> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> ---
>   lib/librte_eal/linuxapp/igb_uio/compat.h  | 26 +++++-----------
>   lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 51 ++++++++++++-------------------
>   2 files changed, 28 insertions(+), 49 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
> index 3825933..67a7ab3 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/compat.h
> +++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
> @@ -15,24 +15,6 @@
>   #define HAVE_PTE_MASK_PAGE_IOMAP
>   #endif
>   
> -#ifndef PCI_MSIX_ENTRY_SIZE
> -#define PCI_MSIX_ENTRY_SIZE             16
> -#define  PCI_MSIX_ENTRY_LOWER_ADDR      0
> -#define  PCI_MSIX_ENTRY_UPPER_ADDR      4
> -#define  PCI_MSIX_ENTRY_DATA            8
> -#define  PCI_MSIX_ENTRY_VECTOR_CTRL     12
> -#define   PCI_MSIX_ENTRY_CTRL_MASKBIT   1
> -#endif
> -
> -/*
> - * for kernels < 2.6.38 and backported patch that moves MSI-X entry definition
> - * to pci_regs.h Those kernels has PCI_MSIX_ENTRY_SIZE defined but not
> - * PCI_MSIX_ENTRY_CTRL_MASKBIT
> - */
> -#ifndef PCI_MSIX_ENTRY_CTRL_MASKBIT
> -#define PCI_MSIX_ENTRY_CTRL_MASKBIT    1
> -#endif
> -
>   #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 34) && \
>   	(!(defined(RHEL_RELEASE_CODE) && \
>   	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(5, 9)))
> @@ -127,3 +109,11 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
>   #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
>   #define HAVE_ALLOC_IRQ_VECTORS 1
>   #endif
> +
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 19, 0)
> +#define HAVE_PCI_MSI_MASK_IRQ 1
> +#endif
> +
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 37)
> +#define HAVE_IRQ_DATA 1
> +#endif
> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> index c570eed..e4ef817 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> @@ -91,27 +91,6 @@ static struct attribute *dev_attrs[] = {
>   static const struct attribute_group dev_attr_grp = {
>   	.attrs = dev_attrs,
>   };
> -/*
> - * It masks the msix on/off of generating MSI-X messages.
> - */
> -static void
> -igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state)
> -{
> -	u32 mask_bits = desc->masked;
> -	unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
> -						PCI_MSIX_ENTRY_VECTOR_CTRL;
> -
> -	if (state != 0)
> -		mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
> -	else
> -		mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
> -
> -	if (mask_bits != desc->masked) {
> -		writel(mask_bits, desc->mask_base + offset);
> -		readl(desc->mask_base);
> -		desc->masked = mask_bits;
> -	}
> -}
>   
>   /**
>    * This is the irqcontrol callback to be registered to uio_info.
> @@ -132,21 +111,31 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
>   	struct rte_uio_pci_dev *udev = info->priv;
>   	struct pci_dev *pdev = udev->pdev;
>   
> -	pci_cfg_access_lock(pdev);
> -	if (udev->mode == RTE_INTR_MODE_LEGACY)
> -		pci_intx(pdev, !!irq_state);
> +#ifdef HAVE_IRQ_DATA
> +	struct irq_data *irq = irq_get_irq_data(udev->info.irq);
> +#else
> +	unsigned int irq = udev->info.irq;
> +#endif
>   
> -	else if (udev->mode == RTE_INTR_MODE_MSIX) {
> -		struct msi_desc *desc;
> +	pci_cfg_access_lock(pdev);
>   
> -#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
> -		list_for_each_entry(desc, &pdev->msi_list, list)
> -			igbuio_msix_mask_irq(desc, irq_state);
> +	if (udev->mode == RTE_INTR_MODE_MSIX) {
> +#ifdef HAVE_PCI_MSI_MASK_IRQ
> +		if (irq_state == 1)
> +			pci_msi_unmask_irq(irq);
> +		else
> +			pci_msi_mask_irq(irq);
>   #else
> -		list_for_each_entry(desc, &pdev->dev.msi_list, list)
> -			igbuio_msix_mask_irq(desc, irq_state);
> +		if (irq_state == 1)
> +			unmask_msi_irq(irq);
> +		else
> +			mask_msi_irq(irq);
>   #endif
>   	}
> +
> +	if (udev->mode == RTE_INTR_MODE_LEGACY)
> +		pci_intx(pdev, !!irq_state);
> +
>   	pci_cfg_access_unlock(pdev);
>   
>   	return 0;
> 

Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X

[Ferruh Yigit]

On 10/9/2017 10:56 PM, Patrick MacArthur wrote:
> Hi, Markus,
> 
> This commit appears to cause a regression on CentOS 7.4 with the in-box 
> Linux 3.10.0-639-2.2.el7.x86_64 kernel. Although the kernel module 
> appears to build correctly, when I attempt to load the module with 
> insmod, it fails and I see the following errors in dmesg:
> 
>> [620323.805125] igb_uio: Unknown symbol unmask_msi_irq (err 0)
>> [620323.805163] igb_uio: Unknown symbol mask_msi_irq (err 0)
> 

Hi Patrick, Mark,

Thanks for reporting, I will check it.


> It also fails with the same dmesg errors if I copy it into 
> /lib/modules/$(uname -r)/extra, run depmod, and try to modprobe it.
> 
> Running git bisect points to this commit as the root cause.
> 
> This issue was identified as part of setting up the DPDK performance 
> test lab CI environment at the University of New Hampshire 
> InterOperability Laboratory.
> 
> Thanks,
> Patrick
> 
<...>

Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X

[Ferruh Yigit]

On 10/12/2017 6:04 PM, Ferruh Yigit wrote:
> On 10/9/2017 10:56 PM, Patrick MacArthur wrote:
>> Hi, Markus,
>>
>> This commit appears to cause a regression on CentOS 7.4 with the in-box 
>> Linux 3.10.0-639-2.2.el7.x86_64 kernel. Although the kernel module 
>> appears to build correctly, when I attempt to load the module with 
>> insmod, it fails and I see the following errors in dmesg:
>>
>>> [620323.805125] igb_uio: Unknown symbol unmask_msi_irq (err 0)
>>> [620323.805163] igb_uio: Unknown symbol mask_msi_irq (err 0)
>>
> 
> Hi Patrick, Mark,
> 
> Thanks for reporting, I will check it.

Can you please test http://dpdk.org/dev/patchwork/patch/30325/

> 
> 
>> It also fails with the same dmesg errors if I copy it into 
>> /lib/modules/$(uname -r)/extra, run depmod, and try to modprobe it.
>>
>> Running git bisect points to this commit as the root cause.
>>
>> This issue was identified as part of setting up the DPDK performance 
>> test lab CI environment at the University of New Hampshire 
>> InterOperability Laboratory.
>>
>> Thanks,
>> Patrick
>>
> <...>
> 

Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X

[Kavanagh, Mark B]

>From: Yigit, Ferruh
>Sent: Friday, October 13, 2017 3:50 AM
>To: Patrick MacArthur <patrick@patrickmacarthur.net>; Markus Theil
><markus.theil@tu-ilmenau.de>; dev@dpdk.org; Kavanagh, Mark B
><mark.b.kavanagh@intel.com>
>Cc: stephen@networkplumber.org; Bob Noseworthy <ren@iol.unh.edu>; Patrick
>MacArthur <pmacarth@iol.unh.edu>; O'Driscoll, Tim <tim.odriscoll@intel.com>
>Subject: Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for
>masking MSI-X
>
>On 10/12/2017 6:04 PM, Ferruh Yigit wrote:
>> On 10/9/2017 10:56 PM, Patrick MacArthur wrote:
>>> Hi, Markus,
>>>
>>> This commit appears to cause a regression on CentOS 7.4 with the in-box
>>> Linux 3.10.0-639-2.2.el7.x86_64 kernel. Although the kernel module
>>> appears to build correctly, when I attempt to load the module with
>>> insmod, it fails and I see the following errors in dmesg:
>>>
>>>> [620323.805125] igb_uio: Unknown symbol unmask_msi_irq (err 0)
>>>> [620323.805163] igb_uio: Unknown symbol mask_msi_irq (err 0)
>>>
>>
>> Hi Patrick, Mark,
>>
>> Thanks for reporting, I will check it.
>
>Can you please test http://dpdk.org/dev/patchwork/patch/30325/

Hey Ferruh,

I can confirm that this patch resolves the build issues on Fedora 20, kernel 3.19.8-100.fc20.x86_64 - thanks!

Incidentally, there is a minor warning reported when applying the patch:
	
	Applying: igb_uio: fix unknown symbols
	/home/<redacted>/.git/rebase-apply/patch:47: new blank line at EOF.
	+

Thanks again,
Mark


>
>>
>>
>>> It also fails with the same dmesg errors if I copy it into
>>> /lib/modules/$(uname -r)/extra, run depmod, and try to modprobe it.
>>>
>>> Running git bisect points to this commit as the root cause.
>>>
>>> This issue was identified as part of setting up the DPDK performance
>>> test lab CI environment at the University of New Hampshire
>>> InterOperability Laboratory.
>>>
>>> Thanks,
>>> Patrick
>>>
>> <...>
>>

Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X

[Kavanagh, Mark B]

Hi,

This commit renders igb_uio unusable on Fedora 20, kernel version 3.19.8-100.fc20.x86_64.

During the build (make -j 20), a warning is issued for igb_uio regarding a missing symbol (pci_msi_unmask):
	WARNING: "pci_msi_unmask_irq" [/home/<redacted>/x86_64-native-linuxapp-gcc/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.ko] undefined!
	WARNING: "pci_msi_mask_irq" [/home/<redacted>/x86_64-native-linuxapp-gcc/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.ko] undefined!

Subsequently, the module is may not be loaded on account of same.
(from dmesg): 
	[673425.712110] igb_uio: Unknown symbol pci_msi_unmask_irq (err 0)
	[673425.712124] igb_uio: Unknown symbol pci_msi_mask_irq (err 0)

Thanks,
Mark

>-----Original Message-----
>From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Markus Theil
>Sent: Tuesday, September 5, 2017 1:04 PM
>To: dev@dpdk.org
>Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stephen@networkplumber.org; Markus
>Theil <markus.theil@tu-ilmenau.de>
>Subject: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking
>MSI-X
>
>This patch removes the custom MSI-X mask/unmask code and
>uses already existing kernel functions.
>
>Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
>---
> lib/librte_eal/linuxapp/igb_uio/compat.h  | 26 +++++-----------
> lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 51 ++++++++++++------------------
>-
> 2 files changed, 28 insertions(+), 49 deletions(-)
>
>diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h
>b/lib/librte_eal/linuxapp/igb_uio/compat.h
>index 3825933..67a7ab3 100644
>--- a/lib/librte_eal/linuxapp/igb_uio/compat.h
>+++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
>@@ -15,24 +15,6 @@
> #define HAVE_PTE_MASK_PAGE_IOMAP
> #endif
>
>-#ifndef PCI_MSIX_ENTRY_SIZE
>-#define PCI_MSIX_ENTRY_SIZE             16
>-#define  PCI_MSIX_ENTRY_LOWER_ADDR      0
>-#define  PCI_MSIX_ENTRY_UPPER_ADDR      4
>-#define  PCI_MSIX_ENTRY_DATA            8
>-#define  PCI_MSIX_ENTRY_VECTOR_CTRL     12
>-#define   PCI_MSIX_ENTRY_CTRL_MASKBIT   1
>-#endif
>-
>-/*
>- * for kernels < 2.6.38 and backported patch that moves MSI-X entry
>definition
>- * to pci_regs.h Those kernels has PCI_MSIX_ENTRY_SIZE defined but not
>- * PCI_MSIX_ENTRY_CTRL_MASKBIT
>- */
>-#ifndef PCI_MSIX_ENTRY_CTRL_MASKBIT
>-#define PCI_MSIX_ENTRY_CTRL_MASKBIT    1
>-#endif
>-
> #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 34) && \
> 	(!(defined(RHEL_RELEASE_CODE) && \
> 	 RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(5, 9)))
>@@ -127,3 +109,11 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
> #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
> #define HAVE_ALLOC_IRQ_VECTORS 1
> #endif
>+
>+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 19, 0)
>+#define HAVE_PCI_MSI_MASK_IRQ 1
>+#endif
>+
>+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 37)
>+#define HAVE_IRQ_DATA 1
>+#endif
>diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>index c570eed..e4ef817 100644
>--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>@@ -91,27 +91,6 @@ static struct attribute *dev_attrs[] = {
> static const struct attribute_group dev_attr_grp = {
> 	.attrs = dev_attrs,
> };
>-/*
>- * It masks the msix on/off of generating MSI-X messages.
>- */
>-static void
>-igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state)
>-{
>-	u32 mask_bits = desc->masked;
>-	unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
>-						PCI_MSIX_ENTRY_VECTOR_CTRL;
>-
>-	if (state != 0)
>-		mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
>-	else
>-		mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
>-
>-	if (mask_bits != desc->masked) {
>-		writel(mask_bits, desc->mask_base + offset);
>-		readl(desc->mask_base);
>-		desc->masked = mask_bits;
>-	}
>-}
>
> /**
>  * This is the irqcontrol callback to be registered to uio_info.
>@@ -132,21 +111,31 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32
>irq_state)
> 	struct rte_uio_pci_dev *udev = info->priv;
> 	struct pci_dev *pdev = udev->pdev;
>
>-	pci_cfg_access_lock(pdev);
>-	if (udev->mode == RTE_INTR_MODE_LEGACY)
>-		pci_intx(pdev, !!irq_state);
>+#ifdef HAVE_IRQ_DATA
>+	struct irq_data *irq = irq_get_irq_data(udev->info.irq);
>+#else
>+	unsigned int irq = udev->info.irq;
>+#endif
>
>-	else if (udev->mode == RTE_INTR_MODE_MSIX) {
>-		struct msi_desc *desc;
>+	pci_cfg_access_lock(pdev);
>
>-#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0))
>-		list_for_each_entry(desc, &pdev->msi_list, list)
>-			igbuio_msix_mask_irq(desc, irq_state);
>+	if (udev->mode == RTE_INTR_MODE_MSIX) {
>+#ifdef HAVE_PCI_MSI_MASK_IRQ
>+		if (irq_state == 1)
>+			pci_msi_unmask_irq(irq);
>+		else
>+			pci_msi_mask_irq(irq);
> #else
>-		list_for_each_entry(desc, &pdev->dev.msi_list, list)
>-			igbuio_msix_mask_irq(desc, irq_state);
>+		if (irq_state == 1)
>+			unmask_msi_irq(irq);
>+		else
>+			mask_msi_irq(irq);
> #endif
> 	}
>+
>+	if (udev->mode == RTE_INTR_MODE_LEGACY)
>+		pci_intx(pdev, !!irq_state);
>+
> 	pci_cfg_access_unlock(pdev);
>
> 	return 0;
>--
>2.7.4

Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X

[Ferruh Yigit]

On 10/12/2017 11:50 AM, Kavanagh, Mark B wrote:
> Hi,
> 
> This commit renders igb_uio unusable on Fedora 20, kernel version 3.19.8-100.fc20.x86_64.
> 
> During the build (make -j 20), a warning is issued for igb_uio regarding a missing symbol (pci_msi_unmask):
> 	WARNING: "pci_msi_unmask_irq" [/home/<redacted>/x86_64-native-linuxapp-gcc/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.ko] undefined!
> 	WARNING: "pci_msi_mask_irq" [/home/<redacted>/x86_64-native-linuxapp-gcc/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.ko] undefined!
> 

Hi Mark,

I can see vanilla 3.19.8 has pci_msi_unmask_irq and pci_msi_mask_irq.

Also I checked Fedora 3.19.8-100.fc20.x86_64 source code, they are still
there.

Not sure why getting this error.

> Subsequently, the module is may not be loaded on account of same.
> (from dmesg): 
> 	[673425.712110] igb_uio: Unknown symbol pci_msi_unmask_irq (err 0)
> 	[673425.712124] igb_uio: Unknown symbol pci_msi_mask_irq (err 0)
> 
> Thanks,
> Mark

Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X

[Stephen Hemminger]

Are the symbols exported?

On Oct 12, 2017 13:58, "Ferruh Yigit" <ferruh.yigit@intel.com> wrote:

> On 10/12/2017 11:50 AM, Kavanagh, Mark B wrote:
> > Hi,
> >
> > This commit renders igb_uio unusable on Fedora 20, kernel version
> 3.19.8-100.fc20.x86_64.
> >
> > During the build (make -j 20), a warning is issued for igb_uio regarding
> a missing symbol (pci_msi_unmask):
> >       WARNING: "pci_msi_unmask_irq" [/home/<redacted>/x86_64-
> native-linuxapp-gcc/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.ko]
> undefined!
> >       WARNING: "pci_msi_mask_irq" [/home/<redacted>/x86_64-
> native-linuxapp-gcc/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.ko]
> undefined!
> >
>
> Hi Mark,
>
> I can see vanilla 3.19.8 has pci_msi_unmask_irq and pci_msi_mask_irq.
>
> Also I checked Fedora 3.19.8-100.fc20.x86_64 source code, they are still
> there.
>
> Not sure why getting this error.
>
> > Subsequently, the module is may not be loaded on account of same.
> > (from dmesg):
> >       [673425.712110] igb_uio: Unknown symbol pci_msi_unmask_irq (err 0)
> >       [673425.712124] igb_uio: Unknown symbol pci_msi_mask_irq (err 0)
> >
> > Thanks,
> > Mark
>
>
>

Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X

[Ferruh Yigit]

On 10/12/2017 10:15 PM, Stephen Hemminger wrote:
> Are the symbols exported?

No!

They started to be exported in v4.5. I will update accordingly.

Thanks,
ferruh

> 
> On Oct 12, 2017 13:58, "Ferruh Yigit" <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
> 
>     On 10/12/2017 11:50 AM, Kavanagh, Mark B wrote:
>     > Hi,
>     >
>     > This commit renders igb_uio unusable on Fedora 20, kernel version
>     3.19.8-100.fc20.x86_64.
>     >
>     > During the build (make -j 20), a warning is issued for igb_uio
>     regarding a missing symbol (pci_msi_unmask):
>     >       WARNING: "pci_msi_unmask_irq"
>     [/home/<redacted>/x86_64-native-linuxapp-gcc/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.ko]
>     undefined!
>     >       WARNING: "pci_msi_mask_irq"
>     [/home/<redacted>/x86_64-native-linuxapp-gcc/build/lib/librte_eal/linuxapp/igb_uio/igb_uio.ko]
>     undefined!
>     >
> 
>     Hi Mark,
> 
>     I can see vanilla 3.19.8 has pci_msi_unmask_irq and pci_msi_mask_irq.
> 
>     Also I checked Fedora 3.19.8-100.fc20.x86_64 source code, they are still
>     there.
> 
>     Not sure why getting this error.
> 
>     > Subsequently, the module is may not be loaded on account of same.
>     > (from dmesg):
>     >       [673425.712110] igb_uio: Unknown symbol pci_msi_unmask_irq
>     (err 0)
>     >       [673425.712124] igb_uio: Unknown symbol pci_msi_mask_irq (err 0)
>     >
>     > Thanks,
>     > Mark
> 
> 

[dpdk-dev] [PATCH v7 6/6] igb_uio: MSI IRQ mode

[Markus Theil]

This patch adds MSI IRQ mode in a way, that should
also work on older kernel versions. The base for my patch
was an attempt to do this in cf705bc36c which was later
reverted in d8ee82745a. Compilation was tested on Linux 3.2,
4.10 and 4.12.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 32 ++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index e4ef817..b578c4a 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -119,7 +119,7 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
 
 	pci_cfg_access_lock(pdev);
 
-	if (udev->mode == RTE_INTR_MODE_MSIX) {
+	if (udev->mode == RTE_INTR_MODE_MSIX || udev->mode == RTE_INTR_MODE_MSI) {
 #ifdef HAVE_PCI_MSI_MASK_IRQ
 		if (irq_state == 1)
 			pci_msi_unmask_irq(irq);
@@ -326,6 +326,25 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 			break;
 		}
 #endif
+	/* fall back to MSI */
+	case RTE_INTR_MODE_MSI:
+#ifndef HAVE_ALLOC_IRQ_VECTORS
+		if (pci_enable_msi(udev->pdev) == 0) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = udev->pdev->irq;
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#else
+		if (pci_alloc_irq_vectors(udev->pdev, 1, 1, PCI_IRQ_MSI) == 1) {
+			dev_dbg(&udev->pdev->dev, "using MSI");
+			udev->info.irq_flags = IRQF_NO_THREAD;
+			udev->info.irq = pci_irq_vector(udev->pdev, 0);
+			udev->mode = RTE_INTR_MODE_MSI;
+			break;
+		}
+#endif
 	/* fall back to INTX */
 	case RTE_INTR_MODE_LEGACY:
 		if (pci_intx_mask_supported(udev->pdev)) {
@@ -336,7 +355,7 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 			break;
 		}
 		dev_notice(&udev->pdev->dev, "PCI INTX mask not supported\n");
-		/* fall back to no IRQ */
+	/* fall back to no IRQ */
 	case RTE_INTR_MODE_NONE:
 		udev->mode = RTE_INTR_MODE_NONE;
 		udev->info.irq = 0;
@@ -357,8 +376,11 @@ igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
 #ifndef HAVE_ALLOC_IRQ_VECTORS
 	if (udev->mode == RTE_INTR_MODE_MSIX)
 		pci_disable_msix(udev->pdev);
+	if (udev->mode == RTE_INTR_MODE_MSI)
+		pci_disable_msi(udev->pdev);
 #else
-	if (udev->mode == RTE_INTR_MODE_MSIX)
+	if (udev->mode == RTE_INTR_MODE_MSIX ||
+	    udev->mode == RTE_INTR_MODE_MSI)
 		pci_free_irq_vectors(udev->pdev);
 #endif
 }
@@ -544,6 +566,9 @@ igbuio_config_intr_mode(char *intr_str)
 	if (!strcmp(intr_str, RTE_INTR_MODE_MSIX_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
 		pr_info("Use MSIX interrupt\n");
+	} else if (!strcmp(intr_str, RTE_INTR_MODE_MSI_NAME)) {
+		igbuio_intr_mode_preferred = RTE_INTR_MODE_MSI;
+		pr_info("Use MSI interrupt\n");
 	} else if (!strcmp(intr_str, RTE_INTR_MODE_LEGACY_NAME)) {
 		igbuio_intr_mode_preferred = RTE_INTR_MODE_LEGACY;
 		pr_info("Use legacy interrupt\n");
@@ -587,6 +612,7 @@ module_param(intr_mode, charp, S_IRUGO);
 MODULE_PARM_DESC(intr_mode,
 "igb_uio interrupt mode (default=msix):\n"
 "    " RTE_INTR_MODE_MSIX_NAME "       Use MSIX interrupt\n"
+"    " RTE_INTR_MODE_MSI_NAME "        Use MSI interrupt\n"
 "    " RTE_INTR_MODE_LEGACY_NAME "     Use Legacy interrupt\n"
 "\n");
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions

[Ferruh Yigit]

On 9/5/2017 1:04 PM, Markus Theil wrote:
> Interrupt setup code in igb_uio has to deal with multiple
> types of interrupts and kernel versions. This patch moves
> the setup and teardown code into own functions, to make
> it more readable.
> 
> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>

Series Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>


!Not tested new added MSI interrupts.
Markus, can you please confirm you have tested MSI and can you please
provide details of the setup you have tested this on?

Thanks,
ferruh

Re: [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions

[Markus Theil]

Tested-by: Markus Theil <markus.theil@tu-ilmenau.de>

The MSI mode was tested in two setups:
1. With a custom driver I'm currently developing.
2. With i210 based APU2C4 boards and check of link status interrupt in
testpmd.

On 11.09.2017 19:48, Ferruh Yigit wrote:
> On 9/5/2017 1:04 PM, Markus Theil wrote:
>> Interrupt setup code in igb_uio has to deal with multiple
>> types of interrupts and kernel versions. This patch moves
>> the setup and teardown code into own functions, to make
>> it more readable.
>>
>> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
> Series Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
>
>
> !Not tested new added MSI interrupts.
> Markus, can you please confirm you have tested MSI and can you please
> provide details of the setup you have tested this on?
>
> Thanks,
> ferruh

-- 
Markus Theil

Technische Universität Ilmenau, Fachgebiet Telematik/Rechnernetze
Postfach 100565
98684 Ilmenau, Germany

Phone: +49 3677 69-4582
Email: markus[dot]theil[at]tu-ilmenau[dot]de
Web: http://www.tu-ilmenau.de/telematik

Re: [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions

[Ferruh Yigit]

On 9/14/2017 1:04 PM, Markus Theil wrote:
<...>
> On 11.09.2017 19:48, Ferruh Yigit wrote:
>> On 9/5/2017 1:04 PM, Markus Theil wrote:
>>> Interrupt setup code in igb_uio has to deal with multiple
>>> types of interrupts and kernel versions. This patch moves
>>> the setup and teardown code into own functions, to make
>>> it more readable.
>>>
>>> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
>> Series Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

> Tested-by: Markus Theil <markus.theil@tu-ilmenau.de>

Series applied to dpdk/master, thanks.