[U-Boot] [PATCH v2 07/15] tee: add OP-TEE driver

Jens Wiklander jens.wiklander at linaro.org
Fri Aug 31 07:02:45 UTC 2018


On Wed, Aug 29, 2018 at 06:28:51PM -0600, Simon Glass wrote:
> Hi Jens,
> 
> On 23 August 2018 at 04:43, Jens Wiklander <jens.wiklander at linaro.org> wrote:
> > Adds a OP-TEE driver.
> >
> > * Targets ARM and ARM64
> > * Supports using any u-boot memory as shared memory
> 
> U-Boot
> 
> Please use this consistent.

Sorry, it seems I've found lots of ways of doing that wrong.

> 
> > * Probes OP-TEE version using SMCs
> > * Uses OPTEE message protocol version 2 to communicate with secure world
> >
> > Tested-by: Igor Opaniuk <igor.opaniuk at linaro.org>
> > Signed-off-by: Jens Wiklander <jens.wiklander at linaro.org>
> > ---
> >  drivers/tee/Kconfig                      |  10 +
> >  drivers/tee/Makefile                     |   1 +
> >  drivers/tee/optee/Kconfig                |   7 +
> >  drivers/tee/optee/Makefile               |   4 +
> >  drivers/tee/optee/core.c                 | 614 +++++++++++++++++++++++
> >  drivers/tee/optee/optee_msg.h            | 423 ++++++++++++++++
> >  drivers/tee/optee/optee_msg_supplicant.h | 234 +++++++++
> >  drivers/tee/optee/optee_private.h        |  12 +
> >  drivers/tee/optee/optee_smc.h            | 444 ++++++++++++++++
> >  drivers/tee/optee/supplicant.c           |  89 ++++
> >  10 files changed, 1838 insertions(+)
> >  create mode 100644 drivers/tee/optee/Kconfig
> >  create mode 100644 drivers/tee/optee/Makefile
> >  create mode 100644 drivers/tee/optee/core.c
> >  create mode 100644 drivers/tee/optee/optee_msg.h
> >  create mode 100644 drivers/tee/optee/optee_msg_supplicant.h
> >  create mode 100644 drivers/tee/optee/optee_private.h
> >  create mode 100644 drivers/tee/optee/optee_smc.h
> >  create mode 100644 drivers/tee/optee/supplicant.c
> >
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> > diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
> > index 817ab331b0f8..3e7fe6ddcc5d 100644
> > --- a/drivers/tee/Kconfig
> > +++ b/drivers/tee/Kconfig
> > @@ -6,3 +6,13 @@ config TEE
> >         help
> >           This implements a generic interface towards a Trusted Execution
> >           Environment (TEE).
> > +
> > +if TEE
> > +
> > +menu "TEE drivers"
> > +
> > +source "drivers/tee/optee/Kconfig"
> > +
> > +endmenu
> > +
> > +endif
> > diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile
> > index b6d8e16e6211..19633b60f235 100644
> > --- a/drivers/tee/Makefile
> > +++ b/drivers/tee/Makefile
> > @@ -1,3 +1,4 @@
> >  # SPDX-License-Identifier: GPL-2.0+
> >
> >  obj-y += tee-uclass.o
> > +obj-$(CONFIG_OPTEE) += optee/
> > diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig
> > new file mode 100644
> > index 000000000000..8f7ebe161111
> > --- /dev/null
> > +++ b/drivers/tee/optee/Kconfig
> > @@ -0,0 +1,7 @@
> > +# OP-TEE Trusted Execution Environment Configuration
> > +config OPTEE
> > +       bool "OP-TEE"
> > +       depends on ARM_SMCCC
> > +       help
> > +         This implements the OP-TEE Trusted Execution Environment (TEE)
> > +         driver.
> 
> for ARM? I think you should expand this help. What does the driver
> support / do?

I'll expand this.

> > diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> > new file mode 100644
> > index 000000000000..6148feb474a5
> > --- /dev/null
> > +++ b/drivers/tee/optee/Makefile
> > @@ -0,0 +1,4 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +
> > +obj-y += core.o
> > +obj-y += supplicant.o
> > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> > new file mode 100644
> > index 000000000000..f2d92d96551b
> > --- /dev/null
> > +++ b/drivers/tee/optee/core.c
> > @@ -0,0 +1,614 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2018 Linaro Limited
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <linux/arm-smccc.h>
> > +#include <linux/io.h>
> > +#include <log.h>
> > +#include <tee.h>
> 
> Put linux/ ones after tee.h

OK

> > +
> > +#include "optee_smc.h"
> > +#include "optee_msg.h"
> > +#include "optee_private.h"
> > +
> > +#define PAGELIST_ENTRIES_PER_PAGE \
> > +       ((OPTEE_MSG_NONCONTIG_PAGE_SIZE / sizeof(u64)) - 1)
> > +
> > +typedef void (optee_invoke_fn)(unsigned long, unsigned long, unsigned long,
> > +                              unsigned long, unsigned long, unsigned long,
> > +                              unsigned long, unsigned long,
> > +                              struct arm_smccc_res *);
> > +
> > +struct optee_pdata {
> > +       optee_invoke_fn *invoke_fn;
> > +};
> > +
> > +struct rpc_param {
> > +       u32     a0;
> > +       u32     a1;
> > +       u32     a2;
> > +       u32     a3;
> > +       u32     a4;
> > +       u32     a5;
> > +       u32     a6;
> > +       u32     a7;
> > +};
> > +
> > +static void *reg_pair_to_ptr(u32 reg0, u32 reg1)
> > +{
> > +       return (void *)(ulong)(((u64)reg0 << 32) | reg1);
> 
> Can you not remove the u64 here?
> 
> E.g.
> 
> (void *)((ulong)reg0 << 32) | reg1)

No, that will cause:
drivers/tee/optee/core.c:42:38: warning: left shift count >= width of type [-Wshift-count-overflow]
  return (void *)(ulong)(((ulong)reg0 << 32) | reg1);
                                      ^~
on ILP32

> 
> > +}
> > +
> > +static void reg_pair_from_64(u32 *reg0, u32 *reg1, u64 val)
> 
> Function comment
> 
> > +{
> > +       *reg0 = val >> 32;
> > +       *reg1 = val;
> > +}
> > +
> > +void *optee_alloc_and_init_page_list(void *buf, ulong len, u64 *phys_buf_ptr)
> 
> Function comment
> 
> > +{
> > +       const unsigned int page_size = OPTEE_MSG_NONCONTIG_PAGE_SIZE;
> > +       const phys_addr_t page_mask = page_size - 1;
> > +       u8 *buf_base;
> > +       unsigned int page_offset;
> > +       unsigned int num_pages;
> > +       unsigned int list_size;
> > +       unsigned int n;
> > +       void *page_list;
> > +       struct {
> > +               u64 pages_list[PAGELIST_ENTRIES_PER_PAGE];
> > +               u64 next_page_data;
> > +       } *pages_data;
> > +
> > +       page_offset = (ulong)buf & page_mask;
> > +       num_pages = roundup(page_offset + len, page_size) / page_size;
> > +       list_size = DIV_ROUND_UP(num_pages, PAGELIST_ENTRIES_PER_PAGE) *
> > +                   page_size;
> > +       page_list = memalign(page_size, list_size);
> > +       if (!page_list)
> > +               return NULL;
> > +
> > +       pages_data = page_list;
> > +       buf_base = (u8 *)(rounddown((ulong)buf, page_size));
> 
> Drop extra pair of ()
> 
> > +       n = 0;
> > +       while (num_pages) {
> > +               pages_data->pages_list[n] = virt_to_phys(buf_base);
> > +               n++;
> > +               buf_base += page_size;
> > +               num_pages--;
> > +
> > +               if (n == PAGELIST_ENTRIES_PER_PAGE) {
> > +                       pages_data->next_page_data =
> > +                               virt_to_phys(pages_data + 1);
> > +                       pages_data++;
> > +                       n = 0;
> > +               }
> > +       }
> > +
> > +       *phys_buf_ptr = virt_to_phys(page_list) | page_offset;
> > +       return page_list;
> > +}
> > +
> > +static void optee_get_version(struct udevice *dev,
> > +                             struct tee_version_data *vers)
> > +{
> > +       struct tee_version_data v = {
> > +               .gen_caps = TEE_GEN_CAP_GP | TEE_GEN_CAP_REG_MEM,
> > +       };
> > +
> > +       *vers = v;
> > +}
> > +
> > +static struct tee_shm *get_msg_arg(struct udevice *dev, ulong num_params,
> > +                                  struct optee_msg_arg **msg_arg)
> > +{
> > +       struct tee_shm *shm;
> > +       struct optee_msg_arg *ma;
> > +
> > +       shm = __tee_shm_add(dev, OPTEE_MSG_NONCONTIG_PAGE_SIZE, NULL,
> > +                           OPTEE_MSG_GET_ARG_SIZE(num_params), TEE_SHM_ALLOC);
> > +       if (!shm)
> > +               return NULL;
> > +
> > +       ma = shm->addr;
> > +       memset(ma, 0, OPTEE_MSG_GET_ARG_SIZE(num_params));
> > +       ma->num_params = num_params;
> > +       *msg_arg = ma;
> > +
> > +       return shm;
> > +}
> > +
> > +static int to_msg_param(struct optee_msg_param *msg_params, ulong num_params,
> > +                       const struct tee_param *params)
> > +{
> > +       ulong n;
> 
> uint? int?
> 
> Save below

OK, I'll change the type for holding number of parameters to uint
here and everywhere else.

> 
> > +
> > +       for (n = 0; n < num_params; n++) {
> > +               const struct tee_param *p = params + n;
> > +               struct optee_msg_param *mp = msg_params + n;
> > +
> > +               switch (p->attr) {
> > +               case TEE_PARAM_ATTR_TYPE_NONE:
> > +                       mp->attr = OPTEE_MSG_ATTR_TYPE_NONE;
> > +                       memset(&mp->u, 0, sizeof(mp->u));
> > +                       break;
> > +               case TEE_PARAM_ATTR_TYPE_VALUE_INPUT:
> > +               case TEE_PARAM_ATTR_TYPE_VALUE_OUTPUT:
> > +               case TEE_PARAM_ATTR_TYPE_VALUE_INOUT:
> > +                       mp->attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT + p->attr -
> > +                                  TEE_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;
> > +                       break;
> > +               case TEE_PARAM_ATTR_TYPE_MEMREF_INPUT:
> > +               case TEE_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
> > +               case TEE_PARAM_ATTR_TYPE_MEMREF_INOUT:
> > +                       mp->attr = OPTEE_MSG_ATTR_TYPE_RMEM_INPUT + p->attr -
> > +                                  TEE_PARAM_ATTR_TYPE_MEMREF_INPUT;
> > +                       mp->u.rmem.shm_ref = (ulong)p->u.memref.shm;
> > +                       mp->u.rmem.size = p->u.memref.size;
> > +                       mp->u.rmem.offs = p->u.memref.shm_offs;
> > +                       break;
> > +               default:
> > +                       return -EINVAL;
> > +               }
> > +       }
> > +       return 0;
> > +}
> > +
> > +static int from_msg_param(struct tee_param *params, ulong num_params,
> > +                         const struct optee_msg_param *msg_params)
> > +{
> > +       ulong n;
> > +       struct tee_shm *shm;
> > +
> > +       for (n = 0; n < num_params; n++) {
> > +               struct tee_param *p = params + n;
> > +               const struct optee_msg_param *mp = msg_params + n;
> > +               u32 attr = mp->attr & OPTEE_MSG_ATTR_TYPE_MASK;
> > +
> > +               switch (attr) {
> > +               case OPTEE_MSG_ATTR_TYPE_NONE:
> > +                       p->attr = TEE_PARAM_ATTR_TYPE_NONE;
> > +                       memset(&p->u, 0, sizeof(p->u));
> > +                       break;
> > +               case OPTEE_MSG_ATTR_TYPE_VALUE_INPUT:
> > +               case OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT:
> > +               case OPTEE_MSG_ATTR_TYPE_VALUE_INOUT:
> > +                       p->attr = TEE_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;
> > +                       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_PARAM_ATTR_TYPE_MEMREF_INPUT + attr -
> > +                                 OPTEE_MSG_ATTR_TYPE_RMEM_INPUT;
> > +                       p->u.memref.size = mp->u.rmem.size;
> > +                       shm = (struct tee_shm *)(ulong)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;
> > +                       break;
> > +               default:
> > +                       return -EINVAL;
> > +               }
> > +       }
> > +       return 0;
> > +}
> > +
> > +static void handle_rpc(struct udevice *dev, struct rpc_param *param,
> > +                      void *page_list)
> > +{
> > +       struct tee_shm *shm;
> > +
> > +       switch (OPTEE_SMC_RETURN_GET_RPC_FUNC(param->a0)) {
> > +       case OPTEE_SMC_RPC_FUNC_ALLOC:
> > +               shm = __tee_shm_add(dev, OPTEE_MSG_NONCONTIG_PAGE_SIZE, NULL,
> > +                                   param->a1,
> > +                                   TEE_SHM_ALLOC | TEE_SHM_REGISTER);
> > +               if (shm) {
> > +                       reg_pair_from_64(&param->a1, &param->a2,
> > +                                        virt_to_phys(shm->addr));
> > +                       /* "cookie" */
> > +                       reg_pair_from_64(&param->a4, &param->a5, (ulong)shm);
> > +               } else {
> > +                       param->a1 = 0;
> > +                       param->a2 = 0;
> > +                       param->a4 = 0;
> > +                       param->a5 = 0;
> > +               }
> > +               break;
> > +       case OPTEE_SMC_RPC_FUNC_FREE:
> > +               shm = reg_pair_to_ptr(param->a1, param->a2);
> > +               tee_shm_free(shm);
> > +               break;
> > +       case OPTEE_SMC_RPC_FUNC_FOREIGN_INTR:
> > +               break;
> > +       case OPTEE_SMC_RPC_FUNC_CMD:
> > +               shm = reg_pair_to_ptr(param->a1, param->a2);
> > +               optee_suppl_cmd(dev, shm, page_list);
> > +               break;
> > +       default:
> > +               break;
> > +       }
> > +
> > +       param->a0 = OPTEE_SMC_CALL_RETURN_FROM_RPC;
> > +}
> > +
> > +static u32 call_err_to_res(u32 call_err)
> > +{
> > +       switch (call_err) {
> > +       case OPTEE_SMC_RETURN_OK:
> > +               return TEE_SUCCESS;
> > +       default:
> > +               return TEE_ERROR_BAD_PARAMETERS;
> > +       }
> > +}
> > +
> > +static u32 do_call_with_arg(struct udevice *dev, struct optee_msg_arg *arg)
> > +{
> > +       struct optee_pdata *pdata = dev_get_platdata(dev);
> > +       struct rpc_param param = { .a0 = OPTEE_SMC_CALL_WITH_ARG };
> > +       void *page_list = NULL;
> > +
> > +       reg_pair_from_64(&param.a1, &param.a2, virt_to_phys(arg));
> > +       while (true) {
> > +               struct arm_smccc_res res;
> > +
> > +               pdata->invoke_fn(param.a0, param.a1, param.a2, param.a3,
> > +                                param.a4, param.a5, param.a6, param.a7, &res);
> > +
> > +               free(page_list);
> > +               page_list = NULL;
> > +
> > +               if (OPTEE_SMC_RETURN_IS_RPC(res.a0)) {
> > +                       param.a0 = res.a0;
> > +                       param.a1 = res.a1;
> > +                       param.a2 = res.a2;
> > +                       param.a3 = res.a3;
> > +                       handle_rpc(dev, &param, &page_list);
> > +               } else {
> > +                       return call_err_to_res(res.a0);
> > +               }
> > +       }
> > +}
> > +
> > +static int optee_close_session(struct udevice *dev, u32 session)
> > +{
> > +       struct tee_shm *shm;
> > +       struct optee_msg_arg *msg_arg;
> > +
> > +       shm = get_msg_arg(dev, 0, &msg_arg);
> > +       if (!shm)
> > +               return -ENOMEM;
> > +
> > +       msg_arg->cmd = OPTEE_MSG_CMD_CLOSE_SESSION;
> > +       msg_arg->session = session;
> > +       do_call_with_arg(dev, msg_arg);
> > +
> > +       tee_shm_free(shm);
> 
> blank line before return
> 
> > +       return 0;
> > +}
> > +
> 
> [..]
> 
> > diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
> > new file mode 100644
> > index 000000000000..ebc16aa8cc31
> > --- /dev/null
> > +++ b/drivers/tee/optee/optee_msg.h
> > @@ -0,0 +1,423 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Copyright (c) 2015-2018, Linaro Limited
> > + */
> > +
> > +#ifndef _OPTEE_MSG_H
> > +#define _OPTEE_MSG_H
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/types.h>
> > +
> > +/*
> > + * This file defines the OP-TEE message protocol used to communicate
> > + * with an instance of OP-TEE running in secure world.
> 
> Does this file have an upstream, or is this specific to U-Boot? If the
> former, please add a reference.

Yes, there's an upstream. I'll update.

> 
> > + *
> > + * This file is divided into three sections.
> > + * 1. Formatting of messages.
> > + * 2. Requests from normal world
> > + * 3. Requests from secure world, Remote Procedure Call (RPC), handled by
> > + *    tee-supplicant.
> > + */
> > +
> 
> [...]

Thanks for the review,
Jens


More information about the U-Boot mailing list