Skip to content

[RFC] Provision TEE threads for system invocation - #108

Closed
etienne-lms wants to merge 2 commits into
linaro-swg:opteefrom
etienne-lms:system-thread
Closed

[RFC] Provision TEE threads for system invocation#108
etienne-lms wants to merge 2 commits into
linaro-swg:opteefrom
etienne-lms:system-thread

Conversation

@etienne-lms

@etienne-lms etienne-lms commented Jan 24, 2023

Copy link
Copy Markdown

Proposal to add an API function to Linux tee driver to invoke OP-TEE for system operation using provisioned thread contexts in OP-TEE world.
Related to OP-TEE/optee_os#5789.

Adds a invocation argument to kernel client command invocation to request
use of a provisioned system context in TEE is availalbe. This feature is
needed when OS system invocation cannot afford to wait for TEE thread to
be freed unless what the OS could deadlock, for example with system
service as SCMI that should reach TEE even many clients have consumed
the TEE threads.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

# Conflicts:
#	drivers/tee/optee/optee_smc.h
Changes SCMI optee transport to call tee_client_invoke_system_func()
instead of tee_client_invoke_func() to post SCMI messages. The former
function can leverage a provisioned system thread context in OP-TEE
preventing possible deadlock in previous implementation where OP-TEE
does not provision specific context for system invocation.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
@jenswikl

Copy link
Copy Markdown

There are quite a few of changes just to carry the system flag. How about passing the flag as:

index eff35f66399e..17618ffbe4f7 100644
--- a/drivers/tee/optee/smc_abi.c
+++ b/drivers/tee/optee/smc_abi.c
@@ -889,7 +889,11 @@ static int optee_smc_do_call_with_arg(struct tee_context *ctx,
 	}
 
 	if  (rpc_arg && tee_shm_is_dynamic(shm)) {
-		param.a0 = OPTEE_SMC_CALL_WITH_REGD_ARG;
+		if (ctx->sys_service &&
+		    (optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_SYSTEM_THREAD))
+			param.a0 = OPTEE_SMC_CALL_SYSTEM_WITH_REGD_ARG;
+		else
+			param.a0 = OPTEE_SMC_CALL_WITH_REGD_ARG;
 		reg_pair_from_64(&param.a1, &param.a2, (u_long)shm);
 		param.a3 = offs;
 	} else {
diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
index 17eb1c5205d3..1ff292ba7679 100644
--- a/include/linux/tee_drv.h
+++ b/include/linux/tee_drv.h
@@ -47,6 +47,9 @@ struct tee_shm_pool;
  *              non-blocking in nature.
  * @cap_memref_null: flag indicating if the TEE Client support shared
  *                   memory buffer with a NULL pointer.
+ * @sys_service: flag set by the TEE Client to indicate that it is part of
+ *		 a system service and that the TEE may use resources reserved
+ *		 for this.
  */
 struct tee_context {
 	struct tee_device *teedev;
@@ -55,6 +58,7 @@ struct tee_context {
 	bool releasing;
 	bool supp_nowait;
 	bool cap_memref_null;
+	bool sys_service;
 };
 
 struct tee_param_memref {

@etienne-lms

Copy link
Copy Markdown
Author

Looks much simpler. From client view, enabling the feature could means the below change, right?

	tee_ctx = tee_client_open_context(NULL, my_ctx_match, NULL, NULL);
	...
+	tee_ctx->sys_service = true;
	ret = tee_client_open_session(tee_ctx, ...);
	...
	ret = tee_client_invoke_func(tee_ctx, ...);
	...
	tee_client_close_session(tee_ctx, ...);
	tee_client_close_context(tee_ctx);

@jenswikl

Copy link
Copy Markdown

Looks much simpler. From client view, enabling the feature could means the below change, right?

Yes

@etienne-lms

Copy link
Copy Markdown
Author

Updated patch Posted to LKML: https://lore.kernel.org/lkml/20230130094157.1082712-1
I'm closing this P-R as this change is now discussed in that thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants