2021-10-28 14:03:09

by Etienne Carriere

[permalink] [raw]
Subject: [PATCH v8 1/2] dt-bindings: arm: Add OP-TEE transport for SCMI

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: [email protected]
Cc: Rob Herring <[email protected]>
Signed-off-by: Etienne Carriere <[email protected]>
---
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>;
+ };
};
};

--
2.17.1


2021-10-29 10:22:51

by Cristian Marussi

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] dt-bindings: arm: Add OP-TEE transport for SCMI

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: [email protected]
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Etienne Carriere <[email protected]>
> ---

Reviewed-by: Cristian Marussi <[email protected]>

Thanks,
Cristian

2021-11-02 13:24:44

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] dt-bindings: arm: Add OP-TEE transport for SCMI

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: [email protected]
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Etienne Carriere <[email protected]>
> ---
> 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 <[email protected]>

2021-11-25 12:27:26

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] dt-bindings: arm: Add OP-TEE transport for SCMI

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


2021-11-25 12:43:45

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] dt-bindings: arm: Add OP-TEE transport for SCMI

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


2022-02-28 17:45:29

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] dt-bindings: arm: Add OP-TEE transport for SCMI

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/[email protected]/

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: [email protected]
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Etienne Carriere <[email protected]>
> ---
> 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 |

2022-03-01 15:27:25

by Etienne Carriere

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] dt-bindings: arm: Add OP-TEE transport for SCMI

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
<[email protected]> wrote:
>
> Hello Ahmad,
>
> On Mon, 28 Feb 2022 at 17:01, Ahmad Fatoum <[email protected]> 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/[email protected]/
> >
> > 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: [email protected]
> > > Cc: Rob Herring <[email protected]>
> > > Signed-off-by: Etienne Carriere <[email protected]>
> > > ---
> > > 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 |

2022-03-01 19:02:06

by Etienne Carriere

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] dt-bindings: arm: Add OP-TEE transport for SCMI

Hello Ahmad,

On Mon, 28 Feb 2022 at 17:01, Ahmad Fatoum <[email protected]> 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/[email protected]/
>
> 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: [email protected]
> > Cc: Rob Herring <[email protected]>
> > Signed-off-by: Etienne Carriere <[email protected]>
> > ---
> > 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 |

2022-03-01 20:15:04

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] dt-bindings: arm: Add OP-TEE transport for SCMI


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/[email protected]/
>
> 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.

--
Regards,
Sudeep

2022-03-08 10:58:53

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] dt-bindings: arm: Add OP-TEE transport for SCMI

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
> <[email protected]> wrote:
>>
>> Hello Ahmad,
>>
>> On Mon, 28 Feb 2022 at 17:01, Ahmad Fatoum <[email protected]> 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/[email protected]/
>>>
>>> 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: [email protected]
>>>> Cc: Rob Herring <[email protected]>
>>>> Signed-off-by: Etienne Carriere <[email protected]>
>>>> ---
>>>> 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 |
>


--
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 |

2022-03-08 15:50:45

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] dt-bindings: arm: Add OP-TEE transport for SCMI

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/[email protected]/
>>
>> 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

--
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 |

2022-03-09 01:23:08

by Etienne Carriere

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] dt-bindings: arm: Add OP-TEE transport for SCMI

Hello Ahmad,

On Tue, 8 Mar 2022 at 10:51, Ahmad Fatoum <[email protected]> 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/[email protected]/
> >>
> >> 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 |

2022-03-17 04:21:50

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [PATCH v8 1/2] dt-bindings: arm: Add OP-TEE transport for SCMI

Hello Etienne,

On 08.03.22 11:18, Etienne Carriere wrote:
> Hello Ahmad,
>
> On Tue, 8 Mar 2022 at 10:51, Ahmad Fatoum <[email protected]> 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/[email protected]/
>>>>
>>>> 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

--
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 |