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

[Ferruh Yigit <ferruh.yigit@intel.com>]

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