[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(¶m->a1, ¶m->a2,
> > + virt_to_phys(shm->addr));
> > + /* "cookie" */
> > + reg_pair_from_64(¶m->a4, ¶m->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(¶m.a1, ¶m.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, ¶m, &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