[PATCH v2 1/2] cmd: Add support for optee hello world ta command
Michal Simek
michal.simek at amd.com
Fri Dec 13 15:02:39 CET 2024
On 12/13/24 13:48, Mattijs Korpershoek wrote:
> Hi Venkatesh,
>
> Thank you for the patch.
>
> On ven., déc. 13, 2024 at 16:30, Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com> wrote:
>
>> Enable "optee hello" command which increments the value passed.
>> This provides easy test for establishing a session with OP-TEE
>> TA and verify.
>>
>> It includes following subcommands:
>> optee hello
>> optee hello <value>; value to increment via OP-TEE HELLO
>> WORLD TA.
>>
>> To enable the OP-TEE side HELLO WORLD example please refer
>> https://optee.readthedocs.io/en/latest/building/gits/optee_examples/optee_examples.html
>>
>> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com>
>> ---
>> cmd/Kconfig | 8 +++
>> cmd/Makefile | 1 +
>> cmd/optee_hello_world_ta.c | 104 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 113 insertions(+)
>> create mode 100644 cmd/optee_hello_world_ta.c
>>
>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index 1d7ddb4ed36..f1f8d1b9571 100644
>> --- a/cmd/Kconfig
>> +++ b/cmd/Kconfig
>> @@ -1446,6 +1446,14 @@ config CMD_OPTEE_RPMB
>> in the Replay Protection Memory Block partition in eMMC by
>> using Persistent Objects in OPTEE
>>
>> +config CMD_OPTEE_HELLO_WORLD
>
> If CMD_OPTEE_HELLO_WORLD has hello as a subcommand and we plan to add
> other commands, shouldn't we give this Kconfig symbol a more generic
> name?
>
> How about CMD_OPTEE or CMD_OPTEE_TEST?
>
> Maybe others have better recommendations for the naming.
>
>> + bool "Enable Hello world TA"
>> + depends on OPTEE
>> + default y
>
> Are we sure we want this enabled by default for everyone?
> It seems indeed like a nice debugging tool, but maybe keep it opt-in?
>
>> + help
>> + Enable the hello world ta command to test the OPTEE by passing
>
> OPTEE -> OP-TEE
>
>> + a "value" which should increment by OPTEE TA example.
>
> Ditto
>
>> +
>> config CMD_MTD
>> bool "mtd"
>> depends on MTD
>> diff --git a/cmd/Makefile b/cmd/Makefile
>> index d1f369deec0..049147a1442 100644
>> --- a/cmd/Makefile
>> +++ b/cmd/Makefile
>> @@ -118,6 +118,7 @@ obj-$(CONFIG_CMD_PAUSE) += pause.o
>> obj-$(CONFIG_CMD_SLEEP) += sleep.o
>> obj-$(CONFIG_CMD_MMC) += mmc.o
>> obj-$(CONFIG_CMD_OPTEE_RPMB) += optee_rpmb.o
>> +obj-$(CONFIG_CMD_OPTEE_HELLO_WORLD) += optee_hello_world_ta.o
>
> Similar remark for the file name.
>
>> obj-$(CONFIG_CMD_MP) += mp.o
>> obj-$(CONFIG_CMD_MTD) += mtd.o
>> obj-$(CONFIG_CMD_MTDPARTS) += mtdparts.o
>> diff --git a/cmd/optee_hello_world_ta.c b/cmd/optee_hello_world_ta.c
>> new file mode 100644
>> index 00000000000..7c398bcad2c
>> --- /dev/null
>> +++ b/cmd/optee_hello_world_ta.c
>> @@ -0,0 +1,104 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * (C) Copyright 2024, Advanced Micro Devices, Inc.
>> + */
>> +#include <command.h>
>> +#include <errno.h>
>> +#include <tee.h>
>> +#include <vsprintf.h>
>> +
>> +static struct udevice *tee;
>> +static u32 session;
>> +
>> +#define TA_HELLO_WORLD_CMD_INC_VALUE 0
>> +/* This needs to match the UUID of the Hello World TA. */
>> +#define TA_HELLO_WORLD_UUID \
>> + { 0x8aaaf200, 0x2450, 0x11e4, \
>> + { 0xab, 0xe2, 0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b} }
>> +
>> +static int hello_world_ta_open_session(void)
>> +{
>> + const struct tee_optee_ta_uuid uuid = TA_HELLO_WORLD_UUID;
>> + struct tee_open_session_arg arg;
>> + int rc;
>> +
>> + tee = tee_find_device(tee, NULL, NULL, NULL);
>> + if (!tee)
>> + return -ENODEV;
>> +
>> + memset(&arg, 0, sizeof(arg));
>> + tee_optee_ta_uuid_to_octets(arg.uuid, &uuid);
>> + rc = tee_open_session(tee, &arg, 0, NULL);
>> + if (rc != 0)
>> + session = arg.session;
>> +
>> + return 0;
return rc?
>> +}
>> +
>> +static int hello_world_ta(unsigned int value)
>> +{
>> + struct tee_param param[2];
>> + struct tee_invoke_arg arg;
>> + int status = -EACCES;
>> +
>> + printf("The Hello World TA is going to be called\n");
>> +
>> + status = hello_world_ta_open_session();
>> + if (status) {
>> + printf("hello_world_ta_open_session failed(%d)", status);
>> + return status;
>> + }
>> +
>> + arg.func = TA_HELLO_WORLD_CMD_INC_VALUE;
>> + arg.session = session;
>> +
>> + param[0].attr = TEE_PARAM_ATTR_TYPE_VALUE_INOUT;
>> + param[0].u.value.a = value;
>> +
>> + printf("TA value: %d\n", (int)param[0].u.value.a);
>> +
>> + tee_invoke_func(tee, &arg, 1, param);
>> +
>> + printf("TA value: %d\n", (int)param[0].u.value.a);
>> +
>> + tee_invoke_func(tee, &arg, 1, param);
>> +
>> + printf("TA value: %d\n", (int)param[0].u.value.a);
Any reason to call it twice? And 3 the same messages will be hard to parse via
test. I would also convert dec to hex.
>> +
>> + status = tee_close_session(tee, session);
>> +
>> + return status;
return tee_close_session.
>> +}
>> +
>> +static int do_optee_hello_world_ta(struct cmd_tbl *cmdtp, int flag, int argc,
>> + char * const argv[])
>> +{
>> + int ret;
>> + int value = 0;
>> +
>> + if (argc != cmdtp->maxargs && argv[1] != NULL) {
this should be the part of already
367 #define U_BOOT_SUBCMDS_DO_CMD(_cmdname) \
368 static int do_##_cmdname(struct cmd_tbl *cmdtp, int flag, \
369 int argc, char *const argv[], \
370 int *repeatable) \
371 { \
372 struct cmd_tbl *subcmd; \
373 \
374 /* We need at least the cmd and subcmd names. */ \
375 if (argc < 2 || argc > CONFIG_SYS_MAXARGS) \
376 return CMD_RET_USAGE; \
377 \
378 subcmd = find_cmd_tbl(argv[1], _cmdname##_subcmds, \
379 ARRAY_SIZE(_cmdname##_subcmds)); \
380 if (!subcmd || argc - 1 > subcmd->maxargs) \
381 return CMD_RET_USAGE; \
382 \
383 if (flag == CMD_FLAG_REPEAT && \
384 !cmd_is_repeatable(subcmd)) \
385 return CMD_RET_SUCCESS; \
386 \
387 return subcmd->cmd_rep(subcmd, flag, argc - 1, \
388 argv + 1, repeatable); \
389 }
And this will allow you to have number of args between 2 and maxargs you setup.
>> + debug("do_optee_hello_world_ta: incorrect parameters passed\n");
>> + return CMD_RET_USAGE;
>> + }
>> +
>> + if (argv[1] != NULL)
>> + value = dectoul(argv[1], NULL);
values on u-boot prompt are mostly hex that's why I think you should decode hex
here.
>> +
>> + ret = hello_world_ta(value);
>> + if (ret)
>> + return CMD_RET_FAILURE;
>> +
>> + return CMD_RET_SUCCESS;
>> +}
>> +
>> +U_BOOT_LONGHELP(optee,
>> + "- commands can be verified on OP-TEE\n\n"
>> + "optee hello\n"
>> + "optee hello <value>\n"
>> + "\n"
>> + "With:\n"
>> + "\t<value>: integer value\n"
>> + );
>> +
>> +U_BOOT_CMD_WITH_SUBCMDS(optee, "OP-TEE commands", optee_help_text,
>> + U_BOOT_SUBCMD_MKENT(hello, 2, 1, do_optee_hello_world_ta));
>> --
>> 2.34.1
M
More information about the U-Boot
mailing list