[dpdk-dev,v6,5/5] igb_uio: MSI IRQ mode, use kernel functions for masking MSI and MSI-X
Checks
Commit Message
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(-)
Comments
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
@@ -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
@@ -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");