| Message ID | 20221111044207.1478350-5-apatel@ventanamicro.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | Linux RISC-V AIA Support | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Guessing tree name failed |
On Thu, Nov 10, 2022 at 8:43 PM Anup Patel <apatel@ventanamicro.com> wrote: > > We add DT bindings document for RISC-V incoming MSI controller (IMSIC) > defined by the RISC-V advanced interrupt architecture (AIA) specification. > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > --- > .../interrupt-controller/riscv,imsic.yaml | 174 ++++++++++++++++++ > 1 file changed, 174 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > new file mode 100644 > index 000000000000..05106eb1955e > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > @@ -0,0 +1,174 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsic.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: RISC-V Incoming MSI Controller (IMSIC) > + > +maintainers: > + - Anup Patel <anup@brainfault.org> > + > +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 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 | > + ------------------------------------------------------------- > + > + 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). > + > +allOf: > + - $ref: /schemas/interrupt-controller.yaml# > + > +properties: > + compatible: > + items: > + - enum: > + - vendor,chip-imsics > + - const: riscv,imsics > + > + reg: > + minItems: 1 > + maxItems: 128 > + description: > + Base address of each IMSIC group. > + > + interrupt-controller: true > + > + "#interrupt-cells": > + const: 0 > + > + msi-controller: true > + > + interrupts-extended: > + minItems: 1 > + maxItems: 32768 > + 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: > + Specifies how many interrupt identities are supported by IMSIC interrupt > + file. > + > + riscv,num-guest-ids: > + $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 63 > + maximum: 2047 > + description: > + Specifies how many interrupt identities are supported by IMSIC guest > + interrupt file. When not specified the number of interrupt identities > + supported by IMSIC guest file is assumed to be same as specified by > + the riscv,num-ids property. > + > + riscv,slow-ipi: > + type: boolean > + description: > + The presence of this property implies that software interrupts (i.e. > + IPIs) using IMSIC software injected MSIs is slower compared to other > + software interrupt mechanisms (such as SBI IPI) on the underlying > + RISC-V platform. > + > + riscv,guest-index-bits: > + minimum: 0 > + maximum: 7 > + description: > + Specifies number of guest index bits in the MSI target address. When > + not specified it is assumed to be 0. > + > + riscv,hart-index-bits: > + minimum: 0 > + maximum: 15 > + description: > + Specifies number of HART index bits in the MSI target address. When > + not specified it is estimated based on the interrupts-extended property. > + > + riscv,group-index-bits: > + minimum: 0 > + maximum: 7 > + description: > + Specifies 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: 24 > + maximum: 55 > + description: > + Specifies the least significant bit of the group index bits in the > + MSI target address. When not specified it is assumed to be 24. > + > +additionalProperties: false > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - msi-controller > + - interrupts-extended > + - riscv,num-ids > + > +examples: > + - | > + // Example 1 (Machine-level IMSIC files with just one group): > + > + imsic_mlevel: interrupt-controller@24000000 { > + compatible = "vendor,chip-imsics", "riscv,imsics"; > + interrupts-extended = <&cpu1_intc 11>, > + <&cpu2_intc 11>, > + <&cpu3_intc 11>, > + <&cpu4_intc 11>; > + reg = <0x28000000 0x4000>; nit: /s/0x28000000/24000000 > + 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 = "vendor,chip-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>; > + }; > +... > -- > 2.34.1 >
Hey Anup, On Fri, Nov 11, 2022 at 10:12:02AM +0530, Anup Patel wrote: > dt-bindings: Add RISC-V incoming MSI controller bindings nit: it looks like the usual prefix here is "dt-bindings: interrupt-controller". > We add DT bindings document for RISC-V incoming MSI controller (IMSIC) > defined by the RISC-V advanced interrupt architecture (AIA) specification. > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > --- > .../interrupt-controller/riscv,imsic.yaml | 174 ++++++++++++++++++ > 1 file changed, 174 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > new file mode 100644 > index 000000000000..05106eb1955e > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > @@ -0,0 +1,174 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsic.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: RISC-V Incoming MSI Controller (IMSIC) > + > +maintainers: > + - Anup Patel <anup@brainfault.org> > + > +description: Is this one of the situations where we want to have a | after "description:" to preserve formatting? > + 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 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 | > + ------------------------------------------------------------- > + > + 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). > + > +allOf: > + - $ref: /schemas/interrupt-controller.yaml# > + > +properties: > + compatible: > + items: > + - enum: > + - vendor,chip-imsics Is it valid to have a dummy here? I did a bit of grepping & could not see a single other yaml binding which used a placeholder like this - other than the example schema itself. I assume you're trying to get across the point that using the bare riscv,imsics is not okay and a vendor should create a custom string for their implementation? Also, the file name says "riscv,imsic", the description says "IMSIC" but you've used "imsics" in the compatible. Is this a typo, or a plural? Thanks, Conor. > + - const: riscv,imsics > + > + reg: > + minItems: 1 > + maxItems: 128 > + description: > + Base address of each IMSIC group. > + > + interrupt-controller: true > + > + "#interrupt-cells": > + const: 0 > + > + msi-controller: true > + > + interrupts-extended: > + minItems: 1 > + maxItems: 32768 > + 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: > + Specifies how many interrupt identities are supported by IMSIC interrupt > + file. > + > + riscv,num-guest-ids: > + $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 63 > + maximum: 2047 > + description: > + Specifies how many interrupt identities are supported by IMSIC guest > + interrupt file. When not specified the number of interrupt identities > + supported by IMSIC guest file is assumed to be same as specified by > + the riscv,num-ids property. > + > + riscv,slow-ipi: > + type: boolean > + description: > + The presence of this property implies that software interrupts (i.e. > + IPIs) using IMSIC software injected MSIs is slower compared to other > + software interrupt mechanisms (such as SBI IPI) on the underlying > + RISC-V platform. > + > + riscv,guest-index-bits: > + minimum: 0 > + maximum: 7 > + description: > + Specifies number of guest index bits in the MSI target address. When > + not specified it is assumed to be 0. > + > + riscv,hart-index-bits: > + minimum: 0 > + maximum: 15 > + description: > + Specifies number of HART index bits in the MSI target address. When > + not specified it is estimated based on the interrupts-extended property. > + > + riscv,group-index-bits: > + minimum: 0 > + maximum: 7 > + description: > + Specifies 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: 24 > + maximum: 55 > + description: > + Specifies the least significant bit of the group index bits in the > + MSI target address. When not specified it is assumed to be 24. > + > +additionalProperties: false > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - msi-controller > + - interrupts-extended > + - riscv,num-ids > + > +examples: > + - | > + // Example 1 (Machine-level IMSIC files with just one group): > + > + imsic_mlevel: interrupt-controller@24000000 { > + compatible = "vendor,chip-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 = "vendor,chip-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>; > + }; > +... > -- > 2.34.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 11/11/2022 05:42, Anup Patel wrote: > We add DT bindings document for RISC-V incoming MSI controller (IMSIC) > defined by the RISC-V advanced interrupt architecture (AIA) specification. > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > --- > .../interrupt-controller/riscv,imsic.yaml | 174 ++++++++++++++++++ > 1 file changed, 174 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > new file mode 100644 > index 000000000000..05106eb1955e > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > @@ -0,0 +1,174 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsic.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: RISC-V Incoming MSI Controller (IMSIC) > + > +maintainers: > + - Anup Patel <anup@brainfault.org> > + > +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 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 | > + ------------------------------------------------------------- > + > + 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). > + > +allOf: > + - $ref: /schemas/interrupt-controller.yaml# > + > +properties: > + compatible: > + items: > + - enum: > + - vendor,chip-imsics There is no such vendor... As Conor pointed out, this does not look correct. Compatibles must be real and specific. > + - const: riscv,imsics > + > + reg: > + minItems: 1 > + maxItems: 128 Is there a DTS with 128 reg items? > + description: > + Base address of each IMSIC group. > + > + interrupt-controller: true > + > + "#interrupt-cells": > + const: 0 > + > + msi-controller: true You want then msi-controller.yaml schema and you can drop properties described there. > + > + interrupts-extended: > + minItems: 1 > + maxItems: 32768 I just wonder if you are not putting some random stuff here... just like this "vendor" company. 32768 inputs it is quite a big chip. Are you sure you have so many pins or internal connections? > + 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: > + Specifies how many interrupt identities are supported by IMSIC interrupt > + file. > + > + riscv,num-guest-ids: > + $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 63 > + maximum: 2047 > + description: > + Specifies how many interrupt identities are supported by IMSIC guest > + interrupt file. When not specified the number of interrupt identities > + supported by IMSIC guest file is assumed to be same as specified by > + the riscv,num-ids property. > + > + riscv,slow-ipi: > + type: boolean > + description: > + The presence of this property implies that software interrupts (i.e. > + IPIs) using IMSIC software injected MSIs is slower compared to other > + software interrupt mechanisms (such as SBI IPI) on the underlying > + RISC-V platform. Is this a property of software or hardware? > + > + riscv,guest-index-bits: > + minimum: 0 > + maximum: 7 > + description: > + Specifies number of guest index bits in the MSI target address. When > + not specified it is assumed to be 0. > + > + riscv,hart-index-bits: > + minimum: 0 > + maximum: 15 > + description: > + Specifies number of HART index bits in the MSI target address. When > + not specified it is estimated based on the interrupts-extended property. > + > + riscv,group-index-bits: > + minimum: 0 > + maximum: 7 > + description: > + Specifies number of group index bits in the MSI target address. When > + not specified it is assumed to be 0. Then default: 0. > + > + riscv,group-index-shift: > + $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 24 > + maximum: 55 > + description: > + Specifies the least significant bit of the group index bits in the Please drop everywhere "Specifies the" and instead just describe the hardware. > + MSI target address. When not specified it is assumed to be 24. > + > +additionalProperties: false unevaluatedProperties: false and drop all properties already described by other schemas. > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - msi-controller > + - interrupts-extended > + - riscv,num-ids > + > +examples: > + - | > + // Example 1 (Machine-level IMSIC files with just one group): > + > + imsic_mlevel: interrupt-controller@24000000 { > + compatible = "vendor,chip-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 = "vendor,chip-imsics", "riscv,imsics"; Please run scripts/checkpatch.pl and fix reported warnings. Best regards, Krzysztof
On Mon, Nov 14, 2022 at 3:19 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 11/11/2022 05:42, Anup Patel wrote: > > We add DT bindings document for RISC-V incoming MSI controller (IMSIC) > > defined by the RISC-V advanced interrupt architecture (AIA) specification. > > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > --- > > .../interrupt-controller/riscv,imsic.yaml | 174 ++++++++++++++++++ > > 1 file changed, 174 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > new file mode 100644 > > index 000000000000..05106eb1955e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > @@ -0,0 +1,174 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsic.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: RISC-V Incoming MSI Controller (IMSIC) > > + > > +maintainers: > > + - Anup Patel <anup@brainfault.org> > > + > > +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 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 | > > + ------------------------------------------------------------- > > + > > + 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). > > + > > +allOf: > > + - $ref: /schemas/interrupt-controller.yaml# > > + > > +properties: > > + compatible: > > + items: > > + - enum: > > + - vendor,chip-imsics > > There is no such vendor... As Conor pointed out, this does not look > correct. Compatibles must be real and specific. Previously, Rob had suggest to: 1) Mandate two compatible strings: one for implementation and and second for specification 2) Since this is new specification with QEMU being the only implementation, we add "vendor,chip-imsics" as dummy implementation specific string for DT schema checkers to pass the examples. Once we have an actual implementation, we will replace this dummy string. Refer, https://www.spinics.net/lists/devicetree/msg442720.html > > > + - const: riscv,imsics > > + > > + reg: > > + minItems: 1 > > + maxItems: 128 > > Is there a DTS with 128 reg items? Not at the moment since this is a new specification. The value "128" is because maximum number of IMSIC groups on an system with both IMSIC and APLIC is 128 where each IMSIC group has a separate base address. This is not a hard limit so I am willing to drop it as well. > > > + description: > > + Base address of each IMSIC group. > > + > > + interrupt-controller: true > > + > > + "#interrupt-cells": > > + const: 0 > > + > > + msi-controller: true > > You want then msi-controller.yaml schema and you can drop properties > described there. Okay, I will include msi-controller.yaml in the next revision. > > > + > > + interrupts-extended: > > + minItems: 1 > > + maxItems: 32768 > > I just wonder if you are not putting some random stuff here... just like > this "vendor" company. > > 32768 inputs it is quite a big chip. Are you sure you have so many pins > or internal connections? The interrupts-extended property describes the association of IMSIC interrupt files with the HARTs. If there are N HARTs then we will have N entries in the interrupts-extended (just like the existing PLIC DT bindings). For example, if the first entry points to HART1 and the second entry points to HART0 then the first interrupt file is associated with HART1 and the second interrupt file is associated with HART0. Currently, the "maxItems" limit reflects the max IMSICs which an APLIC domain can target on a system with both IMSIC and APLIC. Actually, there is a typo here. The "maxItems" should be 16384 as-per the frozen AIA specification. I will update "maxItems" accordingly in next patch revision. > > > + 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: > > + Specifies how many interrupt identities are supported by IMSIC interrupt > > + file. > > + > > + riscv,num-guest-ids: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + minimum: 63 > > + maximum: 2047 > > + description: > > + Specifies how many interrupt identities are supported by IMSIC guest > > + interrupt file. When not specified the number of interrupt identities > > + supported by IMSIC guest file is assumed to be same as specified by > > + the riscv,num-ids property. > > + > > + riscv,slow-ipi: > > + type: boolean > > + description: > > + The presence of this property implies that software interrupts (i.e. > > + IPIs) using IMSIC software injected MSIs is slower compared to other > > + software interrupt mechanisms (such as SBI IPI) on the underlying > > + RISC-V platform. > > Is this a property of software or hardware? This is a property of hardware (or implementation) because IPIs in IMSIC are software injected MSIs so if IMSIC is trap-n-emulated by a hypervisor then all writes to MSI register will trap to hypervisor in which case IPI injection via IMSIC is slow. The presence of "riscv,slow-ipi" DT property provides a hint to driver that using IPIs through IMSIC is slow on this platform so if there are other IPI mechanisms (such as SBI IPI calls) then OS should prefer those mechanisms. > > > + > > + riscv,guest-index-bits: > > + minimum: 0 > > + maximum: 7 > > + description: > > + Specifies number of guest index bits in the MSI target address. When > > + not specified it is assumed to be 0. > > + > > + riscv,hart-index-bits: > > + minimum: 0 > > + maximum: 15 > > + description: > > + Specifies number of HART index bits in the MSI target address. When > > + not specified it is estimated based on the interrupts-extended property. > > + > > + riscv,group-index-bits: > > + minimum: 0 > > + maximum: 7 > > + description: > > + Specifies number of group index bits in the MSI target address. When > > + not specified it is assumed to be 0. > > Then default: 0. Okay, I will update. > > > + > > + riscv,group-index-shift: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + minimum: 24 > > + maximum: 55 > > + description: > > + Specifies the least significant bit of the group index bits in the > > Please drop everywhere "Specifies the" and instead just describe the > hardware. Okay, I will update. > > > + MSI target address. When not specified it is assumed to be 24. > > + > > +additionalProperties: false > > unevaluatedProperties: false and drop all properties already described > by other schemas. Okay, I will update. > > > + > > +required: > > + - compatible > > + - reg > > + - interrupt-controller > > + - msi-controller > > + - interrupts-extended > > + - riscv,num-ids > > + > > +examples: > > + - | > > + // Example 1 (Machine-level IMSIC files with just one group): > > + > > + imsic_mlevel: interrupt-controller@24000000 { > > + compatible = "vendor,chip-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 = "vendor,chip-imsics", "riscv,imsics"; > > Please run scripts/checkpatch.pl and fix reported warnings. I did not see any warnings with ./scripts/checkpatch.pl. Is there any parameter of checkpatch.pl which I should try ? Best Regards, Anup
On Mon, Nov 14, 2022 at 05:36:06PM +0530, Anup Patel wrote: > On Mon, Nov 14, 2022 at 3:19 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: > > > > On 11/11/2022 05:42, Anup Patel wrote: > > > We add DT bindings document for RISC-V incoming MSI controller (IMSIC) > > > defined by the RISC-V advanced interrupt architecture (AIA) specification. > > > > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > > --- > > > .../interrupt-controller/riscv,imsic.yaml | 174 ++++++++++++++++++ > > > 1 file changed, 174 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > > new file mode 100644 > > > index 000000000000..05106eb1955e > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > > @@ -0,0 +1,174 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsic.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: RISC-V Incoming MSI Controller (IMSIC) > > > + > > > +maintainers: > > > + - Anup Patel <anup@brainfault.org> > > > + > > > +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 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 | > > > + ------------------------------------------------------------- > > > + > > > + 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). > > > + > > > +allOf: > > > + - $ref: /schemas/interrupt-controller.yaml# > > > + > > > +properties: > > > + compatible: > > > + items: > > > + - enum: > > > + - vendor,chip-imsics > > > > There is no such vendor... As Conor pointed out, this does not look > > correct. Compatibles must be real and specific. > > Previously, Rob had suggest to: > 1) Mandate two compatible strings: one for implementation and > and second for specification > 2) Since this is new specification with QEMU being the only > implementation, we add "vendor,chip-imsics" as dummy > implementation specific string for DT schema checkers > to pass the examples. Once we have an actual implementation, > we will replace this dummy string. > > Refer, https://www.spinics.net/lists/devicetree/msg442720.html AFAIU, <vendor> and <chip> are wildcards and do not have the same meaning as vendor & chip. That's going off of the dt submitting patches doc though and I don't know if the tooling supports this.
On 14/11/2022 13:06, Anup Patel wrote: > On Mon, Nov 14, 2022 at 3:19 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 11/11/2022 05:42, Anup Patel wrote: >>> We add DT bindings document for RISC-V incoming MSI controller (IMSIC) >>> defined by the RISC-V advanced interrupt architecture (AIA) specification. >>> >>> Signed-off-by: Anup Patel <apatel@ventanamicro.com> >>> --- >>> .../interrupt-controller/riscv,imsic.yaml | 174 ++++++++++++++++++ >>> 1 file changed, 174 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml >>> new file mode 100644 >>> index 000000000000..05106eb1955e >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml >>> @@ -0,0 +1,174 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsic.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: RISC-V Incoming MSI Controller (IMSIC) >>> + >>> +maintainers: >>> + - Anup Patel <anup@brainfault.org> >>> + >>> +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 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 | >>> + ------------------------------------------------------------- >>> + >>> + 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). >>> + >>> +allOf: >>> + - $ref: /schemas/interrupt-controller.yaml# >>> + >>> +properties: >>> + compatible: >>> + items: >>> + - enum: >>> + - vendor,chip-imsics >> >> There is no such vendor... As Conor pointed out, this does not look >> correct. Compatibles must be real and specific. > > Previously, Rob had suggest to: > 1) Mandate two compatible strings: one for implementation and > and second for specification > 2) Since this is new specification with QEMU being the only > implementation, we add "vendor,chip-imsics" as dummy > implementation specific string for DT schema checkers > to pass the examples. Once we have an actual implementation, > we will replace this dummy string. > > Refer, https://www.spinics.net/lists/devicetree/msg442720.html And Rob did not propose vendor as vendor and chip-imsics as device. Read his message again. > >> >>> + - const: riscv,imsics >>> + >>> + reg: >>> + minItems: 1 >>> + maxItems: 128 >> >> Is there a DTS with 128 reg items? > > Not at the moment since this is a new specification. > > The value "128" is because maximum number of > IMSIC groups on an system with both IMSIC and > APLIC is 128 where each IMSIC group has a > separate base address. This is not a hard limit so > I am willing to drop it as well. Is "separate base address" really a separate different range or just spaced by few registers? > >> >>> + description: >>> + Base address of each IMSIC group. >>> + >>> + interrupt-controller: true >>> + >>> + "#interrupt-cells": >>> + const: 0 >>> + >>> + msi-controller: true >> >> You want then msi-controller.yaml schema and you can drop properties >> described there. > > Okay, I will include msi-controller.yaml in the next revision. > >> >>> + >>> + interrupts-extended: >>> + minItems: 1 >>> + maxItems: 32768 >> >> I just wonder if you are not putting some random stuff here... just like >> this "vendor" company. >> >> 32768 inputs it is quite a big chip. Are you sure you have so many pins >> or internal connections? > > The interrupts-extended property describes the association of IMSIC > interrupt files with the HARTs. If there are N HARTs then we will have > N entries in the interrupts-extended (just like the existing PLIC DT bindings). > > For example, if the first entry points to HART1 and the second entry points > to HART0 then the first interrupt file is associated with HART1 and the > second interrupt file is associated with HART0. > > Currently, the "maxItems" limit reflects the max IMSICs which an APLIC > domain can target on a system with both IMSIC and APLIC. > > Actually, there is a typo here. The "maxItems" should be 16384 as-per > the frozen AIA specification. I will update "maxItems" accordingly in > next patch revision. > >> >>> + 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: >>> + Specifies how many interrupt identities are supported by IMSIC interrupt >>> + file. >>> + >>> + riscv,num-guest-ids: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + minimum: 63 >>> + maximum: 2047 >>> + description: >>> + Specifies how many interrupt identities are supported by IMSIC guest >>> + interrupt file. When not specified the number of interrupt identities >>> + supported by IMSIC guest file is assumed to be same as specified by >>> + the riscv,num-ids property. >>> + >>> + riscv,slow-ipi: >>> + type: boolean >>> + description: >>> + The presence of this property implies that software interrupts (i.e. >>> + IPIs) using IMSIC software injected MSIs is slower compared to other >>> + software interrupt mechanisms (such as SBI IPI) on the underlying >>> + RISC-V platform. >> >> Is this a property of software or hardware? > > This is a property of hardware (or implementation) because IPIs > in IMSIC are software injected MSIs so if IMSIC is trap-n-emulated > by a hypervisor then all writes to MSI register will trap to hypervisor > in which case IPI injection via IMSIC is slow. > > The presence of "riscv,slow-ipi" DT property provides a hint to > driver that using IPIs through IMSIC is slow on this platform so > if there are other IPI mechanisms (such as SBI IPI calls) then > OS should prefer those mechanisms. If this is specific to implementation, why it is not included already in the compatible? The name is anyway too vague. What is "slow"? Describe real characteristics of hardware, e.g. trapped via hypervisor. > >> >>> + >>> + riscv,guest-index-bits: >>> + minimum: 0 >>> + maximum: 7 >>> + description: >>> + Specifies number of guest index bits in the MSI target address. When >>> + not specified it is assumed to be 0. >>> + >>> + riscv,hart-index-bits: >>> + minimum: 0 >>> + maximum: 15 >>> + description: >>> + Specifies number of HART index bits in the MSI target address. When >>> + not specified it is estimated based on the interrupts-extended property. >>> + >>> + riscv,group-index-bits: >>> + minimum: 0 >>> + maximum: 7 >>> + description: >>> + Specifies number of group index bits in the MSI target address. When >>> + not specified it is assumed to be 0. >> >> Then default: 0. > > Okay, I will update. > >> >>> + >>> + riscv,group-index-shift: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + minimum: 24 >>> + maximum: 55 >>> + description: >>> + Specifies the least significant bit of the group index bits in the >> >> Please drop everywhere "Specifies the" and instead just describe the >> hardware. > > Okay, I will update. > >> >>> + MSI target address. When not specified it is assumed to be 24. >>> + >>> +additionalProperties: false >> >> unevaluatedProperties: false and drop all properties already described >> by other schemas. > > Okay, I will update. > >> >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - interrupt-controller >>> + - msi-controller >>> + - interrupts-extended >>> + - riscv,num-ids >>> + >>> +examples: >>> + - | >>> + // Example 1 (Machine-level IMSIC files with just one group): >>> + >>> + imsic_mlevel: interrupt-controller@24000000 { >>> + compatible = "vendor,chip-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 = "vendor,chip-imsics", "riscv,imsics"; >> >> Please run scripts/checkpatch.pl and fix reported warnings. > > I did not see any warnings with ./scripts/checkpatch.pl. > > Is there any parameter of checkpatch.pl which I should try ? You should see here or with your DTS warnings about undocumented vendor prefix. Best regards, Krzysztof
On Sun, Nov 13, 2022 at 8:18 PM Conor Dooley <conor@kernel.org> wrote: > > Hey Anup, > > On Fri, Nov 11, 2022 at 10:12:02AM +0530, Anup Patel wrote: > > dt-bindings: Add RISC-V incoming MSI controller bindings > > nit: it looks like the usual prefix here is "dt-bindings: > interrupt-controller". Okay, I will update. > > > We add DT bindings document for RISC-V incoming MSI controller (IMSIC) > > defined by the RISC-V advanced interrupt architecture (AIA) specification. > > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > --- > > .../interrupt-controller/riscv,imsic.yaml | 174 ++++++++++++++++++ > > 1 file changed, 174 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > new file mode 100644 > > index 000000000000..05106eb1955e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > @@ -0,0 +1,174 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsic.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: RISC-V Incoming MSI Controller (IMSIC) > > + > > +maintainers: > > + - Anup Patel <anup@brainfault.org> > > + > > +description: > > Is this one of the situations where we want to have a | after > "description:" to preserve formatting? Okay, I will update. > > > + 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 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 | > > + ------------------------------------------------------------- > > + > > + 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). > > + > > +allOf: > > + - $ref: /schemas/interrupt-controller.yaml# > > + > > +properties: > > + compatible: > > + items: > > + - enum: > > + - vendor,chip-imsics > > Is it valid to have a dummy here? I did a bit of grepping & could not > see a single other yaml binding which used a placeholder like this - > other than the example schema itself. I assume you're trying to get > across the point that using the bare riscv,imsics is not okay and a > vendor should create a custom string for their implementation? Yes, this dummy is a placeholder to mandate two compatible strings. The dummy can eventually be replaced by some actual implementation compatible string. > > Also, the file name says "riscv,imsic", the description says "IMSIC" but > you've used "imsics" in the compatible. Is this a typo, or a plural? Yes, the file name should be consistent. I will update the file name. > > Thanks, > Conor. > > > + - const: riscv,imsics > > + > > + reg: > > + minItems: 1 > > + maxItems: 128 > > + description: > > + Base address of each IMSIC group. > > + > > + interrupt-controller: true > > + > > + "#interrupt-cells": > > + const: 0 > > + > > + msi-controller: true > > + > > + interrupts-extended: > > + minItems: 1 > > + maxItems: 32768 > > + 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: > > + Specifies how many interrupt identities are supported by IMSIC interrupt > > + file. > > + > > + riscv,num-guest-ids: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + minimum: 63 > > + maximum: 2047 > > + description: > > + Specifies how many interrupt identities are supported by IMSIC guest > > + interrupt file. When not specified the number of interrupt identities > > + supported by IMSIC guest file is assumed to be same as specified by > > + the riscv,num-ids property. > > + > > + riscv,slow-ipi: > > + type: boolean > > + description: > > + The presence of this property implies that software interrupts (i.e. > > + IPIs) using IMSIC software injected MSIs is slower compared to other > > + software interrupt mechanisms (such as SBI IPI) on the underlying > > + RISC-V platform. > > + > > + riscv,guest-index-bits: > > + minimum: 0 > > + maximum: 7 > > + description: > > + Specifies number of guest index bits in the MSI target address. When > > + not specified it is assumed to be 0. > > + > > + riscv,hart-index-bits: > > + minimum: 0 > > + maximum: 15 > > + description: > > + Specifies number of HART index bits in the MSI target address. When > > + not specified it is estimated based on the interrupts-extended property. > > + > > + riscv,group-index-bits: > > + minimum: 0 > > + maximum: 7 > > + description: > > + Specifies 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: 24 > > + maximum: 55 > > + description: > > + Specifies the least significant bit of the group index bits in the > > + MSI target address. When not specified it is assumed to be 24. > > + > > +additionalProperties: false > > + > > +required: > > + - compatible > > + - reg > > + - interrupt-controller > > + - msi-controller > > + - interrupts-extended > > + - riscv,num-ids > > + > > +examples: > > + - | > > + // Example 1 (Machine-level IMSIC files with just one group): > > + > > + imsic_mlevel: interrupt-controller@24000000 { > > + compatible = "vendor,chip-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 = "vendor,chip-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>; > > + }; > > +... > > -- > > 2.34.1 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv Regards, Anup
On Mon, Nov 14, 2022 at 5:52 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 14/11/2022 13:06, Anup Patel wrote: > > On Mon, Nov 14, 2022 at 3:19 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 11/11/2022 05:42, Anup Patel wrote: > >>> We add DT bindings document for RISC-V incoming MSI controller (IMSIC) > >>> defined by the RISC-V advanced interrupt architecture (AIA) specification. > >>> > >>> Signed-off-by: Anup Patel <apatel@ventanamicro.com> > >>> --- > >>> .../interrupt-controller/riscv,imsic.yaml | 174 ++++++++++++++++++ > >>> 1 file changed, 174 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > >>> new file mode 100644 > >>> index 000000000000..05106eb1955e > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > >>> @@ -0,0 +1,174 @@ > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>> +%YAML 1.2 > >>> +--- > >>> +$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsic.yaml# > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>> + > >>> +title: RISC-V Incoming MSI Controller (IMSIC) > >>> + > >>> +maintainers: > >>> + - Anup Patel <anup@brainfault.org> > >>> + > >>> +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 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 | > >>> + ------------------------------------------------------------- > >>> + > >>> + 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). > >>> + > >>> +allOf: > >>> + - $ref: /schemas/interrupt-controller.yaml# > >>> + > >>> +properties: > >>> + compatible: > >>> + items: > >>> + - enum: > >>> + - vendor,chip-imsics > >> > >> There is no such vendor... As Conor pointed out, this does not look > >> correct. Compatibles must be real and specific. > > > > Previously, Rob had suggest to: > > 1) Mandate two compatible strings: one for implementation and > > and second for specification > > 2) Since this is new specification with QEMU being the only > > implementation, we add "vendor,chip-imsics" as dummy > > implementation specific string for DT schema checkers > > to pass the examples. Once we have an actual implementation, > > we will replace this dummy string. > > > > Refer, https://www.spinics.net/lists/devicetree/msg442720.html > > And Rob did not propose vendor as vendor and chip-imsics as device. Read > his message again. Okay > > > > >> > >>> + - const: riscv,imsics > >>> + > >>> + reg: > >>> + minItems: 1 > >>> + maxItems: 128 > >> > >> Is there a DTS with 128 reg items? > > > > Not at the moment since this is a new specification. > > > > The value "128" is because maximum number of > > IMSIC groups on an system with both IMSIC and > > APLIC is 128 where each IMSIC group has a > > separate base address. This is not a hard limit so > > I am willing to drop it as well. > > Is "separate base address" really a separate different range or just > spaced by few registers? Yes, "separate base address" of an IMSIC group means a separate different range. We can think of an IMSIC group as a CPU cluster or chiplet or die. The IMSIC files within a group are located next to each other whereas the groups can be far away from each other. > > > > >> > >>> + description: > >>> + Base address of each IMSIC group. > >>> + > >>> + interrupt-controller: true > >>> + > >>> + "#interrupt-cells": > >>> + const: 0 > >>> + > >>> + msi-controller: true > >> > >> You want then msi-controller.yaml schema and you can drop properties > >> described there. > > > > Okay, I will include msi-controller.yaml in the next revision. > > > >> > >>> + > >>> + interrupts-extended: > >>> + minItems: 1 > >>> + maxItems: 32768 > >> > >> I just wonder if you are not putting some random stuff here... just like > >> this "vendor" company. > >> > >> 32768 inputs it is quite a big chip. Are you sure you have so many pins > >> or internal connections? > > > > The interrupts-extended property describes the association of IMSIC > > interrupt files with the HARTs. If there are N HARTs then we will have > > N entries in the interrupts-extended (just like the existing PLIC DT bindings). > > > > For example, if the first entry points to HART1 and the second entry points > > to HART0 then the first interrupt file is associated with HART1 and the > > second interrupt file is associated with HART0. > > > > Currently, the "maxItems" limit reflects the max IMSICs which an APLIC > > domain can target on a system with both IMSIC and APLIC. > > > > Actually, there is a typo here. The "maxItems" should be 16384 as-per > > the frozen AIA specification. I will update "maxItems" accordingly in > > next patch revision. > > > >> > >>> + 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: > >>> + Specifies how many interrupt identities are supported by IMSIC interrupt > >>> + file. > >>> + > >>> + riscv,num-guest-ids: > >>> + $ref: /schemas/types.yaml#/definitions/uint32 > >>> + minimum: 63 > >>> + maximum: 2047 > >>> + description: > >>> + Specifies how many interrupt identities are supported by IMSIC guest > >>> + interrupt file. When not specified the number of interrupt identities > >>> + supported by IMSIC guest file is assumed to be same as specified by > >>> + the riscv,num-ids property. > >>> + > >>> + riscv,slow-ipi: > >>> + type: boolean > >>> + description: > >>> + The presence of this property implies that software interrupts (i.e. > >>> + IPIs) using IMSIC software injected MSIs is slower compared to other > >>> + software interrupt mechanisms (such as SBI IPI) on the underlying > >>> + RISC-V platform. > >> > >> Is this a property of software or hardware? > > > > This is a property of hardware (or implementation) because IPIs > > in IMSIC are software injected MSIs so if IMSIC is trap-n-emulated > > by a hypervisor then all writes to MSI register will trap to hypervisor > > in which case IPI injection via IMSIC is slow. > > > > The presence of "riscv,slow-ipi" DT property provides a hint to > > driver that using IPIs through IMSIC is slow on this platform so > > if there are other IPI mechanisms (such as SBI IPI calls) then > > OS should prefer those mechanisms. > > If this is specific to implementation, why it is not included already in > the compatible? > > The name is anyway too vague. What is "slow"? Describe real > characteristics of hardware, e.g. trapped via hypervisor. Okay, how about renaming it to "riscv,trap-n-emulated" ? Alternately, we can add "riscv,soft-imsics" as an implementation specific compatible string which hypervisors can use to describe trap-n-emulated IMSICs. This "riscv,soft-imsics" can also replace "vendor,chip-imsics" dummy string ? > > > >> > >>> + > >>> + riscv,guest-index-bits: > >>> + minimum: 0 > >>> + maximum: 7 > >>> + description: > >>> + Specifies number of guest index bits in the MSI target address. When > >>> + not specified it is assumed to be 0. > >>> + > >>> + riscv,hart-index-bits: > >>> + minimum: 0 > >>> + maximum: 15 > >>> + description: > >>> + Specifies number of HART index bits in the MSI target address. When > >>> + not specified it is estimated based on the interrupts-extended property. > >>> + > >>> + riscv,group-index-bits: > >>> + minimum: 0 > >>> + maximum: 7 > >>> + description: > >>> + Specifies number of group index bits in the MSI target address. When > >>> + not specified it is assumed to be 0. > >> > >> Then default: 0. > > > > Okay, I will update. > > > >> > >>> + > >>> + riscv,group-index-shift: > >>> + $ref: /schemas/types.yaml#/definitions/uint32 > >>> + minimum: 24 > >>> + maximum: 55 > >>> + description: > >>> + Specifies the least significant bit of the group index bits in the > >> > >> Please drop everywhere "Specifies the" and instead just describe the > >> hardware. > > > > Okay, I will update. > > > >> > >>> + MSI target address. When not specified it is assumed to be 24. > >>> + > >>> +additionalProperties: false > >> > >> unevaluatedProperties: false and drop all properties already described > >> by other schemas. > > > > Okay, I will update. > > > >> > >>> + > >>> +required: > >>> + - compatible > >>> + - reg > >>> + - interrupt-controller > >>> + - msi-controller > >>> + - interrupts-extended > >>> + - riscv,num-ids > >>> + > >>> +examples: > >>> + - | > >>> + // Example 1 (Machine-level IMSIC files with just one group): > >>> + > >>> + imsic_mlevel: interrupt-controller@24000000 { > >>> + compatible = "vendor,chip-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 = "vendor,chip-imsics", "riscv,imsics"; > >> > >> Please run scripts/checkpatch.pl and fix reported warnings. > > > > I did not see any warnings with ./scripts/checkpatch.pl. > > > > Is there any parameter of checkpatch.pl which I should try ? > > You should see here or with your DTS warnings about undocumented vendor > prefix. Okay, I will check. > > Best regards, > Krzysztof > Best Regards, Anup
On 14/11/2022 16:04, Anup Patel wrote: > On Mon, Nov 14, 2022 at 5:52 PM Krzysztof Kozlowski >>>>> + riscv,slow-ipi: >>>>> + type: boolean >>>>> + description: >>>>> + The presence of this property implies that software interrupts (i.e. >>>>> + IPIs) using IMSIC software injected MSIs is slower compared to other >>>>> + software interrupt mechanisms (such as SBI IPI) on the underlying >>>>> + RISC-V platform. >>>> >>>> Is this a property of software or hardware? >>> >>> This is a property of hardware (or implementation) because IPIs >>> in IMSIC are software injected MSIs so if IMSIC is trap-n-emulated >>> by a hypervisor then all writes to MSI register will trap to hypervisor >>> in which case IPI injection via IMSIC is slow. >>> >>> The presence of "riscv,slow-ipi" DT property provides a hint to >>> driver that using IPIs through IMSIC is slow on this platform so >>> if there are other IPI mechanisms (such as SBI IPI calls) then >>> OS should prefer those mechanisms. >> >> If this is specific to implementation, why it is not included already in >> the compatible? >> >> The name is anyway too vague. What is "slow"? Describe real >> characteristics of hardware, e.g. trapped via hypervisor. > > Okay, how about renaming it to "riscv,trap-n-emulated" ? Sounds ok. > > Alternately, we can add "riscv,soft-imsics" as an implementation > specific compatible string which hypervisors can use to describe > trap-n-emulated IMSICs. This "riscv,soft-imsics" can also replace > "vendor,chip-imsics" dummy string ? soft-imsics would work only if it is a real device. My question was rather whether this is something configurable or fixed in given implementation. Best regards, Krzysztof
On Mon, Nov 14, 2022 at 05:59:00PM +0530, Anup Patel wrote: > On Sun, Nov 13, 2022 at 8:18 PM Conor Dooley <conor@kernel.org> wrote: > > Also, the file name says "riscv,imsic", the description says "IMSIC" but > > you've used "imsics" in the compatible. Is this a typo, or a plural? > > Yes, the file name should be consistent. I will update the file name. Is there a reason why the compatible is plural when all of the other mentions etc do not have an "s"? It really did look like a typo to me. It's the "incoming MSI controller", so I am unsure as to where the "s" actually even comes from. Why not just use "riscv,imsic"? Thanks, Conor.
On 15/11/2022 23:34, Conor Dooley wrote: > On Mon, Nov 14, 2022 at 05:59:00PM +0530, Anup Patel wrote: >> On Sun, Nov 13, 2022 at 8:18 PM Conor Dooley <conor@kernel.org> wrote: > >>> Also, the file name says "riscv,imsic", the description says "IMSIC" but >>> you've used "imsics" in the compatible. Is this a typo, or a plural? >> >> Yes, the file name should be consistent. I will update the file name. > > Is there a reason why the compatible is plural when all of the other > mentions etc do not have an "s"? It really did look like a typo to me. > > It's the "incoming MSI controller", so I am unsure as to where the "s" > actually even comes from. Why not just use "riscv,imsic"? Yep, should be rather consistent with all others, and IMSIC stands for Integrated Circuit? Best regards, Krzysztof
On Wed, Nov 16, 2022 at 10:00:27AM +0100, Krzysztof Kozlowski wrote: > On 15/11/2022 23:34, Conor Dooley wrote: > > On Mon, Nov 14, 2022 at 05:59:00PM +0530, Anup Patel wrote: > >> On Sun, Nov 13, 2022 at 8:18 PM Conor Dooley <conor@kernel.org> wrote: > > > >>> Also, the file name says "riscv,imsic", the description says "IMSIC" but > >>> you've used "imsics" in the compatible. Is this a typo, or a plural? > >> > >> Yes, the file name should be consistent. I will update the file name. > > > > Is there a reason why the compatible is plural when all of the other > > mentions etc do not have an "s"? It really did look like a typo to me. > > > > It's the "incoming MSI controller", so I am unsure as to where the "s" > > actually even comes from. Why not just use "riscv,imsic"? > > Yep, should be rather consistent with all others, and IMSIC stands for > Integrated Circuit? Incoming Message Signalled Interrupts Controller, no?
On 16/11/2022 10:20, Conor Dooley wrote: > On Wed, Nov 16, 2022 at 10:00:27AM +0100, Krzysztof Kozlowski wrote: >> On 15/11/2022 23:34, Conor Dooley wrote: >>> On Mon, Nov 14, 2022 at 05:59:00PM +0530, Anup Patel wrote: >>>> On Sun, Nov 13, 2022 at 8:18 PM Conor Dooley <conor@kernel.org> wrote: >>> >>>>> Also, the file name says "riscv,imsic", the description says "IMSIC" but >>>>> you've used "imsics" in the compatible. Is this a typo, or a plural? >>>> >>>> Yes, the file name should be consistent. I will update the file name. >>> >>> Is there a reason why the compatible is plural when all of the other >>> mentions etc do not have an "s"? It really did look like a typo to me. >>> >>> It's the "incoming MSI controller", so I am unsure as to where the "s" >>> actually even comes from. Why not just use "riscv,imsic"? >> >> Yep, should be rather consistent with all others, and IMSIC stands for >> Integrated Circuit? > > Incoming Message Signalled Interrupts Controller, no? Ah, then still singular :) Best regards, Krzysztof
On Wed, Nov 16, 2022 at 2:30 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 15/11/2022 23:34, Conor Dooley wrote: > > On Mon, Nov 14, 2022 at 05:59:00PM +0530, Anup Patel wrote: > >> On Sun, Nov 13, 2022 at 8:18 PM Conor Dooley <conor@kernel.org> wrote: > > > >>> Also, the file name says "riscv,imsic", the description says "IMSIC" but > >>> you've used "imsics" in the compatible. Is this a typo, or a plural? > >> > >> Yes, the file name should be consistent. I will update the file name. > > > > Is there a reason why the compatible is plural when all of the other > > mentions etc do not have an "s"? It really did look like a typo to me. > > > > It's the "incoming MSI controller", so I am unsure as to where the "s" > > actually even comes from. Why not just use "riscv,imsic"? > > Yep, should be rather consistent with all others, and IMSIC stands for > Integrated Circuit? This is intentionally plural because even though we have one IMSIC per-CPU, Linux (and various OSes) expect one DT node as MSI controller targeting all CPUs. The plural compatible string "riscv,imsics" was chosen based on consensus on RISC-V AIA Task Group meetings. Regards, Anup
On Wed, Nov 16, 2022 at 04:04:45PM +0530, Anup Patel wrote: > On Wed, Nov 16, 2022 at 2:30 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: > > > > On 15/11/2022 23:34, Conor Dooley wrote: > > > On Mon, Nov 14, 2022 at 05:59:00PM +0530, Anup Patel wrote: > > >> On Sun, Nov 13, 2022 at 8:18 PM Conor Dooley <conor@kernel.org> wrote: > > > > > >>> Also, the file name says "riscv,imsic", the description says "IMSIC" but > > >>> you've used "imsics" in the compatible. Is this a typo, or a plural? > > >> > > >> Yes, the file name should be consistent. I will update the file name. > > > > > > Is there a reason why the compatible is plural when all of the other > > > mentions etc do not have an "s"? It really did look like a typo to me. > > > > > > It's the "incoming MSI controller", so I am unsure as to where the "s" > > > actually even comes from. Why not just use "riscv,imsic"? > > > > Yep, should be rather consistent with all others, and IMSIC stands for > > Integrated Circuit? > > This is intentionally plural because even though we have one > IMSIC per-CPU, Linux (and various OSes) expect one DT node > as MSI controller targeting all CPUs. Even still, calling it "riscv,imsic" would seem fair to me given the multiple regs make the distinct regions clear. I think I must have missed the bit at the end of the description though: + 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). Perhaps, for eejits like me, that paragraph should become paragraph 3 instead of hiding it below the register layout etc? Anyways, existing name seems fine to me then w/ the filename update & increased prominence of the many-controllers-in-one statement. Maybe the devicetree gods think differently! > The plural compatible string "riscv,imsics" was chosen based > on consensus on RISC-V AIA Task Group meetings. btw, I see the following in the example: + reg = <0x28000000 0x2000>, /* Group0 IMSICs */ + <0x29000000 0x2000>; /* Group1 IMSICs */ And in the property: + reg: + minItems: 1 + maxItems: 128 + description: + Base address of each IMSIC group. It would appear that the comment there conflicts with the description of the reg property itself & it's that lack of consistency me confused (: Should the comments be in the singular form? Thanks, Conor.
On Mon, Nov 14, 2022 at 05:36:06PM +0530, Anup Patel wrote: > On Mon, Nov 14, 2022 at 3:19 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: > > > > On 11/11/2022 05:42, Anup Patel wrote: > > > We add DT bindings document for RISC-V incoming MSI controller (IMSIC) > > > defined by the RISC-V advanced interrupt architecture (AIA) specification. > > > > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > > --- > > > .../interrupt-controller/riscv,imsic.yaml | 174 ++++++++++++++++++ > > > 1 file changed, 174 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > > new file mode 100644 > > > index 000000000000..05106eb1955e > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > > @@ -0,0 +1,174 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsic.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: RISC-V Incoming MSI Controller (IMSIC) > > > + > > > +maintainers: > > > + - Anup Patel <anup@brainfault.org> > > > + > > > +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 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 | > > > + ------------------------------------------------------------- > > > + > > > + 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). > > > + > > > +allOf: > > > + - $ref: /schemas/interrupt-controller.yaml# > > > + > > > +properties: > > > + compatible: > > > + items: > > > + - enum: > > > + - vendor,chip-imsics > > > > There is no such vendor... As Conor pointed out, this does not look > > correct. Compatibles must be real and specific. > > Previously, Rob had suggest to: > 1) Mandate two compatible strings: one for implementation and > and second for specification > 2) Since this is new specification with QEMU being the only > implementation, we add "vendor,chip-imsics" as dummy > implementation specific string for DT schema checkers > to pass the examples. Once we have an actual implementation, > we will replace this dummy string. What will QEMU's DT use? That's an implementation we can and do run validation on. Your choices are define a QEMU specific compatible string or allow the fallback alone. I'm fine either way. With the latter, someone has to review that the fallback is not used alone in .dts files while doing the former allows the tools to check for you. It also encourages making every new difference a property rather than implied by compatible, but those should be caught in review. If you go with the fallback only, just make it clear that it's for QEMU or s/w models only. Rob
On Thu, Nov 17, 2022 at 12:44 AM Rob Herring <robh@kernel.org> wrote: > > On Mon, Nov 14, 2022 at 05:36:06PM +0530, Anup Patel wrote: > > On Mon, Nov 14, 2022 at 3:19 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > > > > > > On 11/11/2022 05:42, Anup Patel wrote: > > > > We add DT bindings document for RISC-V incoming MSI controller (IMSIC) > > > > defined by the RISC-V advanced interrupt architecture (AIA) specification. > > > > > > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > > > --- > > > > .../interrupt-controller/riscv,imsic.yaml | 174 ++++++++++++++++++ > > > > 1 file changed, 174 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > > > new file mode 100644 > > > > index 000000000000..05106eb1955e > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml > > > > @@ -0,0 +1,174 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsic.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: RISC-V Incoming MSI Controller (IMSIC) > > > > + > > > > +maintainers: > > > > + - Anup Patel <anup@brainfault.org> > > > > + > > > > +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 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 | > > > > + ------------------------------------------------------------- > > > > + > > > > + 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). > > > > + > > > > +allOf: > > > > + - $ref: /schemas/interrupt-controller.yaml# > > > > + > > > > +properties: > > > > + compatible: > > > > + items: > > > > + - enum: > > > > + - vendor,chip-imsics > > > > > > There is no such vendor... As Conor pointed out, this does not look > > > correct. Compatibles must be real and specific. > > > > Previously, Rob had suggest to: > > 1) Mandate two compatible strings: one for implementation and > > and second for specification > > 2) Since this is new specification with QEMU being the only > > implementation, we add "vendor,chip-imsics" as dummy > > implementation specific string for DT schema checkers > > to pass the examples. Once we have an actual implementation, > > we will replace this dummy string. > > What will QEMU's DT use? That's an implementation we can and do run > validation on. Your choices are define a QEMU specific compatible string > or allow the fallback alone. I'm fine either way. With the latter, > someone has to review that the fallback is not used alone in .dts files > while doing the former allows the tools to check for you. It also > encourages making every new difference a property rather than implied by > compatible, but those should be caught in review. > > If you go with the fallback only, just make it clear that it's for QEMU > or s/w models only. Sure, I will add "riscv,qemu-imsics" as QEMU specific compatible string. Regards, Anup
diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml new file mode 100644 index 000000000000..05106eb1955e --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml @@ -0,0 +1,174 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsic.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: RISC-V Incoming MSI Controller (IMSIC) + +maintainers: + - Anup Patel <anup@brainfault.org> + +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 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 | + ------------------------------------------------------------- + + 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). + +allOf: + - $ref: /schemas/interrupt-controller.yaml# + +properties: + compatible: + items: + - enum: + - vendor,chip-imsics + - const: riscv,imsics + + reg: + minItems: 1 + maxItems: 128 + description: + Base address of each IMSIC group. + + interrupt-controller: true + + "#interrupt-cells": + const: 0 + + msi-controller: true + + interrupts-extended: + minItems: 1 + maxItems: 32768 + 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: + Specifies how many interrupt identities are supported by IMSIC interrupt + file. + + riscv,num-guest-ids: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 63 + maximum: 2047 + description: + Specifies how many interrupt identities are supported by IMSIC guest + interrupt file. When not specified the number of interrupt identities + supported by IMSIC guest file is assumed to be same as specified by + the riscv,num-ids property. + + riscv,slow-ipi: + type: boolean + description: + The presence of this property implies that software interrupts (i.e. + IPIs) using IMSIC software injected MSIs is slower compared to other + software interrupt mechanisms (such as SBI IPI) on the underlying + RISC-V platform. + + riscv,guest-index-bits: + minimum: 0 + maximum: 7 + description: + Specifies number of guest index bits in the MSI target address. When + not specified it is assumed to be 0. + + riscv,hart-index-bits: + minimum: 0 + maximum: 15 + description: + Specifies number of HART index bits in the MSI target address. When + not specified it is estimated based on the interrupts-extended property. + + riscv,group-index-bits: + minimum: 0 + maximum: 7 + description: + Specifies 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: 24 + maximum: 55 + description: + Specifies the least significant bit of the group index bits in the + MSI target address. When not specified it is assumed to be 24. + +additionalProperties: false + +required: + - compatible + - reg + - interrupt-controller + - msi-controller + - interrupts-extended + - riscv,num-ids + +examples: + - | + // Example 1 (Machine-level IMSIC files with just one group): + + imsic_mlevel: interrupt-controller@24000000 { + compatible = "vendor,chip-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 = "vendor,chip-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>; + }; +...
We add DT bindings document for RISC-V incoming MSI controller (IMSIC) defined by the RISC-V advanced interrupt architecture (AIA) specification. Signed-off-by: Anup Patel <apatel@ventanamicro.com> --- .../interrupt-controller/riscv,imsic.yaml | 174 ++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsic.yaml