| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<OS8PR06MB7541BABFC1B2414F4FA26446F21FA@OS8PR06MB7541.apcprd06.prod.outlook.com>
Date: Thu, 25 Sep 2025 07:07:44 +0000
From: Ryan Chen <ryan_chen@...eedtech.com>
To: Andrew Jeffery <andrew@...econstruct.com.au>, Krzysztof Kozlowski
<krzk@...nel.org>
CC: Thomas Gleixner <tglx@...utronix.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Joel Stanley <joel@....id.au>, Kevin Chen <kevin_chen@...eedtech.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-aspeed@...ts.ozlabs.org"
<linux-aspeed@...ts.ozlabs.org>
Subject: RE: [PATCH v4 1/2] dt-bindings: interrupt-controller: aspeed: Add
parent compatibles and refine documentation
> Subject: Re: [PATCH v4 1/2] dt-bindings: interrupt-controller: aspeed: Add
> parent compatibles and refine documentation
>
> On Thu, 2025-08-14 at 10:03 +0200, Krzysztof Kozlowski wrote:
> > On Tue, Aug 12, 2025 at 06:08:29PM +0800, Ryan Chen wrote:
> > > AST2700 contains two independent top-level interrupt controllers
> > > (INTC0, INTC1). Each occupies its own register space and handles
> > > different sets of peripherals. Above them, the PSP (CA35) GIC is the
> > > root interrupt aggregator. In hardware, INTC1 outputs are routed
> > > into INTC0, and INTC0 outputs are routed into the GIC.
> > >
> > > Introduce distinct compatibles for these parent blocks so the DT can
> > > model the hierarchy and register space layout accurately:
> > >
> > > - aspeed,ast2700-intc0 (parent node at 0x12100000)
> > > - aspeed,ast2700-intc1 (parent node at 0x14c18000)
> > >
> > > The existing child compatible:
> > >
> > > - aspeed,ast2700-intc-ic
> > >
> > > continues to describe the interrupt-controller instances within each
> > > INTC block (e.g. INTC0_0..INTC0_11 and INTC1_0..INTC1_5).
> > >
> > > Signed-off-by: Ryan Chen <ryan_chen@...eedtech.com>
> > > ---
> > > .../aspeed,ast2700-intc.yaml | 158
> > > +++++++++++++-----
> > > 1 file changed, 115 insertions(+), 43 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/interrupt-
> > > controller/aspeed,ast2700-intc.yaml
> > > b/Documentation/devicetree/bindings/interrupt-
> > > controller/aspeed,ast2700-intc.yaml
> > > index 55636d06a674..81304b53c112 100644
> > > ---
> > > a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2
> > > 700-intc.yaml
> > > +++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,
> > > +++ ast2700-intc.yaml
> > > @@ -10,6 +10,33 @@ description:
> > > This interrupt controller hardware is second level interrupt
> > > controller that
> > > is hooked to a parent interrupt controller. It's useful to
> > > combine multiple
> > > interrupt sources into 1 interrupt to parent interrupt controller.
> > > + Depend to which INTC0 or INTC1 used.
> > > + INTC0 and INTC1 are two kinds of interrupt controller with enable
> > > +and raw
> > > + status registers for use.
> > > + INTC0 is used to assert GIC if interrupt in INTC1 asserted.
> > > + INTC1 is used to assert INTC0 if interrupt of modules asserted.
> > > + +-----+ +---------+
> > > + | GIC |---| INTC0 |
> > > + +-----+ +---------+
> >
> > Same problem as last time. This tells me intc0 has not children...
> >
> > > + +---------+
> > > + | |---module0
> > > + | INTC0_0 |---module1
> > > + | |---...
> > > + +---------+---module31
> > > + |---.... |
> > > + +---------+
> > > + | | +---------+
> > > + | INTC0_11| +---| INTC1 |
> > > + | | +---------+
> >
> > ...This tells that inc1 has no children (only intc0_11, which you said
> > is aspeed,ast2700-intc-ic !!!)....
> > (keep scrolling)
> >
> > > + +---------+ +---------+---module0
> > > + | INTC1_0 |---module1
> > > + | |---...
> > > + +---------+---module31
> > > + ...
> > > + +---------+---module0
> > > + | INTC1_5 |---module1
> > > + | |---...
> > > + +---------+---module31
> > >
>
> I've taken a look at the datasheet and had a bit of a think about how to make
> progress here.
>
> My feeling is this diagram (and the previous one) undersells the complexity of
> the design by quite some margin. It's probably best to start by zooming out
> quite a lot:
>
> The AST2700 SoC contains (at least) 4 distinct processors:
>
> 1. The quad-core ARM Cortex-A35 (PSP: Primary Service Processor) 2. A
> Cortex-M4 (SSP: Secondary Service Processor) 3. Another Cortex-M4 (TSP:
> Tertiary Service Processor) 4. The BootMCU - a RISC-V processor to execute
> the mask ROM
>
> While the PSP GIC shown in the diagram above is one possible interrupt
> destination, many of the 480 interrupt sources in the package can be routed to
> the dedicated interrupt controller of any of these four processors. Likewise,
> many peripherals of the SoC are mapped into the physical address space of
> each processor. The routing is handled by the two interrupt controller blocks
> described in the binding text above:
>
> +--------------------+
> | +-------+ +------+ |
> | | | | | |
> | | PSP +-+ GIC <-+----------+
> | | | | | | |
> | +-------+ +------+ | |
> +--------------------+ |
> |
> +--------------------+ |
> | +-------+ +------+ | | +-------+
> | | | | | | | | | INTx
> | | SSP +-+ NVIC <-+----------+----------| INTC0 <-----------
> | | | | | | | | |
> | +-------+ +------+ | | +---^---+
> +--------------------+ | |
> | |
> +--------------------+ | |
> | +-------+ +------+ | | |
> | | | | | | | |
> | | TSP +-+ NVIC <-+----------+ |
> | | | | | | |
> | +-------+ +------+ | |
> +--------------------+ |
> |
> +--------------------+ |
> | +------+ +-------+ | +---+---+
> | | | | | | | | INTy
> | | BMCU +-+ APLIC <-+---------------------+ INTC1 <-----------
> | | | | | | | |
> | +------+ +-------+ | +-------+
> +--------------------+
>
> There's a split in the interrupt domain: 0 <= INTx <= 127 < INTy.
>
> The PSP GIC, SSP NVIC and TSP NVIC destinations for each INTx source is
> selected by a corresponding mux in INTC0.
>
> The destination for each INTy source is selected by a corresponding mux in
> INTC1, where the possible destinations are:
>
> 1. A shared interrupt line Ma, routed through INTC0 to the PSP GIC 2. A mux in
> INTC0, providing a second level of indirection 3. A shared interrupt line Md,
> routed through INTC0 to the PSP GIC 4. A shared interrupt line Me, routed
> through INTC0 to the PSP GIC 5. A shared interrupt line Mb, routed through
> INTC0 to the SSP NVIC 6. A shared interrupt line Mc, routed through INTC0 to
> the TSP NVIC 7. The BootMCU APLIC
>
> Each shared interrupt line from INTC1 to INTC0 merges up to 32 interrupt
> sources.
>
> INTC0 and INTC1 are both mapped into the physical address space of each
> processor. However, to prevent any one of them interfering with interrupts
> dedicated to another, the controller's register map is divided into distinct
> regions whose access is constrained to a specific processor. The enable and
> status registers in INTC0 for shared line sets Ma, Md and Me are constrained to
> the PSP, Mb to the SSP, and Mc to the TSP.
>
> +-------+ +-------+
> | INTC0 | | INTC1 |
> - - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- - - -+- -
> +--------------------+ | | | |
> | +-------+ +------+ | | | | |
> | | | | | | | | | |
> | | PSP +-+ GIC | | | | | |
> | | | | | | | | | |
> | +-------+ | | | | | | |
> | | | | +-------+ | |
> | | 192 <-|-----------+ Ma <------------|---+ |
> | | | | +-------+ | | |
> | | 208 <-|-----------+ Md <------------|---+ |
> | | | | +-------+ | | |
> | | 224 <-|-----------+ Me <------------|---+ |
> | | | | +-------+ | | |
> | +------+ | | | | | |
> +--------------------+ | | | | |
> - - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- -|- -+- -
> +--------------------+ | | | | |
> | +-------+ +------+ | +-------+ | +---|------------
> | | SSP +-+ NVIC <-|-----------+ Mb <------------|---+ | INT128
> | +-------+ +------+ | +-------+ | | |
> +--------------------+ | | | | |
> - - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- -|- -+- -
> +--------------------+ | | | | |
> | +-------+ +------+ | +-------+ | | |
> | | TSP +-+ NVIC <-|-----------+ Mc <------------|---+ |
> | +-------+ +------+ | +-------+ | |
> +--------------------+ | | | |
> - - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- - - -+- -
> +--------------------+ | | | |
> | +------+ +-------+ | | | | |
> | | BMCU +-+ APLIC | | | | | |
> | +------+ +-------+ | | | | |
> +--------------------+ | | | |
> - - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- - - -+- -
> +-------+ +-------+
>
>
> INTC1 mux destination 2 above allows the PSP to reconfigure the routes at
> runtime at INTC0, rather than submit to a route that might've been protected
> by firmware early in the boot chain, so long as the boot firmware chose the
> appropriate route in INTC1. INTC0 merges these interrupts in sets of 32 and
> operates in much the same way as INTC1 in the diagram above, though cannot
> route to the BootMCU.
>
> I'd prefer we acknowledge all this in the binding, and do enough to allow the
> kernel to configure routing as it wishes. While in some applications the routes
> may be constrained by firmware, the platform- specific portion of the
> devicetree can be written reflect this.
>
> From some experimentation this shouldn't require anything bespoke.
> Using interrupts-extended we can represent the route configuration with a
> phandle to the upstream controller on the cascaded node. To avoid some
> arbitrary interrupt index choices at the node for INTC0, I think it's worth
> describing the register sets for interrupt sets Ma, Md and Me as subnodes of
> INTC0 with their own interrupt resources. This feels reasonably tidy, as the
> selection of the Ma, Md or Me sets completely determines its ultimate index
> at the PSP GIC. Doing so also removes them from needing to be described if
> any changes to some default route configuration are required by the platform,
> necessitating overriding the interrupts-extended property of the INTC0 node.
>
> The only curiosity of this approach is that the interrupt-controller nodes for the
> non-PSP processors need to be described so we can reference them via
> phandles for the purpose of routing the interrupts.
> As these controllers are not mapped in the physical address space of the PSP
> we need the devicetree to inform the kernel as much.
>
> Here's an example pseudo-devicetree. Of course there are elements that need
> more work, but I feel we can mine it for parts.
>
> Cheers,
>
> Andrew
Hello Andrew,
Thanks a lot for diagram draw. I agree with your point.
The binding must reflect the full routing capability of INTC0/INTC1,
not just a simple parent/child split.
So that the reason I add a driver for routing in "aspeed,ast2700-intc0/1".
https://lore.kernel.org/all/20250812100830.145578-3-ryan_chen@aspeedtech.com/
- aspeed,ast2700-intc0 (parent node at 0x12100000) - routing driver included
- aspeed,ast2700-intc-ic 0x1b00
- aspeed,ast2700-intc-ic 0x1b10
- aspeed,ast2700-intc-ic 0x1b20
- aspeed,ast2700-intc1 (parent node at 0x14c18000) - routing driver included
- aspeed,ast2700-intc-ic 0x100
- aspeed,ast2700-intc-ic 0x110
- aspeed,ast2700-intc-ic 0x120
- aspeed,ast2700-intc-ic 0x130
- aspeed,ast2700-intc-ic 0x140
- aspeed,ast2700-intc-ic 0x150
Ryan Chen
>
> / {
> primary {
> compatible = "simple-bus";
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
>
> intc0: interrupt-controller@...00000 {
> compatible = "aspeed,ast2700-intc0-a1";
> reg = <0x12100000 0x1b00>;
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
> interrupt-controller;
> #interrupt-cells = <2>;
> interrupts-extended =
> #if GIC
> /* GICINT0 */ <&gic GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> #elif SSP
> /* SSPINT0 */ <&ssp_nvic 0 0>,
> #else /* TSP */
> /* TSPINT0 */ <&tsp_nvic 0 0>,
> #endif
>
> /* ... */
>
> #if GIC_128 /* Route merged 128-159 interrupts to GICINT128 */
> /* 128 */ <&gic GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>, #elif GIC_160
> /* Route merged 128-159 interrupts to GICINT160 */
> /* 128 */ <&gic GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>, #elif GIC_176
> /* Route merged 128-159 interrupts to GICINT176 */
> /* 128 */ <&gic GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>, #elif SSP
> /* 128 */ <&ssp_nvic 128 0>,
> #else /* TSP */
> /* 128 */ <&tsp_nvic 128 0>,
> #endif
>
> /* ... */
>
> /* 186 */ <&gic 186 0>;
>
> intcm0: interrupt-controller@...01b00 {
> compatible = "aspeed,ast2700-intcm-a1";
> reg = <0x12101b00 0x10>;
> interrupt-controller;
> #interrupt-cells = <2>;
> interrupt-parent = <&gic>;
> interrupts =
> <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 201 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> intcm30: interrupt-controller@...01b10 {
> compatible = "aspeed,ast2700-intcm-a1";
> reg = <0x12101b10 0x10>;
> interrupt-controller;
> #interrupt-cells = <2>;
> interrupt-parent = <&gic>;
> interrupts =
> <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 211 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 212 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 213 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 214 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 215 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 216 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 217 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> intcm40: interrupt-controller@...01b20 {
> compatible = "aspeed,ast2700-intcm-a1";
> reg = <0x12101b20 0x10>;
> interrupt-controller;
> #interrupt-cells = <2>;
> interrupt-parent = <&gic>;
> interrupts =
> <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 227 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 228 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 230 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 231 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 232 IRQ_TYPE_LEVEL_HIGH>;
> <GIC_SPI 233 IRQ_TYPE_LEVEL_HIGH>;
> };
> };
>
> intc1: interrupt-controller@...18000 {
> compatible = "aspeed,ast2700-intc1-a1";
> reg = <0x14c18000 0x1000>;
> interrupt-controller;
> #interrupt-cells = <2>;
> interrupts-extended =
> #if M0
> /* 0 */ <&intcm0 0 IRQ_TYPE_LEVEL_HIGH>,
> #elif C0
> /* 0 */ <&intc0 128 IRQ_TYPE_LEVEL_HIGH>,
> #elif M10
> /* 0 */ <&ssp_nvic 160 0>,
> #elif M20
> /* 0 */ <&tsp_nvic 160 0>,
> #elif M30
> /* 0 */ <&intcm30 0 IRQ_TYPE_LEVEL_HIGH>,
> #elif M40
> /* 0 */ <&intcm40 0 IRQ_TYPE_LEVEL_HIGH>,
> #else /* B */
> /* 0 */ <&aplic 128 IRQ_TYPE_LEVEL_HIGH>,
> #endif
> /* ... */
> };
>
> vuart1: serial@...87000 {
> compatible = "aspeed,ast2700-vuart";
> reg = <0x14c30000 0x1000>;
> reg-shift = <2>;
> interrupts-extended = <&intc1 17 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> gic: interrupt-controller@...01000 {
> compatible = "arm,gic-400";
> #interrupt-cells = <3>;
> #address-cells = <0>;
> interrupt-controller;
> reg = <0x0 0xfff01000 0 0x1000>,
> <0x0 0xfff02000 0 0x2000>,
> <0x0 0xfff04000 0 0x2000>,
> <0x0 0xfff06000 0 0x2000>;
> interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>;
> };
> };
>
> secondary {
> #address-cells = <2>;
> /*
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/
> of/address.c?h=v6.16#n491 */
> #size-cells = <0>;
> /*
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/
> of/address.c?h=v6.16#n430 */
>
> ssp_nvic: interrupt-controller@1,e000e100 {
> compatible = "arm,v7m-nvic";
> #interrupt-cells = <2>;
> #address-cells = <0>;
> interrupt-controller;
> reg = <1 0xe000e100>;
> };
> };
>
> tertiary {
> #address-cells = <2>;
> #size-cells = <0>;
>
> tsp_nvic: interrupt-controller@2,e000e100 {
> compatible = "arm,v7m-nvic";
> #interrupt-cells = <2>;
> #address-cells = <0>;
> interrupt-controller;
> reg = <2 0xe000e100>;
> };
> };
>
> bootmcu {
> #address-cells = <2>;
> #size-cells = <0>;
>
> aplic1: interrupt-controller@3,d000000 {
> compatible = "riscv,aplic";
> interrupts-extended = <&cpu1_intc 9>,
> reg = <3 0xd000000>;
> interrupt-controller;
> #interrupt-cells = <2>;
> riscv,num-sources = <480>;
> };
> };
> };
Powered by blists - more mailing lists