Hi Abdellatif,

On Wed, 29 Mar 2023 at 05:12, Abdellatif El Khlifi < abdellatif.elkhlifi@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].

Signed-off-by: Abdellatif El Khlifi abdellatif.elkhlifi@arm.com Cc: Tom Rini trini@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Ilias Apalodimas ilias.apalodimas@linaro.org Cc: Jens Wiklander jens.wiklander@linaro.org Cc: Heinrich Schuchardt xypron.glpk@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@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@arm.com>

• • Authors:
• • Abdellatif El Khlifi abdellatif.elkhlifi@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