mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
To: Sumit Garg <sumit.garg@kernel.org>
Cc: Bjorn Andersson <andersson@kernel.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Jens Wiklander <jens.wiklander@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	<linux-stm32@st-md-mailman.stormreply.com>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-remoteproc@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	<op-tee@lists.trustedfirmware.org>, <devicetree@vger.kernel.org>
Subject: Re: [PATCH v19 4/6] dt-bindings: remoteproc: Add compatibility for TEE support
Date: Wed, 17 Sep 2025 15:47:40 +0200	[thread overview]
Message-ID: <8d385f2e-6470-4d66-be0b-a2a448a81fa4@foss.st.com> (raw)
In-Reply-To: <aMqIhFdIqp5auH22@sumit-X1>



On 9/17/25 12:08, Sumit Garg wrote:
> On Tue, Sep 16, 2025 at 03:26:47PM +0200, Arnaud POULIQUEN wrote:
>> Hello Sumit,
>>
>> On 9/16/25 11:14, Sumit Garg wrote:
>>> Hi Arnaud,
>>>
>>> First of all apologies for such a late review comment as previously I
>>> wasn't CCed or involved in the review of this patch-set. In case any of
>>> my following comments have been discussed in the past then feel free to
>>> point me at relevant discussions.
>> No worries, there are too many versions of this series to follow all the
>> past discussions. I sometimes have difficulty remembering all the
>> discussions myself :)
>>
>>> On Wed, Jun 25, 2025 at 11:40:26AM +0200, Arnaud Pouliquen wrote:
>>>> The "st,stm32mp1-m4-tee" compatible is utilized in a system configuration
>>>> where the Cortex-M4 firmware is loaded by the Trusted Execution Environment
>>>> (TEE).
>>> Having a DT based compatible for a TEE service to me just feels like it
>>> is redundant here. I can see you have also used a TEE bus based device
>>> too but that is not being properly used. I know subsystems like
>>> remoteproc, SCMI and others heavily rely on DT to hardcode properties of
>>> system firmware which are rather better to be discovered dynamically.
>>>
>>> So I have an open question for you and the remoteproc subsystem
>>> maintainers being:
>>>
>>> Is it feasible to rather leverage the benefits of a fully discoverable
>>> TEE bus rather than relying on platform bus/ DT to hardcode firmware
>>> properties?
>> The discoverable TEE bus does not works if the remoteproc is probed
>> before the OP-TEE bus, in such case  no possibility to know if the TEE
>> TA is not yet available or not available at all.
>> This point is mentioned in a comment in rproc_tee_register().

For the discussion, it’s probably better if I provide more details on the
current OP-TEE implementation and the stm32mp processors.

1) STM32MP topology:
   - STM32MP1: only a Cortex-M4 remote processor
   -  STM32MP2x: a Cortex-M33 and a Cortex-M0 remote processors
   At this stage, only the STM32MP15 is upstreamed; the STM32MP25 is waiting
   for this series to be merged.

2) OP-TEE architecture:
- A platform-agnostic Trusted Application (TA) handles the bus service.[1]
   This TA supports managing multiple remote processors. It can be embedded
   regardless of the number of remote processors managed in OP-TEE.
   The decision to embed this service is made at build time based on the
   presence of the remoteproc driver, so it is not device tree dependent.
   - STM32MP15: TA activated only if the remoteproc OP-TEE driver is probed
   - STM32MP2x: TA always activated as the OP-TEE remoteproc driver is 
always probed

- A pseudo Trusted Application implements the platform porting[2],
   relying on registered remoteproc platform drivers.

- Platform driver(s) manage the remote processors.[3][4]
   - If remoteproc is managed by OP-TEE: manages the remoteproc lifecycle
   - If remoteproc is managed by Linux: provides access rights to Linux 
to manage
     the remoteproc

   - STM32MP15: driver probed only if the remoteproc is managed in OP-TEE
   - STM32MP2x: driver probed in both cases for the Cortex-M33
     For the STM32MP25, the TA is always present and queries the driver 
to check
     if it supports secure loading.


[1] https://elixir.bootlin.com/op-tee/4.7.0/source/ta/remoteproc
[2] 
https://elixir.bootlin.com/op-tee/4.7.0/source/core/pta/stm32mp/remoteproc_pta.c
[3]https://elixir.bootlin.com/op-tee/4.7.0/source/core/drivers/remoteproc/stm32_remoteproc.c
[4]https://github.com/STMicroelectronics/optee_os/blob/4.0.0-stm32mp/core/drivers/remoteproc/stm32_remoteproc.c 

> The reason here is that you are mixing platform and TEE bus for remoteproc
> driver. For probe, you rely on platform bus and then try to migrate to
> TEE bus via rproc_tee_register() is the problem here. Instead you should
> rather probe remoteproc device on TEE bus from the beginning.

The approach is interesting, but how can we rely on Device Tree (DT) for
hardware configuration in this case?
At a minimum, I need to define memory regions and mailboxes.

 From my perspective, I would still need a driver probed by DT that 
registers
a driver on the TEE bus. Therefore, I still need a mechanism to decide
whether the remote firmware is managed by the secure or non-secure context.

Another issue would be to be able to share the remoteproc TEE service 
between
several platform remoteproc drivers, in case of multi remote processor 
support.

>
>> Then, it is not only a firmware property in our case. Depending on the
>> compatible string, we manage the hardware differently. The same compatibles
>> are used in both OP-TEE and Linux. Based on the compatible, we can assign
>> memories, clocks, and resets to either the secure or non-secure context.
>> This approach is implemented on the STM32MP15 and STM32MP2x platforms.
> You should have rather used the DT property "secure-status" [1] to say
> the remoteproc device is being managed by OP-TEE instead of Linux. Then
> the Linux driver will solely rely on TEE bus to have OP-TEE mediated
> remoteproc device.
>
> [1] https://github.com/devicetree-org/dt-schema/blob/4b28bc79fdc552f3e0b870ef1362bb711925f4f3/dtschema/schemas/dt-core.yaml#L52

My issue with this property is that this would break the compatibility with
legacy DT that only support loading by Linux
As specified in [5] :If "secure-status" is not specified it defaults to 
the same value as "status"; [5] 
https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/secure.txt

>> More details are available in the ST WIKI:
>> https://wiki.st.com/stm32mpu/wiki/OP-TEE_remoteproc_framework_overview#Device_tree_configuration
>> https://wiki.st.com/stm32mpu/wiki/Linux_remoteproc_framework_overview#Device_tree_configuration
>>
>>>> For instance, this compatible is used in both the Linux and OP-TEE device
>>>> trees:
>>>> - In OP-TEE, a node is defined in the device tree with the
>>>>     "st,stm32mp1-m4-tee" compatible to support signed remoteproc firmware.
>>>>     Based on DT properties, the OP-TEE remoteproc framework is initiated to
>>>>     expose a trusted application service to authenticate and load the remote
>>>>     processor firmware provided by the Linux remoteproc framework, as well
>>>>     as to start and stop the remote processor.
>>>> - In Linux, when the compatibility is set, the Cortex-M resets should not
>>>>     be declared in the device tree. In such a configuration, the reset is
>>>>     managed by the OP-TEE remoteproc driver and is no longer accessible from
>>>>     the Linux kernel.
>>>>
>>>> Associated with this new compatible, add the "st,proc-id" property to
>>>> identify the remote processor. This ID is used to define a unique ID,
>>>> common between Linux, U-Boot, and OP-TEE, to identify a coprocessor.
>>> This "st,proc-id" is just one such property which can rather be directly
>>> probed from the TEE/OP-TEE service rather than hardcoding it in DT here.
>> Do you mean a topology discovery mechanism through the TEE remoteproc
>> service?
>>
>> For the STM32MP15, it could work since we have only one remote processor.
>> However, this is not the case for the STM32MP25, which embeds both a
>> Cortex-M33 and a Cortex-M0.
> I rather mean here whichever properties you can currently dicovering via
> DT can rather be discovered by invoke command taking property name as input
> and value as output.

That would means services to get system resources such as memory region
mailbox, right?

>
>> Could you please elaborate on how you see the support of multiple remote
>> processors without using an hardcoded identifier?
> By multiple remote processors, do you mean there can be multiple
> combinations of which remote processor gets managed via OP-TEE or not?

On stm32mp25 we have 2 remote processors a cortex-M33 and a cortex-M0
We should be able to manage them using the proc_idAnother point is that 
We should allow an other Secure OS could implement the TEE remoteproc 
service managing the remote processors with different proc_id values, to 
avoid to specify somewhere an unique proc ID per remote processor.

>>> I think the same will apply to other properties as well.
>> Could you details the other properties you have in mind?
> I think the memory regions including the resource table can also be
> probed directly from the TEE service too. Is there any other DT property
> you rely upon when remoteproc is managed via OP-TEE?

The memory regions that include the resource table are already declared
in OP-TEE. The memory regions defined in the Linux device tree are for
RPMsg (IPC). These memories are registered by the Linux remoteproc driver
in the Linux rproc core.

Thanks,
Arnaud

>
> -Sumit


  reply	other threads:[~2025-09-17 13:48 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-25  9:40 [PATCH v19 0/6] Introduction of a remoteproc tee to load signed firmware Arnaud Pouliquen
2025-06-25  9:40 ` [PATCH v19 1/6] remoteproc: core: Introduce rproc_pa_to_va helper Arnaud Pouliquen
2025-06-25  9:40 ` [PATCH v19 2/6] remoteproc: Add TEE support Arnaud Pouliquen
2025-07-31 10:25   ` Harshal Dev
2025-08-01  7:23     ` Arnaud POULIQUEN
2025-08-04  9:26       ` Harshal Dev
2025-08-11 14:11         ` Sumit Garg
2025-08-14 10:47           ` Harshal Dev
2025-08-18  5:07             ` Sumit Garg
2025-08-13 16:42   ` Abdellatif El Khlifi
2025-08-14  7:17     ` Sumit Garg
2025-06-25  9:40 ` [PATCH v19 3/6] remoteproc: Introduce optional release_fw operation Arnaud Pouliquen
2025-06-25  9:40 ` [PATCH v19 4/6] dt-bindings: remoteproc: Add compatibility for TEE support Arnaud Pouliquen
2025-09-16  9:14   ` Sumit Garg
2025-09-16 13:26     ` Arnaud POULIQUEN
2025-09-17 10:08       ` Sumit Garg
2025-09-17 13:47         ` Arnaud POULIQUEN [this message]
2025-09-19  6:46           ` Sumit Garg
2025-09-22  8:57             ` Arnaud POULIQUEN
2025-10-07 13:50               ` [Linux-stm32] " Arnaud POULIQUEN
2025-10-08 16:38                 ` Mathieu Poirier
2025-10-10  5:58                   ` Sumit Garg
2025-10-10  5:44                 ` Sumit Garg
2025-10-10 10:04                   ` Arnaud POULIQUEN
2025-10-11 22:49                     ` Sumit Garg
2025-10-13  9:46                       ` Arnaud POULIQUEN
2025-06-25  9:40 ` [PATCH v19 5/6] remoteproc: stm32: Create sub-functions to request shutdown and release Arnaud Pouliquen
2025-06-25  9:40 ` [PATCH v19 6/6] remoteproc: stm32: Add support of an OP-TEE TA to load the firmware Arnaud Pouliquen
2025-09-12 16:03 ` [PATCH v19 0/6] Introduction of a remoteproc tee to load signed firmware Arnaud POULIQUEN
2025-09-15 16:44   ` Mathieu Poirier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8d385f2e-6470-4d66-be0b-a2a448a81fa4@foss.st.com \
    --to=arnaud.pouliquen@foss.st.com \
    --cc=andersson@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jens.wiklander@linaro.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=mathieu.poirier@linaro.org \
    --cc=op-tee@lists.trustedfirmware.org \
    --cc=robh+dt@kernel.org \
    --cc=sumit.garg@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®