[PATCH 0/7] Level-triggered MSI support

[PATCH 0/7] Level-triggered MSI support

[Marc Zyngier]

This series is a first shot at teaching the kernel about the oxymoron
expressed in $SUBJECT. Over the past couple of years, we've seen some
SoCs coming up with ways of signalling level interrupts using a new
flavor of MSIs, where the MSI controller uses two distinct messages:
one that raises a virtual line, and one that lowers it. The target MSI
controller is in charge of maintaining the state of the line.

This allows for a much simplified HW signal routing (no need to have
hundreds of discrete lines to signal level interrupts if you already
have a memory bus), but results in a departure from the current idea
the kernel has of MSIs.

This series takes a minimal approach to the problem, which is to allow
MSI controllers to use not only one, but up to two messages at a
time. This is controlled by a flag exposed at MSI irq domain creation,
and is only supported with platform MSI.

The rest of the series repaints the Marvell ICU/GICP drivers which
already make use of this feature with a side-channel, and adds support
for the same feature in GICv3. A side effect of the last GICv3 patch
is that you can also use SPIs to signal PCI MSIs. This is a last
resort measure for SoCs where the ITS is unusable for unspeakable
reasons.

Marc Zyngier (7):
  genirq/msi: Allow level-triggered MSIs to be exposed by MSI providers
  genirq/msi: Limit level-triggered MSI to platform devices
  irqchip/mvebu-gicp: Use level-triggered MSIs between ICU and GICP
  dma-iommu: Fix compilation when !CONFIG_IOMMU_DMA
  irqchip/gic-v3: Add support for Message Based Interrupts as an MSI
    controller
  irqchip/gic-v3: Add PCI/MSI support to the GICv3 MBI sub-driver
  dt-bindings/gic-v3: Add documentation for MBI support

 .../bindings/interrupt-controller/arm,gic-v3.txt   |  17 +
 drivers/base/platform-msi.c                        |   3 +
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-gic-v3-mbi.c                   | 343 +++++++++++++++++++++
 drivers/irqchip/irq-gic-v3.c                       |   6 +
 drivers/irqchip/irq-mvebu-gicp.c                   |  37 +--
 drivers/irqchip/irq-mvebu-gicp.h                   |  12 -
 drivers/irqchip/irq-mvebu-icu.c                    |  33 +-
 drivers/pci/msi.c                                  |   3 +
 include/linux/dma-iommu.h                          |   1 +
 include/linux/irqchip/arm-gic-v3.h                 |   1 +
 include/linux/msi.h                                |   2 +
 kernel/irq/msi.c                                   |  32 +-
 13 files changed, 427 insertions(+), 65 deletions(-)
 create mode 100644 drivers/irqchip/irq-gic-v3-mbi.c
 delete mode 100644 drivers/irqchip/irq-mvebu-gicp.h

-- 
2.14.2

[PATCH 1/7] genirq/msi: Allow level-triggered MSIs to be exposed by MSI providers

[Marc Zyngier]

So far, MSIs have been used to signal edge-triggered interrupts, as
a write is a good model for an edge (you can't "unwrite" something).
On the other hand, routing zillions of wires in an SoC because you
need level interrupts is a bit extreme.

People have come up with a variety of schemes to support this, which
involves sending two messages: one to signal the interrupt, and one
to clear it. Since the kernel cannot represent this, we've ended up
with side-band mechanisms that are pretty awful.

Instead, let's acknoledge the requirement, and ensure that, under the
right circumstances, the irq_compose_msg and irq_write_msg can take
as a parameter an array of two messages instead of a pointer to a
single one. We also add some checking that the compose method only
clobbers the second message if the MSI domain has been created with
the MSI_FLAG_LEVEL_CAPABLE flags.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/linux/msi.h |  2 ++
 kernel/irq/msi.c    | 32 +++++++++++++++++++++++---------
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/include/linux/msi.h b/include/linux/msi.h
index 1f1bbb5b4679..5839d8062dfc 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -289,6 +289,8 @@ enum {
 	 * MSI_FLAG_ACTIVATE_EARLY has been set.
 	 */
 	MSI_FLAG_MUST_REACTIVATE	= (1 << 5),
+	/* Is level-triggered capable, using two messages */
+	MSI_FLAG_LEVEL_CAPABLE		= (1 << 6),
 };
 
 int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 2a8571f72b17..a6fbf6008dcb 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -76,6 +76,18 @@ static inline void irq_chip_write_msi_msg(struct irq_data *data,
 	data->chip->irq_write_msi_msg(data, msg);
 }
 
+static void msi_check_level(struct irq_domain *domain, struct msi_msg *msg)
+{
+	struct msi_domain_info *info = domain->host_data;
+
+	/*
+	 * If the MSI provider has messed with the second message and
+	 * not advertized that it is level-capable, signal the breakage.
+	 */
+	WARN_ON(!(info->flags & MSI_FLAG_LEVEL_CAPABLE) &&
+		(msg[1].address_lo || msg[1].address_hi || msg[1].data));
+}
+
 /**
  * msi_domain_set_affinity - Generic affinity setter function for MSI domains
  * @irq_data:	The irq data associated to the interrupt
@@ -89,13 +101,14 @@ int msi_domain_set_affinity(struct irq_data *irq_data,
 			    const struct cpumask *mask, bool force)
 {
 	struct irq_data *parent = irq_data->parent_data;
-	struct msi_msg msg;
+	struct msi_msg msg[2] = { [1] = { }, };
 	int ret;
 
 	ret = parent->chip->irq_set_affinity(parent, mask, force);
 	if (ret >= 0 && ret != IRQ_SET_MASK_OK_DONE) {
-		BUG_ON(irq_chip_compose_msi_msg(irq_data, &msg));
-		irq_chip_write_msi_msg(irq_data, &msg);
+		BUG_ON(irq_chip_compose_msi_msg(irq_data, msg));
+		msi_check_level(irq_data->domain, msg);
+		irq_chip_write_msi_msg(irq_data, msg);
 	}
 
 	return ret;
@@ -104,20 +117,21 @@ int msi_domain_set_affinity(struct irq_data *irq_data,
 static int msi_domain_activate(struct irq_domain *domain,
 			       struct irq_data *irq_data, bool early)
 {
-	struct msi_msg msg;
+	struct msi_msg msg[2] = { [1] = { }, };
 
-	BUG_ON(irq_chip_compose_msi_msg(irq_data, &msg));
-	irq_chip_write_msi_msg(irq_data, &msg);
+	BUG_ON(irq_chip_compose_msi_msg(irq_data, msg));
+	msi_check_level(irq_data->domain, msg);
+	irq_chip_write_msi_msg(irq_data, msg);
 	return 0;
 }
 
 static void msi_domain_deactivate(struct irq_domain *domain,
 				  struct irq_data *irq_data)
 {
-	struct msi_msg msg;
+	struct msi_msg msg[2];
 
-	memset(&msg, 0, sizeof(msg));
-	irq_chip_write_msi_msg(irq_data, &msg);
+	memset(msg, 0, sizeof(msg));
+	irq_chip_write_msi_msg(irq_data, msg);
 }
 
 static int msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
-- 
2.14.2

[PATCH 2/7] genirq/msi: Limit level-triggered MSI to platform devices

[Marc Zyngier]

Nobody would be insane enough to try and use level triggered
MSIs on PCI, but let's make sure it doesn't happen. Also,
let's mandate that the irqchip backing the platform MSI domain
is providing an irq_set_type method (or the whole thing would
be a bit useless).

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/base/platform-msi.c | 3 +++
 drivers/pci/msi.c           | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
index 8e22073aeeed..3b9694a6beaa 100644
--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -101,6 +101,9 @@ static void platform_msi_update_chip_ops(struct msi_domain_info *info)
 		chip->irq_set_affinity = msi_domain_set_affinity;
 	if (!chip->irq_write_msi_msg)
 		chip->irq_write_msi_msg = platform_msi_write_msg;
+	if (WARN_ON((info->flags & MSI_FLAG_LEVEL_CAPABLE) &&
+		    !chip->irq_set_type))
+		info->flags &= ~MSI_FLAG_LEVEL_CAPABLE;
 }
 
 static void platform_msi_free_descs(struct device *dev, int base, int nvec)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 30250631efe7..f45b74fcc059 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1434,6 +1434,9 @@ struct irq_domain *pci_msi_create_irq_domain(struct fwnode_handle *fwnode,
 {
 	struct irq_domain *domain;
 
+	if (WARN_ON(info->flags & MSI_FLAG_LEVEL_CAPABLE))
+		info->flags &= ~MSI_FLAG_LEVEL_CAPABLE;
+
 	if (info->flags & MSI_FLAG_USE_DEF_DOM_OPS)
 		pci_msi_domain_update_dom_ops(info);
 	if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
-- 
2.14.2

[PATCH 3/7] irqchip/mvebu-gicp: Use level-triggered MSIs between ICU and GICP

[Marc Zyngier]

The ICU and GICP drivers are using an ugly side-band mechanism to
find out about the "clear" doorbell when using level interrupts.

Let's convert it to level-triggered MSIs, which result in a nice
cleanup.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-mvebu-gicp.c | 37 ++++++++++---------------------------
 drivers/irqchip/irq-mvebu-gicp.h | 12 ------------
 drivers/irqchip/irq-mvebu-icu.c  | 33 +++++++++++++++++----------------
 3 files changed, 27 insertions(+), 55 deletions(-)
 delete mode 100644 drivers/irqchip/irq-mvebu-gicp.h

diff --git a/drivers/irqchip/irq-mvebu-gicp.c b/drivers/irqchip/irq-mvebu-gicp.c
index 17a4a7b6cdbb..b5a3cc8bf370 100644
--- a/drivers/irqchip/irq-mvebu-gicp.c
+++ b/drivers/irqchip/irq-mvebu-gicp.c
@@ -19,8 +19,6 @@
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 
-#include "irq-mvebu-gicp.h"
-
 #define GICP_SETSPI_NSR_OFFSET	0x0
 #define GICP_CLRSPI_NSR_OFFSET	0x8
 
@@ -55,34 +53,18 @@ static int gicp_idx_to_spi(struct mvebu_gicp *gicp, int idx)
 	return -EINVAL;
 }
 
-int mvebu_gicp_get_doorbells(struct device_node *dn, phys_addr_t *setspi,
-			     phys_addr_t *clrspi)
-{
-	struct platform_device *pdev;
-	struct mvebu_gicp *gicp;
-
-	pdev = of_find_device_by_node(dn);
-	if (!pdev)
-		return -ENODEV;
-
-	gicp = platform_get_drvdata(pdev);
-	if (!gicp)
-		return -ENODEV;
-
-	*setspi = gicp->res->start + GICP_SETSPI_NSR_OFFSET;
-	*clrspi = gicp->res->start + GICP_CLRSPI_NSR_OFFSET;
-
-	return 0;
-}
-
 static void gicp_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 {
 	struct mvebu_gicp *gicp = data->chip_data;
 	phys_addr_t setspi = gicp->res->start + GICP_SETSPI_NSR_OFFSET;
-
-	msg->data = data->hwirq;
-	msg->address_lo = lower_32_bits(setspi);
-	msg->address_hi = upper_32_bits(setspi);
+	phys_addr_t clrspi = gicp->res->start + GICP_CLRSPI_NSR_OFFSET;
+
+	msg[0].data = data->hwirq;
+	msg[0].address_lo = lower_32_bits(setspi);
+	msg[0].address_hi = upper_32_bits(setspi);
+	msg[1].data = data->hwirq;
+	msg[1].address_lo = lower_32_bits(clrspi);
+	msg[1].address_hi = upper_32_bits(clrspi);
 }
 
 static struct irq_chip gicp_irq_chip = {
@@ -176,7 +158,8 @@ static struct msi_domain_ops gicp_msi_ops = {
 };
 
 static struct msi_domain_info gicp_msi_domain_info = {
-	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS),
+	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+		   MSI_FLAG_LEVEL_CAPABLE),
 	.ops	= &gicp_msi_ops,
 	.chip	= &gicp_msi_irq_chip,
 };
diff --git a/drivers/irqchip/irq-mvebu-gicp.h b/drivers/irqchip/irq-mvebu-gicp.h
deleted file mode 100644
index eaa12fb72102..000000000000
--- a/drivers/irqchip/irq-mvebu-gicp.h
+++ /dev/null
@@ -1,12 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __MVEBU_GICP_H__
-#define __MVEBU_GICP_H__
-
-#include <linux/types.h>
-
-struct device_node;
-
-int mvebu_gicp_get_doorbells(struct device_node *dn, phys_addr_t *setspi,
-			     phys_addr_t *clrspi);
-
-#endif /* __MVEBU_GICP_H__ */
diff --git a/drivers/irqchip/irq-mvebu-icu.c b/drivers/irqchip/irq-mvebu-icu.c
index e18c48d3a92e..13063339b416 100644
--- a/drivers/irqchip/irq-mvebu-icu.c
+++ b/drivers/irqchip/irq-mvebu-icu.c
@@ -21,8 +21,6 @@
 
 #include <dt-bindings/interrupt-controller/mvebu-icu.h>
 
-#include "irq-mvebu-gicp.h"
-
 /* ICU registers */
 #define ICU_SETSPI_NSR_AL	0x10
 #define ICU_SETSPI_NSR_AH	0x14
@@ -43,6 +41,7 @@ struct mvebu_icu {
 	void __iomem *base;
 	struct irq_domain *domain;
 	struct device *dev;
+	atomic_t initialized;
 };
 
 struct mvebu_icu_irq_data {
@@ -51,6 +50,18 @@ struct mvebu_icu_irq_data {
 	unsigned int type;
 };
 
+static void mvebu_icu_init(struct mvebu_icu *icu, struct msi_msg *msg)
+{
+	if (atomic_cmpxchg(&icu->initialized, false, true))
+		return;
+
+	/* Set Clear/Set ICU SPI message address in AP */
+	writel_relaxed(msg[0].address_hi, icu->base + ICU_SETSPI_NSR_AH);
+	writel_relaxed(msg[0].address_lo, icu->base + ICU_SETSPI_NSR_AL);
+	writel_relaxed(msg[1].address_hi, icu->base + ICU_CLRSPI_NSR_AH);
+	writel_relaxed(msg[1].address_lo, icu->base + ICU_CLRSPI_NSR_AL);
+}
+
 static void mvebu_icu_write_msg(struct msi_desc *desc, struct msi_msg *msg)
 {
 	struct irq_data *d = irq_get_irq_data(desc->irq);
@@ -59,6 +70,8 @@ static void mvebu_icu_write_msg(struct msi_desc *desc, struct msi_msg *msg)
 	unsigned int icu_int;
 
 	if (msg->address_lo || msg->address_hi) {
+		/* One off initialization */
+		mvebu_icu_init(icu, msg);
 		/* Configure the ICU with irq number & type */
 		icu_int = msg->data | ICU_INT_ENABLE;
 		if (icu_irqd->type & IRQ_TYPE_EDGE_RISING)
@@ -197,9 +210,7 @@ static int mvebu_icu_probe(struct platform_device *pdev)
 	struct device_node *node = pdev->dev.of_node;
 	struct device_node *gicp_dn;
 	struct resource *res;
-	phys_addr_t setspi, clrspi;
-	u32 i, icu_int;
-	int ret;
+	int i;
 
 	icu = devm_kzalloc(&pdev->dev, sizeof(struct mvebu_icu),
 			   GFP_KERNEL);
@@ -242,22 +253,12 @@ static int mvebu_icu_probe(struct platform_device *pdev)
 	if (!gicp_dn)
 		return -ENODEV;
 
-	ret = mvebu_gicp_get_doorbells(gicp_dn, &setspi, &clrspi);
-	if (ret)
-		return ret;
-
-	/* Set Clear/Set ICU SPI message address in AP */
-	writel_relaxed(upper_32_bits(setspi), icu->base + ICU_SETSPI_NSR_AH);
-	writel_relaxed(lower_32_bits(setspi), icu->base + ICU_SETSPI_NSR_AL);
-	writel_relaxed(upper_32_bits(clrspi), icu->base + ICU_CLRSPI_NSR_AH);
-	writel_relaxed(lower_32_bits(clrspi), icu->base + ICU_CLRSPI_NSR_AL);
-
 	/*
 	 * Clean all ICU interrupts with type SPI_NSR, required to
 	 * avoid unpredictable SPI assignments done by firmware.
 	 */
 	for (i = 0 ; i < ICU_MAX_IRQS ; i++) {
-		icu_int = readl(icu->base + ICU_INT_CFG(i));
+		u32 icu_int = readl_relaxed(icu->base + ICU_INT_CFG(i));
 		if ((icu_int >> ICU_GROUP_SHIFT) == ICU_GRP_NSR)
 			writel_relaxed(0x0, icu->base + ICU_INT_CFG(i));
 	}
-- 
2.14.2

[PATCH 4/7] dma-iommu: Fix compilation when !CONFIG_IOMMU_DMA

[Marc Zyngier]

Inclusion of include/dma-iommu.h when CONFIG_IOMMU_DMA is not selected
results in the following splat:

In file included from drivers/irqchip/irq-gic-v3-mbi.c:20:0:
./include/linux/dma-iommu.h:95:69: error: unknown type name ‘dma_addr_t’
 static inline int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
                                                                     ^~~~~~~~~~
./include/linux/dma-iommu.h:108:74: warning: ‘struct list_head’ declared inside parameter list will not be visible outside of this definition or declaration
 static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
                                                                          ^~~~~~~~~
scripts/Makefile.build:312: recipe for target 'drivers/irqchip/irq-gic-v3-mbi.o' failed

Fix it by including linux/types.h.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/linux/dma-iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 92f20832fd28..e8ca5e654277 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -17,6 +17,7 @@
 #define __DMA_IOMMU_H
 
 #ifdef __KERNEL__
+#include <linux/types.h>
 #include <asm/errno.h>
 
 #ifdef CONFIG_IOMMU_DMA
-- 
2.14.2

[PATCH 5/7] irqchip/gic-v3: Add support for Message Based Interrupts as an MSI controller

[Marc Zyngier]

GICv3 offers the possibility to signal SPIs using a pair of doorbells
(SETPI, CLRSPI) under the name of Message Based Interrupts (MBI).
They can be used as either traditional (edge) MSIs, or the more exotic
level-triggered flavour.

Let's implement support for platform MSI, which is the original intent
for this feature.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/Makefile           |   2 +-
 drivers/irqchip/irq-gic-v3-mbi.c   | 287 +++++++++++++++++++++++++++++++++++++
 drivers/irqchip/irq-gic-v3.c       |   6 +
 include/linux/irqchip/arm-gic-v3.h |   1 +
 4 files changed, 295 insertions(+), 1 deletion(-)
 create mode 100644 drivers/irqchip/irq-gic-v3-mbi.c

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 5ed465ab1c76..15f268f646bf 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -27,7 +27,7 @@ obj-$(CONFIG_ARM_GIC)			+= irq-gic.o irq-gic-common.o
 obj-$(CONFIG_ARM_GIC_PM)		+= irq-gic-pm.o
 obj-$(CONFIG_ARCH_REALVIEW)		+= irq-gic-realview.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_GIC_V3)		+= irq-gic-v3.o irq-gic-v3-mbi.o irq-gic-common.o
 obj-$(CONFIG_ARM_GIC_V3_ITS)		+= irq-gic-v3-its.o irq-gic-v3-its-platform-msi.o irq-gic-v4.o
 obj-$(CONFIG_ARM_GIC_V3_ITS_PCI)	+= irq-gic-v3-its-pci-msi.o
 obj-$(CONFIG_ARM_GIC_V3_ITS_FSL_MC)	+= irq-gic-v3-its-fsl-mc-msi.o
diff --git a/drivers/irqchip/irq-gic-v3-mbi.c b/drivers/irqchip/irq-gic-v3-mbi.c
new file mode 100644
index 000000000000..20303f54e8c0
--- /dev/null
+++ b/drivers/irqchip/irq-gic-v3-mbi.c
@@ -0,0 +1,287 @@
+/*
+ * Copyright (C) 2018 ARM Limited, All Rights Reserved.
+ * Author: Marc Zyngier <marc.zyngier@arm.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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define pr_fmt(fmt) "GICv3: " fmt
+
+#include <linux/dma-iommu.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/msi.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include <linux/irqchip/arm-gic-v3.h>
+
+struct mbi_range {
+	u32			spi_start;
+	u32			nr_spis;
+	unsigned long		*bm;
+};
+
+static spinlock_t		mbi_lock;
+static phys_addr_t		mbi_phys_base;
+static struct mbi_range		*mbi_ranges;
+static unsigned int		mbi_range_nr;
+
+static struct irq_chip mbi_irq_chip = {
+	.name			= "MBI",
+	.irq_mask		= irq_chip_mask_parent,
+	.irq_unmask		= irq_chip_unmask_parent,
+	.irq_eoi		= irq_chip_eoi_parent,
+	.irq_set_type		= irq_chip_set_type_parent,
+	.irq_set_affinity	= irq_chip_set_affinity_parent,
+};
+
+static int mbi_irq_gic_domain_alloc(struct irq_domain *domain,
+				       unsigned int virq,
+				       irq_hw_number_t hwirq)
+{
+	struct irq_fwspec fwspec;
+	struct irq_data *d;
+	int err;
+
+	/*
+	 * Using ACPI? There is no MBI support in the spec, you
+	 * shouldn't even be here.
+	 */
+	if (!is_of_node(domain->parent->fwnode))
+		return -EINVAL;
+
+	/*
+	 * Let's default to edge. This is consistent with traditional
+	 * MSIs, and systems requiring level signaling will just
+	 * enforce the trigger on their own.
+	 */
+	fwspec.fwnode = domain->parent->fwnode;
+	fwspec.param_count = 3;
+	fwspec.param[0] = 0;
+	fwspec.param[1] = hwirq - 32;
+	fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
+
+	err = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
+	if (err)
+		return err;
+
+	d = irq_domain_get_irq_data(domain->parent, virq);
+	d->chip->irq_set_type(d, IRQ_TYPE_EDGE_RISING);
+
+	return 0;
+}
+
+static void mbi_free_msi(struct mbi_range *mbi, unsigned int hwirq,
+			 int nr_irqs)
+{
+	spin_lock(&mbi_lock);
+	bitmap_release_region(mbi->bm, hwirq - mbi->spi_start,
+			      get_count_order(nr_irqs));
+	spin_unlock(&mbi_lock);
+}
+
+static int mbi_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				   unsigned int nr_irqs, void *args)
+{
+	struct mbi_range *mbi = NULL;
+	int hwirq, offset, i, err = 0;
+
+	spin_lock(&mbi_lock);
+	for (i = 0; i < mbi_range_nr; i++) {
+		offset = bitmap_find_free_region(mbi_ranges[i].bm,
+						 mbi_ranges[i].nr_spis,
+						 get_count_order(nr_irqs));
+		if (offset >= 0) {
+			mbi = &mbi_ranges[i];
+			break;
+		}
+	}
+	spin_unlock(&mbi_lock);
+
+	if (!mbi)
+		return -ENOSPC;
+
+	hwirq = mbi->spi_start + offset;
+
+	for (i = 0; i < nr_irqs; i++) {
+		err = mbi_irq_gic_domain_alloc(domain, virq + i, hwirq + i);
+		if (err)
+			goto fail;
+
+		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+					      &mbi_irq_chip, mbi);
+	}
+
+	return 0;
+
+fail:
+	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
+	mbi_free_msi(mbi, hwirq, nr_irqs);
+	return err;
+}
+
+static void mbi_irq_domain_free(struct irq_domain *domain,
+				unsigned int virq, unsigned int nr_irqs)
+{
+	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
+	struct mbi_range *mbi = irq_data_get_irq_chip_data(d);
+
+	mbi_free_msi(mbi, d->hwirq, nr_irqs);
+	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
+}
+
+static const struct irq_domain_ops mbi_domain_ops = {
+	.alloc			= mbi_irq_domain_alloc,
+	.free			= mbi_irq_domain_free,
+};
+
+static void mbi_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
+{
+	msg[0].address_hi = upper_32_bits(mbi_phys_base + GICD_SETSPI_NSR);
+	msg[0].address_lo = lower_32_bits(mbi_phys_base + GICD_SETSPI_NSR);
+	msg[0].data = data->parent_data->hwirq;
+
+	iommu_dma_map_msi_msg(data->irq, msg);
+}
+
+static void mbi_compose_mbi_msg(struct irq_data *data, struct msi_msg *msg)
+{
+	mbi_compose_msi_msg(data, msg);
+
+	msg[1].address_hi = upper_32_bits(mbi_phys_base + GICD_CLRSPI_NSR);
+	msg[1].address_lo = lower_32_bits(mbi_phys_base + GICD_CLRSPI_NSR);
+	msg[1].data = data->parent_data->hwirq;
+
+	iommu_dma_map_msi_msg(data->irq, &msg[1]);
+}
+
+/* Platform-MSI specific irqchip */
+static struct irq_chip mbi_pmsi_irq_chip = {
+	.name			= "pMSI",
+	.irq_set_type		= irq_chip_set_type_parent,
+	.irq_compose_msi_msg	= mbi_compose_mbi_msg,
+};
+
+static struct msi_domain_ops mbi_pmsi_ops = {
+};
+
+static struct msi_domain_info mbi_pmsi_domain_info = {
+	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+		   MSI_FLAG_LEVEL_CAPABLE),
+	.ops	= &mbi_pmsi_ops,
+	.chip	= &mbi_pmsi_irq_chip,
+};
+
+static int mbi_allocate_domains(struct irq_domain *parent)
+{
+	struct irq_domain *nexus_domain, *plat_domain;
+
+	nexus_domain = irq_domain_create_tree(parent->fwnode,
+					      &mbi_domain_ops, NULL);
+	if (!nexus_domain)
+		return -ENOMEM;
+
+	irq_domain_update_bus_token(nexus_domain, DOMAIN_BUS_NEXUS);
+	nexus_domain->parent = parent;
+
+	plat_domain = platform_msi_create_irq_domain(parent->fwnode,
+						     &mbi_pmsi_domain_info,
+						     nexus_domain);
+
+	if (!plat_domain) {
+		irq_domain_remove(plat_domain);
+		irq_domain_remove(nexus_domain);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+int __init mbi_init(struct fwnode_handle *fwnode, struct irq_domain *parent)
+{
+	struct device_node *np;
+	const __be32 *reg;
+	int ret, n;
+
+	np = to_of_node(fwnode);
+
+	if (!of_property_read_bool(np, "msi-controller"))
+		return 0;
+
+	n = of_property_count_elems_of_size(np, "mbi-ranges", sizeof(u32));
+	if (n <= 0 || n % 2)
+		return -EINVAL;
+
+	mbi_range_nr = n / 2;
+	mbi_ranges = kcalloc(mbi_range_nr, sizeof(*mbi_ranges), GFP_KERNEL);
+	if (!mbi_ranges)
+		return -ENOMEM;
+
+	for (n = 0; n < mbi_range_nr; n++) {
+		ret = of_property_read_u32_index(np, "mbi-ranges", n * 2,
+						 &mbi_ranges[n].spi_start);
+		if (ret)
+			goto err_free_mbi;
+		ret = of_property_read_u32_index(np, "mbi-ranges", n * 2 + 1,
+						 &mbi_ranges[n].nr_spis);
+		if (ret)
+			goto err_free_mbi;
+
+		mbi_ranges[n].bm = kcalloc(BITS_TO_LONGS(mbi_ranges[n].nr_spis),
+					   sizeof(long), GFP_KERNEL);
+		if (!mbi_ranges[n].bm) {
+			ret = -ENOMEM;
+			goto err_free_mbi;
+		}
+		pr_info("MBI range [%d:%d]\n", mbi_ranges[n].spi_start,
+			mbi_ranges[n].spi_start + mbi_ranges[n].nr_spis - 1);
+	}
+
+	reg = of_get_property(np, "mbi-alias", NULL);
+	if (reg) {
+		mbi_phys_base = of_translate_address(np, reg);
+		if (mbi_phys_base == OF_BAD_ADDR) {
+			ret = -ENXIO;
+			goto err_free_mbi;
+		}
+	} else {
+		struct resource res;
+
+		if (of_address_to_resource(np, 0, &res)) {
+			ret = -ENXIO;
+			goto err_free_mbi;
+		}
+
+		mbi_phys_base = res.start;
+	}
+
+	pr_info("Using MBI frame %pa\n", &mbi_phys_base);
+
+	ret = mbi_allocate_domains(parent);
+	if (ret)
+		goto err_free_mbi;
+
+	return 0;
+
+err_free_mbi:
+	if (mbi_ranges) {
+		for (n = 0; n < mbi_range_nr; n++)
+			kfree(mbi_ranges[n].bm);
+		kfree(mbi_ranges);
+	}
+
+	return ret;
+}
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index e5d101418390..1c6d039f22a3 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1112,6 +1112,12 @@ static int __init gic_init_bases(void __iomem *dist_base,
 	pr_info("Distributor has %sRange Selector support\n",
 		gic_data.has_rss ? "" : "no ");
 
+	if (typer & GICD_TYPER_MBIS) {
+		err = mbi_init(handle, gic_data.domain);
+		if (err)
+			pr_err("Failed to initialize MBIs\n");
+	}
+
 	set_handle_irq(gic_handle_irq);
 
 	gic_update_vlpi_properties();
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index f5af3b594e6e..cbb872c1b607 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -587,6 +587,7 @@ struct fwnode_handle;
 int its_cpu_init(void);
 int its_init(struct fwnode_handle *handle, struct rdists *rdists,
 	     struct irq_domain *domain);
+int mbi_init(struct fwnode_handle *fwnode, struct irq_domain *parent);
 
 static inline bool gic_enable_sre(void)
 {
-- 
2.14.2

[PATCH 6/7] irqchip/gic-v3: Add PCI/MSI support to the GICv3 MBI sub-driver

[Marc Zyngier]

You would hope that if you have a GICv3 in your system, you'd use the ITS,
as it provides a large interrupt ID space and device isolation. Sadly,
some SoC integrations are less than perfect, and the ITS is not usesable on
those.

The only solution for these systems is to use the MBI interface, and
rely on a very small number of possible vectors.

This patch thus adds minimal support for PCI/MSI on top of the GICv3
MBI driver. Please don't use it if you can avoid it.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v3-mbi.c | 62 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 59 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-mbi.c b/drivers/irqchip/irq-gic-v3-mbi.c
index 20303f54e8c0..acbeb236f157 100644
--- a/drivers/irqchip/irq-gic-v3-mbi.c
+++ b/drivers/irqchip/irq-gic-v3-mbi.c
@@ -23,6 +23,7 @@
 #include <linux/kernel.h>
 #include <linux/msi.h>
 #include <linux/of_address.h>
+#include <linux/of_pci.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 
@@ -157,6 +158,55 @@ static void mbi_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 	iommu_dma_map_msi_msg(data->irq, msg);
 }
 
+#ifdef CONFIG_PCI_MSI
+/* PCI-specific irqchip */
+static void mbi_mask_msi_irq(struct irq_data *d)
+{
+	pci_msi_mask_irq(d);
+	irq_chip_mask_parent(d);
+}
+
+static void mbi_unmask_msi_irq(struct irq_data *d)
+{
+	pci_msi_unmask_irq(d);
+	irq_chip_unmask_parent(d);
+}
+
+static struct irq_chip mbi_msi_irq_chip = {
+	.name			= "MSI",
+	.irq_mask		= mbi_mask_msi_irq,
+	.irq_unmask		= mbi_unmask_msi_irq,
+	.irq_eoi		= irq_chip_eoi_parent,
+	.irq_compose_msi_msg	= mbi_compose_msi_msg,
+	.irq_write_msi_msg	= pci_msi_domain_write_msg,
+};
+
+static struct msi_domain_info mbi_msi_domain_info = {
+	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+		   MSI_FLAG_PCI_MSIX | MSI_FLAG_MULTI_PCI_MSI),
+	.chip	= &mbi_msi_irq_chip,
+};
+
+static int mbi_allocate_pci_domain(struct irq_domain *nexus_domain,
+				   struct irq_domain **pci_domain)
+{
+	*pci_domain = pci_msi_create_irq_domain(nexus_domain->parent->fwnode,
+						&mbi_msi_domain_info,
+						nexus_domain);
+	if (!*pci_domain)
+		return -ENOMEM;
+
+	return 0;
+}
+#else
+static int mbi_allocate_pci_domain(struct irq_domain *nexus_domain,
+				   struct irq_domain **pci_domain)
+{
+	*pci_domain = NULL;
+	return 0;
+}
+#endif
+
 static void mbi_compose_mbi_msg(struct irq_data *data, struct msi_msg *msg)
 {
 	mbi_compose_msi_msg(data, msg);
@@ -187,7 +237,8 @@ static struct msi_domain_info mbi_pmsi_domain_info = {
 
 static int mbi_allocate_domains(struct irq_domain *parent)
 {
-	struct irq_domain *nexus_domain, *plat_domain;
+	struct irq_domain *nexus_domain, *pci_domain, *plat_domain;
+	int err;
 
 	nexus_domain = irq_domain_create_tree(parent->fwnode,
 					      &mbi_domain_ops, NULL);
@@ -197,12 +248,17 @@ static int mbi_allocate_domains(struct irq_domain *parent)
 	irq_domain_update_bus_token(nexus_domain, DOMAIN_BUS_NEXUS);
 	nexus_domain->parent = parent;
 
+	err = mbi_allocate_pci_domain(nexus_domain, &pci_domain);
+
 	plat_domain = platform_msi_create_irq_domain(parent->fwnode,
 						     &mbi_pmsi_domain_info,
 						     nexus_domain);
 
-	if (!plat_domain) {
-		irq_domain_remove(plat_domain);
+	if (err || !plat_domain) {
+		if (plat_domain)
+			irq_domain_remove(plat_domain);
+		if (pci_domain)
+			irq_domain_remove(pci_domain);
 		irq_domain_remove(nexus_domain);
 		return -ENOMEM;
 	}
-- 
2.14.2

[PATCH 7/7] dt-bindings/gic-v3: Add documentation for MBI support

[Marc Zyngier]

Add the required properties to support the MBI feature on GICv3.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 .../bindings/interrupt-controller/arm,gic-v3.txt        | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
index 0a57f2f4167d..3ea78c4ef887 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
@@ -57,6 +57,20 @@ Optional
   occupied by the redistributors. Required if more than one such
   region is present.
 
+- msi-controller: Boolean property. Identifies the node as an MSI
+  controller. Only present if the Message Based Interrupt
+  functionnality is being exposed by the HW, and the mbi-ranges
+  property present.
+
+- mbi-ranges: A list of pairs <intid span>, where "intid" is the first
+  SPI of a range that can be used an MBI, and "span" the size of that
+  range. Multiple ranges can be provided. Requires "msi-controller" to
+  be set.
+
+- mbi-alias: Address property. Base address of an alias of the GICD
+  region containing only the {SET,CLR}SPI registers to be used if
+  isolation is required, and if supported by the HW.
+
 Sub-nodes:
 
 PPI affinity can be expressed as a single "ppi-partitions" node,
@@ -99,6 +113,9 @@ Examples:
 		      <0x0 0x2c020000 0 0x2000>;	// GICV
 		interrupts = <1 9 4>;
 
+		msi-controller;
+		mbi-ranges = <256 128>;
+
 		gic-its@2c200000 {
 			compatible = "arm,gic-v3-its";
 			msi-controller;
-- 
2.14.2

Re: [PATCH 0/7] Level-triggered MSI support

[Ard Biesheuvel]

On 23 April 2018 at 12:34, Marc Zyngier <marc.zyngier@arm.com> wrote:
> This series is a first shot at teaching the kernel about the oxymoron
> expressed in $SUBJECT. Over the past couple of years, we've seen some
> SoCs coming up with ways of signalling level interrupts using a new
> flavor of MSIs, where the MSI controller uses two distinct messages:
> one that raises a virtual line, and one that lowers it. The target MSI
> controller is in charge of maintaining the state of the line.
>
> This allows for a much simplified HW signal routing (no need to have
> hundreds of discrete lines to signal level interrupts if you already
> have a memory bus), but results in a departure from the current idea
> the kernel has of MSIs.
>
> This series takes a minimal approach to the problem, which is to allow
> MSI controllers to use not only one, but up to two messages at a
> time. This is controlled by a flag exposed at MSI irq domain creation,
> and is only supported with platform MSI.
>
> The rest of the series repaints the Marvell ICU/GICP drivers which
> already make use of this feature with a side-channel, and adds support
> for the same feature in GICv3. A side effect of the last GICv3 patch
> is that you can also use SPIs to signal PCI MSIs. This is a last
> resort measure for SoCs where the ITS is unusable for unspeakable
> reasons.
>

Hi Marc,

I am hitting the splat below when trying this series on SynQuacer,
with mbi range <64 32> (which is reserved in the h/w manual but note
that I haven't confirmed with Socionext whether these are expected to
work or not. However, I don't think that makes any difference
regarding the issue below.)

 Unable to handle kernel read from unreadable memory at virtual address 00000018
 Mem abort info:
   ESR = 0x96000004
   Exception class = DABT (current EL), IL = 32 bits
   SET = 0, FnV = 0
   EA = 0, S1PTW = 0
 Data abort info:
   ISV = 0, ISS = 0x00000004
   CM = 0, WnR = 0
 user pgtable: 4k pages, 48-bit VAs, pgdp =         (ptrval)
 [0000000000000018] pgd=0000000000000000
 Internal error: Oops: 96000004 [#1] PREEMPT SMP
 Modules linked in: gpio_keys(+) efivarfs ip_tables x_tables autofs4
ext4 crc16 mbcache jbd2 fscrypto sr_mod cdrom sd_mod ahci xhci_pci
libahci xhci_hcd libata usbcore scsi_mod realtek netsec of_mdio
fixed_phy libphy i2c_synquacer gpio_mb86s7x
 CPU: 19 PID: 398 Comm: systemd-udevd Tainted: G        W
4.17.0-rc2+ #54
 Hardware name: Socionext SynQuacer E-series DeveloperBox, BIOS build
#101 Apr  2 2018
 pstate: a0400085 (NzCv daIf +PAN -UAO)
 pc : iommu_dma_map_msi_msg+0x40/0x1e8
 lr : iommu_dma_map_msi_msg+0x34/0x1e8
 sp : ffff00000b8db690
 x29: ffff00000b8db690 x28: ffffeca6f07442a0
 x27: 0000000000000000 x26: ffffeca6f07442d4
 x25: ffffeca6f0744398 x24: 0000000000000000
 x23: 0000000000000016 x22: 0000000000000000
 x21: ffffeca6f755ed00 x20: ffff00000b8db770
 x19: 0000000000000016 x18: ffffffffffffffff
 x17: ffff446c203fd000 x16: ffff446c1f3b5108
 x15: ffffeca6f0a095b0 x14: ffffeca6f0a3a587
 x13: ffffeca6f0a3a586 x12: 0000000000000040
 x11: 0000000000000004 x10: 0000000000000016
 x9 : ffffeca6f70009d8 x8 : 0000000000000000
 x7 : ffffeca6f0744200 x6 : ffffeca6f0744200
 x5 : ffffeca6f7000900 x4 : ffffeca6f0744200
 x3 : 0000000000000000 x2 : 0000000000000000
 x1 : ffffeca6f0744258 x0 : 0000000000000000
 Process systemd-udevd (pid: 398, stack limit = 0x        (ptrval))
 Call trace:
  iommu_dma_map_msi_msg+0x40/0x1e8
  mbi_compose_msi_msg+0x54/0x60
  mbi_compose_mbi_msg+0x28/0x68
  irq_chip_compose_msi_msg+0x5c/0x78
  msi_domain_activate+0x40/0x90
  __irq_domain_activate_irq+0x74/0xb8
  __irq_domain_activate_irq+0x3c/0xb8
  irq_domain_activate_irq+0x4c/0x60
  irq_activate+0x40/0x50
  __setup_irq+0x4bc/0x7e0
  request_threaded_irq+0xf0/0x198
  request_any_context_irq+0x6c/0xc0
  devm_request_any_context_irq+0x78/0xf0
  gpio_keys_probe+0x324/0x9a0 [gpio_keys]
  platform_drv_probe+0x60/0xc8
  driver_probe_device+0x2c4/0x490
  __driver_attach+0x10c/0x128
  bus_for_each_dev+0x78/0xe0
  driver_attach+0x30/0x40
  bus_add_driver+0x1d0/0x298
  driver_register+0x68/0x100
  __platform_driver_register+0x54/0x60
  gpio_keys_init+0x24/0x1000 [gpio_keys]
  do_one_initcall+0x68/0x258
  do_init_module+0x64/0x1e0
  load_module+0x1e20/0x21a8
  sys_finit_module+0x108/0x140
 EFI Variables Facility v0.08 2004-May-17
  __sys_trace_return+0x0/0x4
 Code: 97ec1444 b4000ca0 f9400800 f9400800 (f9400c18)
 ---[ end trace f7956dc89d9f3be7 ]---

00000000000014a0 <iommu_dma_map_msi_msg>:
    14a0:       a9ba7bfd        stp     x29, x30, [sp, #-96]!
    14a4:       910003fd        mov     x29, sp
    14a8:       a90153f3        stp     x19, x20, [sp, #16]
    14ac:       a9025bf5        stp     x21, x22, [sp, #32]
    14b0:       a90363f7        stp     x23, x24, [sp, #48]
    14b4:       a9046bf9        stp     x25, x26, [sp, #64]
    14b8:       a90573fb        stp     x27, x28, [sp, #80]
    14bc:       2a0003f3        mov     w19, w0
    14c0:       aa1e03e0        mov     x0, x30
    14c4:       aa0103f4        mov     x20, x1
    14c8:       94000000        bl      0 <_mcount>
    14cc:       2a1303e0        mov     w0, w19
    14d0:       94000000        bl      0 <irq_get_irq_data>
    14d4:       b4000ca0        cbz     x0, 1668 <iommu_dma_map_msi_msg+0x1c8>
    14d8:       f9400800        ldr     x0, [x0, #16]
    14dc:       f9400800        ldr     x0, [x0, #16]
    14e0:       f9400c18        ldr     x24, [x0, #24]
    14e4:       aa1803e0        mov     x0, x24
    14e8:       94000000        bl      0 <iommu_get_domain_for_dev>
    14ec:       aa0003f3        mov     x19, x0
    14f0:       b40008a0        cbz     x0, 1604 <iommu_dma_map_msi_msg+0x164>
    14f4:       f9402015        ldr     x21, [x0, #64]
    14f8:       b4000875        cbz     x21, 1604 <iommu_dma_map_msi_msg+0x164>
    14fc:       911d82b6        add     x22, x21, #0x760

Re: [PATCH 0/7] Level-triggered MSI support

[Marc Zyngier]

HI Ard,

On 23/04/18 12:51, Ard Biesheuvel wrote:
> On 23 April 2018 at 12:34, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> This series is a first shot at teaching the kernel about the oxymoron
>> expressed in $SUBJECT. Over the past couple of years, we've seen some
>> SoCs coming up with ways of signalling level interrupts using a new
>> flavor of MSIs, where the MSI controller uses two distinct messages:
>> one that raises a virtual line, and one that lowers it. The target MSI
>> controller is in charge of maintaining the state of the line.
>>
>> This allows for a much simplified HW signal routing (no need to have
>> hundreds of discrete lines to signal level interrupts if you already
>> have a memory bus), but results in a departure from the current idea
>> the kernel has of MSIs.
>>
>> This series takes a minimal approach to the problem, which is to allow
>> MSI controllers to use not only one, but up to two messages at a
>> time. This is controlled by a flag exposed at MSI irq domain creation,
>> and is only supported with platform MSI.
>>
>> The rest of the series repaints the Marvell ICU/GICP drivers which
>> already make use of this feature with a side-channel, and adds support
>> for the same feature in GICv3. A side effect of the last GICv3 patch
>> is that you can also use SPIs to signal PCI MSIs. This is a last
>> resort measure for SoCs where the ITS is unusable for unspeakable
>> reasons.
>>
> 
> Hi Marc,
> 
> I am hitting the splat below when trying this series on SynQuacer,
> with mbi range <64 32> (which is reserved in the h/w manual but note
> that I haven't confirmed with Socionext whether these are expected to
> work or not. However, I don't think that makes any difference
> regarding the issue below.)

Thanks for giving it a go. Something looks odd below:

> 
>  Unable to handle kernel read from unreadable memory at virtual address 00000018
>  Mem abort info:
>    ESR = 0x96000004
>    Exception class = DABT (current EL), IL = 32 bits
>    SET = 0, FnV = 0
>    EA = 0, S1PTW = 0
>  Data abort info:
>    ISV = 0, ISS = 0x00000004
>    CM = 0, WnR = 0
>  user pgtable: 4k pages, 48-bit VAs, pgdp =         (ptrval)
>  [0000000000000018] pgd=0000000000000000
>  Internal error: Oops: 96000004 [#1] PREEMPT SMP
>  Modules linked in: gpio_keys(+) efivarfs ip_tables x_tables autofs4
> ext4 crc16 mbcache jbd2 fscrypto sr_mod cdrom sd_mod ahci xhci_pci
> libahci xhci_hcd libata usbcore scsi_mod realtek netsec of_mdio
> fixed_phy libphy i2c_synquacer gpio_mb86s7x
>  CPU: 19 PID: 398 Comm: systemd-udevd Tainted: G        W
> 4.17.0-rc2+ #54
>  Hardware name: Socionext SynQuacer E-series DeveloperBox, BIOS build
> #101 Apr  2 2018
>  pstate: a0400085 (NzCv daIf +PAN -UAO)
>  pc : iommu_dma_map_msi_msg+0x40/0x1e8
>  lr : iommu_dma_map_msi_msg+0x34/0x1e8
>  sp : ffff00000b8db690
>  x29: ffff00000b8db690 x28: ffffeca6f07442a0
>  x27: 0000000000000000 x26: ffffeca6f07442d4
>  x25: ffffeca6f0744398 x24: 0000000000000000
>  x23: 0000000000000016 x22: 0000000000000000
>  x21: ffffeca6f755ed00 x20: ffff00000b8db770
>  x19: 0000000000000016 x18: ffffffffffffffff
>  x17: ffff446c203fd000 x16: ffff446c1f3b5108
>  x15: ffffeca6f0a095b0 x14: ffffeca6f0a3a587
>  x13: ffffeca6f0a3a586 x12: 0000000000000040
>  x11: 0000000000000004 x10: 0000000000000016
>  x9 : ffffeca6f70009d8 x8 : 0000000000000000
>  x7 : ffffeca6f0744200 x6 : ffffeca6f0744200
>  x5 : ffffeca6f7000900 x4 : ffffeca6f0744200
>  x3 : 0000000000000000 x2 : 0000000000000000
>  x1 : ffffeca6f0744258 x0 : 0000000000000000
>  Process systemd-udevd (pid: 398, stack limit = 0x        (ptrval))
>  Call trace:
>   iommu_dma_map_msi_msg+0x40/0x1e8

We die here because irq_get_msi_desc() has returned a NULL pointer.
That's not really expected.

>   mbi_compose_msi_msg+0x54/0x60
>   mbi_compose_mbi_msg+0x28/0x68

We're requesting a platform MSI...

>   irq_chip_compose_msi_msg+0x5c/0x78
>   msi_domain_activate+0x40/0x90
>   __irq_domain_activate_irq+0x74/0xb8
>   __irq_domain_activate_irq+0x3c/0xb8
>   irq_domain_activate_irq+0x4c/0x60
>   irq_activate+0x40/0x50
>   __setup_irq+0x4bc/0x7e0
>   request_threaded_irq+0xf0/0x198
>   request_any_context_irq+0x6c/0xc0
>   devm_request_any_context_irq+0x78/0xf0
>   gpio_keys_probe+0x324/0x9a0 [gpio_keys]

from the gpio_keys driver, which shouldn't be doing platform MSI at all.
It looks like we've looked-up the wrong irq domain, and really bad
things happen after that.

Could you point me to the device-tree of this machine? I need to
understand how we can end-up in such a situation.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 0/7] Level-triggered MSI support

[Srinivas Kandagatla]

On 23/04/18 11:34, Marc Zyngier wrote:
> This series is a first shot at teaching the kernel about the oxymoron
> expressed in $SUBJECT. Over the past couple of years, we've seen some
> SoCs coming up with ways of signalling level interrupts using a new
> flavor of MSIs, where the MSI controller uses two distinct messages:
> one that raises a virtual line, and one that lowers it. The target MSI
> controller is in charge of maintaining the state of the line.
> 
> This allows for a much simplified HW signal routing (no need to have
> hundreds of discrete lines to signal level interrupts if you already
> have a memory bus), but results in a departure from the current idea
> the kernel has of MSIs.
> 
> This series takes a minimal approach to the problem, which is to allow
> MSI controllers to use not only one, but up to two messages at a
> time. This is controlled by a flag exposed at MSI irq domain creation,
> and is only supported with platform MSI.
> 
> The rest of the series repaints the Marvell ICU/GICP drivers which
> already make use of this feature with a side-channel, and adds support
> for the same feature in GICv3. A side effect of the last GICv3 patch
> is that you can also use SPIs to signal PCI MSIs. This is a last
> resort measure for SoCs where the ITS is unusable for unspeakable
> reasons.
> 
> Marc Zyngier (7):
>    genirq/msi: Allow level-triggered MSIs to be exposed by MSI providers
>    genirq/msi: Limit level-triggered MSI to platform devices
>    irqchip/mvebu-gicp: Use level-triggered MSIs between ICU and GICP
>    dma-iommu: Fix compilation when !CONFIG_IOMMU_DMA
>    irqchip/gic-v3: Add support for Message Based Interrupts as an MSI
>      controller
>    irqchip/gic-v3: Add PCI/MSI support to the GICv3 MBI sub-driver
>    dt-bindings/gic-v3: Add documentation for MBI support
> 

Thanks for the patches,

Am able to get PCIe MSI interrupts on Qualcomm DragonBoard DB820c with 
this patchset.

root@linaro-alip:~# cat /proc/interrupts  | grep -i MSI
  44:          0          0          0          0     GICv3 437 Edge 
  qcom-pcie-msi
  45:          0          0          0          0     GICv3 445 Edge 
  qcom-pcie-msi
  46:          0          0          0          0     GICv3 453 Edge 
  qcom-pcie-msi
  80:          0          0          0          0       MSI 134217728 
Edge      PCIe PME, aerdrv
114:          0          0          0          0       MSI 268435456 
Edge      PCIe PME, aerdrv
115:          0          0          0          0       MSI 134742016 
Edge      ahci[0001:01:00.0]
197:          0          0          0          0       MSI   0 Edge 
PCIe PME, aerdrv
199:       2108          0          0          0       MSI 524288 Edge 
    ath10k_pci
202:        295          0          0          0       MSI 268959744 
Edge      eth0


Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

--srini

Re: [PATCH 0/7] Level-triggered MSI support

[Miquel Raynal]

Hi Marc,

On Mon, 23 Apr 2018 11:34:33 +0100, Marc Zyngier <marc.zyngier@arm.com>
wrote:

> This series is a first shot at teaching the kernel about the oxymoron
> expressed in $SUBJECT. Over the past couple of years, we've seen some
> SoCs coming up with ways of signalling level interrupts using a new
> flavor of MSIs, where the MSI controller uses two distinct messages:
> one that raises a virtual line, and one that lowers it. The target MSI
> controller is in charge of maintaining the state of the line.
> 
> This allows for a much simplified HW signal routing (no need to have
> hundreds of discrete lines to signal level interrupts if you already
> have a memory bus), but results in a departure from the current idea
> the kernel has of MSIs.
> 
> This series takes a minimal approach to the problem, which is to allow
> MSI controllers to use not only one, but up to two messages at a
> time. This is controlled by a flag exposed at MSI irq domain creation,
> and is only supported with platform MSI.
> 
> The rest of the series repaints the Marvell ICU/GICP drivers which
> already make use of this feature with a side-channel, and adds support
> for the same feature in GICv3. A side effect of the last GICv3 patch
> is that you can also use SPIs to signal PCI MSIs. This is a last
> resort measure for SoCs where the ITS is unusable for unspeakable
> reasons.
> 
> Marc Zyngier (7):
>   genirq/msi: Allow level-triggered MSIs to be exposed by MSI providers
>   genirq/msi: Limit level-triggered MSI to platform devices
>   irqchip/mvebu-gicp: Use level-triggered MSIs between ICU and GICP
>   dma-iommu: Fix compilation when !CONFIG_IOMMU_DMA
>   irqchip/gic-v3: Add support for Message Based Interrupts as an MSI
>     controller
>   irqchip/gic-v3: Add PCI/MSI support to the GICv3 MBI sub-driver
>   dt-bindings/gic-v3: Add documentation for MBI support
> 
>  .../bindings/interrupt-controller/arm,gic-v3.txt   |  17 +
>  drivers/base/platform-msi.c                        |   3 +
>  drivers/irqchip/Makefile                           |   2 +-
>  drivers/irqchip/irq-gic-v3-mbi.c                   | 343 +++++++++++++++++++++
>  drivers/irqchip/irq-gic-v3.c                       |   6 +
>  drivers/irqchip/irq-mvebu-gicp.c                   |  37 +--
>  drivers/irqchip/irq-mvebu-gicp.h                   |  12 -
>  drivers/irqchip/irq-mvebu-icu.c                    |  33 +-
>  drivers/pci/msi.c                                  |   3 +
>  include/linux/dma-iommu.h                          |   1 +
>  include/linux/irqchip/arm-gic-v3.h                 |   1 +
>  include/linux/msi.h                                |   2 +
>  kernel/irq/msi.c                                   |  32 +-
>  13 files changed, 427 insertions(+), 65 deletions(-)
>  create mode 100644 drivers/irqchip/irq-gic-v3-mbi.c
>  delete mode 100644 drivers/irqchip/irq-mvebu-gicp.h
> 

Thanks for the series, I gave it a try on Armada-8040-DB:

# cat /proc/interrupts | grep ICU
 28:          0          0          0          0  ICU.f21e0000 107 Level     ahci[f2540000.sata]
 29:          0          0          0          0  ICU.f41e0000 107 Level     ahci[f4540000.sata]
 35:          2          0          0          0  ICU.f21e0000 129 Level   
 36:          3          0          0          0  ICU.f21e0000  41 Level     eth1
 37:          0          0          0          0  ICU.f21e0000  45 Level     eth1
 38:          0          0          4          0  ICU.f21e0000  49 Level     eth1
 39:          0          0          0          2  ICU.f21e0000  53 Level     eth1
 40:        128          0          0          0  ICU.f21e0000  57 Level     eth1
 47:          2          0          0          0  ICU.f41e0000 129 Level   
 54:         82          0          0          0  ICU.f21e0000 106 Level     xhci-hcd:usb3
 55:         82          0          0          0  ICU.f21e0000 105 Level     xhci-hcd:usb5
 56:          4          0          0          0  ICU.f41e0000 106 Level     xhci-hcd:usb7
 57:          0          0          0          0  ICU.f41e0000 105 Level     xhci-hcd:usb1
 58:          0          0          0          0  ICU.f41e0000  77 Level     f4284000.rtc
 59:        224          0          0          0  ICU.f21e0000 120 Level     mv64xxx_i2c
 60:          0          0          0          0  ICU.f41e0000 120 Level     mv64xxx_i2c
 61:         52          0          0          0  ICU.f21e0000  27 Level     mmc1
 63:          0          0          0          0  ICU.f21e0000  22 Level     armada8k-pcie, PCIe PME, aerdrv
 64:          0          0          0          0  ICU.f21e0000  23 Level     armada8k-pcie, PCIe PME, aerdrv
 65:          0          0          0          0  ICU.f41e0000  22 Level     armada8k-pcie, PCIe PME, aerdrv
 66:          0          0          0          0  ICU.f41e0000  24 Level     armada8k-pcie, PCIe PME, aerdrv
 67:          0          0          0          0  ICU.f41e0000  23 Level     armada8k-pcie, PCIe PME, aerdrv

All these interrupts are NSRs and go through the GICP. I was able to
use the network (CP0), an I2C bus (CP0) and the USB host ports (on each
CP).

Tested-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 0/7] Level-triggered MSI support

[Marc Zyngier]

On 23/04/18 12:51, Ard Biesheuvel wrote:
> On 23 April 2018 at 12:34, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> This series is a first shot at teaching the kernel about the oxymoron
>> expressed in $SUBJECT. Over the past couple of years, we've seen some
>> SoCs coming up with ways of signalling level interrupts using a new
>> flavor of MSIs, where the MSI controller uses two distinct messages:
>> one that raises a virtual line, and one that lowers it. The target MSI
>> controller is in charge of maintaining the state of the line.
>>
>> This allows for a much simplified HW signal routing (no need to have
>> hundreds of discrete lines to signal level interrupts if you already
>> have a memory bus), but results in a departure from the current idea
>> the kernel has of MSIs.
>>
>> This series takes a minimal approach to the problem, which is to allow
>> MSI controllers to use not only one, but up to two messages at a
>> time. This is controlled by a flag exposed at MSI irq domain creation,
>> and is only supported with platform MSI.
>>
>> The rest of the series repaints the Marvell ICU/GICP drivers which
>> already make use of this feature with a side-channel, and adds support
>> for the same feature in GICv3. A side effect of the last GICv3 patch
>> is that you can also use SPIs to signal PCI MSIs. This is a last
>> resort measure for SoCs where the ITS is unusable for unspeakable
>> reasons.
>>
> 
> Hi Marc,
> 
> I am hitting the splat below when trying this series on SynQuacer,
> with mbi range <64 32> (which is reserved in the h/w manual but note
> that I haven't confirmed with Socionext whether these are expected to
> work or not. However, I don't think that makes any difference
> regarding the issue below.)

[...]

For the record: After some IRC debugging with Ard, this turns out to be
due to two issues:
- GICv3 is now advertising several domains, all using the same device nodes
- irq_find_host() is picking the first one, which is the wrong one.

A good way to avoid this mess is to:
- Let GICv3 advertise its core domain with DOMAIN_BUS_WIRED (consistent
with what the armada-370-xp driver is already doing)
- Let irq_find_host return DOMAIN_BUS_WIRED before trying DOMAIN_BUS_ANY
(consistent with the way irq_create_fwspec_mapping works).

I've stashed the whole series at [1] with these two fixes.

	M.

[1] git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git
irq/level-msi
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 0/7] Level-triggered MSI support

[Ard Biesheuvel]

On 23 April 2018 at 17:53, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 23/04/18 12:51, Ard Biesheuvel wrote:
>> On 23 April 2018 at 12:34, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> This series is a first shot at teaching the kernel about the oxymoron
>>> expressed in $SUBJECT. Over the past couple of years, we've seen some
>>> SoCs coming up with ways of signalling level interrupts using a new
>>> flavor of MSIs, where the MSI controller uses two distinct messages:
>>> one that raises a virtual line, and one that lowers it. The target MSI
>>> controller is in charge of maintaining the state of the line.
>>>
>>> This allows for a much simplified HW signal routing (no need to have
>>> hundreds of discrete lines to signal level interrupts if you already
>>> have a memory bus), but results in a departure from the current idea
>>> the kernel has of MSIs.
>>>
>>> This series takes a minimal approach to the problem, which is to allow
>>> MSI controllers to use not only one, but up to two messages at a
>>> time. This is controlled by a flag exposed at MSI irq domain creation,
>>> and is only supported with platform MSI.
>>>
>>> The rest of the series repaints the Marvell ICU/GICP drivers which
>>> already make use of this feature with a side-channel, and adds support
>>> for the same feature in GICv3. A side effect of the last GICv3 patch
>>> is that you can also use SPIs to signal PCI MSIs. This is a last
>>> resort measure for SoCs where the ITS is unusable for unspeakable
>>> reasons.
>>>
>>
>> Hi Marc,
>>
>> I am hitting the splat below when trying this series on SynQuacer,
>> with mbi range <64 32> (which is reserved in the h/w manual but note
>> that I haven't confirmed with Socionext whether these are expected to
>> work or not. However, I don't think that makes any difference
>> regarding the issue below.)
>
> [...]
>
> For the record: After some IRC debugging with Ard, this turns out to be
> due to two issues:
> - GICv3 is now advertising several domains, all using the same device nodes
> - irq_find_host() is picking the first one, which is the wrong one.
>
> A good way to avoid this mess is to:
> - Let GICv3 advertise its core domain with DOMAIN_BUS_WIRED (consistent
> with what the armada-370-xp driver is already doing)
> - Let irq_find_host return DOMAIN_BUS_WIRED before trying DOMAIN_BUS_ANY
> (consistent with the way irq_create_fwspec_mapping works).
>
> I've stashed the whole series at [1] with these two fixes.
>

For the fixed version (1c1eff8a4d6b14aae9a709570e692f5fcd837670)

Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> # SynQuacer

Re: [PATCH 2/7] genirq/msi: Limit level-triggered MSI to platform devices

[Thomas Gleixner]

On Mon, 23 Apr 2018, Marc Zyngier wrote:

> Nobody would be insane enough to try and use level triggered
> MSIs on PCI, but let's make sure it doesn't happen. Also,
> let's mandate that the irqchip backing the platform MSI domain
> is providing an irq_set_type method (or the whole thing would
> be a bit useless).
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/base/platform-msi.c | 3 +++
>  drivers/pci/msi.c           | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
> index 8e22073aeeed..3b9694a6beaa 100644
> --- a/drivers/base/platform-msi.c
> +++ b/drivers/base/platform-msi.c
> @@ -101,6 +101,9 @@ static void platform_msi_update_chip_ops(struct msi_domain_info *info)
>  		chip->irq_set_affinity = msi_domain_set_affinity;
>  	if (!chip->irq_write_msi_msg)
>  		chip->irq_write_msi_msg = platform_msi_write_msg;
> +	if (WARN_ON((info->flags & MSI_FLAG_LEVEL_CAPABLE) &&
> +		    !chip->irq_set_type))

Shouldn't we just require something like IRQCHIP_SUPPORTS_MSI_LEVEL or such
in chip->flags being set instead?

Thanks,

	tglx

Re: [PATCH 5/7] irqchip/gic-v3: Add support for Message Based Interrupts as an MSI controller

[Thomas Gleixner]

On Mon, 23 Apr 2018, Marc Zyngier wrote:
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic-v3-mbi.c
> @@ -0,0 +1,287 @@
> +/*
> + * Copyright (C) 2018 ARM Limited, All Rights Reserved.
> + * Author: Marc Zyngier <marc.zyngier@arm.com>

Can you please:

1) Add the proper SPDX-License-Identifier

// SPDX-License-Identifier: GPL-2.0

at the first line of the file and

2) Remove the boiler plate? Please talk to Jilayne.

> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */

> +struct mbi_range {
> +	u32			spi_start;
> +	u32			nr_spis;
> +	unsigned long		*bm;
> +};
> +
> +static spinlock_t		mbi_lock;

DEFINE_SPINLOCK() please

> +static phys_addr_t		mbi_phys_base;
> +static struct mbi_range		*mbi_ranges;
> +static unsigned int		mbi_range_nr;
> +
> +static struct irq_chip mbi_irq_chip = {
> +	.name			= "MBI",
> +	.irq_mask		= irq_chip_mask_parent,
> +	.irq_unmask		= irq_chip_unmask_parent,
> +	.irq_eoi		= irq_chip_eoi_parent,
> +	.irq_set_type		= irq_chip_set_type_parent,
> +	.irq_set_affinity	= irq_chip_set_affinity_parent,
> +};
> +
> +static int mbi_irq_gic_domain_alloc(struct irq_domain *domain,
> +				       unsigned int virq,
> +				       irq_hw_number_t hwirq)
> +{
> +	struct irq_fwspec fwspec;
> +	struct irq_data *d;
> +	int err;
> +
> +	/*
> +	 * Using ACPI? There is no MBI support in the spec, you
> +	 * shouldn't even be here.
> +	 */
> +	if (!is_of_node(domain->parent->fwnode))
> +		return -EINVAL;
> +
> +	/*
> +	 * Let's default to edge. This is consistent with traditional
> +	 * MSIs, and systems requiring level signaling will just
> +	 * enforce the trigger on their own.
> +	 */
> +	fwspec.fwnode = domain->parent->fwnode;
> +	fwspec.param_count = 3;
> +	fwspec.param[0] = 0;
> +	fwspec.param[1] = hwirq - 32;
> +	fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
> +
> +	err = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
> +	if (err)
> +		return err;
> +
> +	d = irq_domain_get_irq_data(domain->parent, virq);
> +	d->chip->irq_set_type(d, IRQ_TYPE_EDGE_RISING);

  	irq_chip_set_type_parent(d, ....) ?

> +
> +	return 0;
> +}
> +
> +static void mbi_free_msi(struct mbi_range *mbi, unsigned int hwirq,
> +			 int nr_irqs)
> +{
> +	spin_lock(&mbi_lock);
> +	bitmap_release_region(mbi->bm, hwirq - mbi->spi_start,
> +			      get_count_order(nr_irqs));
> +	spin_unlock(&mbi_lock);
> +}
> +
> +static int mbi_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> +				   unsigned int nr_irqs, void *args)
> +{
> +	struct mbi_range *mbi = NULL;
> +	int hwirq, offset, i, err = 0;
> +
> +	spin_lock(&mbi_lock);

There is no real reason that this must be a spinlock, right? Make it a
mutex please.

> +	for (i = 0; i < mbi_range_nr; i++) {
> +		offset = bitmap_find_free_region(mbi_ranges[i].bm,
> +						 mbi_ranges[i].nr_spis,
> +						 get_count_order(nr_irqs));
> +		if (offset >= 0) {
> +			mbi = &mbi_ranges[i];
> +			break;
> +		}
> +	}
> +	spin_unlock(&mbi_lock);

> +/* Platform-MSI specific irqchip */
> +static struct irq_chip mbi_pmsi_irq_chip = {
> +	.name			= "pMSI",
> +	.irq_set_type		= irq_chip_set_type_parent,
> +	.irq_compose_msi_msg	= mbi_compose_mbi_msg,

Fun. I did not expect this to work w/o any of the other callbacks. Magic!

Other than the above nits, this looks pretty good.

Thanks,

	tglx

Re: [PATCH 2/7] genirq/msi: Limit level-triggered MSI to platform devices

[Marc Zyngier]

On 26/04/18 20:50, Thomas Gleixner wrote:
> On Mon, 23 Apr 2018, Marc Zyngier wrote:
> 
>> Nobody would be insane enough to try and use level triggered
>> MSIs on PCI, but let's make sure it doesn't happen. Also,
>> let's mandate that the irqchip backing the platform MSI domain
>> is providing an irq_set_type method (or the whole thing would
>> be a bit useless).
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/base/platform-msi.c | 3 +++
>>  drivers/pci/msi.c           | 3 +++
>>  2 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
>> index 8e22073aeeed..3b9694a6beaa 100644
>> --- a/drivers/base/platform-msi.c
>> +++ b/drivers/base/platform-msi.c
>> @@ -101,6 +101,9 @@ static void platform_msi_update_chip_ops(struct msi_domain_info *info)
>>  		chip->irq_set_affinity = msi_domain_set_affinity;
>>  	if (!chip->irq_write_msi_msg)
>>  		chip->irq_write_msi_msg = platform_msi_write_msg;
>> +	if (WARN_ON((info->flags & MSI_FLAG_LEVEL_CAPABLE) &&
>> +		    !chip->irq_set_type))
> 
> Shouldn't we just require something like IRQCHIP_SUPPORTS_MSI_LEVEL or such
> in chip->flags being set instead?
That definitely makes sense. Thanks for the suggestion.

	M.
-- 
Jazz is not dead. It just smells funny...