The GICv5 architecture implements the Interrupt Wire Bridge (IWB) in
order to support wired interrupts that cannot be connected directly
to an IRS and instead uses the ITS to translate a wire event into
an IRQ signal.
An IWB is a special ITS device with its own deviceID; upon probe,
an IWB calls into the ITS driver to allocate DT/ITT tables for its
events (ie wires).
An IWB is always associated with a single ITS in the system.
An IWB is connected to an ITS and it has its own deviceID for all
interrupt wires that it manages; the IWB input wire number is
exposed to the ITS as an eventID. This eventID is not programmable
and therefore requires special handling in the ITS driver.
Add an IWB driver in order to:
- Probe IWBs in the system and allocate ITS tables
- Manage IWB IRQ domains
- Handle IWB input wires state (enable/disable)
- Add the required IWB IRQchip representation
- Handle firmware representation to Linux IRQ translation
Co-developed-by: Sascha Bischoff <sascha.bischoff@arm.com>
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
Co-developed-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <maz@kernel.org>
---
drivers/irqchip/Makefile | 2 +-
drivers/irqchip/irq-gic-v5-its.c | 68 ++++++--
drivers/irqchip/irq-gic-v5-iwb.c | 356 +++++++++++++++++++++++++++++++++++++++
drivers/irqchip/irq-gic-v5.c | 2 +
drivers/irqchip/irq-gic-v5.h | 28 +++
5 files changed, 437 insertions(+), 19 deletions(-)
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 4280395e3bdff7858102f0b4eaaea1121cace52f..7bfb2369fbe494a64b72308d95ae33de93c6b8c6 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -37,7 +37,7 @@ obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v4.o
obj-$(CONFIG_ARM_GIC_V3_ITS_FSL_MC) += irq-gic-v3-its-fsl-mc-msi.o
obj-$(CONFIG_PARTITION_PERCPU) += irq-partition-percpu.o
obj-$(CONFIG_ARM_GIC_V5) += irq-gic-v5.o irq-gic-v5-irs.o
-obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o
+obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o irq-gic-v5-iwb.o
obj-$(CONFIG_HISILICON_IRQ_MBIGEN) += irq-mbigen.o
obj-$(CONFIG_ARM_NVIC) += irq-nvic.o
obj-$(CONFIG_ARM_VIC) += irq-vic.o
diff --git a/drivers/irqchip/irq-gic-v5-its.c b/drivers/irqchip/irq-gic-v5-its.c
index da349b4709cc5ec8978859237838f039389ca4a1..b5eb4dbfe2296dc6620889eb9291b542cae4aeb6 100644
--- a/drivers/irqchip/irq-gic-v5-its.c
+++ b/drivers/irqchip/irq-gic-v5-its.c
@@ -786,9 +786,8 @@ static struct gicv5_its_dev *gicv5_its_find_device(struct gicv5_its_chip_data *i
return dev ? dev : ERR_PTR(-ENODEV);
}
-static struct gicv5_its_dev *gicv5_its_alloc_device(
- struct gicv5_its_chip_data *its, int nvec,
- u32 dev_id)
+struct gicv5_its_dev *gicv5_its_alloc_device(struct gicv5_its_chip_data *its,
+ int nvec, u32 dev_id, bool is_iwb)
{
struct gicv5_its_dev *its_dev;
int ret;
@@ -815,6 +814,7 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
its_dev->device_id = dev_id;
its_dev->num_events = nvec;
its_dev->num_mapped_events = 0;
+ its_dev->is_iwb = is_iwb;
ret = gicv5_its_device_register(its, its_dev);
if (ret) {
@@ -827,9 +827,11 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
/*
* This is the first time we have seen this device. Hence, it is not
- * shared.
+ * shared, unless it is an IWB that is a shared ITS device by
+ * definition, its eventids are hardcoded and never change - we allocate
+ * it once for all and never free it.
*/
- its_dev->shared = false;
+ its_dev->shared = is_iwb;
its_dev->its_node = its;
@@ -859,7 +861,7 @@ static int gicv5_its_msi_prepare(struct irq_domain *domain, struct device *dev,
guard(mutex)(&its->dev_alloc_lock);
- its_dev = gicv5_its_alloc_device(its, nvec, dev_id);
+ its_dev = gicv5_its_alloc_device(its, nvec, dev_id, false);
if (IS_ERR(its_dev))
return PTR_ERR(its_dev);
@@ -937,28 +939,55 @@ static void gicv5_its_free_event(struct gicv5_its_dev *its_dev, u16 event_id)
}
static int gicv5_its_alloc_eventid(struct gicv5_its_dev *its_dev,
+ msi_alloc_info_t *info,
unsigned int nr_irqs, u32 *eventid)
{
- int ret;
+ int event_id_base;
- ret = bitmap_find_free_region(its_dev->event_map,
- its_dev->num_events,
- get_count_order(nr_irqs));
+ if (!its_dev->is_iwb) {
- if (ret < 0)
- return ret;
+ event_id_base = bitmap_find_free_region(
+ its_dev->event_map, its_dev->num_events,
+ get_count_order(nr_irqs));
+ if (event_id_base < 0)
+ return event_id_base;
+ } else {
+ /*
+ * We want to have a fixed EventID mapped for the IWB.
+ */
+ if (WARN(nr_irqs != 1, "IWB requesting nr_irqs != 1\n"))
+ return -EINVAL;
- *eventid = ret;
+ event_id_base = info->scratchpad[1].ul;
+
+ if (event_id_base >= its_dev->num_events) {
+ pr_err("EventID ouside of ITT range; cannot allocate an ITT entry!\n");
+
+ return -EINVAL;
+ }
+
+ if (test_and_set_bit(event_id_base, its_dev->event_map)) {
+ pr_warn("Can't reserve event_id bitmap\n");
+ return -EINVAL;
+
+ }
+ }
+
+ *eventid = event_id_base;
return 0;
+
}
static void gicv5_its_free_eventid(struct gicv5_its_dev *its_dev,
u32 event_id_base,
unsigned int nr_irqs)
{
- bitmap_release_region(its_dev->event_map, event_id_base,
+ if (!its_dev->is_iwb)
+ bitmap_release_region(its_dev->event_map, event_id_base,
get_count_order(nr_irqs));
+ else
+ clear_bit(event_id_base, its_dev->event_map);
}
static int gicv5_its_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
@@ -986,7 +1015,7 @@ static int gicv5_its_irq_domain_alloc(struct irq_domain *domain, unsigned int vi
return PTR_ERR(its_dev);
}
- ret = gicv5_its_alloc_eventid(its_dev, nr_irqs, &event_id_base);
+ ret = gicv5_its_alloc_eventid(its_dev, info, nr_irqs, &event_id_base);
if (ret) {
mutex_unlock(&its->dev_alloc_lock);
return ret;
@@ -994,9 +1023,12 @@ static int gicv5_its_irq_domain_alloc(struct irq_domain *domain, unsigned int vi
mutex_unlock(&its->dev_alloc_lock);
- ret = iommu_dma_prepare_msi(info->desc, its->its_trans_phys_base);
- if (ret)
- goto out_eventid;
+ /* IWBs are never upstream an IOMMU */
+ if (!its_dev->is_iwb) {
+ ret = iommu_dma_prepare_msi(info->desc, its->its_trans_phys_base);
+ if (ret)
+ goto out_eventid;
+ }
for (i = 0; i < nr_irqs; i++) {
lpi = gicv5_alloc_lpi();
diff --git a/drivers/irqchip/irq-gic-v5-iwb.c b/drivers/irqchip/irq-gic-v5-iwb.c
new file mode 100644
index 0000000000000000000000000000000000000000..a0ff1467f15e6f5cf969ada3309775fdc6a67d2b
--- /dev/null
+++ b/drivers/irqchip/irq-gic-v5-iwb.c
@@ -0,0 +1,356 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024-2025 ARM Limited, All Rights Reserved.
+ */
+#define pr_fmt(fmt) "GICv5 IWB: " fmt
+
+#include <linux/init.h>
+#include <linux/irqchip.h>
+#include <linux/kernel.h>
+#include <linux/msi.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#include "irq-gic-v5.h"
+
+static u32 iwb_readl_relaxed(struct gicv5_iwb_chip_data *iwb_node,
+ const u64 reg_offset)
+{
+ return readl_relaxed(iwb_node->iwb_base + reg_offset);
+}
+
+static void iwb_writel_relaxed(struct gicv5_iwb_chip_data *iwb_node,
+ const u32 val, const u64 reg_offset)
+{
+ writel_relaxed(val, iwb_node->iwb_base + reg_offset);
+}
+
+static int gicv5_iwb_wait_for_wenabler(struct gicv5_iwb_chip_data *iwb_node)
+{
+ int ret;
+
+ ret = gicv5_wait_for_op(iwb_node->iwb_base, GICV5_IWB_WENABLE_STATUSR,
+ GICV5_IWB_WENABLE_STATUSR_IDLE, NULL);
+ if (unlikely(ret == -ETIMEDOUT))
+ pr_err_ratelimited("IWB_WENABLE_STATUSR timeout\n");
+
+ return ret;
+}
+
+static int __gicv5_iwb_set_wire_enable(struct gicv5_iwb_chip_data *iwb_node,
+ u32 iwb_wire, bool enable)
+{
+ u32 n = iwb_wire / 32;
+ u8 i = iwb_wire % 32;
+ u32 val;
+
+ if (n >= iwb_node->nr_regs) {
+ pr_err("IWB_WENABLER<n> is invalid for n=%u\n", n);
+ return -EINVAL;
+ }
+
+ /*
+ * Enable IWB wire/pin at this point
+ * Note: This is not the same as enabling the interrupt
+ */
+ val = iwb_readl_relaxed(iwb_node, GICV5_IWB_WENABLER + (4 * n));
+ if (enable)
+ val |= BIT(i);
+ else
+ val &= ~BIT(i);
+ iwb_writel_relaxed(iwb_node, val, GICV5_IWB_WENABLER + (4 * n));
+
+ return gicv5_iwb_wait_for_wenabler(iwb_node);
+}
+
+static int gicv5_iwb_enable_wire(struct gicv5_iwb_chip_data *iwb_node,
+ u32 iwb_wire)
+{
+ return __gicv5_iwb_set_wire_enable(iwb_node, iwb_wire, true);
+}
+
+static int gicv5_iwb_disable_wire(struct gicv5_iwb_chip_data *iwb_node,
+ u32 iwb_wire)
+{
+ return __gicv5_iwb_set_wire_enable(iwb_node, iwb_wire, false);
+}
+
+static int gicv5_iwb_set_type(struct irq_data *d, unsigned int type)
+{
+ struct gicv5_iwb_chip_data *iwb_node = irq_data_get_irq_chip_data(d);
+ u32 iwb_wire, n, wtmr;
+ u8 i;
+
+ iwb_wire = d->hwirq;
+
+ i = iwb_wire % 32;
+ n = iwb_wire / 32;
+
+ if (n >= iwb_node->nr_regs) {
+ pr_err_once("reg %u out of range\n", n);
+ return -EINVAL;
+ }
+
+ wtmr = iwb_readl_relaxed(iwb_node, GICV5_IWB_WTMR + (4 * n));
+
+ switch (type) {
+ case IRQ_TYPE_LEVEL_HIGH:
+ case IRQ_TYPE_LEVEL_LOW:
+ wtmr |= BIT(i);
+ break;
+ case IRQ_TYPE_EDGE_RISING:
+ case IRQ_TYPE_EDGE_FALLING:
+ wtmr &= ~BIT(i);
+ break;
+ default:
+ pr_debug("unexpected wire trigger mode");
+ return -EINVAL;
+ }
+
+ iwb_writel_relaxed(iwb_node, wtmr, GICV5_IWB_WTMR + (4 * n));
+
+ return 0;
+}
+
+static const struct irq_chip gicv5_iwb_chip = {
+ .name = "GICv5-IWB",
+ .irq_mask = irq_chip_mask_parent,
+ .irq_unmask = irq_chip_unmask_parent,
+ .irq_eoi = irq_chip_eoi_parent,
+ .irq_set_type = gicv5_iwb_set_type,
+ .irq_set_affinity = irq_chip_set_affinity_parent,
+ .irq_get_irqchip_state = irq_chip_get_parent_state,
+ .irq_set_irqchip_state = irq_chip_set_parent_state,
+ .flags = IRQCHIP_SET_TYPE_MASKED |
+ IRQCHIP_SKIP_SET_WAKE |
+ IRQCHIP_MASK_ON_SUSPEND
+};
+
+static int gicv5_iwb_irq_domain_translate(struct irq_domain *d,
+ struct irq_fwspec *fwspec,
+ irq_hw_number_t *hwirq,
+ unsigned int *type)
+{
+ if (!is_of_node(fwspec->fwnode))
+ return -EINVAL;
+
+ if (fwspec->param_count < 2)
+ return -EINVAL;
+
+ /*
+ * param[0] is be the wire
+ * param[1] is the interrupt type
+ */
+ *hwirq = fwspec->param[0];
+ *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
+
+ return 0;
+}
+
+static void gicv5_iwb_irq_domain_free(struct irq_domain *domain,
+ unsigned int virq, unsigned int nr_irqs)
+{
+ /* Free the local data, and then go up the hierarchy doing the same */
+ struct gicv5_iwb_chip_data *iwb_node = domain->host_data;
+ struct irq_data *data;
+
+ if (WARN_ON_ONCE(nr_irqs != 1))
+ return;
+
+ data = irq_domain_get_irq_data(domain, virq);
+ gicv5_iwb_disable_wire(iwb_node, data->hwirq);
+
+ irq_domain_reset_irq_data(data);
+
+ irq_domain_free_irqs_parent(domain, virq, nr_irqs);
+}
+
+/*
+ * Our parent is the ITS, which expects MSI devices with programmable
+ * event IDs. IWB event IDs are hardcoded.
+ *
+ * Use the msi_alloc_info_t structure to convey both our DeviceID
+ * (scratchpad[0]), and the wire that we are attempting to map to an LPI in
+ * the ITT (scratchpad[1]).
+ */
+static int iwb_alloc_lpi_irq_parent(struct irq_domain *domain,
+ unsigned int virq, irq_hw_number_t hwirq)
+{
+ struct gicv5_iwb_chip_data *iwb_node = domain->host_data;
+ msi_alloc_info_t info;
+
+ info.scratchpad[0].ul = iwb_node->device_id;
+ info.scratchpad[1].ul = hwirq;
+ info.hwirq = hwirq;
+
+ return irq_domain_alloc_irqs_parent(domain, virq, 1, &info);
+}
+
+static int gicv5_iwb_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ struct gicv5_iwb_chip_data *iwb_node;
+ unsigned int type = IRQ_TYPE_NONE;
+ struct irq_fwspec *fwspec = arg;
+ irq_hw_number_t hwirq;
+ struct irq_data *irqd;
+ int ret;
+
+ if (WARN_ON_ONCE(nr_irqs != 1))
+ return -EINVAL;
+
+ ret = gicv5_iwb_irq_domain_translate(domain, fwspec, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ if (iwb_alloc_lpi_irq_parent(domain, virq, hwirq))
+ return -EINVAL;
+
+ irqd = irq_get_irq_data(virq);
+ iwb_node = domain->host_data;
+
+ gicv5_iwb_enable_wire(iwb_node, hwirq);
+
+ irq_domain_set_info(domain, virq, hwirq, &gicv5_iwb_chip, iwb_node,
+ handle_fasteoi_irq, NULL, NULL);
+ irq_set_probe(virq);
+ irqd_set_single_target(irqd);
+
+ return 0;
+}
+
+static const struct irq_domain_ops gicv5_iwb_irq_domain_ops = {
+ .translate = gicv5_iwb_irq_domain_translate,
+ .alloc = gicv5_iwb_irq_domain_alloc,
+ .free = gicv5_iwb_irq_domain_free,
+};
+
+static struct gicv5_iwb_chip_data *
+__init gicv5_iwb_init_bases(void __iomem *iwb_base,
+ struct fwnode_handle *handle,
+ struct irq_domain *parent_domain, u32 device_id)
+{
+ struct gicv5_iwb_chip_data *iwb_node;
+ struct msi_domain_info *msi_info;
+ struct gicv5_its_chip_data *its;
+ struct gicv5_its_dev *its_dev;
+ u32 nr_wires, idr0, cr0;
+ int ret;
+
+ msi_info = msi_get_domain_info(parent_domain);
+ its = msi_info->data;
+ if (!its) {
+ pr_warn("IWB %pOF can't find parent ITS, bailing\n",
+ to_of_node(handle));
+ return ERR_PTR(-ENODEV);
+ }
+
+ iwb_node = kzalloc(sizeof(*iwb_node), GFP_KERNEL);
+ if (!iwb_node)
+ return ERR_PTR(-ENOMEM);
+
+ iwb_node->iwb_base = iwb_base;
+ iwb_node->device_id = device_id;
+
+ idr0 = iwb_readl_relaxed(iwb_node, GICV5_IWB_IDR0);
+ nr_wires = (FIELD_GET(GICV5_IWB_IDR0_IW_RANGE, idr0) + 1) * 32;
+
+ iwb_node->domain = irq_domain_create_hierarchy(parent_domain, 0,
+ nr_wires, handle, &gicv5_iwb_irq_domain_ops,
+ iwb_node);
+ irq_domain_update_bus_token(iwb_node->domain, DOMAIN_BUS_WIRED);
+
+ cr0 = iwb_readl_relaxed(iwb_node, GICV5_IWB_CR0);
+ if (!FIELD_GET(GICV5_IWB_CR0_IWBEN, cr0)) {
+ pr_err("IWB %s must be enabled in firmware\n",
+ fwnode_get_name(handle));
+ ret = -EINVAL;
+ goto out_free;
+ }
+
+ iwb_node->nr_regs = FIELD_GET(GICV5_IWB_IDR0_IW_RANGE, idr0) + 1;
+
+ for (unsigned int n = 0; n < iwb_node->nr_regs; n++)
+ iwb_writel_relaxed(iwb_node, 0, GICV5_IWB_WENABLER + (sizeof(u32) * n));
+
+ ret = gicv5_iwb_wait_for_wenabler(iwb_node);
+ if (ret)
+ goto out_free;
+
+ mutex_lock(&its->dev_alloc_lock);
+ its_dev = gicv5_its_alloc_device(its, roundup_pow_of_two(nr_wires),
+ device_id, true);
+ mutex_unlock(&its->dev_alloc_lock);
+ if (IS_ERR(its_dev)) {
+ ret = -ENODEV;
+ goto out_free;
+ }
+
+ return iwb_node;
+out_free:
+ irq_domain_remove(iwb_node->domain);
+ kfree(iwb_node);
+
+ return ERR_PTR(ret);
+}
+
+static int __init gicv5_iwb_of_init(struct device_node *node)
+{
+ struct gicv5_iwb_chip_data *iwb_node;
+ struct irq_domain *parent_domain;
+ struct device_node *parent_its;
+ struct of_phandle_args args;
+ void __iomem *iwb_base;
+ u32 device_id;
+ int ret;
+
+ iwb_base = of_io_request_and_map(node, 0, "IWB");
+ if (IS_ERR(iwb_base)) {
+ pr_err("%pOF: unable to map GICv5 IWB registers\n", node);
+ return PTR_ERR(iwb_base);
+ }
+
+ ret = of_parse_phandle_with_args(node, "msi-parent", "#msi-cells", 0,
+ &args);
+ if (ret) {
+ pr_err("%pOF: Can't retrieve deviceID\n", node);
+ goto out_unmap;
+ }
+
+ parent_its = args.np;
+ parent_domain = irq_find_matching_host(parent_its, DOMAIN_BUS_NEXUS);
+ if (!parent_domain) {
+ pr_err("Unable to find the parent ITS domain for %pOF!\n", node);
+ ret = -ENXIO;
+ goto out_put;
+ }
+
+ device_id = args.args[0];
+ pr_debug("IWB deviceID: 0x%x\n", device_id);
+
+ iwb_node = gicv5_iwb_init_bases(iwb_base, &node->fwnode, parent_domain,
+ device_id);
+ if (IS_ERR(iwb_node)) {
+ ret = PTR_ERR(iwb_node);
+ goto out_put;
+ }
+
+ return 0;
+out_put:
+ of_node_put(parent_its);
+out_unmap:
+ iounmap(iwb_base);
+ return ret;
+}
+
+void __init gicv5_iwb_of_probe(void)
+{
+ struct device_node *np;
+ int ret;
+
+ for_each_compatible_node(np, NULL, "arm,gic-v5-iwb") {
+ ret = gicv5_iwb_of_init(np);
+ if (ret)
+ pr_err("Failed to init IWB %s\n", np->full_name);
+ }
+}
diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
index e483d0774936035b5cf2407da9a65d776bad3138..0eaf1f40073d2659b60b3fa5ac06c66a9e4f2e2f 100644
--- a/drivers/irqchip/irq-gic-v5.c
+++ b/drivers/irqchip/irq-gic-v5.c
@@ -1047,6 +1047,8 @@ static int __init gicv5_of_init(struct device_node *node,
gicv5_irs_its_probe();
+ gicv5_iwb_of_probe();
+
return 0;
out_int:
gicv5_cpu_disable_interrupts();
diff --git a/drivers/irqchip/irq-gic-v5.h b/drivers/irqchip/irq-gic-v5.h
index f5a453599493020b36d9c7f18c08171c51ba8669..a71504cbee2d08c8ff54fb409be72373945bc65a 100644
--- a/drivers/irqchip/irq-gic-v5.h
+++ b/drivers/irqchip/irq-gic-v5.h
@@ -238,6 +238,20 @@
#define GICV5_ITS_HWIRQ_DEVICE_ID GENMASK_ULL(31, 0)
#define GICV5_ITS_HWIRQ_EVENT_ID GENMASK_ULL(63, 32)
+#define GICV5_IWB_IDR0 0x0000
+#define GICV5_IWB_CR0 0x0080
+#define GICV5_IWB_WENABLE_STATUSR 0x00c0
+#define GICV5_IWB_WENABLER 0x2000
+#define GICV5_IWB_WTMR 0x4000
+
+#define GICV5_IWB_IDR0_INT_DOMS GENMASK(14, 11)
+#define GICV5_IWB_IDR0_IW_RANGE GENMASK(10, 0)
+
+#define GICV5_IWB_CR0_IDLE BIT(1)
+#define GICV5_IWB_CR0_IWBEN BIT(0)
+
+#define GICV5_IWB_WENABLE_STATUSR_IDLE BIT(0)
+
struct gicv5_chip_data {
struct fwnode_handle *fwnode;
struct irq_domain *ppi_domain;
@@ -348,6 +362,7 @@ struct gicv5_its_dev {
u32 num_events;
u32 num_mapped_events;
bool shared;
+ bool is_iwb;
};
void gicv5_init_lpis(u32 max);
@@ -357,4 +372,17 @@ int gicv5_alloc_lpi(void);
void gicv5_free_lpi(u32 lpi);
void __init gicv5_its_of_probe(struct device_node *parent);
+struct gicv5_its_dev *gicv5_its_alloc_device(struct gicv5_its_chip_data *its,
+ int nvec, u32 dev_id, bool is_iwb);
+
+struct gicv5_iwb_chip_data {
+ void __iomem *iwb_base;
+ struct irq_domain *domain;
+ u64 flags;
+ u32 device_id;
+ u16 nr_regs;
+};
+
+void gicv5_iwb_of_probe(void);
+
#endif
--
2.48.0On Thu, 24 Apr 2025 11:25:32 +0100,
Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
>
> The GICv5 architecture implements the Interrupt Wire Bridge (IWB) in
> order to support wired interrupts that cannot be connected directly
> to an IRS and instead uses the ITS to translate a wire event into
> an IRQ signal.
>
> An IWB is a special ITS device with its own deviceID; upon probe,
> an IWB calls into the ITS driver to allocate DT/ITT tables for its
> events (ie wires).
>
> An IWB is always associated with a single ITS in the system.
>
> An IWB is connected to an ITS and it has its own deviceID for all
> interrupt wires that it manages; the IWB input wire number is
> exposed to the ITS as an eventID. This eventID is not programmable
> and therefore requires special handling in the ITS driver.
>
> Add an IWB driver in order to:
>
> - Probe IWBs in the system and allocate ITS tables
> - Manage IWB IRQ domains
> - Handle IWB input wires state (enable/disable)
> - Add the required IWB IRQchip representation
> - Handle firmware representation to Linux IRQ translation
>
> Co-developed-by: Sascha Bischoff <sascha.bischoff@arm.com>
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> Co-developed-by: Timothy Hayes <timothy.hayes@arm.com>
> Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Marc Zyngier <maz@kernel.org>
> ---
> drivers/irqchip/Makefile | 2 +-
> drivers/irqchip/irq-gic-v5-its.c | 68 ++++++--
> drivers/irqchip/irq-gic-v5-iwb.c | 356 +++++++++++++++++++++++++++++++++++++++
> drivers/irqchip/irq-gic-v5.c | 2 +
> drivers/irqchip/irq-gic-v5.h | 28 +++
> 5 files changed, 437 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 4280395e3bdff7858102f0b4eaaea1121cace52f..7bfb2369fbe494a64b72308d95ae33de93c6b8c6 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -37,7 +37,7 @@ obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v4.o
> obj-$(CONFIG_ARM_GIC_V3_ITS_FSL_MC) += irq-gic-v3-its-fsl-mc-msi.o
> obj-$(CONFIG_PARTITION_PERCPU) += irq-partition-percpu.o
> obj-$(CONFIG_ARM_GIC_V5) += irq-gic-v5.o irq-gic-v5-irs.o
> -obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o
> +obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o irq-gic-v5-iwb.o
> obj-$(CONFIG_HISILICON_IRQ_MBIGEN) += irq-mbigen.o
> obj-$(CONFIG_ARM_NVIC) += irq-nvic.o
> obj-$(CONFIG_ARM_VIC) += irq-vic.o
> diff --git a/drivers/irqchip/irq-gic-v5-its.c b/drivers/irqchip/irq-gic-v5-its.c
> index da349b4709cc5ec8978859237838f039389ca4a1..b5eb4dbfe2296dc6620889eb9291b542cae4aeb6 100644
> --- a/drivers/irqchip/irq-gic-v5-its.c
> +++ b/drivers/irqchip/irq-gic-v5-its.c
> @@ -786,9 +786,8 @@ static struct gicv5_its_dev *gicv5_its_find_device(struct gicv5_its_chip_data *i
> return dev ? dev : ERR_PTR(-ENODEV);
> }
>
> -static struct gicv5_its_dev *gicv5_its_alloc_device(
> - struct gicv5_its_chip_data *its, int nvec,
> - u32 dev_id)
> +struct gicv5_its_dev *gicv5_its_alloc_device(struct gicv5_its_chip_data *its,
> + int nvec, u32 dev_id, bool is_iwb)
> {
> struct gicv5_its_dev *its_dev;
> int ret;
> @@ -815,6 +814,7 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
> its_dev->device_id = dev_id;
> its_dev->num_events = nvec;
> its_dev->num_mapped_events = 0;
> + its_dev->is_iwb = is_iwb;
>
> ret = gicv5_its_device_register(its, its_dev);
> if (ret) {
> @@ -827,9 +827,11 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
>
> /*
> * This is the first time we have seen this device. Hence, it is not
> - * shared.
> + * shared, unless it is an IWB that is a shared ITS device by
> + * definition, its eventids are hardcoded and never change - we allocate
> + * it once for all and never free it.
I'm not convinced the IWB should be treated differently from any other
device. Its lifetime is not tied to its inputs, so all that's needed
is to probe it, get a bunch of interrupts, and that's about it.
The other thing is that the IWB really is a standalone thing. It
shouldn't have its fingers in the ITS code, and should only rely on
the core infrastructure to get its interrupts.
As much as I dislike it, the MBIGEN actually provides a decent example
of how this could be structured.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On Wed, Apr 30, 2025 at 12:57:01PM +0100, Marc Zyngier wrote:
> On Thu, 24 Apr 2025 11:25:32 +0100,
> Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> >
> > The GICv5 architecture implements the Interrupt Wire Bridge (IWB) in
> > order to support wired interrupts that cannot be connected directly
> > to an IRS and instead uses the ITS to translate a wire event into
> > an IRQ signal.
> >
> > An IWB is a special ITS device with its own deviceID; upon probe,
> > an IWB calls into the ITS driver to allocate DT/ITT tables for its
> > events (ie wires).
> >
> > An IWB is always associated with a single ITS in the system.
> >
> > An IWB is connected to an ITS and it has its own deviceID for all
> > interrupt wires that it manages; the IWB input wire number is
> > exposed to the ITS as an eventID. This eventID is not programmable
> > and therefore requires special handling in the ITS driver.
> >
> > Add an IWB driver in order to:
> >
> > - Probe IWBs in the system and allocate ITS tables
> > - Manage IWB IRQ domains
> > - Handle IWB input wires state (enable/disable)
> > - Add the required IWB IRQchip representation
> > - Handle firmware representation to Linux IRQ translation
> >
> > Co-developed-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > Co-developed-by: Timothy Hayes <timothy.hayes@arm.com>
> > Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> > Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Marc Zyngier <maz@kernel.org>
> > ---
> > drivers/irqchip/Makefile | 2 +-
> > drivers/irqchip/irq-gic-v5-its.c | 68 ++++++--
> > drivers/irqchip/irq-gic-v5-iwb.c | 356 +++++++++++++++++++++++++++++++++++++++
> > drivers/irqchip/irq-gic-v5.c | 2 +
> > drivers/irqchip/irq-gic-v5.h | 28 +++
> > 5 files changed, 437 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index 4280395e3bdff7858102f0b4eaaea1121cace52f..7bfb2369fbe494a64b72308d95ae33de93c6b8c6 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -37,7 +37,7 @@ obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v4.o
> > obj-$(CONFIG_ARM_GIC_V3_ITS_FSL_MC) += irq-gic-v3-its-fsl-mc-msi.o
> > obj-$(CONFIG_PARTITION_PERCPU) += irq-partition-percpu.o
> > obj-$(CONFIG_ARM_GIC_V5) += irq-gic-v5.o irq-gic-v5-irs.o
> > -obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o
> > +obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o irq-gic-v5-iwb.o
> > obj-$(CONFIG_HISILICON_IRQ_MBIGEN) += irq-mbigen.o
> > obj-$(CONFIG_ARM_NVIC) += irq-nvic.o
> > obj-$(CONFIG_ARM_VIC) += irq-vic.o
> > diff --git a/drivers/irqchip/irq-gic-v5-its.c b/drivers/irqchip/irq-gic-v5-its.c
> > index da349b4709cc5ec8978859237838f039389ca4a1..b5eb4dbfe2296dc6620889eb9291b542cae4aeb6 100644
> > --- a/drivers/irqchip/irq-gic-v5-its.c
> > +++ b/drivers/irqchip/irq-gic-v5-its.c
> > @@ -786,9 +786,8 @@ static struct gicv5_its_dev *gicv5_its_find_device(struct gicv5_its_chip_data *i
> > return dev ? dev : ERR_PTR(-ENODEV);
> > }
> >
> > -static struct gicv5_its_dev *gicv5_its_alloc_device(
> > - struct gicv5_its_chip_data *its, int nvec,
> > - u32 dev_id)
> > +struct gicv5_its_dev *gicv5_its_alloc_device(struct gicv5_its_chip_data *its,
> > + int nvec, u32 dev_id, bool is_iwb)
> > {
> > struct gicv5_its_dev *its_dev;
> > int ret;
> > @@ -815,6 +814,7 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
> > its_dev->device_id = dev_id;
> > its_dev->num_events = nvec;
> > its_dev->num_mapped_events = 0;
> > + its_dev->is_iwb = is_iwb;
> >
> > ret = gicv5_its_device_register(its, its_dev);
> > if (ret) {
> > @@ -827,9 +827,11 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
> >
> > /*
> > * This is the first time we have seen this device. Hence, it is not
> > - * shared.
> > + * shared, unless it is an IWB that is a shared ITS device by
> > + * definition, its eventids are hardcoded and never change - we allocate
> > + * it once for all and never free it.
>
> I'm not convinced the IWB should be treated differently from any other
> device. Its lifetime is not tied to its inputs, so all that's needed
> is to probe it, get a bunch of interrupts, and that's about it.
>
> The other thing is that the IWB really is a standalone thing. It
> shouldn't have its fingers in the ITS code, and should only rely on
> the core infrastructure to get its interrupts.
>
> As much as I dislike it, the MBIGEN actually provides a decent example
> of how this could be structured.
I wrote a diff against the heavily reworked series in progress I have,
(so it does not apply on v2 - headers moved) with what I came up with
for the IWB MBIgen like. It works - it removes lots of boilerplate code
but there is a hack we never really liked in:
gicv5_its_msi_prepare()
that is, using the OF compatible string to detect if we are an IWB or not.
If we are, we use the msi_alloc_info_t->hwirq to define the LPI eventid,
basically the IWB wire, if not we just allocate an eventid available from
the device bitmap.
Other than that (and being forced to provide an IWB irqchip.irq_write_msi_msg()
pointer even if the IWB can't write anything otherwise we dereference
NULL) this works.
Is there a better way to implement this ? I would post this code with
v3 but instead of waiting I thought I could inline it here, feel free
to ignore it (or flame me if it is a solved problem I failed to spot,
we need to find a way for the IWB driver to pass the "fixed event" info
to the ITS - IWB eventIDs are hardwired it is not like the MBIgen where
the irq_write_msi_msg() callback programs the wire-to-eventid
translation in HW).
Thanks,
Lorenzo
-- >8 --
diff --git i/drivers/irqchip/irq-gic-v5-its.c w/drivers/irqchip/irq-gic-v5-its.c
index 7d35f3fe4fd9..96e8e1df53f3 100644
--- i/drivers/irqchip/irq-gic-v5-its.c
+++ w/drivers/irqchip/irq-gic-v5-its.c
@@ -826,13 +826,14 @@ static int gicv5_its_msi_prepare(struct irq_domain *domain, struct device *dev,
struct msi_domain_info *msi_info;
struct gicv5_its_chip_data *its;
struct gicv5_its_dev *its_dev;
+ bool is_iwb = of_device_is_compatible(dev->of_node, "arm,gic-v5-iwb");
msi_info = msi_get_domain_info(domain);
its = msi_info->data;
guard(mutex)(&its->dev_alloc_lock);
- its_dev = gicv5_its_alloc_device(its, nvec, dev_id, false);
+ its_dev = gicv5_its_alloc_device(its, nvec, dev_id, is_iwb);
if (IS_ERR(its_dev))
return PTR_ERR(its_dev);
@@ -929,7 +930,7 @@ static int gicv5_its_alloc_eventid(struct gicv5_its_dev *its_dev,
if (WARN(nr_irqs != 1, "IWB requesting nr_irqs != 1\n"))
return -EINVAL;
- event_id_base = info->scratchpad[1].ul;
+ event_id_base = info->hwirq;
if (event_id_base >= its_dev->num_events) {
pr_err("EventID ouside of ITT range; cannot allocate an ITT entry!\n");
diff --git i/drivers/irqchip/irq-gic-v5-iwb.c w/drivers/irqchip/irq-gic-v5-iwb.c
index c8bbfe877eda..ef801ca31f0c 100644
--- i/drivers/irqchip/irq-gic-v5-iwb.c
+++ w/drivers/irqchip/irq-gic-v5-iwb.c
@@ -11,6 +11,12 @@
#include <linux/msi.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/of_platform.h>
+
+struct gicv5_iwb_chip_data {
+ void __iomem *iwb_base;
+ u16 nr_regs;
+};
static u32 iwb_readl_relaxed(struct gicv5_iwb_chip_data *iwb_node,
const u64 reg_offset)
@@ -74,6 +80,22 @@ static int gicv5_iwb_disable_wire(struct gicv5_iwb_chip_data *iwb_node,
return __gicv5_iwb_set_wire_enable(iwb_node, iwb_wire, false);
}
+static void gicv5_iwb_irq_disable(struct irq_data *d)
+{
+ struct gicv5_iwb_chip_data *iwb_node = irq_data_get_irq_chip_data(d);
+
+ gicv5_iwb_disable_wire(iwb_node, d->hwirq);
+ irq_chip_disable_parent(d);
+}
+
+static void gicv5_iwb_irq_enable(struct irq_data *d)
+{
+ struct gicv5_iwb_chip_data *iwb_node = irq_data_get_irq_chip_data(d);
+
+ gicv5_iwb_enable_wire(iwb_node, d->hwirq);
+ irq_chip_enable_parent(d);
+}
+
static int gicv5_iwb_set_type(struct irq_data *d, unsigned int type)
{
struct gicv5_iwb_chip_data *iwb_node = irq_data_get_irq_chip_data(d);
@@ -111,19 +133,11 @@ static int gicv5_iwb_set_type(struct irq_data *d, unsigned int type)
return 0;
}
-static const struct irq_chip gicv5_iwb_chip = {
- .name = "GICv5-IWB",
- .irq_mask = irq_chip_mask_parent,
- .irq_unmask = irq_chip_unmask_parent,
- .irq_eoi = irq_chip_eoi_parent,
- .irq_set_type = gicv5_iwb_set_type,
- .irq_set_affinity = irq_chip_set_affinity_parent,
- .irq_get_irqchip_state = irq_chip_get_parent_state,
- .irq_set_irqchip_state = irq_chip_set_parent_state,
- .flags = IRQCHIP_SET_TYPE_MASKED |
- IRQCHIP_SKIP_SET_WAKE |
- IRQCHIP_MASK_ON_SUSPEND
-};
+static void gicv5_iwb_domain_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc)
+{
+ arg->desc = desc;
+ arg->hwirq = (u32)desc->data.icookie.value;
+}
static int gicv5_iwb_irq_domain_translate(struct irq_domain *d,
struct irq_fwspec *fwspec,
@@ -146,123 +160,67 @@ static int gicv5_iwb_irq_domain_translate(struct irq_domain *d,
return 0;
}
-static void gicv5_iwb_irq_domain_free(struct irq_domain *domain,
- unsigned int virq, unsigned int nr_irqs)
-{
- /* Free the local data, and then go up the hierarchy doing the same */
- struct gicv5_iwb_chip_data *iwb_node = domain->host_data;
- struct irq_data *data;
+static void gicv5_iwb_write_msi_msg(struct irq_data *d, struct msi_msg *msg) {}
- if (WARN_ON_ONCE(nr_irqs != 1))
- return;
+static const struct msi_domain_template iwb_msi_template = {
+ .chip = {
+ .name = "GICv5-IWB",
+ .irq_mask = irq_chip_mask_parent,
+ .irq_unmask = irq_chip_unmask_parent,
+ .irq_enable = gicv5_iwb_irq_enable,
+ .irq_disable = gicv5_iwb_irq_disable,
+ .irq_eoi = irq_chip_eoi_parent,
+ .irq_set_type = gicv5_iwb_set_type,
+ .irq_write_msi_msg = gicv5_iwb_write_msi_msg,
+ .irq_set_affinity = irq_chip_set_affinity_parent,
+ .irq_get_irqchip_state = irq_chip_get_parent_state,
+ .irq_set_irqchip_state = irq_chip_set_parent_state,
+ .flags = IRQCHIP_SET_TYPE_MASKED |
+ IRQCHIP_SKIP_SET_WAKE |
+ IRQCHIP_MASK_ON_SUSPEND
+ },
- data = irq_domain_get_irq_data(domain, virq);
- gicv5_iwb_disable_wire(iwb_node, data->hwirq);
+ .ops = {
+ .set_desc = gicv5_iwb_domain_set_desc,
+ .msi_translate = gicv5_iwb_irq_domain_translate,
+ },
- irq_domain_reset_irq_data(data);
-
- irq_domain_free_irqs_parent(domain, virq, nr_irqs);
-}
-
-/*
- * Our parent is the ITS, which expects MSI devices with programmable
- * event IDs. IWB event IDs are hardcoded.
- *
- * Use the msi_alloc_info_t structure to convey both our DeviceID
- * (scratchpad[0]), and the wire that we are attempting to map to an LPI in
- * the ITT (scratchpad[1]).
- */
-static int iwb_alloc_lpi_irq_parent(struct irq_domain *domain,
- unsigned int virq, irq_hw_number_t hwirq)
-{
- struct gicv5_iwb_chip_data *iwb_node = domain->host_data;
- msi_alloc_info_t info;
-
- info.scratchpad[0].ul = iwb_node->device_id;
- info.scratchpad[1].ul = hwirq;
- info.hwirq = hwirq;
-
- return irq_domain_alloc_irqs_parent(domain, virq, 1, &info);
-}
-
-static int gicv5_iwb_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
- unsigned int nr_irqs, void *arg)
-{
- struct gicv5_iwb_chip_data *iwb_node;
- unsigned int type = IRQ_TYPE_NONE;
- struct irq_fwspec *fwspec = arg;
- irq_hw_number_t hwirq;
- struct irq_data *irqd;
- int ret;
-
- if (WARN_ON_ONCE(nr_irqs != 1))
- return -EINVAL;
-
- ret = gicv5_iwb_irq_domain_translate(domain, fwspec, &hwirq, &type);
- if (ret)
- return ret;
-
- if (iwb_alloc_lpi_irq_parent(domain, virq, hwirq))
- return -EINVAL;
-
- irqd = irq_get_irq_data(virq);
- iwb_node = domain->host_data;
-
- gicv5_iwb_enable_wire(iwb_node, hwirq);
-
- irq_domain_set_info(domain, virq, hwirq, &gicv5_iwb_chip, iwb_node,
- handle_fasteoi_irq, NULL, NULL);
- irq_set_probe(virq);
- irqd_set_single_target(irqd);
-
- return 0;
-}
-
-static const struct irq_domain_ops gicv5_iwb_irq_domain_ops = {
- .translate = gicv5_iwb_irq_domain_translate,
- .alloc = gicv5_iwb_irq_domain_alloc,
- .free = gicv5_iwb_irq_domain_free,
+ .info = {
+ .bus_token = DOMAIN_BUS_WIRED_TO_MSI,
+ .flags = MSI_FLAG_USE_DEV_FWNODE,
+ },
};
+static bool gicv5_iwb_create_device_domain(struct device *dev, unsigned int size,
+ struct gicv5_iwb_chip_data *iwb_node)
+{
+ if (WARN_ON_ONCE(!dev->msi.domain))
+ return false;
+
+ return msi_create_device_irq_domain(dev, MSI_DEFAULT_DOMAIN,
+ &iwb_msi_template, size,
+ NULL, iwb_node);
+}
+
static struct gicv5_iwb_chip_data *
-__init gicv5_iwb_init_bases(void __iomem *iwb_base,
- struct fwnode_handle *handle,
- struct irq_domain *parent_domain, u32 device_id)
+gicv5_iwb_init_bases(void __iomem *iwb_base, struct platform_device *pdev)
{
struct gicv5_iwb_chip_data *iwb_node;
- struct msi_domain_info *msi_info;
- struct gicv5_its_chip_data *its;
- struct gicv5_its_dev *its_dev;
u32 nr_wires, idr0, cr0;
int ret;
- msi_info = msi_get_domain_info(parent_domain);
- its = msi_info->data;
- if (!its) {
- pr_warn("IWB %pOF can't find parent ITS, bailing\n",
- to_of_node(handle));
- return ERR_PTR(-ENODEV);
- }
-
iwb_node = kzalloc(sizeof(*iwb_node), GFP_KERNEL);
if (!iwb_node)
return ERR_PTR(-ENOMEM);
iwb_node->iwb_base = iwb_base;
- iwb_node->device_id = device_id;
idr0 = iwb_readl_relaxed(iwb_node, GICV5_IWB_IDR0);
nr_wires = (FIELD_GET(GICV5_IWB_IDR0_IW_RANGE, idr0) + 1) * 32;
- iwb_node->domain = irq_domain_create_hierarchy(parent_domain, 0,
- nr_wires, handle, &gicv5_iwb_irq_domain_ops,
- iwb_node);
- irq_domain_update_bus_token(iwb_node->domain, DOMAIN_BUS_WIRED);
-
cr0 = iwb_readl_relaxed(iwb_node, GICV5_IWB_CR0);
if (!FIELD_GET(GICV5_IWB_CR0_IWBEN, cr0)) {
- pr_err("IWB %s must be enabled in firmware\n",
- fwnode_get_name(handle));
+ dev_err(&pdev->dev, "IWB must be enabled in firmware\n");
ret = -EINVAL;
goto out_free;
}
@@ -276,80 +234,60 @@ __init gicv5_iwb_init_bases(void __iomem *iwb_base,
if (ret)
goto out_free;
- mutex_lock(&its->dev_alloc_lock);
- its_dev = gicv5_its_alloc_device(its, roundup_pow_of_two(nr_wires),
- device_id, true);
- mutex_unlock(&its->dev_alloc_lock);
- if (IS_ERR(its_dev)) {
- ret = -ENODEV;
+ if (!gicv5_iwb_create_device_domain(&pdev->dev, nr_wires, iwb_node)) {
+ ret = -ENOMEM;
goto out_free;
}
return iwb_node;
out_free:
- irq_domain_remove(iwb_node->domain);
kfree(iwb_node);
return ERR_PTR(ret);
}
-static int __init gicv5_iwb_of_init(struct device_node *node)
+static int gicv5_iwb_device_probe(struct platform_device *pdev)
{
struct gicv5_iwb_chip_data *iwb_node;
- struct irq_domain *parent_domain;
- struct device_node *parent_its;
- struct of_phandle_args args;
void __iomem *iwb_base;
- u32 device_id;
+ struct resource *res;
int ret;
- iwb_base = of_io_request_and_map(node, 0, "IWB");
- if (IS_ERR(iwb_base)) {
- pr_err("%pOF: unable to map GICv5 IWB registers\n", node);
- return PTR_ERR(iwb_base);
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -EINVAL;
+
+ iwb_base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+ if (!iwb_base) {
+ dev_err(&pdev->dev, "failed to ioremap %pR\n", res);
+ return -ENOMEM;
}
- ret = of_parse_phandle_with_args(node, "msi-parent", "#msi-cells", 0,
- &args);
- if (ret) {
- pr_err("%pOF: Can't retrieve deviceID\n", node);
+ iwb_node = gicv5_iwb_init_bases(iwb_base, pdev);
+ if (IS_ERR(iwb_node)) {
+ ret = PTR_ERR(iwb_node);
goto out_unmap;
}
- parent_its = args.np;
- parent_domain = irq_find_matching_host(parent_its, DOMAIN_BUS_NEXUS);
- if (!parent_domain) {
- pr_err("Unable to find the parent ITS domain for %pOF!\n", node);
- ret = -ENXIO;
- goto out_put;
- }
-
- device_id = args.args[0];
- pr_debug("IWB deviceID: 0x%x\n", device_id);
-
- iwb_node = gicv5_iwb_init_bases(iwb_base, &node->fwnode, parent_domain,
- device_id);
- if (IS_ERR(iwb_node)) {
- ret = PTR_ERR(iwb_node);
- goto out_put;
- }
-
return 0;
-out_put:
- of_node_put(parent_its);
out_unmap:
iounmap(iwb_base);
return ret;
}
-void __init gicv5_iwb_of_probe(void)
-{
- struct device_node *np;
- int ret;
+static const struct of_device_id gicv5_iwb_of_match[] = {
+ { .compatible = "arm,gic-v5-iwb" },
+ { /* END */ }
+};
+MODULE_DEVICE_TABLE(of, gicv5_iwb_of_match);
- for_each_compatible_node(np, NULL, "arm,gic-v5-iwb") {
- ret = gicv5_iwb_of_init(np);
- if (ret)
- pr_err("Failed to init IWB %s\n", np->full_name);
- }
-}
+static struct platform_driver gicv5_iwb_platform_driver = {
+ .driver = {
+ .name = "GICv5 IWB",
+ .of_match_table = gicv5_iwb_of_match,
+ .suppress_bind_attrs = true,
+ },
+ .probe = gicv5_iwb_device_probe,
+};
+
+module_platform_driver(gicv5_iwb_platform_driver);
diff --git i/drivers/irqchip/irq-gic-v5.c w/drivers/irqchip/irq-gic-v5.c
index 1d00ad1cd1fc..c1f26718c350 100644
--- i/drivers/irqchip/irq-gic-v5.c
+++ w/drivers/irqchip/irq-gic-v5.c
@@ -1030,8 +1030,6 @@ static int __init gicv5_of_init(struct device_node *node,
gicv5_irs_its_probe();
- gicv5_iwb_of_probe();
-
return 0;
out_int:
gicv5_cpu_disable_interrupts();
diff --git i/include/linux/irqchip/arm-gic-v5.h w/include/linux/irqchip/arm-gic-v5.h
index e28e08bccbb3..05e17223733d 100644
--- i/include/linux/irqchip/arm-gic-v5.h
+++ w/include/linux/irqchip/arm-gic-v5.h
@@ -372,17 +372,4 @@ int gicv5_alloc_lpi(void);
void gicv5_free_lpi(u32 lpi);
void __init gicv5_its_of_probe(struct device_node *parent);
-struct gicv5_its_dev *gicv5_its_alloc_device(struct gicv5_its_chip_data *its,
- int nvec, u32 dev_id, bool is_iwb);
-
-struct gicv5_iwb_chip_data {
- void __iomem *iwb_base;
- struct irq_domain *domain;
- u64 flags;
- u32 device_id;
- u16 nr_regs;
-};
-
-void gicv5_iwb_of_probe(void);
-
#endif
On Wed, 30 Apr 2025 17:25:24 +0100, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote: > > I wrote a diff against the heavily reworked series in progress I have, > (so it does not apply on v2 - headers moved) with what I came up with > for the IWB MBIgen like. It works - it removes lots of boilerplate code > but there is a hack we never really liked in: > > gicv5_its_msi_prepare() > > that is, using the OF compatible string to detect if we are an IWB or not. You shouldn't need that. The MSI_FLAG_USE_DEV_FWNODE should be a good enough indication that this is something of interest, and that ends-up in the .init_dev_msi_info() callback. > If we are, we use the msi_alloc_info_t->hwirq to define the LPI eventid, > basically the IWB wire, if not we just allocate an eventid available from > the device bitmap. > > Other than that (and being forced to provide an IWB irqchip.irq_write_msi_msg() > pointer even if the IWB can't write anything otherwise we dereference > NULL) this works. Not even MBIGEN allows you to change the event. If you really want to ensure things are even tighter, invent a MSI_FLAG_HARDCODED_MSG flag, and pass that down the prepare path. > Is there a better way to implement this ? I would post this code with > v3 but instead of waiting I thought I could inline it here, feel free > to ignore it (or flame me if it is a solved problem I failed to spot, > we need to find a way for the IWB driver to pass the "fixed event" info > to the ITS - IWB eventIDs are hardwired it is not like the MBIgen where > the irq_write_msi_msg() callback programs the wire-to-eventid > translation in HW). It's *exactly* the same. And see above for a potential explicit solution. The empty irq_write_msi_msg() is not a problem. It's actually pretty clean, given how the whole thing works. Please fold this into your v3, and we'll take it from there. M. -- Without deviation from the norm, progress is not possible.
On Thu, May 01, 2025 at 03:15:46PM +0100, Marc Zyngier wrote: [...] > > If we are, we use the msi_alloc_info_t->hwirq to define the LPI eventid, > > basically the IWB wire, if not we just allocate an eventid available from > > the device bitmap. > > > > Other than that (and being forced to provide an IWB irqchip.irq_write_msi_msg() > > pointer even if the IWB can't write anything otherwise we dereference > > NULL) this works. > > Not even MBIGEN allows you to change the event. If you really want to > ensure things are even tighter, invent a MSI_FLAG_HARDCODED_MSG flag, > and pass that down the prepare path. I tried to set a new alloc flag in the IWB msi_domain_template.ops.set_desc() callback and it works. It can be set in the IWB driver (and does not change anything else), it works so happy days. > > Is there a better way to implement this ? I would post this code with > > v3 but instead of waiting I thought I could inline it here, feel free > > to ignore it (or flame me if it is a solved problem I failed to spot, > > we need to find a way for the IWB driver to pass the "fixed event" info > > to the ITS - IWB eventIDs are hardwired it is not like the MBIgen where > > the irq_write_msi_msg() callback programs the wire-to-eventid > > translation in HW). > > It's *exactly* the same. And see above for a potential explicit > solution. The empty irq_write_msi_msg() is not a problem. It's > actually pretty clean, given how the whole thing works. > > Please fold this into your v3, and we'll take it from there. I will with the new alloc flag above, thanks. Lorenzo
On Wed, Apr 30, 2025 at 12:57:01PM +0100, Marc Zyngier wrote:
> On Thu, 24 Apr 2025 11:25:32 +0100,
> Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> >
> > The GICv5 architecture implements the Interrupt Wire Bridge (IWB) in
> > order to support wired interrupts that cannot be connected directly
> > to an IRS and instead uses the ITS to translate a wire event into
> > an IRQ signal.
> >
> > An IWB is a special ITS device with its own deviceID; upon probe,
> > an IWB calls into the ITS driver to allocate DT/ITT tables for its
> > events (ie wires).
> >
> > An IWB is always associated with a single ITS in the system.
> >
> > An IWB is connected to an ITS and it has its own deviceID for all
> > interrupt wires that it manages; the IWB input wire number is
> > exposed to the ITS as an eventID. This eventID is not programmable
> > and therefore requires special handling in the ITS driver.
> >
> > Add an IWB driver in order to:
> >
> > - Probe IWBs in the system and allocate ITS tables
> > - Manage IWB IRQ domains
> > - Handle IWB input wires state (enable/disable)
> > - Add the required IWB IRQchip representation
> > - Handle firmware representation to Linux IRQ translation
> >
> > Co-developed-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > Co-developed-by: Timothy Hayes <timothy.hayes@arm.com>
> > Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> > Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Marc Zyngier <maz@kernel.org>
> > ---
> > drivers/irqchip/Makefile | 2 +-
> > drivers/irqchip/irq-gic-v5-its.c | 68 ++++++--
> > drivers/irqchip/irq-gic-v5-iwb.c | 356 +++++++++++++++++++++++++++++++++++++++
> > drivers/irqchip/irq-gic-v5.c | 2 +
> > drivers/irqchip/irq-gic-v5.h | 28 +++
> > 5 files changed, 437 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index 4280395e3bdff7858102f0b4eaaea1121cace52f..7bfb2369fbe494a64b72308d95ae33de93c6b8c6 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -37,7 +37,7 @@ obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v4.o
> > obj-$(CONFIG_ARM_GIC_V3_ITS_FSL_MC) += irq-gic-v3-its-fsl-mc-msi.o
> > obj-$(CONFIG_PARTITION_PERCPU) += irq-partition-percpu.o
> > obj-$(CONFIG_ARM_GIC_V5) += irq-gic-v5.o irq-gic-v5-irs.o
> > -obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o
> > +obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o irq-gic-v5-iwb.o
> > obj-$(CONFIG_HISILICON_IRQ_MBIGEN) += irq-mbigen.o
> > obj-$(CONFIG_ARM_NVIC) += irq-nvic.o
> > obj-$(CONFIG_ARM_VIC) += irq-vic.o
> > diff --git a/drivers/irqchip/irq-gic-v5-its.c b/drivers/irqchip/irq-gic-v5-its.c
> > index da349b4709cc5ec8978859237838f039389ca4a1..b5eb4dbfe2296dc6620889eb9291b542cae4aeb6 100644
> > --- a/drivers/irqchip/irq-gic-v5-its.c
> > +++ b/drivers/irqchip/irq-gic-v5-its.c
> > @@ -786,9 +786,8 @@ static struct gicv5_its_dev *gicv5_its_find_device(struct gicv5_its_chip_data *i
> > return dev ? dev : ERR_PTR(-ENODEV);
> > }
> >
> > -static struct gicv5_its_dev *gicv5_its_alloc_device(
> > - struct gicv5_its_chip_data *its, int nvec,
> > - u32 dev_id)
> > +struct gicv5_its_dev *gicv5_its_alloc_device(struct gicv5_its_chip_data *its,
> > + int nvec, u32 dev_id, bool is_iwb)
> > {
> > struct gicv5_its_dev *its_dev;
> > int ret;
> > @@ -815,6 +814,7 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
> > its_dev->device_id = dev_id;
> > its_dev->num_events = nvec;
> > its_dev->num_mapped_events = 0;
> > + its_dev->is_iwb = is_iwb;
> >
> > ret = gicv5_its_device_register(its, its_dev);
> > if (ret) {
> > @@ -827,9 +827,11 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
> >
> > /*
> > * This is the first time we have seen this device. Hence, it is not
> > - * shared.
> > + * shared, unless it is an IWB that is a shared ITS device by
> > + * definition, its eventids are hardcoded and never change - we allocate
> > + * it once for all and never free it.
>
> I'm not convinced the IWB should be treated differently from any other
> device. Its lifetime is not tied to its inputs, so all that's needed
> is to probe it, get a bunch of interrupts, and that's about it.
I need to check again how this works for devices requesting wires
from an IWB if we don't allow ITS device sharing.
> The other thing is that the IWB really is a standalone thing. It
> shouldn't have its fingers in the ITS code, and should only rely on
> the core infrastructure to get its interrupts.
>
> As much as I dislike it, the MBIGEN actually provides a decent example
> of how this could be structured.
We wrote that code already, I should have posted it. An MBIgen can
programme the eventids it sents to the ITS, an IWB can't. So yes,
I can make an IWB MBIgen like but the ITS code has to know it is
allocating an IRQ for an IWB - one way or another, the eventids
are not programmable.
I will try to post a v3 with the code in it so that I can get flamed
and find a solution to this niggle.
Thanks,
Lorenzo
On Wed, 30 Apr 2025 14:27:22 +0100,
Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
>
> On Wed, Apr 30, 2025 at 12:57:01PM +0100, Marc Zyngier wrote:
> > On Thu, 24 Apr 2025 11:25:32 +0100,
> > Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > >
> > > The GICv5 architecture implements the Interrupt Wire Bridge (IWB) in
> > > order to support wired interrupts that cannot be connected directly
> > > to an IRS and instead uses the ITS to translate a wire event into
> > > an IRQ signal.
> > >
> > > An IWB is a special ITS device with its own deviceID; upon probe,
> > > an IWB calls into the ITS driver to allocate DT/ITT tables for its
> > > events (ie wires).
> > >
> > > An IWB is always associated with a single ITS in the system.
> > >
> > > An IWB is connected to an ITS and it has its own deviceID for all
> > > interrupt wires that it manages; the IWB input wire number is
> > > exposed to the ITS as an eventID. This eventID is not programmable
> > > and therefore requires special handling in the ITS driver.
> > >
> > > Add an IWB driver in order to:
> > >
> > > - Probe IWBs in the system and allocate ITS tables
> > > - Manage IWB IRQ domains
> > > - Handle IWB input wires state (enable/disable)
> > > - Add the required IWB IRQchip representation
> > > - Handle firmware representation to Linux IRQ translation
> > >
> > > Co-developed-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > > Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > > Co-developed-by: Timothy Hayes <timothy.hayes@arm.com>
> > > Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> > > Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Marc Zyngier <maz@kernel.org>
> > > ---
> > > drivers/irqchip/Makefile | 2 +-
> > > drivers/irqchip/irq-gic-v5-its.c | 68 ++++++--
> > > drivers/irqchip/irq-gic-v5-iwb.c | 356 +++++++++++++++++++++++++++++++++++++++
> > > drivers/irqchip/irq-gic-v5.c | 2 +
> > > drivers/irqchip/irq-gic-v5.h | 28 +++
> > > 5 files changed, 437 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > > index 4280395e3bdff7858102f0b4eaaea1121cace52f..7bfb2369fbe494a64b72308d95ae33de93c6b8c6 100644
> > > --- a/drivers/irqchip/Makefile
> > > +++ b/drivers/irqchip/Makefile
> > > @@ -37,7 +37,7 @@ obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v4.o
> > > obj-$(CONFIG_ARM_GIC_V3_ITS_FSL_MC) += irq-gic-v3-its-fsl-mc-msi.o
> > > obj-$(CONFIG_PARTITION_PERCPU) += irq-partition-percpu.o
> > > obj-$(CONFIG_ARM_GIC_V5) += irq-gic-v5.o irq-gic-v5-irs.o
> > > -obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o
> > > +obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o irq-gic-v5-iwb.o
> > > obj-$(CONFIG_HISILICON_IRQ_MBIGEN) += irq-mbigen.o
> > > obj-$(CONFIG_ARM_NVIC) += irq-nvic.o
> > > obj-$(CONFIG_ARM_VIC) += irq-vic.o
> > > diff --git a/drivers/irqchip/irq-gic-v5-its.c b/drivers/irqchip/irq-gic-v5-its.c
> > > index da349b4709cc5ec8978859237838f039389ca4a1..b5eb4dbfe2296dc6620889eb9291b542cae4aeb6 100644
> > > --- a/drivers/irqchip/irq-gic-v5-its.c
> > > +++ b/drivers/irqchip/irq-gic-v5-its.c
> > > @@ -786,9 +786,8 @@ static struct gicv5_its_dev *gicv5_its_find_device(struct gicv5_its_chip_data *i
> > > return dev ? dev : ERR_PTR(-ENODEV);
> > > }
> > >
> > > -static struct gicv5_its_dev *gicv5_its_alloc_device(
> > > - struct gicv5_its_chip_data *its, int nvec,
> > > - u32 dev_id)
> > > +struct gicv5_its_dev *gicv5_its_alloc_device(struct gicv5_its_chip_data *its,
> > > + int nvec, u32 dev_id, bool is_iwb)
> > > {
> > > struct gicv5_its_dev *its_dev;
> > > int ret;
> > > @@ -815,6 +814,7 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
> > > its_dev->device_id = dev_id;
> > > its_dev->num_events = nvec;
> > > its_dev->num_mapped_events = 0;
> > > + its_dev->is_iwb = is_iwb;
> > >
> > > ret = gicv5_its_device_register(its, its_dev);
> > > if (ret) {
> > > @@ -827,9 +827,11 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
> > >
> > > /*
> > > * This is the first time we have seen this device. Hence, it is not
> > > - * shared.
> > > + * shared, unless it is an IWB that is a shared ITS device by
> > > + * definition, its eventids are hardcoded and never change - we allocate
> > > + * it once for all and never free it.
> >
> > I'm not convinced the IWB should be treated differently from any other
> > device. Its lifetime is not tied to its inputs, so all that's needed
> > is to probe it, get a bunch of interrupts, and that's about it.
>
> I need to check again how this works for devices requesting wires
> from an IWB if we don't allow ITS device sharing.
There is no sharing. Each IWB has its own devid, and the endpoint
drivers don't have to know about anything ITS related.
>
> > The other thing is that the IWB really is a standalone thing. It
> > shouldn't have its fingers in the ITS code, and should only rely on
> > the core infrastructure to get its interrupts.
> >
> > As much as I dislike it, the MBIGEN actually provides a decent example
> > of how this could be structured.
>
> We wrote that code already, I should have posted it. An MBIgen can
> programme the eventids it sents to the ITS, an IWB can't. So yes,
> I can make an IWB MBIgen like but the ITS code has to know it is
> allocating an IRQ for an IWB - one way or another, the eventids
> are not programmable.
They are not programmable on the MBIGEN either, despite what the code
does. Everything on this HW is hardcoded.
> I will try to post a v3 with the code in it so that I can get flamed
> and find a solution to this niggle.
Nobody is flaming you, Lorenzo. We're just trying to get to a point
where the code is in a good enough shape to be merged.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On Thu, May 01, 2025 at 02:27:26PM +0100, Marc Zyngier wrote:
> On Wed, 30 Apr 2025 14:27:22 +0100,
> Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> >
> > On Wed, Apr 30, 2025 at 12:57:01PM +0100, Marc Zyngier wrote:
> > > On Thu, 24 Apr 2025 11:25:32 +0100,
> > > Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > > >
> > > > The GICv5 architecture implements the Interrupt Wire Bridge (IWB) in
> > > > order to support wired interrupts that cannot be connected directly
> > > > to an IRS and instead uses the ITS to translate a wire event into
> > > > an IRQ signal.
> > > >
> > > > An IWB is a special ITS device with its own deviceID; upon probe,
> > > > an IWB calls into the ITS driver to allocate DT/ITT tables for its
> > > > events (ie wires).
> > > >
> > > > An IWB is always associated with a single ITS in the system.
> > > >
> > > > An IWB is connected to an ITS and it has its own deviceID for all
> > > > interrupt wires that it manages; the IWB input wire number is
> > > > exposed to the ITS as an eventID. This eventID is not programmable
> > > > and therefore requires special handling in the ITS driver.
> > > >
> > > > Add an IWB driver in order to:
> > > >
> > > > - Probe IWBs in the system and allocate ITS tables
> > > > - Manage IWB IRQ domains
> > > > - Handle IWB input wires state (enable/disable)
> > > > - Add the required IWB IRQchip representation
> > > > - Handle firmware representation to Linux IRQ translation
> > > >
> > > > Co-developed-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > > > Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > > > Co-developed-by: Timothy Hayes <timothy.hayes@arm.com>
> > > > Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> > > > Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > > Cc: Marc Zyngier <maz@kernel.org>
> > > > ---
> > > > drivers/irqchip/Makefile | 2 +-
> > > > drivers/irqchip/irq-gic-v5-its.c | 68 ++++++--
> > > > drivers/irqchip/irq-gic-v5-iwb.c | 356 +++++++++++++++++++++++++++++++++++++++
> > > > drivers/irqchip/irq-gic-v5.c | 2 +
> > > > drivers/irqchip/irq-gic-v5.h | 28 +++
> > > > 5 files changed, 437 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > > > index 4280395e3bdff7858102f0b4eaaea1121cace52f..7bfb2369fbe494a64b72308d95ae33de93c6b8c6 100644
> > > > --- a/drivers/irqchip/Makefile
> > > > +++ b/drivers/irqchip/Makefile
> > > > @@ -37,7 +37,7 @@ obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v4.o
> > > > obj-$(CONFIG_ARM_GIC_V3_ITS_FSL_MC) += irq-gic-v3-its-fsl-mc-msi.o
> > > > obj-$(CONFIG_PARTITION_PERCPU) += irq-partition-percpu.o
> > > > obj-$(CONFIG_ARM_GIC_V5) += irq-gic-v5.o irq-gic-v5-irs.o
> > > > -obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o
> > > > +obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o irq-gic-v5-iwb.o
> > > > obj-$(CONFIG_HISILICON_IRQ_MBIGEN) += irq-mbigen.o
> > > > obj-$(CONFIG_ARM_NVIC) += irq-nvic.o
> > > > obj-$(CONFIG_ARM_VIC) += irq-vic.o
> > > > diff --git a/drivers/irqchip/irq-gic-v5-its.c b/drivers/irqchip/irq-gic-v5-its.c
> > > > index da349b4709cc5ec8978859237838f039389ca4a1..b5eb4dbfe2296dc6620889eb9291b542cae4aeb6 100644
> > > > --- a/drivers/irqchip/irq-gic-v5-its.c
> > > > +++ b/drivers/irqchip/irq-gic-v5-its.c
> > > > @@ -786,9 +786,8 @@ static struct gicv5_its_dev *gicv5_its_find_device(struct gicv5_its_chip_data *i
> > > > return dev ? dev : ERR_PTR(-ENODEV);
> > > > }
> > > >
> > > > -static struct gicv5_its_dev *gicv5_its_alloc_device(
> > > > - struct gicv5_its_chip_data *its, int nvec,
> > > > - u32 dev_id)
> > > > +struct gicv5_its_dev *gicv5_its_alloc_device(struct gicv5_its_chip_data *its,
> > > > + int nvec, u32 dev_id, bool is_iwb)
> > > > {
> > > > struct gicv5_its_dev *its_dev;
> > > > int ret;
> > > > @@ -815,6 +814,7 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
> > > > its_dev->device_id = dev_id;
> > > > its_dev->num_events = nvec;
> > > > its_dev->num_mapped_events = 0;
> > > > + its_dev->is_iwb = is_iwb;
> > > >
> > > > ret = gicv5_its_device_register(its, its_dev);
> > > > if (ret) {
> > > > @@ -827,9 +827,11 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
> > > >
> > > > /*
> > > > * This is the first time we have seen this device. Hence, it is not
> > > > - * shared.
> > > > + * shared, unless it is an IWB that is a shared ITS device by
> > > > + * definition, its eventids are hardcoded and never change - we allocate
> > > > + * it once for all and never free it.
> > >
> > > I'm not convinced the IWB should be treated differently from any other
> > > device. Its lifetime is not tied to its inputs, so all that's needed
> > > is to probe it, get a bunch of interrupts, and that's about it.
> >
> > I need to check again how this works for devices requesting wires
> > from an IWB if we don't allow ITS device sharing.
>
> There is no sharing. Each IWB has its own devid, and the endpoint
> drivers don't have to know about anything ITS related.
I patched the IWB driver to work like an MBIgen.
It looks like the msi_prepare() ITS callback (ie where the its_device is
allocated) is called everytime an endpoint device driver requests a
wired IRQ through:
gicv5_its_msi_prepare+0x68c/0x6f8
its_pmsi_prepare+0x16c/0x1b8
__msi_domain_alloc_irqs+0x70/0x448
__msi_domain_alloc_irq_at+0xf8/0x194
msi_device_domain_alloc_wired+0x88/0x10c
irq_create_fwspec_mapping+0x3a0/0x4c0
irq_create_of_mapping+0xc0/0xe8
of_irq_get+0xa0/0xe4
platform_get_irq_optional+0x54/0x1c4
platform_get_irq+0x1c/0x50
so it becomes "shared" if multiple IWB wires are requested by endpoint
drivers.
I don't have an MBIgen enabled platform but I don't see how it could
behave differently but it could be that I have stared at this code
path for too long.
> > > The other thing is that the IWB really is a standalone thing. It
> > > shouldn't have its fingers in the ITS code, and should only rely on
> > > the core infrastructure to get its interrupts.
> > >
> > > As much as I dislike it, the MBIGEN actually provides a decent example
> > > of how this could be structured.
> >
> > We wrote that code already, I should have posted it. An MBIgen can
> > programme the eventids it sents to the ITS, an IWB can't. So yes,
> > I can make an IWB MBIgen like but the ITS code has to know it is
> > allocating an IRQ for an IWB - one way or another, the eventids
> > are not programmable.
>
> They are not programmable on the MBIGEN either, despite what the code
> does. Everything on this HW is hardcoded.
I don't understand then how in the GICv3 ITS we can guarantee that the
eventid we "allocate" for a wire matches the one sent on the MBIgen->ITS
bus. AFAICS, the ITS eventid is an offset from the LPI base that is
allocated dynamically.
Let's say an endpoint driver requires wire X. The ITS, in
its_alloc_device_irq() grabs a bit from the lpi_map bitmap that has
nothing to do with X.
I don't get how the two can be made to match unless we do something
like I am going to do with the IWB.
This, unless mbigen_write_msi_msg() does something with the
X<->{msg->data} translation (if that function does nothing it should be
removed because it is really misleading).
I am sorry to drone on about this but we have been raking our brains
over this and I would like to understand how this works once for all.
> > I will try to post a v3 with the code in it so that I can get flamed
> > and find a solution to this niggle.
>
> Nobody is flaming you, Lorenzo.
Absolutely, that's not what I meant, sorry, I am really really grateful
and honestly very happy about all the help provided.
I was referring to the disgusting OF compatible hack I posted, I really
don't like it (and good news, we don't need it either).
> We're just trying to get to a point where the code is in a good enough
> shape to be merged.
Absolutely, no questions, massive thanks.
Lorenzo
On Fri, 02 May 2025 08:59:42 +0100, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote: > > It looks like the msi_prepare() ITS callback (ie where the its_device is > allocated) is called everytime an endpoint device driver requests a > wired IRQ through: > > gicv5_its_msi_prepare+0x68c/0x6f8 > its_pmsi_prepare+0x16c/0x1b8 > __msi_domain_alloc_irqs+0x70/0x448 > __msi_domain_alloc_irq_at+0xf8/0x194 > msi_device_domain_alloc_wired+0x88/0x10c > irq_create_fwspec_mapping+0x3a0/0x4c0 > irq_create_of_mapping+0xc0/0xe8 > of_irq_get+0xa0/0xe4 > platform_get_irq_optional+0x54/0x1c4 > platform_get_irq+0x1c/0x50 > > so it becomes "shared" if multiple IWB wires are requested by endpoint > drivers. Right, I've reproduced on D05 with MBIGEN: [ 5.505530] Reusing ITT for devID 40000 [ 5.505532] CPU: 36 UID: 0 PID: 557 Comm: (udev-worker) Not tainted 6.15.0-rc4-00079-geef147df4841-dirty #4403 PREEMPT [ 5.505535] Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon D05 IT21 Nemo 2.0 RC0 04/18/2018 [ 5.505536] Call trace: [ 5.505537] show_stack+0x20/0x38 (C) [ 5.505540] dump_stack_lvl+0x80/0xf8 [ 5.505543] dump_stack+0x18/0x28 [ 5.505546] its_msi_prepare+0xe4/0x1d0 [ 5.505549] its_pmsi_prepare+0x15c/0x1d0 [ 5.505552] __msi_domain_alloc_irqs+0x80/0x398 [ 5.505556] __msi_domain_alloc_irq_at+0x100/0x168 [ 5.505560] msi_device_domain_alloc_wired+0x9c/0x128 [ 5.505564] irq_create_fwspec_mapping+0x180/0x388 [ 5.505567] acpi_irq_get+0xac/0xe8 [ 5.505570] platform_get_irq_optional+0x1e8/0x208 [ 5.505574] devm_platform_get_irqs_affinity+0x58/0x298 [ 5.505578] hisi_sas_v2_interrupt_preinit+0x60/0xb0 [hisi_sas_v2_hw] [ 5.505582] hisi_sas_probe+0x164/0x278 [hisi_sas_main] [ 5.505588] hisi_sas_v2_probe+0x20/0x38 [hisi_sas_v2_hw] [ 5.505591] platform_probe+0x70/0xd0 [ 5.505595] really_probe+0xc8/0x3a0 [ 5.505598] __driver_probe_device+0x84/0x170 [ 5.505600] driver_probe_device+0x44/0x120 [ 5.505603] __driver_attach+0xfc/0x210 [ 5.505606] bus_for_each_dev+0x7c/0xe8 [ 5.505608] driver_attach+0x2c/0x40 [ 5.505611] bus_add_driver+0x118/0x248 [ 5.505613] driver_register+0x68/0x138 [ 5.505616] __platform_driver_register+0x2c/0x40 [ 5.505619] hisi_sas_v2_driver_init+0x28/0xff8 [hisi_sas_v2_hw] [ 5.505623] do_one_initcall+0x4c/0x2c0 [ 5.505626] do_init_module+0x60/0x230 [ 5.505629] load_module+0xa64/0xb30 [ 5.505631] init_module_from_file+0x8c/0xd8 [ 5.505634] idempotent_init_module+0x1c4/0x2b8 [ 5.505637] __arm64_sys_finit_module+0x74/0xe8 [ 5.505640] invoke_syscall+0x50/0x120 [ 5.505642] el0_svc_common.constprop.0+0x48/0xf0 [ 5.505644] do_el0_svc+0x24/0x38 [ 5.505646] el0_svc+0x34/0xf0 [ 5.505650] el0t_64_sync_handler+0x10c/0x138 [ 5.505654] el0t_64_sync+0x1ac/0x1b0 [ 5.505681] ID:78 pID:8382 vID:143 And that a few dozen times. I'll have a think at how to unfsck this. This was previously avoided by (IIRC) populating the domain upfront and letting the domain matching code do its job. That behaviour seems to have been lost. On the other hand, as long as you don't expect the ITT to *grow*, nothing horrible should happen. But I also get an interesting crash in msi_domain_debig_show(), so there is more than just this corner case that is screwed. M. -- Jazz isn't dead. It just smells funny.
On Fri, May 02, 2025 at 04:43:57PM +0100, Marc Zyngier wrote: > On Fri, 02 May 2025 08:59:42 +0100, > Lorenzo Pieralisi <lpieralisi@kernel.org> wrote: > > > > It looks like the msi_prepare() ITS callback (ie where the its_device is > > allocated) is called everytime an endpoint device driver requests a > > wired IRQ through: > > > > gicv5_its_msi_prepare+0x68c/0x6f8 > > its_pmsi_prepare+0x16c/0x1b8 > > __msi_domain_alloc_irqs+0x70/0x448 > > __msi_domain_alloc_irq_at+0xf8/0x194 > > msi_device_domain_alloc_wired+0x88/0x10c > > irq_create_fwspec_mapping+0x3a0/0x4c0 > > irq_create_of_mapping+0xc0/0xe8 > > of_irq_get+0xa0/0xe4 > > platform_get_irq_optional+0x54/0x1c4 > > platform_get_irq+0x1c/0x50 > > > > so it becomes "shared" if multiple IWB wires are requested by endpoint > > drivers. > > Right, I've reproduced on D05 with MBIGEN: > > [ 5.505530] Reusing ITT for devID 40000 > [ 5.505532] CPU: 36 UID: 0 PID: 557 Comm: (udev-worker) Not tainted 6.15.0-rc4-00079-geef147df4841-dirty #4403 PREEMPT > [ 5.505535] Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon D05 IT21 Nemo 2.0 RC0 04/18/2018 > [ 5.505536] Call trace: > [ 5.505537] show_stack+0x20/0x38 (C) > [ 5.505540] dump_stack_lvl+0x80/0xf8 > [ 5.505543] dump_stack+0x18/0x28 > [ 5.505546] its_msi_prepare+0xe4/0x1d0 > [ 5.505549] its_pmsi_prepare+0x15c/0x1d0 > [ 5.505552] __msi_domain_alloc_irqs+0x80/0x398 > [ 5.505556] __msi_domain_alloc_irq_at+0x100/0x168 > [ 5.505560] msi_device_domain_alloc_wired+0x9c/0x128 > [ 5.505564] irq_create_fwspec_mapping+0x180/0x388 > [ 5.505567] acpi_irq_get+0xac/0xe8 > [ 5.505570] platform_get_irq_optional+0x1e8/0x208 > [ 5.505574] devm_platform_get_irqs_affinity+0x58/0x298 > [ 5.505578] hisi_sas_v2_interrupt_preinit+0x60/0xb0 [hisi_sas_v2_hw] > [ 5.505582] hisi_sas_probe+0x164/0x278 [hisi_sas_main] > [ 5.505588] hisi_sas_v2_probe+0x20/0x38 [hisi_sas_v2_hw] > [ 5.505591] platform_probe+0x70/0xd0 > [ 5.505595] really_probe+0xc8/0x3a0 > [ 5.505598] __driver_probe_device+0x84/0x170 > [ 5.505600] driver_probe_device+0x44/0x120 > [ 5.505603] __driver_attach+0xfc/0x210 > [ 5.505606] bus_for_each_dev+0x7c/0xe8 > [ 5.505608] driver_attach+0x2c/0x40 > [ 5.505611] bus_add_driver+0x118/0x248 > [ 5.505613] driver_register+0x68/0x138 > [ 5.505616] __platform_driver_register+0x2c/0x40 > [ 5.505619] hisi_sas_v2_driver_init+0x28/0xff8 [hisi_sas_v2_hw] > [ 5.505623] do_one_initcall+0x4c/0x2c0 > [ 5.505626] do_init_module+0x60/0x230 > [ 5.505629] load_module+0xa64/0xb30 > [ 5.505631] init_module_from_file+0x8c/0xd8 > [ 5.505634] idempotent_init_module+0x1c4/0x2b8 > [ 5.505637] __arm64_sys_finit_module+0x74/0xe8 > [ 5.505640] invoke_syscall+0x50/0x120 > [ 5.505642] el0_svc_common.constprop.0+0x48/0xf0 > [ 5.505644] do_el0_svc+0x24/0x38 > [ 5.505646] el0_svc+0x34/0xf0 > [ 5.505650] el0t_64_sync_handler+0x10c/0x138 > [ 5.505654] el0t_64_sync+0x1ac/0x1b0 > [ 5.505681] ID:78 pID:8382 vID:143 > > And that a few dozen times. Yep that matches my expectations, thanks a lot for testing it. > I'll have a think at how to unfsck this. This was previously avoided > by (IIRC) populating the domain upfront and letting the domain > matching code do its job. That behaviour seems to have been lost. On > the other hand, as long as you don't expect the ITT to *grow*, nothing > horrible should happen. Yes - I can remove the "shared" ITS device flag but should keep the logic preventing an ITS device with same deviceID to be allocated if found. > But I also get an interesting crash in msi_domain_debig_show(), so > there is more than just this corner case that is screwed. That I can try on my side too to try to help you untangle it. Possibly this was introduced when the MBIgen switched to MSI parent with 752e021f5b9b ? It is pure speculation at this stage just noticed that's a point in time where the domain code changed. Is MBIgen the only example of an IC relying on the ITS as MSI parent ? Thanks, Lorenzo
On Fri, 02 May 2025 08:59:42 +0100,
Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
>
> On Thu, May 01, 2025 at 02:27:26PM +0100, Marc Zyngier wrote:
> > On Wed, 30 Apr 2025 14:27:22 +0100,
> > Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > >
> > > On Wed, Apr 30, 2025 at 12:57:01PM +0100, Marc Zyngier wrote:
> > > > On Thu, 24 Apr 2025 11:25:32 +0100,
> > > > Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > > > >
> > > > > The GICv5 architecture implements the Interrupt Wire Bridge (IWB) in
> > > > > order to support wired interrupts that cannot be connected directly
> > > > > to an IRS and instead uses the ITS to translate a wire event into
> > > > > an IRQ signal.
> > > > >
> > > > > An IWB is a special ITS device with its own deviceID; upon probe,
> > > > > an IWB calls into the ITS driver to allocate DT/ITT tables for its
> > > > > events (ie wires).
> > > > >
> > > > > An IWB is always associated with a single ITS in the system.
> > > > >
> > > > > An IWB is connected to an ITS and it has its own deviceID for all
> > > > > interrupt wires that it manages; the IWB input wire number is
> > > > > exposed to the ITS as an eventID. This eventID is not programmable
> > > > > and therefore requires special handling in the ITS driver.
> > > > >
> > > > > Add an IWB driver in order to:
> > > > >
> > > > > - Probe IWBs in the system and allocate ITS tables
> > > > > - Manage IWB IRQ domains
> > > > > - Handle IWB input wires state (enable/disable)
> > > > > - Add the required IWB IRQchip representation
> > > > > - Handle firmware representation to Linux IRQ translation
> > > > >
> > > > > Co-developed-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > > > > Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > > > > Co-developed-by: Timothy Hayes <timothy.hayes@arm.com>
> > > > > Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> > > > > Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > > > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > > > Cc: Marc Zyngier <maz@kernel.org>
> > > > > ---
> > > > > drivers/irqchip/Makefile | 2 +-
> > > > > drivers/irqchip/irq-gic-v5-its.c | 68 ++++++--
> > > > > drivers/irqchip/irq-gic-v5-iwb.c | 356 +++++++++++++++++++++++++++++++++++++++
> > > > > drivers/irqchip/irq-gic-v5.c | 2 +
> > > > > drivers/irqchip/irq-gic-v5.h | 28 +++
> > > > > 5 files changed, 437 insertions(+), 19 deletions(-)
> > > > >
> > > > > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > > > > index 4280395e3bdff7858102f0b4eaaea1121cace52f..7bfb2369fbe494a64b72308d95ae33de93c6b8c6 100644
> > > > > --- a/drivers/irqchip/Makefile
> > > > > +++ b/drivers/irqchip/Makefile
> > > > > @@ -37,7 +37,7 @@ obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v4.o
> > > > > obj-$(CONFIG_ARM_GIC_V3_ITS_FSL_MC) += irq-gic-v3-its-fsl-mc-msi.o
> > > > > obj-$(CONFIG_PARTITION_PERCPU) += irq-partition-percpu.o
> > > > > obj-$(CONFIG_ARM_GIC_V5) += irq-gic-v5.o irq-gic-v5-irs.o
> > > > > -obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o
> > > > > +obj-$(CONFIG_ARM_GIC_V5_ITS) += irq-gic-v5-its.o irq-gic-v5-iwb.o
> > > > > obj-$(CONFIG_HISILICON_IRQ_MBIGEN) += irq-mbigen.o
> > > > > obj-$(CONFIG_ARM_NVIC) += irq-nvic.o
> > > > > obj-$(CONFIG_ARM_VIC) += irq-vic.o
> > > > > diff --git a/drivers/irqchip/irq-gic-v5-its.c b/drivers/irqchip/irq-gic-v5-its.c
> > > > > index da349b4709cc5ec8978859237838f039389ca4a1..b5eb4dbfe2296dc6620889eb9291b542cae4aeb6 100644
> > > > > --- a/drivers/irqchip/irq-gic-v5-its.c
> > > > > +++ b/drivers/irqchip/irq-gic-v5-its.c
> > > > > @@ -786,9 +786,8 @@ static struct gicv5_its_dev *gicv5_its_find_device(struct gicv5_its_chip_data *i
> > > > > return dev ? dev : ERR_PTR(-ENODEV);
> > > > > }
> > > > >
> > > > > -static struct gicv5_its_dev *gicv5_its_alloc_device(
> > > > > - struct gicv5_its_chip_data *its, int nvec,
> > > > > - u32 dev_id)
> > > > > +struct gicv5_its_dev *gicv5_its_alloc_device(struct gicv5_its_chip_data *its,
> > > > > + int nvec, u32 dev_id, bool is_iwb)
> > > > > {
> > > > > struct gicv5_its_dev *its_dev;
> > > > > int ret;
> > > > > @@ -815,6 +814,7 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
> > > > > its_dev->device_id = dev_id;
> > > > > its_dev->num_events = nvec;
> > > > > its_dev->num_mapped_events = 0;
> > > > > + its_dev->is_iwb = is_iwb;
> > > > >
> > > > > ret = gicv5_its_device_register(its, its_dev);
> > > > > if (ret) {
> > > > > @@ -827,9 +827,11 @@ static struct gicv5_its_dev *gicv5_its_alloc_device(
> > > > >
> > > > > /*
> > > > > * This is the first time we have seen this device. Hence, it is not
> > > > > - * shared.
> > > > > + * shared, unless it is an IWB that is a shared ITS device by
> > > > > + * definition, its eventids are hardcoded and never change - we allocate
> > > > > + * it once for all and never free it.
> > > >
> > > > I'm not convinced the IWB should be treated differently from any other
> > > > device. Its lifetime is not tied to its inputs, so all that's needed
> > > > is to probe it, get a bunch of interrupts, and that's about it.
> > >
> > > I need to check again how this works for devices requesting wires
> > > from an IWB if we don't allow ITS device sharing.
> >
> > There is no sharing. Each IWB has its own devid, and the endpoint
> > drivers don't have to know about anything ITS related.
>
> I patched the IWB driver to work like an MBIgen.
>
> It looks like the msi_prepare() ITS callback (ie where the its_device is
> allocated) is called everytime an endpoint device driver requests a
> wired IRQ through:
>
> gicv5_its_msi_prepare+0x68c/0x6f8
> its_pmsi_prepare+0x16c/0x1b8
> __msi_domain_alloc_irqs+0x70/0x448
> __msi_domain_alloc_irq_at+0xf8/0x194
> msi_device_domain_alloc_wired+0x88/0x10c
> irq_create_fwspec_mapping+0x3a0/0x4c0
> irq_create_of_mapping+0xc0/0xe8
> of_irq_get+0xa0/0xe4
> platform_get_irq_optional+0x54/0x1c4
> platform_get_irq+0x1c/0x50
>
> so it becomes "shared" if multiple IWB wires are requested by endpoint
> drivers.
You shouldn't get into the allocation when the endpoint device probes.
The assumption is that all the interrupts should be allocated only
*once*.
If that's not the case, it probably means that something has been
subtly broken when this code was last massaged, and that it needs
fixing.
I'll resurrect my D05 shortly to collect some traces.
>
> I don't have an MBIgen enabled platform but I don't see how it could
> behave differently but it could be that I have stared at this code
> path for too long.
>
> > > > The other thing is that the IWB really is a standalone thing. It
> > > > shouldn't have its fingers in the ITS code, and should only rely on
> > > > the core infrastructure to get its interrupts.
> > > >
> > > > As much as I dislike it, the MBIGEN actually provides a decent example
> > > > of how this could be structured.
> > >
> > > We wrote that code already, I should have posted it. An MBIgen can
> > > programme the eventids it sents to the ITS, an IWB can't. So yes,
> > > I can make an IWB MBIgen like but the ITS code has to know it is
> > > allocating an IRQ for an IWB - one way or another, the eventids
> > > are not programmable.
> >
> > They are not programmable on the MBIGEN either, despite what the code
> > does. Everything on this HW is hardcoded.
>
> I don't understand then how in the GICv3 ITS we can guarantee that the
> eventid we "allocate" for a wire matches the one sent on the MBIgen->ITS
> bus. AFAICS, the ITS eventid is an offset from the LPI base that is
> allocated dynamically.
>
> Let's say an endpoint driver requires wire X. The ITS, in
> its_alloc_device_irq() grabs a bit from the lpi_map bitmap that has
> nothing to do with X.
>
> I don't get how the two can be made to match unless we do something
> like I am going to do with the IWB.
The driver allocates n LPIs, which are mapped as events 0 to n-1. Just
like with the IWB. When I say it is the same, it is *EXACTLY* the same.
> This, unless mbigen_write_msi_msg() does something with the
> X<->{msg->data} translation (if that function does nothing it should be
> removed because it is really misleading).
It doesn't do anything when it comes to the eventid. But misleading or
not has nothing to do with your problem.
M.
--
Jazz isn't dead. It just smells funny.
© 2016 - 2026 Red Hat, Inc.