Hi Simon,
I have one question inline below.
On Fri, Sep 14, 2018 at 12:53 PM, Simon Glass sjg@chromium.org wrote:
Hi Jens,
On 3 September 2018 at 16:47, Jens Wiklander jens.wiklander@linaro.org wrote:
Adds a sandboxtee driver which emulates a generic TEE with the OP-TEE
sandbox tee
AVB TA.
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org
drivers/tee/Kconfig | 12 +- drivers/tee/Makefile | 1 + drivers/tee/optee/Kconfig | 2 +- drivers/tee/sandbox.c | 299 ++++++++++++++++++++++++++++++++++++++ include/sandboxtee.h | 15 ++ 5 files changed, 326 insertions(+), 3 deletions(-) create mode 100644 drivers/tee/sandbox.c create mode 100644 include/sandboxtee.h
Reviewed-by: Simon Glass sjg@chromium.org
nits below.
diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig index 835c256e9239..4697b80913d6 100644 --- a/drivers/tee/Kconfig +++ b/drivers/tee/Kconfig @@ -1,8 +1,8 @@ # Generic Trusted Execution Environment Configuration config TEE bool "Trusted Execution Environment support"
depends on ARM && (ARM64 || CPU_V7A)select ARM_SMCCC
depends on (ARM && (ARM64 || CPU_V7A)) || SANDBOXselect ARM_SMCCC if ARM help This implements a generic interface towards a Trusted Execution Environment (TEE). A TEE is a trusted OS running in some secure@@ -14,6 +14,14 @@ if TEE
menu "TEE drivers"
+config SANDBOX_TEE
bool "Sandbox TEE emulator"depends on SANDBOXdefault yhelpThis emulates a generic TEE needed for testing including the AVBTA.Can you please expand this? What features does it implement? How do you access it from the U-Boot command line?
I'll expand it.
source "drivers/tee/optee/Kconfig"
endmenu diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile index 19633b60f235..f72c68c09f33 100644 --- a/drivers/tee/Makefile +++ b/drivers/tee/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0+
obj-y += tee-uclass.o +obj-$(CONFIG_SANDBOX) += sandbox.o obj-$(CONFIG_OPTEE) += optee/ diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig index dbfa7846a30f..d489834df926 100644 --- a/drivers/tee/optee/Kconfig +++ b/drivers/tee/optee/Kconfig @@ -10,7 +10,7 @@ config OPTEE handle Remote Procedure Calls (RPC) from OP-TEE needed to execute a service. For more information see: https://www.op-tee.org
-if OPTEE +if OPTEE || SANDBOX
menu "OP-TEE options"
diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c new file mode 100644 index 000000000000..cd073497615f --- /dev/null +++ b/drivers/tee/sandbox.c @@ -0,0 +1,299 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2018 Linaro Limited
- */
+#include <common.h> +#include <dm.h> +#include <sandboxtee.h>
Could this go in asm/ ?
I don't understand. What should go into asm/ and which directory is that?
+#include <tee.h> +#include <tee/optee_ta_avb.h>
+/*
- The sandbox tee driver tries to emulate a generic TEE with the TA
- OPTEE_TA_AVB available.
What is TEE?
What is TA?
Please expand out the names once in this comment so people can see immediately what you are referring to.
- */
+struct ta_entry {
What is this struct for? Please add a comment
OK
struct tee_optee_ta_uuid uuid;u32 (*open_session)(uint num_params, struct tee_param *params);u32 (*invoke_func)(u32 func, uint num_params, struct tee_param *params);+};
+#ifdef CONFIG_OPTEE_TA_AVB +static u32 get_attr(uint n, uint num_params, struct tee_param *params) +{
if (n >= num_params)return TEE_PARAM_ATTR_TYPE_NONE;return params[n].attr;+}
+static u32 check_params(u8 p0, u8 p1, u8 p2, u8 p3, uint num_params,
struct tee_param *params)+{
u8 p[] = { p0, p1, p2, p3};uint n;for (n = 0; n < ARRAY_SIZE(p); n++)if (p[n] != get_attr(n, num_params, params))goto bad_params;for (; n < num_params; n++)if (get_attr(n, num_params, params))goto bad_params;return TEE_SUCCESS;+bad_params:
printf("Bad param attrs\n");return TEE_ERROR_BAD_PARAMETERS;+}
+static u64 ta_avb_rollback_indexes[TA_AVB_MAX_ROLLBACK_LOCATIONS]; +static u32 ta_avb_lock_state;
+static u32 ta_avb_open_session(uint num_params, struct tee_param *params) +{
/** We don't expect additional parameters when opening a session to* this TA.*/return check_params(TEE_PARAM_ATTR_TYPE_NONE, TEE_PARAM_ATTR_TYPE_NONE,TEE_PARAM_ATTR_TYPE_NONE, TEE_PARAM_ATTR_TYPE_NONE,num_params, params);+}
+static u32 ta_avb_invoke_func(u32 func, uint num_params,
struct tee_param *params)+{
u32 res;uint slot;u64 val;switch (func) {case TA_AVB_CMD_READ_ROLLBACK_INDEX:res = check_params(TEE_PARAM_ATTR_TYPE_VALUE_INPUT,TEE_PARAM_ATTR_TYPE_VALUE_OUTPUT,TEE_PARAM_ATTR_TYPE_NONE,TEE_PARAM_ATTR_TYPE_NONE,num_params, params);if (res)return res;slot = params[0].u.value.a;if (slot >= ARRAY_SIZE(ta_avb_rollback_indexes)) {printf("Rollback index slot out of bounds %lu\n", slot);return TEE_ERROR_BAD_PARAMETERS;}val = ta_avb_rollback_indexes[slot];params[1].u.value.a = val >> 32;params[1].u.value.b = val;return TEE_SUCCESS;case TA_AVB_CMD_WRITE_ROLLBACK_INDEX:res = check_params(TEE_PARAM_ATTR_TYPE_VALUE_INPUT,TEE_PARAM_ATTR_TYPE_VALUE_INPUT,TEE_PARAM_ATTR_TYPE_NONE,TEE_PARAM_ATTR_TYPE_NONE,num_params, params);if (res)return res;slot = params[0].u.value.a;if (slot >= ARRAY_SIZE(ta_avb_rollback_indexes)) {printf("Rollback index slot out of bounds %lu\n", slot);return TEE_ERROR_BAD_PARAMETERS;}val = (u64)params[1].u.value.a << 32 | params[1].u.value.b;if (val < ta_avb_rollback_indexes[slot])return TEE_ERROR_SECURITY;ta_avb_rollback_indexes[slot] = val;return TEE_SUCCESS;case TA_AVB_CMD_READ_LOCK_STATE:res = check_params(TEE_PARAM_ATTR_TYPE_VALUE_OUTPUT,TEE_PARAM_ATTR_TYPE_NONE,TEE_PARAM_ATTR_TYPE_NONE,TEE_PARAM_ATTR_TYPE_NONE,num_params, params);if (res)return res;params[0].u.value.a = ta_avb_lock_state;return TEE_SUCCESS;case TA_AVB_CMD_WRITE_LOCK_STATE:res = check_params(TEE_PARAM_ATTR_TYPE_VALUE_INPUT,TEE_PARAM_ATTR_TYPE_NONE,TEE_PARAM_ATTR_TYPE_NONE,TEE_PARAM_ATTR_TYPE_NONE,num_params, params);if (res)return res;if (ta_avb_lock_state != params[0].u.value.a) {ta_avb_lock_state = params[0].u.value.a;memset(ta_avb_rollback_indexes, 0,sizeof(ta_avb_rollback_indexes));}return TEE_SUCCESS;default:return TEE_ERROR_NOT_SUPPORTED;}+} +#endif /*OPTEE_TA_AVB*/
+static const struct ta_entry ta_entries[] = { +#ifdef CONFIG_OPTEE_TA_AVB
{ .uuid = TA_AVB_UUID,.open_session = ta_avb_open_session,.invoke_func = ta_avb_invoke_func,},+#endif +};
+static void sandbox_tee_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 int sandbox_tee_close_session(struct udevice *dev, u32 session) +{
struct sandbox_tee_state *state = dev_get_priv(dev);if (!state->ta || state->session != session)return -EINVAL;state->session = 0;state->ta = NULL;return 0;+}
+static const struct ta_entry *find_ta_entry(u8 uuid[TEE_UUID_LEN]) +{
struct tee_optee_ta_uuid u;uint n;tee_optee_ta_uuid_from_octets(&u, uuid);for (n = 0; n < ARRAY_SIZE(ta_entries); n++)if (!memcmp(&u, &ta_entries[n].uuid, sizeof(u)))return ta_entries + n;return NULL;+}
+static int sandbox_tee_open_session(struct udevice *dev,
struct tee_open_session_arg *arg,uint num_params, struct tee_param *params)+{
struct sandbox_tee_state *state = dev_get_priv(dev);const struct ta_entry *ta;if (state->ta) {printf("A session is already open\n");return -EBUSY;}ta = find_ta_entry(arg->uuid);if (!ta) {printf("Cannot find TA\n");arg->ret = TEE_ERROR_ITEM_NOT_FOUND;arg->ret_origin = TEE_ORIGIN_TEE;return 0;}arg->ret = ta->open_session(num_params, params);arg->ret_origin = TEE_ORIGIN_TRUSTED_APP;if (!arg->ret) {state->ta = (void *)ta;state->session = 1;arg->session = state->session;} else {printf("Cannot open session, TA returns error\n");}return 0;+}
+static int sandbox_tee_invoke_func(struct udevice *dev,
struct tee_invoke_arg *arg,uint num_params, struct tee_param *params)+{
struct sandbox_tee_state *state = dev_get_priv(dev);struct ta_entry *ta = state->ta;if (!arg->session) {printf("Missing session\n");return -EINVAL;}if (!ta) {printf("TA session not available\n");return -EINVAL;}if (arg->session != state->session) {printf("Session mismatch\n");return -EINVAL;}arg->ret = ta->invoke_func(arg->func, num_params, params);arg->ret_origin = TEE_ORIGIN_TRUSTED_APP;return 0;+}
+static int sandbox_tee_shm_register(struct udevice *dev, struct tee_shm *shm) +{
struct sandbox_tee_state *state = dev_get_priv(dev);state->num_shms++;return 0;+}
+static int sandbox_tee_shm_unregister(struct udevice *dev, struct tee_shm *shm) +{
struct sandbox_tee_state *state = dev_get_priv(dev);state->num_shms--;return 0;+}
+static const struct tee_driver_ops sandbox_tee_ops = {
.get_version = sandbox_tee_get_version,.open_session = sandbox_tee_open_session,.close_session = sandbox_tee_close_session,.invoke_func = sandbox_tee_invoke_func,.shm_register = sandbox_tee_shm_register,.shm_unregister = sandbox_tee_shm_unregister,+};
+static const struct udevice_id sandbox_tee_match[] = {
{ .compatible = "sandbox,tee" },{},+};
+U_BOOT_DRIVER(sandbox_tee) = {
.name = "sandbox_tee",.id = UCLASS_TEE,.of_match = sandbox_tee_match,.ops = &sandbox_tee_ops,.priv_auto_alloc_size = sizeof(struct sandbox_tee_state),+}; diff --git a/include/sandboxtee.h b/include/sandboxtee.h new file mode 100644 index 000000000000..59cbb621820b --- /dev/null +++ b/include/sandboxtee.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Copyright (C) 2018 Linaro Limited
- */
+#ifndef __SANDBOXTEE_H +#define __SANDBOXTEE_H
+struct sandbox_tee_state {
u32 session;int num_shms;void *ta;struct/members need comments
OK
Thanks, Jens