From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jens Wiklander Date: Fri, 31 Aug 2018 09:02:45 +0200 Subject: [U-Boot] [PATCH v2 07/15] tee: add OP-TEE driver In-Reply-To: References: <20180823104334.16083-1-jens.wiklander@linaro.org> <20180823104334.16083-8-jens.wiklander@linaro.org> Message-ID: <20180831070242.GC23663@jax.urgonet> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Wed, Aug 29, 2018 at 06:28:51PM -0600, Simon Glass wrote: > Hi Jens, > > On 23 August 2018 at 04:43, Jens Wiklander 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 > > Signed-off-by: Jens Wiklander > > --- > > 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 > > > 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 > > +#include > > +#include > > +#include > > +#include > > +#include > > 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 > > +#include > > + > > +/* > > + * 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