Re: [PATCH v10 06/10] arm_ffa: introduce sandbox FF-A support

Simon Glass
2023-04-02T02:42:27Z

Hi Abdellatif,

On Wed, 29 Mar 2023 at 05:12, Abdellatif El Khlifi <
abdellatif.elkhlifi@arm.com> wrote:
>
> Emulate Secure World's FF-A ABIs and allow testing U-Boot FF-A support
>
> Features of the sandbox FF-A support:
>
> - Introduce an FF-A emulator
> - Introduce an FF-A device driver for FF-A comms with emulated Secure
World
> - Provides test methods allowing to read the status of the inspected ABIs
>
> The sandbox FF-A emulator supports only 64-bit direct messaging.
>
> 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:
>
> * split the FF-A sandbox support into an emulator and a driver
> * read FFA_VERSION and FFA_PARTITION_INFO_GET state using
> sandbox_ffa_query_core_state()
> * drop CONFIG_SANDBOX_FFA config
> * address nits
>
> v9: align FF-A sandbox driver with FF-A discovery through DM
>
> v8: update ffa_bus_prvdata_get() to return a pointer rather than
> a pointer address
>
> v7: state that sandbox driver supports only 64-bit direct messaging
>
> v4: align sandbox driver with the new FF-A driver interfaces
> and new way of error handling
>
> v1: introduce the sandbox driver
>
> MAINTAINERS | 3 +-
> arch/sandbox/dts/sandbox.dtsi | 8 +
> arch/sandbox/dts/test.dts | 8 +
> arch/sandbox/include/asm/sandbox_arm_ffa.h | 68 ++
> .../include/asm/sandbox_arm_ffa_priv.h | 133 ++++
> configs/sandbox64_defconfig | 1 +
> configs/sandbox_defconfig | 1 +
> doc/arch/arm64.ffa.rst | 7 +-
> doc/arch/sandbox/sandbox.rst | 1 +
> drivers/firmware/arm-ffa/Kconfig | 13 +-
> drivers/firmware/arm-ffa/Makefile | 10 +-
> drivers/firmware/arm-ffa/ffa-emul-uclass.c | 732 ++++++++++++++++++
> .../firmware/arm-ffa/sandbox_arm_ffa_priv.h | 14 -
> drivers/firmware/arm-ffa/sandbox_ffa.c | 108 +++
> include/dm/uclass-id.h | 1 +
> 15 files changed, 1087 insertions(+), 21 deletions(-)
> create mode 100644 arch/sandbox/include/asm/sandbox_arm_ffa.h
> create mode 100644 arch/sandbox/include/asm/sandbox_arm_ffa_priv.h
> create mode 100644 drivers/firmware/arm-ffa/ffa-emul-uclass.c
> delete mode 100644 drivers/firmware/arm-ffa/sandbox_arm_ffa_priv.h
> create mode 100644 drivers/firmware/arm-ffa/sandbox_ffa.c
>

[..]

> +/**
> + * sandbox_ffa_get_fwk_version() - Return the FFA framework version
> + * @dev: The sandbox FF-A emulator device
> + * @func_data: Pointer to the FF-A function arguments container
structure
> + *
> + * Return the FFA framework version read from the FF-A emulator data.
> + *
> + * Return:
> + *
> + * 0 on success. Otherwise, failure
> + */
> +static int sandbox_ffa_get_fwk_version(struct udevice *dev, struct
ffa_sandbox_data *func_data)
> +{
> + struct sandbox_ffa_emul *priv = dev_get_priv(dev);
> +
> + if (!func_data)
> + return -EINVAL;
> +
> + if (!func_data->data0 ||
> + func_data->data0_size != sizeof(priv->fwk_version))
> + return -EINVAL;
> +
> + *((u32 *)func_data->data0) = priv->fwk_version;
> +
> + return 0;
> +}
> +
> +/**
> + * sandbox_ffa_get_parts() - Return the address of partitions data
> + * @dev: The sandbox FF-A emulator device
> + * @func_data: Pointer to the FF-A function arguments container
structure
> + *
> + * Return the address of partitions data read from the FF-A emulator
data.
> + *
> + * Return:
> + *
> + * 0 on success. Otherwise, failure
> + */
> +static int sandbox_ffa_get_parts(struct udevice *dev, struct
ffa_sandbox_data *func_data)
> +{
> + struct sandbox_ffa_emul *priv = dev_get_priv(dev);
> +
> + if (!func_data)
> + return -EINVAL;
> +
> + if (!func_data->data0 ||
> + func_data->data0_size != sizeof(struct ffa_partitions *))
> + return -EINVAL;
> +
> + *((struct ffa_partitions **)func_data->data0) = &priv->partitions;
> +
> + return 0;
> +}
> +
> +/**
> + * sandbox_ffa_query_core_state() - Inspect the FF-A ABIs
> + * @queried_func_id: The FF-A function to be queried
> + * @func_data: Pointer to the FF-A function arguments container
structure
> + *
> + * Queries the status of FF-A ABI specified in the input argument.
> + *
> + * Return:
> + *
> + * 0 on success. Otherwise, failure
> + */
> +int sandbox_ffa_query_core_state(u32 queried_func_id,
> + struct ffa_sandbox_data *func_data)
> +{
> + struct udevice *dev;

Please can you use 'emul' for the emulator so it is separate from the
device?

> +
> + uclass_first_device(UCLASS_FFA_EMUL, &dev);
> + if (!dev) {
> + log_err("[FFA][SANDBOX][Emul] Cannot find FF-A emulator
during querying state\n");
> + return -ENODEV;
> + }

How about storing the emulator somewhere? You could have:

struct sandbox_ffa_priv {
struct udevice *emul;
};

.priv = sizeof(sandbox_ffa_priv)

> +
> + switch (queried_func_id) {
> + case FFA_RXTX_MAP:
> + case FFA_RXTX_UNMAP:
> + case FFA_RX_RELEASE:
> + return sandbox_ffa_get_rxbuf_flags(dev, queried_func_id,
func_data);
> + case FFA_VERSION:
> + return sandbox_ffa_get_fwk_version(dev, func_data);
> + case FFA_PARTITION_INFO_GET:
> + return sandbox_ffa_get_parts(dev, func_data);
> + default:
> + log_err("[FFA][Sandbox][Emul] Undefined FF-A interface
(%d)\n",
> + queried_func_id);
> + return -EINVAL;
> + }
> +}
> +
> +/**
> + * sandbox_arm_ffa_smccc_smc() - FF-A SMC call emulation
> + * @args: the SMC call arguments
> + * @res: the SMC call returned data
> + *
> + * Emulates the FF-A ABIs SMC call.
> + * The emulated FF-A ABI is identified and invoked.
> + * FF-A emulation is based on the FF-A specification 1.0
> + *
> + * Return:
> + *
> + * 0 on success. Otherwise, failure.
> + * FF-A protocol error codes are returned using the registers arguments
as
> + * described by the specification
> + */
> +void sandbox_arm_ffa_smccc_smc(ffa_value_t args, ffa_value_t *res)
> +{
> + int ret = 0;
> + struct udevice *dev;
> +
> + uclass_first_device(UCLASS_FFA_EMUL, &dev);
> + if (!dev) {
> + log_err("[FFA][SANDBOX][Emul] Cannot find FF-A emulator
during SMC emulation\n");
> + return;
> + }
> +
> + switch (args.a0) {
> + case FFA_SMC_32(FFA_VERSION):
> + ret = sandbox_ffa_version(dev, &args, res);
> + break;
> + case FFA_SMC_32(FFA_PARTITION_INFO_GET):
> + ret = sandbox_ffa_partition_info_get(dev, &args, res);
> + break;
> + case FFA_SMC_32(FFA_RXTX_UNMAP):
> + ret = sandbox_ffa_rxtx_unmap(dev, &args, res);
> + break;
> + case FFA_SMC_64(FFA_MSG_SEND_DIRECT_REQ):
> + ret = sandbox_ffa_msg_send_direct_req(dev, &args, res);
> + break;
> + case FFA_SMC_32(FFA_ID_GET):
> + ret = sandbox_ffa_id_get(dev, &args, res);
> + break;
> + case FFA_SMC_32(FFA_FEATURES):
> + ret = sandbox_ffa_features(&args, res);
> + break;
> + case FFA_SMC_64(FFA_RXTX_MAP):
> + ret = sandbox_ffa_rxtx_map(dev, &args, res);
> + break;
> + case FFA_SMC_32(FFA_RX_RELEASE):
> + ret = sandbox_ffa_rx_release(dev, &args, res);
> + break;
> + default:
> + log_err("[FFA][Sandbox][Emul] Undefined FF-A interface
(0x%lx)\n",
> + args.a0);
> + }
> +
> + if (ret != 0)
> + log_err("[FFA][Sandbox][Emul] FF-A ABI internal failure
(%d)\n", ret);
> +}
> +
> +/**
> + * ffa_emul_find() - Finds the FF-A emulator
> + * @dev: the sandbox FF-A device (sandbox-arm-ffa)
> + * @emulp: the FF-A emulator device (sandbox-ffa-emul)
> + *
> + * Searches for the FF-A emulator and returns its device pointer.
> + *
> + * Return:
> + * 0 on success. Otherwise, failure
> + */
> +int ffa_emul_find(struct udevice *dev, struct udevice **emulp)
> +{
> + if (!emulp)
> + return -EINVAL;
> +
> + *emulp = NULL;
> +
> + uclass_first_device(UCLASS_FFA_EMUL, emulp);
> + if (!(*emulp)) {
> + log_err("[FFA][SANDBOX][Emul] Cannot find FF-A
emulator\n");
> + return -ENODEV;
> + }
> +
> + log_info("[FFA][Sandbox][Emul] FF-A emulator found\n");
> +
> + return 0;
> +}
> +
> +UCLASS_DRIVER(ffa_emul) = {
> + .name = "ffa_emul",
> + .id = UCLASS_FFA_EMUL,
> +};
> +
> +/* Sandbox Arm FF-A emulator operations */
> +
> +static const struct ffa_emul_ops sandbox_ffa_emul_ops = {
> + .invoke_ffa_fn = sandbox_arm_ffa_smccc_smc,
> +};
> +
> +static const struct udevice_id sandbox_ffa_emul_ids[] = {
> + { .compatible = "sandbox,arm-ffa-emul" },
> + { }
> +};
> +
> +/* Declaring the sandbox FF-A emulator under UCLASS_FFA_EMUL */
> +U_BOOT_DRIVER(sandbox_ffa_emul) = {
> + .name = "sandbox_ffa_emul",
> + .id = UCLASS_FFA_EMUL,
> + .of_match = sandbox_ffa_emul_ids,
> + .ops = &sandbox_ffa_emul_ops,
> + .priv_auto = sizeof(struct sandbox_ffa_emul),
> +};
> diff --git a/drivers/firmware/arm-ffa/sandbox_arm_ffa_priv.h
b/drivers/firmware/arm-ffa/sandbox_arm_ffa_priv.h
> deleted file mode 100644
> index 4338f9c9b1..0000000000
> --- a/drivers/firmware/arm-ffa/sandbox_arm_ffa_priv.h
> +++ /dev/null
> @@ -1,14 +0,0 @@
> -/* 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>
> - */
> -
> -#ifndef __SANDBOX_ARM_FFA_PRV_H
> -#define __SANDBOX_ARM_FFA_PRV_H
> -
> -/* Future sandbox support private declarations */
> -
> -#endif
> diff --git a/drivers/firmware/arm-ffa/sandbox_ffa.c
b/drivers/firmware/arm-ffa/sandbox_ffa.c
> new file mode 100644
> index 0000000000..bb150fd5cd
> --- /dev/null
> +++ b/drivers/firmware/arm-ffa/sandbox_ffa.c
> @@ -0,0 +1,108 @@
> +// 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 <dm.h>
> +#include <log.h>
> +#include <asm/global_data.h>
> +#include <asm/sandbox_arm_ffa_priv.h>
> +#include <dm/device-internal.h>
> +#include <linux/errno.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/**
> + * ffa_set_smc_conduit() - Set the SMC conduit
> + *
> + * Selects the SMC conduit by setting the FF-A ABI invoke function.
> + * The function emulating the SMC call is provided by the FF-A emulator.
> + *
> + * Return:
> + *
> + * 0 on success. Otherwise, failure
> + */
> +int ffa_set_smc_conduit(void)
> +{
> + struct udevice *emul;
> + int ret;
> +
> + ret = ffa_emul_find(NULL, &emul);
> + if (ret) {
> + log_err("[FFA][Sandbox] Cannot find FF-A emulator, SMC
emulation failure\n");
> + return -ENODEV;
> + }
> +
> + if (!ffa_emul_get_ops(emul)->invoke_ffa_fn) {
> + log_err("[FFA][Sandbox] Cannot get FF-A emulator ops, SMC
emulation failure\n");
> + return -ENOSYS;
> + }
> +
> + dscvry_info.invoke_ffa_fn = ffa_emul_get_ops(emul)->invoke_ffa_fn;
> + log_info("[FFA][Sandbox] Using emulated Arm SMC for FF-A
conduit\n");
> +
> + return 0;
> +}
> +
> +/**
> + * sandbox_ffa_probe() - The sandbox FF-A driver probe function
> + * @dev: the sandbox Arm FF-A bus device (sandbox-arm-ffa)
> + *
> + * Probing is done through ffa_do_probe()
> + *
> + * Return:
> + *
> + * 0 on success. Otherwise, failure
> + */
> +static int sandbox_ffa_probe(struct udevice *dev)
> +{
> + return ffa_do_probe(dev);

Move to a uclass pre_probe() method.

> +}

Drop this. You can add it in the uclass as a child_pre_probe member.

> +
> +/**
> + * sandbox_ffa_bind() - The sandbox FF-A driver bind function
> + * @dev: the sandbox-arm-ffa device
> + * Tries to discover the emulated FF-A bus.
> + * Return:
> + *
> + * 0 on success.
> + */
> +static int sandbox_ffa_bind(struct udevice *dev)
> +{
> + bool ret;
> +
> + log_info("[FFA][Sandbox] binding the device\n");
> +
> + ret = ffa_try_discovery();

The code for this is immediately above. Just do that here, or make the
above function static and rename it.

> + if (ret)
> + return 0;
> + else
> + return -ENODEV;

if (ret)
return ret;

return 0;

> +}
> +
> +/* Sandbox Arm FF-A emulator operations */
> +
> +static const struct ffa_bus_ops sandbox_ffa_ops = {
> + .partition_info_get = ffa_get_partitions_info_hdlr,
> + .sync_send_receive = ffa_msg_send_direct_req_hdlr,
> + .rxtx_unmap = ffa_unmap_rxtx_buffers_hdlr,
> +};
> +
> +static const struct udevice_id sandbox_ffa_id[] = {
> + { "sandbox,arm-ffa", 0 },
> + { },
> +};
> +
> +/* Declaring the sandbox FF-A driver under UCLASS_FFA */
> +U_BOOT_DRIVER(sandbox_arm_ffa) = {
> + .name = "sandbox_arm_ffa",
> + .of_match = sandbox_ffa_id,
> + .id = UCLASS_FFA,
> + .probe = sandbox_ffa_probe,
> + .bind = sandbox_ffa_bind,
> + .ops = &sandbox_ffa_ops,
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index df77c7da58..4658411935 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -61,6 +61,7 @@ enum uclass_id {
> UCLASS_ETH, /* Ethernet device */
> UCLASS_ETH_PHY, /* Ethernet PHY device */
> UCLASS_FFA, /* Arm Firmware Framework for Armv8-A */
> + UCLASS_FFA_EMUL, /* sandbox FF-A device emulator */
> UCLASS_FIRMWARE, /* Firmware */
> UCLASS_FPGA, /* FPGA device */
> UCLASS_FUZZING_ENGINE, /* Fuzzing engine */
> --
> 2.25.1
>

Regards,
Simon