From: Anup Patel <apatel@ventanamicro.com> To: Vivian Wang <uwu@dram.page> Cc: Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, Thomas Gleixner <tglx@linutronix.de>, Marc Zyngier <maz@kernel.org>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Atish Patra <atishp@atishpatra.org>, Alistair Francis <Alistair.Francis@wdc.com>, Anup Patel <anup@brainfault.org>, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH v2 6/9] dt-bindings: interrupt-controller: Add RISC-V advanced PLIC Date: Mon, 20 Feb 2023 10:39:56 +0530 [thread overview] Message-ID: <CAK9=C2XpGRH98MvrnwiuS7zhT8woKnnihGSinkNP1vcKz3WtRQ@mail.gmail.com> (raw) In-Reply-To: <4bd8c6da-6ad4-5e1a-169c-48f48560b36b@dram.page> On Sun, Feb 19, 2023 at 5:18 PM Vivian Wang <uwu@dram.page> wrote: > > On 1/3/23 22:14, Anup Patel 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> > > --- > > .../interrupt-controller/riscv,aplic.yaml | 159 ++++++++++++++++++ > > 1 file changed, 159 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..b7f20aad72c2 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml > > @@ -0,0 +1,159 @@ > > > > <snip> > > > > + 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 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,delegate: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + minItems: 1 > > + maxItems: 1024 > > + items: > > + items: > > + - description: child APLIC domain phandle > > + - description: first interrupt number (inclusive) > > + - description: last interrupt number (inclusive) > > + description: > > + A interrupt delegation list where each entry is a triple consisting > > + of child APLIC domain phandle, first interrupt number, and last > > + interrupt number. The firmware will configure interrupt delegation > > + registers based on interrupt delegation list. > > + > > I'm not sure if this is the right place to ask, since it could be more > of a OpenSBI/QEMU problem, but I think a more detailed description about > what 'the firmware' does is appropriate here. > > My main confusion is how to describe wired interrupts connected to > APLICs. Say we have two APLIC nodes with labels aplic_m and aplic_s that > are the APLIC domains for M-mode and S-mode respectively. IIUC, wired > interrupts are connected directly to aplic_m. So how do I refer to it in > the device nodes? Please see my previous reply to Conor about these DT properties. The riscv,children DT property describes HW child numbering whereas the riscv,delegate DT propert is a table of IRQ delegation. In your example, let's assume we have N wired interrupts. This means we will have devices connected to the root APLIC domain (aplic_m). Now since aplic_s is a child of aplic_m, we will have N wired interrupts going from from aplic_m to aplic_s where aplic_m will route a wired/device interrupt x to aplic_s if sourcecfg[x].D = 1 and sourcecfg[x].child = 0. > > 1. <&aplic_s num IRQ_TYPE_foo>, but it would be a lie to M-mode > software, which could be a problem. QEMU 7.2.0 seems to take this > approach. (I could also be misunderstanding QEMU and it actually > does connect wired interrupts to the S-mode APLIC, but then > riscv,children and riscv,delegate would be lies.) No, it's not a lie. The <&aplic_s num IRQ_TYPE_foo> in a device DT node is based on the IRQ delegation fixed by the RISC-V platform. QEMU has its own strategy of delegating IRQs to APLIC S-mode while other platforms can use a different strategy. > 2. <&aplic_m ...>, and when M-mode software gives S-mode software > access to devices, it delegates relevant interrupts and patches it > into <&aplic_s num IRQ_TYPE_foo>. Seems to be the 'correct' > approach, but pretty complicated. The APLIC M-mode domain is not accessible to S-mode software so Linux cannot create an irqdomain using APLIC M-mode DT node. This means device DT nodes must have <&aplic_s num IRQ_TYPE_foo> which points to APLIC S-mode domain. It is totally up to RISC-V firmware and platform if it wants to dynamically add/patch <&aplic_s num IRQ_TYPE_foo> in device DT nodes. Currently, we do not patch device DT nodes in OpenSBI and instead have the device DT nodes point to correct APLIC domain based on the IRQ delegation. > 3. <&aplic_m ...>, S-mode software sees this, and sees that aplic_m has > num in riscv,delegate, so goes to find the child it's been delegated > to, which is (should be) aplic_s. A bit annoyingly abstraction > breaking, since S-mode shouldn't even need to know about aplic_m. Yes, S-mode should know about aplic_m and if it tries to access aplic_m then it will get an access fault. This is exactly why device DT node should have "interrupts" DT property pointing to the actual APLIC domain which is delivering interrupt to S-mode. > > I see that others are also confused by riscv,delegate and riscv,children > properties. It would be great if we could clarify the expected behavior > here rather than just saying 'the firmware will do the thing'. Regards, Anup
WARNING: multiple messages have this Message-ID (diff)
From: Anup Patel <apatel@ventanamicro.com> To: Vivian Wang <uwu@dram.page> Cc: Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, Thomas Gleixner <tglx@linutronix.de>, Marc Zyngier <maz@kernel.org>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Atish Patra <atishp@atishpatra.org>, Alistair Francis <Alistair.Francis@wdc.com>, Anup Patel <anup@brainfault.org>, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH v2 6/9] dt-bindings: interrupt-controller: Add RISC-V advanced PLIC Date: Mon, 20 Feb 2023 10:39:56 +0530 [thread overview] Message-ID: <CAK9=C2XpGRH98MvrnwiuS7zhT8woKnnihGSinkNP1vcKz3WtRQ@mail.gmail.com> (raw) In-Reply-To: <4bd8c6da-6ad4-5e1a-169c-48f48560b36b@dram.page> On Sun, Feb 19, 2023 at 5:18 PM Vivian Wang <uwu@dram.page> wrote: > > On 1/3/23 22:14, Anup Patel 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> > > --- > > .../interrupt-controller/riscv,aplic.yaml | 159 ++++++++++++++++++ > > 1 file changed, 159 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..b7f20aad72c2 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml > > @@ -0,0 +1,159 @@ > > > > <snip> > > > > + 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 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,delegate: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + minItems: 1 > > + maxItems: 1024 > > + items: > > + items: > > + - description: child APLIC domain phandle > > + - description: first interrupt number (inclusive) > > + - description: last interrupt number (inclusive) > > + description: > > + A interrupt delegation list where each entry is a triple consisting > > + of child APLIC domain phandle, first interrupt number, and last > > + interrupt number. The firmware will configure interrupt delegation > > + registers based on interrupt delegation list. > > + > > I'm not sure if this is the right place to ask, since it could be more > of a OpenSBI/QEMU problem, but I think a more detailed description about > what 'the firmware' does is appropriate here. > > My main confusion is how to describe wired interrupts connected to > APLICs. Say we have two APLIC nodes with labels aplic_m and aplic_s that > are the APLIC domains for M-mode and S-mode respectively. IIUC, wired > interrupts are connected directly to aplic_m. So how do I refer to it in > the device nodes? Please see my previous reply to Conor about these DT properties. The riscv,children DT property describes HW child numbering whereas the riscv,delegate DT propert is a table of IRQ delegation. In your example, let's assume we have N wired interrupts. This means we will have devices connected to the root APLIC domain (aplic_m). Now since aplic_s is a child of aplic_m, we will have N wired interrupts going from from aplic_m to aplic_s where aplic_m will route a wired/device interrupt x to aplic_s if sourcecfg[x].D = 1 and sourcecfg[x].child = 0. > > 1. <&aplic_s num IRQ_TYPE_foo>, but it would be a lie to M-mode > software, which could be a problem. QEMU 7.2.0 seems to take this > approach. (I could also be misunderstanding QEMU and it actually > does connect wired interrupts to the S-mode APLIC, but then > riscv,children and riscv,delegate would be lies.) No, it's not a lie. The <&aplic_s num IRQ_TYPE_foo> in a device DT node is based on the IRQ delegation fixed by the RISC-V platform. QEMU has its own strategy of delegating IRQs to APLIC S-mode while other platforms can use a different strategy. > 2. <&aplic_m ...>, and when M-mode software gives S-mode software > access to devices, it delegates relevant interrupts and patches it > into <&aplic_s num IRQ_TYPE_foo>. Seems to be the 'correct' > approach, but pretty complicated. The APLIC M-mode domain is not accessible to S-mode software so Linux cannot create an irqdomain using APLIC M-mode DT node. This means device DT nodes must have <&aplic_s num IRQ_TYPE_foo> which points to APLIC S-mode domain. It is totally up to RISC-V firmware and platform if it wants to dynamically add/patch <&aplic_s num IRQ_TYPE_foo> in device DT nodes. Currently, we do not patch device DT nodes in OpenSBI and instead have the device DT nodes point to correct APLIC domain based on the IRQ delegation. > 3. <&aplic_m ...>, S-mode software sees this, and sees that aplic_m has > num in riscv,delegate, so goes to find the child it's been delegated > to, which is (should be) aplic_s. A bit annoyingly abstraction > breaking, since S-mode shouldn't even need to know about aplic_m. Yes, S-mode should know about aplic_m and if it tries to access aplic_m then it will get an access fault. This is exactly why device DT node should have "interrupts" DT property pointing to the actual APLIC domain which is delivering interrupt to S-mode. > > I see that others are also confused by riscv,delegate and riscv,children > properties. It would be great if we could clarify the expected behavior > here rather than just saying 'the firmware will do the thing'. Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2023-02-20 5:10 UTC|newest] Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-01-03 14:14 [PATCH v2 0/9] Linux RISC-V AIA Support Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 1/9] RISC-V: Add AIA related CSR defines Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-04 23:07 ` Conor Dooley 2023-01-04 23:07 ` Conor Dooley 2023-01-09 5:09 ` Anup Patel 2023-01-09 5:09 ` Anup Patel 2023-01-17 20:42 ` Conor Dooley 2023-01-17 20:42 ` Conor Dooley 2023-01-27 11:58 ` Anup Patel 2023-01-27 11:58 ` Anup Patel 2023-01-27 14:20 ` Conor Dooley 2023-01-27 14:20 ` Conor Dooley 2023-01-03 14:14 ` [PATCH v2 2/9] RISC-V: Detect AIA CSRs from ISA string Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 3/9] irqchip/riscv-intc: Add support for RISC-V AIA Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-13 9:39 ` Marc Zyngier 2023-01-13 9:39 ` Marc Zyngier 2023-01-03 14:14 ` [PATCH v2 4/9] dt-bindings: interrupt-controller: Add RISC-V incoming MSI controller Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-04 23:21 ` Conor Dooley 2023-01-04 23:21 ` Conor Dooley 2023-02-20 3:15 ` Anup Patel 2023-02-20 3:15 ` Anup Patel 2023-01-12 20:49 ` Rob Herring 2023-01-12 20:49 ` Rob Herring 2023-02-20 3:20 ` Anup Patel 2023-02-20 3:20 ` Anup Patel 2023-02-19 11:17 ` Vivian Wang 2023-02-19 11:17 ` Vivian Wang 2023-02-20 3:31 ` Anup Patel 2023-02-20 3:31 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 5/9] irqchip: Add RISC-V incoming MSI controller driver Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-13 10:10 ` Marc Zyngier 2023-01-13 10:10 ` Marc Zyngier 2023-05-01 8:28 ` Anup Patel 2023-05-01 8:28 ` Anup Patel 2023-05-01 8:44 ` Marc Zyngier 2023-05-01 8:44 ` Marc Zyngier [not found] ` <CAPqJEFqhd-=-RYepKqnco7HySoxk7AhEctL+vzNozMSWe0mv7A@mail.gmail.com> [not found] ` <CABvJ_xhcuC92A_oo1mWQoRvtRzE8XXx9bbXKs7N7wKm0=Z3_Cw@mail.gmail.com> 2023-01-18 3:49 ` Fwd: " Vincent Chen 2023-01-18 3:49 ` Vincent Chen 2023-01-18 4:20 ` Anup Patel 2023-01-18 4:20 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 6/9] dt-bindings: interrupt-controller: Add RISC-V advanced PLIC Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-04 22:16 ` Conor Dooley 2023-01-04 22:16 ` Conor Dooley 2023-02-20 4:36 ` Anup Patel 2023-02-20 4:36 ` Anup Patel 2023-02-20 10:32 ` Conor Dooley 2023-02-20 10:32 ` Conor Dooley 2023-02-20 10:56 ` Conor Dooley 2023-02-20 10:56 ` Conor Dooley 2023-01-12 21:02 ` Rob Herring 2023-01-12 21:02 ` Rob Herring 2023-02-19 11:48 ` Vivian Wang 2023-02-19 11:48 ` Vivian Wang 2023-02-20 5:09 ` Anup Patel [this message] 2023-02-20 5:09 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 7/9] irqchip: Add RISC-V advanced PLIC driver Anup Patel 2023-01-03 14:14 ` Anup Patel [not found] ` <CAPqJEFpmAvWiOdackxYwSPBfjo4DnTHXrXVSCC4snMn8tnZXPw@mail.gmail.com> [not found] ` <CABvJ_xhjMa8xTsO-Qa23TOqxPpYxyBYSfV6TmKney-Gp3oi8cA@mail.gmail.com> 2023-01-17 7:09 ` Fwd: " Vincent Chen 2023-01-17 7:09 ` Vincent Chen 2023-01-18 4:37 ` Anup Patel 2023-01-18 4:37 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 8/9] RISC-V: Select APLIC and IMSIC drivers Anup Patel 2023-01-03 14:14 ` Anup Patel 2023-01-03 14:14 ` [PATCH v2 9/9] MAINTAINERS: Add entry for RISC-V AIA drivers Anup Patel 2023-01-03 14:14 ` Anup Patel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAK9=C2XpGRH98MvrnwiuS7zhT8woKnnihGSinkNP1vcKz3WtRQ@mail.gmail.com' \ --to=apatel@ventanamicro.com \ --cc=Alistair.Francis@wdc.com \ --cc=anup@brainfault.org \ --cc=atishp@atishpatra.org \ --cc=devicetree@vger.kernel.org \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=maz@kernel.org \ --cc=palmer@dabbelt.com \ --cc=paul.walmsley@sifive.com \ --cc=robh+dt@kernel.org \ --cc=tglx@linutronix.de \ --cc=uwu@dram.page \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.