Re: [PATCH v8 06/10] arm_ffa: introduce the FF-A Sandbox driver

Simon Glass
2022-11-23T02:09:52Z

Hi Abdellatif,

On Tue, 22 Nov 2022 at 06:18, Abdellatif El Khlifi
<abdellatif.elkhlifi@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@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>
>
> ---
>
> Changelog:
> ===============
>
> 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 +
> configs/sandbox64_defconfig | 2 +
> configs/sandbox_defconfig | 2 +
> doc/arch/sandbox/sandbox.rst | 1 +
> drivers/firmware/arm-ffa/Kconfig | 9 +-
> drivers/firmware/arm-ffa/Makefile | 1 +
> drivers/firmware/arm-ffa/arm_ffa_prv.h | 15 +-

Can those changes be done in the previous patch where you introduced this file?

> drivers/firmware/arm-ffa/core.c | 22 +-
> drivers/firmware/arm-ffa/sandbox.c | 659 ++++++++++++++++++
> .../firmware/arm-ffa/sandbox_arm_ffa_prv.h | 144 ++++
> include/arm_ffa.h | 2 +-
> include/sandbox_arm_ffa.h | 91 +++
> lib/efi_loader/efi_boottime.c | 2 +-
> 13 files changed, 938 insertions(+), 13 deletions(-)
> create mode 100644 drivers/firmware/arm-ffa/sandbox.c
> create mode 100644 drivers/firmware/arm-ffa/sandbox_arm_ffa_prv.h
> create mode 100644 include/sandbox_arm_ffa.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index aa4e87d9f8..9197344df4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -270,6 +270,7 @@ F: cmd/armffa.c
> F: doc/arch/arm64.ffa.rst
> F: drivers/firmware/arm-ffa/
> F: include/arm_ffa.h
> +F: include/sandbox_arm_ffa.h
>
> ARM FREESCALE IMX
> M: Stefano Babic <sbabic@denx.de>
> diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> index cff166b1c1..141ffd1f85 100644
> --- a/configs/sandbox64_defconfig
> +++ b/configs/sandbox64_defconfig
> @@ -258,3 +258,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
> \ No newline at end of file

Please add one

[..]

> diff --git a/drivers/firmware/arm-ffa/core.c b/drivers/firmware/arm-ffa/core.c
> index 0b1f8e6a07..560603b28b 100644
> --- a/drivers/firmware/arm-ffa/core.c
> +++ b/drivers/firmware/arm-ffa/core.c
> @@ -1072,6 +1072,7 @@ static int ffa_msg_send_direct_req(struct udevice *dev, u16 dst_part_id,
> return ffa_to_std_errno(ffa_errno);
> }
>
> +#if !CONFIG_IS_ENABLED(SANDBOX_FFA)
> /**
> * __arm_ffa_fn_smc - SMC wrapper
> * @args: FF-A ABI arguments to be copied to Xn registers
> @@ -1085,6 +1086,7 @@ void __arm_ffa_fn_smc(ffa_value_t args, ffa_value_t *res)
> {
> arm_smccc_1_2_smc(&args, res);
> }
> +#endif
>
> /**
> * ffa_set_smc_conduit - Set the SMC conduit
> @@ -1098,7 +1100,12 @@ void __arm_ffa_fn_smc(ffa_value_t args, ffa_value_t *res)
> */
> static int ffa_set_smc_conduit(void)
> {
> - ffa_priv_data->invoke_ffa_fn = __arm_ffa_fn_smc;
> +#if CONFIG_IS_ENABLED(SANDBOX_FFA)
> + ffa_priv_data->invoke_ffa_fn = sandbox_arm_ffa_smccc_smc;
> + ffa_info("Using SMC emulation");
> +#else
> + ffa_priv_data->invoke_ffa_fn = __arm_ffa_fn_smc;
> +#endif
>
> if (!ffa_priv_data->invoke_ffa_fn) {
> ffa_err("failure to set the invoke function");
> @@ -1275,17 +1282,18 @@ struct ffa_prvdata *ffa_bus_prvdata_get(void)
> }
>
> /**
> - * ffa_bus_discover - discover FF-A bus and probe arm_ffa device
> + * ffa_bus_discover - discover FF-A bus and probe arm_ffa and sandbox_arm_ffa devices
> * @pdev: the address of a device pointer (to be filled when the arm_ffa bus device is created
> * successfully)
> *
> * This function makes sure the FF-A bus is discoverable.
> - * When probing succeeds FF-A discovery is done. The arm_ffa device is ready to use.
> + * When probing succeeds FF-A discovery is done. The arm_ffa and sandbox_arm_ffa devices
> + * are ready to use.
> *
> * When the bus was already discovered successfully the discovery will not run again.
> *
> * Arm FF-A transport is implemented through arm_ffa u-boot device managing the FF-A
> - * communication.
> + * communication. In Sandbox mode sandbox_arm_ffa is used to test arm_ffa driver.
> * All FF-A clients should use the arm_ffa device to use the FF-A transport.
> *
> * Return:
> @@ -1299,6 +1307,12 @@ int ffa_bus_discover(struct udevice **pdev)
> if (!ffa_priv_data) {
> ret = ffa_device_get(pdev);

Against this needs to use driver model properly. There appears to be
no actual device??

>
> +#if CONFIG_IS_ENABLED(SANDBOX_FFA)
> + if (ret == 0)
> + ret = sandbox_ffa_device_get();
> +#endif
> + }
> +
> return ret;
> }
>
> diff --git a/drivers/firmware/arm-ffa/sandbox.c b/drivers/firmware/arm-ffa/sandbox.c
> new file mode 100644
> index 0000000000..16f1ca926e
> --- /dev/null
> +++ b/drivers/firmware/arm-ffa/sandbox.c
> @@ -0,0 +1,659 @@

This seems OK, but I am still confused as to why the device handling
is done outside driver model.

[..]

Regards,
Simon