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:
> > Hello Harshal,
> >
> >
> > On 7/31/25 12:25, Harshal Dev wrote:
> >> Hello Arnaud,
> >>
> >> On 6/25/2025 3:10 PM, Arnaud Pouliquen wrote:
> >>> 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.pouliq...@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>
> >>> +
> >>> +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
