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 88DD5C6FA99 for ; Fri, 10 Mar 2023 20:57:06 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 01B8585321; Fri, 10 Mar 2023 21:51:39 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="FJPr8OfA"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 9F7DF86018; Fri, 10 Mar 2023 21:51:16 +0100 (CET) Received: from mail-yw1-x112c.google.com (mail-yw1-x112c.google.com [IPv6:2607:f8b0:4864:20::112c]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id D7A3486000 for ; Fri, 10 Mar 2023 21:51:11 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-yw1-x112c.google.com with SMTP id 00721157ae682-536bbef1c5eso120732617b3.9 for ; Fri, 10 Mar 2023 12:51:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1678481470; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=38SAiNq7jNaXClD/e8EGVmF66z4/gvAvBHuN4LyyYvw=; b=FJPr8OfAdL96dWJMAdCJUoVeAa3kOT6A/1haM8hJeDl77PrtPU09JiLtCR87rMDJ3M pvfqTCYCl4dN+mB3NrxdSNF0AQXNu0KfC+/FxvrCHeC/my89mXuVOprsyhcBMyy5HJrK wri5hCeTGPI/Ef82OC3NIBynXoyLsWpJQmaog= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678481470; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=38SAiNq7jNaXClD/e8EGVmF66z4/gvAvBHuN4LyyYvw=; b=1YcsflfoF5Dv/jR/snAeKpHFryfyXCe9xOlDhXGihML1Wd0ay/BSy0WslYfMYHke29 tmKymaIiGn/jxobrrsOf2BZWf0PeyQ244JmwE4bBp7798XTfsfBNRcnpIH/np6csK+cG YK69CSxCUg7SQHpPyYkI5eTh2BxlJmAm5ir6bSDZ50BAz2iHT8pOlKY8k5K4sNke51K3 Kor44bFuGgctSc77bZFwxaJ2EqOp3b7Tf9k7M7qEwWkAjT2dcYzmwN+pZXwqhoKMvc29 WEpYmOmFzLmkpWwh6/+0VKK1Cktx0bTh4LEEKwD/8eXV4sqtGNG179pdckxcx1981jDH t6Kw== X-Gm-Message-State: AO0yUKVA4IrzJoLddV9BvLF+Osv8AVHt7kIcL13WOKU/yx98sKW8z2mE 2bPDw6H3bvj6EngHBMtEoUbziHC9hDvX9h5YNJ+wZQ== X-Google-Smtp-Source: AK7set+kZjA8vQE4gdTytoa1dJ1hSYFK53COunG3y9TspFloQJAp3v/2WT8iR6wYows24G8GQkaHDmmOOZn1qtdwLrg= X-Received: by 2002:a81:4424:0:b0:536:3c2c:bf5e with SMTP id r36-20020a814424000000b005363c2cbf5emr17474924ywa.8.1678481470296; Fri, 10 Mar 2023 12:51:10 -0800 (PST) MIME-Version: 1.0 References: <20230310141016.137986-1-abdellatif.elkhlifi@arm.com> <20230310141016.137986-6-abdellatif.elkhlifi@arm.com> In-Reply-To: <20230310141016.137986-6-abdellatif.elkhlifi@arm.com> From: Simon Glass Date: Fri, 10 Mar 2023 12:49:56 -0800 Message-ID: Subject: Re: [PATCH v9 05/10] arm_ffa: introduce armffa command To: Abdellatif El Khlifi Cc: Drew.Reed@arm.com, achin.gupta@arm.com, ilias.apalodimas@linaro.org, jens.wiklander@linaro.org, nd@arm.com, robh@kernel.org, sudeep.holla@arm.com, trini@konsulko.com, u-boot@lists.denx.de, xueliang.zhong@arm.com Content-Type: text/plain; charset="UTF-8" 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 Hi Abdellatif, On Fri, 10 Mar 2023 at 06:10, Abdellatif El Khlifi wrote: > > Provide armffa command showcasing the use of the FF-A driver > > armffa is a command showcasing how to use the FF-A driver 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. > > 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 > > --- > Changelog: > =============== > > 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 | 264 +++++++++++++++++++++++++++++++ > doc/usage/cmd/armffa.rst | 118 ++++++++++++++ > doc/usage/index.rst | 1 + +Heinrich Schuchardt for docs > drivers/firmware/arm-ffa/Kconfig | 1 + > 7 files changed, 398 insertions(+) > create mode 100644 cmd/armffa.c > create mode 100644 doc/usage/cmd/armffa.rst > > diff --git a/MAINTAINERS b/MAINTAINERS > index 1dfa23c1f0..18e9c2ce99 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 > > diff --git a/cmd/Kconfig b/cmd/Kconfig > index a3512836c1..f24c52def4 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 Arm FF-A driver > + 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 2d8bb4fc05..a59ab55ad0 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..f6c017542d > --- /dev/null > +++ b/cmd/armffa.c > @@ -0,0 +1,264 @@ > +// 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 > + > +/** > + * 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 > + * 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; > + struct ffa_bus_ops *ffa_ops = NULL; > + > + if (argc != 1) > + return -EINVAL; > + > + uclass_get_device_by_name(UCLASS_FFA, "arm_ffa", &dev); uclass_first_device() The name is most likely to have a hyphen than an underscore due to devicetree conventions. Anyway, there should be only one. > + if (!dev) { > + log_err("[FFA] Cannot find FF-A bus device\n"); > + return -ENODEV; > + } > + > + ffa_ops = (struct ffa_bus_ops *)ffa_bus_get_ops(dev);unl > + if (!ffa_ops) { > + log_err("[FFA] Invalid FF-A ops\n"); > + return -EINVAL; > + } > + > + /* Mode 1: getting the number of secure partitions */ > + ret = ffa_ops->partition_info_get(dev, argv[0], &count, NULL); These should be uclass operations in armffa.h, i.e. they implement the uclass. See how other things do this, e.g. p2sb.h is a really simple example. Then you implement the callers in armffa-uclass.c rather than accessing the ops here. > + if (ret != 0) { if (ret) Please fix below > + log_err("[FFA] Failure in querying partitions count (error code: %d)\n", ret); > + return ret; > + } > + > + if (!count) { > + log_info("[FFA] 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] Pre-allocating %d partition(s) info structures\n", count); > + > + parts_info = calloc(count, sizeof(struct ffa_partition_info)); > + if (!parts_info) > + return -EINVAL; > + > + /* Ask the driver to fill the buffer with the SPs info */ > + > + ret = ffa_ops->partition_info_get(dev, argv[0], &count, parts_info); here again the operation > + if (ret != 0) { > + log_err("[FFA] 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++) { > + log_info("[FFA] Partition: id = 0x%x , exec_ctxt 0x%x , properties 0x%x\n", > + 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 > + * 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; > + struct ffa_bus_ops *ffa_ops = NULL; > + > + if (argc != 1) > + return -EINVAL; > + > + errno = 0; > + part_id = strtoul(argv[0], NULL, 16); > + > + if (errno) { > + log_err("[FFA] Invalid partition ID\n"); > + return -EINVAL; > + } > + > + uclass_get_device_by_name(UCLASS_FFA, "arm_ffa", &dev); > + if (!dev) { > + log_err("[FFA] Cannot find FF-A bus device\n"); > + return -ENODEV; > + } > + > + ffa_ops = (struct ffa_bus_ops *)ffa_bus_get_ops(dev); > + if (!ffa_ops) { > + log_err("[FFA] Invalid FF-A ops\n"); > + return -EINVAL; > + } > + > + ret = ffa_ops->sync_send_receive(dev, part_id, &msg, 1); > + if (!ret) { > + u8 cnt; > + > + log_info("[FFA] SP response:\n[LSB]\n"); > + for (cnt = 0; > + cnt < sizeof(struct ffa_send_direct_data) / sizeof(u64); > + cnt++) > + log_info("[FFA] 0x%llx\n", ((u64 *)&msg)[cnt]); > + } else { > + log_err("[FFA] 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 devices 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; > + int i; > + > + log_info("[FFA] FF-A uclass entries:\n"); > + > + for (i = 0, uclass_first_device(UCLASS_FFA, &dev); > + dev; > + uclass_next_device(&dev), i++) { uclass_foreacah_dev_probe(dUCLASS_FFA, dev) { > + log_info("[FFA] entry %d - instance %08x, ops %08x, plat %08x\n", > + i, > + (u32)map_to_sysmem(dev), > + (u32)map_to_sysmem(dev->driver->ops), > + (u32)map_to_sysmem(dev_get_plat(dev))); > + } > + > + 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[]) > +{ > + 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; > + > + if (IS_ENABLED(CONFIG_SANDBOX_FFA)) { No, this needs to be bound automatically. Add it to test.dts and let sandbox bind it. > + struct udevice *sdx_dev = NULL; > + /* Probe the FF-A sandbox driver, then bind the FF-A bus driver */ > + uclass_get_device_by_name(UCLASS_FFA, "sandbox_arm_ffa", &sdx_dev); > + if (!sdx_dev) { > + log_err("[FFA] Cannot find FF-A sandbox device\n"); > + return -ENODEV; > + } > + } > + > + ret = armffa_cmd->cmd(armffa_cmd, flag, argc, argv); > + > + return cmd_process_error(armffa_cmd, ret); > +} [..] Regards, Simon