linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sumit Garg <sumit.garg@linaro.org>
To: Jens Wiklander <jens.wiklander@linaro.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	OP-TEE TrustedFirmware <op-tee@lists.trustedfirmware.org>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Marc Bonnici <marc.bonnici@arm.com>,
	Jerome Forissier <jerome@forissier.org>,
	Sughosh Ganu <sughosh.ganu@linaro.org>
Subject: Re: [PATCH v3 3/5] optee: refactor driver with internal callbacks
Date: Thu, 29 Jul 2021 12:45:12 +0530	[thread overview]
Message-ID: <CAFA6WYPXMx8juuHPmr2hQG7zf8NDQkyUj5dgUzD4+adYMdUtjw@mail.gmail.com> (raw)
In-Reply-To: <20210729065623.GB3316601@jade>

On Thu, 29 Jul 2021 at 12:26, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> On Wed, Jul 28, 2021 at 03:29:33PM +0530, Sumit Garg wrote:
> > On Thu, 22 Jul 2021 at 17:48, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> > >
> > > The OP-TEE driver is refactored with three internal callbacks replacing
> > > direct calls to optee_from_msg_param(), optee_to_msg_param() and
> > > optee_do_call_with_arg().
> > >
> > > These functions a central to communicating with OP-TEE in secure world
> > > by using the SMC Calling Convention directly.
> > >
> > > This refactoring makes room for using other primitives to communicate
> > > with OP-TEE in secure world while being able to reuse as much as
> > > possible from the present driver.
> > >
> > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > ---
> > >  drivers/tee/optee/call.c          |  86 +++++++++--------
> > >  drivers/tee/optee/core.c          | 148 ++++++++++++++++++++----------
> > >  drivers/tee/optee/optee_private.h |  35 +++++--
> > >  drivers/tee/optee/rpc.c           |  19 ++--
> > >  4 files changed, 182 insertions(+), 106 deletions(-)
> > >
> > > diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c
> > > index 9d8f5a95e42f..00ecd794e59a 100644
> > > --- a/drivers/tee/optee/call.c
> > > +++ b/drivers/tee/optee/call.c
> > > @@ -1,6 +1,6 @@
> > >  // SPDX-License-Identifier: GPL-2.0-only
> > >  /*
> > > - * Copyright (c) 2015, Linaro Limited
> > > + * Copyright (c) 2015-2021, Linaro Limited
> > >   */
> > >  #include <linux/arm-smccc.h>
> > >  #include <linux/device.h>
> > > @@ -118,20 +118,25 @@ static struct optee_session *find_session(struct optee_context_data *ctxdata,
> > >  /**
> > >   * optee_do_call_with_arg() - Do an SMC to OP-TEE in secure world
> > >   * @ctx:       calling context
> > > - * @parg:      physical address of message to pass to secure world
> > > + * @arg:       shared memory holding the message to pass to secure world
> > >   *
> > >   * Does and SMC to OP-TEE in secure world and handles eventual resulting
> > >   * Remote Procedure Calls (RPC) from OP-TEE.
> > >   *
> > >   * Returns return code from secure world, 0 is OK
> > >   */
> > > -u32 optee_do_call_with_arg(struct tee_context *ctx, phys_addr_t parg)
> > > +int optee_do_call_with_arg(struct tee_context *ctx, struct tee_shm *arg)
> > >  {
> > >         struct optee *optee = tee_get_drvdata(ctx->teedev);
> > >         struct optee_call_waiter w;
> > >         struct optee_rpc_param param = { };
> > >         struct optee_call_ctx call_ctx = { };
> > > -       u32 ret;
> > > +       phys_addr_t parg;
> > > +       int rc;
> > > +
> > > +       rc = tee_shm_get_pa(arg, 0, &parg);
> > > +       if (rc)
> > > +               return rc;
> > >
> > >         param.a0 = OPTEE_SMC_CALL_WITH_ARG;
> > >         reg_pair_from_64(&param.a1, &param.a2, parg);
> > > @@ -160,7 +165,7 @@ u32 optee_do_call_with_arg(struct tee_context *ctx, phys_addr_t parg)
> > >                         param.a3 = res.a3;
> > >                         optee_handle_rpc(ctx, &param, &call_ctx);
> > >                 } else {
> > > -                       ret = res.a0;
> > > +                       rc = res.a0;
> > >                         break;
> > >                 }
> > >         }
> > > @@ -172,14 +177,12 @@ u32 optee_do_call_with_arg(struct tee_context *ctx, phys_addr_t parg)
> > >          */
> > >         optee_cq_wait_final(&optee->call_queue, &w);
> > >
> > > -       return ret;
> > > +       return rc;
> > >  }
> > >
> > >  static struct tee_shm *get_msg_arg(struct tee_context *ctx, size_t num_params,
> > > -                                  struct optee_msg_arg **msg_arg,
> > > -                                  phys_addr_t *msg_parg)
> > > +                                  struct optee_msg_arg **msg_arg)
> > >  {
> > > -       int rc;
> > >         struct tee_shm *shm;
> > >         struct optee_msg_arg *ma;
> > >
> > > @@ -190,22 +193,13 @@ static struct tee_shm *get_msg_arg(struct tee_context *ctx, size_t num_params,
> > >
> > >         ma = tee_shm_get_va(shm, 0);
> > >         if (IS_ERR(ma)) {
> > > -               rc = PTR_ERR(ma);
> > > -               goto out;
> > > +               tee_shm_free(shm);
> > > +               return (void *)ma;
> > >         }
> > >
> > > -       rc = tee_shm_get_pa(shm, 0, msg_parg);
> > > -       if (rc)
> > > -               goto out;
> > > -
> > >         memset(ma, 0, OPTEE_MSG_GET_ARG_SIZE(num_params));
> > >         ma->num_params = num_params;
> > >         *msg_arg = ma;
> > > -out:
> > > -       if (rc) {
> > > -               tee_shm_free(shm);
> > > -               return ERR_PTR(rc);
> > > -       }
> > >
> > >         return shm;
> > >  }
> > > @@ -214,16 +208,16 @@ int optee_open_session(struct tee_context *ctx,
> > >                        struct tee_ioctl_open_session_arg *arg,
> > >                        struct tee_param *param)
> > >  {
> > > +       struct optee *optee = tee_get_drvdata(ctx->teedev);
> > >         struct optee_context_data *ctxdata = ctx->data;
> > >         int rc;
> > >         struct tee_shm *shm;
> > >         struct optee_msg_arg *msg_arg;
> > > -       phys_addr_t msg_parg;
> > >         struct optee_session *sess = NULL;
> > >         uuid_t client_uuid;
> > >
> > >         /* +2 for the meta parameters added below */
> > > -       shm = get_msg_arg(ctx, arg->num_params + 2, &msg_arg, &msg_parg);
> > > +       shm = get_msg_arg(ctx, arg->num_params + 2, &msg_arg);
> > >         if (IS_ERR(shm))
> > >                 return PTR_ERR(shm);
> > >
> > > @@ -247,7 +241,8 @@ int optee_open_session(struct tee_context *ctx,
> > >                 goto out;
> > >         export_uuid(msg_arg->params[1].u.octets, &client_uuid);
> > >
> > > -       rc = optee_to_msg_param(msg_arg->params + 2, arg->num_params, param);
> > > +       rc = optee->ops->to_msg_param(optee, msg_arg->params + 2,
> > > +                                     arg->num_params, param);
> > >         if (rc)
> > >                 goto out;
> > >
> > > @@ -257,7 +252,7 @@ int optee_open_session(struct tee_context *ctx,
> > >                 goto out;
> > >         }
> > >
> > > -       if (optee_do_call_with_arg(ctx, msg_parg)) {
> > > +       if (optee->ops->do_call_with_arg(ctx, shm)) {
> > >                 msg_arg->ret = TEEC_ERROR_COMMUNICATION;
> > >                 msg_arg->ret_origin = TEEC_ORIGIN_COMMS;
> > >         }
> > > @@ -272,7 +267,8 @@ int optee_open_session(struct tee_context *ctx,
> > >                 kfree(sess);
> > >         }
> > >
> > > -       if (optee_from_msg_param(param, arg->num_params, msg_arg->params + 2)) {
> > > +       if (optee->ops->from_msg_param(optee, param, arg->num_params,
> > > +                                      msg_arg->params + 2)) {
> > >                 arg->ret = TEEC_ERROR_COMMUNICATION;
> > >                 arg->ret_origin = TEEC_ORIGIN_COMMS;
> > >                 /* Close session again to avoid leakage */
> > > @@ -291,16 +287,16 @@ int optee_open_session(struct tee_context *ctx,
> > >  int optee_close_session_helper(struct tee_context *ctx, u32 session)
> > >  {
> > >         struct tee_shm *shm;
> > > +       struct optee *optee = tee_get_drvdata(ctx->teedev);
> > >         struct optee_msg_arg *msg_arg;
> > > -       phys_addr_t msg_parg;
> > >
> > > -       shm = get_msg_arg(ctx, 0, &msg_arg, &msg_parg);
> > > +       shm = get_msg_arg(ctx, 0, &msg_arg);
> > >         if (IS_ERR(shm))
> > >                 return PTR_ERR(shm);
> > >
> > >         msg_arg->cmd = OPTEE_MSG_CMD_CLOSE_SESSION;
> > >         msg_arg->session = session;
> > > -       optee_do_call_with_arg(ctx, msg_parg);
> > > +       optee->ops->do_call_with_arg(ctx, shm);
> > >
> > >         tee_shm_free(shm);
> > >
> > > @@ -328,10 +324,10 @@ 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)
> > >  {
> > > +       struct optee *optee = tee_get_drvdata(ctx->teedev);
> > >         struct optee_context_data *ctxdata = ctx->data;
> > >         struct tee_shm *shm;
> > >         struct optee_msg_arg *msg_arg;
> > > -       phys_addr_t msg_parg;
> > >         struct optee_session *sess;
> > >         int rc;
> > >
> > > @@ -342,7 +338,7 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> > >         if (!sess)
> > >                 return -EINVAL;
> > >
> > > -       shm = get_msg_arg(ctx, arg->num_params, &msg_arg, &msg_parg);
> > > +       shm = get_msg_arg(ctx, arg->num_params, &msg_arg);
> > >         if (IS_ERR(shm))
> > >                 return PTR_ERR(shm);
> > >         msg_arg->cmd = OPTEE_MSG_CMD_INVOKE_COMMAND;
> > > @@ -350,16 +346,18 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> > >         msg_arg->session = arg->session;
> > >         msg_arg->cancel_id = arg->cancel_id;
> > >
> > > -       rc = optee_to_msg_param(msg_arg->params, arg->num_params, param);
> > > +       rc = optee->ops->to_msg_param(optee, msg_arg->params, arg->num_params,
> > > +                                     param);
> > >         if (rc)
> > >                 goto out;
> > >
> > > -       if (optee_do_call_with_arg(ctx, msg_parg)) {
> > > +       if (optee->ops->do_call_with_arg(ctx, shm)) {
> > >                 msg_arg->ret = TEEC_ERROR_COMMUNICATION;
> > >                 msg_arg->ret_origin = TEEC_ORIGIN_COMMS;
> > >         }
> > >
> > > -       if (optee_from_msg_param(param, arg->num_params, msg_arg->params)) {
> > > +       if (optee->ops->from_msg_param(optee, param, arg->num_params,
> > > +                                      msg_arg->params)) {
> > >                 msg_arg->ret = TEEC_ERROR_COMMUNICATION;
> > >                 msg_arg->ret_origin = TEEC_ORIGIN_COMMS;
> > >         }
> > > @@ -373,10 +371,10 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> > >
> > >  int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session)
> > >  {
> > > +       struct optee *optee = tee_get_drvdata(ctx->teedev);
> > >         struct optee_context_data *ctxdata = ctx->data;
> > >         struct tee_shm *shm;
> > >         struct optee_msg_arg *msg_arg;
> > > -       phys_addr_t msg_parg;
> > >         struct optee_session *sess;
> > >
> > >         /* Check that the session is valid */
> > > @@ -386,14 +384,14 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session)
> > >         if (!sess)
> > >                 return -EINVAL;
> > >
> > > -       shm = get_msg_arg(ctx, 0, &msg_arg, &msg_parg);
> > > +       shm = get_msg_arg(ctx, 0, &msg_arg);
> > >         if (IS_ERR(shm))
> > >                 return PTR_ERR(shm);
> > >
> > >         msg_arg->cmd = OPTEE_MSG_CMD_CANCEL;
> > >         msg_arg->session = session;
> > >         msg_arg->cancel_id = cancel_id;
> > > -       optee_do_call_with_arg(ctx, msg_parg);
> > > +       optee->ops->do_call_with_arg(ctx, shm);
> > >
> > >         tee_shm_free(shm);
> > >         return 0;
> > > @@ -592,10 +590,10 @@ int optee_shm_register(struct tee_context *ctx, struct tee_shm *shm,
> > >                        struct page **pages, size_t num_pages,
> > >                        unsigned long start)
> > >  {
> > > -       struct tee_shm *shm_arg = NULL;
> > > +       struct optee *optee = tee_get_drvdata(ctx->teedev);
> > >         struct optee_msg_arg *msg_arg;
> > > +       struct tee_shm *shm_arg;
> > >         u64 *pages_list;
> > > -       phys_addr_t msg_parg;
> > >         int rc;
> > >
> > >         if (!num_pages)
> > > @@ -609,7 +607,7 @@ int optee_shm_register(struct tee_context *ctx, struct tee_shm *shm,
> > >         if (!pages_list)
> > >                 return -ENOMEM;
> > >
> > > -       shm_arg = get_msg_arg(ctx, 1, &msg_arg, &msg_parg);
> > > +       shm_arg = get_msg_arg(ctx, 1, &msg_arg);
> > >         if (IS_ERR(shm_arg)) {
> > >                 rc = PTR_ERR(shm_arg);
> > >                 goto out;
> > > @@ -630,7 +628,7 @@ 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_do_call_with_arg(ctx, msg_parg) ||
> > > +       if (optee->ops->do_call_with_arg(ctx, shm_arg) ||
> > >             msg_arg->ret != TEEC_SUCCESS)
> > >                 rc = -EINVAL;
> > >
> > > @@ -642,12 +640,12 @@ int optee_shm_register(struct tee_context *ctx, struct tee_shm *shm,
> > >
> > >  int optee_shm_unregister(struct tee_context *ctx, struct tee_shm *shm)
> > >  {
> > > -       struct tee_shm *shm_arg;
> > > +       struct optee *optee = tee_get_drvdata(ctx->teedev);
> > >         struct optee_msg_arg *msg_arg;
> > > -       phys_addr_t msg_parg;
> > > +       struct tee_shm *shm_arg;
> > >         int rc = 0;
> > >
> > > -       shm_arg = get_msg_arg(ctx, 1, &msg_arg, &msg_parg);
> > > +       shm_arg = get_msg_arg(ctx, 1, &msg_arg);
> > >         if (IS_ERR(shm_arg))
> > >                 return PTR_ERR(shm_arg);
> > >
> > > @@ -656,7 +654,7 @@ 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_do_call_with_arg(ctx, msg_parg) ||
> > > +       if (optee->ops->do_call_with_arg(ctx, shm_arg) ||
> > >             msg_arg->ret != TEEC_SUCCESS)
> > >                 rc = -EINVAL;
> > >         tee_shm_free(shm_arg);
> > > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> > > index 949223b214c3..f689f171a794 100644
> > > --- a/drivers/tee/optee/core.c
> > > +++ b/drivers/tee/optee/core.c
> > > @@ -1,6 +1,6 @@
> > >  // SPDX-License-Identifier: GPL-2.0-only
> > >  /*
> > > - * Copyright (c) 2015, Linaro Limited
> > > + * Copyright (c) 2015-2021, Linaro Limited
> > >   */
> > >
> > >  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > @@ -26,21 +26,87 @@
> > >
> > >  #define OPTEE_SHM_NUM_PRIV_PAGES       CONFIG_OPTEE_SHM_NUM_PRIV_PAGES
> > >
> > > +static void from_msg_param_value(struct tee_param *p, u32 attr,
> > > +                                const struct optee_msg_param *mp)
> > > +{
> > > +       p->attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT +
> > > +                 attr - OPTEE_MSG_ATTR_TYPE_VALUE_INPUT;
> > > +       p->u.value.a = mp->u.value.a;
> > > +       p->u.value.b = mp->u.value.b;
> > > +       p->u.value.c = mp->u.value.c;
> > > +}
> > > +
> > > +static int from_msg_param_tmp_mem(struct tee_param *p, u32 attr,
> > > +                                 const struct optee_msg_param *mp)
> > > +{
> > > +       struct tee_shm *shm;
> > > +       phys_addr_t pa;
> > > +       int rc;
> > > +
> > > +       p->attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT +
> > > +                 attr - OPTEE_MSG_ATTR_TYPE_TMEM_INPUT;
> > > +       p->u.memref.size = mp->u.tmem.size;
> > > +       shm = (struct tee_shm *)(unsigned long)mp->u.tmem.shm_ref;
> > > +       if (!shm) {
> > > +               p->u.memref.shm_offs = 0;
> > > +               p->u.memref.shm = NULL;
> > > +               return 0;
> > > +       }
> > > +
> > > +       rc = tee_shm_get_pa(shm, 0, &pa);
> > > +       if (rc)
> > > +               return rc;
> > > +
> > > +       p->u.memref.shm_offs = mp->u.tmem.buf_ptr - pa;
> > > +       p->u.memref.shm = shm;
> > > +
> > > +       /* Check that the memref is covered by the shm object */
> > > +       if (p->u.memref.size) {
> > > +               size_t o = p->u.memref.shm_offs +
> > > +                          p->u.memref.size - 1;
> > > +
> > > +               rc = tee_shm_get_pa(shm, o, NULL);
> > > +               if (rc)
> > > +                       return rc;
> > > +       }
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static void from_msg_param_reg_mem(struct tee_param *p, u32 attr,
> > > +                                  const struct optee_msg_param *mp)
> > > +{
> > > +       struct tee_shm *shm;
> > > +
> > > +       p->attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT +
> > > +                 attr - OPTEE_MSG_ATTR_TYPE_RMEM_INPUT;
> > > +       p->u.memref.size = mp->u.rmem.size;
> > > +       shm = (struct tee_shm *)(unsigned long)mp->u.rmem.shm_ref;
> > > +
> > > +       if (shm) {
> > > +               p->u.memref.shm_offs = mp->u.rmem.offs;
> > > +               p->u.memref.shm = shm;
> > > +       } else {
> > > +               p->u.memref.shm_offs = 0;
> > > +               p->u.memref.shm = NULL;
> > > +       }
> > > +}
> > > +
> > >  /**
> > >   * optee_from_msg_param() - convert from OPTEE_MSG parameters to
> > >   *                         struct tee_param
> > > + * @optee:     main service struct
> >
> > I don't see this new argument being used. Can you throw some light on
> > the use-case for this additional argument?
>
> This function is registered as a callback and the counter part in the
> FF-A ABI (optee_ffa_from_msg_param()) will need this parameter. So it's
> needed to have the same signature.
>

Okay, I see its usage.

> >
> > >   * @params:    subsystem internal parameter representation
> > >   * @num_params:        number of elements in the parameter arrays
> > >   * @msg_params:        OPTEE_MSG parameters
> > >   * Returns 0 on success or <0 on failure
> > >   */
> > > -int optee_from_msg_param(struct tee_param *params, size_t num_params,
> > > -                        const struct optee_msg_param *msg_params)
> > > +static int optee_from_msg_param(struct optee *optee, struct tee_param *params,
> > > +                               size_t num_params,
> > > +                               const struct optee_msg_param *msg_params)
> > >  {
> > >         int rc;
> > >         size_t n;
> > > -       struct tee_shm *shm;
> > > -       phys_addr_t pa;
> > >
> > >         for (n = 0; n < num_params; n++) {
> > >                 struct tee_param *p = params + n;
> > > @@ -55,48 +121,19 @@ int optee_from_msg_param(struct tee_param *params, size_t num_params,
> > >                 case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT:
> > >                 case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT:
> > >                 case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT:
> > > -                       p->attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT +
> > > -                                 attr - OPTEE_MSG_ATTR_TYPE_VALUE_INPUT;
> > > -                       p->u.value.a = mp->u.value.a;
> > > -                       p->u.value.b = mp->u.value.b;
> > > -                       p->u.value.c = mp->u.value.c;
> > > +                       from_msg_param_value(p, attr, mp);
> > >                         break;
> > >                 case OPTEE_MSG_ATTR_TYPE_TMEM_INPUT:
> > >                 case OPTEE_MSG_ATTR_TYPE_TMEM_OUTPUT:
> > >                 case OPTEE_MSG_ATTR_TYPE_TMEM_INOUT:
> > > -                       p->attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT +
> > > -                                 attr - OPTEE_MSG_ATTR_TYPE_TMEM_INPUT;
> > > -                       p->u.memref.size = mp->u.tmem.size;
> > > -                       shm = (struct tee_shm *)(unsigned long)
> > > -                               mp->u.tmem.shm_ref;
> > > -                       if (!shm) {
> > > -                               p->u.memref.shm_offs = 0;
> > > -                               p->u.memref.shm = NULL;
> > > -                               break;
> > > -                       }
> > > -                       rc = tee_shm_get_pa(shm, 0, &pa);
> > > +                       rc = from_msg_param_tmp_mem(p, attr, mp);
> > >                         if (rc)
> > >                                 return rc;
> > > -                       p->u.memref.shm_offs = mp->u.tmem.buf_ptr - pa;
> > > -                       p->u.memref.shm = shm;
> > >                         break;
> > >                 case OPTEE_MSG_ATTR_TYPE_RMEM_INPUT:
> > >                 case OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT:
> > >                 case OPTEE_MSG_ATTR_TYPE_RMEM_INOUT:
> > > -                       p->attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT +
> > > -                                 attr - OPTEE_MSG_ATTR_TYPE_RMEM_INPUT;
> > > -                       p->u.memref.size = mp->u.rmem.size;
> > > -                       shm = (struct tee_shm *)(unsigned long)
> > > -                               mp->u.rmem.shm_ref;
> > > -
> > > -                       if (!shm) {
> > > -                               p->u.memref.shm_offs = 0;
> > > -                               p->u.memref.shm = NULL;
> > > -                               break;
> > > -                       }
> > > -                       p->u.memref.shm_offs = mp->u.rmem.offs;
> > > -                       p->u.memref.shm = shm;
> > > -
> > > +                       from_msg_param_reg_mem(p, attr, mp);
> > >                         break;
> > >
> > >                 default:
> > > @@ -106,6 +143,16 @@ int optee_from_msg_param(struct tee_param *params, size_t num_params,
> > >         return 0;
> > >  }
> > >
> > > +static void to_msg_param_value(struct optee_msg_param *mp,
> > > +                              const struct tee_param *p)
> > > +{
> > > +       mp->attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT + p->attr -
> > > +                  TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT;
> > > +       mp->u.value.a = p->u.value.a;
> > > +       mp->u.value.b = p->u.value.b;
> > > +       mp->u.value.c = p->u.value.c;
> > > +}
> > > +
> > >  static int to_msg_param_tmp_mem(struct optee_msg_param *mp,
> > >                                 const struct tee_param *p)
> > >  {
> > > @@ -148,13 +195,15 @@ static int to_msg_param_reg_mem(struct optee_msg_param *mp,
> > >
> > >  /**
> > >   * optee_to_msg_param() - convert from struct tee_params to OPTEE_MSG parameters
> > > + * @optee:     main service struct
> >
> > Same here.
>
> This function is also registered as a callback.  Here it's questionable
> though, but I added it to let optee->ops->to_msg_param() and
> optee->ops->from_msg_param() have better symmetry.
>

I am fine with that. FWIW:

Reviewed-by: Sumit Garg <sumit.garg@linaro.org>

-Sumit

> Thanks,
> Jens

  reply	other threads:[~2021-07-29  7:15 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-22 12:17 [PATCH v3 0/5] Add FF-A support in OP-TEE driver Jens Wiklander
2021-07-22 12:17 ` [PATCH v3 1/5] tee: add sec_world_id to struct tee_shm Jens Wiklander
2021-07-28  8:41   ` Sumit Garg
2021-07-22 12:17 ` [PATCH v3 2/5] optee: simplify optee_release() Jens Wiklander
2021-07-28  8:45   ` Sumit Garg
2021-07-29  6:43     ` Jens Wiklander
2021-07-29  7:10       ` Sumit Garg
2021-07-22 12:17 ` [PATCH v3 3/5] optee: refactor driver with internal callbacks Jens Wiklander
2021-07-28  9:59   ` Sumit Garg
2021-07-29  6:56     ` Jens Wiklander
2021-07-29  7:15       ` Sumit Garg [this message]
2021-07-22 12:17 ` [PATCH v3 4/5] optee: isolate smc abi Jens Wiklander
2021-07-28 10:10   ` Sumit Garg
2021-07-29  7:10     ` Jens Wiklander
2021-07-22 12:17 ` [PATCH v3 5/5] optee: add FF-A support Jens Wiklander
2021-07-26 10:29 ` [PATCH v3 0/5] Add FF-A support in OP-TEE driver Sumit Garg
2021-07-26 10:54   ` Sudeep Holla
2021-07-26 11:41     ` Sumit Garg
2021-07-26 15:25       ` Jens Wiklander
2021-07-27  6:13         ` Sumit Garg
2021-07-28  6:40           ` Jens Wiklander
2021-07-28 10:22             ` Sumit Garg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFA6WYPXMx8juuHPmr2hQG7zf8NDQkyUj5dgUzD4+adYMdUtjw@mail.gmail.com \
    --to=sumit.garg@linaro.org \
    --cc=jens.wiklander@linaro.org \
    --cc=jerome@forissier.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.bonnici@arm.com \
    --cc=op-tee@lists.trustedfirmware.org \
    --cc=sudeep.holla@arm.com \
    --cc=sughosh.ganu@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).