Hi Harshal,

On Mon, Aug 04, 2025 at 02:56:18PM +0530, Harshal Dev wrote:

Hi Arnaud,

On 8/1/2025 12:53 PM, Arnaud POULIQUEN wrote:

quoted

Hello Harshal,


On 7/31/25 12:25, Harshal Dev wrote:

quoted

Hello Arnaud,

On 6/25/2025 3:10 PM, Arnaud Pouliquen wrote:

quoted

Add a remoteproc TEE (Trusted Execution Environment) driver that will be
probed by the TEE bus. If the associated Trusted application is supported
on the secure part, this driver offers a client interface to load firmware
by the secure part.
This firmware could be authenticated by the secure trusted application.

A specificity of the implementation is that the firmware has to be
authenticated and optionally decrypted to access the resource table.

Consequently, the boot sequence is:

1) rproc_parse_fw --> rproc_tee_parse_fw
   remoteproc TEE:
   - Requests the TEE application to authenticate and load the firmware
     in the remote processor memories.
   - Requests the TEE application for the address of the resource table.
   - Creates a copy of the resource table stored in rproc->cached_table.

2) rproc_load_segments --> rproc_tee_load_fw
   remoteproc TEE:
   - Requests the TEE application to load the firmware. Nothing is done
     at the TEE application as the firmware is already loaded.
   - In case of recovery, the TEE application has to reload the firmware.

3) rproc_tee_get_loaded_rsc_table
   remoteproc TEE requests the TEE application for the address of the
   resource table.

4) rproc_start --> rproc_tee_start
   - Requests the TEE application to start the remote processor.

The shutdown sequence is:

5) rproc_stop --> rproc_tee_stop
   - Requests the TEE application to stop the remote processor.

6) rproc_tee_release_fw
   This function is used to request the TEE application to perform actions
   to return to the initial state on stop or on error during the boot
   sequence.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
---
Updates vs version [18]:
- rework/fix function headers
- use memremap instead of ioremap for the resource table.
- realign comments to 80 chars limit, with few exceptions for readability
- replace spinlock by mutex and and protect APIs from concurrent access
- add support of 64-bit address in rproc_tee_get_loaded_rsc_table()
- Generalize teston rproc_tee_ctx.dev to prevent an unbind
- update copyright year

Updates vs version [17]:
Fix warning:
warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
---
 drivers/remoteproc/Kconfig          |  10 +
 drivers/remoteproc/Makefile         |   1 +
 drivers/remoteproc/remoteproc_tee.c | 708 ++++++++++++++++++++++++++++
 include/linux/remoteproc_tee.h      |  87 ++++
 4 files changed, 806 insertions(+)
 create mode 100644 drivers/remoteproc/remoteproc_tee.c
 create mode 100644 include/linux/remoteproc_tee.h

<snip>

quoted

quoted

quoted

+
+static int rproc_tee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
+{
+	/* Today we support only the OP-TEE, could be extend to other tees */
+	return (ver->impl_id == TEE_IMPL_ID_OPTEE);
+}
+
+static int rproc_tee_probe(struct device *dev)
+{
+	struct tee_context *tee_ctx;
+	int ret;
+
+	/* Open context with TEE driver */
+	tee_ctx = tee_client_open_context(NULL, rproc_tee_ctx_match, NULL, NULL);
+	if (IS_ERR(tee_ctx))
+		return PTR_ERR(tee_ctx);
+
+	ret = mutex_lock_interruptible(&ctx_lock);
+	if (ret)
+		return ret;
+
+	rproc_tee_ctx.dev = dev;
+	rproc_tee_ctx.tee_ctx = tee_ctx;
+	INIT_LIST_HEAD(&rproc_tee_ctx.sessions);
+	mutex_unlock(&ctx_lock);
+
+	return 0;
+}

As you mentioned above, this could be extended to other TEEs. If so, is it possible for probe
to be called multiple times if we we have other TEE devices exposing the firmware authentication
service? In that case, I think rproc_tee_ctx should be dynamically initializated instead of being
static. And since we are creating a link between the Rproc device and TEE device, a call to a
function like rproc_tee_start() could retreive the associated TEE device, and then the associated
rproc_tee? :)

I have never seen a use case that requires multiple instances, but perhaps you
have some?

We can expect only one TEE, which could be OP-TEE, Trusty, or another.
The device is associated with a unique UUID, so only one instance is expected.

That said, making this driver support multiple instances seems like a valid
future enhancement. However, I would suggest implementing it as a second step
when there is a concrete need.

My thought process on this stems from 1) the recent ARM FF-A developments and 2) from the current
implementation of the TEE subsystem which allows multiple back-end drivers to register themselves
via the tee_device_register() API. This means, that it's possible to have a configuration
where a platform supports multiple TEEs running as Secure Partitions via FF-A, and each of those
TEEs register their services as PTA devices on the TEE bus.

However, I do not really know if it's possible to have a UUID collision in such a case, which
would lead to rproc_tee_probe() being called twice above, which is why I raised this question. :)

All of this aside, I realize now that other TEE client drivers are also implemented with a static
private data similar to how you are doing it. So perhaps we can think of this as a later
enhancement if we believe that the scenario I am describing is not possible in the near future..

Theoretically it is possible for multiple TEE services to be there but
why should a platform/silicon vendor require 2 redundant remoteproc firmware
loading services to be supported? It should either be a service hosted
by the trusted OS or can rather be an independent platform service
running as a FF-A secure partition.

-Sumit