Re: [PATCH v11 04/10] arm_ffa: introduce Arm FF-A support

Simon Glass
2023-04-19T01:53:00Z

Hi Abdellatif,

On Wed, 12 Apr 2023 at 03:43, Abdellatif El Khlifi
<abdellatif.elkhlifi@arm.com> wrote:
>
> Add Arm FF-A support implementing Arm Firmware Framework for Armv8-A v1.0
>
> The Firmware Framework for Arm A-profile processors (FF-A v1.0) [1]
> describes interfaces (ABIs) that standardize communication
> between the Secure World and Normal World leveraging TrustZone
> technology.
>
> This driver uses 64-bit registers as per SMCCCv1.2 spec and comes
> on top of the SMCCC layer. The driver provides the FF-A ABIs needed for
> querying the FF-A framework from the secure world.
>
> The driver uses SMC32 calling convention which means using the first
> 32-bit data of the Xn registers.
>
> All supported ABIs come with their 32-bit version except FFA_RXTX_MAP
> which has 64-bit version supported.
>
> Both 32-bit and 64-bit direct messaging are supported which allows both
> 32-bit and 64-bit clients to use the FF-A bus.
>
> FF-A is a discoverable bus and similar to architecture features.
> FF-A bus is discovered using ARM_SMCCC_FEATURES mechanism performed
> by the PSCI driver.
>
> Clients are able to probe then use the FF-A bus by calling the DM class
> searching APIs (e.g: uclass_first_device).
>
> The Secure World is considered as one entity to communicate with
> using the FF-A bus. FF-A communication is handled by one device and
> one instance (the bus). This FF-A driver takes care of all the
> interactions between Normal world and Secure World.
>
> The driver exports its operations to be used by upper layers.
>
> Exported operations:
>
> - ffa_partition_info_get
> - ffa_sync_send_receive
> - ffa_rxtx_unmap
>
> Generic FF-A methods are implemented in the Uclass (arm-ffa-uclass.c).
> Arm specific methods are implemented in the Arm driver (arm-ffa.c).
>
> For more details please refer to the driver documentation [2].
>
> [1]: https://developer.arm.com/documentation/den0077/latest/
> [2]: doc/arch/arm64.ffa.rst
>
> 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:
> ===============
>
> v11:
>
> * move ffa_try_discovery() from the uclass to the Arm FF-A driver
> * rename ffa_try_discovery() to arm_ffa_discover()
> * pass dev as an argument of arm_ffa_discover()
> * add arm_ prefix to the Arm FF-A driver functions
> * add emul field in struct ffa_discovery_info
> * address nits
>
> v10:
>
> * provide the driver operations through the Uclass
> * move the generic FF-A methods to the Uclass
> * keep Arm specific methods in the Arm driver (arm-ffa.c)
> * rename core.c to arm-ffa.c
> * address nits
>
> v9:
>
> * integrate the FF-A bus discovery in the DM and use ARM_SMCCC_FEATURES for binding
>
> v8:
>
> * make ffa_get_partitions_info() second argument to be an SP count in both
> modes
> * update ffa_bus_prvdata_get() to return a pointer rather than a pointer
> address
> * remove packing from ffa_partition_info and ffa_send_direct_data structures
> * pass the FF-A bus device to the bus operations
>
> v7:
>
> * add support for 32-bit direct messaging
> * rename be_uuid_str_to_le_bin() to uuid_str_to_le_bin()
> * improve the declaration of error handling mapping
> * stating in doc/arch/arm64.ffa.rst that EFI runtime is not supported
>
> v6:
>
> * drop use of EFI runtime support (We decided with Linaro to add this later)
> * drop discovery from initcalls (discovery will be on demand by FF-A users)
> * set the alignment of the RX/TX buffers to the larger translation granule size
> * move FF-A RX/TX buffers unmapping at ExitBootServices() to a separate commit
> * update the documentation and move it to doc/arch/arm64.ffa.rst
>
> v4:
>
> * add doc/README.ffa.drv
> * moving the FF-A driver work to drivers/firmware/arm-ffa
> * use less #ifdefs in lib/efi_loader/efi_boottime.c and replace
> #if defined by #if CONFIG_IS_ENABLED
> * improving error handling by mapping the FF-A errors to standard errors
> and logs
> * replacing panics with an error log and returning an error code
> * improving features discovery in FFA_FEATURES by introducing
> rxtx_min_pages private data field
> * add ffa_remove and ffa_unbind functions
> * improve how the driver behaves when bus discovery is done more than
> once
>
> v3:
>
> * align the interfaces of the U-Boot FF-A driver with those in the linux
> FF-A driver
> * remove the FF-A helper layer
> * make the U-Boot FF-A driver independent from EFI
> * provide an optional config that enables copying the driver data to EFI
> runtime section at ExitBootServices service
> * use 64-bit version of FFA_RXTX_MAP, FFA_MSG_SEND_DIRECT_{REQ, RESP}
>
> v2:
>
> * make FF-A bus discoverable using device_{bind, probe} APIs
> * remove device tree support
>
> v1:
>
> * introduce FF-A bus driver with device tree support
>
> MAINTAINERS | 8 +
> doc/arch/arm64.ffa.rst | 271 ++++
> doc/arch/index.rst | 1 +
> drivers/Makefile | 1 +
> drivers/firmware/Kconfig | 1 +
> drivers/firmware/arm-ffa/Kconfig | 36 +
> drivers/firmware/arm-ffa/Makefile | 8 +
> drivers/firmware/arm-ffa/arm-ffa-uclass.c | 1143 +++++++++++++++++
> drivers/firmware/arm-ffa/arm-ffa.c | 127 ++
> .../firmware/arm-ffa/sandbox_arm_ffa_priv.h | 14 +
> include/arm_ffa.h | 217 ++++
> include/arm_ffa_priv.h | 271 ++++
> include/dm/uclass-id.h | 6 +
> 13 files changed, 2104 insertions(+)
> create mode 100644 doc/arch/arm64.ffa.rst
> create mode 100644 drivers/firmware/arm-ffa/Kconfig
> create mode 100644 drivers/firmware/arm-ffa/Makefile
> create mode 100644 drivers/firmware/arm-ffa/arm-ffa-uclass.c
> create mode 100644 drivers/firmware/arm-ffa/arm-ffa.c
> create mode 100644 drivers/firmware/arm-ffa/sandbox_arm_ffa_priv.h
> create mode 100644 include/arm_ffa.h
> create mode 100644 include/arm_ffa_priv.h
>

Reviewed-by: Simon Glass <sjg@chromium.org>
nits below

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 086e4cb3df..76f0f276ce 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -266,6 +266,14 @@ F: drivers/net/cortina_ni.h
> F: drivers/net/phy/ca_phy.c
> F: configs/cortina_presidio-asic-pnand_defconfig
>
> +ARM FF-A
> +M: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> +S: Maintained
> +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>
> M: Fabio Estevam <festevam@gmail.com>
> diff --git a/doc/arch/arm64.ffa.rst b/doc/arch/arm64.ffa.rst
> new file mode 100644
> index 0000000000..466e77e3cd
> --- /dev/null
> +++ b/doc/arch/arm64.ffa.rst
> @@ -0,0 +1,271 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +
> +Arm FF-A Support
> +================
> +
> +Summary
> +-------
> +
> +FF-A stands for Firmware Framework for Arm A-profile processors.
> +
> +FF-A specifies interfaces that enable a pair of software sandboxes to

This is confusing as we use 'sandbox' in U-Boot to mean something else.

To me, a sandbox is a play area where you can fiddle around, like kids
in a sandbox.

It is widely used though, to mean a secure environment, which is
unfortunate. I think a 'jail' or 'secure container' container would be
a better name.

Anyway, for here, could you just put in brackets:

(not to be confused with the U-Boot sandbox build)

or something like that.

> +communicate with each other. A sandbox aka partition could
> +be a VM in the Normal or Secure world, an application in S-EL0, or a
> +Trusted OS in S-EL1.
> +
> +The U-Boot FF-A support (the bus) implements the interfaces to communicate
> +with partitions in the Secure world aka Secure partitions (SPs).
> +
> +The FF-A support specifically focuses on communicating with SPs that
> +isolate portions of EFI runtime services that must run in a protected
> +environment which is inaccessible by the Host OS or Hypervisor.
> +Examples of such services are set/get variables.
> +
> +The FF-A support uses the SMC ABIs defined by the FF-A specification to:
> +
> +- Discover the presence of SPs of interest
> +- Access an SP's service through communication protocols
> + e.g. EFI MM communication protocol
> +
> +At this stage of development only EFI boot time is supported.

How about: 'EFI boot-time services' are supported

> +Runtime support will be added in future developments.
> +
> +The U-Boot FF-A support provides the following parts:
> +
> +- A Uclass driver providing generic FF-A methods.
> +- An Arm FF-A driver providing Arm specific methods and reusing the Uclass methods.

Arm-specific

> +
> +FF-A and SMC specifications
> +-------------------------------------------
> +
> +The current implementation of the U-Boot FF-A support relies on FF-A
> +specification v1.0

Can you put a nice link here so people can click on it? You can do that in rST.

and uses SMC32 calling convention which means using
> +the first 32-bit data of the Xn registers.
> +
> +At this stage we only need the FF-A v1.0 features.
> +
> +The FF-A support has been tested with OP-TEE which supports SMC32 calling
> +convention.
> +
> +For more details please refer to the FF-A v1.0 spec:
> +https://documentation-service.arm.com/static/5fb7e8a6ca04df4095c1d65e?token=

same here (and drop the ?token=)
> +
> +Hypervisors are supported if they are configured to trap SMC calls.
> +
> +The FF-A support uses 64-bit registers as per SMCCCv1.2 specification.
> +
> +For more details please refer to the SMC Calling Convention v1.2 spec:
> +https://documentation-service.arm.com/static/5f8edaeff86e16515cdbe4c6?token=

same here

> +
> +Supported hardware
> +--------------------------------
> +
> +Aarch64 plaforms
> +
> +Configuration
> +----------------------
> +
> +CONFIG_ARM_FFA_TRANSPORT
> + Enables the FF-A support. Turn this on if you want to use FF-A
> + communication.
> + When using an Arm 64-bit platform, the Arm FF-A driver will be used.
> +
> +FF-A ABIs under the hood
> +---------------------------------------
> +
> +Invoking an FF-A ABI involves providing to the secure world/hypervisor the
> +expected arguments from the ABI.
> +
> +On an Arm 64-bit platform, the ABI arguments are stored in x0 to x7 registers.
> +Then, an SMC instruction is executed.
> +
> +At the secure side level or hypervisor the ABI is handled at a higher exception
> +level and the arguments are read and processed.
> +
> +The response is put back through x0 to x7 registers and control is given back
> +to the U-Boot Arm FF-A driver (non-secure world).
> +
> +The driver reads the response and processes it accordingly.
> +
> +This methodology applies to all the FF-A ABIs.
> +
> +FF-A bus discovery on Arm 64-bit platforms
> +---------------------------------------------
> +
> +When CONFIG_ARM_FFA_TRANSPORT is enabled, the FF-A bus is considered as
> +an architecture feature and discovered using ARM_SMCCC_FEATURES mechanism.
> +This discovery mechanism is performed by the PSCI driver.
> +
> +The PSCI driver comes with a PSCI device tree node which is the root node for all
> +architecture features including FF-A bus.
> +
> +::
> +
> + => dm tree
> +
> + Class Index Probed Driver Name
> + -----------------------------------------------------------
> + ...
> + firmware 0 [ + ] psci |-- psci
> + ffa 0 [ ] arm_ffa | `-- arm_ffa
> + ...
> +
> +The PSCI driver is bound to the PSCI device and when probed it tries to discover
> +the architecture features by calling a callback the features drivers provide.
> +
> +In case of FF-A, the callback is arm_ffa_is_supported() which tries to discover the
> +FF-A framework by querying the FF-A framework version from secure world using
> +FFA_VERSION ABI. When discovery is successful, the ARM_SMCCC_FEATURES
> +mechanism creates a U-Boot device for the FF-A bus and binds the Arm FF-A driver
> +with the device using device_bind_driver().
> +
> +At this stage the FF-A bus is registered with the DM and can be interacted with using
> +the DM APIs.
> +
> +Clients are able to probe then use the FF-A bus by calling uclass_first_device().
> +Please refer to the armffa command implementation as an example of how to probe
> +and interact with the FF-A bus.
> +
> +When calling uclass_first_device(), the FF-A driver is probed and ends up calling
> +ffa_do_probe() provided by the Uclass which does the following:
> +
> + - updating uc_priv with discovery information
> + - querying from secure world the u-boot endpoint ID
> + - querying from secure world the supported features of FFA_RXTX_MAP
> + - mapping the RX/TX buffers
> + - querying from secure world all the partitions information
> +
> +When one of the above actions fails, probing fails and the driver stays not active
> +and can be probed again if needed.
> +
> +FF-A device destruction
> +-------------------------
> +
> +When the FF-A device is removed by the DM, RX/TX buffers are automatically
> +unmapped and freed.
> +
> +For example, at EFI efi_exit_boot_services() active devices are automatically removed
> +by dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL).
> +
> +By consequence, the FF-A RX/TX are unmapped automatically.
> +
> +Requirements for clients
> +-------------------------------------
> +
> +When using the FF-A bus with EFI, clients must query the SPs they are looking for
> +during EFI boot time mode using the service UUID.

boot-time

> +
> +The RX/TX buffers are only available at EFI boot time. Querying partitions is
> +done at boot time and data is cached for future use.
> +
> +RX/TX buffers should be unmapped before EFI runtime mode starts.
> +The driver provides a bus operation for that called ffa_rxtx_unmap() and this is done
> +automatically at efi_exit_boot_services().
> +
> +If RX/TX buffers created by U-Boot are not unmapped and by consequence becoming
> +available at EFI runtime, secure world will get confused about RX/TX buffers
> +ownership (U-Boot vs kernel).

Can you rewrite that a bit? It seems quite strange English.

> +
> +When invoking FF-A direct messaging, clients should specify which ABI protocol
> +they want to use (32-bit vs 64-bit). Selecting the protocol means using
> +the 32-bit or 64-bit version of FFA_MSG_SEND_DIRECT_{REQ, RESP}.
> +The calling convention between U-Boot and the secure world stays the same: SMC32.
> +
> +The bus driver layer
> +------------------------------
> +
> +FF-A support comes on top of the SMCCC layer and is implemented by the FF-A Uclass drivers/firmware/arm-ffa/arm-ffa-uclass.c
> +
> +The following features are provided:
> +
> +- Support for the 32-bit version of the following ABIs:
> +
> + - FFA_VERSION
> + - FFA_ID_GET
> + - FFA_FEATURES
> + - FFA_PARTITION_INFO_GET
> + - FFA_RXTX_UNMAP
> + - FFA_RX_RELEASE
> + - FFA_RUN
> + - FFA_ERROR
> + - FFA_SUCCESS
> + - FFA_INTERRUPT
> + - FFA_MSG_SEND_DIRECT_REQ
> + - FFA_MSG_SEND_DIRECT_RESP
> +
> +- Support for the 64-bit version of the following ABIs:
> +
> + - FFA_RXTX_MAP
> + - FFA_MSG_SEND_DIRECT_REQ
> + - FFA_MSG_SEND_DIRECT_RESP
> +
> +- Processing the received data from the secure world/hypervisor and caching it
> +
> +- Hiding from upper layers the FF-A protocol and registers details. Upper
> + layers focus on exchanged data, FF-A support takes care of how to transport
> + that to the secure world/hypervisor using FF-A
> +
> +- FF-A support provides driver operations to be used by upper layers:
> +
> + - ffa_partition_info_get
> + - ffa_sync_send_receive
> + - ffa_rxtx_unmap
> +
> +- FF-A bus discovery makes sure FF-A framework is responsive and compatible
> + with the driver
> +
> +- FF-A bus can be compiled and used without EFI
> +
> +Example of boot logs with FF-A enabled
> +--------------------------------------
> +
> +For example, when using FF-A with Corstone-1000 the logs are as follows:
> +
> +::
> +
> + U-Boot 2023.01 (Mar 27 2023 - 13:48:33 +0000) corstone1000 aarch64
> +
> + DRAM: 2 GiB
> + Arm FF-A framework discovery
> + Using Arm SMC for FF-A conduit
> + FF-A driver 1.0
> + FF-A framework 1.0
> + FF-A versions are compatible
> + ...
> + Hit any key to stop autoboot: 0
> + Loading kernel from 0x083EE000 to memory ...
> + ...
> + FF-A endpoint ID is 0
> + Using 1 4KB page(s) for FF-A RX/TX buffers size
> + FF-A RX buffer at virtual address 00000000fdf4f000
> + FF-A TX buffer at virtual address 00000000fdf51000
> + FF-A RX/TX buffers mapped
> + Reading FF-A partitions data from the RX buffer
> + FF-A partition ID 8001 : info cached
> + FF-A partition ID 8002 : info cached
> + FF-A partition ID 8003 : info cached
> + 3 FF-A partition(s) found and cached
> + Preparing for checking FF-A partitions count
> + Searching FF-A partitions using the provided UUID
> + No FF-A partition found. Querying framework ...
> + Reading FF-A partitions data from the RX buffer
> + Number of FF-A partition(s) matching the UUID: 1
> + EFI: Pre-allocating 1 partition(s) info structures
> + Preparing for filling FF-A partitions info
> + Searching FF-A partitions using the provided UUID
> + FF-A partition ID 8003 matches the provided UUID
> + EFI: MM partition ID 0x8003
> + EFI: Corstone1000: Capsule shared buffer at 0x80000000 , size 8192 pages
> + Booting /MemoryMapped(0x0,0x88200000,0xf00000)
> + EFI stub: Booting Linux Kernel...
> + EFI stub: Using DTB from configuration table
> + EFI stub: Exiting boot services...
> + unmapping FF-A RX/TX buffers
> + Freeing FF-A RX/TX buffers
> + Booting Linux on physical CPU 0x0000000000 [0x411fd040]
> + Linux version 6.1.9-yocto-standard (oe-user@oe-host) (aarch64-poky-linux-musl-gcc (GCC) 12.2.0, GNU ld (GNU Binutils) 2.40.202301193
> + Machine model: ARM Corstone1000 FPGA MPS3 board
> +
> +Contributors
> +------------
> + * Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> diff --git a/doc/arch/index.rst b/doc/arch/index.rst
> index b8da4b8c8e..2f916f4026 100644
> --- a/doc/arch/index.rst
> +++ b/doc/arch/index.rst
> @@ -8,6 +8,7 @@ Architecture-specific doc
>
> arc
> arm64
> + arm64.ffa
> m68k
> mips
> nios2
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 58be410135..885fdef4dc 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -112,6 +112,7 @@ obj-y += iommu/
> obj-y += smem/
> obj-y += thermal/
> obj-$(CONFIG_TEE) += tee/
> +obj-$(CONFIG_ARM_FFA_TRANSPORT) += firmware/arm-ffa/
> obj-y += axi/
> obj-y += ufs/
> obj-$(CONFIG_W1) += w1/
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index eae1c8ddc9..8789b1ea14 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -45,4 +45,5 @@ config ARM_SMCCC_FEATURES
> the PSCI driver is always probed and binds dirvers registered to the Arm SMCCC
> services if any and reported as supported by the SMCCC firmware.
>
> +source "drivers/firmware/arm-ffa/Kconfig"
> source "drivers/firmware/scmi/Kconfig"
> diff --git a/drivers/firmware/arm-ffa/Kconfig b/drivers/firmware/arm-ffa/Kconfig
> new file mode 100644
> index 0000000000..9200c8028b
> --- /dev/null
> +++ b/drivers/firmware/arm-ffa/Kconfig
> @@ -0,0 +1,36 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +config ARM_FFA_TRANSPORT
> + bool "Enable Arm Firmware Framework for Armv8-A driver"
> + depends on DM && ARM64
> + select ARM_SMCCC
> + select ARM_SMCCC_FEATURES
> + select LIB_UUID
> + select DEVRES
> + help
> + The Firmware Framework for Arm A-profile processors (FF-A)
> + describes interfaces (ABIs) that standardize communication
> + between the Secure World and Normal World leveraging TrustZone
> + technology.
> +
> + The FF-A support in U-Boot is based on FF-A specification v1.0 and uses SMC32
> + calling convention.
> +
> + FF-A specification:
> +
> + https://developer.arm.com/documentation/den0077/a/?lang=en
> +
> + In U-Boot FF-A design, FF-A is considered as a discoverable bus.
> + FF-A bus is discovered using ARM_SMCCC_FEATURES mechanism performed
> + by the PSCI driver.
> + The Secure World is considered as one entity to communicate with
> + using the FF-A bus.
> + FF-A communication is handled by one device and one instance (the bus).
> + The FF-A support on U-Boot takes care of all the interactions between Normal
> + world and Secure World.
> +
> + Generic FF-A methods are implemented in the Uclass (arm-ffa-uclass.c).
> + Arm specific methods are implemented in the Arm driver (arm-ffa.c).
> +
> + For more details about the FF-A support, please refer to doc/arch/arm64.ffa.rst
> +
> diff --git a/drivers/firmware/arm-ffa/Makefile b/drivers/firmware/arm-ffa/Makefile
> new file mode 100644
> index 0000000000..11b1766285
> --- /dev/null
> +++ b/drivers/firmware/arm-ffa/Makefile
> @@ -0,0 +1,8 @@
> +# 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>
> +
> +obj-y += arm-ffa-uclass.o arm-ffa.o
> diff --git a/drivers/firmware/arm-ffa/arm-ffa-uclass.c b/drivers/firmware/arm-ffa/arm-ffa-uclass.c
> new file mode 100644
> index 0000000000..c3ad19b8e7
> --- /dev/null
> +++ b/drivers/firmware/arm-ffa/arm-ffa-uclass.c
> @@ -0,0 +1,1143 @@
> +// 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 <arm_ffa_priv.h>
> +#include <dm.h>
> +#include <log.h>
> +#include <malloc.h>
> +#include <string.h>
> +#include <uuid.h>
> +#include <asm/global_data.h>
> +#include <dm/device-internal.h>
> +#include <dm/devres.h>
> +#include <dm/root.h>
> +#include <linux/errno.h>
> +#include <linux/sizes.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;

> +
> +/* FF-A discovery information */
> +struct ffa_discovery_info dscvry_info;

This should be in the uclass priv data, right? We try to avoid global
vars in driver model.
[..]

Regards,
Simon