devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
To: Sumit Garg <sumit.garg@kernel.org>
Cc: <devicetree@vger.kernel.org>, Conor Dooley <conor+dt@kernel.org>,
	"Mathieu Poirier" <mathieu.poirier@linaro.org>,
	<op-tee@lists.trustedfirmware.org>,
	Bjorn Andersson <andersson@kernel.org>,
	<linux-remoteproc@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	<linux-stm32@st-md-mailman.stormreply.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Jens Wiklander <jens.wiklander@linaro.org>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [Linux-stm32] [PATCH v19 4/6] dt-bindings: remoteproc: Add compatibility for TEE support
Date: Tue, 7 Oct 2025 15:50:32 +0200	[thread overview]
Message-ID: <f5b0e106-a731-461b-b401-1aa1f9892465@foss.st.com> (raw)
In-Reply-To: <0e5a44df-f60a-4523-a791-6318b3c81425@foss.st.com>

Hello Bjorn, Mathieu, Sumit,

On 9/22/25 10:57, Arnaud POULIQUEN wrote:
> 
> 
> On 9/19/25 08:46, Sumit Garg wrote:
>> On Wed, Sep 17, 2025 at 03:47:40PM +0200, Arnaud POULIQUEN wrote:
>>>
>>> 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
>> Thanks for the background here.
>>
>>>> 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.
>> The hardware configuration in DT should be consumed by OP-TEE and the
>> kernel probes remoteproc properties from OP-TEE since it's an OP-TEE
>> mediated remoteproc service you are adding here.
>>>  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.
>> As I mentioned below, this should be achievable using the secure-status
>> property without introducing the new compatible:
>>
>> Kernel managed remoteproc:
>>     status = "okay"; secure-status = "disabled";     /* NS-only */
>>
>> OP-TEE managed remoteproc:
>>     status = "disabled"; secure-status = "okay";     /* S-only */
>>
>>> 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.
>> Making the TEE based remoteproc service independent of DT will surely
>> make it more scalable to other platforms too. Have a look at how OP-TEE
>> based HWRNG service scales across platforms.
> 
> Another important service is SCMI, which drivers use to manage clocks 
> and resets.
> These clocks and resets are declared in the Device Tree (DT). It seems 
> to me that
> in this case, we are closer to SCMI than to the RNG service.
> 
> I propose we discuss this based on a concrete example with the STM32MP25.
> Although not yet upstreamed, our plan is to manage signed firmware for the
> Cortex-M33 and Cortex-M0.
> 
> Please find below my view of the DT resources to address.
> 
> STM32MP25  Cortex-M33 and Cortex-M0 nodes:
> 
> m33_rproc {
>    /* M33 watchdog interrupt */
>    interrupt-parent = <&intc>;
>    interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
>    /* power domain management */
>    power-domains = <&cluster_pd>, <&ret_pd>;
>    power-domain-names = "default", "sleep";
>    /* RPMsg mailboxes + M33 graceful shutdown request */
>    mboxes = <&ipcc1 0x0>, <&ipcc1 0x1>, <&ipcc1 2>;
>    mbox-names = "vq0", "vq1", "shutdown";
>    memory-region = <&vdev0vring0>, <&vdev0vring1>, <&vdev0buffer>;
>    status = "okay";
> };
> 
> m0_rproc {
>    /* mailbox for graceful shutdown */
>    mboxes = <&ipcc2 2>;
>    mbox-names = "shutdown";
>    /* M0 watchdog */
>   interrupt-parent = <&intc>;
>   interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
>   /* M0 peripheral clocking (not accessible by the M0) */
>   clocks = <&scmi_clk CK_SCMI_GPIOZ_AM>,
>   <&scmi_clk CK_SCMI_GPIOZ>,
>   <&scmi_clk CK_SCMI_IPCC2>,
>   <&scmi_clk CK_SCMI_IPCC2_AM>,
>   <&rcc CK_LPTIM3_AM>,
>   <&rcc CK_LPUART1_AM>,
>   <&rcc CK_CPU3_AM>,
>   <&rcc CK_CPU3>,
>   <&rcc CK_LPUART1_C3>,
>   <&rcc CK_GPIOZ_C3>,
>   <&rcc CK_LPTIM3_C3>,
>   <&rcc CK_KER_LPUART1>,
>   <&rcc CK_KER_LPTIM3>,
>   <&scmi_clk CK_SCMI_GPIOZ>,
>   <&scmi_clk CK_SCMI_IPCC2>;
>   status = "okay";
> };
> 
> If we want to remove the DT, we need to consider:
> 
> - Mechanism to differentiate Cortex-M33 and Cortex-M0:
>    Similar to SCMI, the remoteproc OP-TEE service should support
>   multiprocessor setups without instantiating multiple services.
> 
> - Mailboxes:
> 
>    A phandle is needed because the mailbox driver is managed by the
>    Linux mailbox driver. STM32MP2 has two mailboxes.
>    Moving towards your proposal would imply creating a mailbox service
>    in TEE to manage non-secure mailboxes for non-secure IPC. This might
>    not be efficient for inter-processor communication. Hardware-wise, it
>    would require the IRQ to be handled by the secure context.
> 
> - Memory regions:
>   - Hardware limitation: OP-TEE is limited in the number of memory regions
>     it can declare due to Firewall configuration. Moving IPC memory regions
>     reaches this limit. Currently, OP-TEE defines a single region with 
> shareable
>     access rights, which Linux splits into at least three memory regions 
> for RPMsg.
>   - Memory mapping: Memory regions still need to be declared in Linux to 
> prevent
>     Linux from using them.
>   - Virtio/RPMsg: Memory region names are fixed (e.g., dev<X>vring<Y>), 
> so OP-TEE
>    must declare memory regions in its DT according to Linux naming 
> conventions.
> 
> - Clock and reset:
>     Some clocks and resets are managed via SCMI, others are not. This 
> would require
>    managing all clocks and resets through SCMI, with possible side 
> effect on the
>    "unused" clock mechanism in Linux ( to be confirmed)
> 
> - Power domain:
>    Information is needed at the Linux level to determine the low power 
> mode.
> 
> - Watchdog interrupt:
>    Should be managed by OP-TEE, which requires the hardware to have an 
> associated
>    secure IRQ.
> 
> - Miscellaneous vendor DT properties:
>     How to be sure that these can be addressed through TEE services?
> 
> Regarding the existing DT needs, it seems to me that removing the DT 
> would require
> moving all node resource management into TEE ( if really possible). This 
> would
> increase TEE complexity and footprint, reduce system efficiency, and 
> make supporting
> other platforms less scalable.
> 
> That said, it probably also depends on the TEE implementation.
> And  we should support both. This could be done by introducing a second 
> UUID.
> but in this case should it be the same driver?

I am unsure how to move forward here. It seems to me that addressing Sumit's
request for a TEE without a device tree is not compatible with the current
OP-TEE implementation, at least for the STM32MP platforms.

Perhaps the simplest approach is to abandon the effort to make this generic
and instead rename tee_remoteproc.c to stm32_tee_remoteproc.c, making it
platform-dependent. Then, if another platform wants to reuse it with OP-TEE
FFA or another TEE, the file can be renamed.

Does this proposal would make sense to you?

Thanks and Regards,
Arnaud

> 
>>
>>>>> 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
>> No, it's not a DT ABI break at all. It is always possible for a
>> hardware to be re-configured to change assignment of peripherals among
>> OP-TEE and Linux kernel.
>>
>>> 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
>> This is mostly meant for peripherals that can be probed by both OP-TEE
>> and Linux kernel via DT. But here in case of remoteproc, there needs to
>> exclusive access control for either via Linux kernel or OP-TEE. Hence, 
>> the
>> "status" and "secure-status" properties should be updated accordingly.
>>
>>>>> 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?
>> Yeah.
>>
>>>>> 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.
>> Okay I see, so you can add unique proc ID to DT which gets consumed by
>> OP-TEE and Linux discovers the same via the TEE service.
> Yes the Linux passes the proc ID as argument of the 
> tee_client_open_session().
> In OP-TEE, the TEE service checks the match with the proc ID registered 
> by the
> OP-TEE remote proc drivers.
> 
> Regards,
> Arnaud
> 
>>
>>>>>> 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.
>>>
>> Sure, so they can also be discovered by TEE service.
>>
>> -Sumit
> 
> _______________________________________________
> Linux-stm32 mailing list
> Linux-stm32@st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32


  reply	other threads:[~2025-10-07 13:51 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
2025-09-19  6:46           ` Sumit Garg
2025-09-22  8:57             ` Arnaud POULIQUEN
2025-10-07 13:50               ` Arnaud POULIQUEN [this message]
2025-10-08 16:38                 ` [Linux-stm32] " 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=f5b0e106-a731-461b-b401-1aa1f9892465@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;
as well as URLs for NNTP newsgroup(s).