Re: [PATCH v4 08/11] tee: add Qualcomm TEE driver

Kuldeep Singh
2025-05-15T09:06:06Z

On 5/14/2025 6:55 PM, Sumit Garg wrote:
> Hi Amir,
>
> Apologies for getting to this patch review a bit late, mostly due to
> it's enormous size.
>
> On Mon, Apr 28, 2025 at 11:06:29PM -0700, Amirreza Zarrabi wrote:
>> Introduce qcomtee_object, which represents an object in both QTEE and
>> the kernel. QTEE clients can invoke an instance of qcomtee_object to
>> access QTEE services. If this invocation produces a new object in QTEE,
>> an instance of qcomtee_object will be returned.
>>
>> Similarly, QTEE can request services from the kernel by issuing a callback
>> request, which invokes an instance of qcomtee_object in the kernel.
>> Any subsystem that exposes a service to QTEE should allocate and initialize
>> an instance of qcomtee_object with a dispatcher callback that is called
>> when the object is invoked.
>
> I can't see any kernel subsystem exposing a service to QTEE as of now. I
> suppose RPMB is surely going to be the one. So I would suggest you to
> drop exposing kernel APIs for that and instead they should be pushed
> alongside a patch-set adding a real kernel service for QTEE. This will
> help in the review as well.
>
>>
>> Implement initial support for exporting qcomtee_object to userspace
>> and QTEE, enabling the invocation of objects hosted in QTEE and userspace
>> through the TEE subsystem.
>
> I think this is the main goal of this patch-set, so we should limit it
> to that.
>
>>
>> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
>> ---
>> MAINTAINERS | 7 +
>> drivers/tee/Kconfig | 1 +
>> drivers/tee/Makefile | 1 +
>> drivers/tee/qcomtee/Kconfig | 10 +
>> drivers/tee/qcomtee/Makefile | 9 +
>> drivers/tee/qcomtee/async.c | 160 +++++++
>> drivers/tee/qcomtee/call.c | 764 +++++++++++++++++++++++++++++++
>> drivers/tee/qcomtee/core.c | 806 +++++++++++++++++++++++++++++++++
>> drivers/tee/qcomtee/qcom_scm.c | 38 ++
>> drivers/tee/qcomtee/qcomtee_msg.h | 239 ++++++++++
>> drivers/tee/qcomtee/qcomtee_private.h | 222 +++++++++
>> drivers/tee/qcomtee/release.c | 48 ++
>> drivers/tee/qcomtee/shm.c | 149 ++++++
>> drivers/tee/qcomtee/user_obj.c | 713 +++++++++++++++++++++++++++++
>> include/linux/firmware/qcom/qcom_tee.h | 302 ++++++++++++
>> include/uapi/linux/tee.h | 1 +
>> 16 files changed, 3470 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 906881b6c5cb..88a9ad34bcf6 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -20257,6 +20257,13 @@ F: Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
>> F: drivers/net/ethernet/qualcomm/rmnet/
>> F: include/linux/if_rmnet.h
>>
>> +QUALCOMM TEE (QCOMTEE) DRIVER
>> +M: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
>> +L: linux-arm-msm@vger.kernel.org
>> +S: Maintained
>> +F: drivers/tee/qcomtee/
>> +F: include/linux/firmware/qcom/qcom_tee.h
>> +
>> QUALCOMM TRUST ZONE MEMORY ALLOCATOR
>> M: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> L: linux-arm-msm@vger.kernel.org
>> diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
>> index 61b507c18780..3a995d7f0d74 100644
>> --- a/drivers/tee/Kconfig
>> +++ b/drivers/tee/Kconfig
>> @@ -16,5 +16,6 @@ if TEE
>> source "drivers/tee/optee/Kconfig"
>> source "drivers/tee/amdtee/Kconfig"
>> source "drivers/tee/tstee/Kconfig"
>> +source "drivers/tee/qcomtee/Kconfig"
>>
>> endif
>> diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile
>> index 5488cba30bd2..74e987f8f7ea 100644
>> --- a/drivers/tee/Makefile
>> +++ b/drivers/tee/Makefile
>> @@ -6,3 +6,4 @@ tee-objs += tee_shm_pool.o
>> obj-$(CONFIG_OPTEE) += optee/
>> obj-$(CONFIG_AMDTEE) += amdtee/
>> obj-$(CONFIG_ARM_TSTEE) += tstee/
>> +obj-$(CONFIG_QCOMTEE) += qcomtee/
>> diff --git a/drivers/tee/qcomtee/Kconfig b/drivers/tee/qcomtee/Kconfig
>> new file mode 100644
>> index 000000000000..d180a6d07d33
>> --- /dev/null
>> +++ b/drivers/tee/qcomtee/Kconfig
>> @@ -0,0 +1,10 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +# Qualcomm Trusted Execution Environment Configuration
>> +config QCOMTEE
>> + tristate "Qualcomm TEE Support"
>> + select QCOM_SCM
>
> You should add a dependency on QCOM_TZMEM_MODE_SHMBRIDGE too here.

I think you meant QCOM_TZMEM because MODE_SHMBRIDGE is one aspect just
like MODE_GENERIC.

--
Regards
Kuldeep