[v6,1/4] tee: optee: system call property

Message ID 20230505173012.881083-1-etienne.carriere@linaro.org
State New
Headers
Series [v6,1/4] tee: optee: system call property |

Commit Message

Etienne Carriere May 5, 2023, 5:30 p.m. UTC
  Adds an argument to do_call_with_arg() handler to tell whether the call
is a system call or nor. This change always sets this info to false
hence no functional change.

This change prepares management of system invocation proposed in a later
change.

Co-developed-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
No changes since v5

Changes since v4:
- New change, extracted from PATCH v4 1/2 (tee: system invocation") and
  revised to cover preparatory changes in optee driver for system session
  support with contribution from Jens.
---
 drivers/tee/optee/call.c          | 22 ++++++++++++++++------
 drivers/tee/optee/core.c          |  5 +++--
 drivers/tee/optee/ffa_abi.c       |  3 ++-
 drivers/tee/optee/optee_private.h |  7 +++++--
 drivers/tee/optee/smc_abi.c       |  9 +++++----
 5 files changed, 31 insertions(+), 15 deletions(-)
  

Comments

Sumit Garg May 10, 2023, 8:22 a.m. UTC | #1
Hi Etienne,

Apologies for the late re-review for this patchset. It has been quite
a busy last month for me.

As a side note, I would appreciate it if you add a cover letter to
your patchset. A cover letter is generally preferred if there is a
patchset containing 2 or more patches.

On Fri, 5 May 2023 at 23:01, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> Adds an argument to do_call_with_arg() handler to tell whether the call
> is a system call or nor. This change always sets this info to false
> hence no functional change.
>
> This change prepares management of system invocation proposed in a later
> change.
>
> Co-developed-by: Jens Wiklander <jens.wiklander@linaro.org>
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
> No changes since v5
>
> Changes since v4:
> - New change, extracted from PATCH v4 1/2 (tee: system invocation") and
>   revised to cover preparatory changes in optee driver for system session
>   support with contribution from Jens.
> ---
>  drivers/tee/optee/call.c          | 22 ++++++++++++++++------
>  drivers/tee/optee/core.c          |  5 +++--
>  drivers/tee/optee/ffa_abi.c       |  3 ++-
>  drivers/tee/optee/optee_private.h |  7 +++++--
>  drivers/tee/optee/smc_abi.c       |  9 +++++----
>  5 files changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c
> index df5fb5410b72..dba5339b61ae 100644
> --- a/drivers/tee/optee/call.c
> +++ b/drivers/tee/optee/call.c
> @@ -328,7 +328,8 @@ int optee_open_session(struct tee_context *ctx,
>                 goto out;
>         }
>
> -       if (optee->ops->do_call_with_arg(ctx, shm, offs)) {
> +       if (optee->ops->do_call_with_arg(ctx, shm, offs,
> +                                        sess->use_sys_thread)) {
>                 msg_arg->ret = TEEC_ERROR_COMMUNICATION;
>                 msg_arg->ret_origin = TEEC_ORIGIN_COMMS;
>         }
> @@ -360,7 +361,8 @@ int optee_open_session(struct tee_context *ctx,
>         return rc;
>  }
>
> -int optee_close_session_helper(struct tee_context *ctx, u32 session)
> +int optee_close_session_helper(struct tee_context *ctx, u32 session,
> +                              bool system_thread)
>  {
>         struct optee *optee = tee_get_drvdata(ctx->teedev);
>         struct optee_shm_arg_entry *entry;
> @@ -374,7 +376,7 @@ int optee_close_session_helper(struct tee_context *ctx, u32 session)
>
>         msg_arg->cmd = OPTEE_MSG_CMD_CLOSE_SESSION;
>         msg_arg->session = session;
> -       optee->ops->do_call_with_arg(ctx, shm, offs);
> +       optee->ops->do_call_with_arg(ctx, shm, offs, system_thread);
>
>         optee_free_msg_arg(ctx, entry, offs);
>
> @@ -385,6 +387,7 @@ int optee_close_session(struct tee_context *ctx, u32 session)
>  {
>         struct optee_context_data *ctxdata = ctx->data;
>         struct optee_session *sess;
> +       bool system_thread;
>
>         /* Check that the session is valid and remove it from the list */
>         mutex_lock(&ctxdata->mutex);
> @@ -394,9 +397,10 @@ int optee_close_session(struct tee_context *ctx, u32 session)
>         mutex_unlock(&ctxdata->mutex);
>         if (!sess)
>                 return -EINVAL;
> +       system_thread = sess->use_sys_thread;
>         kfree(sess);
>
> -       return optee_close_session_helper(ctx, session);
> +       return optee_close_session_helper(ctx, session, system_thread);
>  }
>
>  int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> @@ -408,12 +412,15 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
>         struct optee_msg_arg *msg_arg;
>         struct optee_session *sess;
>         struct tee_shm *shm;
> +       bool system_thread;
>         u_int offs;
>         int rc;
>
>         /* Check that the session is valid */
>         mutex_lock(&ctxdata->mutex);
>         sess = find_session(ctxdata, arg->session);
> +       if (sess)

This check is redundant if we move the assignment below...

> +               system_thread = sess->use_sys_thread;
>         mutex_unlock(&ctxdata->mutex);
>         if (!sess)
>                 return -EINVAL;

...here as:
           system_thread = sess->use_sys_thread;

> @@ -432,7 +439,7 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
>         if (rc)
>                 goto out;
>
> -       if (optee->ops->do_call_with_arg(ctx, shm, offs)) {
> +       if (optee->ops->do_call_with_arg(ctx, shm, offs, system_thread)) {
>                 msg_arg->ret = TEEC_ERROR_COMMUNICATION;
>                 msg_arg->ret_origin = TEEC_ORIGIN_COMMS;
>         }
> @@ -457,12 +464,15 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session)
>         struct optee_shm_arg_entry *entry;
>         struct optee_msg_arg *msg_arg;
>         struct optee_session *sess;
> +       bool system_thread;
>         struct tee_shm *shm;
>         u_int offs;
>
>         /* Check that the session is valid */
>         mutex_lock(&ctxdata->mutex);
>         sess = find_session(ctxdata, session);
> +       if (sess)

Ditto.

-Sumit

> +               system_thread = sess->use_sys_thread;
>         mutex_unlock(&ctxdata->mutex);
>         if (!sess)
>                 return -EINVAL;
> @@ -474,7 +484,7 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session)
>         msg_arg->cmd = OPTEE_MSG_CMD_CANCEL;
>         msg_arg->session = session;
>         msg_arg->cancel_id = cancel_id;
> -       optee->ops->do_call_with_arg(ctx, shm, offs);
> +       optee->ops->do_call_with_arg(ctx, shm, offs, system_thread);
>
>         optee_free_msg_arg(ctx, entry, offs);
>         return 0;
> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 2a258bd3b6b5..d01ca47f7bde 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -129,7 +129,8 @@ int optee_open(struct tee_context *ctx, bool cap_memref_null)
>
>  static void optee_release_helper(struct tee_context *ctx,
>                                  int (*close_session)(struct tee_context *ctx,
> -                                                     u32 session))
> +                                                     u32 session,
> +                                                     bool system_thread))
>  {
>         struct optee_context_data *ctxdata = ctx->data;
>         struct optee_session *sess;
> @@ -141,7 +142,7 @@ static void optee_release_helper(struct tee_context *ctx,
>         list_for_each_entry_safe(sess, sess_tmp, &ctxdata->sess_list,
>                                  list_node) {
>                 list_del(&sess->list_node);
> -               close_session(ctx, sess->session_id);
> +               close_session(ctx, sess->session_id, sess->use_sys_thread);
>                 kfree(sess);
>         }
>         kfree(ctxdata);
> diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
> index 0828240f27e6..52cec9d06041 100644
> --- a/drivers/tee/optee/ffa_abi.c
> +++ b/drivers/tee/optee/ffa_abi.c
> @@ -612,7 +612,8 @@ static int optee_ffa_yielding_call(struct tee_context *ctx,
>   */
>
>  static int optee_ffa_do_call_with_arg(struct tee_context *ctx,
> -                                     struct tee_shm *shm, u_int offs)
> +                                     struct tee_shm *shm, u_int offs,
> +                                     bool system_thread)
>  {
>         struct ffa_send_direct_data data = {
>                 .data0 = OPTEE_FFA_YIELDING_CALL_WITH_ARG,
> diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> index 72685ee0d53f..3da7960ab34a 100644
> --- a/drivers/tee/optee/optee_private.h
> +++ b/drivers/tee/optee/optee_private.h
> @@ -154,7 +154,8 @@ struct optee;
>   */
>  struct optee_ops {
>         int (*do_call_with_arg)(struct tee_context *ctx,
> -                               struct tee_shm *shm_arg, u_int offs);
> +                               struct tee_shm *shm_arg, u_int offs,
> +                               bool system_thread);
>         int (*to_msg_param)(struct optee *optee,
>                             struct optee_msg_param *msg_params,
>                             size_t num_params, const struct tee_param *params);
> @@ -204,6 +205,7 @@ struct optee {
>  struct optee_session {
>         struct list_head list_node;
>         u32 session_id;
> +       bool use_sys_thread;
>  };
>
>  struct optee_context_data {
> @@ -252,7 +254,8 @@ int optee_supp_send(struct tee_context *ctx, u32 ret, u32 num_params,
>  int optee_open_session(struct tee_context *ctx,
>                        struct tee_ioctl_open_session_arg *arg,
>                        struct tee_param *param);
> -int optee_close_session_helper(struct tee_context *ctx, u32 session);
> +int optee_close_session_helper(struct tee_context *ctx, u32 session,
> +                              bool system_thread);
>  int optee_close_session(struct tee_context *ctx, u32 session);
>  int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
>                       struct tee_param *param);
> diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> index 49702cb08f4f..56ebbb96ac97 100644
> --- a/drivers/tee/optee/smc_abi.c
> +++ b/drivers/tee/optee/smc_abi.c
> @@ -507,7 +507,7 @@ static int optee_shm_register(struct tee_context *ctx, struct tee_shm *shm,
>         msg_arg->params->u.tmem.buf_ptr = virt_to_phys(pages_list) |
>           (tee_shm_get_page_offset(shm) & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1));
>
> -       if (optee->ops->do_call_with_arg(ctx, shm_arg, 0) ||
> +       if (optee->ops->do_call_with_arg(ctx, shm_arg, 0, false) ||
>             msg_arg->ret != TEEC_SUCCESS)
>                 rc = -EINVAL;
>
> @@ -550,7 +550,7 @@ static int optee_shm_unregister(struct tee_context *ctx, struct tee_shm *shm)
>         msg_arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_RMEM_INPUT;
>         msg_arg->params[0].u.rmem.shm_ref = (unsigned long)shm;
>
> -       if (optee->ops->do_call_with_arg(ctx, shm_arg, 0) ||
> +       if (optee->ops->do_call_with_arg(ctx, shm_arg, 0, false) ||
>             msg_arg->ret != TEEC_SUCCESS)
>                 rc = -EINVAL;
>  out:
> @@ -885,7 +885,8 @@ static void optee_handle_rpc(struct tee_context *ctx,
>   * Returns return code from secure world, 0 is OK
>   */
>  static int optee_smc_do_call_with_arg(struct tee_context *ctx,
> -                                     struct tee_shm *shm, u_int offs)
> +                                     struct tee_shm *shm, u_int offs,
> +                                     bool system_thread)
>  {
>         struct optee *optee = tee_get_drvdata(ctx->teedev);
>         struct optee_call_waiter w;
> @@ -977,7 +978,7 @@ static int simple_call_with_arg(struct tee_context *ctx, u32 cmd)
>                 return PTR_ERR(msg_arg);
>
>         msg_arg->cmd = cmd;
> -       optee_smc_do_call_with_arg(ctx, shm, offs);
> +       optee_smc_do_call_with_arg(ctx, shm, offs, false);
>
>         optee_free_msg_arg(ctx, entry, offs);
>         return 0;
> --
> 2.25.1
>
  
Etienne Carriere May 10, 2023, 3:02 p.m. UTC | #2
Hello Sumit,

On Wed, 10 May 2023 at 10:22, Sumit Garg <sumit.garg@linaro.org> wrote:
>
> Hi Etienne,
>
> Apologies for the late re-review for this patchset. It has been quite
> a busy last month for me.
>
> As a side note, I would appreciate it if you add a cover letter to
> your patchset. A cover letter is generally preferred if there is a
> patchset containing 2 or more patches.

Ok, I'll do in patch v7.

>
> On Fri, 5 May 2023 at 23:01, Etienne Carriere
> <etienne.carriere@linaro.org> wrote:
> >
> > Adds an argument to do_call_with_arg() handler to tell whether the call
> > is a system call or nor. This change always sets this info to false
> > hence no functional change.
> >
> > This change prepares management of system invocation proposed in a later
> > change.
> >
> > Co-developed-by: Jens Wiklander <jens.wiklander@linaro.org>
> > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > ---
> > No changes since v5
> >
> > Changes since v4:
> > - New change, extracted from PATCH v4 1/2 (tee: system invocation") and
> >   revised to cover preparatory changes in optee driver for system session
> >   support with contribution from Jens.
> > ---
> >  drivers/tee/optee/call.c          | 22 ++++++++++++++++------
> >  drivers/tee/optee/core.c          |  5 +++--
> >  drivers/tee/optee/ffa_abi.c       |  3 ++-
> >  drivers/tee/optee/optee_private.h |  7 +++++--
> >  drivers/tee/optee/smc_abi.c       |  9 +++++----
> >  5 files changed, 31 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c
> > index df5fb5410b72..dba5339b61ae 100644
> > --- a/drivers/tee/optee/call.c
> > +++ b/drivers/tee/optee/call.c
> > @@ -328,7 +328,8 @@ int optee_open_session(struct tee_context *ctx,
> >                 goto out;
> >         }
> >
> > -       if (optee->ops->do_call_with_arg(ctx, shm, offs)) {
> > +       if (optee->ops->do_call_with_arg(ctx, shm, offs,
> > +                                        sess->use_sys_thread)) {
> >                 msg_arg->ret = TEEC_ERROR_COMMUNICATION;
> >                 msg_arg->ret_origin = TEEC_ORIGIN_COMMS;
> >         }
> > @@ -360,7 +361,8 @@ int optee_open_session(struct tee_context *ctx,
> >         return rc;
> >  }
> >
> > -int optee_close_session_helper(struct tee_context *ctx, u32 session)
> > +int optee_close_session_helper(struct tee_context *ctx, u32 session,
> > +                              bool system_thread)
> >  {
> >         struct optee *optee = tee_get_drvdata(ctx->teedev);
> >         struct optee_shm_arg_entry *entry;
> > @@ -374,7 +376,7 @@ int optee_close_session_helper(struct tee_context *ctx, u32 session)
> >
> >         msg_arg->cmd = OPTEE_MSG_CMD_CLOSE_SESSION;
> >         msg_arg->session = session;
> > -       optee->ops->do_call_with_arg(ctx, shm, offs);
> > +       optee->ops->do_call_with_arg(ctx, shm, offs, system_thread);
> >
> >         optee_free_msg_arg(ctx, entry, offs);
> >
> > @@ -385,6 +387,7 @@ int optee_close_session(struct tee_context *ctx, u32 session)
> >  {
> >         struct optee_context_data *ctxdata = ctx->data;
> >         struct optee_session *sess;
> > +       bool system_thread;
> >
> >         /* Check that the session is valid and remove it from the list */
> >         mutex_lock(&ctxdata->mutex);
> > @@ -394,9 +397,10 @@ int optee_close_session(struct tee_context *ctx, u32 session)
> >         mutex_unlock(&ctxdata->mutex);
> >         if (!sess)
> >                 return -EINVAL;
> > +       system_thread = sess->use_sys_thread;
> >         kfree(sess);
> >
> > -       return optee_close_session_helper(ctx, session);
> > +       return optee_close_session_helper(ctx, session, system_thread);
> >  }
> >
> >  int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> > @@ -408,12 +412,15 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> >         struct optee_msg_arg *msg_arg;
> >         struct optee_session *sess;
> >         struct tee_shm *shm;
> > +       bool system_thread;
> >         u_int offs;
> >         int rc;
> >
> >         /* Check that the session is valid */
> >         mutex_lock(&ctxdata->mutex);
> >         sess = find_session(ctxdata, arg->session);
> > +       if (sess)
>
> This check is redundant if we move the assignment below...

Here we change the sesssion attribute while the mutex is locked, in
case some equivalent call with that session is issued.
Below we return to caller once mutex is unlocked.
I think it is the safer behavior. What do you think?

Best regards,
Etienne

>
> > +               system_thread = sess->use_sys_thread;
> >         mutex_unlock(&ctxdata->mutex);
> >         if (!sess)
> >                 return -EINVAL;
>
> ...here as:
>            system_thread = sess->use_sys_thread;
>
> > @@ -432,7 +439,7 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> >         if (rc)
> >                 goto out;
> >
> > -       if (optee->ops->do_call_with_arg(ctx, shm, offs)) {
> > +       if (optee->ops->do_call_with_arg(ctx, shm, offs, system_thread)) {
> >                 msg_arg->ret = TEEC_ERROR_COMMUNICATION;
> >                 msg_arg->ret_origin = TEEC_ORIGIN_COMMS;
> >         }
> > @@ -457,12 +464,15 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session)
> >         struct optee_shm_arg_entry *entry;
> >         struct optee_msg_arg *msg_arg;
> >         struct optee_session *sess;
> > +       bool system_thread;
> >         struct tee_shm *shm;
> >         u_int offs;
> >
> >         /* Check that the session is valid */
> >         mutex_lock(&ctxdata->mutex);
> >         sess = find_session(ctxdata, session);
> > +       if (sess)
>
> Ditto.
>
> -Sumit
>
> > +               system_thread = sess->use_sys_thread;
> >         mutex_unlock(&ctxdata->mutex);
> >         if (!sess)
> >                 return -EINVAL;
> > @@ -474,7 +484,7 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session)
> >         msg_arg->cmd = OPTEE_MSG_CMD_CANCEL;
> >         msg_arg->session = session;
> >         msg_arg->cancel_id = cancel_id;
> > -       optee->ops->do_call_with_arg(ctx, shm, offs);
> > +       optee->ops->do_call_with_arg(ctx, shm, offs, system_thread);
> >
> >         optee_free_msg_arg(ctx, entry, offs);
> >         return 0;
> > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> > index 2a258bd3b6b5..d01ca47f7bde 100644
> > --- a/drivers/tee/optee/core.c
> > +++ b/drivers/tee/optee/core.c
> > @@ -129,7 +129,8 @@ int optee_open(struct tee_context *ctx, bool cap_memref_null)
> >
> >  static void optee_release_helper(struct tee_context *ctx,
> >                                  int (*close_session)(struct tee_context *ctx,
> > -                                                     u32 session))
> > +                                                     u32 session,
> > +                                                     bool system_thread))
> >  {
> >         struct optee_context_data *ctxdata = ctx->data;
> >         struct optee_session *sess;
> > @@ -141,7 +142,7 @@ static void optee_release_helper(struct tee_context *ctx,
> >         list_for_each_entry_safe(sess, sess_tmp, &ctxdata->sess_list,
> >                                  list_node) {
> >                 list_del(&sess->list_node);
> > -               close_session(ctx, sess->session_id);
> > +               close_session(ctx, sess->session_id, sess->use_sys_thread);
> >                 kfree(sess);
> >         }
> >         kfree(ctxdata);
> > diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
> > index 0828240f27e6..52cec9d06041 100644
> > --- a/drivers/tee/optee/ffa_abi.c
> > +++ b/drivers/tee/optee/ffa_abi.c
> > @@ -612,7 +612,8 @@ static int optee_ffa_yielding_call(struct tee_context *ctx,
> >   */
> >
> >  static int optee_ffa_do_call_with_arg(struct tee_context *ctx,
> > -                                     struct tee_shm *shm, u_int offs)
> > +                                     struct tee_shm *shm, u_int offs,
> > +                                     bool system_thread)
> >  {
> >         struct ffa_send_direct_data data = {
> >                 .data0 = OPTEE_FFA_YIELDING_CALL_WITH_ARG,
> > diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> > index 72685ee0d53f..3da7960ab34a 100644
> > --- a/drivers/tee/optee/optee_private.h
> > +++ b/drivers/tee/optee/optee_private.h
> > @@ -154,7 +154,8 @@ struct optee;
> >   */
> >  struct optee_ops {
> >         int (*do_call_with_arg)(struct tee_context *ctx,
> > -                               struct tee_shm *shm_arg, u_int offs);
> > +                               struct tee_shm *shm_arg, u_int offs,
> > +                               bool system_thread);
> >         int (*to_msg_param)(struct optee *optee,
> >                             struct optee_msg_param *msg_params,
> >                             size_t num_params, const struct tee_param *params);
> > @@ -204,6 +205,7 @@ struct optee {
> >  struct optee_session {
> >         struct list_head list_node;
> >         u32 session_id;
> > +       bool use_sys_thread;
> >  };
> >
> >  struct optee_context_data {
> > @@ -252,7 +254,8 @@ int optee_supp_send(struct tee_context *ctx, u32 ret, u32 num_params,
> >  int optee_open_session(struct tee_context *ctx,
> >                        struct tee_ioctl_open_session_arg *arg,
> >                        struct tee_param *param);
> > -int optee_close_session_helper(struct tee_context *ctx, u32 session);
> > +int optee_close_session_helper(struct tee_context *ctx, u32 session,
> > +                              bool system_thread);
> >  int optee_close_session(struct tee_context *ctx, u32 session);
> >  int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> >                       struct tee_param *param);
> > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> > index 49702cb08f4f..56ebbb96ac97 100644
> > --- a/drivers/tee/optee/smc_abi.c
> > +++ b/drivers/tee/optee/smc_abi.c
> > @@ -507,7 +507,7 @@ static int optee_shm_register(struct tee_context *ctx, struct tee_shm *shm,
> >         msg_arg->params->u.tmem.buf_ptr = virt_to_phys(pages_list) |
> >           (tee_shm_get_page_offset(shm) & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1));
> >
> > -       if (optee->ops->do_call_with_arg(ctx, shm_arg, 0) ||
> > +       if (optee->ops->do_call_with_arg(ctx, shm_arg, 0, false) ||
> >             msg_arg->ret != TEEC_SUCCESS)
> >                 rc = -EINVAL;
> >
> > @@ -550,7 +550,7 @@ static int optee_shm_unregister(struct tee_context *ctx, struct tee_shm *shm)
> >         msg_arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_RMEM_INPUT;
> >         msg_arg->params[0].u.rmem.shm_ref = (unsigned long)shm;
> >
> > -       if (optee->ops->do_call_with_arg(ctx, shm_arg, 0) ||
> > +       if (optee->ops->do_call_with_arg(ctx, shm_arg, 0, false) ||
> >             msg_arg->ret != TEEC_SUCCESS)
> >                 rc = -EINVAL;
> >  out:
> > @@ -885,7 +885,8 @@ static void optee_handle_rpc(struct tee_context *ctx,
> >   * Returns return code from secure world, 0 is OK
> >   */
> >  static int optee_smc_do_call_with_arg(struct tee_context *ctx,
> > -                                     struct tee_shm *shm, u_int offs)
> > +                                     struct tee_shm *shm, u_int offs,
> > +                                     bool system_thread)
> >  {
> >         struct optee *optee = tee_get_drvdata(ctx->teedev);
> >         struct optee_call_waiter w;
> > @@ -977,7 +978,7 @@ static int simple_call_with_arg(struct tee_context *ctx, u32 cmd)
> >                 return PTR_ERR(msg_arg);
> >
> >         msg_arg->cmd = cmd;
> > -       optee_smc_do_call_with_arg(ctx, shm, offs);
> > +       optee_smc_do_call_with_arg(ctx, shm, offs, false);
> >
> >         optee_free_msg_arg(ctx, entry, offs);
> >         return 0;
> > --
> > 2.25.1
> >
  
Sumit Garg May 11, 2023, 6:03 a.m. UTC | #3
On Wed, 10 May 2023 at 20:33, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> Hello Sumit,
>
> On Wed, 10 May 2023 at 10:22, Sumit Garg <sumit.garg@linaro.org> wrote:
> >
> > Hi Etienne,
> >
> > Apologies for the late re-review for this patchset. It has been quite
> > a busy last month for me.
> >
> > As a side note, I would appreciate it if you add a cover letter to
> > your patchset. A cover letter is generally preferred if there is a
> > patchset containing 2 or more patches.
>
> Ok, I'll do in patch v7.
>
> >
> > On Fri, 5 May 2023 at 23:01, Etienne Carriere
> > <etienne.carriere@linaro.org> wrote:
> > >
> > > Adds an argument to do_call_with_arg() handler to tell whether the call
> > > is a system call or nor. This change always sets this info to false
> > > hence no functional change.
> > >
> > > This change prepares management of system invocation proposed in a later
> > > change.
> > >
> > > Co-developed-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > > ---
> > > No changes since v5
> > >
> > > Changes since v4:
> > > - New change, extracted from PATCH v4 1/2 (tee: system invocation") and
> > >   revised to cover preparatory changes in optee driver for system session
> > >   support with contribution from Jens.
> > > ---
> > >  drivers/tee/optee/call.c          | 22 ++++++++++++++++------
> > >  drivers/tee/optee/core.c          |  5 +++--
> > >  drivers/tee/optee/ffa_abi.c       |  3 ++-
> > >  drivers/tee/optee/optee_private.h |  7 +++++--
> > >  drivers/tee/optee/smc_abi.c       |  9 +++++----
> > >  5 files changed, 31 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c
> > > index df5fb5410b72..dba5339b61ae 100644
> > > --- a/drivers/tee/optee/call.c
> > > +++ b/drivers/tee/optee/call.c
> > > @@ -328,7 +328,8 @@ int optee_open_session(struct tee_context *ctx,
> > >                 goto out;
> > >         }
> > >
> > > -       if (optee->ops->do_call_with_arg(ctx, shm, offs)) {
> > > +       if (optee->ops->do_call_with_arg(ctx, shm, offs,
> > > +                                        sess->use_sys_thread)) {
> > >                 msg_arg->ret = TEEC_ERROR_COMMUNICATION;
> > >                 msg_arg->ret_origin = TEEC_ORIGIN_COMMS;
> > >         }
> > > @@ -360,7 +361,8 @@ int optee_open_session(struct tee_context *ctx,
> > >         return rc;
> > >  }
> > >
> > > -int optee_close_session_helper(struct tee_context *ctx, u32 session)
> > > +int optee_close_session_helper(struct tee_context *ctx, u32 session,
> > > +                              bool system_thread)
> > >  {
> > >         struct optee *optee = tee_get_drvdata(ctx->teedev);
> > >         struct optee_shm_arg_entry *entry;
> > > @@ -374,7 +376,7 @@ int optee_close_session_helper(struct tee_context *ctx, u32 session)
> > >
> > >         msg_arg->cmd = OPTEE_MSG_CMD_CLOSE_SESSION;
> > >         msg_arg->session = session;
> > > -       optee->ops->do_call_with_arg(ctx, shm, offs);
> > > +       optee->ops->do_call_with_arg(ctx, shm, offs, system_thread);
> > >
> > >         optee_free_msg_arg(ctx, entry, offs);
> > >
> > > @@ -385,6 +387,7 @@ int optee_close_session(struct tee_context *ctx, u32 session)
> > >  {
> > >         struct optee_context_data *ctxdata = ctx->data;
> > >         struct optee_session *sess;
> > > +       bool system_thread;
> > >
> > >         /* Check that the session is valid and remove it from the list */
> > >         mutex_lock(&ctxdata->mutex);
> > > @@ -394,9 +397,10 @@ int optee_close_session(struct tee_context *ctx, u32 session)
> > >         mutex_unlock(&ctxdata->mutex);
> > >         if (!sess)
> > >                 return -EINVAL;
> > > +       system_thread = sess->use_sys_thread;
> > >         kfree(sess);
> > >
> > > -       return optee_close_session_helper(ctx, session);
> > > +       return optee_close_session_helper(ctx, session, system_thread);
> > >  }
> > >
> > >  int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> > > @@ -408,12 +412,15 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> > >         struct optee_msg_arg *msg_arg;
> > >         struct optee_session *sess;
> > >         struct tee_shm *shm;
> > > +       bool system_thread;
> > >         u_int offs;
> > >         int rc;
> > >
> > >         /* Check that the session is valid */
> > >         mutex_lock(&ctxdata->mutex);
> > >         sess = find_session(ctxdata, arg->session);
> > > +       if (sess)
> >
> > This check is redundant if we move the assignment below...
>
> Here we change the sesssion attribute while the mutex is locked, in
> case some equivalent call with that session is issued.
> Below we return to caller once mutex is unlocked.
> I think it is the safer behavior. What do you think?

Aren't we only reading session attribute in order to capture value in
a local variable: system_thread? I don't think that it would require a
mutex.

-Sumit

>
> Best regards,
> Etienne
>
> >
> > > +               system_thread = sess->use_sys_thread;
> > >         mutex_unlock(&ctxdata->mutex);
> > >         if (!sess)
> > >                 return -EINVAL;
> >
> > ...here as:
> >            system_thread = sess->use_sys_thread;
> >
> > > @@ -432,7 +439,7 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> > >         if (rc)
> > >                 goto out;
> > >
> > > -       if (optee->ops->do_call_with_arg(ctx, shm, offs)) {
> > > +       if (optee->ops->do_call_with_arg(ctx, shm, offs, system_thread)) {
> > >                 msg_arg->ret = TEEC_ERROR_COMMUNICATION;
> > >                 msg_arg->ret_origin = TEEC_ORIGIN_COMMS;
> > >         }
> > > @@ -457,12 +464,15 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session)
> > >         struct optee_shm_arg_entry *entry;
> > >         struct optee_msg_arg *msg_arg;
> > >         struct optee_session *sess;
> > > +       bool system_thread;
> > >         struct tee_shm *shm;
> > >         u_int offs;
> > >
> > >         /* Check that the session is valid */
> > >         mutex_lock(&ctxdata->mutex);
> > >         sess = find_session(ctxdata, session);
> > > +       if (sess)
> >
> > Ditto.
> >
> > -Sumit
> >
> > > +               system_thread = sess->use_sys_thread;
> > >         mutex_unlock(&ctxdata->mutex);
> > >         if (!sess)
> > >                 return -EINVAL;
> > > @@ -474,7 +484,7 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session)
> > >         msg_arg->cmd = OPTEE_MSG_CMD_CANCEL;
> > >         msg_arg->session = session;
> > >         msg_arg->cancel_id = cancel_id;
> > > -       optee->ops->do_call_with_arg(ctx, shm, offs);
> > > +       optee->ops->do_call_with_arg(ctx, shm, offs, system_thread);
> > >
> > >         optee_free_msg_arg(ctx, entry, offs);
> > >         return 0;
> > > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> > > index 2a258bd3b6b5..d01ca47f7bde 100644
> > > --- a/drivers/tee/optee/core.c
> > > +++ b/drivers/tee/optee/core.c
> > > @@ -129,7 +129,8 @@ int optee_open(struct tee_context *ctx, bool cap_memref_null)
> > >
> > >  static void optee_release_helper(struct tee_context *ctx,
> > >                                  int (*close_session)(struct tee_context *ctx,
> > > -                                                     u32 session))
> > > +                                                     u32 session,
> > > +                                                     bool system_thread))
> > >  {
> > >         struct optee_context_data *ctxdata = ctx->data;
> > >         struct optee_session *sess;
> > > @@ -141,7 +142,7 @@ static void optee_release_helper(struct tee_context *ctx,
> > >         list_for_each_entry_safe(sess, sess_tmp, &ctxdata->sess_list,
> > >                                  list_node) {
> > >                 list_del(&sess->list_node);
> > > -               close_session(ctx, sess->session_id);
> > > +               close_session(ctx, sess->session_id, sess->use_sys_thread);
> > >                 kfree(sess);
> > >         }
> > >         kfree(ctxdata);
> > > diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
> > > index 0828240f27e6..52cec9d06041 100644
> > > --- a/drivers/tee/optee/ffa_abi.c
> > > +++ b/drivers/tee/optee/ffa_abi.c
> > > @@ -612,7 +612,8 @@ static int optee_ffa_yielding_call(struct tee_context *ctx,
> > >   */
> > >
> > >  static int optee_ffa_do_call_with_arg(struct tee_context *ctx,
> > > -                                     struct tee_shm *shm, u_int offs)
> > > +                                     struct tee_shm *shm, u_int offs,
> > > +                                     bool system_thread)
> > >  {
> > >         struct ffa_send_direct_data data = {
> > >                 .data0 = OPTEE_FFA_YIELDING_CALL_WITH_ARG,
> > > diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> > > index 72685ee0d53f..3da7960ab34a 100644
> > > --- a/drivers/tee/optee/optee_private.h
> > > +++ b/drivers/tee/optee/optee_private.h
> > > @@ -154,7 +154,8 @@ struct optee;
> > >   */
> > >  struct optee_ops {
> > >         int (*do_call_with_arg)(struct tee_context *ctx,
> > > -                               struct tee_shm *shm_arg, u_int offs);
> > > +                               struct tee_shm *shm_arg, u_int offs,
> > > +                               bool system_thread);
> > >         int (*to_msg_param)(struct optee *optee,
> > >                             struct optee_msg_param *msg_params,
> > >                             size_t num_params, const struct tee_param *params);
> > > @@ -204,6 +205,7 @@ struct optee {
> > >  struct optee_session {
> > >         struct list_head list_node;
> > >         u32 session_id;
> > > +       bool use_sys_thread;
> > >  };
> > >
> > >  struct optee_context_data {
> > > @@ -252,7 +254,8 @@ int optee_supp_send(struct tee_context *ctx, u32 ret, u32 num_params,
> > >  int optee_open_session(struct tee_context *ctx,
> > >                        struct tee_ioctl_open_session_arg *arg,
> > >                        struct tee_param *param);
> > > -int optee_close_session_helper(struct tee_context *ctx, u32 session);
> > > +int optee_close_session_helper(struct tee_context *ctx, u32 session,
> > > +                              bool system_thread);
> > >  int optee_close_session(struct tee_context *ctx, u32 session);
> > >  int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> > >                       struct tee_param *param);
> > > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> > > index 49702cb08f4f..56ebbb96ac97 100644
> > > --- a/drivers/tee/optee/smc_abi.c
> > > +++ b/drivers/tee/optee/smc_abi.c
> > > @@ -507,7 +507,7 @@ static int optee_shm_register(struct tee_context *ctx, struct tee_shm *shm,
> > >         msg_arg->params->u.tmem.buf_ptr = virt_to_phys(pages_list) |
> > >           (tee_shm_get_page_offset(shm) & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1));
> > >
> > > -       if (optee->ops->do_call_with_arg(ctx, shm_arg, 0) ||
> > > +       if (optee->ops->do_call_with_arg(ctx, shm_arg, 0, false) ||
> > >             msg_arg->ret != TEEC_SUCCESS)
> > >                 rc = -EINVAL;
> > >
> > > @@ -550,7 +550,7 @@ static int optee_shm_unregister(struct tee_context *ctx, struct tee_shm *shm)
> > >         msg_arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_RMEM_INPUT;
> > >         msg_arg->params[0].u.rmem.shm_ref = (unsigned long)shm;
> > >
> > > -       if (optee->ops->do_call_with_arg(ctx, shm_arg, 0) ||
> > > +       if (optee->ops->do_call_with_arg(ctx, shm_arg, 0, false) ||
> > >             msg_arg->ret != TEEC_SUCCESS)
> > >                 rc = -EINVAL;
> > >  out:
> > > @@ -885,7 +885,8 @@ static void optee_handle_rpc(struct tee_context *ctx,
> > >   * Returns return code from secure world, 0 is OK
> > >   */
> > >  static int optee_smc_do_call_with_arg(struct tee_context *ctx,
> > > -                                     struct tee_shm *shm, u_int offs)
> > > +                                     struct tee_shm *shm, u_int offs,
> > > +                                     bool system_thread)
> > >  {
> > >         struct optee *optee = tee_get_drvdata(ctx->teedev);
> > >         struct optee_call_waiter w;
> > > @@ -977,7 +978,7 @@ static int simple_call_with_arg(struct tee_context *ctx, u32 cmd)
> > >                 return PTR_ERR(msg_arg);
> > >
> > >         msg_arg->cmd = cmd;
> > > -       optee_smc_do_call_with_arg(ctx, shm, offs);
> > > +       optee_smc_do_call_with_arg(ctx, shm, offs, false);
> > >
> > >         optee_free_msg_arg(ctx, entry, offs);
> > >         return 0;
> > > --
> > > 2.25.1
> > >
  
Etienne Carriere May 11, 2023, 7:20 a.m. UTC | #4
On Thu, 11 May 2023 at 08:03, Sumit Garg <sumit.garg@linaro.org> wrote:
> (snip)
> > > >
> > > >  int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> > > > @@ -408,12 +412,15 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> > > >         struct optee_msg_arg *msg_arg;
> > > >         struct optee_session *sess;
> > > >         struct tee_shm *shm;
> > > > +       bool system_thread;
> > > >         u_int offs;
> > > >         int rc;
> > > >
> > > >         /* Check that the session is valid */
> > > >         mutex_lock(&ctxdata->mutex);
> > > >         sess = find_session(ctxdata, arg->session);
> > > > +       if (sess)
> > >
> > > This check is redundant if we move the assignment below...
> >
> > Here we change the sesssion attribute while the mutex is locked, in
> > case some equivalent call with that session is issued.
> > Below we return to caller once mutex is unlocked.
> > I think it is the safer behavior. What do you think?
>
> Aren't we only reading session attribute in order to capture value in
> a local variable: system_thread? I don't think that it would require a
> mutex.

optee_system_session() sets session::use_sys_thread with mutex locked
hence I think we should get the attribute with the mutex locked.
See "[PATCH v6 3/4] tee: optee: support tracking system threads".

Etienne

>
> -Sumit
>
> >
> > Best regards,
> > Etienne
> >
> > >
> > > > +               system_thread = sess->use_sys_thread;
> > > >         mutex_unlock(&ctxdata->mutex);
> > > >         if (!sess)
> > > >                 return -EINVAL;
> > >
> > > ...here as:
> > >            system_thread = sess->use_sys_thread;
> > > (snip)
  
Sumit Garg May 11, 2023, 7:39 a.m. UTC | #5
On Thu, 11 May 2023 at 12:50, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> On Thu, 11 May 2023 at 08:03, Sumit Garg <sumit.garg@linaro.org> wrote:
> > (snip)
> > > > >
> > > > >  int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> > > > > @@ -408,12 +412,15 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> > > > >         struct optee_msg_arg *msg_arg;
> > > > >         struct optee_session *sess;
> > > > >         struct tee_shm *shm;
> > > > > +       bool system_thread;
> > > > >         u_int offs;
> > > > >         int rc;
> > > > >
> > > > >         /* Check that the session is valid */
> > > > >         mutex_lock(&ctxdata->mutex);
> > > > >         sess = find_session(ctxdata, arg->session);
> > > > > +       if (sess)
> > > >
> > > > This check is redundant if we move the assignment below...
> > >
> > > Here we change the sesssion attribute while the mutex is locked, in
> > > case some equivalent call with that session is issued.
> > > Below we return to caller once mutex is unlocked.
> > > I think it is the safer behavior. What do you think?
> >
> > Aren't we only reading session attribute in order to capture value in
> > a local variable: system_thread? I don't think that it would require a
> > mutex.
>
> optee_system_session() sets session::use_sys_thread with mutex locked
> hence I think we should get the attribute with the mutex locked.
> See "[PATCH v6 3/4] tee: optee: support tracking system threads".
>

Okay I see your point. Although I don't see a practical race between
optee_invoke_func() vs optee_system_session(), you never know what
complex kernel TEE client use-case comes up. So I can live with it
being protected by a mutex.

-Sumit

> Etienne
>
> >
> > -Sumit
> >
> > >
> > > Best regards,
> > > Etienne
> > >
> > > >
> > > > > +               system_thread = sess->use_sys_thread;
> > > > >         mutex_unlock(&ctxdata->mutex);
> > > > >         if (!sess)
> > > > >                 return -EINVAL;
> > > >
> > > > ...here as:
> > > >            system_thread = sess->use_sys_thread;
> > > > (snip)
  

Patch

diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c
index df5fb5410b72..dba5339b61ae 100644
--- a/drivers/tee/optee/call.c
+++ b/drivers/tee/optee/call.c
@@ -328,7 +328,8 @@  int optee_open_session(struct tee_context *ctx,
 		goto out;
 	}
 
-	if (optee->ops->do_call_with_arg(ctx, shm, offs)) {
+	if (optee->ops->do_call_with_arg(ctx, shm, offs,
+					 sess->use_sys_thread)) {
 		msg_arg->ret = TEEC_ERROR_COMMUNICATION;
 		msg_arg->ret_origin = TEEC_ORIGIN_COMMS;
 	}
@@ -360,7 +361,8 @@  int optee_open_session(struct tee_context *ctx,
 	return rc;
 }
 
-int optee_close_session_helper(struct tee_context *ctx, u32 session)
+int optee_close_session_helper(struct tee_context *ctx, u32 session,
+			       bool system_thread)
 {
 	struct optee *optee = tee_get_drvdata(ctx->teedev);
 	struct optee_shm_arg_entry *entry;
@@ -374,7 +376,7 @@  int optee_close_session_helper(struct tee_context *ctx, u32 session)
 
 	msg_arg->cmd = OPTEE_MSG_CMD_CLOSE_SESSION;
 	msg_arg->session = session;
-	optee->ops->do_call_with_arg(ctx, shm, offs);
+	optee->ops->do_call_with_arg(ctx, shm, offs, system_thread);
 
 	optee_free_msg_arg(ctx, entry, offs);
 
@@ -385,6 +387,7 @@  int optee_close_session(struct tee_context *ctx, u32 session)
 {
 	struct optee_context_data *ctxdata = ctx->data;
 	struct optee_session *sess;
+	bool system_thread;
 
 	/* Check that the session is valid and remove it from the list */
 	mutex_lock(&ctxdata->mutex);
@@ -394,9 +397,10 @@  int optee_close_session(struct tee_context *ctx, u32 session)
 	mutex_unlock(&ctxdata->mutex);
 	if (!sess)
 		return -EINVAL;
+	system_thread = sess->use_sys_thread;
 	kfree(sess);
 
-	return optee_close_session_helper(ctx, session);
+	return optee_close_session_helper(ctx, session, system_thread);
 }
 
 int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
@@ -408,12 +412,15 @@  int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
 	struct optee_msg_arg *msg_arg;
 	struct optee_session *sess;
 	struct tee_shm *shm;
+	bool system_thread;
 	u_int offs;
 	int rc;
 
 	/* Check that the session is valid */
 	mutex_lock(&ctxdata->mutex);
 	sess = find_session(ctxdata, arg->session);
+	if (sess)
+		system_thread = sess->use_sys_thread;
 	mutex_unlock(&ctxdata->mutex);
 	if (!sess)
 		return -EINVAL;
@@ -432,7 +439,7 @@  int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
 	if (rc)
 		goto out;
 
-	if (optee->ops->do_call_with_arg(ctx, shm, offs)) {
+	if (optee->ops->do_call_with_arg(ctx, shm, offs, system_thread)) {
 		msg_arg->ret = TEEC_ERROR_COMMUNICATION;
 		msg_arg->ret_origin = TEEC_ORIGIN_COMMS;
 	}
@@ -457,12 +464,15 @@  int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session)
 	struct optee_shm_arg_entry *entry;
 	struct optee_msg_arg *msg_arg;
 	struct optee_session *sess;
+	bool system_thread;
 	struct tee_shm *shm;
 	u_int offs;
 
 	/* Check that the session is valid */
 	mutex_lock(&ctxdata->mutex);
 	sess = find_session(ctxdata, session);
+	if (sess)
+		system_thread = sess->use_sys_thread;
 	mutex_unlock(&ctxdata->mutex);
 	if (!sess)
 		return -EINVAL;
@@ -474,7 +484,7 @@  int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session)
 	msg_arg->cmd = OPTEE_MSG_CMD_CANCEL;
 	msg_arg->session = session;
 	msg_arg->cancel_id = cancel_id;
-	optee->ops->do_call_with_arg(ctx, shm, offs);
+	optee->ops->do_call_with_arg(ctx, shm, offs, system_thread);
 
 	optee_free_msg_arg(ctx, entry, offs);
 	return 0;
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 2a258bd3b6b5..d01ca47f7bde 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -129,7 +129,8 @@  int optee_open(struct tee_context *ctx, bool cap_memref_null)
 
 static void optee_release_helper(struct tee_context *ctx,
 				 int (*close_session)(struct tee_context *ctx,
-						      u32 session))
+						      u32 session,
+						      bool system_thread))
 {
 	struct optee_context_data *ctxdata = ctx->data;
 	struct optee_session *sess;
@@ -141,7 +142,7 @@  static void optee_release_helper(struct tee_context *ctx,
 	list_for_each_entry_safe(sess, sess_tmp, &ctxdata->sess_list,
 				 list_node) {
 		list_del(&sess->list_node);
-		close_session(ctx, sess->session_id);
+		close_session(ctx, sess->session_id, sess->use_sys_thread);
 		kfree(sess);
 	}
 	kfree(ctxdata);
diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
index 0828240f27e6..52cec9d06041 100644
--- a/drivers/tee/optee/ffa_abi.c
+++ b/drivers/tee/optee/ffa_abi.c
@@ -612,7 +612,8 @@  static int optee_ffa_yielding_call(struct tee_context *ctx,
  */
 
 static int optee_ffa_do_call_with_arg(struct tee_context *ctx,
-				      struct tee_shm *shm, u_int offs)
+				      struct tee_shm *shm, u_int offs,
+				      bool system_thread)
 {
 	struct ffa_send_direct_data data = {
 		.data0 = OPTEE_FFA_YIELDING_CALL_WITH_ARG,
diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
index 72685ee0d53f..3da7960ab34a 100644
--- a/drivers/tee/optee/optee_private.h
+++ b/drivers/tee/optee/optee_private.h
@@ -154,7 +154,8 @@  struct optee;
  */
 struct optee_ops {
 	int (*do_call_with_arg)(struct tee_context *ctx,
-				struct tee_shm *shm_arg, u_int offs);
+				struct tee_shm *shm_arg, u_int offs,
+				bool system_thread);
 	int (*to_msg_param)(struct optee *optee,
 			    struct optee_msg_param *msg_params,
 			    size_t num_params, const struct tee_param *params);
@@ -204,6 +205,7 @@  struct optee {
 struct optee_session {
 	struct list_head list_node;
 	u32 session_id;
+	bool use_sys_thread;
 };
 
 struct optee_context_data {
@@ -252,7 +254,8 @@  int optee_supp_send(struct tee_context *ctx, u32 ret, u32 num_params,
 int optee_open_session(struct tee_context *ctx,
 		       struct tee_ioctl_open_session_arg *arg,
 		       struct tee_param *param);
-int optee_close_session_helper(struct tee_context *ctx, u32 session);
+int optee_close_session_helper(struct tee_context *ctx, u32 session,
+			       bool system_thread);
 int optee_close_session(struct tee_context *ctx, u32 session);
 int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
 		      struct tee_param *param);
diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
index 49702cb08f4f..56ebbb96ac97 100644
--- a/drivers/tee/optee/smc_abi.c
+++ b/drivers/tee/optee/smc_abi.c
@@ -507,7 +507,7 @@  static int optee_shm_register(struct tee_context *ctx, struct tee_shm *shm,
 	msg_arg->params->u.tmem.buf_ptr = virt_to_phys(pages_list) |
 	  (tee_shm_get_page_offset(shm) & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1));
 
-	if (optee->ops->do_call_with_arg(ctx, shm_arg, 0) ||
+	if (optee->ops->do_call_with_arg(ctx, shm_arg, 0, false) ||
 	    msg_arg->ret != TEEC_SUCCESS)
 		rc = -EINVAL;
 
@@ -550,7 +550,7 @@  static int optee_shm_unregister(struct tee_context *ctx, struct tee_shm *shm)
 	msg_arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_RMEM_INPUT;
 	msg_arg->params[0].u.rmem.shm_ref = (unsigned long)shm;
 
-	if (optee->ops->do_call_with_arg(ctx, shm_arg, 0) ||
+	if (optee->ops->do_call_with_arg(ctx, shm_arg, 0, false) ||
 	    msg_arg->ret != TEEC_SUCCESS)
 		rc = -EINVAL;
 out:
@@ -885,7 +885,8 @@  static void optee_handle_rpc(struct tee_context *ctx,
  * Returns return code from secure world, 0 is OK
  */
 static int optee_smc_do_call_with_arg(struct tee_context *ctx,
-				      struct tee_shm *shm, u_int offs)
+				      struct tee_shm *shm, u_int offs,
+				      bool system_thread)
 {
 	struct optee *optee = tee_get_drvdata(ctx->teedev);
 	struct optee_call_waiter w;
@@ -977,7 +978,7 @@  static int simple_call_with_arg(struct tee_context *ctx, u32 cmd)
 		return PTR_ERR(msg_arg);
 
 	msg_arg->cmd = cmd;
-	optee_smc_do_call_with_arg(ctx, shm, offs);
+	optee_smc_do_call_with_arg(ctx, shm, offs, false);
 
 	optee_free_msg_arg(ctx, entry, offs);
 	return 0;