[PATCHv3 0/4] OP-TEE I2C trampoline and associated tests

This patchset allows OP-TEE to communicate with I2C devices; a typical use case would be servicing U-Boot requests that require underlying cryptographic operations implemented by an I2C chip.
On a board fitted with the NXP SE050 I2C secure element, OP-TEE can route some of the cryptographic operations it needs to that device (ie RSA, ECC, CTR..).
Before the REE executes, OP-TEE would use its own I2C drivers to communicate with the device on the bus; later on, once the REE is up, accesses to the I2C bus should be coordinated with the REE to avoid collisions. However instead of implementing such a synchronization mechanism, this trampoline service permits OP-TEE to route those I2C requests back to U-boot without then having to worry about collisions.
Lets suppose that U-Boot executes the trusted application Android Verified Boot; when OP-TEE receives the request - and before executing the application - it uses RSA to verify it. So on the back of the TA function invocation, OP-TEE returns to U-boot with a sequence of RPC calls requesting I2C transfers (check carefully the implementation in do_call_with_arg(...) implemented in drivers/tee/optee/core.c and notice the while loop)
When using sandbox testing, RPC is called directly to validate its actual implementation; however as succintly described above, these calls will always be originated in OP-TEE.
Igor Opaniuk (3): test: py: add pygit2 and pyelftools to requirements.txt drivers: tee: sandbox: add rpc test ta emulation test: dm: tee: extend with RPC test
Jorge Ramirez-Ortiz (1): drivers: tee: i2c trampoline driver
drivers/tee/Makefile | 2 + drivers/tee/optee/Kconfig | 9 ++ drivers/tee/optee/Makefile | 1 + drivers/tee/optee/i2c.c | 90 +++++++++++++++ drivers/tee/optee/optee_msg.h | 21 ++++ drivers/tee/optee/optee_msg_supplicant.h | 5 + drivers/tee/optee/optee_private.h | 17 +++ drivers/tee/optee/supplicant.c | 3 + drivers/tee/sandbox.c | 137 ++++++++++++++++++++++- include/tee/optee_ta_rpc_test.h | 28 +++++ test/dm/tee.c | 109 +++++++++++++++++- test/py/requirements.txt | 2 + 12 files changed, 415 insertions(+), 9 deletions(-) create mode 100644 drivers/tee/optee/i2c.c create mode 100644 include/tee/optee_ta_rpc_test.h

This commit gives the secure world access to the I2C bus so it can communicate with I2C slaves (typically those would be secure elements like the NXP SE050).
A similar service implementation has been merged in linux: c05210ab ("drivers: optee: allow op-tee to access devices on the i2c bus")
Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io --- drivers/tee/optee/Makefile | 1 + drivers/tee/optee/i2c.c | 90 ++++++++++++++++++++++++ drivers/tee/optee/optee_msg.h | 21 ++++++ drivers/tee/optee/optee_msg_supplicant.h | 5 ++ drivers/tee/optee/optee_private.h | 17 +++++ drivers/tee/optee/supplicant.c | 3 + 6 files changed, 137 insertions(+) create mode 100644 drivers/tee/optee/i2c.c
diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile index 928d3f8002..068c6e7aa1 100644 --- a/drivers/tee/optee/Makefile +++ b/drivers/tee/optee/Makefile @@ -2,4 +2,5 @@
obj-y += core.o obj-y += supplicant.o +obj-$(CONFIG_DM_I2C) += i2c.o obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o diff --git a/drivers/tee/optee/i2c.c b/drivers/tee/optee/i2c.c new file mode 100644 index 0000000000..ef4e10f991 --- /dev/null +++ b/drivers/tee/optee/i2c.c @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: BSD-2-Clause +/* + * Copyright (c) 2020 Foundries.io Ltd + */ + +#include <common.h> +#include <dm.h> +#include <i2c.h> +#include <tee.h> +#include "optee_msg.h" +#include "optee_private.h" + +static int check_xfer_flags(struct udevice *chip, uint tee_flags) +{ + uint flags; + int ret; + + ret = i2c_get_chip_flags(chip, &flags); + if (ret) + return ret; + + if (tee_flags & OPTEE_MSG_RPC_CMD_I2C_FLAGS_TEN_BIT) { + if (!(flags & DM_I2C_CHIP_10BIT)) + return -EINVAL; + } else { + if (flags & DM_I2C_CHIP_10BIT) + return -EINVAL; + } + + return 0; +} + +void optee_suppl_cmd_i2c_transfer(struct optee_msg_arg *arg) +{ + const u8 attr[] = { + OPTEE_MSG_ATTR_TYPE_VALUE_INPUT, + OPTEE_MSG_ATTR_TYPE_VALUE_INPUT, + OPTEE_MSG_ATTR_TYPE_RMEM_INOUT, + OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT, + }; + struct udevice *chip_dev; + struct tee_shm *shm; + u8 *buf; + int ret; + + if (arg->num_params != ARRAY_SIZE(attr) || + arg->params[0].attr != attr[0] || + arg->params[1].attr != attr[1] || + arg->params[2].attr != attr[2] || + arg->params[3].attr != attr[3]) { + goto bad; + } + + shm = (struct tee_shm *)(unsigned long)arg->params[2].u.rmem.shm_ref; + buf = shm->addr; + if (!buf) + goto bad; + + if (i2c_get_chip_for_busnum((int)arg->params[0].u.value.b, + (int)arg->params[0].u.value.c, + 0, &chip_dev)) + goto bad; + + if (check_xfer_flags(chip_dev, arg->params[1].u.value.a)) + goto bad; + + switch (arg->params[0].u.value.a) { + case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD: + ret = dm_i2c_read(chip_dev, 0, buf, + (size_t)arg->params[2].u.rmem.size); + break; + case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR: + ret = dm_i2c_write(chip_dev, 0, buf, + (size_t)arg->params[2].u.rmem.size); + break; + default: + goto bad; + } + + if (ret) { + arg->ret = TEE_ERROR_COMMUNICATION; + } else { + arg->params[3].u.value.a = arg->params[2].u.rmem.size; + arg->ret = TEE_SUCCESS; + } + + return; +bad: + arg->ret = TEE_ERROR_BAD_PARAMETERS; +} diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h index 24c60960fc..8d40ce60c2 100644 --- a/drivers/tee/optee/optee_msg.h +++ b/drivers/tee/optee/optee_msg.h @@ -422,4 +422,25 @@ struct optee_msg_arg { */ #define OPTEE_MSG_RPC_CMD_SHM_FREE 7
+/* + * Access a device on an i2c bus + * + * [in] param[0].u.value.a mode: RD(0), WR(1) + * [in] param[0].u.value.b i2c adapter + * [in] param[0].u.value.c i2c chip + * + * [in] param[1].u.value.a i2c control flags + * + * [in/out] memref[2] buffer to exchange the transfer data + * with the secure world + * + * [out] param[3].u.value.a bytes transferred by the driver + */ +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER 21 +/* I2C master transfer modes */ +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD 0 +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR 1 +/* I2C master control flags */ +#define OPTEE_MSG_RPC_CMD_I2C_FLAGS_TEN_BIT BIT(0) + #endif /* _OPTEE_MSG_H */ diff --git a/drivers/tee/optee/optee_msg_supplicant.h b/drivers/tee/optee/optee_msg_supplicant.h index a0fb8063c8..963cfd4782 100644 --- a/drivers/tee/optee/optee_msg_supplicant.h +++ b/drivers/tee/optee/optee_msg_supplicant.h @@ -147,6 +147,11 @@ #define OPTEE_MSG_RPC_CMD_SHM_ALLOC 6 #define OPTEE_MSG_RPC_CMD_SHM_FREE 7
+/* + * I2C bus access + */ +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER 21 + /* * Was OPTEE_MSG_RPC_CMD_SQL_FS, which isn't supported any longer */ diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index 9442d1c176..1f07a27ee4 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -60,6 +60,23 @@ static inline void optee_suppl_rpmb_release(struct udevice *dev) } #endif
+#ifdef CONFIG_DM_I2C +/** + * optee_suppl_cmd_i2c_transfer() - route I2C requests to an I2C chip + * @arg: OP-TEE message (layout specified in optee_msg.h) defining the + * transfer mode (read/write), adapter, chip and control flags. + * + * Handles OP-TEE requests to transfer data to the I2C chip on the I2C adapter. + */ +void optee_suppl_cmd_i2c_transfer(struct optee_msg_arg *arg); +#else +static inline void optee_suppl_cmd_i2c_transfer(struct optee_msg_arg *arg) +{ + debug("OPTEE_MSG_RPC_CMD_I2C_TRANSFER not implemented\n"); + arg->ret = TEE_ERROR_NOT_IMPLEMENTED; +} +#endif + void *optee_alloc_and_init_page_list(void *buf, ulong len, u64 *phys_buf_ptr);
#endif /* __OPTEE_PRIVATE_H */ diff --git a/drivers/tee/optee/supplicant.c b/drivers/tee/optee/supplicant.c index ae042b9a20..f9dd874b59 100644 --- a/drivers/tee/optee/supplicant.c +++ b/drivers/tee/optee/supplicant.c @@ -89,6 +89,9 @@ void optee_suppl_cmd(struct udevice *dev, struct tee_shm *shm_arg, case OPTEE_MSG_RPC_CMD_RPMB: optee_suppl_cmd_rpmb(dev, arg); break; + case OPTEE_MSG_RPC_CMD_I2C_TRANSFER: + optee_suppl_cmd_i2c_transfer(arg); + break; default: arg->ret = TEE_ERROR_NOT_IMPLEMENTED; }

On Tue, 12 Jan 2021 at 01:43, Jorge Ramirez-Ortiz jorge@foundries.io wrote:
This commit gives the secure world access to the I2C bus so it can communicate with I2C slaves (typically those would be secure elements like the NXP SE050).
A similar service implementation has been merged in linux: c05210ab ("drivers: optee: allow op-tee to access devices on the i2c bus")
Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io
drivers/tee/optee/Makefile | 1 + drivers/tee/optee/i2c.c | 90 ++++++++++++++++++++++++ drivers/tee/optee/optee_msg.h | 21 ++++++ drivers/tee/optee/optee_msg_supplicant.h | 5 ++ drivers/tee/optee/optee_private.h | 17 +++++ drivers/tee/optee/supplicant.c | 3 + 6 files changed, 137 insertions(+) create mode 100644 drivers/tee/optee/i2c.c
Reviewed-by: Simon Glass sjg@chromium.org

From: Igor Opaniuk igor.opaniuk@foundries.io
Add pygit2 and pyelftools to the list of packages for virtualenv needed to run all sets of pytests.This fixes warnings like:
binman.elf_test.TestElf.testDecodeElf (subunit.RemotedTestCase): Python elftools not available
Signed-off-by: Igor Opaniuk igor.opaniuk@foundries.io --- test/py/requirements.txt | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/test/py/requirements.txt b/test/py/requirements.txt index cf251186f4..926bccad69 100644 --- a/test/py/requirements.txt +++ b/test/py/requirements.txt @@ -10,6 +10,8 @@ packaging==19.2 pbr==5.4.3 pluggy==0.13.0 py==1.8.0 +pyelftools==0.27 +pygit2==1.4.0 pyparsing==2.4.2 pytest==5.2.1 python-mimeparse==1.6.0

On Tue, 12 Jan 2021 at 01:43, Jorge Ramirez-Ortiz jorge@foundries.io wrote:
From: Igor Opaniuk igor.opaniuk@foundries.io
Add pygit2 and pyelftools to the list of packages for virtualenv needed to run all sets of pytests.This fixes warnings like:
binman.elf_test.TestElf.testDecodeElf (subunit.RemotedTestCase): Python elftools not available
Signed-off-by: Igor Opaniuk igor.opaniuk@foundries.io
test/py/requirements.txt | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

From: Igor Opaniuk igor.opaniuk@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.opaniuk@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) +{ + return 0; +} + 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 */

On Tue, 12 Jan 2021 at 01:44, Jorge Ramirez-Ortiz jorge@foundries.io wrote:
From: Igor Opaniuk igor.opaniuk@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.opaniuk@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
Reviewed-by: Simon Glass sjg@chromium.org

On Tue, Jan 12, 2021 at 09:43:39AM +0100, Jorge Ramirez-Ortiz wrote:
From: Igor Opaniuk igor.opaniuk@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.opaniuk@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"?
+{
- 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?
+}
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

Hi Jens,
On Wed, Jan 20, 2021 at 10:49 AM Jens Wiklander jens.wiklander@linaro.org wrote:
On Tue, Jan 12, 2021 at 09:43:39AM +0100, Jorge Ramirez-Ortiz wrote:
From: Igor Opaniuk igor.opaniuk@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.opaniuk@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

Hi Igor,
On Wed, Jan 20, 2021 at 11:39:35AM +0200, Igor Opaniuk wrote:
Hi Jens,
On Wed, Jan 20, 2021 at 10:49 AM Jens Wiklander jens.wiklander@linaro.org wrote:
On Tue, Jan 12, 2021 at 09:43:39AM +0100, Jorge Ramirez-Ortiz wrote:
From: Igor Opaniuk igor.opaniuk@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.opaniuk@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.
Maybe just a comment noting that this function isn't supposed to be called from the test code but needed for linking to succeed. Reporting an error in the test suite rather than aborting the whole thing seems more friendly.
Cheers, Jens
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.opaniuk@foundries.io W: www.foundries.io

From: Igor Opaniuk igor.opaniuk@foundries.io
Extend existing DM tee tests adding test coverage for reverse RPC calls. Currently this commit only adds tests for I2C requests from TEE driver to TEE supplicant, for instance reading/writing data to emulated i2c eeprom defines in standard sandbox test device tree (arch/sandbox/dts/test.dtb):
=> i2c bus Bus 0: i2c@0 (active 0) 2c: eeprom@2c, offset len 1, flags 0 ...
Running TEE tests: => ut dm tee Test: dm_test_tee: tee.c Test: dm_test_tee: tee.c (flat tree) Failures: 0
Signed-off-by: Igor Opaniuk igor.opaniuk@foundries.io --- test/dm/tee.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 104 insertions(+), 5 deletions(-)
diff --git a/test/dm/tee.c b/test/dm/tee.c index ddbdcfb0cf..2cb54c68fb 100644 --- a/test/dm/tee.c +++ b/test/dm/tee.c @@ -13,11 +13,12 @@ #include <test/test.h> #include <test/ut.h> #include <tee/optee_ta_avb.h> +#include <tee/optee_ta_rpc_test.h>
-static int open_session(struct udevice *dev, u32 *session) +static int open_session(struct udevice *dev, u32 *session, + struct tee_optee_ta_uuid uuid) { struct tee_open_session_arg arg; - const struct tee_optee_ta_uuid uuid = TA_AVB_UUID; int rc;
memset(&arg, 0, sizeof(arg)); @@ -32,7 +33,7 @@ static int open_session(struct udevice *dev, u32 *session) return 0; }
-static int invoke_func(struct udevice *dev, u32 session) +static int invoke_func_avb(struct udevice *dev, u32 session) { struct tee_param param = { .attr = TEE_PARAM_ATTR_TYPE_VALUE_OUTPUT }; struct tee_invoke_arg arg; @@ -47,6 +48,47 @@ static int invoke_func(struct udevice *dev, u32 session) return 0; }
+static int invoke_func_rpc_test(struct udevice *dev, u32 session, + u64 op, u64 busnum, u64 chip_addr, + u8 *buf, size_t buf_size) +{ + struct tee_param param[2]; + struct tee_invoke_arg arg; + struct tee_shm *shm_buf; + int rc; + + memset(&arg, 0, sizeof(arg)); + arg.session = session; + arg.func = op; + + rc = tee_shm_alloc(dev, buf_size, + TEE_SHM_ALLOC, &shm_buf); + if (rc) + return rc; + + if (op == TA_RPC_TEST_CMD_I2C_WRITE) + memcpy(shm_buf->addr, buf, buf_size); + + memset(param, 0, sizeof(param)); + param[0].attr = TEE_PARAM_ATTR_TYPE_VALUE_INPUT; + param[0].u.value.a = busnum; + param[0].u.value.b = chip_addr; + param[1].attr = TEE_PARAM_ATTR_TYPE_MEMREF_INOUT; + param[1].u.memref.shm = shm_buf; + param[1].u.memref.size = buf_size; + + if (tee_invoke_func(dev, &arg, 2, param) || arg.ret) { + goto out; + rc = -1; + } + + if (op == TA_RPC_TEST_CMD_I2C_READ) + memcpy(buf, shm_buf->addr, buf_size); +out: + tee_shm_free(shm_buf); + return rc; +} + static int match(struct tee_version_data *vers, const void *data) { return vers->gen_caps & TEE_GEN_CAP_GP; @@ -62,6 +104,7 @@ static int test_tee(struct unit_test_state *uts, struct test_tee_vars *vars) struct tee_version_data vers; struct udevice *dev; struct sandbox_tee_state *state; + struct tee_optee_ta_uuid avb_uuid = TA_AVB_UUID; u32 session = 0; int rc; u8 data[128]; @@ -71,11 +114,11 @@ static int test_tee(struct unit_test_state *uts, struct test_tee_vars *vars) state = dev_get_priv(dev); ut_assert(!state->session);
- rc = open_session(dev, &session); + rc = open_session(dev, &session, avb_uuid); ut_assert(!rc); ut_assert(session == state->session);
- rc = invoke_func(dev, session); + rc = invoke_func_avb(dev, session); ut_assert(!rc);
rc = tee_close_session(dev, session); @@ -103,11 +146,67 @@ static int test_tee(struct unit_test_state *uts, struct test_tee_vars *vars) return 0; }
+#ifdef CONFIG_OPTEE_TA_RPC_TEST +#define BUF_SIZE 64 +static int test_tee_rpc(struct unit_test_state *uts) +{ + struct tee_version_data vers; + struct udevice *dev; + struct sandbox_tee_state *state; + struct tee_optee_ta_uuid rpc_test_uuid = TA_RPC_TEST_UUID; + u32 session = 0; + int rc; + + char *test_str = "Test string"; + u8 data[BUF_SIZE] = {0}; + u8 data_from_eeprom[BUF_SIZE] = {0}; + + /* Use sandbox I2C EEPROM emulation; bus: 0, chip: 0x2c */ + u64 bus = 0; + u64 chip = 0x2c; + + dev = tee_find_device(NULL, match, NULL, &vers); + ut_assert(dev); + state = dev_get_priv(dev); + ut_assert(!state->session); + + /* Test RPC call asking for I2C sevice */ + rc = open_session(dev, &session, rpc_test_uuid); + ut_assert(!rc); + ut_assert(session == state->session); + + /* Write buffer */ + strncpy((char *)data, test_str, strlen(test_str)); + rc = invoke_func_rpc_test(dev, session, TA_RPC_TEST_CMD_I2C_WRITE, + bus, chip, data, sizeof(data)); + ut_assert(!rc); + + /* Read buffer */ + rc = invoke_func_rpc_test(dev, session, TA_RPC_TEST_CMD_I2C_READ, + bus, chip, data_from_eeprom, + sizeof(data_from_eeprom)); + ut_assert(!rc); + + /* Compare */ + ut_assert(!memcmp(data, data_from_eeprom, BUF_SIZE)); + + rc = tee_close_session(dev, session); + ut_assert(!rc); + ut_assert(!state->session); + + return 0; +} +#endif /* CONFIG_OPTEE_TA_RPC_TEST */ + static int dm_test_tee(struct unit_test_state *uts) { struct test_tee_vars vars = { NULL, NULL }; int rc = test_tee(uts, &vars);
+#ifdef CONFIG_OPTEE_TA_RPC_TEST + rc = test_tee_rpc(uts); +#endif /* CONFIG_OPTEE_TA_RPC_TEST */ + /* In case test_tee() asserts these may still remain allocated */ tee_shm_free(vars.reg_shm); tee_shm_free(vars.alloc_shm);

On Tue, 12 Jan 2021 at 01:44, Jorge Ramirez-Ortiz jorge@foundries.io wrote:
From: Igor Opaniuk igor.opaniuk@foundries.io
Extend existing DM tee tests adding test coverage for reverse RPC calls. Currently this commit only adds tests for I2C requests from TEE driver to TEE supplicant, for instance reading/writing data to emulated i2c eeprom defines in standard sandbox test device tree (arch/sandbox/dts/test.dtb):
=> i2c bus Bus 0: i2c@0 (active 0) 2c: eeprom@2c, offset len 1, flags 0 ...
Running TEE tests: => ut dm tee Test: dm_test_tee: tee.c Test: dm_test_tee: tee.c (flat tree) Failures: 0
Signed-off-by: Igor Opaniuk igor.opaniuk@foundries.io
test/dm/tee.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 104 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I wonder if the #ifdefs could go away, or at least be reduced. Perhaps just assuming that the option is enabled on sandbox?

Hi Simon,
On Wed, Jan 13, 2021 at 6:12 PM Simon Glass sjg@chromium.org wrote:
On Tue, 12 Jan 2021 at 01:44, Jorge Ramirez-Ortiz jorge@foundries.io wrote:
From: Igor Opaniuk igor.opaniuk@foundries.io
Extend existing DM tee tests adding test coverage for reverse RPC calls. Currently this commit only adds tests for I2C requests from TEE driver to TEE supplicant, for instance reading/writing data to emulated i2c eeprom defines in standard sandbox test device tree (arch/sandbox/dts/test.dtb):
=> i2c bus Bus 0: i2c@0 (active 0) 2c: eeprom@2c, offset len 1, flags 0 ...
Running TEE tests: => ut dm tee Test: dm_test_tee: tee.c Test: dm_test_tee: tee.c (flat tree) Failures: 0
Signed-off-by: Igor Opaniuk igor.opaniuk@foundries.io
test/dm/tee.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 104 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I wonder if the #ifdefs could go away, or at least be reduced. Perhaps just assuming that the option is enabled on sandbox?
I've switched to "if (IS_ENABLED(CONFIG_*))" in test/dm/tee.c, however for dropping all ifdefs in sandbox.c likely we have to do a complete overhaul of tee sandbox driver structure, moving all TA specific code to separate files (`sandbox-avb.c` etc), which IMHO is out of scope of the current patchset. I'll address that in another patchset if you don't have objections.
Jorge will send another version of this patchset including my changes in test/dm/tee.c.

Hi Igor,
On Thu, 14 Jan 2021 at 02:41, Igor Opaniuk igor.opaniuk@foundries.io wrote:
Hi Simon,
On Wed, Jan 13, 2021 at 6:12 PM Simon Glass sjg@chromium.org wrote:
On Tue, 12 Jan 2021 at 01:44, Jorge Ramirez-Ortiz jorge@foundries.io wrote:
From: Igor Opaniuk igor.opaniuk@foundries.io
Extend existing DM tee tests adding test coverage for reverse RPC calls. Currently this commit only adds tests for I2C requests from TEE driver to TEE supplicant, for instance reading/writing data to emulated i2c eeprom defines in standard sandbox test device tree (arch/sandbox/dts/test.dtb):
=> i2c bus Bus 0: i2c@0 (active 0) 2c: eeprom@2c, offset len 1, flags 0 ...
Running TEE tests: => ut dm tee Test: dm_test_tee: tee.c Test: dm_test_tee: tee.c (flat tree) Failures: 0
Signed-off-by: Igor Opaniuk igor.opaniuk@foundries.io
test/dm/tee.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 104 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I wonder if the #ifdefs could go away, or at least be reduced. Perhaps just assuming that the option is enabled on sandbox?
I've switched to "if (IS_ENABLED(CONFIG_*))" in test/dm/tee.c, however for dropping all ifdefs in sandbox.c likely we have to do a complete overhaul of tee sandbox driver structure, moving all TA specific code to separate files (`sandbox-avb.c` etc), which IMHO is out of scope of the current patchset. I'll address that in another patchset if you don't have objections.
I don't see a great need for it as we do have #ifdefs in tests. But it might work out nicely. See how you go.
Jorge will send another version of this patchset including my changes in test/dm/tee.c.
OK.
Regards, Simon
participants (4)
-
Igor Opaniuk
-
Jens Wiklander
-
Jorge Ramirez-Ortiz
-
Simon Glass