| Message ID | 20250428-qcom-tee-using-tee-ss-without-mem-obj-v4-6-6a143640a6cb@oss.qualcomm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Trusted Execution Environment (TEE) driver for Qualcomm TEE (QTEE) | expand |
On 4/29/2025 11:36 AM, Amirreza Zarrabi wrote: > shm_bridge create/delete functions always use the scm device. > There is no need to pass it as an argument. > > Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com> There are 2 type of APIs exposed by tzmem driver for pool creation. devm_qcom_tzmem_pool_new and qcom_tzmem_pool_new. Device managed pool is created with devm_qcom_tzmem_pool_new but currently qcom_scm is using it's own dev to create/delete bridge which is problamatic here. https://elixir.bootlin.com/linux/v6.14.5/source/drivers/firmware/qcom/qcom_scm.c#L1653 If pool is device managed, same dev should be used in qcom_scm to create/delete bridge rather than using qcom_scm dev. The dev passed as an argument to function should be used instead of __scm->dev. https://elixir.bootlin.com/linux/v6.14.5/source/drivers/firmware/qcom/qcom_scm.c#L1634 To summarize, I believe correct solution should be to pass corresponding dev to bridge create/delete APIs instead of always assuming to be qcom_scm dev for devm_qcom_tzmem_pool_new scenarios. For qcom_tzmem_pool_new, qcom_scm/qcom_tzmem_dev can be used. Bartosz/Amirreza, please share your thoughts as well.
On 5/5/2025 8:58 PM, Kuldeep Singh wrote: > > > On 4/29/2025 11:36 AM, Amirreza Zarrabi wrote: >> shm_bridge create/delete functions always use the scm device. >> There is no need to pass it as an argument. >> >> Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com> > > > There are 2 type of APIs exposed by tzmem driver for pool creation. > devm_qcom_tzmem_pool_new and qcom_tzmem_pool_new. > > Device managed pool is created with devm_qcom_tzmem_pool_new but > currently qcom_scm is using it's own dev to create/delete bridge which > is problamatic here. > > https://elixir.bootlin.com/linux/v6.14.5/source/drivers/firmware/qcom/qcom_scm.c#L1653 > > If pool is device managed, same dev should be used in qcom_scm to > create/delete bridge rather than using qcom_scm dev. > The dev passed as an argument to function should be used instead of > __scm->dev. > https://elixir.bootlin.com/linux/v6.14.5/source/drivers/firmware/qcom/qcom_scm.c#L1634 > > To summarize, I believe correct solution should be to pass corresponding > dev to bridge create/delete APIs instead of always assuming to be > qcom_scm dev for devm_qcom_tzmem_pool_new scenarios. > For qcom_tzmem_pool_new, qcom_scm/qcom_tzmem_dev can be used. > > Bartosz/Amirreza, please share your thoughts as well. > It is not true. Why should shmbridge need to have access to random devices, while the resources are obtained from the scm device, if any? - Amir
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index bff1b0d3306e..9fd5f900d327 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -1631,7 +1631,7 @@ int qcom_scm_shm_bridge_enable(void) } EXPORT_SYMBOL_GPL(qcom_scm_shm_bridge_enable); -int qcom_scm_shm_bridge_create(struct device *dev, u64 pfn_and_ns_perm_flags, +int qcom_scm_shm_bridge_create(u64 pfn_and_ns_perm_flags, u64 ipfn_and_s_perm_flags, u64 size_and_flags, u64 ns_vmids, u64 *handle) { @@ -1659,7 +1659,7 @@ int qcom_scm_shm_bridge_create(struct device *dev, u64 pfn_and_ns_perm_flags, } EXPORT_SYMBOL_GPL(qcom_scm_shm_bridge_create); -int qcom_scm_shm_bridge_delete(struct device *dev, u64 handle) +int qcom_scm_shm_bridge_delete(u64 handle) { struct qcom_scm_desc desc = { .svc = QCOM_SCM_SVC_MP, diff --git a/drivers/firmware/qcom/qcom_tzmem.c b/drivers/firmware/qcom/qcom_tzmem.c index 92b365178235..548dbd346b1b 100644 --- a/drivers/firmware/qcom/qcom_tzmem.c +++ b/drivers/firmware/qcom/qcom_tzmem.c @@ -123,9 +123,9 @@ static int qcom_tzmem_init_area(struct qcom_tzmem_area *area) if (!handle) return -ENOMEM; - ret = qcom_scm_shm_bridge_create(qcom_tzmem_dev, pfn_and_ns_perm, - ipfn_and_s_perm, size_and_flags, - QCOM_SCM_VMID_HLOS, handle); + ret = qcom_scm_shm_bridge_create(pfn_and_ns_perm, ipfn_and_s_perm, + size_and_flags, QCOM_SCM_VMID_HLOS, + handle); if (ret) return ret; @@ -141,7 +141,7 @@ static void qcom_tzmem_cleanup_area(struct qcom_tzmem_area *area) if (!qcom_tzmem_using_shm_bridge) return; - qcom_scm_shm_bridge_delete(qcom_tzmem_dev, *handle); + qcom_scm_shm_bridge_delete(*handle); kfree(handle); } diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h index bf5e64f6deba..33fde08dce70 100644 --- a/include/linux/firmware/qcom/qcom_scm.h +++ b/include/linux/firmware/qcom/qcom_scm.h @@ -149,10 +149,10 @@ bool qcom_scm_lmh_dcvsh_available(void); int qcom_scm_gpu_init_regs(u32 gpu_req); int qcom_scm_shm_bridge_enable(void); -int qcom_scm_shm_bridge_create(struct device *dev, u64 pfn_and_ns_perm_flags, +int qcom_scm_shm_bridge_create(u64 pfn_and_ns_perm_flags, u64 ipfn_and_s_perm_flags, u64 size_and_flags, u64 ns_vmids, u64 *handle); -int qcom_scm_shm_bridge_delete(struct device *dev, u64 handle); +int qcom_scm_shm_bridge_delete(u64 handle); #ifdef CONFIG_QCOM_QSEECOM
shm_bridge create/delete functions always use the scm device. There is no need to pass it as an argument. Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com> --- drivers/firmware/qcom/qcom_scm.c | 4 ++-- drivers/firmware/qcom/qcom_tzmem.c | 8 ++++---- include/linux/firmware/qcom/qcom_scm.h | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-)