The GICv5 interrupt controller architecture is composed of:

- one or more Interrupt Routing Service (IRS)
- zero or more Interrupt Translation Service (ITS)
- zero or more Interrupt Wire Bridge (IWB)

Describe a GICv5 implementation by specifying a top level node
corresponding to the GICv5 system component.

IRS nodes are added as GICv5 system component children.

An ITS is associated with an IRS so ITS nodes are described
as IRS children - use the hierarchy explicitly in the device
tree to define the association.

IWB nodes are described as a separate schema.

An IWB is connected to a single ITS, the connection is made explicit
through the msi-parent property and therefore is not required to be
explicit through a parent-child relationship in the device tree.

Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
---
 .../interrupt-controller/arm,gic-v5-iwb.yaml       |  78 ++++++++
 .../bindings/interrupt-controller/arm,gic-v5.yaml  | 202 +++++++++++++++++++++
 MAINTAINERS                                        |   7 +
 3 files changed, 287 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v5-iwb.yaml b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v5-iwb.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..99a266a62385a35421b5468045152414196bf42d
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v5-iwb.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/arm,gic-v5-iwb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM Generic Interrupt Controller, version 5 Interrupt Wire Bridge (IWB)
+
+maintainers:
+  - Lorenzo Pieralisi <lpieralisi@kernel.org>
+  - Marc Zyngier <maz@kernel.org>
+
+description: |
+  The GICv5 architecture defines the guidelines to implement GICv5
+  compliant interrupt controllers for AArch64 systems.
+
+  The GICv5 specification can be found at
+  https://developer.arm.com/documentation/aes0070
+
+  GICv5 has zero or more Interrupt Wire Bridges (IWB) that are responsible
+  for translating wire signals into interrupt messages to the GICv5 ITS.
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    const: arm,gic-v5-iwb
+
+  reg:
+    items:
+      - description: IWB control frame
+
+  "#address-cells":
+    const: 0
+
+  "#interrupt-cells":
+    description: |
+      The 1st cell corresponds to the IWB wire.
+
+      The 2nd cell is the flags, encoded as follows:
+      bits[3:0] trigger type and level flags.
+
+      1 = low-to-high edge triggered
+      2 = high-to-low edge triggered
+      4 = active high level-sensitive
+      8 = active low level-sensitive
+
+    const: 2
+
+  interrupt-controller: true
+
+  msi-parent:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - "#interrupt-cells"
+  - interrupt-controller
+  - msi-parent
+
+additionalProperties: false
+
+examples:
+  - |
+    interrupt-controller@2f000000 {
+      compatible = "arm,gic-v5-iwb";
+      reg = <0x2f000000 0x10000>;
+
+      #address-cells = <0>;
+
+      #interrupt-cells = <2>;
+      interrupt-controller;
+
+      msi-parent = <&its0 64>;
+    };
+...
diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v5.yaml b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v5.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..c8d124c3aa63fd1ec24acb40de72ac2164adeebd
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v5.yaml
@@ -0,0 +1,202 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/arm,gic-v5.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM Generic Interrupt Controller, version 5
+
+maintainers:
+  - Lorenzo Pieralisi <lpieralisi@kernel.org>
+  - Marc Zyngier <maz@kernel.org>
+
+description: |
+  The GICv5 architecture defines the guidelines to implement GICv5
+  compliant interrupt controllers for AArch64 systems.
+
+  The GICv5 specification can be found at
+  https://developer.arm.com/documentation/aes0070
+
+  The GICv5 architecture is composed of multiple components:
+    - one or more IRS (Interrupt Routing Service)
+    - zero or more ITS (Interrupt Translation Service)
+
+  The architecture defines:
+    - PE-Private Peripheral Interrupts (PPI)
+    - Shared Peripheral Interrupts (SPI)
+    - Logical Peripheral Interrupts (LPI)
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    const: arm,gic-v5
+
+  "#address-cells":
+    enum: [ 1, 2 ]
+
+  "#size-cells":
+    enum: [ 1, 2 ]
+
+  ranges: true
+
+  "#interrupt-cells":
+    description: |
+      The 1st cell corresponds to the INTID.Type field in the INTID; 1 for PPI,
+      3 for SPI. LPI interrupts must not be described in the bindings since
+      they are allocated dynamically by the software component managing them.
+
+      The 2nd cell contains the interrupt INTID.ID field.
+
+      The 3rd cell is the flags, encoded as follows:
+      bits[3:0] trigger type and level flags.
+
+        1 = low-to-high edge triggered
+        2 = high-to-low edge triggered
+        4 = active high level-sensitive
+        8 = active low level-sensitive
+
+    const: 3
+
+  interrupt-controller: true
+
+  interrupts:
+    description:
+      The VGIC maintenance interrupt.
+    maxItems: 1
+
+required:
+  - compatible
+  - "#address-cells"
+  - "#size-cells"
+  - ranges
+  - "#interrupt-cells"
+  - interrupt-controller
+
+patternProperties:
+  "^irs@[0-9a-f]+$":
+    type: object
+    description:
+      GICv5 has one or more Interrupt Routing Services (IRS) that are
+      responsible for handling IRQ state and routing.
+
+    additionalProperties: false
+
+    properties:
+      compatible:
+        const: arm,gic-v5-irs
+
+      reg:
+        minItems: 1
+        items:
+          - description: IRS control frame
+          - description: IRS setlpi frame
+
+      "#address-cells":
+        enum: [ 1, 2 ]
+
+      "#size-cells":
+        enum: [ 1, 2 ]
+
+      ranges: true
+
+      dma-noncoherent:
+        description:
+          Present if the GIC IRS permits programming shareability and
+          cacheability attributes but is connected to a non-coherent
+          downstream interconnect.
+
+      cpus:
+        description:
+          CPUs managed by the IRS.
+
+      arm,iaffids:
+        $ref: /schemas/types.yaml#/definitions/uint16-array
+        description:
+          Interrupt AFFinity ID (IAFFID) associated with the CPU whose
+          CPU node phandle is at the same index in the cpus array.
+
+    patternProperties:
+      "^msi-controller@[0-9a-f]+$":
+        type: object
+        description:
+          GICv5 has zero or more Interrupt Translation Services (ITS) that are
+          used to route Message Signalled Interrupts (MSI) to the CPUs. Each
+          ITS is connected to an IRS.
+        additionalProperties: false
+
+        properties:
+          compatible:
+            const: arm,gic-v5-its
+
+          reg:
+            items:
+              - description: ITS control frame
+              - description: ITS translate frame
+
+          dma-noncoherent:
+            description:
+              Present if the GIC ITS permits programming shareability and
+              cacheability attributes but is connected to a non-coherent
+              downstream interconnect.
+
+          "#msi-cells":
+            description:
+              The single msi-cell is the DeviceID of the device which will
+              generate the MSI.
+            const: 1
+
+          msi-controller: true
+
+        required:
+          - compatible
+          - reg
+          - "#msi-cells"
+          - msi-controller
+
+    required:
+      - compatible
+      - reg
+      - cpus
+      - arm,iaffids
+
+additionalProperties: false
+
+examples:
+  - |
+    interrupt-controller {
+      compatible = "arm,gic-v5";
+
+      #interrupt-cells = <3>;
+      interrupt-controller;
+
+      #address-cells = <1>;
+      #size-cells = <1>;
+      ranges;
+
+      interrupts = <1 25 4>;
+
+      irs@2f1a0000 {
+        compatible = "arm,gic-v5-irs";
+        reg = <0x2f1a0000 0x10000>;  // IRS_CONFIG_FRAME for NS
+
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges;
+
+        cpus = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>, <&cpu4>, <&cpu5>, <&cpu6>, <&cpu7>;
+        arm,iaffids = /bits/ 16 <0 1 2 3 4 5 6 7>;
+
+        msi-controller@2f120000 {
+          compatible = "arm,gic-v5-its";
+          reg = <0x2f120000 0x10000>,   // ITS_CONFIG_FRAME for NS
+                <0x2f130000 0x10000>;   // ITS_TRANSLATE_FRAME
+
+          #msi-cells = <1>;
+          msi-controller;
+
+        };
+      };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 69511c3b2b76fb7090a2a550f4c59a7daf188493..d51efac8f9aa21629a0486977fdc76a2eaf5c52f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1901,6 +1901,13 @@ F:	drivers/irqchip/irq-gic*.[ch]
 F:	include/linux/irqchip/arm-gic*.h
 F:	include/linux/irqchip/arm-vgic-info.h
 
+ARM GENERIC INTERRUPT CONTROLLER V5 DRIVERS
+M:	Lorenzo Pieralisi <lpieralisi@kernel.org>
+M:	Marc Zyngier <maz@kernel.org>
+L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
+S:	Maintained
+F:	Documentation/devicetree/bindings/interrupt-controller/arm,gic-v5*.yaml
+
 ARM HDLCD DRM DRIVER
 M:	Liviu Dudau <liviu.dudau@arm.com>
 S:	Supported

-- 
2.48.0

[+Andre, Peter]

On Tue, May 13, 2025 at 07:47:54PM +0200, Lorenzo Pieralisi wrote:

[...]

> diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v5.yaml b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v5.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..c8d124c3aa63fd1ec24acb40de72ac2164adeebd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v5.yaml
> @@ -0,0 +1,202 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/arm,gic-v5.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARM Generic Interrupt Controller, version 5
> +
> +maintainers:
> +  - Lorenzo Pieralisi <lpieralisi@kernel.org>
> +  - Marc Zyngier <maz@kernel.org>
> +
> +description: |
> +  The GICv5 architecture defines the guidelines to implement GICv5
> +  compliant interrupt controllers for AArch64 systems.
> +
> +  The GICv5 specification can be found at
> +  https://developer.arm.com/documentation/aes0070
> +
> +  The GICv5 architecture is composed of multiple components:
> +    - one or more IRS (Interrupt Routing Service)
> +    - zero or more ITS (Interrupt Translation Service)
> +
> +  The architecture defines:
> +    - PE-Private Peripheral Interrupts (PPI)
> +    - Shared Peripheral Interrupts (SPI)
> +    - Logical Peripheral Interrupts (LPI)
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: arm,gic-v5
> +
> +  "#address-cells":
> +    enum: [ 1, 2 ]
> +
> +  "#size-cells":
> +    enum: [ 1, 2 ]
> +
> +  ranges: true
> +
> +  "#interrupt-cells":
> +    description: |
> +      The 1st cell corresponds to the INTID.Type field in the INTID; 1 for PPI,
> +      3 for SPI. LPI interrupts must not be described in the bindings since
> +      they are allocated dynamically by the software component managing them.
> +
> +      The 2nd cell contains the interrupt INTID.ID field.
> +
> +      The 3rd cell is the flags, encoded as follows:
> +      bits[3:0] trigger type and level flags.
> +
> +        1 = low-to-high edge triggered
> +        2 = high-to-low edge triggered
> +        4 = active high level-sensitive
> +        8 = active low level-sensitive
> +
> +    const: 3
> +
> +  interrupt-controller: true
> +
> +  interrupts:
> +    description:
> +      The VGIC maintenance interrupt.
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - "#address-cells"
> +  - "#size-cells"
> +  - ranges
> +  - "#interrupt-cells"
> +  - interrupt-controller
> +
> +patternProperties:
> +  "^irs@[0-9a-f]+$":
> +    type: object
> +    description:
> +      GICv5 has one or more Interrupt Routing Services (IRS) that are
> +      responsible for handling IRQ state and routing.
> +
> +    additionalProperties: false
> +
> +    properties:
> +      compatible:
> +        const: arm,gic-v5-irs
> +
> +      reg:
> +        minItems: 1
> +        items:
> +          - description: IRS control frame

I came across it while testing EL3 firmware, raising the topic for
discussion.

The IRS (and the ITS) has a config frame (need to patch the typo
s/control/config, already done) per interrupt domain supported, that is,
it can have up to 4 config frames:

- EL3
- Secure
- Realm
- Non-Secure

The one described in this binding is the non-secure one.

IIUC, everything described in the DT represents the non-secure address
space. Two questions:

- I don't have to spell out the IRS/ITS config frame (and SETLPI, by
  the way) as non-secure, since that's implicit, is that correct ?
- How can the schema describe, if present, EL3, Secure and Realm frames ?

It would be good if this schema could be reused in firmware to describe
the platform, for that to happen we need to have the questions above
resolved.

Thanks,
Lorenzo

> +          - description: IRS setlpi frame
> +
> +      "#address-cells":
> +        enum: [ 1, 2 ]
> +
> +      "#size-cells":
> +        enum: [ 1, 2 ]
> +
> +      ranges: true
> +
> +      dma-noncoherent:
> +        description:
> +          Present if the GIC IRS permits programming shareability and
> +          cacheability attributes but is connected to a non-coherent
> +          downstream interconnect.
> +
> +      cpus:
> +        description:
> +          CPUs managed by the IRS.
> +
> +      arm,iaffids:
> +        $ref: /schemas/types.yaml#/definitions/uint16-array
> +        description:
> +          Interrupt AFFinity ID (IAFFID) associated with the CPU whose
> +          CPU node phandle is at the same index in the cpus array.
> +
> +    patternProperties:
> +      "^msi-controller@[0-9a-f]+$":
> +        type: object
> +        description:
> +          GICv5 has zero or more Interrupt Translation Services (ITS) that are
> +          used to route Message Signalled Interrupts (MSI) to the CPUs. Each
> +          ITS is connected to an IRS.
> +        additionalProperties: false
> +
> +        properties:
> +          compatible:
> +            const: arm,gic-v5-its
> +
> +          reg:
> +            items:
> +              - description: ITS control frame
> +              - description: ITS translate frame
> +
> +          dma-noncoherent:
> +            description:
> +              Present if the GIC ITS permits programming shareability and
> +              cacheability attributes but is connected to a non-coherent
> +              downstream interconnect.
> +
> +          "#msi-cells":
> +            description:
> +              The single msi-cell is the DeviceID of the device which will
> +              generate the MSI.
> +            const: 1
> +
> +          msi-controller: true
> +
> +        required:
> +          - compatible
> +          - reg
> +          - "#msi-cells"
> +          - msi-controller
> +
> +    required:
> +      - compatible
> +      - reg
> +      - cpus
> +      - arm,iaffids
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    interrupt-controller {
> +      compatible = "arm,gic-v5";
> +
> +      #interrupt-cells = <3>;
> +      interrupt-controller;
> +
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +      ranges;
> +
> +      interrupts = <1 25 4>;
> +
> +      irs@2f1a0000 {
> +        compatible = "arm,gic-v5-irs";
> +        reg = <0x2f1a0000 0x10000>;  // IRS_CONFIG_FRAME for NS
> +
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges;
> +
> +        cpus = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>, <&cpu4>, <&cpu5>, <&cpu6>, <&cpu7>;
> +        arm,iaffids = /bits/ 16 <0 1 2 3 4 5 6 7>;
> +
> +        msi-controller@2f120000 {
> +          compatible = "arm,gic-v5-its";
> +          reg = <0x2f120000 0x10000>,   // ITS_CONFIG_FRAME for NS
> +                <0x2f130000 0x10000>;   // ITS_TRANSLATE_FRAME
> +
> +          #msi-cells = <1>;
> +          msi-controller;
> +
> +        };
> +      };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 69511c3b2b76fb7090a2a550f4c59a7daf188493..d51efac8f9aa21629a0486977fdc76a2eaf5c52f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1901,6 +1901,13 @@ F:	drivers/irqchip/irq-gic*.[ch]
>  F:	include/linux/irqchip/arm-gic*.h
>  F:	include/linux/irqchip/arm-vgic-info.h
>  
> +ARM GENERIC INTERRUPT CONTROLLER V5 DRIVERS
> +M:	Lorenzo Pieralisi <lpieralisi@kernel.org>
> +M:	Marc Zyngier <maz@kernel.org>
> +L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/interrupt-controller/arm,gic-v5*.yaml
> +
>  ARM HDLCD DRM DRIVER
>  M:	Liviu Dudau <liviu.dudau@arm.com>
>  S:	Supported
> 
> -- 
> 2.48.0
>

On Thu, 29 May 2025 at 13:44, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
>
> [+Andre, Peter]
>
> On Tue, May 13, 2025 at 07:47:54PM +0200, Lorenzo Pieralisi wrote:
> > +      reg:
> > +        minItems: 1
> > +        items:
> > +          - description: IRS control frame
>
> I came across it while testing EL3 firmware, raising the topic for
> discussion.
>
> The IRS (and the ITS) has a config frame (need to patch the typo
> s/control/config, already done) per interrupt domain supported, that is,
> it can have up to 4 config frames:
>
> - EL3
> - Secure
> - Realm
> - Non-Secure
>
> The one described in this binding is the non-secure one.
>
> IIUC, everything described in the DT represents the non-secure address
> space.

The dt bindings do allow for describing Secure-world devices:
Documentation/devicetree/bindings/arm/secure.txt has the
details. We use this in QEMU so we can provide a DTB to
guest EL3 firmware that tells it where the hardware is
(and which EL3 can then pass on to an NS kernel). It would
be helpful for the GICv5 binding to be defined in a way that
we can do this for a GICv5 system too.

> Two questions:
>
> - I don't have to spell out the IRS/ITS config frame (and SETLPI, by
>   the way) as non-secure, since that's implicit, is that correct ?

Do you want the DT binding to handle the case of "CPU and GIC do not
implement EL3, and the only implemented security state is Secure"
without the kernel needing to do something different from "ditto ditto
but the only implemented security state is Nonsecure" ?
(Currently booting.html says you must be in NS, so we effectively
say we don't support booting on this particular unicorn :-)
But the secure.txt bindings envisage "kernel got booted in S",
mostly for the benefit of aarch32.)

> - How can the schema describe, if present, EL3, Secure and Realm frames ?

The tempting thing to do is to have regs[] list the frames
in some given order, but the spec makes them not simple
supersets, allowing all of:
 * NS
 * S
 * NS, S, EL3
 * NS, Realm, EL3
 * NS, Realm, S, EL3

secure.txt says:
# The general principle of the naming scheme for Secure world bindings
# is that any property that needs a different value in the Secure world
# can be supported by prefixing the property name with "secure-". So for
# instance "secure-foo" would override "foo".

So maybe we could have
 reg : the NS frame(s)
 secure-reg : the S frame(s)
 realm-reg : the Realm frame(s)
 root-reg : the EL3 frame(s)

??

thanks
-- PMM

On Thu, May 29, 2025 at 02:17:26PM +0100, Peter Maydell wrote:
> On Thu, 29 May 2025 at 13:44, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> >
> > [+Andre, Peter]
> >
> > On Tue, May 13, 2025 at 07:47:54PM +0200, Lorenzo Pieralisi wrote:
> > > +      reg:
> > > +        minItems: 1
> > > +        items:
> > > +          - description: IRS control frame
> >
> > I came across it while testing EL3 firmware, raising the topic for
> > discussion.
> >
> > The IRS (and the ITS) has a config frame (need to patch the typo
> > s/control/config, already done) per interrupt domain supported, that is,
> > it can have up to 4 config frames:
> >
> > - EL3
> > - Secure
> > - Realm
> > - Non-Secure
> >
> > The one described in this binding is the non-secure one.
> >
> > IIUC, everything described in the DT represents the non-secure address
> > space.
> 
> The dt bindings do allow for describing Secure-world devices:
> Documentation/devicetree/bindings/arm/secure.txt has the
> details. We use this in QEMU so we can provide a DTB to
> guest EL3 firmware that tells it where the hardware is
> (and which EL3 can then pass on to an NS kernel). It would
> be helpful for the GICv5 binding to be defined in a way that
> we can do this for a GICv5 system too.

It would be good to understand what DT {should/should not} describe and
whether this DT usage to configure firmware is under the DT maintainers
radar or it is an attempt at reusing it to avoid implementing a
configuration scheme.

Rob, Krzysztof,

Any thoughts on the matter please ?

[...]

> The tempting thing to do is to have regs[] list the frames
> in some given order, but the spec makes them not simple
> supersets, allowing all of:
>  * NS
>  * S
>  * NS, S, EL3
>  * NS, Realm, EL3
>  * NS, Realm, S, EL3

Maybe reg-names can help ? Even though first we need to understand
what resources should be described in DT.

Current bindings are reviewed and I am not keen on dragging this
discussion on forever - the information the kernel requires is there,
I'd like to bring this to a close.

Thanks,
Lorenzo

> 
> secure.txt says:
> # The general principle of the naming scheme for Secure world bindings
> # is that any property that needs a different value in the Secure world
> # can be supported by prefixing the property name with "secure-". So for
> # instance "secure-foo" would override "foo".
> 
> So maybe we could have
>  reg : the NS frame(s)
>  secure-reg : the S frame(s)
>  realm-reg : the Realm frame(s)
>  root-reg : the EL3 frame(s)
> 
> ??
> 
> thanks
> -- PMM

On Tue, Jun 3, 2025 at 2:48 AM Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
>
> On Thu, May 29, 2025 at 02:17:26PM +0100, Peter Maydell wrote:
> > On Thu, 29 May 2025 at 13:44, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > >
> > > [+Andre, Peter]
> > >
> > > On Tue, May 13, 2025 at 07:47:54PM +0200, Lorenzo Pieralisi wrote:
> > > > +      reg:
> > > > +        minItems: 1
> > > > +        items:
> > > > +          - description: IRS control frame
> > >
> > > I came across it while testing EL3 firmware, raising the topic for
> > > discussion.
> > >
> > > The IRS (and the ITS) has a config frame (need to patch the typo
> > > s/control/config, already done) per interrupt domain supported, that is,
> > > it can have up to 4 config frames:
> > >
> > > - EL3
> > > - Secure
> > > - Realm
> > > - Non-Secure
> > >
> > > The one described in this binding is the non-secure one.
> > >
> > > IIUC, everything described in the DT represents the non-secure address
> > > space.
> >
> > The dt bindings do allow for describing Secure-world devices:
> > Documentation/devicetree/bindings/arm/secure.txt has the
> > details. We use this in QEMU so we can provide a DTB to
> > guest EL3 firmware that tells it where the hardware is
> > (and which EL3 can then pass on to an NS kernel). It would
> > be helpful for the GICv5 binding to be defined in a way that
> > we can do this for a GICv5 system too.
>
> It would be good to understand what DT {should/should not} describe and
> whether this DT usage to configure firmware is under the DT maintainers
> radar or it is an attempt at reusing it to avoid implementing a
> configuration scheme.
>
> Rob, Krzysztof,
>
> Any thoughts on the matter please ?

I'm all for firmware using DT, but using a single DT for all
components with an ABI between all components is an impractical dream.
You can take that a step further even with a single DT for all
processors in a system (aka System DT). Ultimately, the DT is a view
of the system for a client (OS). Different views may need different
DTs.

u-boot and Linux sharing a DT makes sense as they have the same world
view. Secure and NS not so much.

> [...]
>
> > The tempting thing to do is to have regs[] list the frames
> > in some given order, but the spec makes them not simple
> > supersets, allowing all of:
> >  * NS
> >  * S
> >  * NS, S, EL3
> >  * NS, Realm, EL3
> >  * NS, Realm, S, EL3
>
> Maybe reg-names can help ? Even though first we need to understand
> what resources should be described in DT.
>
> Current bindings are reviewed and I am not keen on dragging this
> discussion on forever - the information the kernel requires is there,
> I'd like to bring this to a close.
>
> Thanks,
> Lorenzo
>
> >
> > secure.txt says:
> > # The general principle of the naming scheme for Secure world bindings
> > # is that any property that needs a different value in the Secure world
> > # can be supported by prefixing the property name with "secure-". So for
> > # instance "secure-foo" would override "foo".

Today I would say a 'secure-' prefix is a mistake. To my knowledge,
it's never been used anyways. But I don't have much visibility into
what secure world firmware is doing.

> >
> > So maybe we could have
> >  reg : the NS frame(s)
> >  secure-reg : the S frame(s)
> >  realm-reg : the Realm frame(s)
> >  root-reg : the EL3 frame(s)

Here's why. It really doesn't scale.

Rob

On Tue, Jun 03, 2025 at 10:15:25AM -0500, Rob Herring wrote:
> On Tue, Jun 3, 2025 at 2:48 AM Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> >
> > On Thu, May 29, 2025 at 02:17:26PM +0100, Peter Maydell wrote:
> > > On Thu, 29 May 2025 at 13:44, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > > >
> > > > [+Andre, Peter]
> > > >
> > > > On Tue, May 13, 2025 at 07:47:54PM +0200, Lorenzo Pieralisi wrote:
> > > > > +      reg:
> > > > > +        minItems: 1
> > > > > +        items:
> > > > > +          - description: IRS control frame
> > > >
> > > > I came across it while testing EL3 firmware, raising the topic for
> > > > discussion.
> > > >
> > > > The IRS (and the ITS) has a config frame (need to patch the typo
> > > > s/control/config, already done) per interrupt domain supported, that is,
> > > > it can have up to 4 config frames:
> > > >
> > > > - EL3
> > > > - Secure
> > > > - Realm
> > > > - Non-Secure
> > > >
> > > > The one described in this binding is the non-secure one.
> > > >
> > > > IIUC, everything described in the DT represents the non-secure address
> > > > space.
> > >
> > > The dt bindings do allow for describing Secure-world devices:
> > > Documentation/devicetree/bindings/arm/secure.txt has the
> > > details. We use this in QEMU so we can provide a DTB to
> > > guest EL3 firmware that tells it where the hardware is
> > > (and which EL3 can then pass on to an NS kernel). It would
> > > be helpful for the GICv5 binding to be defined in a way that
> > > we can do this for a GICv5 system too.
> >
> > It would be good to understand what DT {should/should not} describe and
> > whether this DT usage to configure firmware is under the DT maintainers
> > radar or it is an attempt at reusing it to avoid implementing a
> > configuration scheme.
> >
> > Rob, Krzysztof,
> >
> > Any thoughts on the matter please ?
> 
> I'm all for firmware using DT, but using a single DT for all
> components with an ABI between all components is an impractical dream.
> You can take that a step further even with a single DT for all
> processors in a system (aka System DT). Ultimately, the DT is a view
> of the system for a client (OS). Different views may need different
> DTs.

Specifically, for IRS/ITS frames then - what the current schema does is
correct, namely, it does _not_ spell out whether the IRS/ITS config
frame is NS/S/Realm/Root interrupt domain, that's information that the
client implicitly assumes.

Are we OK with this approach ? This would leave open the possibility
of having a DT per security-state.

If in the DT schema I define eg reg -> "IRS NS config frame" by
construction the binding can't be used for anything else.

Please let me know if we are in agreement on this matter.

Lorenzo

> u-boot and Linux sharing a DT makes sense as they have the same world
> view. Secure and NS not so much.
> 
> > [...]
> >
> > > The tempting thing to do is to have regs[] list the frames
> > > in some given order, but the spec makes them not simple
> > > supersets, allowing all of:
> > >  * NS
> > >  * S
> > >  * NS, S, EL3
> > >  * NS, Realm, EL3
> > >  * NS, Realm, S, EL3
> >
> > Maybe reg-names can help ? Even though first we need to understand
> > what resources should be described in DT.
> >
> > Current bindings are reviewed and I am not keen on dragging this
> > discussion on forever - the information the kernel requires is there,
> > I'd like to bring this to a close.
> >
> > Thanks,
> > Lorenzo
> >
> > >
> > > secure.txt says:
> > > # The general principle of the naming scheme for Secure world bindings
> > > # is that any property that needs a different value in the Secure world
> > > # can be supported by prefixing the property name with "secure-". So for
> > > # instance "secure-foo" would override "foo".
> 
> Today I would say a 'secure-' prefix is a mistake. To my knowledge,
> it's never been used anyways. But I don't have much visibility into
> what secure world firmware is doing.
> 
> > >
> > > So maybe we could have
> > >  reg : the NS frame(s)
> > >  secure-reg : the S frame(s)
> > >  realm-reg : the Realm frame(s)
> > >  root-reg : the EL3 frame(s)
> 
> Here's why. It really doesn't scale.
> 
> Rob

On Tue, 3 Jun 2025 at 16:53, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> Specifically, for IRS/ITS frames then - what the current schema does is
> correct, namely, it does _not_ spell out whether the IRS/ITS config
> frame is NS/S/Realm/Root interrupt domain, that's information that the
> client implicitly assumes.
>
> Are we OK with this approach ? This would leave open the possibility
> of having a DT per security-state.
>
> If in the DT schema I define eg reg -> "IRS NS config frame" by
> construction the binding can't be used for anything else.
>
> Please let me know if we are in agreement on this matter.

This would break the QEMU virt board -> EL3 guest firmware ->
EL1 Linux flow. We need a binding which lets us optionally
specify "oh by the way here is where the other non-NS frames are".
I don't have a strong view on the specific syntax.

-- PMM

On Tue, Jun 03, 2025 at 05:04:33PM +0100, Peter Maydell wrote:
> On Tue, 3 Jun 2025 at 16:53, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > Specifically, for IRS/ITS frames then - what the current schema does is
> > correct, namely, it does _not_ spell out whether the IRS/ITS config
> > frame is NS/S/Realm/Root interrupt domain, that's information that the
> > client implicitly assumes.
> >
> > Are we OK with this approach ? This would leave open the possibility
> > of having a DT per security-state.
> >
> > If in the DT schema I define eg reg -> "IRS NS config frame" by
> > construction the binding can't be used for anything else.
> >
> > Please let me know if we are in agreement on this matter.
> 
> This would break the QEMU virt board -> EL3 guest firmware ->
> EL1 Linux flow. We need a binding which lets us optionally
> specify "oh by the way here is where the other non-NS frames are".

Do we "need" a binding ? Or, it is a nice-have to help configure
QEmu (and other SW components, eg bootwrapper/TF-A, etc that decided
to re-use DT for their own consumption) ?

And even so, why can't we have a DT per security state as-per Rob's
reply ?

Given that only the "status" property is tagged with secure- today,
may I ask please how does this work ?

What's "EL3 guest firmware" ? Does it use the secure-status property to
detect that the respective device is secure/non-secure ?

And why does it have to be the same dtb we are passing to the OS client ?

> I don't have a strong view on the specific syntax.

I do because I don't want to revisit this later ;-)

Thanks,
Lorenzo

On Tue, 3 Jun 2025 at 16:15, Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Jun 3, 2025 at 2:48 AM Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> >
> > On Thu, May 29, 2025 at 02:17:26PM +0100, Peter Maydell wrote:
> > > secure.txt says:
> > > # The general principle of the naming scheme for Secure world bindings
> > > # is that any property that needs a different value in the Secure world
> > > # can be supported by prefixing the property name with "secure-". So for
> > > # instance "secure-foo" would override "foo".
>
> Today I would say a 'secure-' prefix is a mistake. To my knowledge,
> it's never been used anyways. But I don't have much visibility into
> what secure world firmware is doing.

QEMU uses it for communicating with the secure firmware if
you run secure firmware on the virt board. It's done that
since we introduced that binding. Indeed that use case is *why*
the binding is there. It works fine for the intended purpose,
which is "most devices are visible in both S and NS, but a few
things are S only (UART, a bit of RAM, secure-only flash").

-- PMM

On Tue, Jun 3, 2025 at 10:37 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 3 Jun 2025 at 16:15, Rob Herring <robh@kernel.org> wrote:
> >
> > On Tue, Jun 3, 2025 at 2:48 AM Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > >
> > > On Thu, May 29, 2025 at 02:17:26PM +0100, Peter Maydell wrote:
> > > > secure.txt says:
> > > > # The general principle of the naming scheme for Secure world bindings
> > > > # is that any property that needs a different value in the Secure world
> > > > # can be supported by prefixing the property name with "secure-". So for
> > > > # instance "secure-foo" would override "foo".
> >
> > Today I would say a 'secure-' prefix is a mistake. To my knowledge,
> > it's never been used anyways. But I don't have much visibility into
> > what secure world firmware is doing.
>
> QEMU uses it for communicating with the secure firmware if
> you run secure firmware on the virt board. It's done that
> since we introduced that binding. Indeed that use case is *why*
> the binding is there. It works fine for the intended purpose,
> which is "most devices are visible in both S and NS, but a few
> things are S only (UART, a bit of RAM, secure-only flash").

I meant "secure-" as a prefix allowed on *any* property, not
"secure-status" specifically, which is the only thing QEMU uses
AFAICT. IOW, I don't think we should be creating secure-reg,
secure-interrupts, secure-clocks, etc.

Rob

On Tue, Jun 03, 2025 at 02:11:34PM -0500, Rob Herring wrote:
> On Tue, Jun 3, 2025 at 10:37 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Tue, 3 Jun 2025 at 16:15, Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Tue, Jun 3, 2025 at 2:48 AM Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > > >
> > > > On Thu, May 29, 2025 at 02:17:26PM +0100, Peter Maydell wrote:
> > > > > secure.txt says:
> > > > > # The general principle of the naming scheme for Secure world bindings
> > > > > # is that any property that needs a different value in the Secure world
> > > > > # can be supported by prefixing the property name with "secure-". So for
> > > > > # instance "secure-foo" would override "foo".
> > >
> > > Today I would say a 'secure-' prefix is a mistake. To my knowledge,
> > > it's never been used anyways. But I don't have much visibility into
> > > what secure world firmware is doing.
> >
> > QEMU uses it for communicating with the secure firmware if
> > you run secure firmware on the virt board. It's done that
> > since we introduced that binding. Indeed that use case is *why*
> > the binding is there. It works fine for the intended purpose,
> > which is "most devices are visible in both S and NS, but a few
> > things are S only (UART, a bit of RAM, secure-only flash").
> 
> I meant "secure-" as a prefix allowed on *any* property, not
> "secure-status" specifically, which is the only thing QEMU uses
> AFAICT. IOW, I don't think we should be creating secure-reg,
> secure-interrupts, secure-clocks, etc.

Reading secure.txt, what does it mean "device present and usable in
the secure world" ?

So:

status = "disabled"
secure-status = "okay"

basically means that the device in question allows secure-only MMIO
access, is that what it says ?

If that's the case and we really want to have all config frames
in a single DT, would it be reasonable to have an IRS/ITS DT node
per-frame ?

Then yes, the secure- tag is not enough any longer (because we have to
cope with 4 interrupt domains) but that's a separate problem - again,
this would leave the current reviewed bindings unchanged.

Other than that as I mentioned we could use (? aka clutching at straws)
reg-names but I don't think it is correct to have in the DT address
space that the CPU is not allowed to address.

Lorenzo

On Wed, 04 Jun 2025 08:24:38 +0100,
Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> 
> On Tue, Jun 03, 2025 at 02:11:34PM -0500, Rob Herring wrote:
> > On Tue, Jun 3, 2025 at 10:37 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > On Tue, 3 Jun 2025 at 16:15, Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > On Tue, Jun 3, 2025 at 2:48 AM Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > > > >
> > > > > On Thu, May 29, 2025 at 02:17:26PM +0100, Peter Maydell wrote:
> > > > > > secure.txt says:
> > > > > > # The general principle of the naming scheme for Secure world bindings
> > > > > > # is that any property that needs a different value in the Secure world
> > > > > > # can be supported by prefixing the property name with "secure-". So for
> > > > > > # instance "secure-foo" would override "foo".
> > > >
> > > > Today I would say a 'secure-' prefix is a mistake. To my knowledge,
> > > > it's never been used anyways. But I don't have much visibility into
> > > > what secure world firmware is doing.
> > >
> > > QEMU uses it for communicating with the secure firmware if
> > > you run secure firmware on the virt board. It's done that
> > > since we introduced that binding. Indeed that use case is *why*
> > > the binding is there. It works fine for the intended purpose,
> > > which is "most devices are visible in both S and NS, but a few
> > > things are S only (UART, a bit of RAM, secure-only flash").
> > 
> > I meant "secure-" as a prefix allowed on *any* property, not
> > "secure-status" specifically, which is the only thing QEMU uses
> > AFAICT. IOW, I don't think we should be creating secure-reg,
> > secure-interrupts, secure-clocks, etc.
> 
> Reading secure.txt, what does it mean "device present and usable in
> the secure world" ?
> 
> So:
> 
> status = "disabled"
> secure-status = "okay"
> 
> basically means that the device in question allows secure-only MMIO
> access, is that what it says ?
> 
> If that's the case and we really want to have all config frames
> in a single DT, would it be reasonable to have an IRS/ITS DT node
> per-frame ?
> 
> Then yes, the secure- tag is not enough any longer (because we have to
> cope with 4 interrupt domains) but that's a separate problem - again,
> this would leave the current reviewed bindings unchanged.

No, this is the same problem, and we need a way to address it.
"secure-*" doesn't cut it in a system with FEAT_RME, where resources
are only available to a single Physical Address Space (PAS). So we
need a way to qualify these resources with a PAS.

Either that, or we have to restrict DT to describe the view of a
single PAS. Which Peter will understandably be unhappy about.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.

On Wed, Jun 04, 2025 at 04:56:02PM +0100, Marc Zyngier wrote:
> On Wed, 04 Jun 2025 08:24:38 +0100,
> Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > 
> > On Tue, Jun 03, 2025 at 02:11:34PM -0500, Rob Herring wrote:
> > > On Tue, Jun 3, 2025 at 10:37 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> > > >
> > > > On Tue, 3 Jun 2025 at 16:15, Rob Herring <robh@kernel.org> wrote:
> > > > >
> > > > > On Tue, Jun 3, 2025 at 2:48 AM Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > > > > >
> > > > > > On Thu, May 29, 2025 at 02:17:26PM +0100, Peter Maydell wrote:
> > > > > > > secure.txt says:
> > > > > > > # The general principle of the naming scheme for Secure world bindings
> > > > > > > # is that any property that needs a different value in the Secure world
> > > > > > > # can be supported by prefixing the property name with "secure-". So for
> > > > > > > # instance "secure-foo" would override "foo".
> > > > >
> > > > > Today I would say a 'secure-' prefix is a mistake. To my knowledge,
> > > > > it's never been used anyways. But I don't have much visibility into
> > > > > what secure world firmware is doing.
> > > >
> > > > QEMU uses it for communicating with the secure firmware if
> > > > you run secure firmware on the virt board. It's done that
> > > > since we introduced that binding. Indeed that use case is *why*
> > > > the binding is there. It works fine for the intended purpose,
> > > > which is "most devices are visible in both S and NS, but a few
> > > > things are S only (UART, a bit of RAM, secure-only flash").
> > > 
> > > I meant "secure-" as a prefix allowed on *any* property, not
> > > "secure-status" specifically, which is the only thing QEMU uses
> > > AFAICT. IOW, I don't think we should be creating secure-reg,
> > > secure-interrupts, secure-clocks, etc.
> > 
> > Reading secure.txt, what does it mean "device present and usable in
> > the secure world" ?
> > 
> > So:
> > 
> > status = "disabled"
> > secure-status = "okay"
> > 
> > basically means that the device in question allows secure-only MMIO
> > access, is that what it says ?
> > 
> > If that's the case and we really want to have all config frames
> > in a single DT, would it be reasonable to have an IRS/ITS DT node
> > per-frame ?
> > 
> > Then yes, the secure- tag is not enough any longer (because we have to
> > cope with 4 interrupt domains) but that's a separate problem - again,
> > this would leave the current reviewed bindings unchanged.
> 
> No, this is the same problem, and we need a way to address it.
> "secure-*" doesn't cut it in a system with FEAT_RME, where resources
> are only available to a single Physical Address Space (PAS). So we
> need a way to qualify these resources with a PAS.

Can I ask again what:

status = "disabled"
secure-status = "okay"

for a device means in practice in the current bindings ?

When I said "a separate problem", I meant that, extending secure- tag
(that applies to the "status" property only) to cover other PASes is
independent from the GICv5 binding *if* we define, for a single DT an eg
IRS device per-PAS (with realm-status, root-status, describing what the
reg property represents. Is that what secure-status does today ? Does
it say "this device MMIO space is secure-only" ?).

It does not look like there is much appetite for tagging the reg
property either and making it GICv5 specific is a shortcut IMO.

> Either that, or we have to restrict DT to describe the view of a
> single PAS. Which Peter will understandably be unhappy about.

Well, I listed a couple of options in this thread, let's try
to converge.

Thanks,
Lorenzo

On Wed, 4 Jun 2025 at 17:35, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> Can I ask again what:
>
> status = "disabled"
> secure-status = "okay"
>
> for a device means in practice in the current bindings ?

From software's point of view, it means "if you're NonSecure,
ignore this; if you're Secure, feel free to use it".
From the point of view of something creating this DT node,
the usual reason for setting up a node like that is that the
device is only present in the Secure memory map, not the NS one;
so marking it that way lets you tell the S firmware where the
device is and also tell the NS kernel to ignore it so it doesn't
try to probe for the device and fall over when it gets an exception.

> When I said "a separate problem", I meant that, extending secure- tag
> (that applies to the "status" property only) to cover other PASes is
> independent from the GICv5 binding *if* we define, for a single DT an eg
> IRS device per-PAS (with realm-status, root-status, describing what the
> reg property represents. Is that what secure-status does today ? Does
> it say "this device MMIO space is secure-only" ?).
>
> It does not look like there is much appetite for tagging the reg
> property either and making it GICv5 specific is a shortcut IMO.

I think something GICv5 specific is not unreasonable.
secure.txt is careful to define the general principles of
how the naming scheme works but then to restrict it only to the
specific cases that we've blessed as OK. I think that's worked
pretty well -- it's fitted the needs we have and the GICv5 is
only the second time in a decade we've had to revisit and say
"we also want XYZ" (the first being /secure-chosen/stdout-path,
in 2018). I think that's a pretty decent track record. In
adding whatever we want to do for GICv5, I agree with Rob
that we don't want to accidentally open the door for more
general use of secure-reg or whatever on other random devices.

thanks
-- PMM

On Wed, Jun 04, 2025 at 09:09:27PM +0100, Peter Maydell wrote:

[...]

> > When I said "a separate problem", I meant that, extending secure- tag
> > (that applies to the "status" property only) to cover other PASes is
> > independent from the GICv5 binding *if* we define, for a single DT an eg
> > IRS device per-PAS (with realm-status, root-status, describing what the
> > reg property represents. Is that what secure-status does today ? Does
> > it say "this device MMIO space is secure-only" ?).
> >
> > It does not look like there is much appetite for tagging the reg
> > property either and making it GICv5 specific is a shortcut IMO.
> 
> I think something GICv5 specific is not unreasonable.

We need to define up to 4 interrupt domains (so max 4 frames per
component per frame type: EL3, Secure, Realm, Non-Secure).

Options:

1) Using reg and reg-names, I don't know if reg-names allows us to
   describe all possible resource names and the order does not matter,
   please let me know. Keep in mind that some resources are optional.

   Something like, for an IRS:

   reg-names = "el3-config", "secure-config", "realm-config",
   "non-secure-config", "el3-setlpi", "secure-setlpi", "realm-setlpi",
   "non-secure-setlpi";

   With that, I would remove the description in the reg property and
   just say minItems: 1

   This implicitly means that describing in DT a resource that the
   CPU possibly is not able to reach depending on
   security-state/exception level is OK. AFAICS reg-names achieves
   the same purpose of tagging below, at the end of the day it is
   a means to say eg "if you are non-secure stay away from something
   that does not belong to you".

2) We add a tagged "reg" property for GICv5 ("reg" refers to non-secure):

   reg
   el3-reg
   secure-reg
   realm-reg

3) We add a GICv5 tagged "status" property and define an eg IRS device per
   interrupt-domain ("status" refers to non-secure):

   status
   el3-status
   secure-status
   realm-status

4) Anything else that I have not thought of

What's the best option ? Please let me know, I'd like to repost the
series at v6.16-rc1 with a solution.

Thanks,
Lorenzo

On Tue, 3 Jun 2025 at 08:48, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
>
> On Thu, May 29, 2025 at 02:17:26PM +0100, Peter Maydell wrote:
> > The dt bindings do allow for describing Secure-world devices:
> > Documentation/devicetree/bindings/arm/secure.txt has the
> > details. We use this in QEMU so we can provide a DTB to
> > guest EL3 firmware that tells it where the hardware is
> > (and which EL3 can then pass on to an NS kernel). It would
> > be helpful for the GICv5 binding to be defined in a way that
> > we can do this for a GICv5 system too.
>
> It would be good to understand what DT {should/should not} describe and
> whether this DT usage to configure firmware is under the DT maintainers
> radar or it is an attempt at reusing it to avoid implementing a
> configuration scheme.

It is definitely on the radar, not under it -- that is the whole
reason that we went to the effort to agree and document secure.txt
in the upstream binding documentation :-)

-- PMM

On Thu, May 29, 2025 at 02:17:26PM +0100, Peter Maydell wrote:
> On Thu, 29 May 2025 at 13:44, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> >
> > [+Andre, Peter]
> >
> > On Tue, May 13, 2025 at 07:47:54PM +0200, Lorenzo Pieralisi wrote:
> > > +      reg:
> > > +        minItems: 1
> > > +        items:
> > > +          - description: IRS control frame
> >
> > I came across it while testing EL3 firmware, raising the topic for
> > discussion.
> >
> > The IRS (and the ITS) has a config frame (need to patch the typo
> > s/control/config, already done) per interrupt domain supported, that is,
> > it can have up to 4 config frames:
> >
> > - EL3
> > - Secure
> > - Realm
> > - Non-Secure
> >
> > The one described in this binding is the non-secure one.
> >
> > IIUC, everything described in the DT represents the non-secure address
> > space.
> 
> The dt bindings do allow for describing Secure-world devices:
> Documentation/devicetree/bindings/arm/secure.txt has the
> details. We use this in QEMU so we can provide a DTB to
> guest EL3 firmware that tells it where the hardware is
> (and which EL3 can then pass on to an NS kernel). It would
> be helpful for the GICv5 binding to be defined in a way that
> we can do this for a GICv5 system too.
> 
> > Two questions:
> >
> > - I don't have to spell out the IRS/ITS config frame (and SETLPI, by
> >   the way) as non-secure, since that's implicit, is that correct ?
> 
> Do you want the DT binding to handle the case of "CPU and GIC do not
> implement EL3, and the only implemented security state is Secure"
> without the kernel needing to do something different from "ditto ditto
> but the only implemented security state is Nonsecure" ?

Not sure I follow you here sorry :)

> (Currently booting.html says you must be in NS, so we effectively
> say we don't support booting on this particular unicorn :-)
> But the secure.txt bindings envisage "kernel got booted in S",
> mostly for the benefit of aarch32.)
> 
> > - How can the schema describe, if present, EL3, Secure and Realm frames ?
> 
> The tempting thing to do is to have regs[] list the frames
> in some given order, but the spec makes them not simple
> supersets, allowing all of:
>  * NS
>  * S
>  * NS, S, EL3
>  * NS, Realm, EL3
>  * NS, Realm, S, EL3
> 
> secure.txt says:
> # The general principle of the naming scheme for Secure world bindings
> # is that any property that needs a different value in the Secure world
> # can be supported by prefixing the property name with "secure-". So for
> # instance "secure-foo" would override "foo".
> 
> So maybe we could have
>  reg : the NS frame(s)
>  secure-reg : the S frame(s)
>  realm-reg : the Realm frame(s)
>  root-reg : the EL3 frame(s)
> 
> ??

I assume someone has to write the root/realm binding extensions.

In Documentation/devicetree/bindings/arm/secure.txt I don't think that
reg is a contemplated property - I don't know if the list of properties
is up-to-date.

If what you suggest is OK, is it really needed to add the
{secure/realm/root}-reg property to this binding ?

Or implicitly a, say, realm-reg property is allowed using the
yet-to-be-written realm.txt rules ?

This would also slightly change the "required" properties, a "reg"
property would not be required if eg the GIC does not implement a NS
interrupt domain (but we would require a secure-reg if it implements a
secure interrupt domain). I am making this up, obviously, I don't know
what's best to do here.

Lorenzo

On Thu, 29 May 2025 at 15:21, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> On Thu, May 29, 2025 at 02:17:26PM +0100, Peter Maydell wrote:
> > The dt bindings do allow for describing Secure-world devices:
> > Documentation/devicetree/bindings/arm/secure.txt has the
> > details. We use this in QEMU so we can provide a DTB to
> > guest EL3 firmware that tells it where the hardware is
> > (and which EL3 can then pass on to an NS kernel). It would
> > be helpful for the GICv5 binding to be defined in a way that
> > we can do this for a GICv5 system too.
> >
> > > Two questions:
> > >
> > > - I don't have to spell out the IRS/ITS config frame (and SETLPI, by
> > >   the way) as non-secure, since that's implicit, is that correct ?
> >
> > Do you want the DT binding to handle the case of "CPU and GIC do not
> > implement EL3, and the only implemented security state is Secure"
> > without the kernel needing to do something different from "ditto ditto
> > but the only implemented security state is Nonsecure" ?
>
> Not sure I follow you here sorry :)

In a hypothetical system like that the dt could either
define the (only) IRS frame in reg[], or in secure-reg[].
The former would let the kernel not care about the fact it was
in Secure, but would be a bit weird. But I think we can probably
ignore this hypothetical in favour of keeping the binding simple.

> > (Currently booting.html says you must be in NS, so we effectively
> > say we don't support booting on this particular unicorn :-)
> > But the secure.txt bindings envisage "kernel got booted in S",
> > mostly for the benefit of aarch32.)
> >
> > > - How can the schema describe, if present, EL3, Secure and Realm frames ?
> >
> > The tempting thing to do is to have regs[] list the frames
> > in some given order, but the spec makes them not simple
> > supersets, allowing all of:
> >  * NS
> >  * S
> >  * NS, S, EL3
> >  * NS, Realm, EL3
> >  * NS, Realm, S, EL3
> >
> > secure.txt says:
> > # The general principle of the naming scheme for Secure world bindings
> > # is that any property that needs a different value in the Secure world
> > # can be supported by prefixing the property name with "secure-". So for
> > # instance "secure-foo" would override "foo".
> >
> > So maybe we could have
> >  reg : the NS frame(s)
> >  secure-reg : the S frame(s)
> >  realm-reg : the Realm frame(s)
> >  root-reg : the EL3 frame(s)
> >
> > ??
>
> I assume someone has to write the root/realm binding extensions.
>
> In Documentation/devicetree/bindings/arm/secure.txt I don't think that
> reg is a contemplated property - I don't know if the list of properties
> is up-to-date.

It's up to date in the sense that so far we've only needed
to have the 'status' property have a secure- variant. My
suggestion here is that we might extend that to also allow
secure-reg, and to have root- and realm- prefixes too.
Though I don't think we would want to permit secure-reg for
any old device, so maybe something more-GICv5-specific would
work better.

thanks
-- PMM

[+Suzuki]

On Thu, May 29, 2025 at 03:30:51PM +0100, Peter Maydell wrote:
> On Thu, 29 May 2025 at 15:21, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > On Thu, May 29, 2025 at 02:17:26PM +0100, Peter Maydell wrote:
> > > The dt bindings do allow for describing Secure-world devices:
> > > Documentation/devicetree/bindings/arm/secure.txt has the
> > > details. We use this in QEMU so we can provide a DTB to
> > > guest EL3 firmware that tells it where the hardware is
> > > (and which EL3 can then pass on to an NS kernel). It would
> > > be helpful for the GICv5 binding to be defined in a way that
> > > we can do this for a GICv5 system too.
> > >
> > > > Two questions:
> > > >
> > > > - I don't have to spell out the IRS/ITS config frame (and SETLPI, by
> > > >   the way) as non-secure, since that's implicit, is that correct ?
> > >
> > > Do you want the DT binding to handle the case of "CPU and GIC do not
> > > implement EL3, and the only implemented security state is Secure"
> > > without the kernel needing to do something different from "ditto ditto
> > > but the only implemented security state is Nonsecure" ?
> >
> > Not sure I follow you here sorry :)
> 
> In a hypothetical system like that the dt could either
> define the (only) IRS frame in reg[], or in secure-reg[].
> The former would let the kernel not care about the fact it was
> in Secure, but would be a bit weird. But I think we can probably
> ignore this hypothetical in favour of keeping the binding simple.
> 
> > > (Currently booting.html says you must be in NS, so we effectively
> > > say we don't support booting on this particular unicorn :-)
> > > But the secure.txt bindings envisage "kernel got booted in S",
> > > mostly for the benefit of aarch32.)
> > >
> > > > - How can the schema describe, if present, EL3, Secure and Realm frames ?
> > >
> > > The tempting thing to do is to have regs[] list the frames
> > > in some given order, but the spec makes them not simple
> > > supersets, allowing all of:
> > >  * NS
> > >  * S
> > >  * NS, S, EL3
> > >  * NS, Realm, EL3
> > >  * NS, Realm, S, EL3
> > >
> > > secure.txt says:
> > > # The general principle of the naming scheme for Secure world bindings
> > > # is that any property that needs a different value in the Secure world
> > > # can be supported by prefixing the property name with "secure-". So for
> > > # instance "secure-foo" would override "foo".
> > >
> > > So maybe we could have
> > >  reg : the NS frame(s)
> > >  secure-reg : the S frame(s)
> > >  realm-reg : the Realm frame(s)
> > >  root-reg : the EL3 frame(s)
> > >
> > > ??
> >
> > I assume someone has to write the root/realm binding extensions.
> >
> > In Documentation/devicetree/bindings/arm/secure.txt I don't think that
> > reg is a contemplated property - I don't know if the list of properties
> > is up-to-date.
> 
> It's up to date in the sense that so far we've only needed
> to have the 'status' property have a secure- variant. My
> suggestion here is that we might extend that to also allow
> secure-reg, and to have root- and realm- prefixes too.
> Though I don't think we would want to permit secure-reg for
> any old device, so maybe something more-GICv5-specific would
> work better.

I am not sure this is a GICv5 only requirement (looking at SMMUv3,
for instance and there might be more IPs that require security
state awareness).

Or maybe it is a non-existing problem IIUC the paragraph below
correctly (albeit to be frank I don't understand how to determine
whether a dtb is consumed by eg secure-world-only).

"Note that it is still valid for bindings intended for purely Secure
world consumers (like kernels that run entirely in Secure) to simply
describe the view of Secure world using the standard bindings. These
secure- bindings only need to be used where both the Secure and Normal
world views need to be described in a single device tree."

I assume "standard bindings" there would mean that "reg" for the
GICv5 would be just eg "config frame" with no NS/S/Realm/Root attached.

We don't strictly need to have the same dts file for NS and S (example),
NS will never "need" the S bindings at least for GICv5.

Thoughts ?

Thanks,
Lorenzo

On Fri, 30 May 2025 at 10:17, Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
>
> [+Suzuki]
>
> On Thu, May 29, 2025 at 03:30:51PM +0100, Peter Maydell wrote:
> > It's up to date in the sense that so far we've only needed
> > to have the 'status' property have a secure- variant. My
> > suggestion here is that we might extend that to also allow
> > secure-reg, and to have root- and realm- prefixes too.
> > Though I don't think we would want to permit secure-reg for
> > any old device, so maybe something more-GICv5-specific would
> > work better.
>
> I am not sure this is a GICv5 only requirement (looking at SMMUv3,
> for instance and there might be more IPs that require security
> state awareness).

For the SMMUv3 I think we're OK, because there's no separate
set of base SMMU registers for S vs NS; there are Secure
VATOS registers and a Secure Command queue control page, but
those addresses are discoverable by looking at SMMU registers
so they don't need to be encoded in the DT.

> Or maybe it is a non-existing problem IIUC the paragraph below
> correctly (albeit to be frank I don't understand how to determine
> whether a dtb is consumed by eg secure-world-only).
>
> "Note that it is still valid for bindings intended for purely Secure
> world consumers (like kernels that run entirely in Secure) to simply
> describe the view of Secure world using the standard bindings. These
> secure- bindings only need to be used where both the Secure and Normal
> world views need to be described in a single device tree."

The purpose of this paragraph is to cover situations like the
old versatile express cortex-a9 board, where the firmware
booted the kernel in the Secure world. The kernel didn't care
about that, the (non-autogenerated) device tree just told it
where the devices were (and didn't mark them up with secure-status
or anything). That setup (and the dts files for it) pre-date
the addition of this secure-status binding documentation.
The text is just saying that it isn't making that pre-existing
setup retrospectively non-compliant.

> I assume "standard bindings" there would mean that "reg" for the
> GICv5 would be just eg "config frame" with no NS/S/Realm/Root attached.

Yes, in the (hypothetical) GICv5 case the dt for a "boot in
Secure" system would give the address of the Secure config frame.

> We don't strictly need to have the same dts file for NS and S (example),
> NS will never "need" the S bindings at least for GICv5.

One common workflow is that EL3 firmware is passed a DTB,
consumes it for its own purposes and passes it on to the
NS kernel mostly untouched. This is particularly useful for
QEMU where the DTB might have been autogenerated. So you do
want to be able to express what EL3 needs and what NS needs
in one DT.

thanks
-- PMM

On Tue, 13 May 2025 19:47:54 +0200, Lorenzo Pieralisi wrote:
> The GICv5 interrupt controller architecture is composed of:
> 
> - one or more Interrupt Routing Service (IRS)
> - zero or more Interrupt Translation Service (ITS)
> - zero or more Interrupt Wire Bridge (IWB)
> 
> Describe a GICv5 implementation by specifying a top level node
> corresponding to the GICv5 system component.
> 
> IRS nodes are added as GICv5 system component children.
> 
> An ITS is associated with an IRS so ITS nodes are described
> as IRS children - use the hierarchy explicitly in the device
> tree to define the association.
> 
> IWB nodes are described as a separate schema.
> 
> An IWB is connected to a single ITS, the connection is made explicit
> through the msi-parent property and therefore is not required to be
> explicit through a parent-child relationship in the device tree.
> 
> Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Marc Zyngier <maz@kernel.org>
> ---
>  .../interrupt-controller/arm,gic-v5-iwb.yaml       |  78 ++++++++
>  .../bindings/interrupt-controller/arm,gic-v5.yaml  | 202 +++++++++++++++++++++
>  MAINTAINERS                                        |   7 +
>  3 files changed, 287 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>