All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: Markus Theil <markus.theil@tu-ilmenau.de>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "Yigit, Ferruh" <ferruh.yigit@intel.com>,
	"stephen@networkplumber.org" <stephen@networkplumber.org>
Subject: Re: [PATCH v5 4/5] igb_uio: use msi mask functions from	kernel, little corrections
Date: Mon, 4 Sep 2017 10:03:58 +0000	[thread overview]
Message-ID: <C6ECDF3AB251BE4894318F4E4512369782293695@IRSMSX109.ger.corp.intel.com> (raw)
In-Reply-To: <1504216020-16067-4-git-send-email-markus.theil@tu-ilmenau.de>

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

  parent reply	other threads:[~2017-09-04 10:04 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-21 17:33 [PATCH] igb_uio: MSI IRQ mode, irq enable/disable refactored Markus Theil
2017-08-21 17:51 ` [PATCH v2 1/2] " Markus Theil
2017-08-21 17:51   ` [PATCH v2 2/2] igb_uio: conform to coding conventions Markus Theil
2017-08-22 13:28     ` [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       ` [PATCH v4 1/3] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-08-31 10:22         ` [PATCH v4 2/3] igb_uio: fix irq disable on recent kernels Markus Theil
2017-08-31 10:22         ` [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           ` [PATCH v5 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-08-31 21:46             ` [PATCH v5 2/5] igb_uio: fix irq disable on recent kernels Markus Theil
2017-08-31 21:46             ` [PATCH v5 3/5] igb_uio: MSI IRQ mode Markus Theil
2017-08-31 21:46             ` [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 [this message]
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             ` [PATCH v5 5/5] igb_uio: release in exact reverse order Markus Theil
2017-08-22 16:55 ` [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 ` [PATCH v6 1/5] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-09-04 18:17   ` [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   ` [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   ` [PATCH v6 4/5] igb_uio: release in exact reverse order Markus Theil
2017-09-04 18:17   ` [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 ` [PATCH v7 1/6] igb_uio: refactor irq enable/disable into own functions Markus Theil
2017-09-05 12:04   ` [PATCH v7 2/6] igb_uio: fix irq disable on recent kernels Markus Theil
2017-09-05 12:04   ` [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   ` [PATCH v7 4/6] igb_uio: release in exact reverse order Markus Theil
2017-09-05 12:04   ` [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   ` [PATCH v7 6/6] igb_uio: MSI IRQ mode Markus Theil
2017-09-11 17:48   ` [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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=C6ECDF3AB251BE4894318F4E4512369782293695@IRSMSX109.ger.corp.intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=markus.theil@tu-ilmenau.de \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.