2021-03-07 06:37:51

by Rijo Thomas

[permalink] [raw]
Subject: [PATCH] tee: amdtee: unload TA only when its refcount becomes 0

Same Trusted Application (TA) can be loaded in multiple TEE contexts.

If it is a single instance TA, the TA should not get unloaded from AMD
Secure Processor, while it is still in use in another TEE context.

Therefore reference count TA and unload it when the count becomes zero.

Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver")
Reviewed-by: Devaraj Rangasamy <[email protected]>
Signed-off-by: Rijo Thomas <[email protected]>
---
drivers/tee/amdtee/amdtee_private.h | 13 +++++
drivers/tee/amdtee/call.c | 73 +++++++++++++++++++++++++++--
drivers/tee/amdtee/core.c | 15 +++---
3 files changed, 92 insertions(+), 9 deletions(-)

diff --git a/drivers/tee/amdtee/amdtee_private.h b/drivers/tee/amdtee/amdtee_private.h
index 337c8d82f74e..6d0f7062bb87 100644
--- a/drivers/tee/amdtee/amdtee_private.h
+++ b/drivers/tee/amdtee/amdtee_private.h
@@ -21,6 +21,7 @@
#define TEEC_SUCCESS 0x00000000
#define TEEC_ERROR_GENERIC 0xFFFF0000
#define TEEC_ERROR_BAD_PARAMETERS 0xFFFF0006
+#define TEEC_ERROR_OUT_OF_MEMORY 0xFFFF000C
#define TEEC_ERROR_COMMUNICATION 0xFFFF000E

#define TEEC_ORIGIN_COMMS 0x00000002
@@ -93,6 +94,18 @@ struct amdtee_shm_data {
u32 buf_id;
};

+/**
+ * struct amdtee_ta_data - Keeps track of all TAs loaded in AMD Secure
+ * Processor
+ * @ta_handle: Handle to TA loaded in TEE
+ * @refcount: Reference count for the loaded TA
+ */
+struct amdtee_ta_data {
+ struct list_head list_node;
+ u32 ta_handle;
+ u32 refcount;
+};
+
#define LOWER_TWO_BYTE_MASK 0x0000FFFF

/**
diff --git a/drivers/tee/amdtee/call.c b/drivers/tee/amdtee/call.c
index 096dd4d92d39..e10601417ea3 100644
--- a/drivers/tee/amdtee/call.c
+++ b/drivers/tee/amdtee/call.c
@@ -121,15 +121,69 @@ static int amd_params_to_tee_params(struct tee_param *tee, u32 count,
return ret;
}

+static DEFINE_MUTEX(ta_refcount_mutex);
+static struct list_head ta_list = LIST_HEAD_INIT(ta_list);
+
+static u32 get_ta_refcount(u32 ta_handle)
+{
+ struct amdtee_ta_data *ta_data;
+ u32 count = 0;
+
+ /* Caller must hold a mutex */
+ list_for_each_entry(ta_data, &ta_list, list_node)
+ if (ta_data->ta_handle == ta_handle)
+ return ++ta_data->refcount;
+
+ ta_data = kzalloc(sizeof(*ta_data), GFP_KERNEL);
+ if (ta_data) {
+ ta_data->ta_handle = ta_handle;
+ ta_data->refcount = 1;
+ count = ta_data->refcount;
+ list_add(&ta_data->list_node, &ta_list);
+ }
+
+ return count;
+}
+
+static u32 put_ta_refcount(u32 ta_handle)
+{
+ struct amdtee_ta_data *ta_data;
+ u32 count = 0;
+
+ /* Caller must hold a mutex */
+ list_for_each_entry(ta_data, &ta_list, list_node)
+ if (ta_data->ta_handle == ta_handle) {
+ count = --ta_data->refcount;
+ if (count == 0) {
+ list_del(&ta_data->list_node);
+ kfree(ta_data);
+ break;
+ }
+ }
+
+ return count;
+}
+
int handle_unload_ta(u32 ta_handle)
{
struct tee_cmd_unload_ta cmd = {0};
- u32 status;
+ u32 status, count;
int ret;

if (!ta_handle)
return -EINVAL;

+ mutex_lock(&ta_refcount_mutex);
+
+ count = put_ta_refcount(ta_handle);
+
+ if (count) {
+ pr_debug("unload ta: not unloading %u count %u\n",
+ ta_handle, count);
+ ret = -EBUSY;
+ goto unlock;
+ }
+
cmd.ta_handle = ta_handle;

ret = psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA, (void *)&cmd,
@@ -137,8 +191,12 @@ int handle_unload_ta(u32 ta_handle)
if (!ret && status != 0) {
pr_err("unload ta: status = 0x%x\n", status);
ret = -EBUSY;
+ } else {
+ pr_debug("unloaded ta handle %u\n", ta_handle);
}

+unlock:
+ mutex_unlock(&ta_refcount_mutex);
return ret;
}

@@ -357,14 +415,23 @@ int handle_load_ta(void *data, u32 size, struct tee_ioctl_open_session_arg *arg)
cmd.low_addr = lower_32_bits(blob);
cmd.size = size;

+ mutex_lock(&ta_refcount_mutex);
+
ret = psp_tee_process_cmd(TEE_CMD_ID_LOAD_TA, (void *)&cmd,
sizeof(cmd), &arg->ret);
if (ret) {
arg->ret_origin = TEEC_ORIGIN_COMMS;
arg->ret = TEEC_ERROR_COMMUNICATION;
- } else {
- set_session_id(cmd.ta_handle, 0, &arg->session);
+ } else if (arg->ret == TEEC_SUCCESS) {
+ ret = get_ta_refcount(cmd.ta_handle);
+ if (!ret) {
+ arg->ret_origin = TEEC_ORIGIN_COMMS;
+ arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
+ } else {
+ set_session_id(cmd.ta_handle, 0, &arg->session);
+ }
}
+ mutex_unlock(&ta_refcount_mutex);

pr_debug("load TA: TA handle = 0x%x, RO = 0x%x, ret = 0x%x\n",
cmd.ta_handle, arg->ret_origin, arg->ret);
diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c
index 8a6a8f30bb42..da6b88e80dc0 100644
--- a/drivers/tee/amdtee/core.c
+++ b/drivers/tee/amdtee/core.c
@@ -59,10 +59,9 @@ static void release_session(struct amdtee_session *sess)
continue;

handle_close_session(sess->ta_handle, sess->session_info[i]);
+ handle_unload_ta(sess->ta_handle);
}

- /* Unload Trusted Application once all sessions are closed */
- handle_unload_ta(sess->ta_handle);
kfree(sess);
}

@@ -224,8 +223,6 @@ static void destroy_session(struct kref *ref)
struct amdtee_session *sess = container_of(ref, struct amdtee_session,
refcount);

- /* Unload the TA from TEE */
- handle_unload_ta(sess->ta_handle);
mutex_lock(&session_list_mutex);
list_del(&sess->list_node);
mutex_unlock(&session_list_mutex);
@@ -238,7 +235,7 @@ int amdtee_open_session(struct tee_context *ctx,
{
struct amdtee_context_data *ctxdata = ctx->data;
struct amdtee_session *sess = NULL;
- u32 session_info;
+ u32 session_info, ta_handle;
size_t ta_size;
int rc, i;
void *ta;
@@ -259,11 +256,14 @@ int amdtee_open_session(struct tee_context *ctx,
if (arg->ret != TEEC_SUCCESS)
goto out;

+ ta_handle = get_ta_handle(arg->session);
+
mutex_lock(&session_list_mutex);
sess = alloc_session(ctxdata, arg->session);
mutex_unlock(&session_list_mutex);

if (!sess) {
+ handle_unload_ta(ta_handle);
rc = -ENOMEM;
goto out;
}
@@ -277,6 +277,7 @@ int amdtee_open_session(struct tee_context *ctx,

if (i >= TEE_NUM_SESSIONS) {
pr_err("reached maximum session count %d\n", TEE_NUM_SESSIONS);
+ handle_unload_ta(ta_handle);
kref_put(&sess->refcount, destroy_session);
rc = -ENOMEM;
goto out;
@@ -289,12 +290,13 @@ int amdtee_open_session(struct tee_context *ctx,
spin_lock(&sess->lock);
clear_bit(i, sess->sess_mask);
spin_unlock(&sess->lock);
+ handle_unload_ta(ta_handle);
kref_put(&sess->refcount, destroy_session);
goto out;
}

sess->session_info[i] = session_info;
- set_session_id(sess->ta_handle, i, &arg->session);
+ set_session_id(ta_handle, i, &arg->session);
out:
free_pages((u64)ta, get_order(ta_size));
return rc;
@@ -329,6 +331,7 @@ int amdtee_close_session(struct tee_context *ctx, u32 session)

/* Close the session */
handle_close_session(ta_handle, session_info);
+ handle_unload_ta(ta_handle);

kref_put(&sess->refcount, destroy_session);

--
2.17.1


2021-03-15 09:31:15

by Jens Wiklander

[permalink] [raw]
Subject: Re: [PATCH] tee: amdtee: unload TA only when its refcount becomes 0

On Sun, Mar 07, 2021 at 12:05:01PM +0530, Rijo Thomas wrote:
> Same Trusted Application (TA) can be loaded in multiple TEE contexts.
>
> If it is a single instance TA, the TA should not get unloaded from AMD
> Secure Processor, while it is still in use in another TEE context.
>
> Therefore reference count TA and unload it when the count becomes zero.
>
> Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver")
> Reviewed-by: Devaraj Rangasamy <[email protected]>
> Signed-off-by: Rijo Thomas <[email protected]>
> ---
> drivers/tee/amdtee/amdtee_private.h | 13 +++++
> drivers/tee/amdtee/call.c | 73 +++++++++++++++++++++++++++--
> drivers/tee/amdtee/core.c | 15 +++---
> 3 files changed, 92 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/tee/amdtee/amdtee_private.h b/drivers/tee/amdtee/amdtee_private.h
> index 337c8d82f74e..6d0f7062bb87 100644
> --- a/drivers/tee/amdtee/amdtee_private.h
> +++ b/drivers/tee/amdtee/amdtee_private.h
> @@ -21,6 +21,7 @@
> #define TEEC_SUCCESS 0x00000000
> #define TEEC_ERROR_GENERIC 0xFFFF0000
> #define TEEC_ERROR_BAD_PARAMETERS 0xFFFF0006
> +#define TEEC_ERROR_OUT_OF_MEMORY 0xFFFF000C
> #define TEEC_ERROR_COMMUNICATION 0xFFFF000E
>
> #define TEEC_ORIGIN_COMMS 0x00000002
> @@ -93,6 +94,18 @@ struct amdtee_shm_data {
> u32 buf_id;
> };
>
> +/**
> + * struct amdtee_ta_data - Keeps track of all TAs loaded in AMD Secure
> + * Processor
> + * @ta_handle: Handle to TA loaded in TEE
> + * @refcount: Reference count for the loaded TA
> + */
> +struct amdtee_ta_data {
> + struct list_head list_node;
> + u32 ta_handle;
> + u32 refcount;
> +};
> +
> #define LOWER_TWO_BYTE_MASK 0x0000FFFF
>
> /**
> diff --git a/drivers/tee/amdtee/call.c b/drivers/tee/amdtee/call.c
> index 096dd4d92d39..e10601417ea3 100644
> --- a/drivers/tee/amdtee/call.c
> +++ b/drivers/tee/amdtee/call.c
> @@ -121,15 +121,69 @@ static int amd_params_to_tee_params(struct tee_param *tee, u32 count,
> return ret;
> }
>
> +static DEFINE_MUTEX(ta_refcount_mutex);
> +static struct list_head ta_list = LIST_HEAD_INIT(ta_list);
> +
> +static u32 get_ta_refcount(u32 ta_handle)
> +{
> + struct amdtee_ta_data *ta_data;
> + u32 count = 0;
> +
> + /* Caller must hold a mutex */
> + list_for_each_entry(ta_data, &ta_list, list_node)
> + if (ta_data->ta_handle == ta_handle)
> + return ++ta_data->refcount;
> +
> + ta_data = kzalloc(sizeof(*ta_data), GFP_KERNEL);
> + if (ta_data) {
> + ta_data->ta_handle = ta_handle;
> + ta_data->refcount = 1;
> + count = ta_data->refcount;
> + list_add(&ta_data->list_node, &ta_list);
> + }
> +
> + return count;
> +}
> +
> +static u32 put_ta_refcount(u32 ta_handle)
> +{
> + struct amdtee_ta_data *ta_data;
> + u32 count = 0;
> +
> + /* Caller must hold a mutex */
> + list_for_each_entry(ta_data, &ta_list, list_node)
> + if (ta_data->ta_handle == ta_handle) {
> + count = --ta_data->refcount;
> + if (count == 0) {
> + list_del(&ta_data->list_node);
> + kfree(ta_data);
> + break;
> + }
> + }
> +
> + return count;
> +}
> +
> int handle_unload_ta(u32 ta_handle)
> {
> struct tee_cmd_unload_ta cmd = {0};
> - u32 status;
> + u32 status, count;
> int ret;
>
> if (!ta_handle)
> return -EINVAL;
>
> + mutex_lock(&ta_refcount_mutex);
> +
> + count = put_ta_refcount(ta_handle);
> +
> + if (count) {
> + pr_debug("unload ta: not unloading %u count %u\n",
> + ta_handle, count);
> + ret = -EBUSY;
> + goto unlock;
> + }
> +
> cmd.ta_handle = ta_handle;
>
> ret = psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA, (void *)&cmd,
> @@ -137,8 +191,12 @@ int handle_unload_ta(u32 ta_handle)
> if (!ret && status != 0) {
> pr_err("unload ta: status = 0x%x\n", status);
> ret = -EBUSY;
> + } else {
> + pr_debug("unloaded ta handle %u\n", ta_handle);
> }
>
> +unlock:
> + mutex_unlock(&ta_refcount_mutex);
> return ret;
> }
>
> @@ -357,14 +415,23 @@ int handle_load_ta(void *data, u32 size, struct tee_ioctl_open_session_arg *arg)
> cmd.low_addr = lower_32_bits(blob);
> cmd.size = size;
>
> + mutex_lock(&ta_refcount_mutex);
> +
> ret = psp_tee_process_cmd(TEE_CMD_ID_LOAD_TA, (void *)&cmd,
> sizeof(cmd), &arg->ret);
> if (ret) {
> arg->ret_origin = TEEC_ORIGIN_COMMS;
> arg->ret = TEEC_ERROR_COMMUNICATION;
> - } else {
> - set_session_id(cmd.ta_handle, 0, &arg->session);
> + } else if (arg->ret == TEEC_SUCCESS) {
> + ret = get_ta_refcount(cmd.ta_handle);
> + if (!ret) {

If this occurs, how is the TA unloaded from secure world?

Thanks,
Jens

> + arg->ret_origin = TEEC_ORIGIN_COMMS;
> + arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
> + } else {
> + set_session_id(cmd.ta_handle, 0, &arg->session);
> + }
> }
> + mutex_unlock(&ta_refcount_mutex);
>
> pr_debug("load TA: TA handle = 0x%x, RO = 0x%x, ret = 0x%x\n",
> cmd.ta_handle, arg->ret_origin, arg->ret);
> diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c
> index 8a6a8f30bb42..da6b88e80dc0 100644
> --- a/drivers/tee/amdtee/core.c
> +++ b/drivers/tee/amdtee/core.c
> @@ -59,10 +59,9 @@ static void release_session(struct amdtee_session *sess)
> continue;
>
> handle_close_session(sess->ta_handle, sess->session_info[i]);
> + handle_unload_ta(sess->ta_handle);
> }
>
> - /* Unload Trusted Application once all sessions are closed */
> - handle_unload_ta(sess->ta_handle);
> kfree(sess);
> }
>
> @@ -224,8 +223,6 @@ static void destroy_session(struct kref *ref)
> struct amdtee_session *sess = container_of(ref, struct amdtee_session,
> refcount);
>
> - /* Unload the TA from TEE */
> - handle_unload_ta(sess->ta_handle);
> mutex_lock(&session_list_mutex);
> list_del(&sess->list_node);
> mutex_unlock(&session_list_mutex);
> @@ -238,7 +235,7 @@ int amdtee_open_session(struct tee_context *ctx,
> {
> struct amdtee_context_data *ctxdata = ctx->data;
> struct amdtee_session *sess = NULL;
> - u32 session_info;
> + u32 session_info, ta_handle;
> size_t ta_size;
> int rc, i;
> void *ta;
> @@ -259,11 +256,14 @@ int amdtee_open_session(struct tee_context *ctx,
> if (arg->ret != TEEC_SUCCESS)
> goto out;
>
> + ta_handle = get_ta_handle(arg->session);
> +
> mutex_lock(&session_list_mutex);
> sess = alloc_session(ctxdata, arg->session);
> mutex_unlock(&session_list_mutex);
>
> if (!sess) {
> + handle_unload_ta(ta_handle);
> rc = -ENOMEM;
> goto out;
> }
> @@ -277,6 +277,7 @@ int amdtee_open_session(struct tee_context *ctx,
>
> if (i >= TEE_NUM_SESSIONS) {
> pr_err("reached maximum session count %d\n", TEE_NUM_SESSIONS);
> + handle_unload_ta(ta_handle);
> kref_put(&sess->refcount, destroy_session);
> rc = -ENOMEM;
> goto out;
> @@ -289,12 +290,13 @@ int amdtee_open_session(struct tee_context *ctx,
> spin_lock(&sess->lock);
> clear_bit(i, sess->sess_mask);
> spin_unlock(&sess->lock);
> + handle_unload_ta(ta_handle);
> kref_put(&sess->refcount, destroy_session);
> goto out;
> }
>
> sess->session_info[i] = session_info;
> - set_session_id(sess->ta_handle, i, &arg->session);
> + set_session_id(ta_handle, i, &arg->session);
> out:
> free_pages((u64)ta, get_order(ta_size));
> return rc;
> @@ -329,6 +331,7 @@ int amdtee_close_session(struct tee_context *ctx, u32 session)
>
> /* Close the session */
> handle_close_session(ta_handle, session_info);
> + handle_unload_ta(ta_handle);
>
> kref_put(&sess->refcount, destroy_session);
>
> --
> 2.17.1
>

2021-04-05 08:35:13

by Rijo Thomas

[permalink] [raw]
Subject: Re: [PATCH] tee: amdtee: unload TA only when its refcount becomes 0



On 15/03/21 2:59 pm, Jens Wiklander wrote:
> On Sun, Mar 07, 2021 at 12:05:01PM +0530, Rijo Thomas wrote:
>> Same Trusted Application (TA) can be loaded in multiple TEE contexts.
>>
>> If it is a single instance TA, the TA should not get unloaded from AMD
>> Secure Processor, while it is still in use in another TEE context.
>>
>> Therefore reference count TA and unload it when the count becomes zero.
>>
>> Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver")
>> Reviewed-by: Devaraj Rangasamy <[email protected]>
>> Signed-off-by: Rijo Thomas <[email protected]>
>> ---
>> drivers/tee/amdtee/amdtee_private.h | 13 +++++
>> drivers/tee/amdtee/call.c | 73 +++++++++++++++++++++++++++--
>> drivers/tee/amdtee/core.c | 15 +++---
>> 3 files changed, 92 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/tee/amdtee/amdtee_private.h b/drivers/tee/amdtee/amdtee_private.h
>> index 337c8d82f74e..6d0f7062bb87 100644
>> --- a/drivers/tee/amdtee/amdtee_private.h
>> +++ b/drivers/tee/amdtee/amdtee_private.h
>> @@ -21,6 +21,7 @@
>> #define TEEC_SUCCESS 0x00000000
>> #define TEEC_ERROR_GENERIC 0xFFFF0000
>> #define TEEC_ERROR_BAD_PARAMETERS 0xFFFF0006
>> +#define TEEC_ERROR_OUT_OF_MEMORY 0xFFFF000C
>> #define TEEC_ERROR_COMMUNICATION 0xFFFF000E
>>
>> #define TEEC_ORIGIN_COMMS 0x00000002
>> @@ -93,6 +94,18 @@ struct amdtee_shm_data {
>> u32 buf_id;
>> };
>>
>> +/**
>> + * struct amdtee_ta_data - Keeps track of all TAs loaded in AMD Secure
>> + * Processor
>> + * @ta_handle: Handle to TA loaded in TEE
>> + * @refcount: Reference count for the loaded TA
>> + */
>> +struct amdtee_ta_data {
>> + struct list_head list_node;
>> + u32 ta_handle;
>> + u32 refcount;
>> +};
>> +
>> #define LOWER_TWO_BYTE_MASK 0x0000FFFF
>>
>> /**
>> diff --git a/drivers/tee/amdtee/call.c b/drivers/tee/amdtee/call.c
>> index 096dd4d92d39..e10601417ea3 100644
>> --- a/drivers/tee/amdtee/call.c
>> +++ b/drivers/tee/amdtee/call.c
>> @@ -121,15 +121,69 @@ static int amd_params_to_tee_params(struct tee_param *tee, u32 count,
>> return ret;
>> }
>>
>> +static DEFINE_MUTEX(ta_refcount_mutex);
>> +static struct list_head ta_list = LIST_HEAD_INIT(ta_list);
>> +
>> +static u32 get_ta_refcount(u32 ta_handle)
>> +{
>> + struct amdtee_ta_data *ta_data;
>> + u32 count = 0;
>> +
>> + /* Caller must hold a mutex */
>> + list_for_each_entry(ta_data, &ta_list, list_node)
>> + if (ta_data->ta_handle == ta_handle)
>> + return ++ta_data->refcount;
>> +
>> + ta_data = kzalloc(sizeof(*ta_data), GFP_KERNEL);
>> + if (ta_data) {
>> + ta_data->ta_handle = ta_handle;
>> + ta_data->refcount = 1;
>> + count = ta_data->refcount;
>> + list_add(&ta_data->list_node, &ta_list);
>> + }
>> +
>> + return count;
>> +}
>> +
>> +static u32 put_ta_refcount(u32 ta_handle)
>> +{
>> + struct amdtee_ta_data *ta_data;
>> + u32 count = 0;
>> +
>> + /* Caller must hold a mutex */
>> + list_for_each_entry(ta_data, &ta_list, list_node)
>> + if (ta_data->ta_handle == ta_handle) {
>> + count = --ta_data->refcount;
>> + if (count == 0) {
>> + list_del(&ta_data->list_node);
>> + kfree(ta_data);
>> + break;
>> + }
>> + }
>> +
>> + return count;
>> +}
>> +
>> int handle_unload_ta(u32 ta_handle)
>> {
>> struct tee_cmd_unload_ta cmd = {0};
>> - u32 status;
>> + u32 status, count;
>> int ret;
>>
>> if (!ta_handle)
>> return -EINVAL;
>>
>> + mutex_lock(&ta_refcount_mutex);
>> +
>> + count = put_ta_refcount(ta_handle);
>> +
>> + if (count) {
>> + pr_debug("unload ta: not unloading %u count %u\n",
>> + ta_handle, count);
>> + ret = -EBUSY;
>> + goto unlock;
>> + }
>> +
>> cmd.ta_handle = ta_handle;
>>
>> ret = psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA, (void *)&cmd,
>> @@ -137,8 +191,12 @@ int handle_unload_ta(u32 ta_handle)
>> if (!ret && status != 0) {
>> pr_err("unload ta: status = 0x%x\n", status);
>> ret = -EBUSY;
>> + } else {
>> + pr_debug("unloaded ta handle %u\n", ta_handle);
>> }
>>
>> +unlock:
>> + mutex_unlock(&ta_refcount_mutex);
>> return ret;
>> }
>>
>> @@ -357,14 +415,23 @@ int handle_load_ta(void *data, u32 size, struct tee_ioctl_open_session_arg *arg)
>> cmd.low_addr = lower_32_bits(blob);
>> cmd.size = size;
>>
>> + mutex_lock(&ta_refcount_mutex);
>> +
>> ret = psp_tee_process_cmd(TEE_CMD_ID_LOAD_TA, (void *)&cmd,
>> sizeof(cmd), &arg->ret);
>> if (ret) {
>> arg->ret_origin = TEEC_ORIGIN_COMMS;
>> arg->ret = TEEC_ERROR_COMMUNICATION;
>> - } else {
>> - set_session_id(cmd.ta_handle, 0, &arg->session);
>> + } else if (arg->ret == TEEC_SUCCESS) {
>> + ret = get_ta_refcount(cmd.ta_handle);
>> + if (!ret) {
>
> If this occurs, how is the TA unloaded from secure world?
>

Ack. Shall re-post update in next version.

Thanks,
Rijo

> Thanks,
> Jens
>
>> + arg->ret_origin = TEEC_ORIGIN_COMMS;
>> + arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
>> + } else {
>> + set_session_id(cmd.ta_handle, 0, &arg->session);
>> + }
>> }
>> + mutex_unlock(&ta_refcount_mutex);
>>
>> pr_debug("load TA: TA handle = 0x%x, RO = 0x%x, ret = 0x%x\n",
>> cmd.ta_handle, arg->ret_origin, arg->ret);
>> diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c
>> index 8a6a8f30bb42..da6b88e80dc0 100644
>> --- a/drivers/tee/amdtee/core.c
>> +++ b/drivers/tee/amdtee/core.c
>> @@ -59,10 +59,9 @@ static void release_session(struct amdtee_session *sess)
>> continue;
>>
>> handle_close_session(sess->ta_handle, sess->session_info[i]);
>> + handle_unload_ta(sess->ta_handle);
>> }
>>
>> - /* Unload Trusted Application once all sessions are closed */
>> - handle_unload_ta(sess->ta_handle);
>> kfree(sess);
>> }
>>
>> @@ -224,8 +223,6 @@ static void destroy_session(struct kref *ref)
>> struct amdtee_session *sess = container_of(ref, struct amdtee_session,
>> refcount);
>>
>> - /* Unload the TA from TEE */
>> - handle_unload_ta(sess->ta_handle);
>> mutex_lock(&session_list_mutex);
>> list_del(&sess->list_node);
>> mutex_unlock(&session_list_mutex);
>> @@ -238,7 +235,7 @@ int amdtee_open_session(struct tee_context *ctx,
>> {
>> struct amdtee_context_data *ctxdata = ctx->data;
>> struct amdtee_session *sess = NULL;
>> - u32 session_info;
>> + u32 session_info, ta_handle;
>> size_t ta_size;
>> int rc, i;
>> void *ta;
>> @@ -259,11 +256,14 @@ int amdtee_open_session(struct tee_context *ctx,
>> if (arg->ret != TEEC_SUCCESS)
>> goto out;
>>
>> + ta_handle = get_ta_handle(arg->session);
>> +
>> mutex_lock(&session_list_mutex);
>> sess = alloc_session(ctxdata, arg->session);
>> mutex_unlock(&session_list_mutex);
>>
>> if (!sess) {
>> + handle_unload_ta(ta_handle);
>> rc = -ENOMEM;
>> goto out;
>> }
>> @@ -277,6 +277,7 @@ int amdtee_open_session(struct tee_context *ctx,
>>
>> if (i >= TEE_NUM_SESSIONS) {
>> pr_err("reached maximum session count %d\n", TEE_NUM_SESSIONS);
>> + handle_unload_ta(ta_handle);
>> kref_put(&sess->refcount, destroy_session);
>> rc = -ENOMEM;
>> goto out;
>> @@ -289,12 +290,13 @@ int amdtee_open_session(struct tee_context *ctx,
>> spin_lock(&sess->lock);
>> clear_bit(i, sess->sess_mask);
>> spin_unlock(&sess->lock);
>> + handle_unload_ta(ta_handle);
>> kref_put(&sess->refcount, destroy_session);
>> goto out;
>> }
>>
>> sess->session_info[i] = session_info;
>> - set_session_id(sess->ta_handle, i, &arg->session);
>> + set_session_id(ta_handle, i, &arg->session);
>> out:
>> free_pages((u64)ta, get_order(ta_size));
>> return rc;
>> @@ -329,6 +331,7 @@ int amdtee_close_session(struct tee_context *ctx, u32 session)
>>
>> /* Close the session */
>> handle_close_session(ta_handle, session_info);
>> + handle_unload_ta(ta_handle);
>>
>> kref_put(&sess->refcount, destroy_session);
>>
>> --
>> 2.17.1
>>