[RFC] Another attempt at Provision TEE threads for system invocation #110
Conversation
Adds an argument to do_call_with_arg() handler to tell whether the call is a system call or nor. This change always sets this info to false hence no functional change. This change prepares management of system invocation proposed in a later change. Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org> jw: reverted the optee_close_session_helper() prototype added use_sys_thread to struct optee_session pass use_sys_thread to optee->ops->do_call_with_arg Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Adds kernel client API function tee_client_system_session() for a client to request a system service entry in TEE context This feature is needed to prevent a system deadlock when several TEE client applications invoke TEE, consuming all TEE thread contexts available in the secure world. The deadlock can happen in the OP-TEE driver for example if all these TEE threads issue an RPC call from TEE to Linux OS to access eMMC RPMB partition data which device clock or regulator controller is accessed through an OP-TEE SCMI services. In that case, Linux SCMI driver must reach OP-TEE SCMI service without waiting one of the consumed TEE thread is freed. Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org> [jw: removed the OP-TEE driver stuff, when reposting, feel free to use: Co-developed-by: Jens Wiklander <jens.wiklander@linaro.org>] Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Adds support in the OP-TEE driver to keep track of reserved system threads. The optee_cq_*() functions are updated to handle this if enabled. The SMC ABI part of the driver enables this tracking, but the FF-A ABI part does not. Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Changes SCMI optee transport to call tee_client_system_session() to request optee driver to provision an entry context in OP-TEE for processing OP-TEE messages. This prevents possible deadlock in case OP-TEE threads are all consumed while these may be waiting for a clock or regulator to be enable which SCMI OP-TEE service which requires a free thread context to execute. Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
There was a problem hiding this comment.
Looks good to me. It is a nice way to let system sessions simply always reach TEE.
Some questions on missing mutex locking.
There is no fallback if an earlier execution stage left a suspended tee thread open. Worst case effect would be regular sessions will not reach OP-TEE, in case sys_thread_cnt == total_thread_cnt - lost_thread_cnt. Should it be addressed? A warning in optee_smc_do_call_with_arg()?
if (res.a0 == OPTEE_SMC_RETURN_ETHREAD_LIMIT) {
+ WARN_ONCE(!w->sys_thread, "TEE thread count mismatch\n")
/*
* Out of threads in secure world, wait for a thread
* become available.
*/
optee_cq_wait_for_completion(&optee->call_queue, &w);
} else if (OPTEE_SMC_RETURN_IS_RPC(res.a0)) {| * released. | ||
| */ | ||
| if (cq->free_normal_thread_count > 0) | ||
| cq->free_normal_thread_count--; |
There was a problem hiding this comment.
some thread protection around this incrementation?
| @@ -104,6 +158,9 @@ void optee_cq_wait_final(struct optee_call_queue *cq, | |||
| /* Get out of the list */ | |||
| list_del(&w->list_node); | |||
|
|
|||
| if (!w->sys_thread) | |||
| cq->free_normal_thread_count++; /* Release a normal thread */ | |||
The worst case is a total deadlock in the secure world if threads are left suspended, that's basically a DOS by starvation of CPU.
Yes, it makes sense. |
Addressing a comment. Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
|
Adding the |
|
I've posted the changes to LKML: system session patches v5. |
|
Patch v6 posted, see https://lore.kernel.org/lkml/20230505173012.881083 series. |
|
Closing. The feature is being addressed through the LKML: see latest post https://lore.kernel.org/lkml/20231030084812.905549-1-etienne.carriere@foss.st.com/. |
An alternative to what is proposed in #109
@etienne-lms feel free to use these patches if they make sense to you.