From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 90EB4C77B61 for ; Thu, 13 Apr 2023 10:15:29 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id D040C85CBD; Thu, 13 Apr 2023 12:15:26 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 744FE85CBD; Thu, 13 Apr 2023 12:15:24 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id 39D25859C4 for ; Thu, 13 Apr 2023 12:15:19 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=abdellatif.elkhlifi@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5ABB54B3; Thu, 13 Apr 2023 03:16:02 -0700 (PDT) Received: from e130802.arm.com (e130802.arm.com [10.1.33.41]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E9AF93F6C4; Thu, 13 Apr 2023 03:15:16 -0700 (PDT) Date: Thu, 13 Apr 2023 11:15:08 +0100 From: Abdellatif El Khlifi To: Heinrich Schuchardt Cc: u-boot@lists.denx.de, nd@arm.com Subject: Re: [PATCH v11 05/10] arm_ffa: introduce armffa command Message-ID: <20230413101508.GA54270@e130802.arm.com> References: <20230328161157.219375-1-abdellatif.elkhlifi@arm.com> <20230412094245.44674-1-abdellatif.elkhlifi@arm.com> <20230412094245.44674-6-abdellatif.elkhlifi@arm.com> <20230412155323.GA260072@e130802.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean On Wed, Apr 12, 2023 at 10:00:38PM +0200, Heinrich Schuchardt wrote: > > > Am 12. April 2023 17:53:23 MESZ schrieb Abdellatif El Khlifi : > >On Wed, Apr 12, 2023 at 04:02:43PM +0200, Heinrich Schuchardt wrote: > >> On 4/12/23 11:42, Abdellatif El Khlifi 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 > >> > Cc: Tom Rini > >> > Cc: Simon Glass > >> > Cc: Ilias Apalodimas > >> > Cc: Jens Wiklander > >> > Cc: Heinrich Schuchardt > >> > > >> > --- > >> > Changelog: > >> > =============== > >> > > >> > v11: > >> > > >> > * use U_BOOT_CMD_WITH_SUBCMDS > >> > * address nits > >> > > >> > 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 | 212 +++++++++++++++++++++++++++++++ > >> > doc/arch/arm64.ffa.rst | 7 + > >> > doc/usage/cmd/armffa.rst | 105 +++++++++++++++ > >> > doc/usage/index.rst | 1 + > >> > drivers/firmware/arm-ffa/Kconfig | 1 + > >> > 8 files changed, 340 insertions(+) > >> > create mode 100644 cmd/armffa.c > >> > create mode 100644 doc/usage/cmd/armffa.rst > >> > > >> > diff --git a/MAINTAINERS b/MAINTAINERS > >> > index 76f0f276ce..c64804ca2d 100644 > >> > --- a/MAINTAINERS > >> > +++ b/MAINTAINERS > >> > @@ -269,7 +269,9 @@ F: configs/cortina_presidio-asic-pnand_defconfig > >> > ARM FF-A > >> > M: Abdellatif El Khlifi > >> > 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 8c9b430f99..4cb0b2c167 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 e032091621..9130b9078d 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..ab88412c7d > >> > --- /dev/null > >> > +++ b/cmd/armffa.c > >> > @@ -0,0 +1,212 @@ > >> > +// SPDX-License-Identifier: GPL-2.0+ > >> > +/* > >> > + * Copyright 2022-2023 Arm Limited and/or its affiliates > >> > + * > >> > + * Authors: > >> > + * Abdellatif El Khlifi > >> > + */ > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > + > >> > +/** > >> > + * ffa_get_dev() - Return the FF-A device > >> > + * @devp: pointer to the FF-A device > >> > + * > >> > + * Search for the FF-A device. > >> > + * > >> > + * Return: > >> > + * 0 on success. Otherwise, failure > >> > + */ > >> > +int ffa_get_dev(struct udevice **devp) > >> > +{ > >> > + int ret; > >> > + > >> > + ret = uclass_first_device_err(UCLASS_FFA, devp); > >> > + if (ret) { > >> > + log_err("Cannot find FF-A bus device\n"); > >> > + return -ENODEV; > >> > + } > >> > + > >> > + return 0; > >> > +} > >> > + > >> > +/** > >> > + * do_ffa_getpart() - implementation of the getpart subcommand > >> > + * @cmdtp: Command Table > >> > + * @flag: flags > >> > + * @argc: number of arguments > >> > + * @argv: arguments > >> > + * > >> > + * Query the secure partition information which the UUID is provided > >> > + * 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 i; > >> > + struct udevice *dev; > >> > + > >> > + ret = ffa_get_dev(&dev); > >> > + if (ret) > >> > + return CMD_RET_FAILURE; > >> > + > >> > + /* Mode 1: getting the number of secure partitions */ > >> > + ret = ffa_partition_info_get(dev, argv[1], &count, NULL); > >> > + if (ret) { > >> > + log_err("Failure in querying partitions count (error code: %d)\n", ret); > >> > + return CMD_RET_FAILURE; > >> > + } > >> > + > >> > + if (!count) { > >> > + log_info("No secure partition found\n"); > >> > + return CMD_RET_FAILURE; > >> > + } > >> > + > >> > + /* > >> > + * Pre-allocate a buffer to be filled by the driver > >> > + * with ffa_partition_info structs > >> > + */ > >> > + > >> > + log_info("Pre-allocating %d partition(s) info structures\n", count); > >> > + > >> > + parts_info = calloc(count, sizeof(struct ffa_partition_info)); > >> > + if (!parts_info) > >> > + return CMD_RET_FAILURE; > >> > + > >> > + /* Ask the driver to fill the buffer with the SPs info */ > >> > + > >> > + ret = ffa_partition_info_get(dev, argv[1], &count, parts_info); > >> > + if (ret) { > >> > + log_err("Failure in querying partition(s) info (error code: %d)\n", ret); > >> > + free(parts_info); > >> > + return CMD_RET_FAILURE; > >> > + } > >> > + > >> > + /* SPs found , show the partition information */ > >> > + for (i = 0; i < count ; i++) { > >> > + log_info("Partition: id = %x , exec_ctxt %x , properties %x\n", > >> > + parts_info[i].id, > >> > + parts_info[i].exec_ctxt, > >> > + parts_info[i].properties); > >> > + } > >> > + > >> > + free(parts_info); > >> > + > >> > + return CMD_RET_SUCCESS; > >> > +} > >> > + > >> > +/** > >> > + * do_ffa_ping() - implementation of the ping subcommand > >> > + * @cmdtp: Command Table > >> > + * @flag: flags > >> > + * @argc: number of arguments > >> > + * @argv: arguments > >> > + * > >> > + * Send data to the secure partition which the ID is provided > >> > + * as an argument. Use 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; > >> > + > >> > + errno = 0; > >> > + part_id = strtoul(argv[1], NULL, 16); > >> > + > >> > + if (errno) { > >> > + log_err("Invalid partition ID\n"); > >> > + return CMD_RET_USAGE; > >> > + } > >> > + > >> > + ret = ffa_get_dev(&dev); > >> > + if (ret) > >> > + return CMD_RET_FAILURE; > >> > + > >> > + ret = ffa_sync_send_receive(dev, part_id, &msg, 1); > >> > + if (!ret) { > >> > + u8 cnt; > >> > + > >> > + log_info("SP response:\n[LSB]\n"); > >> > + for (cnt = 0; > >> > + cnt < sizeof(struct ffa_send_direct_data) / sizeof(u64); > >> > + cnt++) > >> > + log_info("%llx\n", ((u64 *)&msg)[cnt]); > >> > + return CMD_RET_SUCCESS; > >> > + } > >> > + > >> > + log_err("Sending direct request error (%d)\n", ret); > >> > + return CMD_RET_FAILURE; > >> > +} > >> > + > >> > +/** > >> > + *do_ffa_devlist() - implementation of the devlist subcommand > >> > + * @cmdtp: [in] Command Table > >> > + * @flag: flags > >> > + * @argc: number of arguments > >> > + * @argv: arguments > >> > + * > >> > + * Query 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; > >> > + int ret; > >> > + > >> > + ret = ffa_get_dev(&dev); > >> > + if (ret) > >> > + return CMD_RET_FAILURE; > >> > + > >> > + log_info("device name %s, dev %p, driver name %s, ops %p\n", > >> > + dev->name, > >> > + (void *)map_to_sysmem(dev), > >> > + dev->driver->name, > >> > + (void *)map_to_sysmem(dev->driver->ops)); > >> > + > >> > + return CMD_RET_SUCCESS; > >> > +} > >> > + > >> > +static char armffa_help_text[] = > >> > + "getpart \n" > >> > + " - lists the partition(s) info\n" > >> > + "ping \n" > >> > + " - sends a data pattern to the specified partition\n" > >> > + "devlist\n" > >> > + " - displays information about the FF-A device/driver\n"; > >> > + > >> > +U_BOOT_CMD_WITH_SUBCMDS(armffa, "Arm FF-A test command", armffa_help_text, > >> > + U_BOOT_SUBCMD_MKENT(getpart, 2, 1, do_ffa_getpart), > >> > + U_BOOT_SUBCMD_MKENT(ping, 2, 1, do_ffa_ping), > >> > + U_BOOT_SUBCMD_MKENT(devlist, 1, 1, do_ffa_devlist)); > >> > diff --git a/doc/arch/arm64.ffa.rst b/doc/arch/arm64.ffa.rst > >> > index 466e77e3cd..ed7240aef7 100644 > >> > --- a/doc/arch/arm64.ffa.rst > >> > +++ b/doc/arch/arm64.ffa.rst > >> > @@ -216,6 +216,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..e73d03ae51 > >> > --- /dev/null > >> > +++ b/doc/usage/cmd/armffa.rst > >> > @@ -0,0 +1,105 @@ > >> > +.. 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 > >> > + SP response: > >> > + [LSB] > >> > + fffffffe > >> > + 0 > >> > + 0 > >> > + 0 > >> > + 0 > >> > + > >> > +* ping (failure case) > >> > + > >> > +:: > >> > + > >> > + corstone1000# armffa ping 0 > >> > + Sending direct request error (-22) > >> > + > >> > +* getpart > >> > + > >> > +:: > >> > + > >> > + corstone1000# armffa getpart 33d532ed-e699-0942-c09c-a798d9cd722d > >> > + Preparing for checking FF-A partitions count > >> > + Searching FF-A partitions using the provided UUID > >> > + No FF-A partition found. Querying framework ... > >> > + Reading FF-A partitions data from the RX buffer > >> > + Number of FF-A partition(s) matching the UUID: 1 > >> > + Pre-allocating 1 partition(s) info structures > >> > + Preparing for filling FF-A partitions info > >> > + Searching FF-A partitions using the provided UUID > >> > + FF-A partition ID 8003 matches the provided UUID > >> > + Partition: id = 8003 , exec_ctxt 1 , properties 3 > >> > + > >> > +* getpart (failure case) > >> > + > >> > +:: > >> > + > >> > + corstone1000# armffa getpart 33d532ed-e699-0942-c09c-a798d9cd7221 > >> > + Preparing for checking FF-A partitions count > >> > + Searching FF-A partitions using the provided UUID > >> > + No FF-A partition found. Querying framework ... > >> > + INVALID_PARAMETERS: Unrecognized UUID > >> > + Failure in querying partitions count (error code: -22) > >> > + > >> > +* devlist > >> > + > >> > +:: > >> > + > >> > + corstone1000# armffa devlist > >> > + device name arm_ffa, dev 00000000fdf41c30, driver name arm_ffa, ops 00000000fffc0e98 > >> > + > >> > +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. > >> > >> CMD_RET_FAILURE results in $? being 1 (false). Did you actually see a > >> negative value? > > > >For armffa, the possible return codes are: CMD_RET_USAGE (-1), CMD_RET_FAILURE (1) and CMD_RET_SUCCESS (0) > > > >I suggest replacing the sentence with: > > > >The return value $? is 0 (true) on success, -1 on usage error and 1 (false) on FF-A related failures. > > CMD_RET_USAGE results in $? =1. Ok, I'll take that into account. Thanks. > > > > >Cheers > >Abdellatif > > > >> > >> Best regards > >> > >> Heinrich > >> > >> > >> > 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 > >>