Re: [PATCH v7 11/15] dt-bindings: interrupt-controller: Add RISC-V advanced PLIC

[Anup Patel <apatel@ventanamicro.com>]

On Thu, Aug 10, 2023 at 4:59 PM Vincent Chen <vincent.chen@sifive.com> wrote:
>
>
>
> On Thu, Aug 10, 2023 at 4:08 PM Anup Patel <apatel@ventanamicro.com> wrote:
>>
>> On Thu, Aug 10, 2023 at 1:31 PM Vincent Chen <vincent.chen@sifive.com> wrote:
>> >
>> >
>> >
>> > On Wed, Aug 2, 2023 at 11:02 PM Anup Patel <apatel@ventanamicro.com> wrote:
>> >>
>> >> We add DT bindings document for RISC-V advanced platform level interrupt
>> >> controller (APLIC) defined by the RISC-V advanced interrupt architecture
>> >> (AIA) specification.
>> >>
>> >> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>> >> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>> >> ---
>> >>  .../interrupt-controller/riscv,aplic.yaml     | 172 ++++++++++++++++++
>> >>  1 file changed, 172 insertions(+)
>> >>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
>> >> new file mode 100644
>> >> index 000000000000..190a6499c932
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
>> >> @@ -0,0 +1,172 @@
>> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> >> +%YAML 1.2
>> >> +---
>> >> +$id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
>> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> >> +
>> >> +title: RISC-V Advanced Platform Level Interrupt Controller (APLIC)
>> >> +
>> >> +maintainers:
>> >> +  - Anup Patel <anup@brainfault.org>
>> >> +
>> >> +description:
>> >> +  The RISC-V advanced interrupt architecture (AIA) defines an advanced
>> >> +  platform level interrupt controller (APLIC) for handling wired interrupts
>> >> +  in a RISC-V platform. The RISC-V AIA specification can be found at
>> >> +  https://github.com/riscv/riscv-aia.
>> >> +
>> >> +  The RISC-V APLIC is implemented as hierarchical APLIC domains where all
>> >> +  interrupt sources connect to the root APLIC domain and a parent APLIC
>> >> +  domain can delegate interrupt sources to it's child APLIC domains. There
>> >> +  is one device tree node for each APLIC domain.
>> >> +
>> >> +allOf:
>> >> +  - $ref: /schemas/interrupt-controller.yaml#
>> >> +
>> >> +properties:
>> >> +  compatible:
>> >> +    items:
>> >> +      - enum:
>> >> +          - qemu,aplic
>> >> +      - const: riscv,aplic
>> >> +
>> >> +  reg:
>> >> +    maxItems: 1
>> >> +
>> >> +  interrupt-controller: true
>> >> +
>> >> +  "#interrupt-cells":
>> >> +    const: 2
>> >> +
>> >> +  interrupts-extended:
>> >> +    minItems: 1
>> >> +    maxItems: 16384
>> >> +    description:
>> >> +      Given APLIC domain directly injects external interrupts to a set of
>> >> +      RISC-V HARTS (or CPUs). Each node pointed to should be a riscv,cpu-intc
>> >> +      node, which has a CPU node (i.e. RISC-V HART) as parent.
>> >> +
>> >> +  msi-parent:
>> >> +    description:
>> >> +      Given APLIC domain forwards wired interrupts as MSIs to a AIA incoming
>> >> +      message signaled interrupt controller (IMSIC). If both "msi-parent" and
>> >> +      "interrupts-extended" properties are present then it means the APLIC
>> >> +      domain supports both MSI mode and Direct mode in HW. In this case, the
>> >> +      APLIC driver has to choose between MSI mode or Direct mode.
>> >> +
>> >> +  riscv,num-sources:
>> >> +    $ref: /schemas/types.yaml#/definitions/uint32
>> >> +    minimum: 1
>> >> +    maximum: 1023
>> >> +    description:
>> >> +      Specifies the number of wired interrupt sources supported by this
>> >> +      APLIC domain.
>> >> +
>> >> +  riscv,children:
>> >> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> >> +    minItems: 1
>> >> +    maxItems: 1024
>> >> +    items:
>> >> +      maxItems: 1
>> >> +    description:
>> >> +      A list of child APLIC domains for the given APLIC domain. Each child
>> >> +      APLIC domain is assigned a child index in increasing order, with the
>> >> +      first child APLIC domain assigned child index 0. The APLIC domain child
>> >> +      index is used by firmware to delegate interrupts from the given APLIC
>> >> +      domain to a particular child APLIC domain.
>> >> +
>> >> +  riscv,delegation:
>> >> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> >> +    minItems: 1
>> >> +    maxItems: 1024
>> >> +    items:
>> >> +      items:
>> >> +        - description: child APLIC domain phandle
>> >> +        - description: first interrupt number of the parent APLIC domain (inclusive)
>> >> +        - description: last interrupt number of the parent APLIC domain (inclusive)
>> >> +    description:
>> >> +      A interrupt delegation list where each entry is a triple consisting
>> >> +      of child APLIC domain phandle, first interrupt number of the parent
>> >> +      APLIC domain, and last interrupt number of the parent APLIC domain.
>> >> +      Firmware must configure interrupt delegation registers based on
>> >> +      interrupt delegation list.
>> >> +
>> >> +dependencies:
>> >> +  riscv,delegation: [ "riscv,children" ]
>> >> +
>> >> +required:
>> >> +  - compatible
>> >> +  - reg
>> >> +  - interrupt-controller
>> >> +  - "#interrupt-cells"
>> >> +  - riscv,num-sources
>> >> +
>> >> +anyOf:
>> >> +  - required:
>> >> +      - interrupts-extended
>> >> +  - required:
>> >> +      - msi-parent
>> >> +
>> >> +unevaluatedProperties: false
>> >> +
>> >> +examples:
>> >> +  - |
>> >> +    // Example 1 (APLIC domains directly injecting interrupt to HARTs):
>> >> +
>> >> +    interrupt-controller@c000000 {
>> >> +      compatible = "qemu,aplic", "riscv,aplic";
>> >> +      interrupts-extended = <&cpu1_intc 11>,
>> >> +                            <&cpu2_intc 11>,
>> >> +                            <&cpu3_intc 11>,
>> >> +                            <&cpu4_intc 11>;
>> >> +      reg = <0xc000000 0x4080>;
>> >> +      interrupt-controller;
>> >> +      #interrupt-cells = <2>;
>> >> +      riscv,num-sources = <63>;
>> >> +      riscv,children = <&aplic1>, <&aplic2>;
>> >> +      riscv,delegation = <&aplic1 1 63>;
>> >> +    };
>> >> +
>> >> +    aplic1: interrupt-controller@d000000 {
>> >> +      compatible = "qemu,aplic", "riscv,aplic";
>> >> +      interrupts-extended = <&cpu1_intc 9>,
>> >> +                            <&cpu2_intc 9>;
>> >> +      reg = <0xd000000 0x4080>;
>> >> +      interrupt-controller;
>> >> +      #interrupt-cells = <2>;
>> >> +      riscv,num-sources = <63>;
>> >> +    };
>> >> +
>> >> +    aplic2: interrupt-controller@e000000 {
>> >> +      compatible = "qemu,aplic", "riscv,aplic";
>> >> +      interrupts-extended = <&cpu3_intc 9>,
>> >> +                            <&cpu4_intc 9>;
>> >> +      reg = <0xe000000 0x4080>;
>> >> +      interrupt-controller;
>> >> +      #interrupt-cells = <2>;
>> >> +      riscv,num-sources = <63>;
>> >> +    };
>> >> +
>> >
>> >
>> > Hi Anup,
>> > I have some thoughts regarding the APLIC DTS node. While I understand that it might be a bit late to discuss this matter within the v7 patch (sorry for this), I hope you could still consider the following idea.
>> >
>> > For example 1, my understanding is that each APLIC DTS node represents an interrupt domain. IIUC, in physical, these tree Interrupt domains should belong to an APLIC device so the M-mode IRQ domain can delegate interrupts to the child domain. Given this perspective, wrapping all interrupt domain DTS nodes with another DTS node seems to present the real scene more clearly. Maybe we can add "simple-bus" to the compatible property of this wrapped DTS node, so it still can be compatible with your driver implementations. Therefore, example 1 may become the following.
>> >
>> > interrupt-controller {
>> >         compatible = "riscv,aplics", "simple-bus";
>> >         ranges;
>> >         aplic0: interrupt-domain@c000000 {
>> >                 compatible = "qemu,aplic", "riscv,aplic";
>> >                 interrupts-extended = <&cpu1_intc 11>,
>> >                                       <&cpu2_intc 11>,
>> >                                       <&cpu3_intc 11>,
>> >                                       <&cpu4_intc 11>;
>> >                 reg = <0xc000000 0x4080>;
>> >                 interrupt-controller;
>> >                 #interrupt-cells = <2>;
>> >                 riscv,num-sources = <63>;
>> >                 riscv,children = <&aplic1>, <&aplic2>;
>> >                 riscv,delegation = <&aplic1 1 63>;
>> >         };
>> >
>> >         aplic1: interrupt-domain@d000000 {
>> >                 compatible = "qemu,aplic", "riscv,aplic";
>> >                 interrupts-extended = <&cpu1_intc 9>,
>> >                                       <&cpu2_intc 9>;
>> >                 reg = <0xd000000 0x4080>;
>> >                 interrupt-controller;
>> >                 #interrupt-cells = <2>;
>> >                 riscv,num-sources = <63>;
>> >         };
>> >
>> >         aplic2: interrupt-domain@e000000 {
>> >                 compatible = "qemu,aplic", "riscv,aplic";
>> >                 interrupts-extended = <&cpu3_intc 9>,
>> >                                       <&cpu4_intc 9>;
>> >                 reg = <0xe000000 0x4080>;
>> >                 interrupt-controller;
>> >                 #interrupt-cells = <2>;
>> >                 riscv,num-sources = <63>;
>> >         };
>> > };
>> > Is it feasible for you?
>>
>> This clubbing of APLIC domains and placing them close to each
>> other is a platform implementation choice. On multi-die or multi-socket
>> systems the APIC domains can be really far apart on different physical
>> dies so the clubbing you suggest is not always true.
>>
>> Regards,
>> Anup
>>
> IIUC, I think my suggestion might be able to apply to this scenario if these interrupt domains belong to the same APLIC device. For this scenario, one thing I am concerned about is that the MMIO region of other devices locates between interrupt domains. However, currently, I have not found that the DTS specification explicitly prohibits it. Do you have the same concern?

One APLIC device implementing multiple APLIC domains is perfectly fine
and there is nothing in this DT bindings which prevents that.

We can always group multiple APLIC domain DT nodes under another DT
node but we don't need any special DT bindings for such grouping and
the APLIC driver does not need to be aware of this grouping. Further,
the AIA spec does not mandate grouping of APLIC domains under one
APLIC device.

Like I said previously, the grouping of APLIC domains under one DT
node is implementation/platform specific.

Regards,
Anup

>
> Thanks,
> Vincent
>>
>> >
>> > Thanks,
>> > Vincent
>> >
>> >>
>> >> +  - |
>> >> +    // Example 2 (APLIC domains forwarding interrupts as MSIs):
>> >> +
>> >> +    interrupt-controller@c000000 {
>> >> +      compatible = "qemu,aplic", "riscv,aplic";
>> >> +      msi-parent = <&imsic_mlevel>;
>> >> +      reg = <0xc000000 0x4000>;
>> >> +      interrupt-controller;
>> >> +      #interrupt-cells = <2>;
>> >> +      riscv,num-sources = <63>;
>> >> +      riscv,children = <&aplic3>;
>> >> +      riscv,delegation = <&aplic3 1 63>;
>> >> +    };
>> >> +
>> >> +    aplic3: interrupt-controller@d000000 {
>> >> +      compatible = "qemu,aplic", "riscv,aplic";
>> >> +      msi-parent = <&imsic_slevel>;
>> >> +      reg = <0xd000000 0x4000>;
>> >> +      interrupt-controller;
>> >> +      #interrupt-cells = <2>;
>> >> +      riscv,num-sources = <63>;
>> >> +    };
>> >> +...
>> >> --
>> >> 2.34.1
>> >>
>> >>
>> >> _______________________________________________
>> >> linux-riscv mailing list
>> >> linux-riscv@lists.infradead.org
>> >> http://lists.infradead.org/mailman/listinfo/linux-riscv