Hi Jens, On Wed, Jan 20, 2021 at 10:49 AM Jens Wiklander <jens.wiklan...@linaro.org> wrote: > > On Tue, Jan 12, 2021 at 09:43:39AM +0100, Jorge Ramirez-Ortiz wrote: > > From: Igor Opaniuk <igor.opan...@foundries.io> > > > > This adds support for RPC test trusted application emulation, which > > permits to test reverse RPC calls to TEE supplicant. Currently it covers > > requests to the I2C bus from TEE. > > > > Signed-off-by: Igor Opaniuk <igor.opan...@foundries.io> > > --- > > drivers/tee/Makefile | 2 + > > drivers/tee/optee/Kconfig | 9 +++ > > drivers/tee/sandbox.c | 137 +++++++++++++++++++++++++++++++- > > include/tee/optee_ta_rpc_test.h | 28 +++++++ > > 4 files changed, 172 insertions(+), 4 deletions(-) > > create mode 100644 include/tee/optee_ta_rpc_test.h > > > > diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile > > index 5c8ffdbce8..ff844195ae 100644 > > --- a/drivers/tee/Makefile > > +++ b/drivers/tee/Makefile > > @@ -2,5 +2,7 @@ > > > > obj-y += tee-uclass.o > > obj-$(CONFIG_SANDBOX) += sandbox.o > > +obj-$(CONFIG_OPTEE_TA_RPC_TEST) += optee/supplicant.o > > +obj-$(CONFIG_OPTEE_TA_RPC_TEST) += optee/i2c.o > > obj-$(CONFIG_OPTEE) += optee/ > > obj-y += broadcom/ > > diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig > > index d489834df9..65622f30b1 100644 > > --- a/drivers/tee/optee/Kconfig > > +++ b/drivers/tee/optee/Kconfig > > @@ -22,6 +22,15 @@ config OPTEE_TA_AVB > > The TA can support the "avb" subcommands "read_rb", "write"rb" > > and "is_unlocked". > > > > +config OPTEE_TA_RPC_TEST > > + bool "Support RPC TEST TA" > > + depends on SANDBOX_TEE > > + default y > > + help > > + Enables support for RPC test trusted application emulation, which > > + permits to test reverse RPC calls to TEE supplicant. Should > > + be used only in sandbox env. > > + > > endmenu > > > > endif > > diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c > > index 4b91e7db1b..1cacd443f4 100644 > > --- a/drivers/tee/sandbox.c > > +++ b/drivers/tee/sandbox.c > > @@ -7,11 +7,15 @@ > > #include <sandboxtee.h> > > #include <tee.h> > > #include <tee/optee_ta_avb.h> > > +#include <tee/optee_ta_rpc_test.h> > > + > > +#include "optee/optee_msg.h" > > +#include "optee/optee_private.h" > > > > /* > > * The sandbox tee driver tries to emulate a generic Trusted Exectution > > - * Environment (TEE) with the Trusted Application (TA) OPTEE_TA_AVB > > - * available. > > + * Environment (TEE) with the Trusted Applications (TA) OPTEE_TA_AVB and > > + * OPTEE_TA_RPC_TEST available. > > */ > > > > static const u32 pstorage_max = 16; > > @@ -32,7 +36,32 @@ struct ta_entry { > > struct tee_param *params); > > }; > > > > -#ifdef CONFIG_OPTEE_TA_AVB > > +static int get_msg_arg(struct udevice *dev, uint num_params, > > + struct tee_shm **shmp, struct optee_msg_arg **msg_arg) > > +{ > > + int rc; > > + struct optee_msg_arg *ma; > > + > > + rc = __tee_shm_add(dev, OPTEE_MSG_NONCONTIG_PAGE_SIZE, NULL, > > + OPTEE_MSG_GET_ARG_SIZE(num_params), TEE_SHM_ALLOC, > > + shmp); > > + if (rc) > > + return rc; > > + > > + ma = (*shmp)->addr; > > + memset(ma, 0, OPTEE_MSG_GET_ARG_SIZE(num_params)); > > + ma->num_params = num_params; > > + *msg_arg = ma; > > + > > + return 0; > > +} > > + > > +inline void *optee_alloc_and_init_page_list(void *buf, ulong len, > > + u64 *phys_buf_ptr) > > Why "inline"? That should not be there, will fix in new patch series.
> > > +{ > > + return 0; > > How can this work? I'd expect an eventual caller to get a bit > disappointed. By the way aren't we usually using NULL for pointers in > U-Boot? I didn't want to pull-in the full core.c of OP-TEE driver, so I kept a simple stub for optee_alloc_and_init_page_list() (taking into account that currently it's not being called anyware) to avoid compilation issues in supplicant.c . As you already said, if someone decides to extend RPC tests and test OPTEE_MSG_RPC_CMD_SHM_ALLOC etc - we will definitely have a problem here (but why do that?). As a suggestion I can put a simple panic() call in optee_alloc_and_init_page_list() to cover that case. Personally I don't like either the idea of testing OP-TEE supplicant functionality from sandbox driver and I believed from the very beginning that it's not a good idea at all, as initial sandbox driver is supposed to be used to test TEE Client API and not to validate bits and pieces of another driver (OP-TEE). > > > +} > > + > > static u32 get_attr(uint n, uint num_params, struct tee_param *params) > > { > > if (n >= num_params) > > @@ -63,6 +92,7 @@ bad_params: > > return TEE_ERROR_BAD_PARAMETERS; > > } > > > > +#ifdef CONFIG_OPTEE_TA_AVB > > static u32 ta_avb_open_session(struct udevice *dev, uint num_params, > > struct tee_param *params) > > { > > @@ -214,7 +244,100 @@ static u32 ta_avb_invoke_func(struct udevice *dev, > > u32 func, uint num_params, > > return TEE_ERROR_NOT_SUPPORTED; > > } > > } > > -#endif /*OPTEE_TA_AVB*/ > > +#endif /* OPTEE_TA_AVB */ > > + > > +#ifdef CONFIG_OPTEE_TA_RPC_TEST > > +static u32 ta_rpc_test_open_session(struct udevice *dev, 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 void fill_i2c_rpc_params(struct optee_msg_arg *msg_arg, u64 bus_num, > > + u64 chip_addr, u64 op, > > + struct tee_param_memref memref) > > +{ > > + msg_arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT; > > + msg_arg->params[1].attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT; > > + msg_arg->params[2].attr = OPTEE_MSG_ATTR_TYPE_RMEM_INOUT; > > + msg_arg->params[3].attr = OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT; > > + > > + /* trigger I2C services of TEE supplicant */ > > + msg_arg->cmd = OPTEE_MSG_RPC_CMD_I2C_TRANSFER; > > + > > + msg_arg->params[0].u.value.a = op; > > + msg_arg->params[0].u.value.b = bus_num; > > + msg_arg->params[0].u.value.c = chip_addr; > > + > > + /* buffer to read/write data */ > > + msg_arg->params[2].u.rmem.shm_ref = (ulong)memref.shm; > > + msg_arg->params[2].u.rmem.size = memref.size; > > + msg_arg->params[2].u.rmem.offs = memref.shm_offs; > > + > > + msg_arg->num_params = 4; > > +} > > + > > +static u32 ta_rpc_test_invoke_func(struct udevice *dev, u32 func, > > + uint num_params, > > + struct tee_param *params) > > +{ > > + struct tee_shm *shm; > > + struct tee_param_memref memref_data; > > + struct optee_msg_arg *msg_arg; > > + int chip_addr, bus_num, op; > > + int res; > > + > > + res = check_params(TEE_PARAM_ATTR_TYPE_VALUE_INPUT, > > + TEE_PARAM_ATTR_TYPE_MEMREF_INOUT, > > + TEE_PARAM_ATTR_TYPE_NONE, > > + TEE_PARAM_ATTR_TYPE_NONE, > > + num_params, params); > > + if (res) > > + return TEE_ERROR_BAD_PARAMETERS; > > + > > + bus_num = params[0].u.value.a; > > + chip_addr = params[0].u.value.b; > > + memref_data = params[1].u.memref; > > + > > + switch (func) { > > + case TA_RPC_TEST_CMD_I2C_READ: > > + op = OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD; > > + break; > > + case TA_RPC_TEST_CMD_I2C_WRITE: > > + op = OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR; > > + break; > > + default: > > + return TEE_ERROR_NOT_SUPPORTED; > > + } > > + > > + /* > > + * Fill params for an RPC call to tee supplicant > > + */ > > + res = get_msg_arg(dev, 4, &shm, &msg_arg); > > + if (res) > > + goto bad; > > + > > + fill_i2c_rpc_params(msg_arg, bus_num, chip_addr, op, memref_data); > > + > > + /* Make an RPC call to tee supplicant */ > > + optee_suppl_cmd(dev, shm, 0); > > + res = msg_arg->ret; > > + > > +bad: > > + tee_shm_free(shm); > > + > > + if (res) > > + return res; > > + > > + return TEE_SUCCESS; > > +} > > +#endif /* CONFIG_OPTEE_TA_RPC_TEST */ > > > > static const struct ta_entry ta_entries[] = { > > #ifdef CONFIG_OPTEE_TA_AVB > > @@ -223,6 +346,12 @@ static const struct ta_entry ta_entries[] = { > > .invoke_func = ta_avb_invoke_func, > > }, > > #endif > > +#ifdef CONFIG_OPTEE_TA_RPC_TEST > > + { .uuid = TA_RPC_TEST_UUID, > > + .open_session = ta_rpc_test_open_session, > > + .invoke_func = ta_rpc_test_invoke_func, > > + }, > > +#endif > > }; > > > > static void sandbox_tee_get_version(struct udevice *dev, > > diff --git a/include/tee/optee_ta_rpc_test.h > > b/include/tee/optee_ta_rpc_test.h > > new file mode 100644 > > index 0000000000..cae2fb04b4 > > --- /dev/null > > +++ b/include/tee/optee_ta_rpc_test.h > > @@ -0,0 +1,28 @@ > > +/* SPDX-License-Identifier: BSD-2-Clause */ > > +/* Copyright (c) 2020 Foundries Ltd */ > > + > > +#ifndef __TA_RPC_TEST_H > > +#define __TA_RPC_TEST_H > > + > > +#define TA_RPC_TEST_UUID { 0x48420575, 0x96ca, 0x401a, \ > > + { 0x89, 0x91, 0x1e, 0xfd, 0xce, 0xbd, 0x7d, 0x04 } } > > + > > +/* > > + * Does a reverse RPC call for I2C read > > + * > > + * in params[0].value.a: bus number > > + * in params[0].value.b: chip address > > + * inout params[1].u.memref: buffer to read data > > + */ > > +#define TA_RPC_TEST_CMD_I2C_READ 0 > > + > > +/* > > + * Does a reverse RPC call for I2C write > > + * > > + * in params[0].value.a: bus number > > + * in params[0].value.b: chip address > > + * inout params[1].u.memref: buffer with data to write > > + */ > > +#define TA_RPC_TEST_CMD_I2C_WRITE 1 > > + > > +#endif /* __TA_RPC_TEST_H */ > > -- > > 2.17.1 > > -- Best regards - Freundliche GrĂ¼sse - Meilleures salutations Igor Opaniuk Embedded Software Engineer T: +380 938364067 E: igor.opan...@foundries.io W: www.foundries.io