2023-05-09 08:08:05

by Rijo Thomas

Subject: [PATCH v2 1/1] tee: amdtee: Add return_origin to 'struct tee_cmd_load_ta'

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: [email protected]
Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver")
Tested-by: Sourabh Das <[email protected]>
Signed-off-by: Rijo Thomas <[email protected]>
---
v2:
* Added Fixes tag.

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

2023-05-10 11:35:16

by Sumit Garg

Subject: Re: [PATCH v2 1/1] tee: amdtee: Add return_origin to 'struct tee_cmd_load_ta'

On Tue, 9 May 2023 at 13:03, Rijo Thomas <[email protected]> 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 patch has been verified on Phoenix Birman setup. On older APUs,
> return_origin value will be 0.
>
> Cc: [email protected]
> Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver")
> Tested-by: Sourabh Das <[email protected]>
> Signed-off-by: Rijo Thomas <[email protected]>
> ---
> v2:
> * Added Fixes tag.
>
> drivers/tee/amdtee/amdtee_if.h | 10 ++++++----
> drivers/tee/amdtee/call.c | 30 +++++++++++++++++-------------
> 2 files changed, 23 insertions(+), 17 deletions(-)
>

Acked-by: Sumit Garg <[email protected]>

-Sumit

> 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
>

2023-05-23 06:16:12

by Jens Wiklander

Subject: Re: [PATCH v2 1/1] tee: amdtee: Add return_origin to 'struct tee_cmd_load_ta'

On Wed, May 10, 2023 at 1:19 PM Sumit Garg <[email protected]> wrote:
>
> On Tue, 9 May 2023 at 13:03, Rijo Thomas <[email protected]> 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 patch has been verified on Phoenix Birman setup. On older APUs,
> > return_origin value will be 0.
> >
> > Cc: [email protected]
> > Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver")
> > Tested-by: Sourabh Das <[email protected]>
> > Signed-off-by: Rijo Thomas <[email protected]>
> > ---
> > v2:
> > * Added Fixes tag.
> >
> > drivers/tee/amdtee/amdtee_if.h | 10 ++++++----
> > drivers/tee/amdtee/call.c | 30 +++++++++++++++++-------------
> > 2 files changed, 23 insertions(+), 17 deletions(-)
> >
>
> Acked-by: Sumit Garg <[email protected]>

I'm picking up this.

Thanks,
Jens

>
> -Sumit
>
> > 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
> >