[PATCH v9 06/10] arm_ffa: introduce the FF-A Sandbox driver
Simon Glass
sjg at chromium.org
Wed Mar 15 15:05:19 CET 2023
Hi Abdellatif,
On Tue, 14 Mar 2023 at 11:59, Abdellatif El Khlifi
<abdellatif.elkhlifi at arm.com> wrote:
>
> Hi Simon,
>
> > Hi Abdellatif,
> >
> > On Fri, 10 Mar 2023 at 06:10, Abdellatif El Khlifi
> > <abdellatif.elkhlifi at arm.com> wrote:
> > >
> > > Provide a Sandbox driver to emulate the FF-A ABIs
> > >
> > > The emulated ABIs are those supported by the FF-A core driver
> > > and according to FF-A specification v1.0.
> > >
> > > The Sandbox driver provides operations allowing the test
> > > application to read the status of all the inspected ABIs
> > > and perform functional tests based on that.
> > >
> > > sandbox driver supports only 64-bit direct messaging.
> > >
> > > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> > > Cc: Tom Rini <trini at konsulko.com>
> > > Cc: Simon Glass <sjg at chromium.org>
> > > Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > > Cc: Jens Wiklander <jens.wiklander at linaro.org>
> > >
> > > ---
> > > Changelog:
> > > ===============
> > >
> > > 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 | 1 +
> > > arch/sandbox/dts/sandbox.dtsi | 4 +
> > > arch/sandbox/dts/test.dts | 4 +
> > > configs/sandbox64_defconfig | 2 +
> > > configs/sandbox_defconfig | 2 +
> > > doc/arch/arm64.ffa.rst | 4 +
> > > doc/arch/sandbox/sandbox.rst | 1 +
> > > drivers/firmware/arm-ffa/Kconfig | 11 +-
> > > drivers/firmware/arm-ffa/Makefile | 1 +
> > > drivers/firmware/arm-ffa/core.c | 36 +-
> > > drivers/firmware/arm-ffa/sandbox.c | 610 ++++++++++++++++++
> > > .../firmware/arm-ffa/sandbox_arm_ffa_priv.h | 129 ++++
> > > include/arm_ffa.h | 5 +-
> > > include/sandbox_arm_ffa.h | 124 ++++
> > > 14 files changed, 928 insertions(+), 6 deletions(-)
> > > create mode 100644 drivers/firmware/arm-ffa/sandbox.c
> > > create mode 100644 drivers/firmware/arm-ffa/sandbox_arm_ffa_priv.h
> > > create mode 100644 include/sandbox_arm_ffa.h
> > >
> >
> > Could you use 80 columns where possible? There seem to be a lot of
> > things that extend beyond that without much of a reason.
> >
> > Also you can use 'ulong' instead of 'unsigned long'. It is less
> > verbose and a U-Boot standard.
> >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 18e9c2ce99..2b9d33e964 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -274,6 +274,7 @@ 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
> > >
> > > ARM FREESCALE IMX
> > > M: Stefano Babic <sbabic at denx.de>
> > > diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
> > > index 30a305c4d2..059c273277 100644
> > > --- a/arch/sandbox/dts/sandbox.dtsi
> > > +++ b/arch/sandbox/dts/sandbox.dtsi
> > > @@ -445,6 +445,10 @@
> > > thermal {
> > > compatible = "sandbox,thermal";
> > > };
> > > +
> > > + sandbox_arm_ffa {
> > > + compatible = "sandbox,arm_ffa";
> > > + };
> > > };
> > >
> > > &cros_ec {
> > > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> > > index d72d7a567a..11dc6ed0d9 100644
> > > --- a/arch/sandbox/dts/test.dts
> > > +++ b/arch/sandbox/dts/test.dts
> > > @@ -1802,6 +1802,10 @@
> > > compatible = "u-boot,fwu-mdata-gpt";
> > > fwu-mdata-store = <&mmc0>;
> > > };
> > > +
> > > + sandbox_arm_ffa {
> > > + compatible = "sandbox,arm_ffa";
> > > + };
> >
> > I see that you have this, so the driver should bind automatically.
> >
> > Is the problem that you are trying to bind the sandbox emulator?
> > Again, you can actually add that to the DT. See how this works for
> > i2c, SPI, PCI, for example.
> >
> > > };
> > >
> > > #include "sandbox_pmic.dtsi"
> > > diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> > > index ccbc18aad0..35d4676cf7 100644
> > > --- a/configs/sandbox64_defconfig
> > > +++ b/configs/sandbox64_defconfig
> > > @@ -259,3 +259,5 @@ CONFIG_FWU_MULTI_BANK_UPDATE=y
> > > CONFIG_UNIT_TEST=y
> > > CONFIG_UT_TIME=y
> > > CONFIG_UT_DM=y
> > > +CONFIG_ARM_FFA_TRANSPORT=y
> > > +CONFIG_SANDBOX_FFA=y
> > > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > > index a0fbdad20a..8aab8dda31 100644
> > > --- a/configs/sandbox_defconfig
> > > +++ b/configs/sandbox_defconfig
> > > @@ -336,3 +336,5 @@ CONFIG_TEST_FDTDEC=y
> > > CONFIG_UNIT_TEST=y
> > > CONFIG_UT_TIME=y
> > > CONFIG_UT_DM=y
> > > +CONFIG_ARM_FFA_TRANSPORT=y
> > > +CONFIG_SANDBOX_FFA=y
> > > \ No newline at end of file
> > > diff --git a/doc/arch/arm64.ffa.rst b/doc/arch/arm64.ffa.rst
> > > index 8fad9ef3d0..555ee9a6ae 100644
> > > --- a/doc/arch/arm64.ffa.rst
> > > +++ b/doc/arch/arm64.ffa.rst
> > > @@ -64,6 +64,10 @@ CONFIG_ARM_FFA_TRANSPORT
> > > Enables the FF-A bus driver. Turn this on if you want to use FF-A
> > > communication.
> > >
> > > +CONFIG_SANDBOX_FFA
> > > + Enables FF-A Sandbox driver. This emulates the FF-A ABIs handling under
> > > + Sandbox and provides functional tests for FF-A.
> >
> > OK that is why I am confused. Please don't call this a driver. It is
> > an emulator. When you have an emulator and a driver for the same thing
> > it gets confusing.
> >
> > > +
> > > FF-A ABIs under the hood
> > > ---------------------------------------
> > >
> > > diff --git a/doc/arch/sandbox/sandbox.rst b/doc/arch/sandbox/sandbox.rst
> > > index cd7f8a2cb0..c5df372e00 100644
> > > --- a/doc/arch/sandbox/sandbox.rst
> > > +++ b/doc/arch/sandbox/sandbox.rst
> > > @@ -200,6 +200,7 @@ Supported Drivers
> > >
> > > U-Boot sandbox supports these emulations:
> > >
> > > +- Arm FF-A
> > > - Block devices
> > > - Chrome OS EC
> > > - GPIO
> > > diff --git a/drivers/firmware/arm-ffa/Kconfig b/drivers/firmware/arm-ffa/Kconfig
> > > index 2cfd7ef5fc..b5430eb6f4 100644
> > > --- a/drivers/firmware/arm-ffa/Kconfig
> > > +++ b/drivers/firmware/arm-ffa/Kconfig
> > > @@ -2,9 +2,9 @@
> > >
> > > config ARM_FFA_TRANSPORT
> > > bool "Enable Arm Firmware Framework for Armv8-A driver"
> > > - depends on DM && ARM64
> > > - select ARM_SMCCC
> > > - select ARM_SMCCC_FEATURES
> > > + depends on DM && (ARM64 || SANDBOX)
> > > + select ARM_SMCCC if !SANDBOX
> > > + select ARM_SMCCC_FEATURES if !SANDBOX
> > > imply CMD_ARMFFA
> > > select LIB_UUID
> > > select DEVRES
> > > @@ -32,3 +32,8 @@ config ARM_FFA_TRANSPORT
> > >
> > > For more details about the FF-A driver, please refer to doc/arch/arm64.ffa.rst
> > >
> > > +config SANDBOX_FFA
> > > + bool "FF-A Sandbox driver"
> > > + depends on ARM_FFA_TRANSPORT && SANDBOX
> > > + help
> > > + This emulates the FF-A handling under Sandbox and allows to test the FF-A driver
> > > diff --git a/drivers/firmware/arm-ffa/Makefile b/drivers/firmware/arm-ffa/Makefile
> > > index c8d83b4bc9..d22c1ba609 100644
> > > --- a/drivers/firmware/arm-ffa/Makefile
> > > +++ b/drivers/firmware/arm-ffa/Makefile
> > > @@ -6,3 +6,4 @@
> > > # Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> > >
> > > obj-y += arm-ffa-uclass.o core.o
> > > +obj-$(CONFIG_SANDBOX_FFA) += sandbox.o
> > > diff --git a/drivers/firmware/arm-ffa/core.c b/drivers/firmware/arm-ffa/core.c
> > > index 2210f5343c..0d2e6ff0d4 100644
> > > --- a/drivers/firmware/arm-ffa/core.c
> > > +++ b/drivers/firmware/arm-ffa/core.c
> > > @@ -1042,6 +1042,7 @@ bool ffa_try_discovery(void)
> > > return true;
> > > }
> > >
> > > +#if !CONFIG_IS_ENABLED(SANDBOX_FFA)
> >
> > We should not need #ifdefs here. The sandbox driver is just another
> > driver, just like there is the ARM driver. We select the correct one
> > using the devicetree and everything just works.
>
> The FF-A core driver (core.c) has Arm specific code that is not available
> when running sandbox. So we still need to use these ifdefs in core.c
> to prevent the Arm specific code from building under sandbox.
> For example these are Arm specific:
>
> arm_smccc_1_2_smc()
> struct arm_smccc_res
> ARM_SMCCC_FEATURE_DRIVER
> The SMC conduit. In sandbox mode the core driver should use the emulated conduit implemented by sandbox_arm_ffa_smccc_smc()
>
> >
> > > /**
> > > * __arm_ffa_fn_smc() - SMC wrapper
> > > * @args: FF-A ABI arguments to be copied to Xn registers
> > > @@ -1069,6 +1070,7 @@ void __arm_ffa_fn_smc(ffa_value_t args, ffa_value_t *res)
> > > * The FF-A driver supports the SMCCCv1.2 extended input/output registers.
> > > * So, the legacy SMC invocation is not used.
> > > *
> > > + * In Sandbox mode sandbox_arm_ffa is used to test arm_ffa driver.
> > > * Return:
> > > *
> > > * 0 on success. Otherwise, failure
> > > @@ -1088,6 +1090,30 @@ ARM_SMCCC_FEATURE_DRIVER(arm_ffa) = {
> > > .driver_name = FFA_DRV_NAME,
> > > .is_supported = ffa_bus_is_supported,
> > > };
> > > +#else
> > > +/* SANDBOX_FFA */
> > > +
> > > +/**
> > > + * ffa_bind() - The driver bind function
> > > + * @dev: the arm_ffa device
> > > + * When using sandbox tries to discover the emulated FF-A bus.
> > > + * Return:
> > > + *
> > > + * 0 on success.
> > > + */
> > > +static int ffa_bind(struct udevice *dev)
> >
> > I don't think this is binding anything. How about ffa_get_dev() ?
>
> In sandbox mode, when the FF-A sandbox driver (emulates the secure world) is probed, it calls
> device_bind_driver() to bind the FF-A core driver (a similar approach to the PSCI way).
> ffa_bind() is needed in sandbox mode so the FF-A discovery is setup properly.
>
> In an Arm platform, the PSCI driver calls the FF-A discovery callback
> ffa_bus_is_supported() which tries to discover FF-A. When discovery is successful,
> PSCI driver binds the FF-A core device by calling device_bind_driver()
>
> The core driver needs to work on both Arm and sandbox.
> The sandbox FF-A driver plays the role of a secure world emulator with which
> the FF-A core driver exchanges data.
I think there is some sort of fundamental misunderstanding of driver
model. If you put the methods in the uclass then you won't need any
#ifdefs. What am I missing?
Yes, the emulator should be called an emulator, since it is confusing
if you call it a driver.
>
> >
> > > +{
> > > + bool ret;
> > > +
> > > + log_info("[FFA] binding the device\n");
> > > +
> > > + ret = ffa_try_discovery();
> > > + if (ret)
> > > + return 0;
> > > + else
> > > + return -ENODEV;
> > > +}
> > > +#endif
> > >
> > > /**
> > > * ffa_set_smc_conduit() - Set the SMC conduit
> > > @@ -1101,7 +1127,12 @@ ARM_SMCCC_FEATURE_DRIVER(arm_ffa) = {
> > > */
> > > static int ffa_set_smc_conduit(void)
> > > {
> > > - dscvry_info.invoke_ffa_fn = __arm_ffa_fn_smc;
> > > +#if CONFIG_IS_ENABLED(SANDBOX_FFA)
> > > + dscvry_info.invoke_ffa_fn = sandbox_arm_ffa_smccc_smc;
> > > + log_info("[FFA] Using SMC emulation\n");
> > > +#else
> > > + dscvry_info.invoke_ffa_fn = __arm_ffa_fn_smc;
> > > +#endif
> >
> > This needs to be reworked to go through the uclass to the correct
> > driver. Basically you need an ARM driver and a sandbox driver (which
> > attaches to the emulator).
> >
> > There should not be #idefs in this sort of code...driver model should
> > handle it. See other uclasses for examples.
>
> In sandbox we are testing the FF-A core driver. The core driver has some Arm specific code which
> explains why we need the ifdefs. In Arm the SMC conduit is implemented by __arm_ffa_fn_smc()
> which ends up calling Arm instructions.
> When building the core driver with sandbox, the SMC conduit is set to the emulated version of the SMC calls
> implemented by sandbox_arm_ffa_smccc_smc()
>
> I'm happy to rename the FF-A sandbox driver to the FF-A sandbox emulator if that clears the doubts.
> However, the emulator is still a driver and bound to a sandbox device (the parent of the core).
>
> Cheers,
> Abdellatif
>
> >
> > >
> > > log_info("[FFA] Conduit is SMC\n");
> > >
> > > @@ -1246,4 +1277,7 @@ U_BOOT_DRIVER(arm_ffa) = {
> > > .remove = ffa_remove,
> > > .unbind = ffa_unbind,
> > > .ops = &ffa_ops,
> > > +#if CONFIG_IS_ENABLED(SANDBOX_FFA)
> > > + .bind = ffa_bind,
> >
> > Can drop this
> >
> > > +#endif
> > > };
> > > diff --git a/drivers/firmware/arm-ffa/sandbox.c b/drivers/firmware/arm-ffa/sandbox.c
> >
> > sandbox_emul.c pehaps?
> >
> > I am pretty sure this is an emulator
> >
> > > new file mode 100644
> > > index 0000000000..84c2fc929f
> > > --- /dev/null
> > > +++ b/drivers/firmware/arm-ffa/sandbox.c
> > > @@ -0,0 +1,610 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright 2022-2023 Arm Limited and/or its affiliates <open-source-office at arm.com>
> > > + *
> > > + * Authors:
> > > + * Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> > > + */
> > > +
> > > +#include <common.h>
> > > +#include <dm.h>
> > > +#include <mapmem.h>
> > > +#include <string.h>
> > > +#include <asm/global_data.h>
> > > +#include <dm/device-internal.h>
> > > +#include <dm/lists.h>
> > > +#include <dm/root.h>
> > > +#include <linux/errno.h>
> > > +#include <linux/sizes.h>
> > > +#include "sandbox_arm_ffa_priv.h"
> > > +
> > > +DECLARE_GLOBAL_DATA_PTR;
> > > +
> > > +/* The partitions (SPs) table */
> > > +static struct ffa_partition_desc sandbox_partitions[SANDBOX_PARTITIONS_CNT] = {
> > > + {
> > > + .info = { .id = SANDBOX_SP1_ID, .exec_ctxt = 0x5687, .properties = 0x89325621 },
> > > + .sp_uuid = {
> > > + .a1 = SANDBOX_SERVICE1_UUID_A1,
> > > + .a2 = SANDBOX_SERVICE1_UUID_A2,
> > > + .a3 = SANDBOX_SERVICE1_UUID_A3,
> > > + .a4 = SANDBOX_SERVICE1_UUID_A4,
> > > + }
> > > + },
> > > + {
> > > + .info = { .id = SANDBOX_SP2_ID, .exec_ctxt = 0x9587, .properties = 0x45325621 },
> > > + .sp_uuid = {
> > > + .a1 = SANDBOX_SERVICE2_UUID_A1,
> > > + .a2 = SANDBOX_SERVICE2_UUID_A2,
> > > + .a3 = SANDBOX_SERVICE2_UUID_A3,
> > > + .a4 = SANDBOX_SERVICE2_UUID_A4,
> > > + }
> > > + },
> > > + {
> > > + .info = { .id = SANDBOX_SP3_ID, .exec_ctxt = 0x7687, .properties = 0x23325621 },
> > > + .sp_uuid = {
> > > + .a1 = SANDBOX_SERVICE1_UUID_A1,
> > > + .a2 = SANDBOX_SERVICE1_UUID_A2,
> > > + .a3 = SANDBOX_SERVICE1_UUID_A3,
> > > + .a4 = SANDBOX_SERVICE1_UUID_A4,
> > > + }
> > > + },
> > > + {
> > > + .info = { .id = SANDBOX_SP4_ID, .exec_ctxt = 0x1487, .properties = 0x70325621 },
> > > + .sp_uuid = {
> > > + .a1 = SANDBOX_SERVICE2_UUID_A1,
> > > + .a2 = SANDBOX_SERVICE2_UUID_A2,
> > > + .a3 = SANDBOX_SERVICE2_UUID_A3,
> > > + .a4 = SANDBOX_SERVICE2_UUID_A4,
> > > + }
> > > + }
> > > +
> > > +};
> > > +
> > > +/* Driver functions */
> > > +
> > > +/**
> > > + * sandbox_ffa_version() - Emulated FFA_VERSION handler function
> > > + * @dev: the sandbox FF-A device
> > > + * @{a0-a7} , res: The SMC call arguments and return structure.
> > > + *
> > > + * This is the function that emulates FFA_VERSION FF-A function.
> > > + *
> > > + * Return:
> > > + *
> > > + * 0 on success. Otherwise, failure
> > > + */
> > > +SANDBOX_SMC_FFA_ABI(ffa_version)
> >
> > Instead of the macro can you write this out in full? It defeats ctags, etc.
> >
> > > +{
> > > + struct sandbox_ffa_priv *priv = dev_get_priv(dev);
> > > +
> > > + priv->fwk_version = FFA_VERSION_1_0;
> > > + res->a0 = priv->fwk_version;
> >
> > Where is res defined?
> >
> > > +
> > > + /* x1-x7 MBZ */
> > > + memset(FFA_X1X7_MBZ_REG_START, 0, FFA_X1X7_MBZ_CNT * sizeof(unsigned long));
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * sandbox_ffa_id_get() - Emulated FFA_ID_GET handler function
> > > + * @dev: the sandbox FF-A device
> > > + * @{a0-a7} , res: The SMC call arguments and return structure.
> > > + *
> > > + * This is the function that emulates FFA_ID_GET FF-A function.
> >
> > s/This is the function that e/E/g
> >
> > > + *
> > > + * Return:
> > > + *
> > > + * 0 on success. Otherwise, failure
> > > + */
> > > +SANDBOX_SMC_FFA_ABI(ffa_id_get)
> > > +{
> > > + struct sandbox_ffa_priv *priv = dev_get_priv(dev);
> > > +
> > > + res->a0 = FFA_SMC_32(FFA_SUCCESS);
> > > + res->a1 = 0;
> > > +
> > > + priv->id = NS_PHYS_ENDPOINT_ID;
> > > + res->a2 = priv->id;
> > > +
> > > + /* x3-x7 MBZ */
> > > + memset(FFA_X3_MBZ_REG_START, 0, FFA_X3X7_MBZ_CNT * sizeof(unsigned long));
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * sandbox_ffa_features() - Emulated FFA_FEATURES handler function
> > > + * @dev: the sandbox FF-A device
> > > + * @{a0-a7} , res: The SMC call arguments and return structure.
> > > + *
> > > + * This is the function that emulates FFA_FEATURES FF-A function.
> > > + *
> > > + * Return:
> > > + *
> > > + * 0 on success. Otherwise, failure
> > > + */
> > > +SANDBOX_SMC_FFA_ABI(ffa_features)
> > > +{
> > > + if (pargs->a1 == FFA_SMC_64(FFA_RXTX_MAP)) {
> > > + res->a0 = FFA_SMC_32(FFA_SUCCESS);
> > > + res->a2 = RXTX_BUFFERS_MIN_SIZE;
> > > + res->a3 = 0;
> > > + /* x4-x7 MBZ */
> > > + memset(FFA_X4X7_MBZ_REG_START,
> > > + 0, FFA_X4X7_MBZ_CNT * sizeof(unsigned long));
> > > + } else {
> > > + res->a0 = FFA_SMC_32(FFA_ERROR);
> > > + res->a2 = FFA_ERR_STAT_NOT_SUPPORTED;
> > > + /* x3-x7 MBZ */
> > > + memset(FFA_X3_MBZ_REG_START,
> > > + 0, FFA_X3X7_MBZ_CNT * sizeof(unsigned long));
> > > + log_err("[FFA] [Sandbox] FF-A interface 0x%lx not implemented\n", pargs->a1);
> >
> > return -ENOSYS
> >
> > > + }
> > > +
> > > + res->a1 = 0;
> > > +
> > > + return 0;
> > > +}
> > > +
> >
> > [..]
> >
> > > +SANDBOX_SMC_FFA_ABI(ffa_msg_send_direct_req)
> > > +{
> > > + u16 part_id;
> > > + struct sandbox_ffa_priv *priv = dev_get_priv(dev);
> > > +
> > > + part_id = GET_DST_SP_ID(pargs->a1);
> > > +
> > > + if ((GET_NS_PHYS_ENDPOINT_ID(pargs->a1) != priv->id) ||
> >
> > drop extra brackets
> >
> > > + !sandbox_ffa_sp_valid(dev, part_id) || pargs->a2) {
> > > + res->a0 = FFA_SMC_32(FFA_ERROR);
> > > + res->a1 = 0;
> > > + res->a2 = FFA_ERR_STAT_INVALID_PARAMETERS;
> > > +
> > > + /* x3-x7 MBZ */
> > > + memset(FFA_X3_MBZ_REG_START,
> > > + 0, FFA_X3X7_MBZ_CNT * sizeof(unsigned long));
> > > +
> > > + return 0;
> > > + }
> > > +
> > > + res->a0 = FFA_SMC_64(FFA_MSG_SEND_DIRECT_RESP);
> > > +
> > > + res->a1 = PREP_SRC_SP_ID(part_id) |
> > > + PREP_NS_PHYS_ENDPOINT_ID(priv->id);
> > > +
> > > + res->a2 = 0;
> > > +
> > > + /* Return 0xff bytes as a response */
> > > + res->a3 = 0xffffffffffffffff;
> > > + res->a4 = 0xffffffffffffffff;
> > > + res->a5 = 0xffffffffffffffff;
> > > + res->a6 = 0xffffffffffffffff;
> > > + res->a7 = 0xffffffffffffffff;
> >
> > Would -1UL work?
> >
> > > +
> > > + return 0;
> > > +}
> > > +
> >
> > [..]
> >
> > > +static int sandbox_ffa_probe(struct udevice *dev)
> > > +{
> > > + struct udevice *child_dev = NULL;
> > > + int ret;
> > > +
> > > + ret = device_bind_driver(dev, FFA_DRV_NAME, FFA_DRV_NAME, &child_dev);
> >
> > Is this binding the emulator? Add it to the DT instead.
> >
> > If you put
> >
> > .post_bind = dm_scan_fdt_dev()
> >
> > in your uclass (like spi-uclass.c does) then it will binding
> > child-node devices automatically. Then you can make your emulator a
> > child node.
> >
> > > + if (ret) {
> > > + pr_err("%s was not bound: %d, aborting\n", FFA_DRV_NAME, ret);
> > > + return -ENODEV;
> > > + }
> > > +
> > > + dev_set_parent_plat(child_dev, dev_get_plat(dev));
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct udevice_id sandbox_ffa_id[] = {
> > > + { "sandbox,arm_ffa", 0 },
> > > + { },
> > > +};
> > > +
> > > +/* Declaring the sandbox_arm_ffa driver under UCLASS_FFA */
> > > +U_BOOT_DRIVER(sandbox_arm_ffa) = {
> > > + .name = FFA_SANDBOX_DRV_NAME,
> > > + .of_match = sandbox_ffa_id,
> > > + .id = UCLASS_FFA,
> > > + .probe = sandbox_ffa_probe,
> > > + .priv_auto = sizeof(struct sandbox_ffa_priv),
> > > +};
> > > diff --git a/drivers/firmware/arm-ffa/sandbox_arm_ffa_priv.h b/drivers/firmware/arm-ffa/sandbox_arm_ffa_priv.h
> > > new file mode 100644
> > > index 0000000000..c35d80de16
> > > --- /dev/null
> > > +++ b/drivers/firmware/arm-ffa/sandbox_arm_ffa_priv.h
> > > @@ -0,0 +1,129 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * Copyright 2022-2023 Arm Limited and/or its affiliates <open-source-office at arm.com>
> > > + *
> > > + * Authors:
> > > + * Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> > > + */
> > > +
> > > +#ifndef __SANDBOX_ARM_FFA_PRV_H
> > > +#define __SANDBOX_ARM_FFA_PRV_H
> > > +
> > > +#include <sandbox_arm_ffa.h>
> > > +#include "arm_ffa_priv.h"
> > > +
> > > +/* This header is exclusively used by the Sandbox FF-A driver and sandbox tests */
> > > +
> > > +/* FF-A core driver name */
> > > +#define FFA_SANDBOX_DRV_NAME "sandbox_arm_ffa"
> > > +
> > > +/* FF-A ABIs internal error codes (as defined by the spec) */
> > > +
> > > +#define FFA_ERR_STAT_NOT_SUPPORTED -1
> > > +#define FFA_ERR_STAT_INVALID_PARAMETERS -2
> > > +#define FFA_ERR_STAT_NO_MEMORY -3
> > > +#define FFA_ERR_STAT_BUSY -4
> > > +#define FFA_ERR_STAT_DENIED -6
> >
> > It's a convention to put negative values in brackets just in case of
> > operator-precedence issues. Or you could use an enum.
> >
> > > +
> > > +/* Non-secure physical FF-A instance */
> > > +#define NS_PHYS_ENDPOINT_ID (0)
> > > +
> > > +#define GET_NS_PHYS_ENDPOINT_ID_MASK GENMASK(31, 16)
> > > +#define GET_NS_PHYS_ENDPOINT_ID(x) \
> > > + ((u16)(FIELD_GET(GET_NS_PHYS_ENDPOINT_ID_MASK, (x))))
> > > +
> > > +/* Helper macro for reading the destination partition ID */
> > > +#define GET_DST_SP_ID_MASK GENMASK(15, 0)
> > > +#define GET_DST_SP_ID(x) \
> > > + ((u16)(FIELD_GET(GET_DST_SP_ID_MASK, (x))))
> > > +
> > > +/* Helper macro for setting the source partition ID */
> > > +#define PREP_SRC_SP_ID_MASK GENMASK(31, 16)
> > > +#define PREP_SRC_SP_ID(x) \
> > > + (FIELD_PREP(PREP_SRC_SP_ID_MASK, (x)))
> > > +
> > > +/* Helper macro for setting the destination endpoint ID */
> > > +#define PREP_NS_PHYS_ENDPOINT_ID_MASK GENMASK(15, 0)
> > > +#define PREP_NS_PHYS_ENDPOINT_ID(x) \
> > > + (FIELD_PREP(PREP_NS_PHYS_ENDPOINT_ID_MASK, (x)))
> > > +
> > > +/* RX/TX buffers minimum size */
> > > +#define RXTX_BUFFERS_MIN_SIZE (RXTX_4K)
> > > +#define RXTX_BUFFERS_MIN_PAGES (1)
> > > +
> > > +/* MBZ registers info */
> > > +
> > > +/* x1-x7 MBZ */
> > > +#define FFA_X1X7_MBZ_CNT (7)
> > > +#define FFA_X1X7_MBZ_REG_START (&res->a1)
> > > +
> > > +/* x4-x7 MBZ */
> > > +#define FFA_X4X7_MBZ_CNT (4)
> > > +#define FFA_X4X7_MBZ_REG_START (&res->a4)
> > > +
> > > +/* x3-x7 MBZ */
> > > +#define FFA_X3X7_MBZ_CNT (5)
> > > +#define FFA_X3_MBZ_REG_START (&res->a3)
> > > +
> > > +/* number of secure partitions emulated by the FF-A sandbox driver */
> > > +#define SANDBOX_PARTITIONS_CNT (4)
> > > +
> > > +/* Binary data of services UUIDs emulated by the FF-A sandbox driver */
> > > +
> > > +/* service 1 UUID binary data (little-endian format) */
> > > +#define SANDBOX_SERVICE1_UUID_A1 0xed32d533
> > > +#define SANDBOX_SERVICE1_UUID_A2 0x99e64209
> > > +#define SANDBOX_SERVICE1_UUID_A3 0x9cc02d72
> > > +#define SANDBOX_SERVICE1_UUID_A4 0xcdd998a7
> > > +
> > > +/* service 2 UUID binary data (little-endian format) */
> > > +#define SANDBOX_SERVICE2_UUID_A1 0xed32d544
> > > +#define SANDBOX_SERVICE2_UUID_A2 0x99e64209
> > > +#define SANDBOX_SERVICE2_UUID_A3 0x9cc02d72
> > > +#define SANDBOX_SERVICE2_UUID_A4 0xcdd998a7
> > > +
> > > +/**
> > > + * struct ffa_rxtxpair_info - structure hosting the RX/TX buffers flags
> > > + * @rxbuf_owned: RX buffer ownership flag (the owner is non secure world: the consumer)
> > > + * @rxbuf_mapped: RX buffer mapping flag
> > > + * @txbuf_owned TX buffer ownership flag
> > > + * @txbuf_mapped: TX buffer mapping flag
> > > + * @rxtx_buf_size: RX/TX buffers size as set by the FF-A core driver
> > > + *
> > > + * Data structure hosting the ownership/mapping flags of the RX/TX buffers
> > > + * When a buffer is owned/mapped its corresponding flag is set to 1 otherwise 0.
> > > + */
> > > +struct ffa_rxtxpair_info {
> > > + u8 rxbuf_owned;
> > > + u8 rxbuf_mapped;
> > > + u8 txbuf_owned;
> > > + u8 txbuf_mapped;
> > > + u32 rxtx_buf_size;
> > > +};
> > > +
> > > +/**
> > > + * struct sandbox_ffa_priv - the driver private data structure
> > > + *
> > > + * @dev: The arm_ffa device under u-boot driver model
> > > + * @fwk_version: FF-A framework version
> > > + * @id: u-boot endpoint ID
> > > + * @partitions: The partitions descriptors structure
> > > + * @pair: The RX/TX buffers pair
> > > + * @pair_info: The RX/TX buffers pair flags and size
> > > + * @conduit: The selected conduit
> > > + *
> > > + * The driver data structure hosting all the emulated secure world data.
> > > + */
> > > +struct sandbox_ffa_priv {
> > > + struct udevice *dev;
> > > + u32 fwk_version;
> > > + u16 id;
> > > + struct ffa_partitions partitions;
> > > + struct ffa_rxtxpair pair;
> > > + struct ffa_rxtxpair_info pair_info;
> > > +};
> > > +
> > > +#define SANDBOX_SMC_FFA_ABI(ffabi) static int sandbox_##ffabi(struct udevice *dev, \
> > > + ffa_value_t *pargs, ffa_value_t *res)
> >
> > drop that
> >
> > [..]
Regards,
Simon
More information about the U-Boot
mailing list