[PATCH v10 05/10] arm_ffa: introduce armffa command
Simon Glass
sjg at chromium.org
Sun Apr 2 04:40:34 CEST 2023
Hi Abdellatif,
On Wed, 29 Mar 2023 at 05:12, Abdellatif El Khlifi <
abdellatif.elkhlifi at arm.com> wrote:
>
> Provide armffa command showcasing the use of the U-Boot FF-A support
>
> armffa is a command showcasing how to invoke FF-A operations.
> This provides a guidance to the client developers on how to
> call the FF-A bus interfaces. The command also allows to gather secure
> partitions information and ping these partitions. The command is also
> helpful in testing the communication with secure partitions.
>
> For more details please refer to the command documentation [1].
>
> [1]: doc/usage/cmd/armffa.rst
>
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> Cc: Tom Rini <trini at konsulko.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> Cc: Jens Wiklander <jens.wiklander at linaro.org>
> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
>
> ---
> Changelog:
> ===============
>
> v10:
>
> * use the FF-A driver Uclass operations
> * use uclass_first_device()
> * address nits
>
> v9:
>
> * remove manual FF-A discovery and use DM
> * use DM class APIs to probe and interact with the FF-A bus
> * add doc/usage/cmd/armffa.rst
>
> v8:
>
> * update partition_info_get() second argument to be an SP count
> * pass NULL device pointer to the FF-A bus discovery and operations
>
> v7:
>
> * adapt do_ffa_dev_list() following the recent update on
> uclass_first_device/uclass_next_device functions (they return void now)
> * set armffa command to use 64-bit direct messaging
>
> v4:
>
> * remove pattern data in do_ffa_msg_send_direct_req
>
> v3:
>
> * use the new driver interfaces (partition_info_get, sync_send_receive)
> in armffa command
>
> v2:
>
> * replace use of ffa_helper_init_device function by
> ffa_helper_bus_discover
>
> v1:
>
> * introduce armffa command
>
> MAINTAINERS | 2 +
> cmd/Kconfig | 10 ++
> cmd/Makefile | 2 +
> cmd/armffa.c | 238 +++++++++++++++++++++++++++++++
> doc/arch/arm64.ffa.rst | 7 +
> doc/usage/cmd/armffa.rst | 107 ++++++++++++++
> doc/usage/index.rst | 1 +
> drivers/firmware/arm-ffa/Kconfig | 1 +
> 8 files changed, 368 insertions(+)
> create mode 100644 cmd/armffa.c
> create mode 100644 doc/usage/cmd/armffa.rst
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 62c30184bb..add208e4ef 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -269,7 +269,9 @@ F: configs/cortina_presidio-asic-pnand_defconfig
> ARM FF-A
> M: Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> S: Maintained
> +F: cmd/armffa.c
> F: doc/arch/arm64.ffa.rst
> +F: doc/usage/cmd/armffa.rst
> F: drivers/firmware/arm-ffa/
> F: include/arm_ffa.h
> F: include/sandbox_arm_ffa.h
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index ba5ec69293..b814a20d8a 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -934,6 +934,16 @@ endmenu
>
> menu "Device access commands"
>
> +config CMD_ARMFFA
> + bool "Arm FF-A test command"
> + depends on ARM_FFA_TRANSPORT
> + help
> + Provides a test command for the FF-A support
> + supported options:
> + - Listing the partition(s) info
> + - Sending a data pattern to the specified partition
> + - Displaying the arm_ffa device info
> +
> config CMD_ARMFLASH
> #depends on FLASH_CFI_DRIVER
> bool "armflash"
> diff --git a/cmd/Makefile b/cmd/Makefile
> index d95833b2de..a1eb45f881 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -12,6 +12,8 @@ obj-y += panic.o
> obj-y += version.o
>
> # command
> +
> +obj-$(CONFIG_CMD_ARMFFA) += armffa.o
> obj-$(CONFIG_CMD_ACPI) += acpi.o
> obj-$(CONFIG_CMD_ADDRMAP) += addrmap.o
> obj-$(CONFIG_CMD_AES) += aes.o
> diff --git a/cmd/armffa.c b/cmd/armffa.c
> new file mode 100644
> index 0000000000..d983a23bbc
> --- /dev/null
> +++ b/cmd/armffa.c
> @@ -0,0 +1,238 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2022-2023 Arm Limited and/or its affiliates <
open-source-office at arm.com>
> + *
> + * Authors:
> + * Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> + */
> +#include <common.h>
> +#include <arm_ffa.h>
> +#include <command.h>
> +#include <dm.h>
> +#include <mapmem.h>
> +#include <stdlib.h>
> +#include <asm/io.h>
> +
> +/**
> + * do_ffa_getpart() - implementation of the getpart subcommand
> + * @cmdtp: Command Table
> + * @flag: flags
> + * @argc: number of arguments
> + * @argv: arguments
> + *
> + * This function queries the secure partition information which the UUID
is provided
s/This function queries/Query/
We know it is a function so try to be brief and use the imperative mood
like you do in commit messages.
> + * as an argument. The function uses the arm_ffa driver
partition_info_get operation
> + * which implements FFA_PARTITION_INFO_GET ABI to retrieve the data.
> + * The input UUID string is expected to be in big endian format.
> + *
> + * Return:
> + *
> + * CMD_RET_SUCCESS: on success, otherwise failure
> + */
> +static int do_ffa_getpart(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
> +{
> + u32 count = 0;
> + int ret;
> + struct ffa_partition_info *parts_info;
> + u32 info_idx;
> + struct udevice *dev = NULL;
> +
> + if (argc != 1)
> + return -EINVAL;
Since this is a command handler, you must return CMD_RET_USAGE here. Please
fix globally.
> +
> + uclass_first_device(UCLASS_FFA, &dev);
> + if (!dev) {
ret = uclass_first_device_err(...)
if (ret) {
(please fix globally)
> + log_err("[FFA][CMD] Cannot find FF-A bus device\n");
> + return -ENODEV;
CMD_RET_FAILURE - please fix throughout. Often it is easier to put all this
code (after arg checking) in a separate function which returns an error
code, then have the do_... function check that and either return 0 or print
a message and return CMD_RET_FAILURE.
> + }
> +
> + /* Mode 1: getting the number of secure partitions */
> + ret = ffa_partition_info_get(dev, argv[0], &count, NULL);
> + if (ret) {
> + log_err("[FFA][CMD] Failure in querying partitions count
(error code: %d)\n", ret);
%dE gives you a nice error name if you want it.
Do you think you need the [FFA][CMD] stuff? Just the message should be
enough.
> + return ret;
> + }
> +
> + if (!count) {
> + log_info("[FFA][CMD] No secure partition found\n");
> + return ret;
> + }
> +
> + /*
> + * Pre-allocate a buffer to be filled by the driver
> + * with ffa_partition_info structs
> + */
> +
> + log_info("[FFA][CMD] Pre-allocating %d partition(s) info
structures\n", count);
> +
> + parts_info = calloc(count, sizeof(struct ffa_partition_info));
Just use a local variable and avoid the allocation.
> + if (!parts_info)
> + return -EINVAL;
> +
> + /* Ask the driver to fill the buffer with the SPs info */
> +
> + ret = ffa_partition_info_get(dev, argv[0], &count, parts_info);
> + if (ret) {
> + log_err("[FFA][CMD] Failure in querying partition(s) info
(error code: %d)\n", ret);
> + free(parts_info);
> + return ret;
> + }
> +
> + /* SPs found , show the partition information */
> + for (info_idx = 0; info_idx < count ; info_idx++) {
We generally use 'i' for loops as it is shorter.
> + log_info("[FFA][CMD] Partition: id = 0x%x , exec_ctxt
0x%x , properties 0x%x\n",
You don't need the 0x. Generally hex is used for everything in U-Boot. If
you feel there is ambiguity, use %#x
> + parts_info[info_idx].id,
> + parts_info[info_idx].exec_ctxt,
> + parts_info[info_idx].properties);
> + }
> +
> + free(parts_info);
> +
> + return 0;
> +}
> +
> +/**
> + * do_ffa_ping() - implementation of the ping subcommand
> + * @cmdtp: Command Table
> + * @flag: flags
> + * @argc: number of arguments
> + * @argv: arguments
> + *
> + * This function sends data to the secure partition which the ID is
provided
partition for which ?
> + * as an argument. The function uses the arm_ffa driver
sync_send_receive operation
> + * which implements FFA_MSG_SEND_DIRECT_{REQ,RESP} ABIs to send/receive
data.
> + *
> + * Return:
> + *
> + * CMD_RET_SUCCESS: on success, otherwise failure
> + */
> +int do_ffa_ping(struct cmd_tbl *cmdtp, int flag, int argc, char *const
argv[])
> +{
> + struct ffa_send_direct_data msg = {
> + .data0 = 0xaaaaaaaa,
> + .data1 = 0xbbbbbbbb,
> + .data2 = 0xcccccccc,
> + .data3 = 0xdddddddd,
> + .data4 = 0xeeeeeeee,
> + };
> + u16 part_id;
> + int ret;
> + struct udevice *dev = NULL;
> +
> + if (argc != 1)
> + return -EINVAL;
> +
> + errno = 0;
> + part_id = strtoul(argv[0], NULL, 16);
> +
> + if (errno) {
> + log_err("[FFA][CMD] Invalid partition ID\n");
> + return -EINVAL;
> + }
> +
> + uclass_first_device(UCLASS_FFA, &dev);
> + if (!dev) {
> + log_err("[FFA][CMD] Cannot find FF-A bus device\n");
> + return -ENODEV;
> + }
> +
> + ret = ffa_sync_send_receive(dev, part_id, &msg, 1);
> + if (!ret) {
> + u8 cnt;
> +
> + log_info("[FFA][CMD] SP response:\n[LSB]\n");
> + for (cnt = 0;
> + cnt < sizeof(struct ffa_send_direct_data) /
sizeof(u64);
> + cnt++)
> + log_info("[FFA][CMD] 0x%llx\n", ((u64
*)&msg)[cnt]);
> + } else {
> + log_err("[FFA][CMD] Sending direct request error (%d)\n",
ret);
> + }
> +
> + return ret;
> +}
> +
> +/**
> + *do_ffa_devlist() - implementation of the devlist subcommand
> + * @cmdtp: [in] Command Table
> + * @flag: flags
> + * @argc: number of arguments
> + * @argv: arguments
> + *
> + * This function queries the device belonging to the UCLASS_FFA
> + * class.
> + *
> + * Return:
> + *
> + * CMD_RET_SUCCESS: on success, otherwise failure
> + */
> +int do_ffa_devlist(struct cmd_tbl *cmdtp, int flag, int argc, char
*const argv[])
> +{
> + struct udevice *dev = NULL;
> +
> + uclass_first_device(UCLASS_FFA, &dev);
> + if (!dev) {
> + log_err("[FFA][CMD] Cannot find FF-A bus device\n");
> + return -ENODEV;
> + }
> +
> + log_info("[FFA][CMD] device name %s, dev %08x, driver name %s,
ops %08x\n",
> + dev->name,
> + (u32)map_to_sysmem(dev),
use %p as this is a pointer
> + dev->driver->name,
> + (u32)map_to_sysmem(dev->driver->ops));
Use %p as this is a pointer
> +
> + return 0;
> +}
> +
> +static struct cmd_tbl armffa_commands[] = {
> + U_BOOT_CMD_MKENT(getpart, 1, 1, do_ffa_getpart, "", ""),
> + U_BOOT_CMD_MKENT(ping, 1, 1, do_ffa_ping, "", ""),
> + U_BOOT_CMD_MKENT(devlist, 0, 1, do_ffa_devlist, "", ""),
> +};
> +
> +/**
> + * do_armffa() - the armffa command main function
> + * @cmdtp: Command Table
> + * @flag: flags
> + * @argc: number of arguments
> + * @argv: arguments
> + *
> + * This function identifies which armffa subcommand to run.
> + * Then, it makes sure the arm_ffa device is probed and
> + * ready for use.
> + * Then, it runs the subcommand.
> + *
> + * Return:
> + *
> + * CMD_RET_SUCCESS: on success, otherwise failure
> + */
> +static int do_armffa(struct cmd_tbl *cmdtp, int flag, int argc, char
*const argv[])
You can use U_BOOT_CMD_WITH_SUBCMDS and drop this function
> +{
> + struct cmd_tbl *armffa_cmd;
> + int ret;
> +
> + if (argc < 2)
> + return CMD_RET_USAGE;
> +
> + armffa_cmd = find_cmd_tbl(argv[1], armffa_commands,
ARRAY_SIZE(armffa_commands));
> +
> + argc -= 2;
> + argv += 2;
> +
> + if (!armffa_cmd || argc > armffa_cmd->maxargs)
> + return CMD_RET_USAGE;
> +
> + ret = armffa_cmd->cmd(armffa_cmd, flag, argc, argv);
> +
> + return cmd_process_error(armffa_cmd, ret);
> +}
> +
> +U_BOOT_CMD(armffa, 4, 1, do_armffa,
> + "Arm FF-A operations test command",
> + "getpart <partition UUID>\n"
> + " - lists the partition(s) info\n"
> + "ping <partition ID>\n"
> + " - sends a data pattern to the specified partition\n"
> + "devlist\n"
> + " - displays information about the FF-A device/driver\n");
> diff --git a/doc/arch/arm64.ffa.rst b/doc/arch/arm64.ffa.rst
> index ddf6435402..5fedb0c255 100644
> --- a/doc/arch/arm64.ffa.rst
> +++ b/doc/arch/arm64.ffa.rst
> @@ -218,6 +218,13 @@ The following features are provided:
>
> - FF-A bus can be compiled and used without EFI
>
> +The armffa command
> +-----------------------------------
> +
> +armffa is an implementation defined command showcasing how to use the
FF-A bus and how to invoke the driver operations.
> +
> +Please refer the command documentation at doc/usage/cmd/armffa.rst
> +
> Example of boot logs with FF-A enabled
> --------------------------------------
>
> diff --git a/doc/usage/cmd/armffa.rst b/doc/usage/cmd/armffa.rst
> new file mode 100644
> index 0000000000..9bf59e393b
> --- /dev/null
> +++ b/doc/usage/cmd/armffa.rst
> @@ -0,0 +1,107 @@
> +.. SPDX-License-Identifier: GPL-2.0+:
> +
> +armffa command
> +==============
> +
> +Synopsis
> +--------
> +
> +::
> +
> + armffa [sub-command] [arguments]
> +
> + sub-commands:
> +
> + getpart [partition UUID]
> +
> + lists the partition(s) info
> +
> + ping [partition ID]
> +
> + sends a data pattern to the specified partition
> +
> + devlist
> +
> + displays information about the FF-A device/driver
> +
> +Description
> +-----------
> +
> +armffa is a command showcasing how to use the FF-A bus and how to invoke
its operations.
> +
> +This provides a guidance to the client developers on how to call the
FF-A bus interfaces.
> +
> +The command also allows to gather secure partitions information and ping
these partitions.
> +
> +The command is also helpful in testing the communication with secure
partitions.
> +
> +Example
> +-------
> +
> +The following examples are run on Corstone-1000 platform with debug logs
enabled.
> +
> +* ping
> +
> +::
> +
> + corstone1000# armffa ping 0x8003
> + [FFA][CMD] SP response:
> + [LSB]
> + [FFA][CMD] 0xfffffffe
> + [FFA][CMD] 0x0
> + [FFA][CMD] 0x0
> + [FFA][CMD] 0x0
> + [FFA][CMD] 0x0
> +
> +* ping (failure case)
> +
> +::
> +
> + corstone1000# armffa ping 0x8009
> + [FFA][CMD] Sending direct request error (-22)
> + Command 'ping' failed: Error -22
> +
> +* getpart
> +
> +::
> +
> + corstone1000# armffa getpart 33d532ed-e699-0942-c09c-a798d9cd722d
> + [FFA] Preparing for checking partitions count
> + [FFA] Searching partitions using the provided UUID
> + [FFA] No partition found. Querying framework ...
> + [FFA] Reading partitions data from the RX buffer
> + [FFA] Number of partition(s) matching the UUID: 1
> + [FFA][CMD] Pre-allocating 1 partition(s) info structures
> + [FFA] Preparing for filling partitions info
> + [FFA] Searching partitions using the provided UUID
> + [FFA] Partition ID 8003 matches the provided UUID
> + [FFA][CMD] Partition: id = 0x8003 , exec_ctxt 0x1 , properties 0x3
To me this [FFA] stuff is redundant and it looks awful. Please drop it.
> +
> +* getpart (failure case)
> +
> +::
> +
> + corstone1000# armffa getpart 33d532ed-e699-0942-c09c-a798d9cd7228
> + [FFA] Preparing for checking partitions count
> + [FFA] Searching partitions using the provided UUID
> + [FFA] No partition found. Querying framework ...
> + [FFA] INVALID_PARAMETERS: Unrecognized UUID
> + [FFA][CMD] Failure in querying partitions count (error code: -22)
> + Command 'getpart' failed: Error -22
> +
> +* devlist
> +
> +::
> +
> + corstone1000# armffa devlist
> + [FFA][CMD] device name arm_ffa, dev fdf41c30, driver name arm_ffa,
ops fffc0fc8
> +
> +Configuration
> +-------------
> +
> +The command is available if CONFIG_CMD_ARMFFA=y and
CONFIG_ARM_FFA_TRANSPORT=y.
> +
> +Return value
> +------------
> +
> +The return value $? is 0 (true) on success and a negative error code on
failure.
> diff --git a/doc/usage/index.rst b/doc/usage/index.rst
> index bc85e1d49a..df107fb710 100644
> --- a/doc/usage/index.rst
> +++ b/doc/usage/index.rst
> @@ -21,6 +21,7 @@ Shell commands
>
> cmd/acpi
> cmd/addrmap
> + cmd/armffa
> cmd/askenv
> cmd/base
> cmd/bdinfo
> diff --git a/drivers/firmware/arm-ffa/Kconfig
b/drivers/firmware/arm-ffa/Kconfig
> index 9200c8028b..a7d5392859 100644
> --- a/drivers/firmware/arm-ffa/Kconfig
> +++ b/drivers/firmware/arm-ffa/Kconfig
> @@ -5,6 +5,7 @@ config ARM_FFA_TRANSPORT
> depends on DM && ARM64
> select ARM_SMCCC
> select ARM_SMCCC_FEATURES
> + imply CMD_ARMFFA
> select LIB_UUID
> select DEVRES
> help
> --
> 2.25.1
>
Regards,
SImon
More information about the U-Boot
mailing list