Linux RISC-V AIA Support

The RISC-V AIA specification improves handling per-HART local interrupts in a backward compatible manner. This patch adds defines for new RISC-V AIA CSRs.

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

---
 arch/riscv/include/asm/csr.h | 92 ++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

arch/riscv/include/asm/csr.h +12 -0

index 0e571f6483d9..4e1356bad7b2 100644
@@ -73,7 +73,10 @@
 #define IRQ_S_EXT		9
 #define IRQ_VS_EXT		10
 #define IRQ_M_EXT		11
+#define IRQ_S_GEXT		12
 #define IRQ_PMU_OVF		13
+#define IRQ_LOCAL_MAX		(IRQ_PMU_OVF + 1)
+#define IRQ_LOCAL_MASK		((_AC(1, UL) << IRQ_LOCAL_MAX) - 1)
 
 /* Exception causes */
 #define EXC_INST_MISALIGNED	0

@@ -156,6 +159,26 @@
 				 (_AC(1, UL) << IRQ_S_TIMER) | \
 				 (_AC(1, UL) << IRQ_S_EXT))
 
+/* AIA CSR bits */
+#define TOPI_IID_SHIFT		16
+#define TOPI_IID_MASK		0xfff
+#define TOPI_IPRIO_MASK		0xff
+#define TOPI_IPRIO_BITS		8
+
+#define TOPEI_ID_SHIFT		16
+#define TOPEI_ID_MASK		0x7ff
+#define TOPEI_PRIO_MASK		0x7ff

Hey Anup!

Quoted 45 lines · Anup Patel

The RISC-V AIA specification improves handling per-HART local interrupts in a backward compatible manner. This patch adds defines for new RISC-V AIA CSRs.

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

---
 arch/riscv/include/asm/csr.h | 92 ++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

arch/riscv/include/asm/csr.h +20 -0

index 0e571f6483d9..4e1356bad7b2 100644
@@ -73,7 +73,10 @@
 #define IRQ_S_EXT            9
 #define IRQ_VS_EXT           10
 #define IRQ_M_EXT            11
+#define IRQ_S_GEXT           12
 #define IRQ_PMU_OVF          13
+#define IRQ_LOCAL_MAX                (IRQ_PMU_OVF + 1)
+#define IRQ_LOCAL_MASK               ((_AC(1, UL) << IRQ_LOCAL_MAX) - 1)
 
 /* Exception causes */
 #define EXC_INST_MISALIGNED  0

@@ -156,6 +159,26 @@
                               (_AC(1, UL) << IRQ_S_TIMER) | \
                               (_AC(1, UL) << IRQ_S_EXT))
 
+/* AIA CSR bits */
+#define TOPI_IID_SHIFT               16
+#define TOPI_IID_MASK                0xfff
+#define TOPI_IPRIO_MASK              0xff
+#define TOPI_IPRIO_BITS              8
+
+#define TOPEI_ID_SHIFT               16
+#define TOPEI_ID_MASK                0x7ff
+#define TOPEI_PRIO_MASK              0x7ff
+
+#define ISELECT_IPRIO0               0x30
+#define ISELECT_IPRIO15              0x3f
+#define ISELECT_MASK         0x1ff
+
+#define HVICTL_VTI           0x40000000
+#define HVICTL_IID           0x0fff0000
+#define HVICTL_IID_SHIFT     16

arch/riscv/include/asm/csr.h line 180

Quoted 2 lines

arch/riscv/include/asm/csr.h +2 -0

+#define HVICTL_IPRIOM                0x00000100
+#define HVICTL_IPRIO         0x000000ff

Why not name these as masks, like you did for the other masks? Also, the mask/shift defines appear inconsistent. TOPI_IID_MASK is intended to be used post-shift AFAICT, but HVICTL_IID_SHIFT is intended to be used *pre*-shift.
Some consistency in naming and function would be great.

arch/riscv/include/asm/csr.h line 372

Quoted 4 lines

arch/riscv/include/asm/csr.h +2 -0

+/* Machine-Level High-Half CSRs (AIA) */
+#define CSR_MIDELEGH         0x313

I feel like I could find Midelegh in an Irish dictionary lol Anyways, I went through the CSRs and they do all seem correct.

Thanks,
Conor.

Quoted 5 lines · Conor Dooley

arch/riscv/include/asm/csr.h line 164

Quoted 3 lines · Anup Patel

arch/riscv/include/asm/csr.h +3 -0

+/* AIA CSR bits */
+#define TOPI_IID_SHIFT               16
+#define TOPI_IID_MASK                0xfff

Quoted 14 lines

Quoted 14 lines

arch/riscv/include/asm/csr.h +14 -0

+#define TOPI_IPRIO_MASK              0xff
+#define TOPI_IPRIO_BITS              8
+
+#define TOPEI_ID_SHIFT               16
+#define TOPEI_ID_MASK                0x7ff
+#define TOPEI_PRIO_MASK              0x7ff
+
+#define ISELECT_IPRIO0               0x30
+#define ISELECT_IPRIO15              0x3f
+#define ISELECT_MASK         0x1ff
+
+#define HVICTL_VTI           0x40000000
+#define HVICTL_IID           0x0fff0000
+#define HVICTL_IID_SHIFT     16

Quoted 8 lines

arch/riscv/include/asm/csr.h line 180

Quoted 2 lines

arch/riscv/include/asm/csr.h +2 -0

+#define HVICTL_IPRIOM                0x00000100
+#define HVICTL_IPRIO         0x000000ff

Why not name these as masks, like you did for the other masks? Also, the mask/shift defines appear inconsistent. TOPI_IID_MASK is intended to be used post-shift AFAICT, but HVICTL_IID_SHIFT is intended to be used *pre*-shift.
Some consistency in naming and function would be great.

The following convention is being followed in asm/csr.h for defining MASK of any XYZ field in ABC CSR:
1. ABC_XYZ : This name is used for MASK which is intended to be used before SHIFT
2. ABC_XYZ_MASK: This name is used for MASK which is intended to be used after SHIFT

The existing defines for [M|S]STATUS, HSTATUS, SATP, and xENVCFG follows the above convention. The only outlier is HGATPx_VMID_MASK define which I will fix in my next KVM RISC-V series.

I don't see how any of the AIA CSR defines are violating the above convention.

Hey Anup,

I thought I had already replied here but clearly not, sorry!

Quoted 2 lines · Anup Patel

Quoted 1 line · Conor Dooley

On Tue, Jan 03, 2023 at 07:44:01PM +0530, Anup Patel wrote:

Quoted 3 lines

Quoted 3 lines

arch/riscv/include/asm/csr.h line 164

Quoted 3 lines

arch/riscv/include/asm/csr.h +3 -0

+/* AIA CSR bits */
+#define TOPI_IID_SHIFT               16
+#define TOPI_IID_MASK                0xfff

While I think of it, it'd be worth noting that these are generic across all of topi, mtopi etc. Initially I thought that this mask was wrong as the topi section says:
bits 25:16 Interrupt identity (source number)
bits 7:0 Interrupt priority

Quoted 24 lines

Quoted 15 lines

Quoted 15 lines

arch/riscv/include/asm/csr.h +14 -0

+#define TOPI_IPRIO_MASK              0xff
+#define TOPI_IPRIO_BITS              8
+
+#define TOPEI_ID_SHIFT               16
+#define TOPEI_ID_MASK                0x7ff
+#define TOPEI_PRIO_MASK              0x7ff
+
+#define ISELECT_IPRIO0               0x30
+#define ISELECT_IPRIO15              0x3f
+#define ISELECT_MASK         0x1ff
+
+#define HVICTL_VTI           0x40000000
+#define HVICTL_IID           0x0fff0000
+#define HVICTL_IID_SHIFT     16

Quoted 8 lines

arch/riscv/include/asm/csr.h line 180

Quoted 2 lines

arch/riscv/include/asm/csr.h +2 -0

+#define HVICTL_IPRIOM                0x00000100
+#define HVICTL_IPRIO         0x000000ff

Why not name these as masks, like you did for the other masks? Also, the mask/shift defines appear inconsistent. TOPI_IID_MASK is intended to be used post-shift AFAICT, but HVICTL_IID_SHIFT is intended to be used *pre*-shift.
Some consistency in naming and function would be great.

Quoted 6 lines

The following convention is being followed in asm/csr.h for defining MASK of any XYZ field in ABC CSR:
1. ABC_XYZ : This name is used for MASK which is intended to be used before SHIFT
2. ABC_XYZ_MASK: This name is used for MASK which is intended to be used after SHIFT

Which makes sense in theory.

Quoted 3 lines

The existing defines for [M|S]STATUS, HSTATUS, SATP, and xENVCFG follows the above convention. The only outlier is HGATPx_VMID_MASK define which I will fix in my next KVM RISC-V series.

Yup, it is liable to end up like that.

Quoted 2 lines

I don't see how any of the AIA CSR defines are violating the above convention.

What I was advocating for was picking one style and sticking to it. These copy-paste from docs things are tedious and error prone to review, and I don't think having multiple styles is helpful.

Tedious as it was, I did check all the numbers though, so in that respect:

Reviewed-by Conor Dooley <[email protected]>

Quoted 9 lines · Conor Dooley

Quoted 3 lines

Quoted 3 lines

arch/riscv/include/asm/csr.h line 164

Quoted 3 lines

arch/riscv/include/asm/csr.h +3 -0

+/* AIA CSR bits */
+#define TOPI_IID_SHIFT               16
+#define TOPI_IID_MASK                0xfff

While I think of it, it'd be worth noting that these are generic across all of topi, mtopi etc. Initially I thought that this mask was wrong as the topi section says:
bits 25:16 Interrupt identity (source number)
bits 7:0 Interrupt priority

These defines are for the AIA CSRs and not AIA APLIC IDC registers.

As per the latest frozen spec, the mtopi/stopi/vstopi has following bits:

    bits: 27:16 IID
    bits: 7:0 IPRIO

Quoted 3 lines

What I was advocating for was picking one style and sticking to it. These copy-paste from docs things are tedious and error prone to review, and I don't think having multiple styles is helpful.

On the other hand, I think we should let developers choose a style which is better suited for a particular register field instead enforcing it here. The best we can do is follow a naming convention for defines.

Quoted 3 lines

Tedious as it was, I did check all the numbers though, so in that respect:

Reviewed-by Conor Dooley <[email protected]>

BTW, this patch is shared with KVM AIA CSR series so most likely I will take this patch through that series.

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

This patch adds support for RISC-V AIA in the RISC-V intc driver.

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

---
 drivers/irqchip/irq-riscv-intc.c | 37 ++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

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

index f229e3e66387..880d1639aadc 100644
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/smp.h>
+#include <asm/hwcap.h>
 
 static struct irq_domain *intc_domain;
 

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

+{
+	unsigned long topi;
+
+	while ((topi = csr_read(CSR_TOPI)))
+		generic_handle_domain_irq(intc_domain,
+					  topi >> TOPI_IID_SHIFT);
+}
+
 /*
  * On RISC-V systems local interrupts are masked or unmasked by writing
  * the SIE (Supervisor Interrupt Enable) CSR.  As CSRs can only be written

drivers/irqchip/irq-riscv-intc.c +8 -2

+		csr_clear(CSR_IE, BIT(d->hwirq));
+	else
+		csr_clear(CSR_IEH, BIT(d->hwirq - BITS_PER_LONG));
 }
 
 static void riscv_intc_irq_unmask(struct irq_data *d)
 {
-	csr_set(CSR_IE, BIT(d->hwirq));
+	if (d->hwirq < BITS_PER_LONG)
+		csr_set(CSR_IE, BIT(d->hwirq));
+	else
+		csr_set(CSR_IEH, BIT(d->hwirq - BITS_PER_LONG));
 }
 
 static void riscv_intc_irq_eoi(struct irq_data *d)

@@ -115,7 +131,7 @@ static struct fwnode_handle *riscv_intc_hwnode(void)
 static int __init riscv_intc_init(struct device_node *node,
 				  struct device_node *parent)
 {
-	int rc;
+	int rc, nr_irqs;
 	unsigned long hartid;
 
 	rc = riscv_of_parent_hartid(node, &hartid);

We add DT bindings document for the RISC-V incoming MSI controller (IMSIC) defined by the RISC-V advanced interrupt architecture (AIA) specification.

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

(quoted diff) +30 -0

+  The IMSIC is a per-CPU (or per-HART) device with separate interrupt file
+  for each privilege level (machine or supervisor).
 
+  The device tree of a RISC-V platform will have one IMSIC device tree node
+  for each privilege level (machine or supervisor) which collectively describe
+  IMSIC interrupt files at that privilege level across CPUs (or HARTs).
 
+examples:
+  - |
+    // Example 1 (Machine-level IMSIC files with just one group):
+
+    imsic_mlevel: interrupt-controller@24000000 {
+      compatible = "riscv,qemu-imsics", "riscv,imsics";
+      interrupts-extended = <&cpu1_intc 11>,
+                            <&cpu2_intc 11>,
+                            <&cpu3_intc 11>,
+                            <&cpu4_intc 11>;
+      reg = <0x28000000 0x4000>;
+      interrupt-controller;
+      #interrupt-cells = <0>;
+      msi-controller;
+      riscv,num-ids = <127>;
+    };
+
+  - |
+    // Example 2 (Supervisor-level IMSIC files with two groups):
+
+    imsic_slevel: interrupt-controller@28000000 {
+      compatible = "riscv,qemu-imsics", "riscv,imsics";
+      interrupts-extended = <&cpu1_intc 9>,
+                            <&cpu2_intc 9>,
+                            <&cpu3_intc 9>,

Hey Anup,

Quoted 44 lines · Anup Patel

We add DT bindings document for the RISC-V incoming MSI controller (IMSIC) defined by the RISC-V advanced interrupt architecture (AIA) specification.

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

(quoted diff) +37 -0

+  The IMSIC is a per-CPU (or per-HART) device with separate interrupt file
+  for each privilege level (machine or supervisor).
 
+  The device tree of a RISC-V platform will have one IMSIC device tree node
+  for each privilege level (machine or supervisor) which collectively describe
+  IMSIC interrupt files at that privilege level across CPUs (or HARTs).
 
+examples:
+  - |
+    // Example 1 (Machine-level IMSIC files with just one group):
+
+    imsic_mlevel: interrupt-controller@24000000 {
+      compatible = "riscv,qemu-imsics", "riscv,imsics";
+      interrupts-extended = <&cpu1_intc 11>,
+                            <&cpu2_intc 11>,
+                            <&cpu3_intc 11>,
+                            <&cpu4_intc 11>;
+      reg = <0x28000000 0x4000>;
+      interrupt-controller;
+      #interrupt-cells = <0>;
+      msi-controller;
+      riscv,num-ids = <127>;
+    };
+
+  - |
+    // Example 2 (Supervisor-level IMSIC files with two groups):
+
+    imsic_slevel: interrupt-controller@28000000 {
+      compatible = "riscv,qemu-imsics", "riscv,imsics";
+      interrupts-extended = <&cpu1_intc 9>,
+                            <&cpu2_intc 9>,
+                            <&cpu3_intc 9>,
+                            <&cpu4_intc 9>;
+      reg = <0x28000000 0x2000>, /* Group0 IMSICs */
+            <0x29000000 0x2000>; /* Group1 IMSICs */
+      interrupt-controller;
+      #interrupt-cells = <0>;
+      msi-controller;
+      riscv,num-ids = <127>;

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml line 167

Quoted 3 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +3 -0

+      riscv,group-index-bits = <1>;
+      riscv,group-index-shift = <24>;
+    };

How is, say linux, meant to know which of the per-level imsic DT nodes applies to it?
I had a quick look in the driver, but could see no mechanism for it. Apologies if I missed something obvious!

We add DT bindings document for the RISC-V incoming MSI controller (IMSIC) defined by the RISC-V advanced interrupt architecture (AIA) specification.

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

---
 .../interrupt-controller/riscv,imsics.yaml    | 168 ++++++++++++++++++
 1 file changed, 168 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +40 -0

new file mode 100644
index 000000000000..b9db03b6e95f
@@ -0,0 +1,168 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V Incoming MSI Controller (IMSIC)
+
+maintainers:
+  - Anup Patel <[email protected]>
+
+description: |
+  The RISC-V advanced interrupt architecture (AIA) defines a per-CPU incoming
+  MSI controller (IMSIC) for handling MSIs in a RISC-V platform. The RISC-V
+  AIA specification can be found at https://github.com/riscv/riscv-aia.
+
+  The IMSIC is a per-CPU (or per-HART) device with separate interrupt file
+  for each privilege level (machine or supervisor). The configuration of
+  a IMSIC interrupt file is done using AIA CSRs and it also has a 4KB MMIO
+  space to receive MSIs from devices. Each IMSIC interrupt file supports a
+  fixed number of interrupt identities (to distinguish MSIs from devices)
+  which is same for given privilege level across CPUs (or HARTs).
+
+  The device tree of a RISC-V platform will have one IMSIC device tree node
+  for each privilege level (machine or supervisor) which collectively describe
+  IMSIC interrupt files at that privilege level across CPUs (or HARTs).
+
+  The arrangement of IMSIC interrupt files in MMIO space of a RISC-V platform
+  follows a particular scheme defined by the RISC-V AIA specification. A IMSIC
+  group is a set of IMSIC interrupt files co-located in MMIO space and we can
+  have multiple IMSIC groups (i.e. clusters, sockets, chiplets, etc) in a
+  RISC-V platform. The MSI target address of a IMSIC interrupt file at given
+  privilege level (machine or supervisor) encodes group index, HART index,
+  and guest index (shown below).
+
+  XLEN-1           >=24                                 12    0
+  |                  |                                  |     |
+  -------------------------------------------------------------
+  |xxxxxx|Group Index|xxxxxxxxxxx|HART Index|Guest Index|  0  |
+  -------------------------------------------------------------

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +37 -0

+      - const: riscv,imsics
+
+  reg:
+    minItems: 1
+    maxItems: 16384
+    description:
+      Base address of each IMSIC group.
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 0
+
+  msi-controller: true
+
+  interrupts-extended:
+    minItems: 1
+    maxItems: 16384
+    description:
+      This property represents the set of CPUs (or HARTs) for which given
+      device tree node describes the IMSIC interrupt files. Each node pointed
+      to should be a riscv,cpu-intc node, which has a riscv node (i.e. RISC-V
+      HART) as parent.
+
+  riscv,num-ids:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 63
+    maximum: 2047
+    description:
+      Number of interrupt identities supported by IMSIC interrupt file.
+
+  riscv,num-guest-ids:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 63
+    maximum: 2047
+    description:
+      Number of interrupt identities are supported by IMSIC guest interrupt

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +13 -0

+
+  riscv,group-index-bits:
+    minimum: 0
+    maximum: 7
+    default: 0
+    description:
+      Number of group index bits in the MSI target address. When not
+      specified it is assumed to be 0.
+
+  riscv,group-index-shift:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 55

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +31 -0

+      compatible = "riscv,qemu-imsics", "riscv,imsics";
+      interrupts-extended = <&cpu1_intc 11>,
+                            <&cpu2_intc 11>,
+                            <&cpu3_intc 11>,
+                            <&cpu4_intc 11>;
+      reg = <0x28000000 0x4000>;
+      interrupt-controller;
+      #interrupt-cells = <0>;
+      msi-controller;
+      riscv,num-ids = <127>;
+    };
+
+  - |
+    // Example 2 (Supervisor-level IMSIC files with two groups):
+
+    imsic_slevel: interrupt-controller@28000000 {
+      compatible = "riscv,qemu-imsics", "riscv,imsics";
+      interrupts-extended = <&cpu1_intc 9>,
+                            <&cpu2_intc 9>,
+                            <&cpu3_intc 9>,
+                            <&cpu4_intc 9>;
+      reg = <0x28000000 0x2000>, /* Group0 IMSICs */
+            <0x29000000 0x2000>; /* Group1 IMSICs */
+      interrupt-controller;
+      #interrupt-cells = <0>;
+      msi-controller;
+      riscv,num-ids = <127>;
+      riscv,group-index-bits = <1>;
+      riscv,group-index-shift = <24>;
+    };
+...

Signature · 1 line

2.34.1

Quoted 60 lines · Anup Patel

We add DT bindings document for the RISC-V incoming MSI controller (IMSIC) defined by the RISC-V advanced interrupt architecture (AIA) specification.

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

---
 .../interrupt-controller/riscv,imsics.yaml    | 168 ++++++++++++++++++
 1 file changed, 168 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +44 -0

new file mode 100644
index 000000000000..b9db03b6e95f
@@ -0,0 +1,168 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V Incoming MSI Controller (IMSIC)
+
+maintainers:
+  - Anup Patel <[email protected]>
+
+description: |
+  The RISC-V advanced interrupt architecture (AIA) defines a per-CPU incoming
+  MSI controller (IMSIC) for handling MSIs in a RISC-V platform. The RISC-V
+  AIA specification can be found at https://github.com/riscv/riscv-aia.
+
+  The IMSIC is a per-CPU (or per-HART) device with separate interrupt file
+  for each privilege level (machine or supervisor). The configuration of
+  a IMSIC interrupt file is done using AIA CSRs and it also has a 4KB MMIO
+  space to receive MSIs from devices. Each IMSIC interrupt file supports a
+  fixed number of interrupt identities (to distinguish MSIs from devices)
+  which is same for given privilege level across CPUs (or HARTs).
+
+  The device tree of a RISC-V platform will have one IMSIC device tree node
+  for each privilege level (machine or supervisor) which collectively describe
+  IMSIC interrupt files at that privilege level across CPUs (or HARTs).
+
+  The arrangement of IMSIC interrupt files in MMIO space of a RISC-V platform
+  follows a particular scheme defined by the RISC-V AIA specification. A IMSIC
+  group is a set of IMSIC interrupt files co-located in MMIO space and we can
+  have multiple IMSIC groups (i.e. clusters, sockets, chiplets, etc) in a
+  RISC-V platform. The MSI target address of a IMSIC interrupt file at given
+  privilege level (machine or supervisor) encodes group index, HART index,
+  and guest index (shown below).
+
+  XLEN-1           >=24                                 12    0
+  |                  |                                  |     |
+  -------------------------------------------------------------
+  |xxxxxx|Group Index|xxxxxxxxxxx|HART Index|Guest Index|  0  |
+  -------------------------------------------------------------
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+  - $ref: /schemas/interrupt-controller/msi-controller.yaml#

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml line 50

Quoted 6 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +6 -0

+
+properties:
+  compatible:
+    items:
+      - enum:
+          - riscv,qemu-imsics

The implmentation/vendor is qemu, so: qemu,imsics (or qemu,riscv-imsics?)

Quoted 42 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +41 -0

+      - const: riscv,imsics
+
+  reg:
+    minItems: 1
+    maxItems: 16384
+    description:
+      Base address of each IMSIC group.
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 0
+
+  msi-controller: true
+
+  interrupts-extended:
+    minItems: 1
+    maxItems: 16384
+    description:
+      This property represents the set of CPUs (or HARTs) for which given
+      device tree node describes the IMSIC interrupt files. Each node pointed
+      to should be a riscv,cpu-intc node, which has a riscv node (i.e. RISC-V
+      HART) as parent.
+
+  riscv,num-ids:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 63
+    maximum: 2047
+    description:
+      Number of interrupt identities supported by IMSIC interrupt file.
+
+  riscv,num-guest-ids:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 63
+    maximum: 2047
+    description:
+      Number of interrupt identities are supported by IMSIC guest interrupt
+      file. When not specified it is assumed to be same as specified by the
+      riscv,num-ids property.
+
+  riscv,guest-index-bits:

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml line 97

Quoted 6 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +6 -0

+    minimum: 0
+    maximum: 7
+    default: 0
+    description:
+      Number of guest index bits in the MSI target address. When not
+      specified it is assumed to be 0.

No need to repeat what 'default: 0' defines.

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml line 104

Quoted 8 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +7 -0

+
+  riscv,hart-index-bits:
+    minimum: 0
+    maximum: 15
+    description:
+      Number of HART index bits in the MSI target address. When not
+      specified it is estimated based on the interrupts-extended property.

If guessing works, why do you need the property? Perhaps s/estimated/calculated/?

Quoted 19 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +18 -0

+
+  riscv,group-index-bits:
+    minimum: 0
+    maximum: 7
+    default: 0
+    description:
+      Number of group index bits in the MSI target address. When not
+      specified it is assumed to be 0.
+
+  riscv,group-index-shift:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 55
+    default: 24
+    description:
+      The least significant bit position of the group index bits in the
+      MSI target address. When not specified it is assumed to be 24.
+

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml line 127

Quoted 5 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +5 -0

+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - msi-controller

#msi-cells should be defined (as 0) and required. Best to be explicit #and not rely on the default.

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml line 137

Quoted 11 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +10 -0

+  - interrupts-extended
+  - riscv,num-ids
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    // Example 1 (Machine-level IMSIC files with just one group):
+
+    imsic_mlevel: interrupt-controller@24000000 {

Drop unused labels.

Quoted 27 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +26 -0

+      compatible = "riscv,qemu-imsics", "riscv,imsics";
+      interrupts-extended = <&cpu1_intc 11>,
+                            <&cpu2_intc 11>,
+                            <&cpu3_intc 11>,
+                            <&cpu4_intc 11>;
+      reg = <0x28000000 0x4000>;
+      interrupt-controller;
+      #interrupt-cells = <0>;
+      msi-controller;
+      riscv,num-ids = <127>;
+    };
+
+  - |
+    // Example 2 (Supervisor-level IMSIC files with two groups):
+
+    imsic_slevel: interrupt-controller@28000000 {
+      compatible = "riscv,qemu-imsics", "riscv,imsics";
+      interrupts-extended = <&cpu1_intc 9>,
+                            <&cpu2_intc 9>,
+                            <&cpu3_intc 9>,
+                            <&cpu4_intc 9>;
+      reg = <0x28000000 0x2000>, /* Group0 IMSICs */
+            <0x29000000 0x2000>; /* Group1 IMSICs */
+      interrupt-controller;
+      #interrupt-cells = <0>;
+      msi-controller;

Quoted 8 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +5 -1

+      riscv,num-ids = <127>;
+      riscv,group-index-bits = <1>;
+      riscv,group-index-shift = <24>;
+    };
+...
--

2.34.1

We add DT bindings document for the RISC-V incoming MSI controller (IMSIC) defined by the RISC-V advanced interrupt architecture (AIA) specification.

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

---
 .../interrupt-controller/riscv,imsics.yaml    | 168 ++++++++++++++++++
 1 file changed, 168 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +0 -0

new file mode 100644
index 000000000000..b9db03b6e95f
<snip>

<snip>
+...

Quoted 16 lines · Anup Patel

We add DT bindings document for the RISC-V incoming MSI controller (IMSIC) defined by the RISC-V advanced interrupt architecture (AIA) specification.

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

---
 .../interrupt-controller/riscv,imsics.yaml    | 168 ++++++++++++++++++
 1 file changed, 168 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +0 -0

new file mode 100644
index 000000000000..b9db03b6e95f
<snip>

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml line 74

Quoted 10 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +10 -0

+
+  interrupts-extended:
+    minItems: 1
+    maxItems: 16384
+    description:
+      This property represents the set of CPUs (or HARTs) for which given
+      device tree node describes the IMSIC interrupt files. Each node pointed
+      to should be a riscv,cpu-intc node, which has a riscv node (i.e. RISC-V
+      HART) as parent.
+

This property doesn't seem to describe guest external interrupts. Should we add a reference to e.g. <&cpuN_intc 12> to indicate that IMSIC can send a 'Supervisor guest external interrupt'? Or just an idea, maybe we can add an additional interrupt controller to the CPU nodes to handle
SGEI: (Various properties omitted)

cpu0: cpu@N {
        compatible = "riscv";

        cpu0_intc: interrupt-controller {
                compatible = "riscv,cpu-intc";

                cpu0_gei: interrupt-controller {
                        /* intc for hart-local hgeie/hgeip */
                        compatible = "riscv,..."; /* Something here */
                        interrupt-parent = <&cpu0_intc>;
                        interrupts = <12>; /* SGEI */
                        interrupt-controller;
                        #interrupt-cells = <1>;
                }
        }
}

interrupt-controller@... {
        compatible = "riscv,imsics";
        interrupts-extended = <&cpu0_intc 11>, <&cpu0_gei 1>, <&cpu0_gei 2> /* ... */;
}

I feel that this would be more appropriate, since the guest external interrupts are defined in the privileged architecture specification and are not specific to AIA. Though please do suggest more appropriate ways to formulate it.

The RISC-V advanced interrupt architecture (AIA) specification defines a new MSI controller for managing MSIs on a RISC-V platform. This new MSI controller is referred to as incoming message signaled interrupt controller (IMSIC) which manages MSI on per-HART (or per-CPU) basis. (For more details refer https://github.com/riscv/riscv-aia)

This patch adds an irqchip driver for RISC-V IMSIC found on RISC-V platforms.

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

---
 drivers/irqchip/Kconfig             |   14 +-
 drivers/irqchip/Makefile            |    1 +
 drivers/irqchip/irq-riscv-imsic.c   | 1174 +++++++++++++++++++++++++++
 include/linux/irqchip/riscv-imsic.h |   92 +++
 4 files changed, 1280 insertions(+), 1 deletion(-)
 create mode 100644 drivers/irqchip/irq-riscv-imsic.c
 create mode 100644 include/linux/irqchip/riscv-imsic.h

drivers/irqchip/Kconfig +2 -1

index 9e65345ca3f6..a1315189a595 100644
@@ -29,7 +29,6 @@ config ARM_GIC_V2M
 
 config GIC_NON_BANKED
 	bool
-
 config ARM_GIC_V3
 	bool
 	select IRQ_DOMAIN_HIERARCHY

@@ -548,6 +547,19 @@ config SIFIVE_PLIC
 	select IRQ_DOMAIN_HIERARCHY
 	select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP
 
+config RISCV_IMSIC
+	bool

drivers/irqchip/Kconfig +1 -0

+
 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 87b49a10962c..22c723cc6ec8 100644
@@ -96,6 +96,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.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.c new file +112 -0

new file mode 100644
index 000000000000..4c16b66738d6
@@ -0,0 +1,1174 @@
+// 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/bitmap.h>
+#include <linux/cpu.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iommu.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqchip/riscv-imsic.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/msi.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/smp.h>
+#include <asm/hwcap.h>
+
+#define IMSIC_DISABLE_EIDELIVERY	0
+#define IMSIC_ENABLE_EIDELIVERY		1
+#define IMSIC_DISABLE_EITHRESHOLD	1
+#define IMSIC_ENABLE_EITHRESHOLD	0
+
+#define imsic_csr_write(__c, __v)	\
+do {					\
+	csr_write(CSR_ISELECT, __c);	\
+	csr_write(CSR_IREG, __v);	\
+} while (0)
+
+#define imsic_csr_read(__c)		\
+({					\
+	unsigned long __v;		\
+	csr_write(CSR_ISELECT, __c);	\
+	__v = csr_read(CSR_IREG);	\
+	__v;				\
+})
+
+#define imsic_csr_set(__c, __v)		\
+do {					\
+	csr_write(CSR_ISELECT, __c);	\
+	csr_set(CSR_IREG, __v);		\
+} while (0)
+
+#define imsic_csr_clear(__c, __v)	\
+do {					\
+	csr_write(CSR_ISELECT, __c);	\
+	csr_clear(CSR_IREG, __v);	\
+} while (0)
+
+struct imsic_mmio {
+	phys_addr_t pa;
+	void __iomem *va;
+	unsigned long size;
+};
+
+struct imsic_priv {
+	/* Global configuration common for all HARTs */
+	struct imsic_global_config global;
+
+	/* MMIO regions */
+	u32 num_mmios;
+	struct imsic_mmio *mmios;
+
+	/* Global state of interrupt identities */
+	raw_spinlock_t ids_lock;
+	unsigned long *ids_used_bimap;
+	unsigned long *ids_enabled_bimap;
+	unsigned int *ids_target_cpu;
+
+	/* Mask for connected CPUs */
+	struct cpumask lmask;
+
+	/* IPI interrupt identity */
+	u32 ipi_id;
+	u32 ipi_lsync_id;
+
+	/* IRQ domains */
+	struct irq_domain *base_domain;
+	struct irq_domain *pci_domain;
+	struct irq_domain *plat_domain;
+};
+
+struct imsic_handler {
+	/* Local configuration for given HART */
+	struct imsic_local_config local;
+
+	/* Pointer to private context */
+	struct imsic_priv *priv;
+};
+
+static bool imsic_init_done;
+
+static int imsic_parent_irq;
+static DEFINE_PER_CPU(struct imsic_handler, imsic_handlers);
+
+const struct imsic_global_config *imsic_get_global_config(void)
+{
+	struct imsic_handler *handler = this_cpu_ptr(&imsic_handlers);
+
+	if (!handler || !handler->priv)
+		return NULL;
+

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

+
+static int imsic_cpu_page_phys(unsigned int cpu,
+			       unsigned int guest_index,
+			       phys_addr_t *out_msi_pa)
+{
+	struct imsic_handler *handler = per_cpu_ptr(&imsic_handlers, cpu);
+	struct imsic_global_config *global;
+	struct imsic_local_config *local;
+
+	if (!handler || !handler->priv)
+		return -ENODEV;
+	local = &handler->local;
+	global = &handler->priv->global;
+
+	if (BIT(global->guest_index_bits) <= guest_index)
+		return -EINVAL;
+
+	if (out_msi_pa)
+		*out_msi_pa = local->msi_pa +
+			      (guest_index * IMSIC_MMIO_PAGE_SZ);
+
+	return 0;
+}
+
+static int imsic_get_cpu(struct imsic_priv *priv,
+			 const struct cpumask *mask_val, bool force,
+			 unsigned int *out_target_cpu)
+{
+	struct cpumask amask;
+	unsigned int cpu;
+
+	cpumask_and(&amask, &priv->lmask, mask_val);
+
+	if (force)
+		cpu = cpumask_first(&amask);
+	else
+		cpu = cpumask_any_and(&amask, cpu_online_mask);
+
+	if (cpu >= nr_cpu_ids)
+		return -EINVAL;
+
+	if (out_target_cpu)
+		*out_target_cpu = cpu;
+
+	return 0;
+}
+
+static int imsic_get_cpu_msi_msg(unsigned int cpu, unsigned int id,
+				 struct msi_msg *msg)
+{
+	phys_addr_t msi_addr;
+	int err;
+
+	err = imsic_cpu_page_phys(cpu, 0, &msi_addr);
+	if (err)
+		return err;
+
+	msg->address_hi = upper_32_bits(msi_addr);
+	msg->address_lo = lower_32_bits(msi_addr);
+	msg->data = id;
+
+	return err;
+}
+
+static void imsic_id_set_target(struct imsic_priv *priv,
+				 unsigned int id, unsigned int target_cpu)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&priv->ids_lock, flags);
+	priv->ids_target_cpu[id] = target_cpu;
+	raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+}
+
+static unsigned int imsic_id_get_target(struct imsic_priv *priv,
+					unsigned int id)
+{
+	unsigned int ret;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&priv->ids_lock, flags);
+	ret = priv->ids_target_cpu[id];
+	raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+
+	return ret;
+}
+
+static void __imsic_eix_update(unsigned long base_id,
+			       unsigned long num_id, bool pend, bool val)
+{
+	unsigned long i, isel, ireg, flags;
+	unsigned long id = base_id, last_id = base_id + num_id;
+
+	while (id < last_id) {
+		isel = id / BITS_PER_LONG;
+		isel *= BITS_PER_LONG / IMSIC_EIPx_BITS;
+		isel += (pend) ? IMSIC_EIP0 : IMSIC_EIE0;
+
+		ireg = 0;
+		for (i = id & (__riscv_xlen - 1);
+		     (id < last_id) && (i < __riscv_xlen); i++) {
+			ireg |= BIT(i);
+			id++;
+		}
+
+		/*
+		 * The IMSIC EIEx and EIPx registers are indirectly

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

+	}
+}
+
+#define __imsic_id_enable(__id)		\
+	__imsic_eix_update((__id), 1, false, true)
+#define __imsic_id_disable(__id)	\

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

+	}
+}
+#else
+#define __imsic_id_smp_sync(__priv)
+#endif
+
+static void imsic_id_enable(struct imsic_priv *priv, unsigned int id)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&priv->ids_lock, flags);
+	bitmap_set(priv->ids_enabled_bimap, id, 1);
+	__imsic_id_enable(id);
+	raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+
+	__imsic_id_smp_sync(priv);
+}
+
+static void imsic_id_disable(struct imsic_priv *priv, unsigned int id)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&priv->ids_lock, flags);
+	bitmap_clear(priv->ids_enabled_bimap, id, 1);
+	__imsic_id_disable(id);
+	raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+
+	__imsic_id_smp_sync(priv);
+}
+
+static void imsic_ids_local_sync(struct imsic_priv *priv)
+{
+	int i;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&priv->ids_lock, flags);
+	for (i = 1; i <= priv->global.nr_ids; i++) {
+		if (priv->ipi_id == i || priv->ipi_lsync_id == i)
+			continue;
+
+		if (test_bit(i, priv->ids_enabled_bimap))
+			__imsic_id_enable(i);
+		else
+			__imsic_id_disable(i);
+	}
+	raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+}
+
+static void imsic_ids_local_delivery(struct imsic_priv *priv, bool enable)
+{
+	if (enable) {
+		imsic_csr_write(IMSIC_EITHRESHOLD, IMSIC_ENABLE_EITHRESHOLD);
+		imsic_csr_write(IMSIC_EIDELIVERY, IMSIC_ENABLE_EIDELIVERY);
+	} else {
+		imsic_csr_write(IMSIC_EIDELIVERY, IMSIC_DISABLE_EIDELIVERY);
+		imsic_csr_write(IMSIC_EITHRESHOLD, IMSIC_DISABLE_EITHRESHOLD);
+	}
+}

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

+
+	raw_spin_lock_irqsave(&priv->ids_lock, flags);
+	ret = bitmap_find_free_region(priv->ids_used_bimap,
+				      max_id + 1, order);
+	raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+
+	return ret;
+}
+
+static void imsic_ids_free(struct imsic_priv *priv, unsigned int base_id,
+			   unsigned int order)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&priv->ids_lock, flags);
+	bitmap_release_region(priv->ids_used_bimap, base_id, order);
+	raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+}
+

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

+	if (!priv->ids_used_bimap)
+		return -ENOMEM;
+
+	/* Allocate enabled bitmap */
+	priv->ids_enabled_bimap = kcalloc(BITS_TO_LONGS(global->nr_ids + 1),
+					   sizeof(unsigned long), GFP_KERNEL);
+	if (!priv->ids_enabled_bimap) {
+		kfree(priv->ids_used_bimap);
+		return -ENOMEM;
+	}
+
+	/* Allocate target CPU array */
+	priv->ids_target_cpu = kcalloc(global->nr_ids + 1,
+				       sizeof(unsigned int), GFP_KERNEL);
+	if (!priv->ids_target_cpu) {
+		kfree(priv->ids_enabled_bimap);
+		kfree(priv->ids_used_bimap);
+		return -ENOMEM;
+	}
+	for (i = 0; i <= global->nr_ids; i++)
+		priv->ids_target_cpu[i] = UINT_MAX;
+
+	/* Reserve ID#0 because it is special and never implemented */
+	bitmap_set(priv->ids_used_bimap, 0, 1);
+
+	return 0;
+}
+
+static void __init imsic_ids_cleanup(struct imsic_priv *priv)
+{
+	kfree(priv->ids_target_cpu);
+	kfree(priv->ids_enabled_bimap);
+	kfree(priv->ids_used_bimap);
+}
+
+#ifdef CONFIG_SMP
+static void imsic_ipi_send(unsigned int cpu)
+{
+	struct imsic_handler *handler = per_cpu_ptr(&imsic_handlers, cpu);
+
+	if (!handler || !handler->priv || !handler->local.msi_va) {
+		pr_warn("CPU%d: handler not initialized\n", cpu);
+		return;
+	}
+
+	writel(handler->priv->ipi_id, handler->local.msi_va);
+}
+
+static void imsic_ipi_enable(struct imsic_priv *priv)
+{
+	__imsic_id_enable(priv->ipi_id);
+	__imsic_id_enable(priv->ipi_lsync_id);
+}
+
+static int __init imsic_ipi_domain_init(struct imsic_priv *priv)
+{

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

+	if (virq <= 0) {
+		imsic_ids_free(priv, priv->ipi_id, get_count_order(1));
+		return (virq < 0) ? virq : -ENOMEM;
+	}
+
+	/* Set vIRQ range */
+	riscv_ipi_set_virq_range(virq, BITS_PER_BYTE, true);
+
+	/* Allocate interrupt identity for local enable/disable sync */
+	virq = imsic_ids_alloc(priv, priv->global.nr_ids, get_count_order(1));
+	if (virq < 0) {
+		imsic_ids_free(priv, priv->ipi_id, get_count_order(1));
+		return virq;
+	}
+	priv->ipi_lsync_id = virq;
+
+	return 0;
+}
+
+static void __init imsic_ipi_domain_cleanup(struct imsic_priv *priv)
+{
+	imsic_ids_free(priv, priv->ipi_lsync_id, get_count_order(1));
+	if (priv->ipi_id)
+		imsic_ids_free(priv, priv->ipi_id, get_count_order(1));
+}
+#else
+static void imsic_ipi_enable(struct imsic_priv *priv)
+{
+}
+
+static int __init imsic_ipi_domain_init(struct imsic_priv *priv)
+{
+	/* Clear the IPI ids because we are not using IPIs */
+	priv->ipi_id = 0;
+	priv->ipi_lsync_id = 0;
+	return 0;
+}
+
+static void __init imsic_ipi_domain_cleanup(struct imsic_priv *priv)
+{
+}
+#endif
+
+static void imsic_irq_mask(struct irq_data *d)
+{
+	imsic_id_disable(irq_data_get_irq_chip_data(d), d->hwirq);
+}
+
+static void imsic_irq_unmask(struct irq_data *d)
+{
+	imsic_id_enable(irq_data_get_irq_chip_data(d), d->hwirq);
+}
+
+static void imsic_irq_compose_msi_msg(struct irq_data *d,
+				      struct msi_msg *msg)
+{
+	struct imsic_priv *priv = irq_data_get_irq_chip_data(d);
+	unsigned int cpu;
+	int err;
+
+	cpu = imsic_id_get_target(priv, d->hwirq);
+	WARN_ON(cpu == UINT_MAX);
+
+	err = imsic_get_cpu_msi_msg(cpu, d->hwirq, msg);
+	WARN_ON(err);
+
+	iommu_dma_compose_msi_msg(irq_data_get_msi_desc(d), msg);
+}
+
+#ifdef CONFIG_SMP
+static int imsic_irq_set_affinity(struct irq_data *d,
+				  const struct cpumask *mask_val,
+				  bool force)
+{
+	struct imsic_priv *priv = irq_data_get_irq_chip_data(d);
+	unsigned int target_cpu;
+	int rc;
+
+	rc = imsic_get_cpu(priv, mask_val, force, &target_cpu);
+	if (rc)
+		return rc;
+
+	imsic_id_set_target(priv, d->hwirq, target_cpu);
+	irq_data_update_effective_affinity(d, cpumask_of(target_cpu));
+
+	return IRQ_SET_MASK_OK;
+}
+#endif
+
+static struct irq_chip imsic_irq_base_chip = {
+	.name			= "RISC-V IMSIC-BASE",
+	.irq_mask		= imsic_irq_mask,
+	.irq_unmask		= imsic_irq_unmask,
+#ifdef CONFIG_SMP
+	.irq_set_affinity	= imsic_irq_set_affinity,
+#endif
+	.irq_compose_msi_msg	= imsic_irq_compose_msi_msg,
+	.flags			= IRQCHIP_SKIP_SET_WAKE |
+				  IRQCHIP_MASK_ON_SUSPEND,
+};
+
+static int imsic_irq_domain_alloc(struct irq_domain *domain,
+				  unsigned int virq,
+				  unsigned int nr_irqs,
+				  void *args)
+{
+	struct imsic_priv *priv = domain->host_data;
+	msi_alloc_info_t *info = args;
+	phys_addr_t msi_addr;
+	int i, hwirq, err = 0;
+	unsigned int cpu;
+
+	err = imsic_get_cpu(priv, &priv->lmask, false, &cpu);
+	if (err)
+		return err;
+
+	err = imsic_cpu_page_phys(cpu, 0, &msi_addr);
+	if (err)
+		return err;
+
+	hwirq = imsic_ids_alloc(priv, priv->global.nr_ids,
+				get_count_order(nr_irqs));
+	if (hwirq < 0)
+		return hwirq;
+
+	err = iommu_dma_prepare_msi(info->desc, msi_addr);
+	if (err)
+		goto fail;
+
+	for (i = 0; i < nr_irqs; i++) {
+		imsic_id_set_target(priv, hwirq + i, cpu);
+		irq_domain_set_info(domain, virq + i, hwirq + i,
+				    &imsic_irq_base_chip, priv,
+				    handle_simple_irq, NULL, NULL);
+		irq_set_noprobe(virq + i);
+		irq_set_affinity(virq + i, &priv->lmask);
+	}
+
+	return 0;
+
+fail:
+	imsic_ids_free(priv, hwirq, get_count_order(nr_irqs));
+	return err;
+}
+
+static void imsic_irq_domain_free(struct irq_domain *domain,
+				  unsigned int virq,
+				  unsigned int nr_irqs)
+{
+	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
+	struct imsic_priv *priv = domain->host_data;
+
+	imsic_ids_free(priv, d->hwirq, get_count_order(nr_irqs));
+	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
+}
+
+static const struct irq_domain_ops imsic_base_domain_ops = {
+	.alloc		= imsic_irq_domain_alloc,
+	.free		= imsic_irq_domain_free,
+};
+
+#ifdef CONFIG_RISCV_IMSIC_PCI
+
+static void imsic_pci_mask_irq(struct irq_data *d)
+{
+	pci_msi_mask_irq(d);
+	irq_chip_mask_parent(d);
+}
+
+static void imsic_pci_unmask_irq(struct irq_data *d)
+{
+	pci_msi_unmask_irq(d);
+	irq_chip_unmask_parent(d);
+}
+
+static struct irq_chip imsic_pci_irq_chip = {
+	.name			= "RISC-V IMSIC-PCI",
+	.irq_mask		= imsic_pci_mask_irq,
+	.irq_unmask		= imsic_pci_unmask_irq,
+	.irq_eoi		= irq_chip_eoi_parent,
+};
+
+static struct msi_domain_ops imsic_pci_domain_ops = {
+};
+
+static struct msi_domain_info imsic_pci_domain_info = {
+	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+		   MSI_FLAG_PCI_MSIX | MSI_FLAG_MULTI_PCI_MSI),
+	.ops	= &imsic_pci_domain_ops,
+	.chip	= &imsic_pci_irq_chip,
+};
+
+#endif
+
+static struct irq_chip imsic_plat_irq_chip = {
+	.name			= "RISC-V IMSIC-PLAT",
+};
+
+static struct msi_domain_ops imsic_plat_domain_ops = {
+};
+
+static struct msi_domain_info imsic_plat_domain_info = {
+	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS),
+	.ops	= &imsic_plat_domain_ops,
+	.chip	= &imsic_plat_irq_chip,
+};
+
+static int __init imsic_irq_domains_init(struct imsic_priv *priv,
+					 struct fwnode_handle *fwnode)
+{
+	/* Create Base IRQ domain */
+	priv->base_domain = irq_domain_create_tree(fwnode,
+						&imsic_base_domain_ops, priv);
+	if (!priv->base_domain) {
+		pr_err("Failed to create IMSIC base domain\n");
+		return -ENOMEM;
+	}
+	irq_domain_update_bus_token(priv->base_domain, DOMAIN_BUS_NEXUS);
+
+#ifdef CONFIG_RISCV_IMSIC_PCI
+	/* Create PCI MSI domain */
+	priv->pci_domain = pci_msi_create_irq_domain(fwnode,
+						&imsic_pci_domain_info,
+						priv->base_domain);
+	if (!priv->pci_domain) {
+		pr_err("Failed to create IMSIC PCI domain\n");
+		irq_domain_remove(priv->base_domain);
+		return -ENOMEM;
+	}
+#endif
+
+	/* Create Platform MSI domain */
+	priv->plat_domain = platform_msi_create_irq_domain(fwnode,
+						&imsic_plat_domain_info,
+						priv->base_domain);
+	if (!priv->plat_domain) {
+		pr_err("Failed to create IMSIC platform domain\n");
+		if (priv->pci_domain)
+			irq_domain_remove(priv->pci_domain);
+		irq_domain_remove(priv->base_domain);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+/*
+ * 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)
+{
+	struct imsic_handler *handler = this_cpu_ptr(&imsic_handlers);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct imsic_priv *priv = handler->priv;
+	irq_hw_number_t hwirq;
+	int err;
+
+	WARN_ON_ONCE(!handler->priv);
+
+	chained_irq_enter(chip, desc);
+
+	while ((hwirq = csr_swap(CSR_TOPEI, 0))) {
+		hwirq = hwirq >> TOPEI_ID_SHIFT;
+
+		if (hwirq == priv->ipi_id) {
+#ifdef CONFIG_SMP
+			ipi_mux_process();
+#endif
+			continue;
+		} else if (hwirq == priv->ipi_lsync_id) {
+			imsic_ids_local_sync(priv);
+			continue;
+		}
+
+		err = generic_handle_domain_irq(priv->base_domain, hwirq);
+		if (unlikely(err))
+			pr_warn_ratelimited(
+				"hwirq %lu mapping not found\n", hwirq);
+	}
+
+	chained_irq_exit(chip, desc);
+}

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

+
+	/* Enable IPIs */
+	imsic_ipi_enable(priv);
+
+	/*
+	 * Interrupts identities might have been enabled/disabled while
+	 * this CPU was not running so sync-up local enable/disable state.
+	 */
+	imsic_ids_local_sync(priv);
+
+	/* Locally enable interrupt delivery */
+	imsic_ids_local_delivery(priv, true);
+
+	return 0;
+}
+
+struct imsic_fwnode_ops {
+	u32 (*nr_parent_irq)(struct fwnode_handle *fwnode,
+			     void *fwopaque);
+	int (*parent_hartid)(struct fwnode_handle *fwnode,
+			     void *fwopaque, u32 index,
+			     unsigned long *out_hartid);
+	u32 (*nr_mmio)(struct fwnode_handle *fwnode, void *fwopaque);

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

+
+static int __init imsic_init(struct imsic_fwnode_ops *fwops,
+			     struct fwnode_handle *fwnode,
+			     void *fwopaque)
+{
+	struct resource res;
+	phys_addr_t base_addr;
+	int rc, nr_parent_irqs;
+	struct imsic_mmio *mmio;
+	struct imsic_priv *priv;
+	struct irq_domain *domain;
+	struct imsic_handler *handler;
+	struct imsic_global_config *global;
+	u32 i, tmp, nr_handlers = 0;
+
+	if (imsic_init_done) {
+		pr_err("%pfwP: already initialized hence ignoring\n",
+			fwnode);
+		return -ENODEV;
+	}
+
+	if (!riscv_isa_extension_available(NULL, SxAIA)) {
+		pr_err("%pfwP: AIA support not available\n", fwnode);
+		return -ENODEV;
+	}
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	global = &priv->global;
+
+	/* Find number of parent interrupts */
+	nr_parent_irqs = fwops->nr_parent_irq(fwnode, fwopaque);
+	if (!nr_parent_irqs) {
+		pr_err("%pfwP: no parent irqs available\n", fwnode);
+		return -EINVAL;
+	}
+
+	/* Find number of guest index bits in MSI address */
+	rc = fwops->read_u32(fwnode, fwopaque, "riscv,guest-index-bits",
+			     &global->guest_index_bits);
+	if (rc)
+		global->guest_index_bits = 0;
+	tmp = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT;
+	if (tmp < global->guest_index_bits) {
+		pr_err("%pfwP: guest index bits too big\n", fwnode);
+		return -EINVAL;
+	}
+
+	/* Find number of HART index bits */
+	rc = fwops->read_u32(fwnode, fwopaque, "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++;
+	}
+	tmp = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT -
+	      global->guest_index_bits;
+	if (tmp < global->hart_index_bits) {
+		pr_err("%pfwP: HART index bits too big\n", fwnode);
+		return -EINVAL;
+	}
+
+	/* Find number of group index bits */
+	rc = fwops->read_u32(fwnode, fwopaque, "riscv,group-index-bits",
+			     &global->group_index_bits);
+	if (rc)
+		global->group_index_bits = 0;
+	tmp = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT -
+	      global->guest_index_bits - global->hart_index_bits;
+	if (tmp < global->group_index_bits) {
+		pr_err("%pfwP: group index bits too big\n", fwnode);
+		return -EINVAL;
+	}
+
+	/*
+	 * Find first bit position of group index.
+	 * If not specified assumed the default APLIC-IMSIC configuration.
+	 */
+	rc = fwops->read_u32(fwnode, fwopaque, "riscv,group-index-shift",
+			     &global->group_index_shift);
+	if (rc)
+		global->group_index_shift = IMSIC_MMIO_PAGE_SHIFT * 2;
+	tmp = global->group_index_bits + global->group_index_shift - 1;
+	if (tmp >= BITS_PER_LONG) {
+		pr_err("%pfwP: group index shift too big\n", fwnode);
+		return -EINVAL;
+	}
+
+	/* Find number of interrupt identities */
+	rc = fwops->read_u32(fwnode, fwopaque, "riscv,num-ids",
+			     &global->nr_ids);
+	if (rc) {
+		pr_err("%pfwP: number of interrupt identities not found\n",
+			fwnode);
+		return rc;
+	}
+	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);
+		return -EINVAL;
+	}
+
+	/* Find number of guest interrupt identities */

Quoted 6 lines · Anup Patel

The RISC-V advanced interrupt architecture (AIA) specification defines a new MSI controller for managing MSIs on a RISC-V platform. This new MSI controller is referred to as incoming message signaled interrupt controller (IMSIC) which manages MSI on per-HART (or per-CPU) basis. (For more details refer https://github.com/riscv/riscv-aia)

And how about IPIs, which this driver seems to be concerned about?

Quoted 31 lines

This patch adds an irqchip driver for RISC-V IMSIC found on RISC-V platforms.

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

---
 drivers/irqchip/Kconfig             |   14 +-
 drivers/irqchip/Makefile            |    1 +
 drivers/irqchip/irq-riscv-imsic.c   | 1174 +++++++++++++++++++++++++++
 include/linux/irqchip/riscv-imsic.h |   92 +++
 4 files changed, 1280 insertions(+), 1 deletion(-)
 create mode 100644 drivers/irqchip/irq-riscv-imsic.c
 create mode 100644 include/linux/irqchip/riscv-imsic.h

drivers/irqchip/Kconfig +0 -1

index 9e65345ca3f6..a1315189a595 100644
@@ -29,7 +29,6 @@ config ARM_GIC_V2M
 
 config GIC_NON_BANKED
      bool
-
 config ARM_GIC_V3
      bool
      select IRQ_DOMAIN_HIERARCHY

@@ -548,6 +547,19 @@ config SIFIVE_PLIC
      select IRQ_DOMAIN_HIERARCHY
      select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP
 

drivers/irqchip/Kconfig config SIFIVE_PLIC · line 561

Quoted 12 lines

drivers/irqchip/Kconfig +12 -0

+config RISCV_IMSIC
+     bool
+     depends on RISCV
+     select IRQ_DOMAIN_HIERARCHY
+     select GENERIC_MSI_IRQ_DOMAIN
+
+config RISCV_IMSIC_PCI
+     bool
+     depends on RISCV_IMSIC
+     depends on PCI
+     depends on PCI_MSI
+     default RISCV_IMSIC

This should definitely tell you that this driver needs splitting.

Quoted 139 lines

drivers/irqchip/Kconfig +1 -0

+
 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 87b49a10962c..22c723cc6ec8 100644
@@ -96,6 +96,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.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.c new file +116 -0

new file mode 100644
index 000000000000..4c16b66738d6
@@ -0,0 +1,1174 @@
+// 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/bitmap.h>
+#include <linux/cpu.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iommu.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqchip/riscv-imsic.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/msi.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/smp.h>
+#include <asm/hwcap.h>
+
+#define IMSIC_DISABLE_EIDELIVERY     0
+#define IMSIC_ENABLE_EIDELIVERY              1
+#define IMSIC_DISABLE_EITHRESHOLD    1
+#define IMSIC_ENABLE_EITHRESHOLD     0
+
+#define imsic_csr_write(__c, __v)    \
+do {                                 \
+     csr_write(CSR_ISELECT, __c);    \
+     csr_write(CSR_IREG, __v);       \
+} while (0)
+
+#define imsic_csr_read(__c)          \
+({                                   \
+     unsigned long __v;              \
+     csr_write(CSR_ISELECT, __c);    \
+     __v = csr_read(CSR_IREG);       \
+     __v;                            \
+})
+
+#define imsic_csr_set(__c, __v)              \
+do {                                 \
+     csr_write(CSR_ISELECT, __c);    \
+     csr_set(CSR_IREG, __v);         \
+} while (0)
+
+#define imsic_csr_clear(__c, __v)    \
+do {                                 \
+     csr_write(CSR_ISELECT, __c);    \
+     csr_clear(CSR_IREG, __v);       \
+} while (0)
+
+struct imsic_mmio {
+     phys_addr_t pa;
+     void __iomem *va;
+     unsigned long size;
+};
+
+struct imsic_priv {
+     /* Global configuration common for all HARTs */
+     struct imsic_global_config global;
+
+     /* MMIO regions */
+     u32 num_mmios;
+     struct imsic_mmio *mmios;
+
+     /* Global state of interrupt identities */
+     raw_spinlock_t ids_lock;
+     unsigned long *ids_used_bimap;
+     unsigned long *ids_enabled_bimap;
+     unsigned int *ids_target_cpu;
+
+     /* Mask for connected CPUs */
+     struct cpumask lmask;
+
+     /* IPI interrupt identity */
+     u32 ipi_id;
+     u32 ipi_lsync_id;
+
+     /* IRQ domains */
+     struct irq_domain *base_domain;
+     struct irq_domain *pci_domain;
+     struct irq_domain *plat_domain;
+};
+
+struct imsic_handler {
+     /* Local configuration for given HART */
+     struct imsic_local_config local;
+
+     /* Pointer to private context */
+     struct imsic_priv *priv;
+};
+
+static bool imsic_init_done;
+
+static int imsic_parent_irq;
+static DEFINE_PER_CPU(struct imsic_handler, imsic_handlers);
+
+const struct imsic_global_config *imsic_get_global_config(void)
+{
+     struct imsic_handler *handler = this_cpu_ptr(&imsic_handlers);
+
+     if (!handler || !handler->priv)
+             return NULL;
+
+     return &handler->priv->global;
+}
+EXPORT_SYMBOL_GPL(imsic_get_global_config);
+

drivers/irqchip/irq-riscv-imsic.c line 122

Quoted 6 lines

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

+const struct imsic_local_config *imsic_get_local_config(unsigned int cpu)
+{
+     struct imsic_handler *handler = per_cpu_ptr(&imsic_handlers, cpu);
+
+     if (!handler || !handler->priv)
+             return NULL;

How can this happen?

drivers/irqchip/irq-riscv-imsic.c line 126

Quoted 5 lines

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

+
+     return &handler->local;
+}
+EXPORT_SYMBOL_GPL(imsic_get_local_config);

Why are these symbols exported? They have no user, so they shouldn't even exist here. I also seriously doubt there is a valid use case for exposing this information to the rest of the kernel.

Quoted 114 lines

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

+
+static int imsic_cpu_page_phys(unsigned int cpu,
+                            unsigned int guest_index,
+                            phys_addr_t *out_msi_pa)
+{
+     struct imsic_handler *handler = per_cpu_ptr(&imsic_handlers, cpu);
+     struct imsic_global_config *global;
+     struct imsic_local_config *local;
+
+     if (!handler || !handler->priv)
+             return -ENODEV;
+     local = &handler->local;
+     global = &handler->priv->global;
+
+     if (BIT(global->guest_index_bits) <= guest_index)
+             return -EINVAL;
+
+     if (out_msi_pa)
+             *out_msi_pa = local->msi_pa +
+                           (guest_index * IMSIC_MMIO_PAGE_SZ);
+
+     return 0;
+}
+
+static int imsic_get_cpu(struct imsic_priv *priv,
+                      const struct cpumask *mask_val, bool force,
+                      unsigned int *out_target_cpu)
+{
+     struct cpumask amask;
+     unsigned int cpu;
+
+     cpumask_and(&amask, &priv->lmask, mask_val);
+
+     if (force)
+             cpu = cpumask_first(&amask);
+     else
+             cpu = cpumask_any_and(&amask, cpu_online_mask);
+
+     if (cpu >= nr_cpu_ids)
+             return -EINVAL;
+
+     if (out_target_cpu)
+             *out_target_cpu = cpu;
+
+     return 0;
+}
+
+static int imsic_get_cpu_msi_msg(unsigned int cpu, unsigned int id,
+                              struct msi_msg *msg)
+{
+     phys_addr_t msi_addr;
+     int err;
+
+     err = imsic_cpu_page_phys(cpu, 0, &msi_addr);
+     if (err)
+             return err;
+
+     msg->address_hi = upper_32_bits(msi_addr);
+     msg->address_lo = lower_32_bits(msi_addr);
+     msg->data = id;
+
+     return err;
+}
+
+static void imsic_id_set_target(struct imsic_priv *priv,
+                              unsigned int id, unsigned int target_cpu)
+{
+     unsigned long flags;
+
+     raw_spin_lock_irqsave(&priv->ids_lock, flags);
+     priv->ids_target_cpu[id] = target_cpu;
+     raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+}
+
+static unsigned int imsic_id_get_target(struct imsic_priv *priv,
+                                     unsigned int id)
+{
+     unsigned int ret;
+     unsigned long flags;
+
+     raw_spin_lock_irqsave(&priv->ids_lock, flags);
+     ret = priv->ids_target_cpu[id];
+     raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+
+     return ret;
+}
+
+static void __imsic_eix_update(unsigned long base_id,
+                            unsigned long num_id, bool pend, bool val)
+{
+     unsigned long i, isel, ireg, flags;
+     unsigned long id = base_id, last_id = base_id + num_id;
+
+     while (id < last_id) {
+             isel = id / BITS_PER_LONG;
+             isel *= BITS_PER_LONG / IMSIC_EIPx_BITS;
+             isel += (pend) ? IMSIC_EIP0 : IMSIC_EIE0;
+
+             ireg = 0;
+             for (i = id & (__riscv_xlen - 1);
+                  (id < last_id) && (i < __riscv_xlen); i++) {
+                     ireg |= BIT(i);
+                     id++;
+             }
+
+             /*
+              * The IMSIC EIEx and EIPx registers are indirectly
+              * accessed via using ISELECT and IREG CSRs so we
+              * save/restore local IRQ to ensure that we don't
+              * get preempted while accessing IMSIC registers.
+              */
+             local_irq_save(flags);
+             if (val)

drivers/irqchip/irq-riscv-imsic.c line 243

Quoted 4 lines

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

+                     imsic_csr_set(isel, ireg);
+             else
+                     imsic_csr_clear(isel, ireg);
+             local_irq_restore(flags);

What is the actual requirement? no preemption? or no interrupts? This isn't the same thing. Also, a bunch of the users already disable interrupts. Consistency wouldn't hurt here.

Quoted 11 lines

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

+     }
+}
+
+#define __imsic_id_enable(__id)              \
+     __imsic_eix_update((__id), 1, false, true)
+#define __imsic_id_disable(__id)     \
+     __imsic_eix_update((__id), 1, false, false)
+
+#ifdef CONFIG_SMP
+static void __imsic_id_smp_sync(struct imsic_priv *priv)

drivers/irqchip/irq-riscv-imsic.c line 259

Quoted 6 lines

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

+{
+     struct imsic_handler *handler;
+     struct cpumask amask;
+     int cpu;
+
+     cpumask_and(&amask, &priv->lmask, cpu_online_mask);

Can't this race against a CPU going down?

drivers/irqchip/irq-riscv-imsic.c line 266

Quoted 8 lines

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

+     for_each_cpu(cpu, &amask) {
+             if (cpu == smp_processor_id())
+                     continue;
+
+             handler = per_cpu_ptr(&imsic_handlers, cpu);
+             if (!handler || !handler->priv || !handler->local.msi_va) {
+                     pr_warn("CPU%d: handler not initialized\n", cpu);

How many times are you going to do that? On each failing synchronisation?

drivers/irqchip/irq-riscv-imsic.c line 270

Quoted 5 lines

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

+                     continue;
+             }
+
+             writel(handler->priv->ipi_lsync_id, handler->local.msi_va);

As I understand it, this is a "behind the scenes" IPI. Why isn't that a *real* IPI?

Quoted 64 lines

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

+     }
+}
+#else
+#define __imsic_id_smp_sync(__priv)
+#endif
+
+static void imsic_id_enable(struct imsic_priv *priv, unsigned int id)
+{
+     unsigned long flags;
+
+     raw_spin_lock_irqsave(&priv->ids_lock, flags);
+     bitmap_set(priv->ids_enabled_bimap, id, 1);
+     __imsic_id_enable(id);
+     raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+
+     __imsic_id_smp_sync(priv);
+}
+
+static void imsic_id_disable(struct imsic_priv *priv, unsigned int id)
+{
+     unsigned long flags;
+
+     raw_spin_lock_irqsave(&priv->ids_lock, flags);
+     bitmap_clear(priv->ids_enabled_bimap, id, 1);
+     __imsic_id_disable(id);
+     raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+
+     __imsic_id_smp_sync(priv);
+}
+
+static void imsic_ids_local_sync(struct imsic_priv *priv)
+{
+     int i;
+     unsigned long flags;
+
+     raw_spin_lock_irqsave(&priv->ids_lock, flags);
+     for (i = 1; i <= priv->global.nr_ids; i++) {
+             if (priv->ipi_id == i || priv->ipi_lsync_id == i)
+                     continue;
+
+             if (test_bit(i, priv->ids_enabled_bimap))
+                     __imsic_id_enable(i);
+             else
+                     __imsic_id_disable(i);
+     }
+     raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+}
+
+static void imsic_ids_local_delivery(struct imsic_priv *priv, bool enable)
+{
+     if (enable) {
+             imsic_csr_write(IMSIC_EITHRESHOLD, IMSIC_ENABLE_EITHRESHOLD);
+             imsic_csr_write(IMSIC_EIDELIVERY, IMSIC_ENABLE_EIDELIVERY);
+     } else {
+             imsic_csr_write(IMSIC_EIDELIVERY, IMSIC_DISABLE_EIDELIVERY);
+             imsic_csr_write(IMSIC_EITHRESHOLD, IMSIC_DISABLE_EITHRESHOLD);
+     }
+}
+
+static int imsic_ids_alloc(struct imsic_priv *priv,
+                        unsigned int max_id, unsigned int order)
+{
+     int ret;

drivers/irqchip/irq-riscv-imsic.c line 338

Quoted 5 lines

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

+     unsigned long flags;
+
+     if ((priv->global.nr_ids < max_id) ||
+         (max_id < BIT(order)))
+             return -EINVAL;

Why do we need this check? Shouldn't that be guaranteed by construction?

Quoted 24 lines

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

+
+     raw_spin_lock_irqsave(&priv->ids_lock, flags);
+     ret = bitmap_find_free_region(priv->ids_used_bimap,
+                                   max_id + 1, order);
+     raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+
+     return ret;
+}
+
+static void imsic_ids_free(struct imsic_priv *priv, unsigned int base_id,
+                        unsigned int order)
+{
+     unsigned long flags;
+
+     raw_spin_lock_irqsave(&priv->ids_lock, flags);
+     bitmap_release_region(priv->ids_used_bimap, base_id, order);
+     raw_spin_unlock_irqrestore(&priv->ids_lock, flags);
+}
+
+static int __init imsic_ids_init(struct imsic_priv *priv)
+{
+     int i;
+     struct imsic_global_config *global = &priv->global;

drivers/irqchip/irq-riscv-imsic.c line 367

Quoted 6 lines

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

+
+     raw_spin_lock_init(&priv->ids_lock);
+
+     /* Allocate used bitmap */
+     priv->ids_used_bimap = kcalloc(BITS_TO_LONGS(global->nr_ids + 1),
+                                     sizeof(unsigned long), GFP_KERNEL);

How about bitmap_alloc?

Quoted 63 lines

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

+     if (!priv->ids_used_bimap)
+             return -ENOMEM;
+
+     /* Allocate enabled bitmap */
+     priv->ids_enabled_bimap = kcalloc(BITS_TO_LONGS(global->nr_ids + 1),
+                                        sizeof(unsigned long), GFP_KERNEL);
+     if (!priv->ids_enabled_bimap) {
+             kfree(priv->ids_used_bimap);
+             return -ENOMEM;
+     }
+
+     /* Allocate target CPU array */
+     priv->ids_target_cpu = kcalloc(global->nr_ids + 1,
+                                    sizeof(unsigned int), GFP_KERNEL);
+     if (!priv->ids_target_cpu) {
+             kfree(priv->ids_enabled_bimap);
+             kfree(priv->ids_used_bimap);
+             return -ENOMEM;
+     }
+     for (i = 0; i <= global->nr_ids; i++)
+             priv->ids_target_cpu[i] = UINT_MAX;
+
+     /* Reserve ID#0 because it is special and never implemented */
+     bitmap_set(priv->ids_used_bimap, 0, 1);
+
+     return 0;
+}
+
+static void __init imsic_ids_cleanup(struct imsic_priv *priv)
+{
+     kfree(priv->ids_target_cpu);
+     kfree(priv->ids_enabled_bimap);
+     kfree(priv->ids_used_bimap);
+}
+
+#ifdef CONFIG_SMP
+static void imsic_ipi_send(unsigned int cpu)
+{
+     struct imsic_handler *handler = per_cpu_ptr(&imsic_handlers, cpu);
+
+     if (!handler || !handler->priv || !handler->local.msi_va) {
+             pr_warn("CPU%d: handler not initialized\n", cpu);
+             return;
+     }
+
+     writel(handler->priv->ipi_id, handler->local.msi_va);
+}
+
+static void imsic_ipi_enable(struct imsic_priv *priv)
+{
+     __imsic_id_enable(priv->ipi_id);
+     __imsic_id_enable(priv->ipi_lsync_id);
+}
+
+static int __init imsic_ipi_domain_init(struct imsic_priv *priv)
+{
+     int virq;
+
+     /* Allocate interrupt identity for IPIs */
+     virq = imsic_ids_alloc(priv, priv->global.nr_ids, get_count_order(1));
+     if (virq < 0)
+             return virq;

drivers/irqchip/irq-riscv-imsic.c line 433

Quoted 4 lines

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

+     priv->ipi_id = virq;
+
+     /* Create IMSIC IPI multiplexing */
+     virq = ipi_mux_create(BITS_PER_BYTE, imsic_ipi_send);

Please! This BITS_PER_BYTE makes zero sense here. Have a proper define that says 8, and document *why* this is 8! You're not defining a type system, you're writing a irqchip driver.

Quoted 289 lines

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

+     if (virq <= 0) {
+             imsic_ids_free(priv, priv->ipi_id, get_count_order(1));
+             return (virq < 0) ? virq : -ENOMEM;
+     }
+
+     /* Set vIRQ range */
+     riscv_ipi_set_virq_range(virq, BITS_PER_BYTE, true);
+
+     /* Allocate interrupt identity for local enable/disable sync */
+     virq = imsic_ids_alloc(priv, priv->global.nr_ids, get_count_order(1));
+     if (virq < 0) {
+             imsic_ids_free(priv, priv->ipi_id, get_count_order(1));
+             return virq;
+     }
+     priv->ipi_lsync_id = virq;
+
+     return 0;
+}
+
+static void __init imsic_ipi_domain_cleanup(struct imsic_priv *priv)
+{
+     imsic_ids_free(priv, priv->ipi_lsync_id, get_count_order(1));
+     if (priv->ipi_id)
+             imsic_ids_free(priv, priv->ipi_id, get_count_order(1));
+}
+#else
+static void imsic_ipi_enable(struct imsic_priv *priv)
+{
+}
+
+static int __init imsic_ipi_domain_init(struct imsic_priv *priv)
+{
+     /* Clear the IPI ids because we are not using IPIs */
+     priv->ipi_id = 0;
+     priv->ipi_lsync_id = 0;
+     return 0;
+}
+
+static void __init imsic_ipi_domain_cleanup(struct imsic_priv *priv)
+{
+}
+#endif
+
+static void imsic_irq_mask(struct irq_data *d)
+{
+     imsic_id_disable(irq_data_get_irq_chip_data(d), d->hwirq);
+}
+
+static void imsic_irq_unmask(struct irq_data *d)
+{
+     imsic_id_enable(irq_data_get_irq_chip_data(d), d->hwirq);
+}
+
+static void imsic_irq_compose_msi_msg(struct irq_data *d,
+                                   struct msi_msg *msg)
+{
+     struct imsic_priv *priv = irq_data_get_irq_chip_data(d);
+     unsigned int cpu;
+     int err;
+
+     cpu = imsic_id_get_target(priv, d->hwirq);
+     WARN_ON(cpu == UINT_MAX);
+
+     err = imsic_get_cpu_msi_msg(cpu, d->hwirq, msg);
+     WARN_ON(err);
+
+     iommu_dma_compose_msi_msg(irq_data_get_msi_desc(d), msg);
+}
+
+#ifdef CONFIG_SMP
+static int imsic_irq_set_affinity(struct irq_data *d,
+                               const struct cpumask *mask_val,
+                               bool force)
+{
+     struct imsic_priv *priv = irq_data_get_irq_chip_data(d);
+     unsigned int target_cpu;
+     int rc;
+
+     rc = imsic_get_cpu(priv, mask_val, force, &target_cpu);
+     if (rc)
+             return rc;
+
+     imsic_id_set_target(priv, d->hwirq, target_cpu);
+     irq_data_update_effective_affinity(d, cpumask_of(target_cpu));
+
+     return IRQ_SET_MASK_OK;
+}
+#endif
+
+static struct irq_chip imsic_irq_base_chip = {
+     .name                   = "RISC-V IMSIC-BASE",
+     .irq_mask               = imsic_irq_mask,
+     .irq_unmask             = imsic_irq_unmask,
+#ifdef CONFIG_SMP
+     .irq_set_affinity       = imsic_irq_set_affinity,
+#endif
+     .irq_compose_msi_msg    = imsic_irq_compose_msi_msg,
+     .flags                  = IRQCHIP_SKIP_SET_WAKE |
+                               IRQCHIP_MASK_ON_SUSPEND,
+};
+
+static int imsic_irq_domain_alloc(struct irq_domain *domain,
+                               unsigned int virq,
+                               unsigned int nr_irqs,
+                               void *args)
+{
+     struct imsic_priv *priv = domain->host_data;
+     msi_alloc_info_t *info = args;
+     phys_addr_t msi_addr;
+     int i, hwirq, err = 0;
+     unsigned int cpu;
+
+     err = imsic_get_cpu(priv, &priv->lmask, false, &cpu);
+     if (err)
+             return err;
+
+     err = imsic_cpu_page_phys(cpu, 0, &msi_addr);
+     if (err)
+             return err;
+
+     hwirq = imsic_ids_alloc(priv, priv->global.nr_ids,
+                             get_count_order(nr_irqs));
+     if (hwirq < 0)
+             return hwirq;
+
+     err = iommu_dma_prepare_msi(info->desc, msi_addr);
+     if (err)
+             goto fail;
+
+     for (i = 0; i < nr_irqs; i++) {
+             imsic_id_set_target(priv, hwirq + i, cpu);
+             irq_domain_set_info(domain, virq + i, hwirq + i,
+                                 &imsic_irq_base_chip, priv,
+                                 handle_simple_irq, NULL, NULL);
+             irq_set_noprobe(virq + i);
+             irq_set_affinity(virq + i, &priv->lmask);
+     }
+
+     return 0;
+
+fail:
+     imsic_ids_free(priv, hwirq, get_count_order(nr_irqs));
+     return err;
+}
+
+static void imsic_irq_domain_free(struct irq_domain *domain,
+                               unsigned int virq,
+                               unsigned int nr_irqs)
+{
+     struct irq_data *d = irq_domain_get_irq_data(domain, virq);
+     struct imsic_priv *priv = domain->host_data;
+
+     imsic_ids_free(priv, d->hwirq, get_count_order(nr_irqs));
+     irq_domain_free_irqs_parent(domain, virq, nr_irqs);
+}
+
+static const struct irq_domain_ops imsic_base_domain_ops = {
+     .alloc          = imsic_irq_domain_alloc,
+     .free           = imsic_irq_domain_free,
+};
+
+#ifdef CONFIG_RISCV_IMSIC_PCI
+
+static void imsic_pci_mask_irq(struct irq_data *d)
+{
+     pci_msi_mask_irq(d);
+     irq_chip_mask_parent(d);
+}
+
+static void imsic_pci_unmask_irq(struct irq_data *d)
+{
+     pci_msi_unmask_irq(d);
+     irq_chip_unmask_parent(d);
+}
+
+static struct irq_chip imsic_pci_irq_chip = {
+     .name                   = "RISC-V IMSIC-PCI",
+     .irq_mask               = imsic_pci_mask_irq,
+     .irq_unmask             = imsic_pci_unmask_irq,
+     .irq_eoi                = irq_chip_eoi_parent,
+};
+
+static struct msi_domain_ops imsic_pci_domain_ops = {
+};
+
+static struct msi_domain_info imsic_pci_domain_info = {
+     .flags  = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+                MSI_FLAG_PCI_MSIX | MSI_FLAG_MULTI_PCI_MSI),
+     .ops    = &imsic_pci_domain_ops,
+     .chip   = &imsic_pci_irq_chip,
+};
+
+#endif
+
+static struct irq_chip imsic_plat_irq_chip = {
+     .name                   = "RISC-V IMSIC-PLAT",
+};
+
+static struct msi_domain_ops imsic_plat_domain_ops = {
+};
+
+static struct msi_domain_info imsic_plat_domain_info = {
+     .flags  = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS),
+     .ops    = &imsic_plat_domain_ops,
+     .chip   = &imsic_plat_irq_chip,
+};
+
+static int __init imsic_irq_domains_init(struct imsic_priv *priv,
+                                      struct fwnode_handle *fwnode)
+{
+     /* Create Base IRQ domain */
+     priv->base_domain = irq_domain_create_tree(fwnode,
+                                             &imsic_base_domain_ops, priv);
+     if (!priv->base_domain) {
+             pr_err("Failed to create IMSIC base domain\n");
+             return -ENOMEM;
+     }
+     irq_domain_update_bus_token(priv->base_domain, DOMAIN_BUS_NEXUS);
+
+#ifdef CONFIG_RISCV_IMSIC_PCI
+     /* Create PCI MSI domain */
+     priv->pci_domain = pci_msi_create_irq_domain(fwnode,
+                                             &imsic_pci_domain_info,
+                                             priv->base_domain);
+     if (!priv->pci_domain) {
+             pr_err("Failed to create IMSIC PCI domain\n");
+             irq_domain_remove(priv->base_domain);
+             return -ENOMEM;
+     }
+#endif
+
+     /* Create Platform MSI domain */
+     priv->plat_domain = platform_msi_create_irq_domain(fwnode,
+                                             &imsic_plat_domain_info,
+                                             priv->base_domain);
+     if (!priv->plat_domain) {
+             pr_err("Failed to create IMSIC platform domain\n");
+             if (priv->pci_domain)
+                     irq_domain_remove(priv->pci_domain);
+             irq_domain_remove(priv->base_domain);
+             return -ENOMEM;
+     }
+
+     return 0;
+}
+
+/*
+ * 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)
+{
+     struct imsic_handler *handler = this_cpu_ptr(&imsic_handlers);
+     struct irq_chip *chip = irq_desc_get_chip(desc);
+     struct imsic_priv *priv = handler->priv;
+     irq_hw_number_t hwirq;
+     int err;
+
+     WARN_ON_ONCE(!handler->priv);
+
+     chained_irq_enter(chip, desc);
+
+     while ((hwirq = csr_swap(CSR_TOPEI, 0))) {
+             hwirq = hwirq >> TOPEI_ID_SHIFT;
+
+             if (hwirq == priv->ipi_id) {
+#ifdef CONFIG_SMP
+                     ipi_mux_process();
+#endif
+                     continue;
+             } else if (hwirq == priv->ipi_lsync_id) {
+                     imsic_ids_local_sync(priv);
+                     continue;
+             }
+
+             err = generic_handle_domain_irq(priv->base_domain, hwirq);
+             if (unlikely(err))
+                     pr_warn_ratelimited(
+                             "hwirq %lu mapping not found\n", hwirq);
+     }
+
+     chained_irq_exit(chip, desc);
+}
+
+static int imsic_starting_cpu(unsigned int cpu)
+{
+     struct imsic_handler *handler = this_cpu_ptr(&imsic_handlers);

drivers/irqchip/irq-riscv-imsic.c line 727

Quoted 6 lines

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

+     struct imsic_priv *priv = handler->priv;
+
+     /* Enable per-CPU parent interrupt */
+     if (imsic_parent_irq)
+             enable_percpu_irq(imsic_parent_irq,
+                               irq_get_trigger_type(imsic_parent_irq));

Shouldn't that be the default already?

drivers/irqchip/irq-riscv-imsic.c line 729

Quoted 3 lines

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

+     else
+             pr_warn("cpu%d: parent irq not available\n", cpu);

And yet continue in sequence? Duh...

Quoted 28 lines

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

+
+     /* Enable IPIs */
+     imsic_ipi_enable(priv);
+
+     /*
+      * Interrupts identities might have been enabled/disabled while
+      * this CPU was not running so sync-up local enable/disable state.
+      */
+     imsic_ids_local_sync(priv);
+
+     /* Locally enable interrupt delivery */
+     imsic_ids_local_delivery(priv, true);
+
+     return 0;
+}
+
+struct imsic_fwnode_ops {
+     u32 (*nr_parent_irq)(struct fwnode_handle *fwnode,
+                          void *fwopaque);
+     int (*parent_hartid)(struct fwnode_handle *fwnode,
+                          void *fwopaque, u32 index,
+                          unsigned long *out_hartid);
+     u32 (*nr_mmio)(struct fwnode_handle *fwnode, void *fwopaque);
+     int (*mmio_to_resource)(struct fwnode_handle *fwnode,
+                             void *fwopaque, u32 index,
+                             struct resource *res);
+     void __iomem *(*mmio_map)(struct fwnode_handle *fwnode,

drivers/irqchip/irq-riscv-imsic.c line 762

Quoted 6 lines

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

+                               void *fwopaque, u32 index);
+     int (*read_u32)(struct fwnode_handle *fwnode,
+                     void *fwopaque, const char *prop, u32 *out_val);
+     bool (*read_bool)(struct fwnode_handle *fwnode,
+                       void *fwopaque, const char *prop);
+};

Why do we need this sort of (terrible) indirection?

Quoted 113 lines

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

+
+static int __init imsic_init(struct imsic_fwnode_ops *fwops,
+                          struct fwnode_handle *fwnode,
+                          void *fwopaque)
+{
+     struct resource res;
+     phys_addr_t base_addr;
+     int rc, nr_parent_irqs;
+     struct imsic_mmio *mmio;
+     struct imsic_priv *priv;
+     struct irq_domain *domain;
+     struct imsic_handler *handler;
+     struct imsic_global_config *global;
+     u32 i, tmp, nr_handlers = 0;
+
+     if (imsic_init_done) {
+             pr_err("%pfwP: already initialized hence ignoring\n",
+                     fwnode);
+             return -ENODEV;
+     }
+
+     if (!riscv_isa_extension_available(NULL, SxAIA)) {
+             pr_err("%pfwP: AIA support not available\n", fwnode);
+             return -ENODEV;
+     }
+
+     priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+     if (!priv)
+             return -ENOMEM;
+     global = &priv->global;
+
+     /* Find number of parent interrupts */
+     nr_parent_irqs = fwops->nr_parent_irq(fwnode, fwopaque);
+     if (!nr_parent_irqs) {
+             pr_err("%pfwP: no parent irqs available\n", fwnode);
+             return -EINVAL;
+     }
+
+     /* Find number of guest index bits in MSI address */
+     rc = fwops->read_u32(fwnode, fwopaque, "riscv,guest-index-bits",
+                          &global->guest_index_bits);
+     if (rc)
+             global->guest_index_bits = 0;
+     tmp = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT;
+     if (tmp < global->guest_index_bits) {
+             pr_err("%pfwP: guest index bits too big\n", fwnode);
+             return -EINVAL;
+     }
+
+     /* Find number of HART index bits */
+     rc = fwops->read_u32(fwnode, fwopaque, "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++;
+     }
+     tmp = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT -
+           global->guest_index_bits;
+     if (tmp < global->hart_index_bits) {
+             pr_err("%pfwP: HART index bits too big\n", fwnode);
+             return -EINVAL;
+     }
+
+     /* Find number of group index bits */
+     rc = fwops->read_u32(fwnode, fwopaque, "riscv,group-index-bits",
+                          &global->group_index_bits);
+     if (rc)
+             global->group_index_bits = 0;
+     tmp = BITS_PER_LONG - IMSIC_MMIO_PAGE_SHIFT -
+           global->guest_index_bits - global->hart_index_bits;
+     if (tmp < global->group_index_bits) {
+             pr_err("%pfwP: group index bits too big\n", fwnode);
+             return -EINVAL;
+     }
+
+     /*
+      * Find first bit position of group index.
+      * If not specified assumed the default APLIC-IMSIC configuration.
+      */
+     rc = fwops->read_u32(fwnode, fwopaque, "riscv,group-index-shift",
+                          &global->group_index_shift);
+     if (rc)
+             global->group_index_shift = IMSIC_MMIO_PAGE_SHIFT * 2;
+     tmp = global->group_index_bits + global->group_index_shift - 1;
+     if (tmp >= BITS_PER_LONG) {
+             pr_err("%pfwP: group index shift too big\n", fwnode);
+             return -EINVAL;
+     }
+
+     /* Find number of interrupt identities */
+     rc = fwops->read_u32(fwnode, fwopaque, "riscv,num-ids",
+                          &global->nr_ids);
+     if (rc) {
+             pr_err("%pfwP: number of interrupt identities not found\n",
+                     fwnode);
+             return rc;
+     }
+     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);
+             return -EINVAL;
+     }
+
+     /* Find number of guest interrupt identities */
+     if (fwops->read_u32(fwnode, fwopaque, "riscv,num-guest-ids",
+                         &global->nr_guest_ids))
+             global->nr_guest_ids = global->nr_ids;
+     if ((global->nr_guest_ids < IMSIC_MIN_ID) ||

drivers/irqchip/irq-riscv-imsic.c line 880

Quoted 6 lines

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

+         (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);
+             return -EINVAL;
+     }

Please split the whole guest stuff out. It is totally unused!