Linux RISC-V AIA Support

The PLIC driver does not require very early initialization so let us convert it into a platform driver.

As part of the conversion, the PLIC probing undergoes the following changes:
1. Use dev_info(), dev_err() and dev_warn() instead of pr_info(), pr_err() and pr_warn()
2. Use devm_xyz() APIs wherever applicable
3. PLIC is now probed after CPUs are brought-up so we have to setup cpuhp state after context handler of all online CPUs are initialized otherwise we see crash on multi-socket systems

Quoted 2 lines · Anup Patel

The PLIC driver does not require very early initialization so let us convert it into a platform driver.

s/let us convert/convert/

Please us passive voice and imperative mood all over the changelogs. No we/us, let....

Quoted 9 lines

As part of the conversion, the PLIC probing undergoes the following changes:
1. Use dev_info(), dev_err() and dev_warn() instead of pr_info(), pr_err() and pr_warn()
2. Use devm_xyz() APIs wherever applicable
3. PLIC is now probed after CPUs are brought-up so we have to setup cpuhp state after context handler of all online CPUs are initialized otherwise we see crash on multi-socket systems

This patch is really doing too many things at once, which makes it hard to review. Can you split this into digestable pieces please?

drivers/irqchip/irq-sifive-plic.c static void plic_handle_irq(struct irq_desc *desc) · line 376

Quoted 6 lines

if (unlikely(err))

drivers/irqchip/irq-sifive-plic.c +2 -1

-                     pr_warn_ratelimited("can't find mapping for hwirq %lu\n",
+                     dev_warn_ratelimited(handler->priv->dev,
+                                     "can't find mapping for hwirq %lu\n",
                                      hwirq);

Nit. Please use brackets around the condition. See:

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#bracket-rules

for reasoning.

Now that PLIC driver is probed as a regular platform driver, the lock dependency validator complains about the safety of handler->enable_lock usage:

[    0.956775]  Possible interrupt unsafe locking scenario:

[    0.956998]        CPU0                    CPU1
[    0.957247]        ----                    ----
[    0.957439]   lock(&handler->enable_lock);
[    0.957607]                                local_irq_disable();
[    0.957793]                                lock(&irq_desc_lock_class);
[    0.958021]                                lock(&handler->enable_lock);
[    0.958246]   <Interrupt>
[    0.958342]     lock(&irq_desc_lock_class);
[    0.958501]
                *** DEADLOCK ***

To address above, let's use raw_spin_lock_irqsave/unlock_irqrestore() instead of raw_spin_lock/unlock().

Quoted 17 lines · Anup Patel

Now that PLIC driver is probed as a regular platform driver, the lock dependency validator complains about the safety of handler->enable_lock usage:

[    0.956775]  Possible interrupt unsafe locking scenario:

[    0.956998]        CPU0                    CPU1
[    0.957247]        ----                    ----
[    0.957439]   lock(&handler->enable_lock);
[    0.957607]                                local_irq_disable();
[    0.957793]                                lock(&irq_desc_lock_class);
[    0.958021]                                lock(&handler->enable_lock);
[    0.958246]   <Interrupt>
[    0.958342]     lock(&irq_desc_lock_class);
[    0.958501]
                *** DEADLOCK ***

Quoted 2 lines

To address above, let's use raw_spin_lock_irqsave/unlock_irqrestore() instead of raw_spin_lock/unlock().

s/let's//

The RISC-V advanced interrupt architecture (AIA) extends the per-HART local interrupts in following ways:
1. Minimum 64 local interrupts for both RV32 and RV64
2. Ability to process multiple pending local interrupts in same interrupt handler
3. Priority configuration for each local interrupts
4. Special CSRs to configure/access the per-HART MSI controller

We add support for #1 and #2 described above in the RISC-V intc driver.

Quoted 8 lines · Anup Patel

The RISC-V advanced interrupt architecture (AIA) extends the per-HART local interrupts in following ways:
1. Minimum 64 local interrupts for both RV32 and RV64
2. Ability to process multiple pending local interrupts in same interrupt handler
3. Priority configuration for each local interrupts
4. Special CSRs to configure/access the per-HART MSI controller

Quoted 2 lines

We add support for #1 and #2 described above in the RISC-V intc driver.

S/We add/Add/

drivers/irqchip/irq-riscv-intc.c static asmlinkage void riscv_intc_irq(struct pt_regs *regs) · line 40

Quoted 8 lines

drivers/irqchip/irq-riscv-intc.c +7 -0

+static asmlinkage void riscv_intc_aia_irq(struct pt_regs *regs)
+{
+     unsigned long topi;
+
+     while ((topi = csr_read(CSR_TOPI)))
+             generic_handle_domain_irq(intc_domain,
+                                       topi >> TOPI_IID_SHIFT);

Please let it stick out. You got 100 characters. All over the place.

The RISC-V advanced interrupt architecture (AIA) specification defines a new MSI controller called incoming message signalled interrupt controller (IMSIC) which manages MSI on per-HART (or per-CPU) basis. It also supports IPIs as software injected MSIs. (For more details refer https://github.com/riscv/riscv-aia)

Let us add an early irqchip driver for RISC-V IMSIC which sets up the IMSIC state and provide IPIs.

Signed-off-by Anup Patel <[email protected]>

---
 drivers/irqchip/Kconfig                 |   7 +
 drivers/irqchip/Makefile                |   1 +
 drivers/irqchip/irq-riscv-imsic-early.c | 213 ++++++
 drivers/irqchip/irq-riscv-imsic-state.c | 906 ++++++++++++++++++++++++
 drivers/irqchip/irq-riscv-imsic-state.h |  98 +++
 include/linux/irqchip/riscv-imsic.h     |  87 +++
 6 files changed, 1312 insertions(+)
 create mode 100644 drivers/irqchip/irq-riscv-imsic-early.c
 create mode 100644 drivers/irqchip/irq-riscv-imsic-state.c
 create mode 100644 drivers/irqchip/irq-riscv-imsic-state.h
 create mode 100644 include/linux/irqchip/riscv-imsic.h

drivers/irqchip/Kconfig +7 -0

index f7149d0f3d45..85f86e31c996 100644
@@ -546,6 +546,13 @@ config SIFIVE_PLIC
 	select IRQ_DOMAIN_HIERARCHY
 	select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP
 
+config RISCV_IMSIC
+	bool
+	depends on RISCV
+	select IRQ_DOMAIN_HIERARCHY
+	select GENERIC_IRQ_MATRIX_ALLOCATOR
+	select GENERIC_MSI_IRQ
+
 config EXYNOS_IRQ_COMBINER
 	bool "Samsung Exynos IRQ combiner support" if COMPILE_TEST
 	depends on (ARCH_EXYNOS && ARM) || COMPILE_TEST

drivers/irqchip/Makefile +1 -0

index ffd945fe71aa..d714724387ce 100644
@@ -95,6 +95,7 @@ obj-$(CONFIG_QCOM_MPM)			+= irq-qcom-mpm.o
 obj-$(CONFIG_CSKY_MPINTC)		+= irq-csky-mpintc.o
 obj-$(CONFIG_CSKY_APB_INTC)		+= irq-csky-apb-intc.o
 obj-$(CONFIG_RISCV_INTC)		+= irq-riscv-intc.o
+obj-$(CONFIG_RISCV_IMSIC)		+= irq-riscv-imsic-state.o irq-riscv-imsic-early.o
 obj-$(CONFIG_SIFIVE_PLIC)		+= irq-sifive-plic.o
 obj-$(CONFIG_IMX_IRQSTEER)		+= irq-imx-irqsteer.o
 obj-$(CONFIG_IMX_INTMUX)		+= irq-imx-intmux.o

drivers/irqchip/irq-riscv-imsic-early.c new file +39 -0

new file mode 100644
index 000000000000..32fe428b1c19
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
+ * Copyright (C) 2022 Ventana Micro Systems Inc.
+ */
+
+#define pr_fmt(fmt) "riscv-imsic: " fmt
+#include <linux/cpu.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/smp.h>
+
+#include "irq-riscv-imsic-state.h"
+
+static int imsic_parent_irq;
+
+#ifdef CONFIG_SMP
+static void imsic_ipi_send(unsigned int cpu)
+{
+	struct imsic_local_config *local = per_cpu_ptr(imsic->global.local, cpu);
+
+	writel_relaxed(IMSIC_IPI_ID, local->msi_va);
+}
+
+static void imsic_ipi_starting_cpu(void)
+{
+	/* Enable IPIs for current CPU. */
+	__imsic_id_set_enable(IMSIC_IPI_ID);
+}
+
+static void imsic_ipi_dying_cpu(void)
+{
+	/* Disable IPIs for current CPU. */
+	__imsic_id_clear_enable(IMSIC_IPI_ID);

drivers/irqchip/irq-riscv-imsic-early.c new file +30 -0

+
+	/* Set vIRQ range */
+	riscv_ipi_set_virq_range(virq, IMSIC_NR_IPI, true);
+
+	/* Announce that IMSIC is providing IPIs */
+	pr_info("%pfwP: providing IPIs using interrupt %d\n", imsic->fwnode, IMSIC_IPI_ID);
+
+	return 0;
+}
+#else
+static void imsic_ipi_starting_cpu(void)
+{
+}
+
+static void imsic_ipi_dying_cpu(void)
+{
+}
+
+static int __init imsic_ipi_domain_init(void)
+{
+	return 0;
+}
+#endif
+
+/*
+ * To handle an interrupt, we read the TOPEI CSR and write zero in one
+ * instruction. If TOPEI CSR is non-zero then we translate TOPEI.ID to
+ * Linux interrupt number and let Linux IRQ subsystem handle it.
+ */
+static void imsic_handle_irq(struct irq_desc *desc)

drivers/irqchip/irq-riscv-imsic-early.c new file +4 -0

+			continue;
+		}
+
+		if (unlikely(!imsic->base_domain))

drivers/irqchip/irq-riscv-imsic-early.c new file +82 -0

+		if (unlikely(err))
+			pr_warn_ratelimited("hwirq 0x%x mapping not found\n", vec->hwirq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static int imsic_starting_cpu(unsigned int cpu)
+{
+	/* Mark per-CPU IMSIC state as online */
+	imsic_state_online();
+
+	/* Enable per-CPU parent interrupt */
+	enable_percpu_irq(imsic_parent_irq, irq_get_trigger_type(imsic_parent_irq));
+
+	/* Setup IPIs */
+	imsic_ipi_starting_cpu();
+
+	/*
+	 * Interrupts identities might have been enabled/disabled while
+	 * this CPU was not running so sync-up local enable/disable state.
+	 */
+	imsic_local_sync_all();
+
+	/* Enable local interrupt delivery */
+	imsic_local_delivery(true);
+
+	return 0;
+}
+
+static int imsic_dying_cpu(unsigned int cpu)
+{
+	/* Cleanup IPIs */
+	imsic_ipi_dying_cpu();
+
+	/* Mark per-CPU IMSIC state as offline */
+	imsic_state_offline();
+
+	return 0;
+}
+
+static int __init imsic_early_probe(struct fwnode_handle *fwnode)
+{
+	struct irq_domain *domain;
+	int rc;
+
+	/* Find parent domain and register chained handler */
+	domain = irq_find_matching_fwnode(riscv_get_intc_hwnode(), DOMAIN_BUS_ANY);
+	if (!domain) {
+		pr_err("%pfwP: Failed to find INTC domain\n", fwnode);
+		return -ENOENT;
+	}
+	imsic_parent_irq = irq_create_mapping(domain, RV_IRQ_EXT);
+	if (!imsic_parent_irq) {
+		pr_err("%pfwP: Failed to create INTC mapping\n", fwnode);
+		return -ENOENT;
+	}
+
+	/* Initialize IPI domain */
+	rc = imsic_ipi_domain_init();
+	if (rc) {
+		pr_err("%pfwP: Failed to initialize IPI domain\n", fwnode);
+		return rc;
+	}
+
+	/* Setup chained handler to the parent domain interrupt */
+	irq_set_chained_handler(imsic_parent_irq, imsic_handle_irq);
+
+	/*
+	 * Setup cpuhp state (must be done after setting imsic_parent_irq)
+	 *
+	 * Don't disable per-CPU IMSIC file when CPU goes offline
+	 * because this affects IPI and the masking/unmasking of
+	 * virtual IPIs is done via generic IPI-Mux
+	 */
+	cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "irqchip/riscv/imsic:starting",
+			  imsic_starting_cpu, imsic_dying_cpu);
+
+	return 0;
+}
+
+static int __init imsic_early_dt_init(struct device_node *node,

drivers/irqchip/irq-riscv-imsic-early.c new file +13 -0

+		return rc;
+	}
+
+	/* Do early setup of IPIs */
+	rc = imsic_early_probe(fwnode);
+	if (rc)
+		return rc;
+
+	/* Ensure that OF platform device gets probed */
+	of_node_clear_flag(node, OF_POPULATED);
+	return 0;
+}
+IRQCHIP_DECLARE(riscv_imsic, "riscv,imsics", imsic_early_dt_init);

drivers/irqchip/irq-riscv-imsic-state.c new file +65 -0

new file mode 100644
index 000000000000..4f347486ec7c
@@ -0,0 +1,906 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
+ * Copyright (C) 2022 Ventana Micro Systems Inc.
+ */
+
+#define pr_fmt(fmt) "riscv-imsic: " fmt
+#include <linux/cpu.h>
+#include <linux/bitmap.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/seq_file.h>
+#include <linux/spinlock.h>
+#include <linux/smp.h>
+#include <asm/hwcap.h>
+
+#include "irq-riscv-imsic-state.h"
+
+#define IMSIC_DISABLE_EIDELIVERY		0
+#define IMSIC_ENABLE_EIDELIVERY			1
+#define IMSIC_DISABLE_EITHRESHOLD		1
+#define IMSIC_ENABLE_EITHRESHOLD		0
+
+static inline void imsic_csr_write(unsigned long reg, unsigned long val)
+{
+	csr_write(CSR_ISELECT, reg);
+	csr_write(CSR_IREG, val);
+}
+
+static inline unsigned long imsic_csr_read(unsigned long reg)
+{
+	csr_write(CSR_ISELECT, reg);
+	return csr_read(CSR_IREG);
+}
+
+static inline unsigned long imsic_csr_read_clear(unsigned long reg, unsigned long val)
+{
+	csr_write(CSR_ISELECT, reg);
+	return csr_read_clear(CSR_IREG, val);
+}
+
+static inline void imsic_csr_set(unsigned long reg, unsigned long val)
+{
+	csr_write(CSR_ISELECT, reg);
+	csr_set(CSR_IREG, val);
+}
+
+static inline void imsic_csr_clear(unsigned long reg, unsigned long val)
+{
+	csr_write(CSR_ISELECT, reg);
+	csr_clear(CSR_IREG, val);
+}
+
+struct imsic_priv *imsic;
+
+const struct imsic_global_config *imsic_get_global_config(void)
+{
+	return imsic ? &imsic->global : NULL;
+}
+EXPORT_SYMBOL_GPL(imsic_get_global_config);
+

drivers/irqchip/irq-riscv-imsic-state.c new file +11 -0

+}
+
+static inline bool __imsic_id_read_clear_enabled(unsigned long id)
+{
+	return __imsic_eix_read_clear(id, false);
+}
+
+static inline bool __imsic_id_read_clear_pending(unsigned long id)
+{
+	return __imsic_eix_read_clear(id, true);
+}

drivers/irqchip/irq-riscv-imsic-state.c new file +112 -0

+			ireg |= BIT(i);
+			id++;
+		}
+
+		/*
+		 * The IMSIC EIEx and EIPx registers are indirectly
+		 * accessed via using ISELECT and IREG CSRs so we
+		 * need to access these CSRs without getting preempted.
+		 *
+		 * All existing users of this function call this
+		 * function with local IRQs disabled so we don't
+		 * need to do anything special here.
+		 */
+		if (val)
+			imsic_csr_set(isel, ireg);
+		else
+			imsic_csr_clear(isel, ireg);
+	}
+}
+
+/* MUST be called with lpriv->lock held */
+static void __imsic_local_sync(struct imsic_local_priv *lpriv)
+{
+	struct imsic_local_config *mlocal;
+	struct imsic_vector *vec, *mvec;
+	int i;
+
+	/* This pairs with the barrier in __imsic_remote_sync(). */
+	smp_mb();
+
+	for_each_set_bit(i, lpriv->dirty_bitmap, imsic->global.nr_ids + 1) {
+		if (!i || i == IMSIC_IPI_ID)
+			goto skip;
+		vec = &lpriv->vectors[i];
+
+		if (vec->enable)
+			__imsic_id_set_enable(i);
+		else
+			__imsic_id_clear_enable(i);
+
+		/*
+		 * If the ID was being moved to a new ID on some other CPU
+		 * then we can get a MSI during the movement so check the
+		 * ID pending bit and re-trigger the new ID on other CPU
+		 * using MMIO write.
+		 */
+		mvec = vec->move;
+		vec->move = NULL;
+		if (mvec && mvec != vec) {
+			if (__imsic_id_read_clear_pending(i)) {
+				mlocal = per_cpu_ptr(imsic->global.local, mvec->cpu);
+				writel_relaxed(mvec->local_id, mlocal->msi_va);
+			}
+
+			imsic_vector_free(&lpriv->vectors[i]);
+		}
+
+skip:
+		bitmap_clear(lpriv->dirty_bitmap, i, 1);
+	}
+}
+
+void imsic_local_sync_all(void)
+{
+	struct imsic_local_priv *lpriv = this_cpu_ptr(imsic->lpriv);
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&lpriv->lock, flags);
+	bitmap_fill(lpriv->dirty_bitmap, imsic->global.nr_ids + 1);
+	__imsic_local_sync(lpriv);
+	raw_spin_unlock_irqrestore(&lpriv->lock, flags);
+}
+
+void imsic_local_delivery(bool enable)
+{
+	if (enable) {
+		imsic_csr_write(IMSIC_EITHRESHOLD, IMSIC_ENABLE_EITHRESHOLD);
+		imsic_csr_write(IMSIC_EIDELIVERY, IMSIC_ENABLE_EIDELIVERY);
+		return;
+	}
+
+	imsic_csr_write(IMSIC_EIDELIVERY, IMSIC_DISABLE_EIDELIVERY);
+	imsic_csr_write(IMSIC_EITHRESHOLD, IMSIC_DISABLE_EITHRESHOLD);
+}
+
+#ifdef CONFIG_SMP
+static void imsic_local_timer_callback(struct timer_list *timer)
+{
+	struct imsic_local_priv *lpriv = this_cpu_ptr(imsic->lpriv);
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&lpriv->lock, flags);
+	__imsic_local_sync(lpriv);
+	raw_spin_unlock_irqrestore(&lpriv->lock, flags);
+}
+
+/* MUST be called with lpriv->lock held */
+static void __imsic_remote_sync(struct imsic_local_priv *lpriv, unsigned int cpu)
+{
+	/*
+	 * Ensure that changes to vector enable, vector move and
+	 * dirty bitmap are visible to the target CPU.
+	 *
+	 * This pairs with the barrier in __imsic_local_sync().
+	 */
+	smp_mb();
+
+	/*
+	 * We schedule a timer on the target CPU if the target CPU is not
+	 * same as the current CPU. An offline CPU will unconditionally
+	 * synchronize IDs through imsic_starting_cpu() when the
+	 * CPU is brought up.

drivers/irqchip/irq-riscv-imsic-state.c new file +21 -0

+	}
+}
+#else
+/* MUST be called with lpriv->lock held */
+static void __imsic_remote_sync(struct imsic_local_priv *lpriv, unsigned int cpu)
+{
+	__imsic_local_sync(lpriv);
+}
+#endif
+
+void imsic_vector_mask(struct imsic_vector *vec)
+{
+	struct imsic_local_priv *lpriv;
+
+	lpriv = per_cpu_ptr(imsic->lpriv, vec->cpu);
+	if (WARN_ON(&lpriv->vectors[vec->local_id] != vec))
+		return;
+
+	/*
+	 * This function is called through Linux irq subsystem with
+	 * irqs disabled so no need to save/restore irq flags.

drivers/irqchip/irq-riscv-imsic-state.c new file +108 -0

+
+void imsic_vector_unmask(struct imsic_vector *vec)
+{
+	struct imsic_local_priv *lpriv;
+
+	lpriv = per_cpu_ptr(imsic->lpriv, vec->cpu);
+	if (WARN_ON(&lpriv->vectors[vec->local_id] != vec))
+		return;
+
+	/*
+	 * This function is called through Linux irq subsystem with
+	 * irqs disabled so no need to save/restore irq flags.
+	 */
+
+	raw_spin_lock(&lpriv->lock);
+
+	vec->enable = true;
+	bitmap_set(lpriv->dirty_bitmap, vec->local_id, 1);
+	__imsic_remote_sync(lpriv, vec->cpu);
+
+	raw_spin_unlock(&lpriv->lock);
+}
+
+
+bool imsic_vector_isenabled(struct imsic_vector *vec)
+{
+	struct imsic_local_priv *lpriv = per_cpu_ptr(imsic->lpriv, vec->cpu);
+	unsigned long flags;
+	bool ret;
+
+	raw_spin_lock_irqsave(&lpriv->lock, flags);
+	ret = vec->enable;
+	raw_spin_unlock_irqrestore(&lpriv->lock, flags);
+
+	return ret;
+}
+
+struct imsic_vector *imsic_vector_get_move(struct imsic_vector *vec)
+{
+	struct imsic_local_priv *lpriv = per_cpu_ptr(imsic->lpriv, vec->cpu);
+	struct imsic_vector *ret;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&lpriv->lock, flags);
+	ret = vec->move;
+	raw_spin_unlock_irqrestore(&lpriv->lock, flags);
+
+	return ret;
+}
+
+static bool imsic_vector_move_update(struct imsic_local_priv *lpriv, struct imsic_vector *vec,
+				     bool new_enable, struct imsic_vector *new_move)
+{
+	unsigned long flags;
+	bool enabled;
+
+	raw_spin_lock_irqsave(&lpriv->lock, flags);
+
+	/* Update enable and move details */
+	enabled = vec->enable;
+	vec->enable = new_enable;
+	vec->move = new_move;
+
+	/* Mark the vector as dirty and synchronize */
+	bitmap_set(lpriv->dirty_bitmap, vec->local_id, 1);
+	__imsic_remote_sync(lpriv, vec->cpu);
+
+	raw_spin_unlock_irqrestore(&lpriv->lock, flags);
+
+	return enabled;
+}
+
+void imsic_vector_move(struct imsic_vector *old_vec, struct imsic_vector *new_vec)
+{
+	struct imsic_local_priv *old_lpriv, *new_lpriv;
+	bool enabled;
+
+	if (WARN_ON(old_vec->cpu == new_vec->cpu))
+		return;
+
+	old_lpriv = per_cpu_ptr(imsic->lpriv, old_vec->cpu);
+	if (WARN_ON(&old_lpriv->vectors[old_vec->local_id] != old_vec))
+		return;
+
+	new_lpriv = per_cpu_ptr(imsic->lpriv, new_vec->cpu);
+	if (WARN_ON(&new_lpriv->vectors[new_vec->local_id] != new_vec))
+		return;
+
+	/*
+	 * Move and re-trigger the new vector based on the pending
+	 * state of the old vector because we might get a device
+	 * interrupt on the old vector while device was being moved
+	 * to the new vector.
+	 */
+	enabled = imsic_vector_move_update(old_lpriv, old_vec, false, new_vec);
+	imsic_vector_move_update(new_lpriv, new_vec, enabled, new_vec);
+}
+
+#ifdef CONFIG_GENERIC_IRQ_DEBUGFS
+void imsic_vector_debug_show(struct seq_file *m, struct imsic_vector *vec, int ind)
+{
+	struct imsic_local_priv *lpriv;
+	struct imsic_vector *mvec;
+	bool is_enabled;
+
+	lpriv = per_cpu_ptr(imsic->lpriv, vec->cpu);
+	if (WARN_ON(&lpriv->vectors[vec->local_id] != vec))
+		return;

drivers/irqchip/irq-riscv-imsic-state.c new file +220 -0

+	if (mvec) {
+		seq_printf(m, "%*smove_cpu        : %5u\n", ind, "", mvec->cpu);
+		seq_printf(m, "%*smove_local_id   : %5u\n", ind, "", mvec->local_id);
+	}
+}
+
+void imsic_vector_debug_show_summary(struct seq_file *m, int ind)
+{
+	irq_matrix_debug_show(m, imsic->matrix, ind);
+}
+#endif
+
+struct imsic_vector *imsic_vector_from_local_id(unsigned int cpu, unsigned int local_id)
+{
+	struct imsic_local_priv *lpriv = per_cpu_ptr(imsic->lpriv, cpu);
+
+	if (!lpriv || imsic->global.nr_ids < local_id)
+		return NULL;
+
+	return &lpriv->vectors[local_id];
+}
+
+struct imsic_vector *imsic_vector_alloc(unsigned int hwirq, const struct cpumask *mask)
+{
+	struct imsic_vector *vec = NULL;
+	struct imsic_local_priv *lpriv;
+	unsigned long flags;
+	unsigned int cpu;
+	int local_id;
+
+	raw_spin_lock_irqsave(&imsic->matrix_lock, flags);
+	local_id = irq_matrix_alloc(imsic->matrix, mask, false, &cpu);
+	raw_spin_unlock_irqrestore(&imsic->matrix_lock, flags);
+	if (local_id < 0)
+		return NULL;
+
+	lpriv = per_cpu_ptr(imsic->lpriv, cpu);
+	vec = &lpriv->vectors[local_id];
+	vec->hwirq = hwirq;
+	vec->enable = false;
+	vec->move = NULL;
+
+	return vec;
+}
+
+void imsic_vector_free(struct imsic_vector *vec)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&imsic->matrix_lock, flags);
+	vec->hwirq = UINT_MAX;
+	irq_matrix_free(imsic->matrix, vec->cpu, vec->local_id, false);
+	raw_spin_unlock_irqrestore(&imsic->matrix_lock, flags);
+}
+
+static void __init imsic_local_cleanup(void)
+{
+	int cpu;
+	struct imsic_local_priv *lpriv;
+
+	for_each_possible_cpu(cpu) {
+		lpriv = per_cpu_ptr(imsic->lpriv, cpu);
+
+		bitmap_free(lpriv->dirty_bitmap);
+		kfree(lpriv->vectors);
+	}
+
+	free_percpu(imsic->lpriv);
+}
+
+static int __init imsic_local_init(void)
+{
+	struct imsic_global_config *global = &imsic->global;
+	struct imsic_local_priv *lpriv;
+	struct imsic_vector *vec;
+	int cpu, i;
+
+	/* Allocate per-CPU private state */
+	imsic->lpriv = alloc_percpu(typeof(*(imsic->lpriv)));
+	if (!imsic->lpriv)
+		return -ENOMEM;
+
+	/* Setup per-CPU private state */
+	for_each_possible_cpu(cpu) {
+		lpriv = per_cpu_ptr(imsic->lpriv, cpu);
+
+		raw_spin_lock_init(&lpriv->lock);
+
+		/* Allocate dirty bitmap */
+		lpriv->dirty_bitmap = bitmap_zalloc(global->nr_ids + 1, GFP_KERNEL);
+		if (!lpriv->dirty_bitmap)
+			goto fail_local_cleanup;
+
+#ifdef CONFIG_SMP
+		/* Setup lazy timer for synchronization */
+		timer_setup(&lpriv->timer, imsic_local_timer_callback, TIMER_PINNED);
+#endif
+
+		/* Allocate vector array */
+		lpriv->vectors = kcalloc(global->nr_ids + 1, sizeof(*lpriv->vectors),
+					 GFP_KERNEL);
+		if (!lpriv->vectors)
+			goto fail_local_cleanup;
+
+		/* Setup vector array */
+		for (i = 0; i <= global->nr_ids; i++) {
+			vec = &lpriv->vectors[i];
+			vec->cpu = cpu;
+			vec->local_id = i;
+			vec->hwirq = UINT_MAX;
+		}
+	}
+
+	return 0;
+
+fail_local_cleanup:
+	imsic_local_cleanup();
+	return -ENOMEM;
+}
+
+void imsic_state_online(void)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&imsic->matrix_lock, flags);
+	irq_matrix_online(imsic->matrix);
+	raw_spin_unlock_irqrestore(&imsic->matrix_lock, flags);
+}
+
+void imsic_state_offline(void)
+{
+#ifdef CONFIG_SMP
+	struct imsic_local_priv *lpriv = this_cpu_ptr(imsic->lpriv);
+#endif
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&imsic->matrix_lock, flags);
+	irq_matrix_offline(imsic->matrix);
+	raw_spin_unlock_irqrestore(&imsic->matrix_lock, flags);
+
+#ifdef CONFIG_SMP
+	raw_spin_lock_irqsave(&lpriv->lock, flags);
+	WARN_ON_ONCE(try_to_del_timer_sync(&lpriv->timer) < 0);
+	raw_spin_unlock_irqrestore(&lpriv->lock, flags);
+#endif
+}
+
+static int __init imsic_matrix_init(void)
+{
+	struct imsic_global_config *global = &imsic->global;
+
+	raw_spin_lock_init(&imsic->matrix_lock);
+	imsic->matrix = irq_alloc_matrix(global->nr_ids + 1,
+					 0, global->nr_ids + 1);
+	if (!imsic->matrix)
+		return -ENOMEM;
+
+	/* Reserve ID#0 because it is special and never implemented */
+	irq_matrix_assign_system(imsic->matrix, 0, false);
+
+	/* Reserve IPI ID because it is special and used internally */
+	irq_matrix_assign_system(imsic->matrix, IMSIC_IPI_ID, false);
+
+	return 0;
+}
+
+static int __init imsic_get_parent_hartid(struct fwnode_handle *fwnode,
+					  u32 index, unsigned long *hartid)
+{
+	struct of_phandle_args parent;
+	int rc;
+
+	/*
+	 * Currently, only OF fwnode is supported so extend this
+	 * function for ACPI support.
+	 */
+	if (!is_of_node(fwnode))
+		return -EINVAL;
+
+	rc = of_irq_parse_one(to_of_node(fwnode), index, &parent);
+	if (rc)
+		return rc;
+
+	/*
+	 * Skip interrupts other than external interrupts for
+	 * current privilege level.
+	 */
+	if (parent.args[0] != RV_IRQ_EXT)
+		return -EINVAL;
+
+	return riscv_of_parent_hartid(parent.np, hartid);
+}
+
+static int __init imsic_get_mmio_resource(struct fwnode_handle *fwnode,
+					  u32 index, struct resource *res)
+{
+	/*
+	 * Currently, only OF fwnode is supported so extend this
+	 * function for ACPI support.
+	 */
+	if (!is_of_node(fwnode))
+		return -EINVAL;
+
+	return of_address_to_resource(to_of_node(fwnode), index, res);
+}
+
+static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
+				     struct imsic_global_config *global,
+				     u32 *nr_parent_irqs,
+				     u32 *nr_mmios)
+{
+	unsigned long hartid;
+	struct resource res;
+	int rc;
+	u32 i;
+
+	/*
+	 * Currently, only OF fwnode is supported so extend this
+	 * function for ACPI support.
+	 */

drivers/irqchip/irq-riscv-imsic-state.c new file +75 -0

+		pr_err("%pfwP: no parent irqs available\n", fwnode);
+		return -EINVAL;
+	}
+
+	/* Find number of guest index bits in MSI address */
+	rc = of_property_read_u32(to_of_node(fwnode), "riscv,guest-index-bits",
+				  &global->guest_index_bits);
+	if (rc)
+		global->guest_index_bits = 0;
+
+	/* Find number of HART index bits */
+	rc = of_property_read_u32(to_of_node(fwnode), "riscv,hart-index-bits",
+				  &global->hart_index_bits);
+	if (rc) {
+		/* Assume default value */
+		global->hart_index_bits = __fls(*nr_parent_irqs);
+		if (BIT(global->hart_index_bits) < *nr_parent_irqs)
+			global->hart_index_bits++;
+	}
+
+	/* Find number of group index bits */
+	rc = of_property_read_u32(to_of_node(fwnode), "riscv,group-index-bits",
+				  &global->group_index_bits);
+	if (rc)
+		global->group_index_bits = 0;
+
+	/*
+	 * Find first bit position of group index.
+	 * If not specified assumed the default APLIC-IMSIC configuration.
+	 */
+	rc = of_property_read_u32(to_of_node(fwnode), "riscv,group-index-shift",
+				  &global->group_index_shift);
+	if (rc)
+		global->group_index_shift = IMSIC_MMIO_PAGE_SHIFT * 2;
+
+	/* Find number of interrupt identities */
+	rc = of_property_read_u32(to_of_node(fwnode), "riscv,num-ids",
+				  &global->nr_ids);
+	if (rc) {
+		pr_err("%pfwP: number of interrupt identities not found\n",
+			fwnode);
+		return rc;
+	}
+
+	/* Find number of guest interrupt identities */
+	rc = of_property_read_u32(to_of_node(fwnode), "riscv,num-guest-ids",
+				  &global->nr_guest_ids);
+	if (rc)
+		global->nr_guest_ids = global->nr_ids;
+
+	/* Sanity check guest index bits */
+	i = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT;
+	if (i < global->guest_index_bits) {
+		pr_err("%pfwP: guest index bits too big\n", fwnode);
+		return -EINVAL;
+	}
+
+	/* Sanity check HART index bits */
+	i = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT - global->guest_index_bits;
+	if (i < global->hart_index_bits) {
+		pr_err("%pfwP: HART index bits too big\n", fwnode);
+		return -EINVAL;
+	}
+
+	/* Sanity check group index bits */
+	i = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT -
+	    global->guest_index_bits - global->hart_index_bits;
+	if (i < global->group_index_bits) {
+		pr_err("%pfwP: group index bits too big\n", fwnode);
+		return -EINVAL;
+	}
+
+	/* Sanity check group index shift */
+	i = global->group_index_bits + global->group_index_shift - 1;
+	if (i >= BITS_PER_LONG) {

drivers/irqchip/irq-riscv-imsic-state.c new file +34 -0

+		return -EINVAL;
+	}
+
+	/* Compute base address */
+	rc = imsic_get_mmio_resource(fwnode, 0, &res);
+	if (rc) {
+		pr_err("%pfwP: first MMIO resource not found\n", fwnode);
+		return -EINVAL;
+	}
+	global->base_addr = res.start;
+	global->base_addr &= ~(BIT(global->guest_index_bits +
+				   global->hart_index_bits +
+				   IMSIC_MMIO_PAGE_SHIFT) - 1);
+	global->base_addr &= ~((BIT(global->group_index_bits) - 1) <<
+			       global->group_index_shift);
+
+	/* Find number of MMIO register sets */
+	while (!imsic_get_mmio_resource(fwnode, *nr_mmios, &res))
+		(*nr_mmios)++;
+
+	return 0;
+}
+
+int __init imsic_setup_state(struct fwnode_handle *fwnode)
+{
+	u32 i, j, index, nr_parent_irqs, nr_mmios, nr_handlers = 0;
+	struct imsic_global_config *global;
+	struct imsic_local_config *local;
+	void __iomem **mmios_va = NULL;
+	struct resource *mmios = NULL;
+	unsigned long reloff, hartid;
+	phys_addr_t base_addr;
+	int rc, cpu;
+

drivers/irqchip/irq-riscv-imsic-state.c new file +35 -0

+		return -EALREADY;
+	}
+
+	if (!riscv_isa_extension_available(NULL, SxAIA)) {
+		pr_err("%pfwP: AIA support not available\n", fwnode);
+		return -ENODEV;
+	}
+
+	imsic = kzalloc(sizeof(*imsic), GFP_KERNEL);
+	if (!imsic)
+		return -ENOMEM;
+	imsic->fwnode = fwnode;
+	global = &imsic->global;
+
+	global->local = alloc_percpu(typeof(*(global->local)));
+	if (!global->local) {
+		rc = -ENOMEM;
+		goto out_free_priv;
+	}
+
+	/* Parse IMSIC fwnode */
+	rc = imsic_parse_fwnode(fwnode, global, &nr_parent_irqs, &nr_mmios);
+	if (rc)
+		goto out_free_local;
+
+	/* Allocate MMIO resource array */
+	mmios = kcalloc(nr_mmios, sizeof(*mmios), GFP_KERNEL);
+	if (!mmios) {
+		rc = -ENOMEM;
+		goto out_free_local;
+	}
+
+	/* Allocate MMIO virtual address array */
+	mmios_va = kcalloc(nr_mmios, sizeof(*mmios_va), GFP_KERNEL);
+	if (!mmios_va) {

Anup,

This version is so much easier to follow! Thanks a lot for then cleanups/design changes.

A bunch of nits, and a major one, below.

Quoted 98 lines · Anup Patel

The RISC-V advanced interrupt architecture (AIA) specification defines a new MSI controller called incoming message signalled interrupt controller (IMSIC) which manages MSI on per-HART (or per-CPU) basis. It also supports IPIs as software injected MSIs. (For more details refer https://github.com/riscv/riscv-aia)

Let us add an early irqchip driver for RISC-V IMSIC which sets up the IMSIC state and provide IPIs.

Signed-off-by Anup Patel <[email protected]>

---
 drivers/irqchip/Kconfig                 |   7 +
 drivers/irqchip/Makefile                |   1 +
 drivers/irqchip/irq-riscv-imsic-early.c | 213 ++++++
 drivers/irqchip/irq-riscv-imsic-state.c | 906 ++++++++++++++++++++++++
 drivers/irqchip/irq-riscv-imsic-state.h |  98 +++
 include/linux/irqchip/riscv-imsic.h     |  87 +++
 6 files changed, 1312 insertions(+)
 create mode 100644 drivers/irqchip/irq-riscv-imsic-early.c
 create mode 100644 drivers/irqchip/irq-riscv-imsic-state.c
 create mode 100644 drivers/irqchip/irq-riscv-imsic-state.h
 create mode 100644 include/linux/irqchip/riscv-imsic.h

drivers/irqchip/Kconfig +7 -0

index f7149d0f3d45..85f86e31c996 100644
@@ -546,6 +546,13 @@ config SIFIVE_PLIC
      select IRQ_DOMAIN_HIERARCHY
      select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP
 
+config RISCV_IMSIC
+     bool
+     depends on RISCV
+     select IRQ_DOMAIN_HIERARCHY
+     select GENERIC_IRQ_MATRIX_ALLOCATOR
+     select GENERIC_MSI_IRQ
+
 config EXYNOS_IRQ_COMBINER
      bool "Samsung Exynos IRQ combiner support" if COMPILE_TEST
      depends on (ARCH_EXYNOS && ARM) || COMPILE_TEST

drivers/irqchip/Makefile +1 -0

index ffd945fe71aa..d714724387ce 100644
@@ -95,6 +95,7 @@ obj-$(CONFIG_QCOM_MPM)                      += irq-qcom-mpm.o
 obj-$(CONFIG_CSKY_MPINTC)            += irq-csky-mpintc.o
 obj-$(CONFIG_CSKY_APB_INTC)          += irq-csky-apb-intc.o
 obj-$(CONFIG_RISCV_INTC)             += irq-riscv-intc.o
+obj-$(CONFIG_RISCV_IMSIC)            += irq-riscv-imsic-state.o irq-riscv-imsic-early.o
 obj-$(CONFIG_SIFIVE_PLIC)            += irq-sifive-plic.o
 obj-$(CONFIG_IMX_IRQSTEER)           += irq-imx-irqsteer.o
 obj-$(CONFIG_IMX_INTMUX)             += irq-imx-intmux.o

drivers/irqchip/irq-riscv-imsic-early.c new file +39 -0

new file mode 100644
index 000000000000..32fe428b1c19
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
+ * Copyright (C) 2022 Ventana Micro Systems Inc.
+ */
+
+#define pr_fmt(fmt) "riscv-imsic: " fmt
+#include <linux/cpu.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/smp.h>
+
+#include "irq-riscv-imsic-state.h"
+
+static int imsic_parent_irq;
+
+#ifdef CONFIG_SMP
+static void imsic_ipi_send(unsigned int cpu)
+{
+     struct imsic_local_config *local = per_cpu_ptr(imsic->global.local, cpu);
+
+     writel_relaxed(IMSIC_IPI_ID, local->msi_va);
+}
+
+static void imsic_ipi_starting_cpu(void)
+{
+     /* Enable IPIs for current CPU. */
+     __imsic_id_set_enable(IMSIC_IPI_ID);
+}
+
+static void imsic_ipi_dying_cpu(void)
+{
+     /* Disable IPIs for current CPU. */
+     __imsic_id_clear_enable(IMSIC_IPI_ID);

drivers/irqchip/irq-riscv-imsic-early.c line 49

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-early.c new file +10 -0

+}
+
+static int __init imsic_ipi_domain_init(void)
+{
+     int virq;
+
+     /* Create IMSIC IPI multiplexing */
+     virq = ipi_mux_create(IMSIC_NR_IPI, imsic_ipi_send);
+     if (virq <= 0)
+             return (virq < 0) ? virq : -ENOMEM;

Nit: No parenthesis need to clutter.

Quoted 30 lines

drivers/irqchip/irq-riscv-imsic-early.c new file +30 -0

+
+     /* Set vIRQ range */
+     riscv_ipi_set_virq_range(virq, IMSIC_NR_IPI, true);
+
+     /* Announce that IMSIC is providing IPIs */
+     pr_info("%pfwP: providing IPIs using interrupt %d\n", imsic->fwnode, IMSIC_IPI_ID);
+
+     return 0;
+}
+#else
+static void imsic_ipi_starting_cpu(void)
+{
+}
+
+static void imsic_ipi_dying_cpu(void)
+{
+}
+
+static int __init imsic_ipi_domain_init(void)
+{
+     return 0;
+}
+#endif
+
+/*
+ * To handle an interrupt, we read the TOPEI CSR and write zero in one
+ * instruction. If TOPEI CSR is non-zero then we translate TOPEI.ID to
+ * Linux interrupt number and let Linux IRQ subsystem handle it.
+ */
+static void imsic_handle_irq(struct irq_desc *desc)

drivers/irqchip/irq-riscv-imsic-early.c line 89

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-early.c new file +10 -0

+{
+     struct irq_chip *chip = irq_desc_get_chip(desc);
+     int err, cpu = smp_processor_id();
+     struct imsic_vector *vec;
+     unsigned long local_id;
+
+     chained_irq_enter(chip, desc);
+
+     while ((local_id = csr_swap(CSR_TOPEI, 0))) {
+             local_id = local_id >> TOPEI_ID_SHIFT;

Nit: Wdyt about moving shift into the loop predicate, or using >>=?

drivers/irqchip/irq-riscv-imsic-early.c line 94

Quoted 5 lines

drivers/irqchip/irq-riscv-imsic-early.c new file +5 -0

+
+             if (local_id == IMSIC_IPI_ID) {
+#ifdef CONFIG_SMP
+                     ipi_mux_process();
+#endif

Is IMSIC_IPI_ID a thing on !IS_ENABLED(CONFIG_SMP)?

Quoted 4 lines

drivers/irqchip/irq-riscv-imsic-early.c new file +4 -0

+                     continue;
+             }
+
+             if (unlikely(!imsic->base_domain))

drivers/irqchip/irq-riscv-imsic-early.c line 108

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-early.c new file +10 -0

+                     continue;
+
+             vec = imsic_vector_from_local_id(cpu, local_id);
+             if (!vec) {
+                     pr_warn_ratelimited("vector not found for local ID 0x%lx\n", local_id);
+                     continue;
+             }
+
+             err = generic_handle_domain_irq(imsic->base_domain,
+                                             vec->hwirq);

Nit: 100 chars

Quoted 82 lines

drivers/irqchip/irq-riscv-imsic-early.c new file +82 -0

+             if (unlikely(err))
+                     pr_warn_ratelimited("hwirq 0x%x mapping not found\n", vec->hwirq);
+     }
+
+     chained_irq_exit(chip, desc);
+}
+
+static int imsic_starting_cpu(unsigned int cpu)
+{
+     /* Mark per-CPU IMSIC state as online */
+     imsic_state_online();
+
+     /* Enable per-CPU parent interrupt */
+     enable_percpu_irq(imsic_parent_irq, irq_get_trigger_type(imsic_parent_irq));
+
+     /* Setup IPIs */
+     imsic_ipi_starting_cpu();
+
+     /*
+      * Interrupts identities might have been enabled/disabled while
+      * this CPU was not running so sync-up local enable/disable state.
+      */
+     imsic_local_sync_all();
+
+     /* Enable local interrupt delivery */
+     imsic_local_delivery(true);
+
+     return 0;
+}
+
+static int imsic_dying_cpu(unsigned int cpu)
+{
+     /* Cleanup IPIs */
+     imsic_ipi_dying_cpu();
+
+     /* Mark per-CPU IMSIC state as offline */
+     imsic_state_offline();
+
+     return 0;
+}
+
+static int __init imsic_early_probe(struct fwnode_handle *fwnode)
+{
+     struct irq_domain *domain;
+     int rc;
+
+     /* Find parent domain and register chained handler */
+     domain = irq_find_matching_fwnode(riscv_get_intc_hwnode(), DOMAIN_BUS_ANY);
+     if (!domain) {
+             pr_err("%pfwP: Failed to find INTC domain\n", fwnode);
+             return -ENOENT;
+     }
+     imsic_parent_irq = irq_create_mapping(domain, RV_IRQ_EXT);
+     if (!imsic_parent_irq) {
+             pr_err("%pfwP: Failed to create INTC mapping\n", fwnode);
+             return -ENOENT;
+     }
+
+     /* Initialize IPI domain */
+     rc = imsic_ipi_domain_init();
+     if (rc) {
+             pr_err("%pfwP: Failed to initialize IPI domain\n", fwnode);
+             return rc;
+     }
+
+     /* Setup chained handler to the parent domain interrupt */
+     irq_set_chained_handler(imsic_parent_irq, imsic_handle_irq);
+
+     /*
+      * Setup cpuhp state (must be done after setting imsic_parent_irq)
+      *
+      * Don't disable per-CPU IMSIC file when CPU goes offline
+      * because this affects IPI and the masking/unmasking of
+      * virtual IPIs is done via generic IPI-Mux
+      */
+     cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "irqchip/riscv/imsic:starting",
+                       imsic_starting_cpu, imsic_dying_cpu);
+
+     return 0;
+}
+
+static int __init imsic_early_dt_init(struct device_node *node,

drivers/irqchip/irq-riscv-imsic-early.c line 200

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-early.c new file +10 -0

+                                   struct device_node *parent)
+{
+     struct fwnode_handle *fwnode = &node->fwnode;
+     int rc;
+
+     /* Setup IMSIC state */
+     rc = imsic_setup_state(fwnode);
+     if (rc) {
+             pr_err("%pfwP: failed to setup state (error %d)\n",
+                     fwnode, rc);

Nit. 100 chars

Quoted 84 lines

drivers/irqchip/irq-riscv-imsic-early.c new file +13 -0

+             return rc;
+     }
+
+     /* Do early setup of IPIs */
+     rc = imsic_early_probe(fwnode);
+     if (rc)
+             return rc;
+
+     /* Ensure that OF platform device gets probed */
+     of_node_clear_flag(node, OF_POPULATED);
+     return 0;
+}
+IRQCHIP_DECLARE(riscv_imsic, "riscv,imsics", imsic_early_dt_init);

drivers/irqchip/irq-riscv-imsic-state.c new file +65 -0

new file mode 100644
index 000000000000..4f347486ec7c
@@ -0,0 +1,906 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
+ * Copyright (C) 2022 Ventana Micro Systems Inc.
+ */
+
+#define pr_fmt(fmt) "riscv-imsic: " fmt
+#include <linux/cpu.h>
+#include <linux/bitmap.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/seq_file.h>
+#include <linux/spinlock.h>
+#include <linux/smp.h>
+#include <asm/hwcap.h>
+
+#include "irq-riscv-imsic-state.h"
+
+#define IMSIC_DISABLE_EIDELIVERY             0
+#define IMSIC_ENABLE_EIDELIVERY                      1
+#define IMSIC_DISABLE_EITHRESHOLD            1
+#define IMSIC_ENABLE_EITHRESHOLD             0
+
+static inline void imsic_csr_write(unsigned long reg, unsigned long val)
+{
+     csr_write(CSR_ISELECT, reg);
+     csr_write(CSR_IREG, val);
+}
+
+static inline unsigned long imsic_csr_read(unsigned long reg)
+{
+     csr_write(CSR_ISELECT, reg);
+     return csr_read(CSR_IREG);
+}
+
+static inline unsigned long imsic_csr_read_clear(unsigned long reg, unsigned long val)
+{
+     csr_write(CSR_ISELECT, reg);
+     return csr_read_clear(CSR_IREG, val);
+}
+
+static inline void imsic_csr_set(unsigned long reg, unsigned long val)
+{
+     csr_write(CSR_ISELECT, reg);
+     csr_set(CSR_IREG, val);
+}
+
+static inline void imsic_csr_clear(unsigned long reg, unsigned long val)
+{
+     csr_write(CSR_ISELECT, reg);
+     csr_clear(CSR_IREG, val);
+}
+
+struct imsic_priv *imsic;
+
+const struct imsic_global_config *imsic_get_global_config(void)
+{
+     return imsic ? &imsic->global : NULL;
+}
+EXPORT_SYMBOL_GPL(imsic_get_global_config);
+

drivers/irqchip/irq-riscv-imsic-state.c line 75

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +10 -0

+static bool __imsic_eix_read_clear(unsigned long id, bool pend)
+{
+     unsigned long isel, imask;
+
+     isel = id / BITS_PER_LONG;
+     isel *= BITS_PER_LONG / IMSIC_EIPx_BITS;
+     isel += pend ? IMSIC_EIP0 : IMSIC_EIE0;
+     imask = BIT(id & (__riscv_xlen - 1));
+
+     return (imsic_csr_read_clear(isel, imask) & imask) ? true : false;

Nit: use return !!(imsic_csr_read_clear(isel, imask) & imask)

Quoted 11 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +11 -0

+}
+
+static inline bool __imsic_id_read_clear_enabled(unsigned long id)
+{
+     return __imsic_eix_read_clear(id, false);
+}
+
+static inline bool __imsic_id_read_clear_pending(unsigned long id)
+{
+     return __imsic_eix_read_clear(id, true);
+}

drivers/irqchip/irq-riscv-imsic-state.c line 96

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +10 -0

+
+void __imsic_eix_update(unsigned long base_id, unsigned long num_id, bool pend, bool val)
+{
+     unsigned long id = base_id, last_id = base_id + num_id;
+     unsigned long i, isel, ireg;
+
+     while (id < last_id) {
+             isel = id / BITS_PER_LONG;
+             isel *= BITS_PER_LONG / IMSIC_EIPx_BITS;
+             isel += (pend) ? IMSIC_EIP0 : IMSIC_EIE0;

Nit: Redundant parenthesis.

drivers/irqchip/irq-riscv-imsic-state.c line 105

Quoted 9 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +9 -0

+
+             /*
+              * Prepare the ID mask to be programmed in the
+              * IMSIC EIEx and EIPx registers. These registers
+              * are XLEN-wide and we must not touch IDs which
+              * are < base_id and >= (base_id + num_id).
+              */
+             ireg = 0;
+             for (i = id & (__riscv_xlen - 1); (id < last_id) && (i < __riscv_xlen); i++) {

Nit: Redundant parenthesis "(id < last_id) && (i < __riscv_xlen)", which is also inconsistent with other usage in this changeset.

Quoted 112 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +112 -0

+                     ireg |= BIT(i);
+                     id++;
+             }
+
+             /*
+              * The IMSIC EIEx and EIPx registers are indirectly
+              * accessed via using ISELECT and IREG CSRs so we
+              * need to access these CSRs without getting preempted.
+              *
+              * All existing users of this function call this
+              * function with local IRQs disabled so we don't
+              * need to do anything special here.
+              */
+             if (val)
+                     imsic_csr_set(isel, ireg);
+             else
+                     imsic_csr_clear(isel, ireg);
+     }
+}
+
+/* MUST be called with lpriv->lock held */
+static void __imsic_local_sync(struct imsic_local_priv *lpriv)
+{
+     struct imsic_local_config *mlocal;
+     struct imsic_vector *vec, *mvec;
+     int i;
+
+     /* This pairs with the barrier in __imsic_remote_sync(). */
+     smp_mb();
+
+     for_each_set_bit(i, lpriv->dirty_bitmap, imsic->global.nr_ids + 1) {
+             if (!i || i == IMSIC_IPI_ID)
+                     goto skip;
+             vec = &lpriv->vectors[i];
+
+             if (vec->enable)
+                     __imsic_id_set_enable(i);
+             else
+                     __imsic_id_clear_enable(i);
+
+             /*
+              * If the ID was being moved to a new ID on some other CPU
+              * then we can get a MSI during the movement so check the
+              * ID pending bit and re-trigger the new ID on other CPU
+              * using MMIO write.
+              */
+             mvec = vec->move;
+             vec->move = NULL;
+             if (mvec && mvec != vec) {
+                     if (__imsic_id_read_clear_pending(i)) {
+                             mlocal = per_cpu_ptr(imsic->global.local, mvec->cpu);
+                             writel_relaxed(mvec->local_id, mlocal->msi_va);
+                     }
+
+                     imsic_vector_free(&lpriv->vectors[i]);
+             }
+
+skip:
+             bitmap_clear(lpriv->dirty_bitmap, i, 1);
+     }
+}
+
+void imsic_local_sync_all(void)
+{
+     struct imsic_local_priv *lpriv = this_cpu_ptr(imsic->lpriv);
+     unsigned long flags;
+
+     raw_spin_lock_irqsave(&lpriv->lock, flags);
+     bitmap_fill(lpriv->dirty_bitmap, imsic->global.nr_ids + 1);
+     __imsic_local_sync(lpriv);
+     raw_spin_unlock_irqrestore(&lpriv->lock, flags);
+}
+
+void imsic_local_delivery(bool enable)
+{
+     if (enable) {
+             imsic_csr_write(IMSIC_EITHRESHOLD, IMSIC_ENABLE_EITHRESHOLD);
+             imsic_csr_write(IMSIC_EIDELIVERY, IMSIC_ENABLE_EIDELIVERY);
+             return;
+     }
+
+     imsic_csr_write(IMSIC_EIDELIVERY, IMSIC_DISABLE_EIDELIVERY);
+     imsic_csr_write(IMSIC_EITHRESHOLD, IMSIC_DISABLE_EITHRESHOLD);
+}
+
+#ifdef CONFIG_SMP
+static void imsic_local_timer_callback(struct timer_list *timer)
+{
+     struct imsic_local_priv *lpriv = this_cpu_ptr(imsic->lpriv);
+     unsigned long flags;
+
+     raw_spin_lock_irqsave(&lpriv->lock, flags);
+     __imsic_local_sync(lpriv);
+     raw_spin_unlock_irqrestore(&lpriv->lock, flags);
+}
+
+/* MUST be called with lpriv->lock held */
+static void __imsic_remote_sync(struct imsic_local_priv *lpriv, unsigned int cpu)
+{
+     /*
+      * Ensure that changes to vector enable, vector move and
+      * dirty bitmap are visible to the target CPU.
+      *
+      * This pairs with the barrier in __imsic_local_sync().
+      */
+     smp_mb();
+
+     /*
+      * We schedule a timer on the target CPU if the target CPU is not
+      * same as the current CPU. An offline CPU will unconditionally
+      * synchronize IDs through imsic_starting_cpu() when the
+      * CPU is brought up.

drivers/irqchip/irq-riscv-imsic-state.c line 227

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +10 -0

+      */
+     if (cpu_online(cpu)) {
+             if (cpu != smp_processor_id()) {
+                     if (!timer_pending(&lpriv->timer)) {
+                             lpriv->timer.expires = jiffies + 1;
+                             add_timer_on(&lpriv->timer, cpu);
+                     }
+             } else {
+                     __imsic_local_sync(lpriv);
+             }

Nit: Early exit/return vs else-clause for readability

Quoted 21 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +21 -0

+     }
+}
+#else
+/* MUST be called with lpriv->lock held */
+static void __imsic_remote_sync(struct imsic_local_priv *lpriv, unsigned int cpu)
+{
+     __imsic_local_sync(lpriv);
+}
+#endif
+
+void imsic_vector_mask(struct imsic_vector *vec)
+{
+     struct imsic_local_priv *lpriv;
+
+     lpriv = per_cpu_ptr(imsic->lpriv, vec->cpu);
+     if (WARN_ON(&lpriv->vectors[vec->local_id] != vec))
+             return;
+
+     /*
+      * This function is called through Linux irq subsystem with
+      * irqs disabled so no need to save/restore irq flags.

drivers/irqchip/irq-riscv-imsic-state.c line 258

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +10 -0

+      */
+
+     raw_spin_lock(&lpriv->lock);
+
+     vec->enable = false;
+     bitmap_set(lpriv->dirty_bitmap, vec->local_id, 1);
+     __imsic_remote_sync(lpriv, vec->cpu);
+
+     raw_spin_unlock(&lpriv->lock);
+}

Really nice that you're using a timer for the vector affinity change, and got rid of the special/weird IMSIC/sync IPI. Can you really use a timer for mask/unmask? That makes the mask/unmask operation asynchronous!

That was what I was trying to get though with this comment:
https://lore.kernel.org/linux-riscv/[email protected]/

Also, using the smp_* IPI functions, you can pass arguments, so you don't need the dirty_bitmap tracking the changes.

Quoted 109 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +108 -0

+
+void imsic_vector_unmask(struct imsic_vector *vec)
+{
+     struct imsic_local_priv *lpriv;
+
+     lpriv = per_cpu_ptr(imsic->lpriv, vec->cpu);
+     if (WARN_ON(&lpriv->vectors[vec->local_id] != vec))
+             return;
+
+     /*
+      * This function is called through Linux irq subsystem with
+      * irqs disabled so no need to save/restore irq flags.
+      */
+
+     raw_spin_lock(&lpriv->lock);
+
+     vec->enable = true;
+     bitmap_set(lpriv->dirty_bitmap, vec->local_id, 1);
+     __imsic_remote_sync(lpriv, vec->cpu);
+
+     raw_spin_unlock(&lpriv->lock);
+}
+
+
+bool imsic_vector_isenabled(struct imsic_vector *vec)
+{
+     struct imsic_local_priv *lpriv = per_cpu_ptr(imsic->lpriv, vec->cpu);
+     unsigned long flags;
+     bool ret;
+
+     raw_spin_lock_irqsave(&lpriv->lock, flags);
+     ret = vec->enable;
+     raw_spin_unlock_irqrestore(&lpriv->lock, flags);
+
+     return ret;
+}
+
+struct imsic_vector *imsic_vector_get_move(struct imsic_vector *vec)
+{
+     struct imsic_local_priv *lpriv = per_cpu_ptr(imsic->lpriv, vec->cpu);
+     struct imsic_vector *ret;
+     unsigned long flags;
+
+     raw_spin_lock_irqsave(&lpriv->lock, flags);
+     ret = vec->move;
+     raw_spin_unlock_irqrestore(&lpriv->lock, flags);
+
+     return ret;
+}
+
+static bool imsic_vector_move_update(struct imsic_local_priv *lpriv, struct imsic_vector *vec,
+                                  bool new_enable, struct imsic_vector *new_move)
+{
+     unsigned long flags;
+     bool enabled;
+
+     raw_spin_lock_irqsave(&lpriv->lock, flags);
+
+     /* Update enable and move details */
+     enabled = vec->enable;
+     vec->enable = new_enable;
+     vec->move = new_move;
+
+     /* Mark the vector as dirty and synchronize */
+     bitmap_set(lpriv->dirty_bitmap, vec->local_id, 1);
+     __imsic_remote_sync(lpriv, vec->cpu);
+
+     raw_spin_unlock_irqrestore(&lpriv->lock, flags);
+
+     return enabled;
+}
+
+void imsic_vector_move(struct imsic_vector *old_vec, struct imsic_vector *new_vec)
+{
+     struct imsic_local_priv *old_lpriv, *new_lpriv;
+     bool enabled;
+
+     if (WARN_ON(old_vec->cpu == new_vec->cpu))
+             return;
+
+     old_lpriv = per_cpu_ptr(imsic->lpriv, old_vec->cpu);
+     if (WARN_ON(&old_lpriv->vectors[old_vec->local_id] != old_vec))
+             return;
+
+     new_lpriv = per_cpu_ptr(imsic->lpriv, new_vec->cpu);
+     if (WARN_ON(&new_lpriv->vectors[new_vec->local_id] != new_vec))
+             return;
+
+     /*
+      * Move and re-trigger the new vector based on the pending
+      * state of the old vector because we might get a device
+      * interrupt on the old vector while device was being moved
+      * to the new vector.
+      */
+     enabled = imsic_vector_move_update(old_lpriv, old_vec, false, new_vec);
+     imsic_vector_move_update(new_lpriv, new_vec, enabled, new_vec);
+}
+
+#ifdef CONFIG_GENERIC_IRQ_DEBUGFS
+void imsic_vector_debug_show(struct seq_file *m, struct imsic_vector *vec, int ind)
+{
+     struct imsic_local_priv *lpriv;
+     struct imsic_vector *mvec;
+     bool is_enabled;
+
+     lpriv = per_cpu_ptr(imsic->lpriv, vec->cpu);
+     if (WARN_ON(&lpriv->vectors[vec->local_id] != vec))
+             return;

drivers/irqchip/irq-riscv-imsic-state.c line 376

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +10 -0

+
+     is_enabled = imsic_vector_isenabled(vec);
+     mvec = imsic_vector_get_move(vec);
+
+     seq_printf(m, "%*starget_cpu      : %5u\n", ind, "", vec->cpu);
+     seq_printf(m, "%*starget_local_id : %5u\n", ind, "", vec->local_id);
+     seq_printf(m, "%*sis_reserved     : %5u\n", ind, "",
+                (vec->local_id <= IMSIC_IPI_ID) ? 1 : 0);
+     seq_printf(m, "%*sis_enabled      : %5u\n", ind, "", (is_enabled) ? 1 : 0);
+     seq_printf(m, "%*sis_move_pending : %5u\n", ind, "", (mvec) ? 1 : 0);

Nit: Redundant parenthesis.

Quoted 220 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +220 -0

+     if (mvec) {
+             seq_printf(m, "%*smove_cpu        : %5u\n", ind, "", mvec->cpu);
+             seq_printf(m, "%*smove_local_id   : %5u\n", ind, "", mvec->local_id);
+     }
+}
+
+void imsic_vector_debug_show_summary(struct seq_file *m, int ind)
+{
+     irq_matrix_debug_show(m, imsic->matrix, ind);
+}
+#endif
+
+struct imsic_vector *imsic_vector_from_local_id(unsigned int cpu, unsigned int local_id)
+{
+     struct imsic_local_priv *lpriv = per_cpu_ptr(imsic->lpriv, cpu);
+
+     if (!lpriv || imsic->global.nr_ids < local_id)
+             return NULL;
+
+     return &lpriv->vectors[local_id];
+}
+
+struct imsic_vector *imsic_vector_alloc(unsigned int hwirq, const struct cpumask *mask)
+{
+     struct imsic_vector *vec = NULL;
+     struct imsic_local_priv *lpriv;
+     unsigned long flags;
+     unsigned int cpu;
+     int local_id;
+
+     raw_spin_lock_irqsave(&imsic->matrix_lock, flags);
+     local_id = irq_matrix_alloc(imsic->matrix, mask, false, &cpu);
+     raw_spin_unlock_irqrestore(&imsic->matrix_lock, flags);
+     if (local_id < 0)
+             return NULL;
+
+     lpriv = per_cpu_ptr(imsic->lpriv, cpu);
+     vec = &lpriv->vectors[local_id];
+     vec->hwirq = hwirq;
+     vec->enable = false;
+     vec->move = NULL;
+
+     return vec;
+}
+
+void imsic_vector_free(struct imsic_vector *vec)
+{
+     unsigned long flags;
+
+     raw_spin_lock_irqsave(&imsic->matrix_lock, flags);
+     vec->hwirq = UINT_MAX;
+     irq_matrix_free(imsic->matrix, vec->cpu, vec->local_id, false);
+     raw_spin_unlock_irqrestore(&imsic->matrix_lock, flags);
+}
+
+static void __init imsic_local_cleanup(void)
+{
+     int cpu;
+     struct imsic_local_priv *lpriv;
+
+     for_each_possible_cpu(cpu) {
+             lpriv = per_cpu_ptr(imsic->lpriv, cpu);
+
+             bitmap_free(lpriv->dirty_bitmap);
+             kfree(lpriv->vectors);
+     }
+
+     free_percpu(imsic->lpriv);
+}
+
+static int __init imsic_local_init(void)
+{
+     struct imsic_global_config *global = &imsic->global;
+     struct imsic_local_priv *lpriv;
+     struct imsic_vector *vec;
+     int cpu, i;
+
+     /* Allocate per-CPU private state */
+     imsic->lpriv = alloc_percpu(typeof(*(imsic->lpriv)));
+     if (!imsic->lpriv)
+             return -ENOMEM;
+
+     /* Setup per-CPU private state */
+     for_each_possible_cpu(cpu) {
+             lpriv = per_cpu_ptr(imsic->lpriv, cpu);
+
+             raw_spin_lock_init(&lpriv->lock);
+
+             /* Allocate dirty bitmap */
+             lpriv->dirty_bitmap = bitmap_zalloc(global->nr_ids + 1, GFP_KERNEL);
+             if (!lpriv->dirty_bitmap)
+                     goto fail_local_cleanup;
+
+#ifdef CONFIG_SMP
+             /* Setup lazy timer for synchronization */
+             timer_setup(&lpriv->timer, imsic_local_timer_callback, TIMER_PINNED);
+#endif
+
+             /* Allocate vector array */
+             lpriv->vectors = kcalloc(global->nr_ids + 1, sizeof(*lpriv->vectors),
+                                      GFP_KERNEL);
+             if (!lpriv->vectors)
+                     goto fail_local_cleanup;
+
+             /* Setup vector array */
+             for (i = 0; i <= global->nr_ids; i++) {
+                     vec = &lpriv->vectors[i];
+                     vec->cpu = cpu;
+                     vec->local_id = i;
+                     vec->hwirq = UINT_MAX;
+             }
+     }
+
+     return 0;
+
+fail_local_cleanup:
+     imsic_local_cleanup();
+     return -ENOMEM;
+}
+
+void imsic_state_online(void)
+{
+     unsigned long flags;
+
+     raw_spin_lock_irqsave(&imsic->matrix_lock, flags);
+     irq_matrix_online(imsic->matrix);
+     raw_spin_unlock_irqrestore(&imsic->matrix_lock, flags);
+}
+
+void imsic_state_offline(void)
+{
+#ifdef CONFIG_SMP
+     struct imsic_local_priv *lpriv = this_cpu_ptr(imsic->lpriv);
+#endif
+     unsigned long flags;
+
+     raw_spin_lock_irqsave(&imsic->matrix_lock, flags);
+     irq_matrix_offline(imsic->matrix);
+     raw_spin_unlock_irqrestore(&imsic->matrix_lock, flags);
+
+#ifdef CONFIG_SMP
+     raw_spin_lock_irqsave(&lpriv->lock, flags);
+     WARN_ON_ONCE(try_to_del_timer_sync(&lpriv->timer) < 0);
+     raw_spin_unlock_irqrestore(&lpriv->lock, flags);
+#endif
+}
+
+static int __init imsic_matrix_init(void)
+{
+     struct imsic_global_config *global = &imsic->global;
+
+     raw_spin_lock_init(&imsic->matrix_lock);
+     imsic->matrix = irq_alloc_matrix(global->nr_ids + 1,
+                                      0, global->nr_ids + 1);
+     if (!imsic->matrix)
+             return -ENOMEM;
+
+     /* Reserve ID#0 because it is special and never implemented */
+     irq_matrix_assign_system(imsic->matrix, 0, false);
+
+     /* Reserve IPI ID because it is special and used internally */
+     irq_matrix_assign_system(imsic->matrix, IMSIC_IPI_ID, false);
+
+     return 0;
+}
+
+static int __init imsic_get_parent_hartid(struct fwnode_handle *fwnode,
+                                       u32 index, unsigned long *hartid)
+{
+     struct of_phandle_args parent;
+     int rc;
+
+     /*
+      * Currently, only OF fwnode is supported so extend this
+      * function for ACPI support.
+      */
+     if (!is_of_node(fwnode))
+             return -EINVAL;
+
+     rc = of_irq_parse_one(to_of_node(fwnode), index, &parent);
+     if (rc)
+             return rc;
+
+     /*
+      * Skip interrupts other than external interrupts for
+      * current privilege level.
+      */
+     if (parent.args[0] != RV_IRQ_EXT)
+             return -EINVAL;
+
+     return riscv_of_parent_hartid(parent.np, hartid);
+}
+
+static int __init imsic_get_mmio_resource(struct fwnode_handle *fwnode,
+                                       u32 index, struct resource *res)
+{
+     /*
+      * Currently, only OF fwnode is supported so extend this
+      * function for ACPI support.
+      */
+     if (!is_of_node(fwnode))
+             return -EINVAL;
+
+     return of_address_to_resource(to_of_node(fwnode), index, res);
+}
+
+static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
+                                  struct imsic_global_config *global,
+                                  u32 *nr_parent_irqs,
+                                  u32 *nr_mmios)
+{
+     unsigned long hartid;
+     struct resource res;
+     int rc;
+     u32 i;
+
+     /*
+      * Currently, only OF fwnode is supported so extend this
+      * function for ACPI support.
+      */

drivers/irqchip/irq-riscv-imsic-state.c line 606

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +10 -0

+     if (!is_of_node(fwnode))
+             return -EINVAL;
+
+     *nr_parent_irqs = 0;
+     *nr_mmios = 0;
+
+     /* Find number of parent interrupts */
+     while (!imsic_get_parent_hartid(fwnode, *nr_parent_irqs, &hartid))
+             (*nr_parent_irqs)++;
+     if (!(*nr_parent_irqs)) {

Nit: Redundant parenthesis

Quoted 75 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +75 -0

+             pr_err("%pfwP: no parent irqs available\n", fwnode);
+             return -EINVAL;
+     }
+
+     /* Find number of guest index bits in MSI address */
+     rc = of_property_read_u32(to_of_node(fwnode), "riscv,guest-index-bits",
+                               &global->guest_index_bits);
+     if (rc)
+             global->guest_index_bits = 0;
+
+     /* Find number of HART index bits */
+     rc = of_property_read_u32(to_of_node(fwnode), "riscv,hart-index-bits",
+                               &global->hart_index_bits);
+     if (rc) {
+             /* Assume default value */
+             global->hart_index_bits = __fls(*nr_parent_irqs);
+             if (BIT(global->hart_index_bits) < *nr_parent_irqs)
+                     global->hart_index_bits++;
+     }
+
+     /* Find number of group index bits */
+     rc = of_property_read_u32(to_of_node(fwnode), "riscv,group-index-bits",
+                               &global->group_index_bits);
+     if (rc)
+             global->group_index_bits = 0;
+
+     /*
+      * Find first bit position of group index.
+      * If not specified assumed the default APLIC-IMSIC configuration.
+      */
+     rc = of_property_read_u32(to_of_node(fwnode), "riscv,group-index-shift",
+                               &global->group_index_shift);
+     if (rc)
+             global->group_index_shift = IMSIC_MMIO_PAGE_SHIFT * 2;
+
+     /* Find number of interrupt identities */
+     rc = of_property_read_u32(to_of_node(fwnode), "riscv,num-ids",
+                               &global->nr_ids);
+     if (rc) {
+             pr_err("%pfwP: number of interrupt identities not found\n",
+                     fwnode);
+             return rc;
+     }
+
+     /* Find number of guest interrupt identities */
+     rc = of_property_read_u32(to_of_node(fwnode), "riscv,num-guest-ids",
+                               &global->nr_guest_ids);
+     if (rc)
+             global->nr_guest_ids = global->nr_ids;
+
+     /* Sanity check guest index bits */
+     i = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT;
+     if (i < global->guest_index_bits) {
+             pr_err("%pfwP: guest index bits too big\n", fwnode);
+             return -EINVAL;
+     }
+
+     /* Sanity check HART index bits */
+     i = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT - global->guest_index_bits;
+     if (i < global->hart_index_bits) {
+             pr_err("%pfwP: HART index bits too big\n", fwnode);
+             return -EINVAL;
+     }
+
+     /* Sanity check group index bits */
+     i = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT -
+         global->guest_index_bits - global->hart_index_bits;
+     if (i < global->group_index_bits) {
+             pr_err("%pfwP: group index bits too big\n", fwnode);
+             return -EINVAL;
+     }
+
+     /* Sanity check group index shift */
+     i = global->group_index_bits + global->group_index_shift - 1;
+     if (i >= BITS_PER_LONG) {

drivers/irqchip/irq-riscv-imsic-state.c line 691

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +10 -0

+             pr_err("%pfwP: group index shift too big\n", fwnode);
+             return -EINVAL;
+     }
+
+     /* Sanity check number of interrupt identities */
+     if ((global->nr_ids < IMSIC_MIN_ID) ||
+         (global->nr_ids >= IMSIC_MAX_ID) ||
+         ((global->nr_ids & IMSIC_MIN_ID) != IMSIC_MIN_ID)) {
+             pr_err("%pfwP: invalid number of interrupt identities\n",
+                     fwnode);

Nit: 100 chars

drivers/irqchip/irq-riscv-imsic-state.c line 700

Quoted 9 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +9 -0

+             return -EINVAL;
+     }
+
+     /* Sanity check number of guest interrupt identities */
+     if ((global->nr_guest_ids < IMSIC_MIN_ID) ||
+         (global->nr_guest_ids >= IMSIC_MAX_ID) ||
+         ((global->nr_guest_ids & IMSIC_MIN_ID) != IMSIC_MIN_ID)) {
+             pr_err("%pfwP: invalid number of guest interrupt identities\n",
+                     fwnode);

Nit: 100 chars

Quoted 34 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +34 -0

+             return -EINVAL;
+     }
+
+     /* Compute base address */
+     rc = imsic_get_mmio_resource(fwnode, 0, &res);
+     if (rc) {
+             pr_err("%pfwP: first MMIO resource not found\n", fwnode);
+             return -EINVAL;
+     }
+     global->base_addr = res.start;
+     global->base_addr &= ~(BIT(global->guest_index_bits +
+                                global->hart_index_bits +
+                                IMSIC_MMIO_PAGE_SHIFT) - 1);
+     global->base_addr &= ~((BIT(global->group_index_bits) - 1) <<
+                            global->group_index_shift);
+
+     /* Find number of MMIO register sets */
+     while (!imsic_get_mmio_resource(fwnode, *nr_mmios, &res))
+             (*nr_mmios)++;
+
+     return 0;
+}
+
+int __init imsic_setup_state(struct fwnode_handle *fwnode)
+{
+     u32 i, j, index, nr_parent_irqs, nr_mmios, nr_handlers = 0;
+     struct imsic_global_config *global;
+     struct imsic_local_config *local;
+     void __iomem **mmios_va = NULL;
+     struct resource *mmios = NULL;
+     unsigned long reloff, hartid;
+     phys_addr_t base_addr;
+     int rc, cpu;
+

drivers/irqchip/irq-riscv-imsic-state.c line 744

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +10 -0

+     /*
+      * Only one IMSIC instance allowed in a platform for clean
+      * implementation of SMP IRQ affinity and per-CPU IPIs.
+      *
+      * This means on a multi-socket (or multi-die) platform we
+      * will have multiple MMIO regions for one IMSIC instance.
+      */
+     if (imsic) {
+             pr_err("%pfwP: already initialized hence ignoring\n",
+                     fwnode);

Nit: 100 chars

Quoted 35 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +35 -0

+             return -EALREADY;
+     }
+
+     if (!riscv_isa_extension_available(NULL, SxAIA)) {
+             pr_err("%pfwP: AIA support not available\n", fwnode);
+             return -ENODEV;
+     }
+
+     imsic = kzalloc(sizeof(*imsic), GFP_KERNEL);
+     if (!imsic)
+             return -ENOMEM;
+     imsic->fwnode = fwnode;
+     global = &imsic->global;
+
+     global->local = alloc_percpu(typeof(*(global->local)));
+     if (!global->local) {
+             rc = -ENOMEM;
+             goto out_free_priv;
+     }
+
+     /* Parse IMSIC fwnode */
+     rc = imsic_parse_fwnode(fwnode, global, &nr_parent_irqs, &nr_mmios);
+     if (rc)
+             goto out_free_local;
+
+     /* Allocate MMIO resource array */
+     mmios = kcalloc(nr_mmios, sizeof(*mmios), GFP_KERNEL);
+     if (!mmios) {
+             rc = -ENOMEM;
+             goto out_free_local;
+     }
+
+     /* Allocate MMIO virtual address array */
+     mmios_va = kcalloc(nr_mmios, sizeof(*mmios_va), GFP_KERNEL);
+     if (!mmios_va) {

drivers/irqchip/irq-riscv-imsic-state.c line 789

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +10 -0

+             rc = -ENOMEM;
+             goto out_iounmap;
+     }
+
+     /* Parse and map MMIO register sets */
+     for (i = 0; i < nr_mmios; i++) {
+             rc = imsic_get_mmio_resource(fwnode, i, &mmios[i]);
+             if (rc) {
+                     pr_err("%pfwP: unable to parse MMIO regset %d\n",
+                             fwnode, i);

Nit: 100 chars

drivers/irqchip/irq-riscv-imsic-state.c line 802

Quoted 13 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +13 -0

+                     goto out_iounmap;
+             }
+
+             base_addr = mmios[i].start;
+             base_addr &= ~(BIT(global->guest_index_bits +
+                                global->hart_index_bits +
+                                IMSIC_MMIO_PAGE_SHIFT) - 1);
+             base_addr &= ~((BIT(global->group_index_bits) - 1) <<
+                            global->group_index_shift);
+             if (base_addr != global->base_addr) {
+                     rc = -EINVAL;
+                     pr_err("%pfwP: address mismatch for regset %d\n",
+                             fwnode, i);

Nit: 100 chars... and all the places below where applicable.

Björn

Quoted 27 lines

Quoted 11 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +11 -0

+void imsic_vector_mask(struct imsic_vector *vec)
+{
+     struct imsic_local_priv *lpriv;
+
+     lpriv = per_cpu_ptr(imsic->lpriv, vec->cpu);
+     if (WARN_ON(&lpriv->vectors[vec->local_id] != vec))
+             return;
+
+     /*
+      * This function is called through Linux irq subsystem with
+      * irqs disabled so no need to save/restore irq flags.

drivers/irqchip/irq-riscv-imsic-state.c line 258

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +10 -0

+      */
+
+     raw_spin_lock(&lpriv->lock);
+
+     vec->enable = false;
+     bitmap_set(lpriv->dirty_bitmap, vec->local_id, 1);
+     __imsic_remote_sync(lpriv, vec->cpu);
+
+     raw_spin_unlock(&lpriv->lock);
+}

Really nice that you're using a timer for the vector affinity change, and got rid of the special/weird IMSIC/sync IPI. Can you really use a timer for mask/unmask? That makes the mask/unmask operation asynchronous!

Quoted 5 lines

That was what I was trying to get though with this comment:
https://lore.kernel.org/linux-riscv/[email protected]/

Also, using the smp_* IPI functions, you can pass arguments, so you don't need the dirty_bitmap tracking the changes.

The mask/unmask operations are called with irqs disabled so if CPU X does synchronous IPI to another CPU Y from mask/unmask operation then while CPU X is waiting for IPI to complete it cannot receive IPI from other CPUs which can lead to crashes and stalls.

In general, we should not do any block/busy-wait work in mask/unmask operation of an irqchip driver.

The AIA IMSIC spec allows setting ID pending bit using MSI write irrespective whether ID is enabled or not but the interrupt will be taken only after ID is enabled. In other words, there will be no loss of interrupt with delayed mask/unmask using async IPI or lazy timer.

Quoted 33 lines · Anup Patel

Quoted 27 lines

Quoted 11 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +11 -0

+void imsic_vector_mask(struct imsic_vector *vec)
+{
+     struct imsic_local_priv *lpriv;
+
+     lpriv = per_cpu_ptr(imsic->lpriv, vec->cpu);
+     if (WARN_ON(&lpriv->vectors[vec->local_id] != vec))
+             return;
+
+     /*
+      * This function is called through Linux irq subsystem with
+      * irqs disabled so no need to save/restore irq flags.

drivers/irqchip/irq-riscv-imsic-state.c line 258

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +10 -0

+      */
+
+     raw_spin_lock(&lpriv->lock);
+
+     vec->enable = false;
+     bitmap_set(lpriv->dirty_bitmap, vec->local_id, 1);
+     __imsic_remote_sync(lpriv, vec->cpu);
+
+     raw_spin_unlock(&lpriv->lock);
+}

Really nice that you're using a timer for the vector affinity change, and got rid of the special/weird IMSIC/sync IPI. Can you really use a timer for mask/unmask? That makes the mask/unmask operation asynchronous!

Quoted 5 lines

That was what I was trying to get though with this comment:
https://lore.kernel.org/linux-riscv/[email protected]/

Also, using the smp_* IPI functions, you can pass arguments, so you don't need the dirty_bitmap tracking the changes.

Quoted 7 lines

The mask/unmask operations are called with irqs disabled so if CPU X does synchronous IPI to another CPU Y from mask/unmask operation then while CPU X is waiting for IPI to complete it cannot receive IPI from other CPUs which can lead to crashes and stalls.

In general, we should not do any block/busy-wait work in mask/unmask operation of an irqchip driver.

Hmm, OK. Still, a bit odd that when the .irq_mask callback return, the masking is not actually completed.

1. CPU 0 tries to mask an interrupt tried to CPU 1.
2. The timer is queued on CPU 1.
3. The call irq_mask returns on CPU 0
4. ...the irq is masked at some future point, determined by the callback at CPU 1

Is that the expected outcome?

There are .irq_mask implementation that does seem to go at length (blocking) to perform the mask, e.g.: gic_mask_irq() which calls gic_{re,}dist_wait_for_rwp that have sleep/retry loops. The GIC3 ITS code has similar things going on.

I'm not saying you're wrong, I'm just trying to wrap my head around the masking semantics.

Quoted 5 lines

The AIA IMSIC spec allows setting ID pending bit using MSI write irrespective whether ID is enabled or not but the interrupt will be taken only after ID is enabled. In other words, there will be no loss of interrupt with delayed mask/unmask using async IPI or lazy timer.

No loss, but we might *get* an interrupt when we explicitly asked not to get any. Maybe that's ok?

Quoted 47 lines · Björn Töpel

Quoted 33 lines · Anup Patel

Quoted 27 lines

Quoted 11 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +11 -0

+void imsic_vector_mask(struct imsic_vector *vec)
+{
+     struct imsic_local_priv *lpriv;
+
+     lpriv = per_cpu_ptr(imsic->lpriv, vec->cpu);
+     if (WARN_ON(&lpriv->vectors[vec->local_id] != vec))
+             return;
+
+     /*
+      * This function is called through Linux irq subsystem with
+      * irqs disabled so no need to save/restore irq flags.

drivers/irqchip/irq-riscv-imsic-state.c line 258

Quoted 10 lines

drivers/irqchip/irq-riscv-imsic-state.c new file +10 -0

+      */
+
+     raw_spin_lock(&lpriv->lock);
+
+     vec->enable = false;
+     bitmap_set(lpriv->dirty_bitmap, vec->local_id, 1);
+     __imsic_remote_sync(lpriv, vec->cpu);
+
+     raw_spin_unlock(&lpriv->lock);
+}

Really nice that you're using a timer for the vector affinity change, and got rid of the special/weird IMSIC/sync IPI. Can you really use a timer for mask/unmask? That makes the mask/unmask operation asynchronous!

Quoted 5 lines

That was what I was trying to get though with this comment:
https://lore.kernel.org/linux-riscv/[email protected]/

Also, using the smp_* IPI functions, you can pass arguments, so you don't need the dirty_bitmap tracking the changes.

Quoted 7 lines

The mask/unmask operations are called with irqs disabled so if CPU X does synchronous IPI to another CPU Y from mask/unmask operation then while CPU X is waiting for IPI to complete it cannot receive IPI from other CPUs which can lead to crashes and stalls.

In general, we should not do any block/busy-wait work in mask/unmask operation of an irqchip driver.

Hmm, OK. Still, a bit odd that when the .irq_mask callback return, the masking is not actually completed.

Quoted 7 lines

1. CPU 0 tries to mask an interrupt tried to CPU 1.
2. The timer is queued on CPU 1.
3. The call irq_mask returns on CPU 0
4. ...the irq is masked at some future point, determined by the callback at CPU 1

Is that the expected outcome?

Yes, that's right.

Quoted 5 lines

There are .irq_mask implementation that does seem to go at length (blocking) to perform the mask, e.g.: gic_mask_irq() which calls gic_{re,}dist_wait_for_rwp that have sleep/retry loops. The GIC3 ITS code has similar things going on.

The gic_{re,}dist_wait_for_rwp() polls on a HW register for completion which will certainly complete in a predictable time whereas waiting for IPI to be executed by another CPU is not predictable and fragile.

Quoted 10 lines

I'm not saying you're wrong, I'm just trying to wrap my head around the masking semantics.

Quoted 5 lines

The AIA IMSIC spec allows setting ID pending bit using MSI write irrespective whether ID is enabled or not but the interrupt will be taken only after ID is enabled. In other words, there will be no loss of interrupt with delayed mask/unmask using async IPI or lazy timer.