Re: [PATCH v2 4/9] dt-bindings: interrupt-controller: Add RISC-V incoming MSI controller

[Vivian Wang <uwu@xxxxxxxxx>]

On 1/3/23 22:14, Anup Patel wrote:
> 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 <apatel@xxxxxxxxxxxxxxxx>
> ---
>  .../interrupt-controller/riscv,imsics.yaml    | 168 ++++++++++++++++++
>  1 file changed, 168 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml
> new file mode 100644
> index 000000000000..b9db03b6e95f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml
> <snip>
> +
> +  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.
> <snip>
> +...