diff mbox series

[v8,1/2] dt-bindings: arm: Add OP-TEE transport for SCMI

Message ID 20211028140009.23331-1-etienne.carriere@linaro.org (mailing list archive)
State New, archived
Headers show
Series [v8,1/2] dt-bindings: arm: Add OP-TEE transport for SCMI | expand

Commit Message

Etienne Carriere Oct. 28, 2021, 2 p.m. UTC
Introduce compatible "linaro,scmi-optee" for SCMI transport channel
based on an OP-TEE service invocation. The compatible mandates a
channel ID defined with property "linaro,optee-channel-id".

Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes since v6:
 - Remove maxItems from linaro,optee-channel-id description

No change since v5

Changes since v4:
 - Fix sram node name in DTS example: s/-shm-/-sram-/

Changes since v3:
 - Add description for linaro,optee-channel-id in patternProperties
   specifying protocol can optionaly define a dedicated channel id.
 - Fix DTS example (duplicated phandles issue, subnodes ordering)
 - Fix typo in DTS example and description comments.

Changes since v2:
 - Define mandatory property linaro,optee-channel-id
 - Rebased on yaml description file

Changes since v1:
 - Removed modification regarding mboxes property description.
---
 .../bindings/firmware/arm,scmi.yaml           | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)

Comments

Cristian Marussi Oct. 29, 2021, 10:21 a.m. UTC | #1
On Thu, Oct 28, 2021 at 04:00:08PM +0200, Etienne Carriere wrote:
> Introduce compatible "linaro,scmi-optee" for SCMI transport channel
> based on an OP-TEE service invocation. The compatible mandates a
> channel ID defined with property "linaro,optee-channel-id".
> 
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---

Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>

Thanks,
Cristian
Rob Herring Nov. 2, 2021, 1:21 p.m. UTC | #2
On Thu, 28 Oct 2021 16:00:08 +0200, Etienne Carriere wrote:
> Introduce compatible "linaro,scmi-optee" for SCMI transport channel
> based on an OP-TEE service invocation. The compatible mandates a
> channel ID defined with property "linaro,optee-channel-id".
> 
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
> Changes since v6:
>  - Remove maxItems from linaro,optee-channel-id description
> 
> No change since v5
> 
> Changes since v4:
>  - Fix sram node name in DTS example: s/-shm-/-sram-/
> 
> Changes since v3:
>  - Add description for linaro,optee-channel-id in patternProperties
>    specifying protocol can optionaly define a dedicated channel id.
>  - Fix DTS example (duplicated phandles issue, subnodes ordering)
>  - Fix typo in DTS example and description comments.
> 
> Changes since v2:
>  - Define mandatory property linaro,optee-channel-id
>  - Rebased on yaml description file
> 
> Changes since v1:
>  - Removed modification regarding mboxes property description.
> ---
>  .../bindings/firmware/arm,scmi.yaml           | 65 +++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Sudeep Holla Nov. 25, 2021, 12:25 p.m. UTC | #3
On Thu, 28 Oct 2021 16:00:08 +0200, Etienne Carriere wrote:
> Introduce compatible "linaro,scmi-optee" for SCMI transport channel
> based on an OP-TEE service invocation. The compatible mandates a
> channel ID defined with property "linaro,optee-channel-id".
> 
> 


Applied to sudeep.holla/linux (for-next/scmi), thanks!




[1/2] dt-bindings: arm: Add OP-TEE transport for SCMI
      (no commit info)
[2/2] firmware: arm_scmi: Add optee transport
      (no commit info)

--

Regards,
Sudeep
Sudeep Holla Nov. 25, 2021, 12:41 p.m. UTC | #4
On Thu, 28 Oct 2021 16:00:08 +0200, Etienne Carriere wrote:
> Introduce compatible "linaro,scmi-optee" for SCMI transport channel
> based on an OP-TEE service invocation. The compatible mandates a
> channel ID defined with property "linaro,optee-channel-id".
>
>

Applied to sudeep.holla/linux (for-next/scmi), thanks!

[1/2] dt-bindings: arm: Add OP-TEE transport for SCMI
      https://git.kernel.org/sudeep.holla/c/b7d2cf7c81
[2/2] firmware: arm_scmi: Add optee transport
      https://git.kernel.org/sudeep.holla/c/5f90f189a0

Sorry for the earlier incomplete message.

--
Regards,
Sudeep
Ahmad Fatoum Feb. 28, 2022, 4:01 p.m. UTC | #5
Hello Etienne,

On 28.10.21 16:00, Etienne Carriere wrote:
> Introduce compatible "linaro,scmi-optee" for SCMI transport channel
> based on an OP-TEE service invocation. The compatible mandates a
> channel ID defined with property "linaro,optee-channel-id".

I just found this thread via the compatible in the STM32MP131 patch set:
https://lore.kernel.org/all/20220225133137.813919-1-gabriel.fernandez@foss.st.com/

Linux doesn't care whether PSCI is provided by TF-A, OP-TEE or something
else, so there is just the arm,psci* compatible.

What's different about SCMI that this is not possible? Why couldn't the
existing binding and driver be used to communicate with OP-TEE as secure
monitor as well?

Cheers,
Ahmad

> 
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
> Changes since v6:
>  - Remove maxItems from linaro,optee-channel-id description
> 
> No change since v5
> 
> Changes since v4:
>  - Fix sram node name in DTS example: s/-shm-/-sram-/
> 
> Changes since v3:
>  - Add description for linaro,optee-channel-id in patternProperties
>    specifying protocol can optionaly define a dedicated channel id.
>  - Fix DTS example (duplicated phandles issue, subnodes ordering)
>  - Fix typo in DTS example and description comments.
> 
> Changes since v2:
>  - Define mandatory property linaro,optee-channel-id
>  - Rebased on yaml description file
> 
> Changes since v1:
>  - Removed modification regarding mboxes property description.
> ---
>  .../bindings/firmware/arm,scmi.yaml           | 65 +++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index 5c4c6782e052..eae15df36eef 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -38,6 +38,9 @@ properties:
>                       The virtio transport only supports a single device.
>          items:
>            - const: arm,scmi-virtio
> +      - description: SCMI compliant firmware with OP-TEE transport
> +        items:
> +          - const: linaro,scmi-optee
>  
>    interrupts:
>      description:
> @@ -83,6 +86,11 @@ properties:
>      description:
>        SMC id required when using smc or hvc transports
>  
> +  linaro,optee-channel-id:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Channel specifier required when using OP-TEE transport.
> +
>    protocol@11:
>      type: object
>      properties:
> @@ -195,6 +203,12 @@ patternProperties:
>          minItems: 1
>          maxItems: 2
>  
> +      linaro,optee-channel-id:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Channel specifier required when using OP-TEE transport and
> +          protocol has a dedicated communication channel.
> +
>      required:
>        - reg
>  
> @@ -226,6 +240,16 @@ else:
>        - arm,smc-id
>        - shmem
>  
> +  else:
> +    if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: linaro,scmi-optee
> +    then:
> +      required:
> +        - linaro,optee-channel-id
> +
>  examples:
>    - |
>      firmware {
> @@ -340,7 +364,48 @@ examples:
>                  reg = <0x11>;
>                  #power-domain-cells = <1>;
>              };
> +        };
> +    };
> +
> +  - |
> +    firmware {
> +        scmi {
> +            compatible = "linaro,scmi-optee";
> +            linaro,optee-channel-id = <0>;
> +
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            scmi_dvfs1: protocol@13 {
> +                reg = <0x13>;
> +                linaro,optee-channel-id = <1>;
> +                shmem = <&cpu_optee_lpri0>;
> +                #clock-cells = <1>;
> +            };
> +
> +            scmi_clk0: protocol@14 {
> +                reg = <0x14>;
> +                #clock-cells = <1>;
> +            };
> +        };
> +    };
>  
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        sram@51000000 {
> +            compatible = "mmio-sram";
> +            reg = <0x0 0x51000000 0x0 0x10000>;
> +
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            ranges = <0 0x0 0x51000000 0x10000>;
> +
> +            cpu_optee_lpri0: optee-sram-section@0 {
> +                compatible = "arm,scmi-shmem";
> +                reg = <0x0 0x80>;
> +            };
>          };
>      };
>
Etienne Carriere March 1, 2022, 2:05 p.m. UTC | #6
Hello Ahmad,

On Mon, 28 Feb 2022 at 17:01, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Etienne,
>
> On 28.10.21 16:00, Etienne Carriere wrote:
> > Introduce compatible "linaro,scmi-optee" for SCMI transport channel
> > based on an OP-TEE service invocation. The compatible mandates a
> > channel ID defined with property "linaro,optee-channel-id".
>
> I just found this thread via the compatible in the STM32MP131 patch set:
> https://lore.kernel.org/all/20220225133137.813919-1-gabriel.fernandez@foss.st.com/
>
> Linux doesn't care whether PSCI is provided by TF-A, OP-TEE or something
> else, so there is just the arm,psci* compatible.
>
> What's different about SCMI that this is not possible? Why couldn't the
> existing binding and driver be used to communicate with OP-TEE as secure
> monitor as well?

Compatible "linaro,scmi-optee" denote a alternate SCMI transport to
those already in v5.16.
"arm,psci" defines a mailbox + shmem interface.
"arm,psci
 it uses an optee service as in service


>
> Cheers,
> Ahmad
>
> >
> > Cc: devicetree@vger.kernel.org
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > ---
> > Changes since v6:
> >  - Remove maxItems from linaro,optee-channel-id description
> >
> > No change since v5
> >
> > Changes since v4:
> >  - Fix sram node name in DTS example: s/-shm-/-sram-/
> >
> > Changes since v3:
> >  - Add description for linaro,optee-channel-id in patternProperties
> >    specifying protocol can optionaly define a dedicated channel id.
> >  - Fix DTS example (duplicated phandles issue, subnodes ordering)
> >  - Fix typo in DTS example and description comments.
> >
> > Changes since v2:
> >  - Define mandatory property linaro,optee-channel-id
> >  - Rebased on yaml description file
> >
> > Changes since v1:
> >  - Removed modification regarding mboxes property description.
> > ---
> >  .../bindings/firmware/arm,scmi.yaml           | 65 +++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > index 5c4c6782e052..eae15df36eef 100644
> > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > @@ -38,6 +38,9 @@ properties:
> >                       The virtio transport only supports a single device.
> >          items:
> >            - const: arm,scmi-virtio
> > +      - description: SCMI compliant firmware with OP-TEE transport
> > +        items:
> > +          - const: linaro,scmi-optee
> >
> >    interrupts:
> >      description:
> > @@ -83,6 +86,11 @@ properties:
> >      description:
> >        SMC id required when using smc or hvc transports
> >
> > +  linaro,optee-channel-id:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      Channel specifier required when using OP-TEE transport.
> > +
> >    protocol@11:
> >      type: object
> >      properties:
> > @@ -195,6 +203,12 @@ patternProperties:
> >          minItems: 1
> >          maxItems: 2
> >
> > +      linaro,optee-channel-id:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        description:
> > +          Channel specifier required when using OP-TEE transport and
> > +          protocol has a dedicated communication channel.
> > +
> >      required:
> >        - reg
> >
> > @@ -226,6 +240,16 @@ else:
> >        - arm,smc-id
> >        - shmem
> >
> > +  else:
> > +    if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: linaro,scmi-optee
> > +    then:
> > +      required:
> > +        - linaro,optee-channel-id
> > +
> >  examples:
> >    - |
> >      firmware {
> > @@ -340,7 +364,48 @@ examples:
> >                  reg = <0x11>;
> >                  #power-domain-cells = <1>;
> >              };
> > +        };
> > +    };
> > +
> > +  - |
> > +    firmware {
> > +        scmi {
> > +            compatible = "linaro,scmi-optee";
> > +            linaro,optee-channel-id = <0>;
> > +
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            scmi_dvfs1: protocol@13 {
> > +                reg = <0x13>;
> > +                linaro,optee-channel-id = <1>;
> > +                shmem = <&cpu_optee_lpri0>;
> > +                #clock-cells = <1>;
> > +            };
> > +
> > +            scmi_clk0: protocol@14 {
> > +                reg = <0x14>;
> > +                #clock-cells = <1>;
> > +            };
> > +        };
> > +    };
> >
> > +    soc {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        sram@51000000 {
> > +            compatible = "mmio-sram";
> > +            reg = <0x0 0x51000000 0x0 0x10000>;
> > +
> > +            #address-cells = <1>;
> > +            #size-cells = <1>;
> > +            ranges = <0 0x0 0x51000000 0x10000>;
> > +
> > +            cpu_optee_lpri0: optee-sram-section@0 {
> > +                compatible = "arm,scmi-shmem";
> > +                reg = <0x0 0x80>;
> > +            };
> >          };
> >      };
> >
>
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Etienne Carriere March 1, 2022, 2:11 p.m. UTC | #7
Hi sorry,

I sent the mail while i was still typing it...
Here is with the full answer.


On Tue, 1 Mar 2022 at 15:05, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> Hello Ahmad,
>
> On Mon, 28 Feb 2022 at 17:01, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >
> > Hello Etienne,
> >
> > On 28.10.21 16:00, Etienne Carriere wrote:
> > > Introduce compatible "linaro,scmi-optee" for SCMI transport channel
> > > based on an OP-TEE service invocation. The compatible mandates a
> > > channel ID defined with property "linaro,optee-channel-id".
> >
> > I just found this thread via the compatible in the STM32MP131 patch set:
> > https://lore.kernel.org/all/20220225133137.813919-1-gabriel.fernandez@foss.st.com/
> >
> > Linux doesn't care whether PSCI is provided by TF-A, OP-TEE or something
> > else, so there is just the arm,psci* compatible.
> >
> > What's different about SCMI that this is not possible? Why couldn't the
> > existing binding and driver be used to communicate with OP-TEE as secure
> > monitor as well?
>
> Compatible "linaro,scmi-optee" denote a alternate SCMI transport to
> those already in v5.16.

It is names scmi-optee because the interface exposed to access SCMI services is
based on TEE's interface (UUID to open a session with and invoke commands).

The compatible is described in the Linux Documentation but not yet
merged in the linux-next.
It can be found in the tree of arm_scmi driver maintainers:
https://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git/log/?h=for-linux-next

This commit:
https://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git/commit/?h=for-linux-next&id=b7d2cf7c817b86e705b97f72c6be192a6760a14f

Br,
Etienne

>
>
> >
> > Cheers,
> > Ahmad
> >
> > >
> > > Cc: devicetree@vger.kernel.org
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > > ---
> > > Changes since v6:
> > >  - Remove maxItems from linaro,optee-channel-id description
> > >
> > > No change since v5
> > >
> > > Changes since v4:
> > >  - Fix sram node name in DTS example: s/-shm-/-sram-/
> > >
> > > Changes since v3:
> > >  - Add description for linaro,optee-channel-id in patternProperties
> > >    specifying protocol can optionaly define a dedicated channel id.
> > >  - Fix DTS example (duplicated phandles issue, subnodes ordering)
> > >  - Fix typo in DTS example and description comments.
> > >
> > > Changes since v2:
> > >  - Define mandatory property linaro,optee-channel-id
> > >  - Rebased on yaml description file
> > >
> > > Changes since v1:
> > >  - Removed modification regarding mboxes property description.
> > > ---
> > >  .../bindings/firmware/arm,scmi.yaml           | 65 +++++++++++++++++++
> > >  1 file changed, 65 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > index 5c4c6782e052..eae15df36eef 100644
> > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > @@ -38,6 +38,9 @@ properties:
> > >                       The virtio transport only supports a single device.
> > >          items:
> > >            - const: arm,scmi-virtio
> > > +      - description: SCMI compliant firmware with OP-TEE transport
> > > +        items:
> > > +          - const: linaro,scmi-optee
> > >
> > >    interrupts:
> > >      description:
> > > @@ -83,6 +86,11 @@ properties:
> > >      description:
> > >        SMC id required when using smc or hvc transports
> > >
> > > +  linaro,optee-channel-id:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description:
> > > +      Channel specifier required when using OP-TEE transport.
> > > +
> > >    protocol@11:
> > >      type: object
> > >      properties:
> > > @@ -195,6 +203,12 @@ patternProperties:
> > >          minItems: 1
> > >          maxItems: 2
> > >
> > > +      linaro,optee-channel-id:
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        description:
> > > +          Channel specifier required when using OP-TEE transport and
> > > +          protocol has a dedicated communication channel.
> > > +
> > >      required:
> > >        - reg
> > >
> > > @@ -226,6 +240,16 @@ else:
> > >        - arm,smc-id
> > >        - shmem
> > >
> > > +  else:
> > > +    if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            const: linaro,scmi-optee
> > > +    then:
> > > +      required:
> > > +        - linaro,optee-channel-id
> > > +
> > >  examples:
> > >    - |
> > >      firmware {
> > > @@ -340,7 +364,48 @@ examples:
> > >                  reg = <0x11>;
> > >                  #power-domain-cells = <1>;
> > >              };
> > > +        };
> > > +    };
> > > +
> > > +  - |
> > > +    firmware {
> > > +        scmi {
> > > +            compatible = "linaro,scmi-optee";
> > > +            linaro,optee-channel-id = <0>;
> > > +
> > > +            #address-cells = <1>;
> > > +            #size-cells = <0>;
> > > +
> > > +            scmi_dvfs1: protocol@13 {
> > > +                reg = <0x13>;
> > > +                linaro,optee-channel-id = <1>;
> > > +                shmem = <&cpu_optee_lpri0>;
> > > +                #clock-cells = <1>;
> > > +            };
> > > +
> > > +            scmi_clk0: protocol@14 {
> > > +                reg = <0x14>;
> > > +                #clock-cells = <1>;
> > > +            };
> > > +        };
> > > +    };
> > >
> > > +    soc {
> > > +        #address-cells = <2>;
> > > +        #size-cells = <2>;
> > > +
> > > +        sram@51000000 {
> > > +            compatible = "mmio-sram";
> > > +            reg = <0x0 0x51000000 0x0 0x10000>;
> > > +
> > > +            #address-cells = <1>;
> > > +            #size-cells = <1>;
> > > +            ranges = <0 0x0 0x51000000 0x10000>;
> > > +
> > > +            cpu_optee_lpri0: optee-sram-section@0 {
> > > +                compatible = "arm,scmi-shmem";
> > > +                reg = <0x0 0x80>;
> > > +            };
> > >          };
> > >      };
> > >
> >
> >
> > --
> > Pengutronix e.K.                           |                             |
> > Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> > 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> > Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Sudeep Holla March 1, 2022, 3:12 p.m. UTC | #8
Hi Ahmad,

On Mon, Feb 28, 2022 at 05:01:39PM +0100, Ahmad Fatoum wrote:
> Hello Etienne,
> 
> On 28.10.21 16:00, Etienne Carriere wrote:
> > Introduce compatible "linaro,scmi-optee" for SCMI transport channel
> > based on an OP-TEE service invocation. The compatible mandates a
> > channel ID defined with property "linaro,optee-channel-id".
>

Not sure if Etienne's reply addressed your queries/concerns correctly.
I thought I will add my view anyways.

> I just found this thread via the compatible in the STM32MP131 patch set:
> https://lore.kernel.org/all/20220225133137.813919-1-gabriel.fernandez@foss.st.com/
> 
> Linux doesn't care whether PSCI is provided by TF-A, OP-TEE or something
> else, so there is just the arm,psci* compatible.
>

Correct, the interface to the kernel is fixed and hence we must be able
to manage with the standard and fixed sole set of bindings for the same.

> What's different about SCMI that this is not possible? Why couldn't the
> existing binding and driver be used to communicate with OP-TEE as secure
> monitor as well?
>

However with SCMI, the spec concentrates and standardises all the aspects
of the protocol used for the communication while it allows the transport
used for such a communication to be implementation specific. It does
address some standard transports like mailbox and PCC(ACPI). However,
because of the flexibility and also depending on the hardware(or VM),
different transports have been added to the list. SMC/HVC was the one,
followed by the virtio and OPTEE. While I agree SMC/HVC and OPTEE seem
to have lot of common and may have avoided separate bindings.

However the FIDs for SMC/HVC is vendor defined(the spec doesn't cover this
and hence we utilised/exploited DT). Some vendors wanted interrupt support
too which got added. OPTEE eliminates the need for FID and can also provide
dynamic shared memory info. In short, it does differ in a way that the driver
needs to understand the difference and act differently with each of the
unique transports defined in the binding.

Hope that explains and addresses your concern.
Ahmad Fatoum March 8, 2022, 9:51 a.m. UTC | #9
Hello Sudeep,

On 01.03.22 16:12, Sudeep Holla wrote:
> 
> Hi Ahmad,
> 
> On Mon, Feb 28, 2022 at 05:01:39PM +0100, Ahmad Fatoum wrote:
>> Hello Etienne,
>>
>> On 28.10.21 16:00, Etienne Carriere wrote:
>>> Introduce compatible "linaro,scmi-optee" for SCMI transport channel
>>> based on an OP-TEE service invocation. The compatible mandates a
>>> channel ID defined with property "linaro,optee-channel-id".
>>
> 
> Not sure if Etienne's reply addressed your queries/concerns correctly.
> I thought I will add my view anyways.
> 
>> I just found this thread via the compatible in the STM32MP131 patch set:
>> https://lore.kernel.org/all/20220225133137.813919-1-gabriel.fernandez@foss.st.com/
>>
>> Linux doesn't care whether PSCI is provided by TF-A, OP-TEE or something
>> else, so there is just the arm,psci* compatible.
>>
> 
> Correct, the interface to the kernel is fixed and hence we must be able
> to manage with the standard and fixed sole set of bindings for the same.
> 
>> What's different about SCMI that this is not possible? Why couldn't the
>> existing binding and driver be used to communicate with OP-TEE as secure
>> monitor as well?
>>
> 
> However with SCMI, the spec concentrates and standardises all the aspects
> of the protocol used for the communication while it allows the transport
> used for such a communication to be implementation specific. It does
> address some standard transports like mailbox and PCC(ACPI). However,
> because of the flexibility and also depending on the hardware(or VM),
> different transports have been added to the list. SMC/HVC was the one,
> followed by the virtio and OPTEE. While I agree SMC/HVC and OPTEE seem
> to have lot of common and may have avoided separate bindings.
> 
> However the FIDs for SMC/HVC is vendor defined(the spec doesn't cover this
> and hence we utilised/exploited DT). Some vendors wanted interrupt support
> too which got added. OPTEE eliminates the need for FID and can also provide
> dynamic shared memory info. In short, it does differ in a way that the driver
> needs to understand the difference and act differently with each of the
> unique transports defined in the binding.
> 
> Hope that explains and addresses your concern.

Thanks for the elaborate answer. I see now why it's beneficial to have
an OP-TEE transport in general. I don't yet see the benefit to use it
in the STM32MP13x instead of SMCs like with STM32MP15x, but that a discussion
that I need to have in the aforementioned thread.

Thanks again!
Ahmad
Ahmad Fatoum March 8, 2022, 9:53 a.m. UTC | #10
Hell Etienne,

On 01.03.22 15:11, Etienne Carriere wrote:
> Hi sorry,
> 
> I sent the mail while i was still typing it...
> Here is with the full answer.

Thanks for taking the time.

> On Tue, 1 Mar 2022 at 15:05, Etienne Carriere
> <etienne.carriere@linaro.org> wrote:
>>
>> Hello Ahmad,
>>
>> On Mon, 28 Feb 2022 at 17:01, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>
>>> Hello Etienne,
>>>
>>> On 28.10.21 16:00, Etienne Carriere wrote:
>>>> Introduce compatible "linaro,scmi-optee" for SCMI transport channel
>>>> based on an OP-TEE service invocation. The compatible mandates a
>>>> channel ID defined with property "linaro,optee-channel-id".
>>>
>>> I just found this thread via the compatible in the STM32MP131 patch set:
>>> https://lore.kernel.org/all/20220225133137.813919-1-gabriel.fernandez@foss.st.com/
>>>
>>> Linux doesn't care whether PSCI is provided by TF-A, OP-TEE or something
>>> else, so there is just the arm,psci* compatible.
>>>
>>> What's different about SCMI that this is not possible? Why couldn't the
>>> existing binding and driver be used to communicate with OP-TEE as secure
>>> monitor as well?
>>
>> Compatible "linaro,scmi-optee" denote a alternate SCMI transport to
>> those already in v5.16.
> 
> It is names scmi-optee because the interface exposed to access SCMI services is
> based on TEE's interface (UUID to open a session with and invoke commands).

I gathered as much, but I didn't understand why it had to be an extra transport
when SCMI over SMC already exists. Sudeep cleared this point up for me.

Cheers,
Ahmad

> 
> The compatible is described in the Linux Documentation but not yet
> merged in the linux-next.
> It can be found in the tree of arm_scmi driver maintainers:
> https://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git/log/?h=for-linux-next
> 
> This commit:
> https://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git/commit/?h=for-linux-next&id=b7d2cf7c817b86e705b97f72c6be192a6760a14f
> 
> Br,
> Etienne
> 
>>
>>
>>>
>>> Cheers,
>>> Ahmad
>>>
>>>>
>>>> Cc: devicetree@vger.kernel.org
>>>> Cc: Rob Herring <robh+dt@kernel.org>
>>>> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
>>>> ---
>>>> Changes since v6:
>>>>  - Remove maxItems from linaro,optee-channel-id description
>>>>
>>>> No change since v5
>>>>
>>>> Changes since v4:
>>>>  - Fix sram node name in DTS example: s/-shm-/-sram-/
>>>>
>>>> Changes since v3:
>>>>  - Add description for linaro,optee-channel-id in patternProperties
>>>>    specifying protocol can optionaly define a dedicated channel id.
>>>>  - Fix DTS example (duplicated phandles issue, subnodes ordering)
>>>>  - Fix typo in DTS example and description comments.
>>>>
>>>> Changes since v2:
>>>>  - Define mandatory property linaro,optee-channel-id
>>>>  - Rebased on yaml description file
>>>>
>>>> Changes since v1:
>>>>  - Removed modification regarding mboxes property description.
>>>> ---
>>>>  .../bindings/firmware/arm,scmi.yaml           | 65 +++++++++++++++++++
>>>>  1 file changed, 65 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>>>> index 5c4c6782e052..eae15df36eef 100644
>>>> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>>>> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>>>> @@ -38,6 +38,9 @@ properties:
>>>>                       The virtio transport only supports a single device.
>>>>          items:
>>>>            - const: arm,scmi-virtio
>>>> +      - description: SCMI compliant firmware with OP-TEE transport
>>>> +        items:
>>>> +          - const: linaro,scmi-optee
>>>>
>>>>    interrupts:
>>>>      description:
>>>> @@ -83,6 +86,11 @@ properties:
>>>>      description:
>>>>        SMC id required when using smc or hvc transports
>>>>
>>>> +  linaro,optee-channel-id:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    description:
>>>> +      Channel specifier required when using OP-TEE transport.
>>>> +
>>>>    protocol@11:
>>>>      type: object
>>>>      properties:
>>>> @@ -195,6 +203,12 @@ patternProperties:
>>>>          minItems: 1
>>>>          maxItems: 2
>>>>
>>>> +      linaro,optee-channel-id:
>>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>>> +        description:
>>>> +          Channel specifier required when using OP-TEE transport and
>>>> +          protocol has a dedicated communication channel.
>>>> +
>>>>      required:
>>>>        - reg
>>>>
>>>> @@ -226,6 +240,16 @@ else:
>>>>        - arm,smc-id
>>>>        - shmem
>>>>
>>>> +  else:
>>>> +    if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            const: linaro,scmi-optee
>>>> +    then:
>>>> +      required:
>>>> +        - linaro,optee-channel-id
>>>> +
>>>>  examples:
>>>>    - |
>>>>      firmware {
>>>> @@ -340,7 +364,48 @@ examples:
>>>>                  reg = <0x11>;
>>>>                  #power-domain-cells = <1>;
>>>>              };
>>>> +        };
>>>> +    };
>>>> +
>>>> +  - |
>>>> +    firmware {
>>>> +        scmi {
>>>> +            compatible = "linaro,scmi-optee";
>>>> +            linaro,optee-channel-id = <0>;
>>>> +
>>>> +            #address-cells = <1>;
>>>> +            #size-cells = <0>;
>>>> +
>>>> +            scmi_dvfs1: protocol@13 {
>>>> +                reg = <0x13>;
>>>> +                linaro,optee-channel-id = <1>;
>>>> +                shmem = <&cpu_optee_lpri0>;
>>>> +                #clock-cells = <1>;
>>>> +            };
>>>> +
>>>> +            scmi_clk0: protocol@14 {
>>>> +                reg = <0x14>;
>>>> +                #clock-cells = <1>;
>>>> +            };
>>>> +        };
>>>> +    };
>>>>
>>>> +    soc {
>>>> +        #address-cells = <2>;
>>>> +        #size-cells = <2>;
>>>> +
>>>> +        sram@51000000 {
>>>> +            compatible = "mmio-sram";
>>>> +            reg = <0x0 0x51000000 0x0 0x10000>;
>>>> +
>>>> +            #address-cells = <1>;
>>>> +            #size-cells = <1>;
>>>> +            ranges = <0 0x0 0x51000000 0x10000>;
>>>> +
>>>> +            cpu_optee_lpri0: optee-sram-section@0 {
>>>> +                compatible = "arm,scmi-shmem";
>>>> +                reg = <0x0 0x80>;
>>>> +            };
>>>>          };
>>>>      };
>>>>
>>>
>>>
>>> --
>>> Pengutronix e.K.                           |                             |
>>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
Etienne Carriere March 8, 2022, 10:18 a.m. UTC | #11
Hello Ahmad,

On Tue, 8 Mar 2022 at 10:51, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Sudeep,
>
> On 01.03.22 16:12, Sudeep Holla wrote:
> >
> > Hi Ahmad,
> >
> > On Mon, Feb 28, 2022 at 05:01:39PM +0100, Ahmad Fatoum wrote:
> >> Hello Etienne,
> >>
> >> On 28.10.21 16:00, Etienne Carriere wrote:
> >>> Introduce compatible "linaro,scmi-optee" for SCMI transport channel
> >>> based on an OP-TEE service invocation. The compatible mandates a
> >>> channel ID defined with property "linaro,optee-channel-id".
> >>
> >
> > Not sure if Etienne's reply addressed your queries/concerns correctly.
> > I thought I will add my view anyways.
> >
> >> I just found this thread via the compatible in the STM32MP131 patch set:
> >> https://lore.kernel.org/all/20220225133137.813919-1-gabriel.fernandez@foss.st.com/
> >>
> >> Linux doesn't care whether PSCI is provided by TF-A, OP-TEE or something
> >> else, so there is just the arm,psci* compatible.
> >>
> >
> > Correct, the interface to the kernel is fixed and hence we must be able
> > to manage with the standard and fixed sole set of bindings for the same.
> >
> >> What's different about SCMI that this is not possible? Why couldn't the
> >> existing binding and driver be used to communicate with OP-TEE as secure
> >> monitor as well?
> >>
> >
> > However with SCMI, the spec concentrates and standardises all the aspects
> > of the protocol used for the communication while it allows the transport
> > used for such a communication to be implementation specific. It does
> > address some standard transports like mailbox and PCC(ACPI). However,
> > because of the flexibility and also depending on the hardware(or VM),
> > different transports have been added to the list. SMC/HVC was the one,
> > followed by the virtio and OPTEE. While I agree SMC/HVC and OPTEE seem
> > to have lot of common and may have avoided separate bindings.
> >
> > However the FIDs for SMC/HVC is vendor defined(the spec doesn't cover this
> > and hence we utilised/exploited DT). Some vendors wanted interrupt support
> > too which got added. OPTEE eliminates the need for FID and can also provide
> > dynamic shared memory info. In short, it does differ in a way that the driver
> > needs to understand the difference and act differently with each of the
> > unique transports defined in the binding.
> >
> > Hope that explains and addresses your concern.
>
> Thanks for the elaborate answer. I see now why it's beneficial to have
> an OP-TEE transport in general. I don't yet see the benefit to use it
> in the STM32MP13x instead of SMCs like with STM32MP15x, but that a discussion
> that I need to have in the aforementioned thread.

Some SCMI operations in OP-TEE need to execute in a threaded context
(preemptible, ...).
There is no SMC function ID defined for an SCMI thread entry in
OP-TEE. We rather use standard invocation of a TEE service: opening a
session and invoking commands.
Invoked commands are executed in an OP-TEE native threaded context.
The service accessed is referred to as the OP-TEE SCMI PTA.

As for STM32MP15x, one willing to extend resources assigned to secure
world may also need to move mp15 SCMI from SMC transport to optee
transport.

Regards,
Etienne

>
> Thanks again!
> Ahmad
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Ahmad Fatoum March 16, 2022, 11:18 a.m. UTC | #12
Hello Etienne,

On 08.03.22 11:18, Etienne Carriere wrote:
> Hello Ahmad,
> 
> On Tue, 8 Mar 2022 at 10:51, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> Hello Sudeep,
>>
>> On 01.03.22 16:12, Sudeep Holla wrote:
>>>
>>> Hi Ahmad,
>>>
>>> On Mon, Feb 28, 2022 at 05:01:39PM +0100, Ahmad Fatoum wrote:
>>>> Hello Etienne,
>>>>
>>>> On 28.10.21 16:00, Etienne Carriere wrote:
>>>>> Introduce compatible "linaro,scmi-optee" for SCMI transport channel
>>>>> based on an OP-TEE service invocation. The compatible mandates a
>>>>> channel ID defined with property "linaro,optee-channel-id".
>>>>
>>>
>>> Not sure if Etienne's reply addressed your queries/concerns correctly.
>>> I thought I will add my view anyways.
>>>
>>>> I just found this thread via the compatible in the STM32MP131 patch set:
>>>> https://lore.kernel.org/all/20220225133137.813919-1-gabriel.fernandez@foss.st.com/
>>>>
>>>> Linux doesn't care whether PSCI is provided by TF-A, OP-TEE or something
>>>> else, so there is just the arm,psci* compatible.
>>>>
>>>
>>> Correct, the interface to the kernel is fixed and hence we must be able
>>> to manage with the standard and fixed sole set of bindings for the same.
>>>
>>>> What's different about SCMI that this is not possible? Why couldn't the
>>>> existing binding and driver be used to communicate with OP-TEE as secure
>>>> monitor as well?
>>>>
>>>
>>> However with SCMI, the spec concentrates and standardises all the aspects
>>> of the protocol used for the communication while it allows the transport
>>> used for such a communication to be implementation specific. It does
>>> address some standard transports like mailbox and PCC(ACPI). However,
>>> because of the flexibility and also depending on the hardware(or VM),
>>> different transports have been added to the list. SMC/HVC was the one,
>>> followed by the virtio and OPTEE. While I agree SMC/HVC and OPTEE seem
>>> to have lot of common and may have avoided separate bindings.
>>>
>>> However the FIDs for SMC/HVC is vendor defined(the spec doesn't cover this
>>> and hence we utilised/exploited DT). Some vendors wanted interrupt support
>>> too which got added. OPTEE eliminates the need for FID and can also provide
>>> dynamic shared memory info. In short, it does differ in a way that the driver
>>> needs to understand the difference and act differently with each of the
>>> unique transports defined in the binding.
>>>
>>> Hope that explains and addresses your concern.
>>
>> Thanks for the elaborate answer. I see now why it's beneficial to have
>> an OP-TEE transport in general. I don't yet see the benefit to use it
>> in the STM32MP13x instead of SMCs like with STM32MP15x, but that a discussion
>> that I need to have in the aforementioned thread.
> 
> Some SCMI operations in OP-TEE need to execute in a threaded context
> (preemptible, ...).
> There is no SMC function ID defined for an SCMI thread entry in
> OP-TEE. We rather use standard invocation of a TEE service: opening a
> session and invoking commands.
> Invoked commands are executed in an OP-TEE native threaded context.
> The service accessed is referred to as the OP-TEE SCMI PTA.
> 
> As for STM32MP15x, one willing to extend resources assigned to secure
> world may also need to move mp15 SCMI from SMC transport to optee
> transport.

Yes. Makes sense.

Thanks again for explaining,
Ahmad

> 
> Regards,
> Etienne
> 
>>
>> Thanks again!
>> Ahmad
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
etienn
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 5c4c6782e052..eae15df36eef 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -38,6 +38,9 @@  properties:
                      The virtio transport only supports a single device.
         items:
           - const: arm,scmi-virtio
+      - description: SCMI compliant firmware with OP-TEE transport
+        items:
+          - const: linaro,scmi-optee
 
   interrupts:
     description:
@@ -83,6 +86,11 @@  properties:
     description:
       SMC id required when using smc or hvc transports
 
+  linaro,optee-channel-id:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Channel specifier required when using OP-TEE transport.
+
   protocol@11:
     type: object
     properties:
@@ -195,6 +203,12 @@  patternProperties:
         minItems: 1
         maxItems: 2
 
+      linaro,optee-channel-id:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Channel specifier required when using OP-TEE transport and
+          protocol has a dedicated communication channel.
+
     required:
       - reg
 
@@ -226,6 +240,16 @@  else:
       - arm,smc-id
       - shmem
 
+  else:
+    if:
+      properties:
+        compatible:
+          contains:
+            const: linaro,scmi-optee
+    then:
+      required:
+        - linaro,optee-channel-id
+
 examples:
   - |
     firmware {
@@ -340,7 +364,48 @@  examples:
                 reg = <0x11>;
                 #power-domain-cells = <1>;
             };
+        };
+    };
+
+  - |
+    firmware {
+        scmi {
+            compatible = "linaro,scmi-optee";
+            linaro,optee-channel-id = <0>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            scmi_dvfs1: protocol@13 {
+                reg = <0x13>;
+                linaro,optee-channel-id = <1>;
+                shmem = <&cpu_optee_lpri0>;
+                #clock-cells = <1>;
+            };
+
+            scmi_clk0: protocol@14 {
+                reg = <0x14>;
+                #clock-cells = <1>;
+            };
+        };
+    };
 
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        sram@51000000 {
+            compatible = "mmio-sram";
+            reg = <0x0 0x51000000 0x0 0x10000>;
+
+            #address-cells = <1>;
+            #size-cells = <1>;
+            ranges = <0 0x0 0x51000000 0x10000>;
+
+            cpu_optee_lpri0: optee-sram-section@0 {
+                compatible = "arm,scmi-shmem";
+                reg = <0x0 0x80>;
+            };
         };
     };