Hi Mathieu, On Wed, Oct 08, 2025 at 10:38:07AM -0600, Mathieu Poirier wrote: > On Tue, 7 Oct 2025 at 07:50, Arnaud POULIQUEN > <[email protected]> wrote: > > > > 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? > > I would certainly like to see a consensus, and more specifically, an > implementation that follows what other drivers that interact with the > secure world do. I currently do not have a clear understanding of > what those other drivers do, and doing the research will take > bandwidth that I also currently do not have.
The major problem I see with this patchset is the probing of remoteproc on platform bus and then try to move to a discoverable TEE bus. As I replied in the other thread, we should address the unavoidable DT dependency following what other discoverable buses like PCI etc. does. -Sumit > This situation is > expected to persist at least until December. > > As such I see two avenues for this patchset: > (1) You seek to find a solution that is amenable to you, Sumit, > Abdellatif and Harshal (I had to add the latter two to this email > thread). > (2) You wait until December, and likely beyond, until I have time to > do the research needed to advise on the way forward. >
