* [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored
@ 2017-08-21 17:33 Markus Theil
2017-08-21 17:51 ` [dpdk-dev] [PATCH v2 1/2] " Markus Theil
` (3 more replies)
0 siblings, 4 replies; 56+ messages in thread
From: Markus Theil @ 2017-08-21 17:33 UTC (permalink / raw)
To: ferruh.yigit; +Cc: dev, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v2 1/2] igb_uio: MSI IRQ mode, irq enable/disable refactored
2017-08-21 17:33 [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored Markus Theil
@ 2017-08-21 17:51 ` Markus Theil
2017-08-21 17:51 ` [dpdk-dev] [PATCH v2 2/2] igb_uio: conform to coding conventions Markus Theil
2017-08-22 16:55 ` [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored Stephen Hemminger
` (2 subsequent siblings)
3 siblings, 1 reply; 56+ messages in thread
From: Markus Theil @ 2017-08-21 17:51 UTC (permalink / raw)
To: dev; +Cc: 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v2 2/2] igb_uio: conform to coding conventions
2017-08-21 17:51 ` [dpdk-dev] [PATCH v2 1/2] " Markus Theil
@ 2017-08-21 17:51 ` Markus Theil
2017-08-22 13:28 ` [dpdk-dev] [PATCH v3] igb_uio: MSI IRQ mode, irq enable/disable refactored Markus Theil
0 siblings, 1 reply; 56+ messages in thread
From: Markus Theil @ 2017-08-21 17:51 UTC (permalink / raw)
To: dev; +Cc: 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v3] igb_uio: MSI IRQ mode, irq enable/disable refactored
2017-08-21 17:51 ` [dpdk-dev] [PATCH v2 2/2] igb_uio: conform to coding conventions Markus Theil
@ 2017-08-22 13:28 ` Markus Theil
2017-08-30 16:32 ` Ferruh Yigit
2017-08-31 10:22 ` [dpdk-dev] [PATCH v4 1/3] igb_uio: refactor irq enable/disable into own functions Markus Theil
0 siblings, 2 replies; 56+ messages in thread
From: Markus Theil @ 2017-08-22 13:28 UTC (permalink / raw)
To: dev; +Cc: 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored
2017-08-21 17:33 [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored Markus Theil
2017-08-21 17:51 ` [dpdk-dev] [PATCH v2 1/2] " Markus Theil
@ 2017-08-22 16:55 ` Stephen Hemminger
2017-08-22 18:19 ` Markus Theil
2017-08-23 8:47 ` Bruce Richardson
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Markus Theil
3 siblings, 2 replies; 56+ messages in thread
From: Stephen Hemminger @ 2017-08-22 16:55 UTC (permalink / raw)
To: Markus Theil; +Cc: ferruh.yigit, dev
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.
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored
2017-08-22 16:55 ` [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored Stephen Hemminger
@ 2017-08-22 18:19 ` Markus Theil
2017-08-23 8:47 ` Bruce Richardson
1 sibling, 0 replies; 56+ messages in thread
From: Markus Theil @ 2017-08-22 18:19 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: ferruh.yigit, dev
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.
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored
2017-08-22 16:55 ` [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored Stephen Hemminger
2017-08-22 18:19 ` Markus Theil
@ 2017-08-23 8:47 ` Bruce Richardson
2017-08-23 8:51 ` Bruce Richardson
1 sibling, 1 reply; 56+ messages in thread
From: Bruce Richardson @ 2017-08-23 8:47 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Markus Theil, ferruh.yigit, dev
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.
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored
2017-08-23 8:47 ` Bruce Richardson
@ 2017-08-23 8:51 ` Bruce Richardson
0 siblings, 0 replies; 56+ messages in thread
From: Bruce Richardson @ 2017-08-23 8:51 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Markus Theil, ferruh.yigit, dev
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v3] igb_uio: MSI IRQ mode, irq enable/disable refactored
2017-08-22 13:28 ` [dpdk-dev] [PATCH v3] igb_uio: MSI IRQ mode, irq enable/disable refactored Markus Theil
@ 2017-08-30 16:32 ` Ferruh Yigit
2017-08-31 22:05 ` Markus Theil
2017-08-31 10:22 ` [dpdk-dev] [PATCH v4 1/3] igb_uio: refactor irq enable/disable into own functions Markus Theil
1 sibling, 1 reply; 56+ messages in thread
From: Ferruh Yigit @ 2017-08-30 16:32 UTC (permalink / raw)
To: Markus Theil, dev
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;
> +}
<...>
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v4 1/3] igb_uio: refactor irq enable/disable into own functions
2017-08-22 13:28 ` [dpdk-dev] [PATCH v3] igb_uio: MSI IRQ mode, irq enable/disable refactored Markus Theil
2017-08-30 16:32 ` Ferruh Yigit
@ 2017-08-31 10:22 ` Markus Theil
2017-08-31 10:22 ` [dpdk-dev] [PATCH v4 2/3] igb_uio: fix irq disable on recent kernels Markus Theil
2017-08-31 10:22 ` [dpdk-dev] [PATCH v4 3/3] igb_uio: MSI IRQ mode Markus Theil
1 sibling, 2 replies; 56+ messages in thread
From: Markus Theil @ 2017-08-31 10:22 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v4 2/3] igb_uio: fix irq disable on recent kernels
2017-08-31 10:22 ` [dpdk-dev] [PATCH v4 1/3] igb_uio: refactor irq enable/disable into own functions Markus Theil
@ 2017-08-31 10:22 ` Markus Theil
2017-08-31 10:22 ` [dpdk-dev] [PATCH v4 3/3] igb_uio: MSI IRQ mode Markus Theil
1 sibling, 0 replies; 56+ messages in thread
From: Markus Theil @ 2017-08-31 10:22 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, Markus Theil, nicolas.dichtel
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v4 3/3] igb_uio: MSI IRQ mode
2017-08-31 10:22 ` [dpdk-dev] [PATCH v4 1/3] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-08-31 10:22 ` [dpdk-dev] [PATCH v4 2/3] igb_uio: fix irq disable on recent kernels Markus Theil
@ 2017-08-31 10:22 ` Markus Theil
2017-08-31 15:32 ` Stephen Hemminger
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
1 sibling, 2 replies; 56+ messages in thread
From: Markus Theil @ 2017-08-31 10:22 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v4 3/3] igb_uio: MSI IRQ mode
2017-08-31 10:22 ` [dpdk-dev] [PATCH v4 3/3] igb_uio: MSI IRQ mode Markus Theil
@ 2017-08-31 15:32 ` Stephen Hemminger
2017-08-31 22:07 ` Markus Theil
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
1 sibling, 1 reply; 56+ messages in thread
From: Stephen Hemminger @ 2017-08-31 15:32 UTC (permalink / raw)
To: Markus Theil; +Cc: dev, ferruh.yigit
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()?
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v5 1/5] igb_uio: refactor irq enable/disable into own functions
2017-08-31 10:22 ` [dpdk-dev] [PATCH v4 3/3] igb_uio: MSI IRQ mode Markus Theil
2017-08-31 15:32 ` Stephen Hemminger
@ 2017-08-31 21:46 ` Markus Theil
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 2/5] igb_uio: fix irq disable on recent kernels Markus Theil
` (3 more replies)
1 sibling, 4 replies; 56+ messages in thread
From: Markus Theil @ 2017-08-31 21:46 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v5 2/5] igb_uio: fix irq disable on recent kernels
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
@ 2017-08-31 21:46 ` Markus Theil
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 3/5] igb_uio: MSI IRQ mode Markus Theil
` (2 subsequent siblings)
3 siblings, 0 replies; 56+ messages in thread
From: Markus Theil @ 2017-08-31 21:46 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, Markus Theil, nicolas.dichtel
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v5 3/5] igb_uio: MSI IRQ mode
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 2/5] igb_uio: fix irq disable on recent kernels Markus Theil
@ 2017-08-31 21:46 ` Markus Theil
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections Markus Theil
2017-08-31 21:47 ` [dpdk-dev] [PATCH v5 5/5] igb_uio: release in exact reverse order Markus Theil
3 siblings, 0 replies; 56+ messages in thread
From: Markus Theil @ 2017-08-31 21:46 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 2/5] igb_uio: fix irq disable on recent kernels Markus Theil
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 3/5] igb_uio: MSI IRQ mode Markus Theil
@ 2017-08-31 21:46 ` Markus Theil
2017-09-01 15:40 ` Stephen Hemminger
` (2 more replies)
2017-08-31 21:47 ` [dpdk-dev] [PATCH v5 5/5] igb_uio: release in exact reverse order Markus Theil
3 siblings, 3 replies; 56+ messages in thread
From: Markus Theil @ 2017-08-31 21:46 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v5 5/5] igb_uio: release in exact reverse order
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
` (2 preceding siblings ...)
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections Markus Theil
@ 2017-08-31 21:47 ` Markus Theil
3 siblings, 0 replies; 56+ messages in thread
From: Markus Theil @ 2017-08-31 21:47 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v3] igb_uio: MSI IRQ mode, irq enable/disable refactored
2017-08-30 16:32 ` Ferruh Yigit
@ 2017-08-31 22:05 ` Markus Theil
0 siblings, 0 replies; 56+ messages in thread
From: Markus Theil @ 2017-08-31 22:05 UTC (permalink / raw)
To: Ferruh Yigit, dev
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.
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v4 3/3] igb_uio: MSI IRQ mode
2017-08-31 15:32 ` Stephen Hemminger
@ 2017-08-31 22:07 ` Markus Theil
0 siblings, 0 replies; 56+ messages in thread
From: Markus Theil @ 2017-08-31 22:07 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, ferruh.yigit
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.
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections Markus Theil
@ 2017-09-01 15:40 ` Stephen Hemminger
2017-09-04 10:03 ` Burakov, Anatoly
2017-09-04 12:43 ` Ferruh Yigit
2 siblings, 0 replies; 56+ messages in thread
From: Stephen Hemminger @ 2017-09-01 15:40 UTC (permalink / raw)
To: Markus Theil; +Cc: dev, ferruh.yigit
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 )
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections Markus Theil
2017-09-01 15:40 ` Stephen Hemminger
@ 2017-09-04 10:03 ` Burakov, Anatoly
2017-09-04 12:43 ` Ferruh Yigit
2 siblings, 0 replies; 56+ messages in thread
From: Burakov, Anatoly @ 2017-09-04 10:03 UTC (permalink / raw)
To: Markus Theil, dev; +Cc: Yigit, Ferruh, stephen
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections Markus Theil
2017-09-01 15:40 ` Stephen Hemminger
2017-09-04 10:03 ` Burakov, Anatoly
@ 2017-09-04 12:43 ` Ferruh Yigit
2017-09-04 15:16 ` Markus Theil
2 siblings, 1 reply; 56+ messages in thread
From: Ferruh Yigit @ 2017-09-04 12:43 UTC (permalink / raw)
To: Markus Theil, dev; +Cc: stephen
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...
<...>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections
2017-09-04 12:43 ` Ferruh Yigit
@ 2017-09-04 15:16 ` Markus Theil
2017-09-04 16:55 ` Ferruh Yigit
0 siblings, 1 reply; 56+ messages in thread
From: Markus Theil @ 2017-09-04 15:16 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: stephen
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.
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections
2017-09-04 15:16 ` Markus Theil
@ 2017-09-04 16:55 ` Ferruh Yigit
0 siblings, 0 replies; 56+ messages in thread
From: Ferruh Yigit @ 2017-09-04 16:55 UTC (permalink / raw)
To: Markus Theil, dev; +Cc: stephen
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v6 1/5] igb_uio: refactor irq enable/disable into own functions
2017-08-21 17:33 [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored Markus Theil
2017-08-21 17:51 ` [dpdk-dev] [PATCH v2 1/2] " Markus Theil
2017-08-22 16:55 ` [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored Stephen Hemminger
@ 2017-09-04 18:17 ` Markus Theil
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 2/5] igb_uio: fix irq disable on recent kernels Markus Theil
` (3 more replies)
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Markus Theil
3 siblings, 4 replies; 56+ messages in thread
From: Markus Theil @ 2017-09-04 18:17 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v6 2/5] igb_uio: fix irq disable on recent kernels
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
@ 2017-09-04 18:17 ` Markus Theil
2017-09-04 21:32 ` Ferruh Yigit
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 3/5] igb_uio: fix MSI-X IRQ assignment with new IRQ function Markus Theil
` (2 subsequent siblings)
3 siblings, 1 reply; 56+ messages in thread
From: Markus Theil @ 2017-09-04 18:17 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, Markus Theil, nicolas.dichtel
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v6 3/5] igb_uio: fix MSI-X IRQ assignment with new IRQ function
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 2/5] igb_uio: fix irq disable on recent kernels Markus Theil
@ 2017-09-04 18:17 ` Markus Theil
2017-09-04 21:34 ` Ferruh Yigit
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 4/5] igb_uio: release in exact reverse order Markus Theil
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 5/5] igb_uio: MSI IRQ mode, use kernel functions for masking MSI and MSI-X Markus Theil
3 siblings, 1 reply; 56+ messages in thread
From: Markus Theil @ 2017-09-04 18:17 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, Markus Theil, nicolas.dichtel
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v6 4/5] igb_uio: release in exact reverse order
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 2/5] igb_uio: fix irq disable on recent kernels Markus Theil
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 3/5] igb_uio: fix MSI-X IRQ assignment with new IRQ function Markus Theil
@ 2017-09-04 18:17 ` Markus Theil
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 5/5] igb_uio: MSI IRQ mode, use kernel functions for masking MSI and MSI-X Markus Theil
3 siblings, 0 replies; 56+ messages in thread
From: Markus Theil @ 2017-09-04 18:17 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v6 5/5] igb_uio: MSI IRQ mode, use kernel functions for masking MSI and MSI-X
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
` (2 preceding siblings ...)
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 4/5] igb_uio: release in exact reverse order Markus Theil
@ 2017-09-04 18:17 ` Markus Theil
2017-09-04 21:38 ` Ferruh Yigit
3 siblings, 1 reply; 56+ messages in thread
From: Markus Theil @ 2017-09-04 18:17 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v6 2/5] igb_uio: fix irq disable on recent kernels
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 2/5] igb_uio: fix irq disable on recent kernels Markus Theil
@ 2017-09-04 21:32 ` Ferruh Yigit
0 siblings, 0 replies; 56+ messages in thread
From: Ferruh Yigit @ 2017-09-04 21:32 UTC (permalink / raw)
To: Markus Theil, dev; +Cc: stephen, nicolas.dichtel
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;
<...>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v6 3/5] igb_uio: fix MSI-X IRQ assignment with new IRQ function
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 3/5] igb_uio: fix MSI-X IRQ assignment with new IRQ function Markus Theil
@ 2017-09-04 21:34 ` Ferruh Yigit
0 siblings, 0 replies; 56+ messages in thread
From: Ferruh Yigit @ 2017-09-04 21:34 UTC (permalink / raw)
To: Markus Theil, dev; +Cc: stephen, nicolas.dichtel
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;
>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v6 5/5] igb_uio: MSI IRQ mode, use kernel functions for masking MSI and MSI-X
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 5/5] igb_uio: MSI IRQ mode, use kernel functions for masking MSI and MSI-X Markus Theil
@ 2017-09-04 21:38 ` Ferruh Yigit
0 siblings, 0 replies; 56+ messages in thread
From: Ferruh Yigit @ 2017-09-04 21:38 UTC (permalink / raw)
To: Markus Theil, dev; +Cc: stephen
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions
2017-08-21 17:33 [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored Markus Theil
` (2 preceding siblings ...)
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
@ 2017-09-05 12:04 ` Markus Theil
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 2/6] igb_uio: fix irq disable on recent kernels Markus Theil
` (5 more replies)
3 siblings, 6 replies; 56+ messages in thread
From: Markus Theil @ 2017-09-05 12:04 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v7 2/6] igb_uio: fix irq disable on recent kernels
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Markus Theil
@ 2017-09-05 12:04 ` Markus Theil
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function Markus Theil
` (4 subsequent siblings)
5 siblings, 0 replies; 56+ messages in thread
From: Markus Theil @ 2017-09-05 12:04 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, Markus Theil, nicolas.dichtel
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 2/6] igb_uio: fix irq disable on recent kernels Markus Theil
@ 2017-09-05 12:04 ` Markus Theil
2017-09-11 17:56 ` Ferruh Yigit
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 4/6] igb_uio: release in exact reverse order Markus Theil
` (3 subsequent siblings)
5 siblings, 1 reply; 56+ messages in thread
From: Markus Theil @ 2017-09-05 12:04 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, Markus Theil, nicolas.dichtel
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v7 4/6] igb_uio: release in exact reverse order
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 2/6] igb_uio: fix irq disable on recent kernels Markus Theil
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function Markus Theil
@ 2017-09-05 12:04 ` Markus Theil
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X Markus Theil
` (2 subsequent siblings)
5 siblings, 0 replies; 56+ messages in thread
From: Markus Theil @ 2017-09-05 12:04 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Markus Theil
` (2 preceding siblings ...)
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 4/6] igb_uio: release in exact reverse order Markus Theil
@ 2017-09-05 12:04 ` Markus Theil
2017-10-09 21:56 ` Patrick MacArthur
2017-10-12 10:50 ` Kavanagh, Mark B
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 6/6] igb_uio: MSI IRQ mode Markus Theil
2017-09-11 17:48 ` [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Ferruh Yigit
5 siblings, 2 replies; 56+ messages in thread
From: Markus Theil @ 2017-09-05 12:04 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* [dpdk-dev] [PATCH v7 6/6] igb_uio: MSI IRQ mode
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Markus Theil
` (3 preceding siblings ...)
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X Markus Theil
@ 2017-09-05 12:04 ` Markus Theil
2017-09-11 17:48 ` [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Ferruh Yigit
5 siblings, 0 replies; 56+ messages in thread
From: Markus Theil @ 2017-09-05 12:04 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, stephen, 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
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Markus Theil
` (4 preceding siblings ...)
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 6/6] igb_uio: MSI IRQ mode Markus Theil
@ 2017-09-11 17:48 ` Ferruh Yigit
2017-09-14 12:04 ` Markus Theil
5 siblings, 1 reply; 56+ messages in thread
From: Ferruh Yigit @ 2017-09-11 17:48 UTC (permalink / raw)
To: Markus Theil, dev; +Cc: stephen
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function Markus Theil
@ 2017-09-11 17:56 ` Ferruh Yigit
2017-09-11 22:04 ` Stephen Hemminger
2017-09-12 8:16 ` Bruce Richardson
0 siblings, 2 replies; 56+ messages in thread
From: Ferruh Yigit @ 2017-09-11 17:56 UTC (permalink / raw)
To: Markus Theil, dev; +Cc: stephen, nicolas.dichtel
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;
>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function
2017-09-11 17:56 ` Ferruh Yigit
@ 2017-09-11 22:04 ` Stephen Hemminger
2017-09-12 8:14 ` Bruce Richardson
2017-09-12 8:16 ` Bruce Richardson
1 sibling, 1 reply; 56+ messages in thread
From: Stephen Hemminger @ 2017-09-11 22:04 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Markus Theil, dev, nicolas.dichtel
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;
> >
>
>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function
2017-09-11 22:04 ` Stephen Hemminger
@ 2017-09-12 8:14 ` Bruce Richardson
2017-09-12 15:01 ` Stephen Hemminger
0 siblings, 1 reply; 56+ messages in thread
From: Bruce Richardson @ 2017-09-12 8:14 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Ferruh Yigit, Markus Theil, dev, nicolas.dichtel
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;
> > >
> >
> >
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function
2017-09-11 17:56 ` Ferruh Yigit
2017-09-11 22:04 ` Stephen Hemminger
@ 2017-09-12 8:16 ` Bruce Richardson
2017-09-12 16:31 ` Kevin Traynor
1 sibling, 1 reply; 56+ messages in thread
From: Bruce Richardson @ 2017-09-12 8:16 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Markus Theil, dev, stephen, nicolas.dichtel
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function
2017-09-12 8:14 ` Bruce Richardson
@ 2017-09-12 15:01 ` Stephen Hemminger
0 siblings, 0 replies; 56+ messages in thread
From: Stephen Hemminger @ 2017-09-12 15:01 UTC (permalink / raw)
To: Bruce Richardson; +Cc: Ferruh Yigit, Markus Theil, dev, nicolas.dichtel
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;
> > > >
> > >
> > >
>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function
2017-09-12 8:16 ` Bruce Richardson
@ 2017-09-12 16:31 ` Kevin Traynor
0 siblings, 0 replies; 56+ messages in thread
From: Kevin Traynor @ 2017-09-12 16:31 UTC (permalink / raw)
To: Bruce Richardson, Ferruh Yigit
Cc: Markus Theil, dev, stephen, nicolas.dichtel
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
>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions
2017-09-11 17:48 ` [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Ferruh Yigit
@ 2017-09-14 12:04 ` Markus Theil
2017-10-06 22:05 ` Ferruh Yigit
0 siblings, 1 reply; 56+ messages in thread
From: Markus Theil @ 2017-09-14 12:04 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: stephen
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions
2017-09-14 12:04 ` Markus Theil
@ 2017-10-06 22:05 ` Ferruh Yigit
0 siblings, 0 replies; 56+ messages in thread
From: Ferruh Yigit @ 2017-10-06 22:05 UTC (permalink / raw)
To: Markus Theil, dev; +Cc: stephen
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.
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X Markus Theil
@ 2017-10-09 21:56 ` Patrick MacArthur
2017-10-12 17:04 ` Ferruh Yigit
2017-10-12 10:50 ` Kavanagh, Mark B
1 sibling, 1 reply; 56+ messages in thread
From: Patrick MacArthur @ 2017-10-09 21:56 UTC (permalink / raw)
To: Markus Theil, dev
Cc: ferruh.yigit, stephen, Bob Noseworthy, Patrick MacArthur,
O'Driscoll, Tim
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;
>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X Markus Theil
2017-10-09 21:56 ` Patrick MacArthur
@ 2017-10-12 10:50 ` Kavanagh, Mark B
2017-10-12 20:58 ` Ferruh Yigit
1 sibling, 1 reply; 56+ messages in thread
From: Kavanagh, Mark B @ 2017-10-12 10:50 UTC (permalink / raw)
To: Markus Theil, dev; +Cc: Yigit, Ferruh, stephen
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X
2017-10-09 21:56 ` Patrick MacArthur
@ 2017-10-12 17:04 ` Ferruh Yigit
2017-10-13 2:49 ` Ferruh Yigit
0 siblings, 1 reply; 56+ messages in thread
From: Ferruh Yigit @ 2017-10-12 17:04 UTC (permalink / raw)
To: Patrick MacArthur, Markus Theil, dev, Mark Kavanagh
Cc: stephen, Bob Noseworthy, Patrick MacArthur, O'Driscoll, Tim
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
>
<...>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X
2017-10-12 10:50 ` Kavanagh, Mark B
@ 2017-10-12 20:58 ` Ferruh Yigit
2017-10-12 21:15 ` Stephen Hemminger
0 siblings, 1 reply; 56+ messages in thread
From: Ferruh Yigit @ 2017-10-12 20:58 UTC (permalink / raw)
To: Kavanagh, Mark B, Markus Theil, dev; +Cc: stephen
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
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X
2017-10-12 20:58 ` Ferruh Yigit
@ 2017-10-12 21:15 ` Stephen Hemminger
2017-10-13 0:28 ` Ferruh Yigit
0 siblings, 1 reply; 56+ messages in thread
From: Stephen Hemminger @ 2017-10-12 21:15 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, Kavanagh, Mark B, Markus Theil
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
>
>
>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X
2017-10-12 21:15 ` Stephen Hemminger
@ 2017-10-13 0:28 ` Ferruh Yigit
0 siblings, 0 replies; 56+ messages in thread
From: Ferruh Yigit @ 2017-10-13 0:28 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, Kavanagh, Mark B, Markus Theil
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
>
>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X
2017-10-12 17:04 ` Ferruh Yigit
@ 2017-10-13 2:49 ` Ferruh Yigit
2017-10-13 8:28 ` Kavanagh, Mark B
0 siblings, 1 reply; 56+ messages in thread
From: Ferruh Yigit @ 2017-10-13 2:49 UTC (permalink / raw)
To: Patrick MacArthur, Markus Theil, dev, Mark Kavanagh
Cc: stephen, Bob Noseworthy, Patrick MacArthur, O'Driscoll, Tim
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
>>
> <...>
>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X
2017-10-13 2:49 ` Ferruh Yigit
@ 2017-10-13 8:28 ` Kavanagh, Mark B
0 siblings, 0 replies; 56+ messages in thread
From: Kavanagh, Mark B @ 2017-10-13 8:28 UTC (permalink / raw)
To: Yigit, Ferruh, Patrick MacArthur, Markus Theil, dev
Cc: stephen, Bob Noseworthy, Patrick MacArthur, O'Driscoll, Tim
>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
>>>
>> <...>
>>
^ permalink raw reply [flat|nested] 56+ messages in thread
end of thread, other threads:[~2017-10-13 8:28 UTC | newest]
Thread overview: 56+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-21 17:33 [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored Markus Theil
2017-08-21 17:51 ` [dpdk-dev] [PATCH v2 1/2] " Markus Theil
2017-08-21 17:51 ` [dpdk-dev] [PATCH v2 2/2] igb_uio: conform to coding conventions Markus Theil
2017-08-22 13:28 ` [dpdk-dev] [PATCH v3] igb_uio: MSI IRQ mode, irq enable/disable refactored Markus Theil
2017-08-30 16:32 ` Ferruh Yigit
2017-08-31 22:05 ` Markus Theil
2017-08-31 10:22 ` [dpdk-dev] [PATCH v4 1/3] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-08-31 10:22 ` [dpdk-dev] [PATCH v4 2/3] igb_uio: fix irq disable on recent kernels Markus Theil
2017-08-31 10:22 ` [dpdk-dev] [PATCH v4 3/3] igb_uio: MSI IRQ mode Markus Theil
2017-08-31 15:32 ` Stephen Hemminger
2017-08-31 22:07 ` Markus Theil
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 2/5] igb_uio: fix irq disable on recent kernels Markus Theil
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 3/5] igb_uio: MSI IRQ mode Markus Theil
2017-08-31 21:46 ` [dpdk-dev] [PATCH v5 4/5] igb_uio: use msi mask functions from kernel, little corrections Markus Theil
2017-09-01 15:40 ` Stephen Hemminger
2017-09-04 10:03 ` Burakov, Anatoly
2017-09-04 12:43 ` Ferruh Yigit
2017-09-04 15:16 ` Markus Theil
2017-09-04 16:55 ` Ferruh Yigit
2017-08-31 21:47 ` [dpdk-dev] [PATCH v5 5/5] igb_uio: release in exact reverse order Markus Theil
2017-08-22 16:55 ` [dpdk-dev] [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored Stephen Hemminger
2017-08-22 18:19 ` Markus Theil
2017-08-23 8:47 ` Bruce Richardson
2017-08-23 8:51 ` Bruce Richardson
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 2/5] igb_uio: fix irq disable on recent kernels Markus Theil
2017-09-04 21:32 ` Ferruh Yigit
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 3/5] igb_uio: fix MSI-X IRQ assignment with new IRQ function Markus Theil
2017-09-04 21:34 ` Ferruh Yigit
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 4/5] igb_uio: release in exact reverse order Markus Theil
2017-09-04 18:17 ` [dpdk-dev] [PATCH v6 5/5] igb_uio: MSI IRQ mode, use kernel functions for masking MSI and MSI-X Markus Theil
2017-09-04 21:38 ` Ferruh Yigit
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 2/6] igb_uio: fix irq disable on recent kernels Markus Theil
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 3/6] igb_uio: fix MSI-X IRQ assignment with new IRQ function Markus Theil
2017-09-11 17:56 ` Ferruh Yigit
2017-09-11 22:04 ` Stephen Hemminger
2017-09-12 8:14 ` Bruce Richardson
2017-09-12 15:01 ` Stephen Hemminger
2017-09-12 8:16 ` Bruce Richardson
2017-09-12 16:31 ` Kevin Traynor
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 4/6] igb_uio: release in exact reverse order Markus Theil
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 5/6] igb_uio: use kernel functions for masking MSI-X Markus Theil
2017-10-09 21:56 ` Patrick MacArthur
2017-10-12 17:04 ` Ferruh Yigit
2017-10-13 2:49 ` Ferruh Yigit
2017-10-13 8:28 ` Kavanagh, Mark B
2017-10-12 10:50 ` Kavanagh, Mark B
2017-10-12 20:58 ` Ferruh Yigit
2017-10-12 21:15 ` Stephen Hemminger
2017-10-13 0:28 ` Ferruh Yigit
2017-09-05 12:04 ` [dpdk-dev] [PATCH v7 6/6] igb_uio: MSI IRQ mode Markus Theil
2017-09-11 17:48 ` [dpdk-dev] [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Ferruh Yigit
2017-09-14 12:04 ` Markus Theil
2017-10-06 22:05 ` Ferruh Yigit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).