After TEE has completed processing of TEE_CMD_ID_LOAD_TA, set proper value in 'return_origin' argument passed by open_session() call. To do so, add 'return_origin' field to the structure tee_cmd_load_ta. The Trusted OS shall update return_origin as part of TEE processing.
This change to 'struct tee_cmd_load_ta' interface requires a similar update in AMD-TEE Trusted OS's TEE_CMD_ID_LOAD_TA interface.
This patch has been verified on Phoenix Birman setup. On older APUs, return_origin value will be 0.
Cc: stable@vger.kernel.org Tested-by: Sourabh Das sourabh.das@amd.com Signed-off-by: Rijo Thomas Rijo-john.Thomas@amd.com --- drivers/tee/amdtee/amdtee_if.h | 10 ++++++---- drivers/tee/amdtee/call.c | 30 +++++++++++++++++------------- 2 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/drivers/tee/amdtee/amdtee_if.h b/drivers/tee/amdtee/amdtee_if.h index ff48c3e47375..e2014e21530a 100644 --- a/drivers/tee/amdtee/amdtee_if.h +++ b/drivers/tee/amdtee/amdtee_if.h @@ -118,16 +118,18 @@ struct tee_cmd_unmap_shared_mem {
/** * struct tee_cmd_load_ta - load Trusted Application (TA) binary into TEE - * @low_addr: [in] bits [31:0] of the physical address of the TA binary - * @hi_addr: [in] bits [63:32] of the physical address of the TA binary - * @size: [in] size of TA binary in bytes - * @ta_handle: [out] return handle of the loaded TA + * @low_addr: [in] bits [31:0] of the physical address of the TA binary + * @hi_addr: [in] bits [63:32] of the physical address of the TA binary + * @size: [in] size of TA binary in bytes + * @ta_handle: [out] return handle of the loaded TA + * @return_origin: [out] origin of return code after TEE processing */ struct tee_cmd_load_ta { u32 low_addr; u32 hi_addr; u32 size; u32 ta_handle; + u32 return_origin; };
/** diff --git a/drivers/tee/amdtee/call.c b/drivers/tee/amdtee/call.c index e8cd9aaa3467..e9b63dcb3194 100644 --- a/drivers/tee/amdtee/call.c +++ b/drivers/tee/amdtee/call.c @@ -423,19 +423,23 @@ int handle_load_ta(void *data, u32 size, struct tee_ioctl_open_session_arg *arg) if (ret) { arg->ret_origin = TEEC_ORIGIN_COMMS; arg->ret = TEEC_ERROR_COMMUNICATION; - } else if (arg->ret == TEEC_SUCCESS) { - ret = get_ta_refcount(load_cmd.ta_handle); - if (!ret) { - arg->ret_origin = TEEC_ORIGIN_COMMS; - arg->ret = TEEC_ERROR_OUT_OF_MEMORY; - - /* Unload the TA on error */ - unload_cmd.ta_handle = load_cmd.ta_handle; - psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA, - (void *)&unload_cmd, - sizeof(unload_cmd), &ret); - } else { - set_session_id(load_cmd.ta_handle, 0, &arg->session); + } else { + arg->ret_origin = load_cmd.return_origin; + + if (arg->ret == TEEC_SUCCESS) { + ret = get_ta_refcount(load_cmd.ta_handle); + if (!ret) { + arg->ret_origin = TEEC_ORIGIN_COMMS; + arg->ret = TEEC_ERROR_OUT_OF_MEMORY; + + /* Unload the TA on error */ + unload_cmd.ta_handle = load_cmd.ta_handle; + psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA, + (void *)&unload_cmd, + sizeof(unload_cmd), &ret); + } else { + set_session_id(load_cmd.ta_handle, 0, &arg->session); + } } } mutex_unlock(&ta_refcount_mutex);
Hi,
On Tue, May 2, 2023 at 8:25 AM Rijo Thomas Rijo-john.Thomas@amd.com wrote:
After TEE has completed processing of TEE_CMD_ID_LOAD_TA, set proper value in 'return_origin' argument passed by open_session() call. To do so, add 'return_origin' field to the structure tee_cmd_load_ta. The Trusted OS shall update return_origin as part of TEE processing.
This change to 'struct tee_cmd_load_ta' interface requires a similar update in AMD-TEE Trusted OS's TEE_CMD_ID_LOAD_TA interface.
This is an ABI change, but it's not clear if it's an incompatible ABI change or not. What happens if the AMD-TEE Trusted OS isn't updated?
This patch has been verified on Phoenix Birman setup. On older APUs, return_origin value will be 0.
Why, because MD-TEE Trusted OS will not be updated on the older APUs?
Cc: stable@vger.kernel.org
Which stable kernels are you targeting? A Fixes tag might answer that.
Thanks, Jens
Tested-by: Sourabh Das sourabh.das@amd.com Signed-off-by: Rijo Thomas Rijo-john.Thomas@amd.com
drivers/tee/amdtee/amdtee_if.h | 10 ++++++---- drivers/tee/amdtee/call.c | 30 +++++++++++++++++------------- 2 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/drivers/tee/amdtee/amdtee_if.h b/drivers/tee/amdtee/amdtee_if.h index ff48c3e47375..e2014e21530a 100644 --- a/drivers/tee/amdtee/amdtee_if.h +++ b/drivers/tee/amdtee/amdtee_if.h @@ -118,16 +118,18 @@ struct tee_cmd_unmap_shared_mem {
/**
- struct tee_cmd_load_ta - load Trusted Application (TA) binary into TEE
- @low_addr: [in] bits [31:0] of the physical address of the TA binary
- @hi_addr: [in] bits [63:32] of the physical address of the TA binary
- @size: [in] size of TA binary in bytes
- @ta_handle: [out] return handle of the loaded TA
- @low_addr: [in] bits [31:0] of the physical address of the TA binary
- @hi_addr: [in] bits [63:32] of the physical address of the TA binary
- @size: [in] size of TA binary in bytes
- @ta_handle: [out] return handle of the loaded TA
*/
- @return_origin: [out] origin of return code after TEE processing
struct tee_cmd_load_ta { u32 low_addr; u32 hi_addr; u32 size; u32 ta_handle;
u32 return_origin;};
/** diff --git a/drivers/tee/amdtee/call.c b/drivers/tee/amdtee/call.c index e8cd9aaa3467..e9b63dcb3194 100644 --- a/drivers/tee/amdtee/call.c +++ b/drivers/tee/amdtee/call.c @@ -423,19 +423,23 @@ int handle_load_ta(void *data, u32 size, struct tee_ioctl_open_session_arg *arg) if (ret) { arg->ret_origin = TEEC_ORIGIN_COMMS; arg->ret = TEEC_ERROR_COMMUNICATION;
} else if (arg->ret == TEEC_SUCCESS) {ret = get_ta_refcount(load_cmd.ta_handle);if (!ret) {arg->ret_origin = TEEC_ORIGIN_COMMS;arg->ret = TEEC_ERROR_OUT_OF_MEMORY;/* Unload the TA on error */unload_cmd.ta_handle = load_cmd.ta_handle;psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA,(void *)&unload_cmd,sizeof(unload_cmd), &ret);} else {set_session_id(load_cmd.ta_handle, 0, &arg->session);
} else {arg->ret_origin = load_cmd.return_origin;if (arg->ret == TEEC_SUCCESS) {ret = get_ta_refcount(load_cmd.ta_handle);if (!ret) {arg->ret_origin = TEEC_ORIGIN_COMMS;arg->ret = TEEC_ERROR_OUT_OF_MEMORY;/* Unload the TA on error */unload_cmd.ta_handle = load_cmd.ta_handle;psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA,(void *)&unload_cmd,sizeof(unload_cmd), &ret);} else {set_session_id(load_cmd.ta_handle, 0, &arg->session);} } } mutex_unlock(&ta_refcount_mutex);-- 2.25.1
On 5/5/2023 1:00 PM, Jens Wiklander wrote:
Hi,
On Tue, May 2, 2023 at 8:25 AM Rijo Thomas Rijo-john.Thomas@amd.com wrote:
After TEE has completed processing of TEE_CMD_ID_LOAD_TA, set proper value in 'return_origin' argument passed by open_session() call. To do so, add 'return_origin' field to the structure tee_cmd_load_ta. The Trusted OS shall update return_origin as part of TEE processing.
This change to 'struct tee_cmd_load_ta' interface requires a similar update in AMD-TEE Trusted OS's TEE_CMD_ID_LOAD_TA interface.
This is an ABI change, but it's not clear if it's an incompatible ABI change or not. What happens if the AMD-TEE Trusted OS isn't updated?
If AMD-TEE Trusted OS isn't updated, load_cmd.return_origin value will be 0.
load_cmd.return_origin will have non-zero value only if AMD-TEE Trusted OS on the platform has the necessary ABI change.
At present, without this patch, arg->ret_origin is 0 and even with this patch it will be 0 unless AMD-TEE Trusted OS on the platform has the ABI update. So, this is not an incompatible ABI change.
This patch has been verified on Phoenix Birman setup. On older APUs, return_origin value will be 0.
Why, because MD-TEE Trusted OS will not be updated on the older APUs?
Yes, that's correct - older APUs will not have updated AMD-TEE Trusted OS.
Cc: stable@vger.kernel.org
Which stable kernels are you targeting? A Fixes tag might answer that.
Okay, I will add a Fixes tag and post v2 patch.
Thanks, Rijo
Thanks, Jens
Tested-by: Sourabh Das sourabh.das@amd.com Signed-off-by: Rijo Thomas Rijo-john.Thomas@amd.com
drivers/tee/amdtee/amdtee_if.h | 10 ++++++---- drivers/tee/amdtee/call.c | 30 +++++++++++++++++------------- 2 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/drivers/tee/amdtee/amdtee_if.h b/drivers/tee/amdtee/amdtee_if.h index ff48c3e47375..e2014e21530a 100644 --- a/drivers/tee/amdtee/amdtee_if.h +++ b/drivers/tee/amdtee/amdtee_if.h @@ -118,16 +118,18 @@ struct tee_cmd_unmap_shared_mem {
/**
- struct tee_cmd_load_ta - load Trusted Application (TA) binary into TEE
- @low_addr: [in] bits [31:0] of the physical address of the TA binary
- @hi_addr: [in] bits [63:32] of the physical address of the TA binary
- @size: [in] size of TA binary in bytes
- @ta_handle: [out] return handle of the loaded TA
- @low_addr: [in] bits [31:0] of the physical address of the TA binary
- @hi_addr: [in] bits [63:32] of the physical address of the TA binary
- @size: [in] size of TA binary in bytes
- @ta_handle: [out] return handle of the loaded TA
*/
- @return_origin: [out] origin of return code after TEE processing
struct tee_cmd_load_ta { u32 low_addr; u32 hi_addr; u32 size; u32 ta_handle;
u32 return_origin;};
/** diff --git a/drivers/tee/amdtee/call.c b/drivers/tee/amdtee/call.c index e8cd9aaa3467..e9b63dcb3194 100644 --- a/drivers/tee/amdtee/call.c +++ b/drivers/tee/amdtee/call.c @@ -423,19 +423,23 @@ int handle_load_ta(void *data, u32 size, struct tee_ioctl_open_session_arg *arg) if (ret) { arg->ret_origin = TEEC_ORIGIN_COMMS; arg->ret = TEEC_ERROR_COMMUNICATION;
} else if (arg->ret == TEEC_SUCCESS) {ret = get_ta_refcount(load_cmd.ta_handle);if (!ret) {arg->ret_origin = TEEC_ORIGIN_COMMS;arg->ret = TEEC_ERROR_OUT_OF_MEMORY;/* Unload the TA on error */unload_cmd.ta_handle = load_cmd.ta_handle;psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA,(void *)&unload_cmd,sizeof(unload_cmd), &ret);} else {set_session_id(load_cmd.ta_handle, 0, &arg->session);
} else {arg->ret_origin = load_cmd.return_origin;if (arg->ret == TEEC_SUCCESS) {ret = get_ta_refcount(load_cmd.ta_handle);if (!ret) {arg->ret_origin = TEEC_ORIGIN_COMMS;arg->ret = TEEC_ERROR_OUT_OF_MEMORY;/* Unload the TA on error */unload_cmd.ta_handle = load_cmd.ta_handle;psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA,(void *)&unload_cmd,sizeof(unload_cmd), &ret);} else {set_session_id(load_cmd.ta_handle, 0, &arg->session);} } } mutex_unlock(&ta_refcount_mutex);-- 2.25.1
op-tee@lists.trustedfirmware.org
-
Jens Wiklander -
Rijo Thomas