| Message ID | 20210112084340.23858-5-jorge@foundries.io |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | OP-TEE I2C trampoline and associated tests | expand |
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
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);