On 12/18/24 05:19, Venkatesh Yadav Abbarapu wrote:
> Add the basic 'hello world ta' command which increment
> of the value passed. This provides easy test for
> establishing a session with OP-TEE TA and verify.
> 
> It includes following "hello world ta" subcommands:
> optee hello; default value '0' is passed and gets incremented.
> 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 <[email protected]>
> ---
>  cmd/Kconfig  |  6 +++++
>  cmd/Makefile |  1 +
>  cmd/optee.c  | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 83 insertions(+)
>  create mode 100644 cmd/optee.c
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 1d7ddb4ed36..4b33468891e 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1446,6 +1446,12 @@ config CMD_OPTEE_RPMB
>         in the Replay Protection Memory Block partition in eMMC by
>         using Persistent Objects in OPTEE
>  
> +config CMD_OPTEE
> +     bool "Enable OP-TEE commands"
> +     depends on OPTEE
> +     help
> +      OP-TEE commands support.
> +
>  config CMD_MTD
>       bool "mtd"
>       depends on MTD
> diff --git a/cmd/Makefile b/cmd/Makefile
> index d1f369deec0..533d0f6a1be 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) += optee.o
>  obj-$(CONFIG_CMD_MP) += mp.o
>  obj-$(CONFIG_CMD_MTD) += mtd.o
>  obj-$(CONFIG_CMD_MTDPARTS) += mtdparts.o
> diff --git a/cmd/optee.c b/cmd/optee.c
> new file mode 100644
> index 00000000000..e8d5f4ceb36
> --- /dev/null
> +++ b/cmd/optee.c
> @@ -0,0 +1,76 @@
> +// 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>
> +
> +#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(unsigned int value)
> +{
> +     const struct tee_optee_ta_uuid uuid = TA_HELLO_WORLD_UUID;
> +     struct tee_open_session_arg session_arg;
> +     struct udevice *tee = NULL;
> +     struct tee_invoke_arg arg;
> +     struct tee_param param[2];
> +     int rc;
> +
> +     tee = tee_find_device(tee, NULL, NULL, NULL);
> +     if (!tee)
> +             return -ENODEV;
> +
> +     memset(&session_arg, 0, sizeof(session_arg));
> +     tee_optee_ta_uuid_to_octets(session_arg.uuid, &uuid);
> +     rc = tee_open_session(tee, &session_arg, 0, NULL);
> +     if (rc) {
> +             printf("tee_open_session(): failed(%d)\n", rc);
> +             return rc;
> +     }
> +
> +     arg.func = TA_HELLO_WORLD_CMD_INC_VALUE;
> +     arg.session = session_arg.session;
> +
> +     param[0].attr = TEE_PARAM_ATTR_TYPE_VALUE_INOUT;
> +     param[0].u.value.a = value;
> +
> +     printf("Value before: 0x%x\n", (int)param[0].u.value.a);
> +     printf("Calling TA\n");
> +     tee_invoke_func(tee, &arg, 1, param);
> +
> +     printf("Value after: 0x%x\n", (int)param[0].u.value.a);
> +     return tee_close_session(tee, session_arg.session);
> +}
> +
> +static int do_optee_hello_world_ta(struct cmd_tbl *cmdtp, int flag, int argc,
> +                                char * const argv[])
> +{
> +     int ret, value = 0;
> +
> +     if (strcmp(argv[1], NULL))
> +             value = hextoul(argv[1], NULL);
> +
> +     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"

What is this supposed to mean?

> +             "optee hello\n"
> +             "optee hello <value>\n"

Optional values are usually indicated with []

> +             "\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));

I believe the following should be clear and good enough:

U_BOOT_LONGHELP(optee,
        "hello [<value>]   Invoke the OP-TEE 'Hello World' TA\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));


With that:

Reviewed-by: Jerome Forissier <[email protected]>

Thanks,
-- 
Jerome

Reply via email to