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 42CF7C6FD1D for ; Wed, 15 Mar 2023 14:05:53 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id A31DD8582C; Wed, 15 Mar 2023 15:05:49 +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="XnoTunPg"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 532CF85602; Wed, 15 Mar 2023 15:05:47 +0100 (CET) Received: from mail-ed1-x536.google.com (mail-ed1-x536.google.com [IPv6:2a00:1450:4864:20::536]) (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 134EB85D8A for ; Wed, 15 Mar 2023 15:05:32 +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-ed1-x536.google.com with SMTP id cy23so75895702edb.12 for ; Wed, 15 Mar 2023 07:05:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1678889131; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=JjKh4lICjT6qE0Rrr0Uj3F1vvW7RhLMnACX7a3ZWW3A=; b=XnoTunPgUPvdWMq7iIh+wE4FzFOzQ/dvgv5uwSzl4KZ5Zlz/hesQWkKjyW2O7vrz07 /tVHdc8LJnGk2t4Tbd/9TnF+h7+Rh0i0JK3gPZ/VWs5Nx4ll2+9jtMP/usdym3MGgOmb HjrUrqfKHUvxMUYiQEfnvSqvhEAIRBTZ+mWHc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678889131; 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=JjKh4lICjT6qE0Rrr0Uj3F1vvW7RhLMnACX7a3ZWW3A=; b=At3xZsuV2dlppIwmtUNN7xBUtLIFk/DKUysaeIjbTlIo4Oxtz54pYeEHUkQHdCsQVw FVscZhfmEEQ6dgqAZ3IWJ+ztMTLJ30ph4w7NVr52+9dL86Eq8PjQOnBd5B09Wl41tDpq jguPxLWG4FtksDDWM8m9jgQ+hkT/ZXjQN0J8G2P8ZPq3+VeubFlJQbj6msIBqcmE3GWT NYH4fkqPFKGcmbcFOG/yRGvH8zS5O6c2nGUW9p7yBDg9V+g+nmN+6wdc2JEIJthsDmro LxMl8N/ZOScnCm9+4GXRY+Zt/wvxkNzTe6my0wAjFw9UrnbmyaFz/ZAQuk+djXkv7R42 Hl+A== X-Gm-Message-State: AO0yUKWhA82aFhTc9wbetBDJ8HNLqxXDLdrW+BNjMyVBMUSh56LMMFSw Vg2XEP84CySiq99z7m9K/XZvYI5q6hlwDOKWuXqhREefdyyWlbdwA0Q= X-Google-Smtp-Source: AK7set/pTtGoEdSHVhejIQnDxXOr6i+o7pAxRz9kR2MCKAJwkwy7detP71W5awoQd8YbXu3jfGWC+57bTZtWfmJL14k= X-Received: by 2002:a17:907:96a9:b0:92e:5db0:666a with SMTP id hd41-20020a17090796a900b0092e5db0666amr2100572ejc.14.1678889131102; Wed, 15 Mar 2023 07:05:31 -0700 (PDT) MIME-Version: 1.0 References: <20230310141016.137986-1-abdellatif.elkhlifi@arm.com> <20230310141016.137986-7-abdellatif.elkhlifi@arm.com> <20230314175911.GA87607@e130802.arm.com> In-Reply-To: <20230314175911.GA87607@e130802.arm.com> From: Simon Glass Date: Wed, 15 Mar 2023 08:05:19 -0600 Message-ID: Subject: Re: [PATCH v9 06/10] arm_ffa: introduce the FF-A Sandbox driver To: Abdellatif El Khlifi Cc: xueliang.zhong@arm.com, nd@arm.com, u-boot@lists.denx.de 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 Tue, 14 Mar 2023 at 11:59, Abdellatif El Khlifi wrote: > > Hi Simon, > > > Hi Abdellatif, > > > > On Fri, 10 Mar 2023 at 06:10, Abdellatif El Khlifi > > 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 > > > Cc: Tom Rini > > > Cc: Simon Glass > > > Cc: Ilias Apalodimas > > > Cc: Jens Wiklander > > > > > > --- > > > 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 > > > 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 > > > > > > 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 > > > + * > > > + * Authors: > > > + * Abdellatif El Khlifi > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#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 > > > + * > > > + * Authors: > > > + * Abdellatif El Khlifi > > > + */ > > > + > > > +#ifndef __SANDBOX_ARM_FFA_PRV_H > > > +#define __SANDBOX_ARM_FFA_PRV_H > > > + > > > +#include > > > +#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