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

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@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@linaro.org Signed-off-by: Jens Wiklander jens.wiklander@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@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