core: fine grained tee_ta_mutex locking

Changes TA loading and session initialization to use fine grained locking
based on the tee_ta_mutex.

This avoids a potential dead lock with PGT cache where we're waiting for
new page tables with tee_ta_mutex locked, which prevents
tee_ta_close_session() to indirectly return any page tables.

This change also removes the last really big critical section. With this
TAs can be loaded in parallel.

Reported-by: Zhizhou Zhang <zhizhouzhang@asrmicro.com>
Tested-by: Zhizhou Zhang <zhizhouzhang@asrmicro.com>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (HiKey960)
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

core/arch/arm/include/kernel/pseudo_ta.h

@@ -58,8 +58,7 @@ static inline struct pseudo_ta_ctx *to_pseudo_ta_ctx(struct tee_ta_ctx *ctx)
 	return container_of(ctx, struct pseudo_ta_ctx, ctx);
 }
 
-TEE_Result tee_ta_init_pseudo_ta_session(const TEE_UUID *uuid,
-			struct tee_ta_session *s);
+TEE_Result pseudo_ta_get_ctx(const TEE_UUID *uuid, struct tee_ta_ctx **ctx);
 
 #endif /* KERNEL_PSEUDO_TA_H */
 

core/arch/arm/include/kernel/user_ta.h

@@ -60,13 +60,11 @@ static inline struct user_ta_ctx *to_user_ta_ctx(struct tee_ta_ctx *ctx)
 struct user_ta_store_ops;
 
 #ifdef CFG_WITH_USER_TA
-TEE_Result tee_ta_init_user_ta_session(const TEE_UUID *uuid,
-			struct tee_ta_session *s);
+TEE_Result user_ta_get_ctx(const TEE_UUID *uuid, struct tee_ta_ctx **ctx);
 TEE_Result tee_ta_register_ta_store(struct user_ta_store_ops *ops);
 #else
-static inline TEE_Result tee_ta_init_user_ta_session(
-			const TEE_UUID *uuid __unused,
-			struct tee_ta_session *s __unused)
+static inline TEE_Result user_ta_get_ctx(const TEE_UUID *uuid __unused,
+					 struct tee_ta_ctx **ctx __unused)
 {
 	return TEE_ERROR_ITEM_NOT_FOUND;
 }

core/arch/arm/kernel/pseudo_ta.c

@@ -279,12 +279,8 @@ static TEE_Result verify_pseudo_tas_conformance(void)
 
 service_init(verify_pseudo_tas_conformance);
 
-/*-----------------------------------------------------------------------------
- * Initialises a session based on the UUID or ptr to the ta
- * Returns ptr to the session (ta_session) and a TEE_Result
- *---------------------------------------------------------------------------*/
-TEE_Result tee_ta_init_pseudo_ta_session(const TEE_UUID *uuid,
-			struct tee_ta_session *s)
+/* Initializes and returns a TA context based on the UUID of a pseudo TA. */
+TEE_Result pseudo_ta_get_ctx(const TEE_UUID *uuid, struct tee_ta_ctx **ctx_ret)
 {
 	struct pseudo_ta_ctx *stc = NULL;
 	struct tee_ta_ctx *ctx;
@@ -309,13 +305,13 @@ TEE_Result tee_ta_init_pseudo_ta_session(const TEE_UUID *uuid,
 	ctx = &stc->ctx;
 
 	ctx->ref_count = 1;
-	s->ctx = ctx;
 	ctx->flags = ta->flags;
 	stc->pseudo_ta = ta;
 	ctx->uuid = ta->uuid;
 	ctx->ops = &pseudo_ta_ops;
-	TAILQ_INSERT_TAIL(&tee_ctxes, ctx, link);
+	tee_ta_register_ctx(ctx);
 
+	*ctx_ret = ctx;
 	DMSG("%s : %pUl", stc->pseudo_ta->name, (void *)&ctx->uuid);
 
 	return TEE_SUCCESS;

core/arch/arm/kernel/user_ta.c

@@ -320,7 +320,7 @@ static TEE_Result ta_load(const TEE_UUID *uuid,
 	utc->entry_func = ta_head->entry.ptr64;
 	utc->ctx.ref_count = 1;
 	condvar_init(&utc->ctx.busy_cv);
-	TAILQ_INSERT_TAIL(&tee_ctxes, &utc->ctx, link);
+	tee_ta_register_ctx(&utc->ctx);
 	*ta_ctx = &utc->ctx;
 
 	tee_mmu_set_ctx(NULL);
@@ -626,20 +626,19 @@ TEE_Result tee_ta_register_ta_store(struct user_ta_store_ops *ops)
 	return TEE_SUCCESS;
 }
 
-TEE_Result tee_ta_init_user_ta_session(const TEE_UUID *uuid,
-			struct tee_ta_session *s)
+TEE_Result user_ta_get_ctx(const TEE_UUID *uuid, struct tee_ta_ctx **ctx)
 {
 	const struct user_ta_store_ops *store;
 	TEE_Result res;
 
 	SLIST_FOREACH(store, &uta_store_list, link) {
 		DMSG("Lookup user TA %pUl (%s)", (void *)uuid,
 		     store->description);
-		res = ta_load(uuid, store, &s->ctx);
+		res = ta_load(uuid, store, ctx);
 		if (res == TEE_ERROR_ITEM_NOT_FOUND)
 			continue;
 		if (res == TEE_SUCCESS)
-			s->ctx->ops = &user_ta_ops;
+			(*ctx)->ops = &user_ta_ops;
 		else
 			DMSG("res=0x%x", res);
 		return res;

core/include/kernel/tee_ta_manager.h

@@ -129,10 +129,7 @@ struct tee_ta_session {
 #endif
 };
 
-/* Registered contexts */
-extern struct tee_ta_ctx_head tee_ctxes;
-
-extern struct mutex tee_ta_mutex;
+void tee_ta_register_ctx(struct tee_ta_ctx *ctx);
 
 TEE_Result tee_ta_open_session(TEE_ErrorOrigin *err,
 			       struct tee_ta_session **sess,

core/kernel/tee_ta_manager.c

@@ -55,8 +55,8 @@
 #include <util.h>
 
 /* This mutex protects the critical section in tee_ta_init_session */
-struct mutex tee_ta_mutex = MUTEX_INITIALIZER;
-struct tee_ta_ctx_head tee_ctxes = TAILQ_HEAD_INITIALIZER(tee_ctxes);
+static struct mutex tee_ta_mutex = MUTEX_INITIALIZER;
+static struct tee_ta_ctx_head tee_ctxes = TAILQ_HEAD_INITIALIZER(tee_ctxes);
 
 #ifndef CFG_CONCURRENT_SINGLE_INSTANCE_TA
 static struct condvar tee_ta_cv = CONDVAR_INITIALIZER;
@@ -460,9 +460,11 @@ TEE_Result tee_ta_close_session(struct tee_ta_session *csess,
 	return TEE_SUCCESS;
 }
 
-static TEE_Result tee_ta_init_session_with_context(struct tee_ta_ctx *ctx,
-			struct tee_ta_session *s)
+static TEE_Result get_ctx(struct tee_ta_ctx *ctx)
 {
+	if (!ctx)
+		return TEE_ERROR_ITEM_NOT_FOUND;
+
 	/*
 	 * If TA isn't single instance it should be loaded as new
 	 * instance instead of doing anything with this instance.
@@ -482,10 +484,15 @@ static TEE_Result tee_ta_init_session_with_context(struct tee_ta_ctx *ctx,
 	DMSG("Re-open TA %pUl", (void *)&ctx->uuid);
 
 	ctx->ref_count++;
-	s->ctx = ctx;
 	return TEE_SUCCESS;
 }
 
+void tee_ta_register_ctx(struct tee_ta_ctx *ctx)
+{
+	mutex_lock(&tee_ta_mutex);
+	TAILQ_INSERT_TAIL(&tee_ctxes, ctx, link);
+	mutex_unlock(&tee_ta_mutex);
+}
 
 static TEE_Result tee_ta_init_session(TEE_ErrorOrigin *err,
 				struct tee_ta_session_head *open_sessions,
@@ -500,48 +507,43 @@ static TEE_Result tee_ta_init_session(TEE_ErrorOrigin *err,
 	if (!s)
 		return TEE_ERROR_OUT_OF_MEMORY;
 
-	s->cancel_mask = true;
-	condvar_init(&s->refc_cv);
-	condvar_init(&s->lock_cv);
-	s->lock_thread = THREAD_ID_INVALID;
-	s->ref_count = 1;
-
-
-	/*
-	 * We take the global TA mutex here and hold it while doing
-	 * RPC to load the TA. This big critical section should be broken
-	 * down into smaller pieces.
-	 */
-
-
-	mutex_lock(&tee_ta_mutex);
-	TAILQ_INSERT_TAIL(open_sessions, s, link);
 
 	/* Look for already loaded TA */
+	mutex_lock(&tee_ta_mutex);
 	ctx = tee_ta_context_find(uuid);
-	if (ctx) {
-		res = tee_ta_init_session_with_context(ctx, s);
-		if (res == TEE_SUCCESS || res != TEE_ERROR_ITEM_NOT_FOUND)
-			goto out;
-	}
+	res = get_ctx(ctx);
+	mutex_unlock(&tee_ta_mutex);
+	if (res != TEE_ERROR_ITEM_NOT_FOUND)
+		goto out;
 
 	/* Look for static TA */
-	res = tee_ta_init_pseudo_ta_session(uuid, s);
-	if (res == TEE_SUCCESS || res != TEE_ERROR_ITEM_NOT_FOUND)
+	res = pseudo_ta_get_ctx(uuid, &ctx);
+	if (res != TEE_ERROR_ITEM_NOT_FOUND)
 		goto out;
 
 	/* Look for user TA */
-	res = tee_ta_init_user_ta_session(uuid, s);
-
+	res = user_ta_get_ctx(uuid, &ctx);
 out:
-	if (res == TEE_SUCCESS) {
-		*sess = s;
-	} else {
-		TAILQ_REMOVE(open_sessions, s, link);
+	if (res) {
 		free(s);
+		return res;
 	}
+
+	/*
+	 * We have a context, let's finish initialization of the session
+	 */
+	s->cancel_mask = true;
+	condvar_init(&s->refc_cv);
+	condvar_init(&s->lock_cv);
+	s->lock_thread = THREAD_ID_INVALID;
+	s->ref_count = 1;
+	s->ctx = ctx;
+	mutex_lock(&tee_ta_mutex);
+	TAILQ_INSERT_TAIL(open_sessions, s, link);
 	mutex_unlock(&tee_ta_mutex);
-	return res;
+	*sess = s;
+
+	return TEE_SUCCESS;
 }
 
 TEE_Result tee_ta_open_session(TEE_ErrorOrigin *err,