On Thu, Aug 14, 2025 at 04:17:55PM +0530, Harshal Dev wrote:
> Hi Sumit,
>
> On 8/11/2025 7:41 PM, Sumit Garg wrote:
> > 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.
> >
> I agree that it doesn't make sense for a system integrator to have two
> remoteproc firmware
> loading services supported from two different TEEs running as Secure
> Partitions.
> After all, one service exposed by one TEE is good enough for fulfilling any
> use-case.
>
> My concern is that ARM FF-A makes its possible to have a platform running two
> TEEs, which
> each have their own remoteproc firmware authentication service implemented
> (as usually TEEs do).
> In such a scenario, when both TEEs enumerate their services on the TEE bus,
> and find a match
> because the rproc_tee_id_table has a UUID for say, both the TS-TEE remoteproc
> service and
> OP-TEE remoteproc service, rproc_tee_probe() will be called twice, and the
> current implementation
> will break because it uses a single static rproc_tee_ctx, whose contents
> would be overwritten
> leading to unexpected scenarios.
>
> And so, should TEE subsystem clients (like this one) be prepared to handle
> such as scenario?
With Arm FF-A and platforms supporting S-EL2, yes it is possible to run
multiple trusted OS in parallel. However, the recommendation is to then
host the platform specific services as independent FF-A based Secure
Partitions (SPs) which aren't tied to any particular trusted OS. I have
CCed some Arm folks who on the other thread were interested to extend
this driver to support remoteproc service hosted as a FF-A based secure
partition.
-Sumit
