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

Abdellatif El Khlifi
2023-04-12T09:52:50Z

On Sun, Apr 02, 2023 at 02:41:01PM +1200, Simon Glass wrote:
> 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)
>

in patchset v11 the sandbox emulator pointer is stored in the FF-A device uc_priv (struct ffa_priv)

Cheers,
Abdellatif

> > +
> > + 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