Re: [PATCH 10/14] cxl: Add support for interrupts on the Mellanox CX4

[Frederic Barrat <fbarrat@linux.vnet.ibm.com>]

Le 04/07/2016 15:22, Ian Munsie a écrit :
> From: Ian Munsie <imunsie@au1.ibm.com>
>
> The Mellanox CX4 in cxl mode uses a hybrid interrupt model, where
> interrupts are routed from the networking hardware to the XSL using the
> MSIX table, and from there will be transformed back into an MSIX
> interrupt using the cxl style interrupts (i.e. using IVTE entries and
> ranges to map a PE and AFU interrupt number to an MSIX address).
>
> We want to hide the implementation details of cxl interrupts as much as
> possible. To this end, we use a special version of the MSI setup &
> teardown routines in the PHB while in cxl mode to allocate the cxl
> interrupts and configure the IVTE entries in the process element.
>
> This function does not configure the MSIX table - the CX4 card uses a
> custom format in that table and it would not be appropriate to fill that
> out in generic code. The rest of the functionality is similar to the
> "Full MSI-X mode" described in the CAIA, and this could be easily
> extended to support other adapters that use that mode in the future.
>
> The interrupts will be associated with the default context. If the
> maximum number of interrupts per context has been limited (e.g. by the
> mlx5 driver), it will automatically allocate additional kernel contexts
> to associate extra interrupts as required. These contexts will be
> started using the same WED that was used to start the default context.
>
> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> ---
>   arch/powerpc/platforms/powernv/pci-cxl.c  | 84 +++++++++++++++++++++++++++++++
>   arch/powerpc/platforms/powernv/pci-ioda.c |  4 ++
>   arch/powerpc/platforms/powernv/pci.h      |  2 +
>   drivers/misc/cxl/api.c                    | 71 ++++++++++++++++++++++++++
>   drivers/misc/cxl/base.c                   | 31 ++++++++++++
>   drivers/misc/cxl/cxl.h                    |  4 ++
>   drivers/misc/cxl/main.c                   |  2 +
>   include/misc/cxl-base.h                   |  4 ++
>   8 files changed, 202 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-cxl.c b/arch/powerpc/platforms/powernv/pci-cxl.c
> index 2f386f5..1559ca2 100644
> --- a/arch/powerpc/platforms/powernv/pci-cxl.c
> +++ b/arch/powerpc/platforms/powernv/pci-cxl.c
> @@ -8,6 +8,7 @@
>    */
>
>   #include <linux/module.h>
> +#include <linux/msi.h>
>   #include <asm/pci-bridge.h>
>   #include <asm/pnv-pci.h>
>   #include <asm/opal.h>
> @@ -273,3 +274,86 @@ void pnv_cxl_disable_device(struct pci_dev *dev)
>   	cxl_pci_disable_device(dev);
>   	cxl_afu_put(afu);
>   }
> +
> +/*
> + * This is a special version of pnv_setup_msi_irqs for cards in cxl mode. This
> + * function handles setting up the IVTE entries for the XSL to use.
> + *
> + * We are currently not filling out the MSIX table, since the only currently
> + * supported adapter (CX4) uses a custom MSIX table format in cxl mode and it
> + * is up to their driver to fill that out. In the future we may fill out the
> + * MSIX table (and change the IVTE entries to be an index to the MSIX table)
> + * for adapters implementing the Full MSI-X mode described in the CAIA.
> + */
> +int pnv_cxl_cx4_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
> +{
> +	struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> +	struct pnv_phb *phb = hose->private_data;
> +	struct msi_desc *entry;
> +	struct cxl_context *ctx = NULL;
> +	unsigned int virq;
> +	int hwirq;
> +	int afu_irq = 0;
> +	int rc;
> +
> +	if (WARN_ON(!phb) || !phb->msi_bmp.bitmap)
> +		return -ENODEV;
> +
> +	if (pdev->no_64bit_msi && !phb->msi32_support)
> +		return -ENODEV;
> +
> +	rc = cxl_cx4_setup_msi_irqs(pdev, nvec, type);
> +	if (rc)
> +		return rc;
> +
> +	for_each_pci_msi_entry(entry, pdev) {
> +		if (!entry->msi_attrib.is_64 && !phb->msi32_support) {
> +			pr_warn("%s: Supports only 64-bit MSIs\n",
> +				pci_name(pdev));
> +			return -ENXIO;
> +		}
> +
> +		hwirq = cxl_next_msi_hwirq(pdev, &ctx, &afu_irq);
> +		if (WARN_ON(hwirq < 0))
> +			return hwirq;

I think we want:
	if (WARN_ON(hwirq <= 0))
cxl_find_afu_irq() returns 0 if doesn't find the irq, which is not 
supposed to happen here.

   Fred