[V8 0/2] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

[V8 0/2] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

[suravee.suthikulpanit at amd.com]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch set introduces support for MSI(-X) in GICv2m specification,
which is implemented in some variation of GIC400.

This depends on and has been tested with the following patch set which
implements PCI supports for ARM64:

    [PATCH v11 00/10] Support for creating generic PCI host bridges from DT
    https://lkml.org/lkml/2014/9/17/732

    [PATCH v11] Add support for PCI in AArch64
    https://lkml.org/lkml/2014/9/17/736

This patch set is rebased from:
    git://git.infradead.org/users/jcooper/linux.git irqchip/core
 
Changes in V8:
    * Minor clean up suggested by Marc
    * Acked-by: Marc Zyngier <marc.zyngier@arm.com>

Changes in V7:
    * Fix error handling logic in gicv2m_of_init() and gicv2m_init_one().
      per Marc suggestions.
    * Restructure the patch to integrate the multi-MSI support for V2m
      into the patch 2/2.
    * Introduce "arm,gic-v2m-frame" compatible ID for the v2m DT binding.
    * Introduce "arm,msi-base-spi" and "arm,msi-num-spi" property in the
      v2m DT binding for overwriting value in MSI_TYPER register.
    * Add irq-gic-v2m.c: is_msi_spi_valid() to validate the SPI base and
      number of SPIs.
    * Fix various comments from Marc (Many thanks).
    * Add the missing CONFIG_ARM_GICV2M (per Marcin Juszkiewicz comment)

Suravee Suthikulpanit (2):
  irqchip: gic: Add support for multiple MSI for ARM64
  irqchip: gicv2m: Add supports for ARM GICv2m MSI(-X)

 Documentation/devicetree/bindings/arm/gic.txt |  55 ++++
 arch/arm64/Kconfig                            |   1 +
 arch/arm64/kernel/Makefile                    |   1 +
 arch/arm64/kernel/msi.c                       |  41 +++
 drivers/irqchip/Kconfig                       |   5 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-gic-common.c              |  12 +
 drivers/irqchip/irq-gic-common.h              |   4 +
 drivers/irqchip/irq-gic-v2m.c                 | 356 ++++++++++++++++++++++++++
 drivers/irqchip/irq-gic.c                     |  82 +++---
 drivers/irqchip/irq-gic.h                     |  54 ++++
 11 files changed, 582 insertions(+), 30 deletions(-)
 create mode 100644 arch/arm64/kernel/msi.c
 create mode 100644 drivers/irqchip/irq-gic-v2m.c
 create mode 100644 drivers/irqchip/irq-gic.h

-- 
1.9.3

[V8 1/2] irqchip: gic: Add support for multiple MSI for ARM64

[suravee.suthikulpanit at amd.com]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch implelments the ARM64 version of arch_setup_msi_irqs(),
which does not return 1 for when PCI_CAP_ID_MSI and nvec > 1.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Acked-by: Marc Zyngier <Marc.Zyngier@arm.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Catalin Marinas <Catalin.Marinas@arm.com>
Cc: Will Deacon <Will.Deacon@arm.com>
---
 arch/arm64/kernel/Makefile |  1 +
 arch/arm64/kernel/msi.c    | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)
 create mode 100644 arch/arm64/kernel/msi.c

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index df7ef87..a921c42 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -29,6 +29,7 @@ arm64-obj-$(CONFIG_ARM64_CPU_SUSPEND)	+= sleep.o suspend.o
 arm64-obj-$(CONFIG_JUMP_LABEL)		+= jump_label.o
 arm64-obj-$(CONFIG_KGDB)		+= kgdb.o
 arm64-obj-$(CONFIG_EFI)			+= efi.o efi-stub.o efi-entry.o
+arm64-obj-$(CONFIG_PCI_MSI)		+= msi.o
 
 obj-y					+= $(arm64-obj-y) vdso/
 obj-m					+= $(arm64-obj-m)
diff --git a/arch/arm64/kernel/msi.c b/arch/arm64/kernel/msi.c
new file mode 100644
index 0000000..a295862
--- /dev/null
+++ b/arch/arm64/kernel/msi.c
@@ -0,0 +1,41 @@
+/*
+ * ARM64 architectural MSI implemention
+ *
+ * Support for Message Signalelled Interrupts for systems that
+ * implement ARM Generic Interrupt Controller: GICv2m.
+ *
+ * Copyright (C) 2014 Advanced Micro Devices, Inc.
+ * Authors: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <linux/irq.h>
+#include <linux/msi.h>
+#include <linux/pci.h>
+
+/*
+ * ARM64 function for seting up MSI irqs.
+ * Based on driver/pci/msi.c: arch_setup_msi_irqs().
+ *
+ * Note:
+ * Current implementation assumes that all interrupt controller used in
+ * ARM64 architecture _MUST_ supports multi-MSI.
+ */
+int arm64_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+{
+	struct msi_desc *entry;
+	int ret;
+
+	list_for_each_entry(entry, &dev->msi_list, list) {
+		ret = arch_setup_msi_irq(dev, entry);
+		if (ret < 0)
+			return ret;
+		if (ret > 0)
+			return -ENOSPC;
+	}
+
+	return 0;
+}
-- 
1.9.3

[V8 2/2] irqchip: gicv2m: Add supports for ARM GICv2m MSI(-X)

[suravee.suthikulpanit at amd.com]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

ARM GICv2m specification extends GICv2 to support MSI(-X) with
a new set of register frame. This patch introduces support for
the non-secure GICv2m register frame. Currently, GICV2m is available
in certain version of GIC-400.

The patch introduces a new property in ARM gic binding, the v2m subnode.
It is optional.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Acked-by: Marc Zyngier <Marc.Zyngier@arm.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Catalin Marinas <Catalin.Marinas@arm.com>
Cc: Will Deacon <Will.Deacon@arm.com>
---
 Documentation/devicetree/bindings/arm/gic.txt |  55 ++++
 arch/arm64/Kconfig                            |   1 +
 drivers/irqchip/Kconfig                       |   5 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-gic-common.c              |  12 +
 drivers/irqchip/irq-gic-common.h              |   4 +
 drivers/irqchip/irq-gic-v2m.c                 | 356 ++++++++++++++++++++++++++
 drivers/irqchip/irq-gic.c                     |  82 +++---
 drivers/irqchip/irq-gic.h                     |  54 ++++
 9 files changed, 540 insertions(+), 30 deletions(-)
 create mode 100644 drivers/irqchip/irq-gic-v2m.c
 create mode 100644 drivers/irqchip/irq-gic.h

diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
index c7d2fa1..38b2156 100644
--- a/Documentation/devicetree/bindings/arm/gic.txt
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -96,3 +96,58 @@ Example:
 		      <0x2c006000 0x2000>;
 		interrupts = <1 9 0xf04>;
 	};
+
+
+* GICv2m extension for MSI/MSI-x support (Optional)
+
+Certain revision of GIC-400 supports MSI/MSI-x via V2M register frame.
+This is enabled by specifying v2m sub-node.
+
+Required properties:
+
+- compatible        : The value here should be "arm,gic-v2m-frame".
+
+- msi-controller    : Identifies the node as an MSI controller.
+
+- reg               : GICv2m MSI interface register base and size
+
+Optional properties:
+
+- arm,msi-base-spi  : Specify base SPI the MSI frame.
+                      The SPI base value can be from 32 to 1019.
+
+- arm,msi-num-spi   : Returns the number of contiguous SPIs assigned
+		      to the frame.
+
+Note: "arm,msi-base-spi" and "arm,msi-num-spi" are used mainly for
+       providing HW workaround in the case where the MSI_TYPER register
+       is corrupted.
+
+Example:
+
+	interrupt-controller at e1101000 {
+		compatible = "arm,gic-400";
+		#interrupt-cells = <3>;
+		#address-cells = <2>;
+		#size-cells = <2>;
+		interrupt-controller;
+		interrupts = <1 8 0xf04>;
+		ranges = <0 0 0 0xe1100000 0 0x100000>;
+		reg = <0x0 0xe1110000 0 0x01000>,
+		      <0x0 0xe112f000 0 0x02000>,
+		      <0x0 0xe1140000 0 0x10000>,
+		      <0x0 0xe1160000 0 0x10000>;
+		v2m0: v2m at 0x8000 {
+			compatible = "arm,gic-v2m-frame";
+			msi-controller;
+			reg = <0x0 0x80000 0 0x1000>;
+		};
+
+		....
+
+		v2mN: v2m at 0x9000 {
+			compatible = "arm,gic-v2m-frame";
+			msi-controller;
+			reg = <0x0 0x90000 0 0x1000>;
+		};
+	};
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index fd4e81a..83d5556 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -12,6 +12,7 @@ config ARM64
 	select ARM_ARCH_TIMER
 	select ARM_GIC
 	select AUDIT_ARCH_COMPAT_GENERIC
+	select ARM_GIC_V2M
 	select ARM_GIC_V3
 	select BUILDTIME_EXTABLE_SORT
 	select CLONE_BACKWARDS
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index b8632bf..61d18d9 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -7,6 +7,11 @@ config ARM_GIC
 	select IRQ_DOMAIN
 	select MULTI_IRQ_HANDLER
 
+config ARM_GIC_V2M
+	bool
+	depends on ARM_GIC
+	depends on PCI && PCI_MSI
+
 config GIC_NON_BANKED
 	bool
 
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 73052ba..3bda951 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_ARCH_SUNXI)		+= irq-sun4i.o
 obj-$(CONFIG_ARCH_SUNXI)		+= irq-sunxi-nmi.o
 obj-$(CONFIG_ARCH_SPEAR3XX)		+= spear-shirq.o
 obj-$(CONFIG_ARM_GIC)			+= irq-gic.o irq-gic-common.o
+obj-$(CONFIG_ARM_GIC_V2M)		+= irq-gic-v2m.o
 obj-$(CONFIG_ARM_GIC_V3)		+= irq-gic-v3.o irq-gic-common.o
 obj-$(CONFIG_ARM_NVIC)			+= irq-nvic.o
 obj-$(CONFIG_ARM_VIC)			+= irq-vic.o
diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index 60ac704..4b8cff2 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -113,3 +113,15 @@ void gic_cpu_config(void __iomem *base, void (*sync_access)(void))
 	if (sync_access)
 		sync_access();
 }
+
+int gic_msi_get_vec_count(struct pci_dev *pdev, struct msi_desc *desc)
+{
+	int ret = -EINVAL;
+#ifdef CONFIG_PCI_MSI
+	if (desc->msi_attrib.is_msix)
+		ret = pci_msix_vec_count(pdev);
+	else
+		ret = pci_msi_vec_count(pdev);
+#endif
+	return ret;
+}
diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
index b41f024..95049e5 100644
--- a/drivers/irqchip/irq-gic-common.h
+++ b/drivers/irqchip/irq-gic-common.h
@@ -19,6 +19,8 @@
 
 #include <linux/of.h>
 #include <linux/irqdomain.h>
+#include <linux/pci.h>
+#include <linux/msi.h>
 
 void gic_configure_irq(unsigned int irq, unsigned int type,
                        void __iomem *base, void (*sync_access)(void));
@@ -26,4 +28,6 @@ void gic_dist_config(void __iomem *base, int gic_irqs,
 		     void (*sync_access)(void));
 void gic_cpu_config(void __iomem *base, void (*sync_access)(void));
 
+int gic_msi_get_vec_count(struct pci_dev *pdev, struct msi_desc *desc);
+
 #endif /* _IRQ_GIC_COMMON_H */
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
new file mode 100644
index 0000000..554b1b1
--- /dev/null
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -0,0 +1,356 @@
+/*
+ * ARM GIC v2m MSI(-X) support
+ * Support for Message Signaled Interrupts for systems that
+ * implement ARM Generic Interrupt Controller: GICv2m.
+ *
+ * Copyright (C) 2014 Advanced Micro Devices, Inc.
+ * Authors: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
+ *          Harish Kasiviswanathan <harish.kasiviswanathan@amd.com>
+ *          Brandon Anderson <brandon.anderson@amd.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) "GICv2m: " fmt
+
+#include <linux/bitmap.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_pci.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include "irqchip.h"
+#include "irq-gic.h"
+#include "irq-gic-common.h"
+
+/*
+* MSI_TYPER:
+*     [31:26] Reserved
+*     [25:16] lowest SPI assigned to MSI
+*     [15:10] Reserved
+*     [9:0]   Numer of SPIs assigned to MSI
+*/
+#define V2M_MSI_TYPER			0x008
+#define V2M_MSI_TYPER_BASE_SHIFT	16
+#define V2M_MSI_TYPER_BASE_MASK		0x3FF
+#define V2M_MSI_TYPER_NUM_MASK		0x3FF
+#define V2M_MSI_SETSPI_NS		0x040
+#define V2M_MIN_SPI			32
+#define V2M_MAX_SPI			1019
+
+#define V2M_MSI_TYPER_BASE_SPI(x)	\
+		(((x) >> V2M_MSI_TYPER_BASE_SHIFT) & V2M_MSI_TYPER_BASE_MASK)
+
+#define V2M_MSI_TYPER_NUM_SPI(x)	((x) & V2M_MSI_TYPER_NUM_MASK)
+
+/*
+ * alloc_msi_irq - Allocate MSIs from available MSI bitmap.
+ * @data: Pointer to v2m_data
+ * @nvec: Number of interrupts to allocate
+ * @irq: Pointer to the allocated irq
+ *
+ * Allocates interrupts only if the contiguous range of MSIs
+ * with specified nvec are available. Otherwise return the number
+ * of available interrupts. If none are available, then returns -ENOENT.
+ */
+static int alloc_msi_irq(struct v2m_data *data, int nvec, int *irq)
+{
+	int size = data->nr_spis;
+	int next = size, i = nvec, ret;
+
+	/* We should never allocate more than available nr_spis */
+	if (i >= size)
+		i = size;
+
+	spin_lock(&data->msi_cnt_lock);
+
+	for (; i > 0; i--) {
+		next = bitmap_find_next_zero_area(data->bm,
+					size, 0, i, 0);
+		if (next < size)
+			break;
+	}
+
+	if (i != nvec) {
+		ret = i ? : -ENOENT;
+	} else {
+		bitmap_set(data->bm, next, nvec);
+		*irq = data->spi_start + next;
+		ret = 0;
+	}
+
+	spin_unlock(&data->msi_cnt_lock);
+
+	return ret;
+}
+
+static void gicv2m_teardown_msi_irq(struct msi_chip *chip, unsigned int irq)
+{
+	int pos;
+	struct v2m_data *data = container_of(chip, struct v2m_data, msi_chip);
+
+	spin_lock(&data->msi_cnt_lock);
+
+	pos = irq - data->spi_start;
+	if (pos >= 0 && pos < data->nr_spis)
+		bitmap_clear(data->bm, pos, 1);
+
+	spin_unlock(&data->msi_cnt_lock);
+}
+
+bool gicv2m_check_msi_range(struct gic_chip_data *gic, irq_hw_number_t hw)
+{
+	struct v2m_data *v2m = NULL;
+
+	list_for_each_entry(v2m, &gic->v2m_list, list) {
+		if (hw >= v2m->spi_start &&
+		    hw <  v2m->spi_start + v2m->nr_spis)
+			return true;
+	}
+	return false;
+}
+
+static int gicv2m_setup_msi_irq(struct msi_chip *chip,
+				struct pci_dev *pdev,
+				struct msi_desc *desc)
+{
+	int i, irq, nvec, avail;
+	struct msi_msg msg;
+	phys_addr_t addr;
+	struct msi_desc *entry;
+	struct v2m_data *data = container_of(chip, struct v2m_data, msi_chip);
+
+	if (!desc) {
+		dev_err(&pdev->dev,
+			"MSI setup failed. Invalid msi descriptor\n");
+		return -EINVAL;
+	}
+
+	if (desc->msi_attrib.is_msix) {
+		/**
+		 * For MSIx:
+		 * We allocate one irq at a time
+		 */
+		avail = alloc_msi_irq(data, 1, &irq);
+		if (avail != 0) {
+			dev_err(&pdev->dev,
+				"MSI setup failed. Cannnot allocate IRQ\n");
+			return -ENOSPC;
+		}
+
+		irq_set_chip_data(irq, chip);
+		irq_set_msi_desc(irq, desc);
+		irq_set_irq_type(irq, IRQ_TYPE_EDGE_RISING);
+	} else {
+		/**
+		 * For MSI and Multi-MSI:
+		 * All requested irqs are allocated and setup at
+		 * once. Subsequent calls to this function would simply return
+		 * success. This is to avoid having to implement a separate
+		 * function for setting up multiple irqs.
+		 */
+		BUG_ON(list_empty(&pdev->msi_list));
+		WARN_ON(!list_is_singular(&pdev->msi_list));
+
+		nvec = gic_msi_get_vec_count(pdev, desc);
+		if (WARN_ON(nvec <= 0))
+			return nvec;
+
+		entry = list_first_entry(&pdev->msi_list,
+					 struct msi_desc, list);
+
+		if ((nvec > 1) && (entry->msi_attrib.multiple))
+			return 0;
+
+		avail = alloc_msi_irq(data, nvec, &irq);
+		if (avail != 0) {
+			dev_err(&pdev->dev,
+				"Failed to allocate %d irqs.\n", nvec);
+			return avail;
+		}
+
+		if (nvec > 1) {
+			/* Set lowest of the new interrupts assigned
+			 * to the PCI device
+			 */
+			entry->nvec_used = nvec;
+			entry->msi_attrib.multiple = ilog2(
+						__roundup_pow_of_two(nvec));
+		}
+
+		for (i = 0; i < nvec; i++) {
+			irq_set_chip_data(irq+i, chip);
+			if (irq_set_msi_desc_off(irq, i, entry)) {
+				dev_err(&pdev->dev,
+					"Failed to set up MSI irq %d\n",
+					(irq+i));
+				return -EINVAL;
+			}
+
+			irq_set_irq_type((irq+i), IRQ_TYPE_EDGE_RISING);
+		}
+	}
+
+	addr = data->res.start + V2M_MSI_SETSPI_NS;
+	msg.address_hi = (u32)(addr >> 32);
+	msg.address_lo = (u32)(addr);
+	msg.data = irq;
+	write_msi_msg(irq, &msg);
+
+	return 0;
+}
+
+static void gicv2m_mask_irq(struct irq_data *d)
+{
+	gic_mask_irq(d);
+	if (d->msi_desc)
+		mask_msi_irq(d);
+}
+
+static void gicv2m_unmask_irq(struct irq_data *d)
+{
+	gic_unmask_irq(d);
+	if (d->msi_desc)
+		unmask_msi_irq(d);
+}
+
+static bool is_msi_spi_valid(u32 base, u32 num)
+{
+	if (base < V2M_MIN_SPI) {
+		pr_err("Invalid MSI base SPI (base:%u)\n", base);
+		return false;
+	}
+
+	if ((num == 0) || (base + num > V2M_MAX_SPI)) {
+		pr_err("Number of SPIs (%u) exceed maximum (%u)\n",
+		       num, V2M_MAX_SPI - V2M_MIN_SPI + 1);
+		return false;
+	}
+
+	return true;
+}
+
+static int __init
+gicv2m_init_one(struct device_node *node, struct v2m_data **v,
+		struct gic_chip_data *gic)
+{
+	int ret;
+	struct v2m_data *v2m = NULL;
+
+	*v = kzalloc(sizeof(struct v2m_data), GFP_KERNEL);
+	if (!*v) {
+		pr_err("Failed to allocate struct v2m_data.\n");
+		return -ENOMEM;
+	}
+
+	v2m = *v;
+	v2m->gic = gic;
+	v2m->msi_chip.owner = THIS_MODULE;
+	v2m->msi_chip.of_node = node;
+	v2m->msi_chip.setup_irq = gicv2m_setup_msi_irq;
+	v2m->msi_chip.teardown_irq = gicv2m_teardown_msi_irq;
+	ret = of_address_to_resource(node, 0, &v2m->res);
+	if (ret) {
+		pr_err("Failed to allocate v2m resource.\n");
+		goto err_out;
+	}
+
+	v2m->base = ioremap(v2m->res.start, resource_size(&v2m->res));
+	if (!v2m->base) {
+		pr_err("Failed to map GICv2m resource\n");
+		ret = -EINVAL;
+		goto err_out;
+	}
+
+	ret = of_pci_msi_chip_add(&v2m->msi_chip);
+	if (ret) {
+		pr_info("Failed to add msi_chip.\n");
+		goto err_out;
+	}
+
+	if (!of_property_read_u32(node, "arm,msi-base-spi", &v2m->spi_start) &&
+	    !of_property_read_u32(node, "arm,msi-num-spi", &v2m->nr_spis)) {
+		pr_info("Overriding V2M MSI_TYPER (base:%u, num:%u)\n",
+			v2m->spi_start, v2m->nr_spis);
+	} else {
+		u32 typer = readl_relaxed(v2m->base + V2M_MSI_TYPER);
+
+		v2m->spi_start = V2M_MSI_TYPER_BASE_SPI(typer);
+		v2m->nr_spis = V2M_MSI_TYPER_NUM_SPI(typer);
+	}
+
+	if (!is_msi_spi_valid(v2m->spi_start, v2m->nr_spis)) {
+		ret = -EINVAL;
+		goto err_out;
+	}
+
+	v2m->bm = kzalloc(sizeof(long) * BITS_TO_LONGS(v2m->nr_spis),
+			  GFP_KERNEL);
+	if (!v2m->bm) {
+		pr_err("Failed to allocate MSI bitmap\n");
+		ret = -ENOMEM;
+		goto err_out;
+	}
+
+	spin_lock_init(&v2m->msi_cnt_lock);
+
+	pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
+		(unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
+		v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
+
+	return 0;
+err_out:
+	of_pci_msi_chip_remove(&v2m->msi_chip);
+	if (v2m->base)
+		iounmap(v2m->base);
+	kfree(v2m);
+	return ret;
+}
+
+int __init gicv2m_of_init(struct device_node *node,
+			  struct gic_chip_data *gic,
+			  struct irq_chip *v2m_chip)
+{
+	int ret = 0;
+	struct v2m_data *v2m;
+	struct device_node *child = NULL;
+
+	INIT_LIST_HEAD(&gic->v2m_list);
+
+	v2m_chip->irq_mask = gicv2m_mask_irq;
+	v2m_chip->irq_unmask = gicv2m_unmask_irq;
+
+	for (;;) {
+		child = of_get_next_child(node, child);
+		if (!child)
+			break;
+
+		if (!of_device_is_compatible(child, "arm,gic-v2m-frame"))
+			continue;
+
+		if (!of_find_property(child, "msi-controller", NULL))
+			continue;
+
+		ret = gicv2m_init_one(child, &v2m, gic);
+		if (ret) {
+			of_node_put(node);
+			break;
+		}
+
+		list_add_tail(&v2m->list, &gic->v2m_list);
+	}
+
+	if (ret && list_empty(&gic->v2m_list)) {
+		pr_warn("Warning: Failed to enable GICv2m support.\n");
+		return -EINVAL;
+	}
+
+	return ret;
+}
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 4b959e6..9f8e1e0 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -46,30 +46,9 @@
 #include <asm/smp_plat.h>
 
 #include "irq-gic-common.h"
+#include "irq-gic.h"
 #include "irqchip.h"
 
-union gic_base {
-	void __iomem *common_base;
-	void __percpu * __iomem *percpu_base;
-};
-
-struct gic_chip_data {
-	union gic_base dist_base;
-	union gic_base cpu_base;
-#ifdef CONFIG_CPU_PM
-	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
-	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
-	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
-	u32 __percpu *saved_ppi_enable;
-	u32 __percpu *saved_ppi_conf;
-#endif
-	struct irq_domain *domain;
-	unsigned int gic_irqs;
-#ifdef CONFIG_GIC_NON_BANKED
-	void __iomem *(*get_base)(union gic_base *);
-#endif
-};
-
 static DEFINE_RAW_SPINLOCK(irq_controller_lock);
 
 /*
@@ -131,15 +110,36 @@ static inline void gic_set_base_accessor(struct gic_chip_data *data,
 #define gic_set_base_accessor(d, f)
 #endif
 
+static inline
+struct gic_chip_data *irq_data_get_gic_chip_data(struct irq_data *d)
+{
+	struct gic_chip_data *gic_data;
+	struct msi_chip *mchip;
+	struct v2m_data *v2mdat;
+
+	/*
+	 * For MSI, irq_data.chip_data points to struct msi_chip.
+	 * For non-MSI, irq_data.chip_data points to struct gic_chip_data.
+	 */
+	if (d->msi_desc) {
+		mchip = irq_data_get_irq_chip_data(d);
+		v2mdat = container_of(mchip, struct v2m_data, msi_chip);
+		gic_data = v2mdat->gic;
+	} else {
+		gic_data = irq_data_get_irq_chip_data(d);
+	}
+	return gic_data;
+}
+
 static inline void __iomem *gic_dist_base(struct irq_data *d)
 {
-	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
+	struct gic_chip_data *gic_data = irq_data_get_gic_chip_data(d);
 	return gic_data_dist_base(gic_data);
 }
 
 static inline void __iomem *gic_cpu_base(struct irq_data *d)
 {
-	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
+	struct gic_chip_data *gic_data = irq_data_get_gic_chip_data(d);
 	return gic_data_cpu_base(gic_data);
 }
 
@@ -151,7 +151,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
 /*
  * Routines to acknowledge, disable and enable interrupts
  */
-static void gic_mask_irq(struct irq_data *d)
+void gic_mask_irq(struct irq_data *d)
 {
 	u32 mask = 1 << (gic_irq(d) % 32);
 
@@ -162,7 +162,7 @@ static void gic_mask_irq(struct irq_data *d)
 	raw_spin_unlock(&irq_controller_lock);
 }
 
-static void gic_unmask_irq(struct irq_data *d)
+void gic_unmask_irq(struct irq_data *d)
 {
 	u32 mask = 1 << (gic_irq(d) % 32);
 
@@ -325,6 +325,15 @@ static struct irq_chip gic_chip = {
 	.irq_set_wake		= gic_set_wake,
 };
 
+static struct irq_chip v2m_chip = {
+	.name			= "GICv2m",
+	.irq_eoi		= gic_eoi_irq,
+	.irq_set_type		= gic_set_type,
+#ifdef CONFIG_SMP
+	.irq_set_affinity	= gic_set_affinity,
+#endif
+};
+
 void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
 {
 	if (gic_nr >= MAX_GIC_NR)
@@ -767,19 +776,29 @@ void __init gic_init_physaddr(struct device_node *node)
 static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
 				irq_hw_number_t hw)
 {
+	struct gic_chip_data *gic = d->host_data;
+
+	irq_set_chip_data(irq, gic);
+
 	if (hw < 32) {
+		/* PPIs */
 		irq_set_percpu_devid(irq);
 		irq_set_chip_and_handler(irq, &gic_chip,
 					 handle_percpu_devid_irq);
 		set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
 	} else {
-		irq_set_chip_and_handler(irq, &gic_chip,
-					 handle_fasteoi_irq);
+		/* SPIs */
 		set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
 
-		gic_routable_irq_domain_ops->map(d, irq, hw);
+		if (!gicv2m_check_msi_range(gic, hw)) {
+			irq_set_chip_and_handler(irq, &gic_chip,
+						 handle_fasteoi_irq);
+			gic_routable_irq_domain_ops->map(d, irq, hw);
+		} else {
+			irq_set_chip_and_handler(irq, &v2m_chip,
+					 handle_fasteoi_irq);
+		}
 	}
-	irq_set_chip_data(irq, d->host_data);
 	return 0;
 }
 
@@ -1010,6 +1029,9 @@ gic_of_init(struct device_node *node, struct device_node *parent)
 	if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
 		percpu_offset = 0;
 
+	if (IS_ENABLED(CONFIG_ARM_GIC_V2M))
+		gicv2m_of_init(node, &gic_data[gic_cnt], &v2m_chip);
+
 	gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
 	if (!gic_cnt)
 		gic_init_physaddr(node);
diff --git a/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
new file mode 100644
index 0000000..3021665
--- /dev/null
+++ b/drivers/irqchip/irq-gic.h
@@ -0,0 +1,54 @@
+#ifndef _IRQ_GIC_H_
+#define _IRQ_GIC_H_
+
+#include <linux/msi.h>
+#include <linux/pci.h>
+
+union gic_base {
+	void __iomem *common_base;
+	void __percpu * __iomem *percpu_base;
+};
+
+struct gic_chip_data;
+
+struct v2m_data {
+#ifdef CONFIG_ARM_GIC_V2M
+	struct list_head list;
+	spinlock_t msi_cnt_lock;
+	struct msi_chip msi_chip;
+	struct resource res;      /* GICv2m resource */
+	void __iomem *base;       /* GICv2m virt address */
+	unsigned int spi_start;   /* The SPI number that MSIs start */
+	unsigned int nr_spis;     /* The number of SPIs for MSIs */
+	unsigned long *bm;        /* MSI vector bitmap */
+	struct gic_chip_data *gic;
+#endif
+};
+
+struct gic_chip_data {
+	union gic_base dist_base;
+	union gic_base cpu_base;
+#ifdef CONFIG_CPU_PM
+	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
+	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
+	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
+	u32 __percpu *saved_ppi_enable;
+	u32 __percpu *saved_ppi_conf;
+#endif
+	struct irq_domain *domain;
+	unsigned int gic_irqs;
+#ifdef CONFIG_GIC_NON_BANKED
+	void __iomem *(*get_base)(union gic_base *);
+#endif
+#ifdef CONFIG_ARM_GIC_V2M
+	struct list_head v2m_list;
+#endif
+};
+
+void gic_mask_irq(struct irq_data *d);
+void gic_unmask_irq(struct irq_data *d);
+int gicv2m_of_init(struct device_node *node, struct gic_chip_data *gic,
+		   struct irq_chip *v2m_chip) __init;
+bool gicv2m_check_msi_range(struct gic_chip_data *gic, irq_hw_number_t hw);
+
+#endif /* _IRQ_GIC_H_ */
-- 
1.9.3

[V8 1/2] irqchip: gic: Add support for multiple MSI for ARM64

[Will Deacon]

Hi Suravee,

Just a few minor comments.

On Sat, Sep 20, 2014 at 05:31:37PM +0100, suravee.suthikulpanit at amd.com wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> This patch implelments the ARM64 version of arch_setup_msi_irqs(),
> which does not return 1 for when PCI_CAP_ID_MSI and nvec > 1.

s/implelments/implements/

> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> Acked-by: Marc Zyngier <Marc.Zyngier@arm.com>
> Cc: Mark Rutland <Mark.Rutland@arm.com>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Catalin Marinas <Catalin.Marinas@arm.com>
> Cc: Will Deacon <Will.Deacon@arm.com>
> ---
>  arch/arm64/kernel/Makefile |  1 +
>  arch/arm64/kernel/msi.c    | 41 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+)
>  create mode 100644 arch/arm64/kernel/msi.c
> 
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index df7ef87..a921c42 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -29,6 +29,7 @@ arm64-obj-$(CONFIG_ARM64_CPU_SUSPEND)	+= sleep.o suspend.o
>  arm64-obj-$(CONFIG_JUMP_LABEL)		+= jump_label.o
>  arm64-obj-$(CONFIG_KGDB)		+= kgdb.o
>  arm64-obj-$(CONFIG_EFI)			+= efi.o efi-stub.o efi-entry.o
> +arm64-obj-$(CONFIG_PCI_MSI)		+= msi.o
>  
>  obj-y					+= $(arm64-obj-y) vdso/
>  obj-m					+= $(arm64-obj-m)
> diff --git a/arch/arm64/kernel/msi.c b/arch/arm64/kernel/msi.c
> new file mode 100644
> index 0000000..a295862
> --- /dev/null
> +++ b/arch/arm64/kernel/msi.c
> @@ -0,0 +1,41 @@
> +/*
> + * ARM64 architectural MSI implemention
> + *
> + * Support for Message Signalelled Interrupts for systems that
> + * implement ARM Generic Interrupt Controller: GICv2m.

You can drop the GICv2M reference here, as there's not really anything
GIC-specific in this file.

> + * Copyright (C) 2014 Advanced Micro Devices, Inc.
> + * Authors: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/msi.h>
> +#include <linux/pci.h>
> +
> +/*
> + * ARM64 function for seting up MSI irqs.
> + * Based on driver/pci/msi.c: arch_setup_msi_irqs().
> + *
> + * Note:
> + * Current implementation assumes that all interrupt controller used in
> + * ARM64 architecture _MUST_ supports multi-MSI.

I think you can remove this comment, to be honest.

> + */
> +int arm64_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> +{

Shouldn't this be called arch_setup_msi_irqs, since the intention is that
you override the weak symbol?

> +	struct msi_desc *entry;
> +	int ret;
> +
> +	list_for_each_entry(entry, &dev->msi_list, list) {
> +		ret = arch_setup_msi_irq(dev, entry);
> +		if (ret < 0)
> +			return ret;
> +		if (ret > 0)
> +			return -ENOSPC;

When does this function ever return a value > 0?

Will

[V8 2/2] irqchip: gicv2m: Add supports for ARM GICv2m MSI(-X)

[Mark Rutland]

Hi Suravee,

This looks good, I just have a few fixups that would be nice to apply
below.

On Sat, Sep 20, 2014 at 05:31:38PM +0100, suravee.suthikulpanit at amd.com wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> ARM GICv2m specification extends GICv2 to support MSI(-X) with
> a new set of register frame. This patch introduces support for
> the non-secure GICv2m register frame. Currently, GICV2m is available
> in certain version of GIC-400.
> 
> The patch introduces a new property in ARM gic binding, the v2m subnode.
> It is optional.
> 
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> Acked-by: Marc Zyngier <Marc.Zyngier@arm.com>
> Cc: Mark Rutland <Mark.Rutland@arm.com>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Catalin Marinas <Catalin.Marinas@arm.com>
> Cc: Will Deacon <Will.Deacon@arm.com>
> ---
>  Documentation/devicetree/bindings/arm/gic.txt |  55 ++++
>  arch/arm64/Kconfig                            |   1 +
>  drivers/irqchip/Kconfig                       |   5 +
>  drivers/irqchip/Makefile                      |   1 +
>  drivers/irqchip/irq-gic-common.c              |  12 +
>  drivers/irqchip/irq-gic-common.h              |   4 +
>  drivers/irqchip/irq-gic-v2m.c                 | 356 ++++++++++++++++++++++++++
>  drivers/irqchip/irq-gic.c                     |  82 +++---
>  drivers/irqchip/irq-gic.h                     |  54 ++++
>  9 files changed, 540 insertions(+), 30 deletions(-)
>  create mode 100644 drivers/irqchip/irq-gic-v2m.c
>  create mode 100644 drivers/irqchip/irq-gic.h
> 
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index c7d2fa1..38b2156 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -96,3 +96,58 @@ Example:
>                       <0x2c006000 0x2000>;
>                 interrupts = <1 9 0xf04>;
>         };
> +
> +
> +* GICv2m extension for MSI/MSI-x support (Optional)
> +
> +Certain revision of GIC-400 supports MSI/MSI-x via V2M register frame.
> +This is enabled by specifying v2m sub-node.

It looks like this was missed when the binding was updated for multiple
frames, so:

s/revision/revisions/
s/frame/frame(s)/
s/sub-node/sub-node(s)/

> +
> +Required properties:
> +
> +- compatible        : The value here should be "arm,gic-v2m-frame".

- compatible: should contain "arm,gic-v2m-frame".

> +
> +- msi-controller    : Identifies the node as an MSI controller.
> +
> +- reg               : GICv2m MSI interface register base and size
> +
> +Optional properties:
> +
> +- arm,msi-base-spi  : Specify base SPI the MSI frame.
> +                      The SPI base value can be from 32 to 1019.
> +
> +- arm,msi-num-spi   : Returns the number of contiguous SPIs assigned
> +                     to the frame.
> +
> +Note: "arm,msi-base-spi" and "arm,msi-num-spi" are used mainly for
> +       providing HW workaround in the case where the MSI_TYPER register
> +       is corrupted.

s/num-spi/num-spis/ please

Can we get rid of the note and reword this like:

- arm,msi-base-spi: When the MSI_TYPER register contains an incorrect
                    value, this property should contain the SPI base of
		    the MSI frame, overriding the HW value.

- arm,msi-num-spis: When the MSI_TYPER register contains an incorrect
                    value, this property should contain the number of
                    SPIs assigned to the frame, overriding the HW value.

I realise it's a little redundant, but it's very easy to miss usage
constraints in notes.

With that, for the binding:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

[V8 1/2] irqchip: gic: Add support for multiple MSI for ARM64

[Thomas Gleixner]

On Sat, 20 Sep 2014, suravee.suthikulpanit at amd.com wrote:

> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> This patch implelments the ARM64 version of arch_setup_msi_irqs(),
> which does not return 1 for when PCI_CAP_ID_MSI and nvec > 1.

I can see that myself. What your changelog is missing is the reason
WHY you think that copying that code from drivers/pci/msi.c and
removing the "PCI_CAP_ID_MSI and nvec > 1" has any value.
 
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> Acked-by: Marc Zyngier <Marc.Zyngier@arm.com>

....

>  arch/arm64/kernel/Makefile |  1 +
>  arch/arm64/kernel/msi.c    | 41 +++++++++++++++++++++++++++++++++++++++++

And that new function "arm64_setup_msi_irqs" is declared in which
header file exactly?

> diff --git a/arch/arm64/kernel/msi.c b/arch/arm64/kernel/msi.c
> new file mode 100644
> index 0000000..a295862
> --- /dev/null
> +++ b/arch/arm64/kernel/msi.c
> @@ -0,0 +1,41 @@
> +/*
> + * ARM64 architectural MSI implemention
> + *
> + * Support for Message Signalelled Interrupts for systems that
> + * implement ARM Generic Interrupt Controller: GICv2m.
> + *
> + * Copyright (C) 2014 Advanced Micro Devices, Inc.
> + * Authors: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

Copying stuff from A to B makes a real case for copyright, i.e. I'm
impressed by your ability to copy right.

> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/msi.h>
> +#include <linux/pci.h>
> +
> +/*
> + * ARM64 function for seting up MSI irqs.
> + * Based on driver/pci/msi.c: arch_setup_msi_irqs().

This is not based on. This is a verbatim copy with the omission of two
lines. Very creative that ...

> + *
> + * Note:
> + * Current implementation assumes that all interrupt controller used in
> + * ARM64 architecture _MUST_ supports multi-MSI.

Great assumption....

> + */
> +int arm64_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)

What the heck is calling that code? The follow up patch does not and
due to lack of a declaration this patch is completely pointless.

So you add a new file with a pointless changelog and a boasting
copyright notice which adds completely useless functionality?

Well done.

At least you are consistent on the useless side of affairs:

> +{
> +	struct msi_desc *entry;
> +	int ret;
> +
> +	list_for_each_entry(entry, &dev->msi_list, list) {
> +		ret = arch_setup_msi_irq(dev, entry);

Anyone who has the slightest idea how multi-MSI works will know that
this CANNOT work at all, but that's none of my business.

What's part of my business is to state that in my role as the
maintainer of all things related to interrupts this is the worst patch
I've seen in the last 10 years. Along with the saddening fact that it
carries the Acked-by of someone who should have known better.

At least if confirms my suspicion that ARM64 is a dignified successor
of the already infamous ARM32 universe.

I really can't bear the suspension to see the first 1500+ vendor patch
series of dubious quality supporting a real ARM64.

Thanks,

	tglx

[V8 1/2] irqchip: gic: Add support for multiple MSI for ARM64

[Suravee Suthikulpanit]

Thomas,

Sorry again for the mistake on my part. Let me try to address some other 
concerns you have below.


On 09/22/2014 04:08 PM, Thomas Gleixner wrote:
> On Sat, 20 Sep 2014, suravee.suthikulpanit at amd.com wrote:
>
>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>
>> This patch implelments the ARM64 version of arch_setup_msi_irqs(),
>> which does not return 1 for when PCI_CAP_ID_MSI and nvec > 1.
>
> I can see that myself. What your changelog is missing is the reason
> WHY you think that copying that code from drivers/pci/msi.c and
> removing the "PCI_CAP_ID_MSI and nvec > 1" has any value.

[Suravee] This is mainly be cause the weak version of 
arch_setup_msi_irqs() in the drivers/pci/msi.c doesn't support 
multi-MSI. Sorry for not being clear in the commit message.

>
> And that new function "arm64_setup_msi_irqs" is declared in which
> header file exactly?

[Suravee] This was supposed to be arch_setup_msi_irqs(). My bad. I'm 
fixing this in the next version.

......

>> + *
>> + * Note:
>> + * Current implementation assumes that all interrupt controller used in
>> + * ARM64 architecture _MUST_ supports multi-MSI.
>
> Great assumption....
>

[Suravee] So, Marc and I have discussed in the past that at this point, 
we are not seeing the case that there will be interrupt or 
MSI-controller that will not support multi-MSI.  If you think this 
should not be the case, would you please share your thought.

......

>
> At least you are consistent on the useless side of affairs:
>
>> +{
>> +	struct msi_desc *entry;
>> +	int ret;
>> +
>> +	list_for_each_entry(entry, &dev->msi_list, list) {
>> +		ret = arch_setup_msi_irq(dev, entry);
>
> Anyone who has the slightest idea how multi-MSI works will know that
> this CANNOT work at all, but that's none of my business.

[Suravee] I noticed that in the x86 version, there is a callback that 
each MSI controller need to register for handling the multi-MSI stuff.

In gicv2m_setup_msi_irq(), there is logic which handles the setup for 
multi-MSI and MSIx separately. In case of multi-MSI, the vectors are 
allocated on the first call to arch_setup_msi_irq(). Here, Marc and I 
are trying to simplify the arch-specific code so that each GIC 
controller (V2m and V3) would not need to implement and register the 
callbacks separately for handling multi-MSI.

The thing that is broken here is the error handling where the 
arch_setup_msi_irqs() is supposed to return the number of available MSI 
vectors. It would fail to do so because the arch_setup_msi_irq() would 
not return positive value. We should be able to fix this by 
re-implementing the arch_setup_msi_irq() and arch_setup_msi_irqs() to 
allow returning of positive values.

Please let me know what you think. I am open for suggestions :)

Thanks,

Suravee

[V8 1/2] irqchip: gic: Add support for multiple MSI for ARM64

[Thomas Gleixner]

On Tue, 23 Sep 2014, Suravee Suthikulpanit wrote:
> > > This patch implelments the ARM64 version of arch_setup_msi_irqs(),
> > > which does not return 1 for when PCI_CAP_ID_MSI and nvec > 1.
> > 
> > I can see that myself. What your changelog is missing is the reason
> > WHY you think that copying that code from drivers/pci/msi.c and
> > removing the "PCI_CAP_ID_MSI and nvec > 1" has any value.
> 
> [Suravee] This is mainly be cause the weak version of arch_setup_msi_irqs() in
> the drivers/pci/msi.c doesn't support multi-MSI. Sorry for not being clear in
> the commit message.

Groan. I asked you: 

> > WHY you think that copying that code from drivers/pci/msi.c and
> > removing the "PCI_CAP_ID_MSI and nvec > 1" has any value.

And your answer is that the function in drivers/pci/msi.c does not
support Multi-MSI. Hell I know that myself. And there is a fricking
good reason why allocating multi-MSI via

    for_each_msi()
	alloc_msi_irq();

is wrong. And while it might work by chance, there is no guarantee
that it will work. It works for Multi-MSIX, but that has an additional
X at the end and is a different beast when it comes to interrupts.

I have no idea how crooked you are trying to work around that on the
GIC side, but its going to be wrong and convoluted.

Read and understand the MSI and MSI-X spec and the subtle differences
of interrupt delivery. And if you groked that come back with a proper
explanation why that patch makes sense or just go back to the drawing
board and do it proper.

Thanks,

	tglx