Hi Abdellatif On Fri, Apr 15, 2022 at 01:27:58PM +0100, abdellatif.elkhl...@arm.com wrote: > From: Abdellatif El Khlifi <abdellatif.elkhl...@arm.com> > > Add the driver implementing Arm Firmware Framework for Armv8-A v1.0 > > 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. This driver uses SMC32 calling convention. > > In u-boot FF-A design, FF-A is considered as a discoverable bus. > 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 provides helper FF-A interfaces for user layers. > These helper functions allow clients to pass data and select the > FF-A function to use for the communication with secure world. > > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhl...@arm.com> > +++ b/arch/arm/cpu/armv8/smccc-call.S > @@ -1,6 +1,8 @@ > /* SPDX-License-Identifier: GPL-2.0 */ > /* > * Copyright (c) 2015, Linaro Limited > + * (C) Copyright 2022 ARM Limited > + * Abdellatif El Khlifi <abdellatif.elkhl...@arm.com> > */ > #include <linux/linkage.h> > #include <linux/arm-smccc.h> > @@ -45,3 +47,28 @@ ENDPROC(__arm_smccc_smc) > ENTRY(__arm_smccc_hvc) > SMCCC hvc > ENDPROC(__arm_smccc_hvc) > + > +#if (IS_ENABLED(CONFIG_ARM_FFA_TRANSPORT)) > + > + .macro FFASMCCC instr > + .cfi_startproc > + \instr #0 > + ldr x9, [sp] > + stp x0, x1, [x9, #ARM_SMCCC_RES_X0_OFFS] > + stp x2, x3, [x9, #ARM_SMCCC_RES_X2_OFFS] > + stp x4, x5, [x9, #ARM_SMCCC_RES_X4_OFFS] > + stp x6, x7, [x9, #ARM_SMCCC_RES_X6_OFFS] > + ret > + .cfi_endproc > + .endm > + > +/* > + * void arm_ffa_smccc_smc(unsigned long a0, unsigned long a1, unsigned long > a2, > + * unsigned long a3, unsigned long a4, unsigned long a5, > + * unsigned long a6, unsigned long a7, struct arm_smccc_res *res) > + */ > +ENTRY(__arm_ffa_smccc_smc) > + FFASMCCC smc > +ENDPROC(__arm_ffa_smccc_smc) > + > +#endif > diff --git a/arch/arm/lib/asm-offsets.c b/arch/arm/lib/asm-offsets.c > index 22fd541f9a..02a4a42fe6 100644 > --- a/arch/arm/lib/asm-offsets.c > +++ b/arch/arm/lib/asm-offsets.c > @@ -9,6 +9,8 @@ > * generate asm statements containing #defines, > * compile this file to assembler, and then extract the > * #defines from the assembly-language output. > + * > + * (C) Copyright 2022 ARM Limited > */ > > #include <common.h> > @@ -115,6 +117,10 @@ int main(void) > #ifdef CONFIG_ARM_SMCCC > DEFINE(ARM_SMCCC_RES_X0_OFFS, offsetof(struct arm_smccc_res, a0)); > DEFINE(ARM_SMCCC_RES_X2_OFFS, offsetof(struct arm_smccc_res, a2)); > +#if (IS_ENABLED(CONFIG_ARM_FFA_TRANSPORT)) > + DEFINE(ARM_SMCCC_RES_X4_OFFS, offsetof(struct arm_smccc_res, a4)); > + DEFINE(ARM_SMCCC_RES_X6_OFFS, offsetof(struct arm_smccc_res, a6)); > +#endif > DEFINE(ARM_SMCCC_QUIRK_ID_OFFS, offsetof(struct arm_smccc_quirk, id)); > DEFINE(ARM_SMCCC_QUIRK_STATE_OFFS, offsetof(struct arm_smccc_quirk, > state)); > #endif > diff --git a/common/board_r.c b/common/board_r.c > index b92c1bb0be..bb5f1d0aa6 100644 > --- a/common/board_r.c > +++ b/common/board_r.c > @@ -62,6 +62,10 @@ > #include <asm-generic/gpio.h> > #include <efi_loader.h> > > +#ifdef CONFIG_ARM_FFA_TRANSPORT > +#include <arm_ffa_helper.h> > +#endif > + > DECLARE_GLOBAL_DATA_PTR; > > ulong monitor_flash_len; > @@ -771,6 +775,9 @@ static init_fnc_t init_sequence_r[] = { > INIT_FUNC_WATCHDOG_RESET > initr_net, > #endif > +#ifdef CONFIG_ARM_FFA_TRANSPORT > + ffa_helper_bus_discover, > +#endif > #ifdef CONFIG_POST > initr_post, > #endif > diff --git a/drivers/Kconfig b/drivers/Kconfig > index b26ca8cf70..e83c23789d 100644 > --- a/drivers/Kconfig > +++ b/drivers/Kconfig > @@ -6,6 +6,8 @@ source "drivers/core/Kconfig" > > source "drivers/adc/Kconfig" > > +source "drivers/arm-ffa/Kconfig" > + > source "drivers/ata/Kconfig" > > source "drivers/axi/Kconfig" > diff --git a/drivers/Makefile b/drivers/Makefile > index 4e7cf28440..6671d2a604 100644 > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -107,6 +107,7 @@ obj-y += iommu/ > obj-y += smem/ > obj-y += thermal/ > obj-$(CONFIG_TEE) += tee/ > +obj-$(CONFIG_ARM_FFA_TRANSPORT) += arm-ffa/ > obj-y += axi/ > obj-y += ufs/ > obj-$(CONFIG_W1) += w1/ > diff --git a/drivers/arm-ffa/Kconfig b/drivers/arm-ffa/Kconfig > new file mode 100644 > index 0000000000..23815534c4 > --- /dev/null > +++ b/drivers/arm-ffa/Kconfig > @@ -0,0 +1,27 @@ > +# 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 if ARM64 > + select LIB_UUID > + select ARM_FFA_TRANSPORT_HELPERS > + 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. > + > + This driver 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. > + 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. > diff --git a/drivers/arm-ffa/Makefile b/drivers/arm-ffa/Makefile > new file mode 100644 > index 0000000000..7bc9a336a9 > --- /dev/null > +++ b/drivers/arm-ffa/Makefile > @@ -0,0 +1,6 @@ > +# SPDX-License-Identifier: GPL-2.0+ > +# > +# (C) Copyright 2022 Abdellatif El Khlifi <abdellatif.elkhl...@arm.com> > +# > + > +obj-y += arm-ffa-uclass.o core.o > diff --git a/drivers/arm-ffa/arm-ffa-uclass.c > b/drivers/arm-ffa/arm-ffa-uclass.c > new file mode 100644 > index 0000000000..2439f87586 > --- /dev/null > +++ b/drivers/arm-ffa/arm-ffa-uclass.c > @@ -0,0 +1,64 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * (C) Copyright 2022 ARM Limited > + * Abdellatif El Khlifi <abdellatif.elkhl...@arm.com> > + */ > + > +#include <common.h> > +#include <dm.h> > +#include <arm_ffa.h> > +#include <errno.h> > +#include <log.h> > +#include <asm/global_data.h> > + > +DECLARE_GLOBAL_DATA_PTR; > + > +UCLASS_DRIVER(ffa) = { > + .name = "ffa", > + .id = UCLASS_FFA, > +}; > + > +/** > + * ffa_get_invoke_func - performs a call to the FF-A driver dispatcher > + * @func_id: The FF-A function to be used > + * @func_data: Pointer to the FF-A function arguments > + * container structure. This also includes > + * pointers to the returned data needed by > + * clients. > + * > + * This runtime function passes the FF-A function ID and its arguments to > + * the FF-A driver dispatcher. > + * This function is called by the FF-A helper functions. > + * > + * Return: > + * > + * FFA_ERR_STAT_SUCCESS on success. Otherwise, failure > + */ > +int __ffa_runtime ffa_get_invoke_func(u32 func_id, struct ffa_interface_data > *func_data) > +{ > + if (!ffa_device_get_ops()->invoke_func) > + return -EINVAL; > + > + return ffa_device_get_ops()->invoke_func(func_id, func_data); > +} > + > +/** > + * ffa_bus_discover - discover FF-A bus and probe the arm_ffa device > + * > + * This boot time function makes sure the FF-A bus is discoverable. > + * Then, the arm_ffa device is probed and ready to use. > + * This function is called automatically at initcalls > + * level (after u-boot relocation). > + * > + * Arm FF-A transport is implemented through arm_ffa u-boot device managing > the FF-A > + * communication. > + * All FF-A clients should use the arm_ffa device to use the FF-A transport. > + * > + * Return: > + * > + * FFA_ERR_STAT_SUCCESS on success. Otherwise, failure > + */ > +int ffa_bus_discover(void) > +{ > + return ffa_get_device(); > +} > diff --git a/drivers/arm-ffa/arm_ffa_prv.h b/drivers/arm-ffa/arm_ffa_prv.h > new file mode 100644 > index 0000000000..44f258addb > --- /dev/null > +++ b/drivers/arm-ffa/arm_ffa_prv.h > @@ -0,0 +1,193 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * (C) Copyright 2022 ARM Limited > + * Abdellatif El Khlifi <abdellatif.elkhl...@arm.com> > + */ > + > +#ifndef __ARM_FFA_PRV_H > +#define __ARM_FFA_PRV_H > + > +#include <arm_ffa.h> > +#include <linux/bitfield.h> > +#include <linux/bitops.h> > +#include <dm/read.h> > + > +/* > + * This header is private. It is exclusively used by the FF-A driver > + */ > + > +/* FF-A core driver name */ > +#define FFA_DRV_NAME "arm_ffa" > + > +/* FF-A driver version definitions */ > + > +#define MAJOR_VERSION_MASK GENMASK(30, 16) > +#define MINOR_VERSION_MASK GENMASK(15, 0) > +#define GET_FFA_MAJOR_VERSION(x) \ > + ((u16)(FIELD_GET(MAJOR_VERSION_MASK, (x)))) > +#define GET_FFA_MINOR_VERSION(x) \ > + ((u16)(FIELD_GET(MINOR_VERSION_MASK, (x)))) > +#define PACK_VERSION_INFO(major, minor) \ > + (FIELD_PREP(MAJOR_VERSION_MASK, (major)) | \ > + FIELD_PREP(MINOR_VERSION_MASK, (minor))) > + > +#define FFA_MAJOR_VERSION (1) > +#define FFA_MINOR_VERSION (0) > +#define FFA_VERSION_1_0 \ > + PACK_VERSION_INFO(FFA_MAJOR_VERSION, FFA_MINOR_VERSION) > + > +/* Endpoint ID mask (u-boot endpoint ID) */ > + > +#define GET_SELF_ENDPOINT_ID_MASK GENMASK(15, 0) > +#define GET_SELF_ENDPOINT_ID(x) \ > + ((u16)(FIELD_GET(GET_SELF_ENDPOINT_ID_MASK, (x)))) > + > +#define PREP_SELF_ENDPOINT_ID_MASK GENMASK(31, 16) > +#define PREP_SELF_ENDPOINT_ID(x) \ > + (FIELD_PREP(PREP_SELF_ENDPOINT_ID_MASK, (x))) > + > +/* Partition endpoint ID mask (partition with which u-boot communicates > with) */ > + > +#define PREP_PART_ENDPOINT_ID_MASK GENMASK(15, 0) > +#define PREP_PART_ENDPOINT_ID(x) \ > + (FIELD_PREP(PREP_PART_ENDPOINT_ID_MASK, (x))) > + > +/* The FF-A SMC function prototype definition */ > + > +typedef void (*invoke_ffa_fn_t)(unsigned long a0, unsigned long a1, > + unsigned long a2, unsigned long a3, unsigned long a4, > + unsigned long a5, unsigned long a6, unsigned long a7, > + struct arm_smccc_res *res); > + > +/** > + * enum ffa_conduit - Arm FF-A conduits supported by the Arm FF-A driver > + * Currently only SMC32 is supported. > + */ > +enum ffa_conduit { > + FFA_CONDUIT_SMC = 0, > +}; > + > +/** > + * FFA_DECLARE_ARGS - FF-A functions local variables > + * @a0-a7: local variables used to set registers x0-x7 > + * @res: the structure hosting the FF-A function return data > + * > + * A helper macro for declaring local variables for the FF-A functions > arguments. > + * The x0-x7 registers are used to exchange data with the secure world. > + * But, only the bottom 32-bit of thes registers contains the data. > + */ > +#define FFA_DECLARE_ARGS \ > + unsigned long a0 = 0; \ > + unsigned long a1 = 0; \ > + unsigned long a2 = 0; \ > + unsigned long a3 = 0; \ > + unsigned long a4 = 0; \ > + unsigned long a5 = 0; \ > + unsigned long a6 = 0; \ > + unsigned long a7 = 0; \ > + struct arm_smccc_res res = {0} > + > +/* FF-A error codes */ > +#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_INTERRUPTED (-5) > +#define FFA_ERR_STAT_DENIED (-6) > +#define FFA_ERR_STAT_RETRY (-7) > +#define FFA_ERR_STAT_ABORTED (-8) > + > +/** > + * struct ffa_features_desc - FF-A functions features > + * @func_id: FF-A function > + * @field1: features read from register w2 > + * @field2: features read from register w3 > + * > + * Data structure describing the features of the FF-A functions queried by > + * FFA_FEATURES > + */ > +struct ffa_features_desc { > + u32 func_id; > + u32 field1; > + u32 field2; > +}; > + > +/** > + * enum ffa_rxtx_buf_sizes - minimum sizes supported > + * for the RX/TX buffers > + */ > +enum ffa_rxtx_buf_sizes { > + RXTX_4K, > + RXTX_64K, > + RXTX_16K > +}; > + > +/* > + * Number of the FF-A interfaces features descriptors > + * currently only FFA_RXTX_MAP descriptor is supported > + */ > +#define FFA_FEATURE_DESC_CNT (1) > + > +/** > + * struct ffa_rxtxpair - structure hosting the RX/TX buffers virtual > addresses > + * @rxbuf: virtual address of the RX buffer > + * @txbuf: virtual address of the TX buffer > + * > + * Data structure hosting the virtual addresses of the mapped RX/TX buffers > + * These addresses are used by the FF-A functions that use the RX/TX buffers > + */ > +struct ffa_rxtxpair { > + u64 rxbuf; /* virtual address */ > + u64 txbuf; /* virtual address */ > +}; > + > +/** > + * struct ffa_partition_desc - the secure partition descriptor > + * @info: partition information > + * @UUID: UUID > + * > + * Each partition has its descriptor containing the partitions information > and the UUID > + */ > +struct ffa_partition_desc { > + struct ffa_partition_info info; > + union ffa_partition_uuid UUID; > +}; > + > +/** > + * struct ffa_partitions - descriptors for all secure partitions > + * @count: The number of partitions descriptors > + * @descs The partitions descriptors table > + * > + * This data structure contains the partitions descriptors table > + */ > +struct ffa_partitions { > + u32 count; > + struct ffa_partition_desc *descs; /* virtual address */ > +}; > + > +/** > + * struct ffa_prvdata - 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 > + * @conduit: The selected conduit > + * @invoke_ffa_fn: The function executing the FF-A function > + * @features: Table of the FF-A functions having features > + * > + * The driver data structure hosting all resident data. > + */ > +struct ffa_prvdata { > + struct udevice *dev; > + u32 fwk_version; > + u16 id; > + struct ffa_partitions partitions; > + struct ffa_rxtxpair pair; > + enum ffa_conduit conduit; > + invoke_ffa_fn_t invoke_ffa_fn; > + struct ffa_features_desc features[FFA_FEATURE_DESC_CNT]; > +}; > + > +#endif > diff --git a/drivers/arm-ffa/core.c b/drivers/arm-ffa/core.c > new file mode 100644 > index 0000000000..09e4eb753a > --- /dev/null > +++ b/drivers/arm-ffa/core.c > @@ -0,0 +1,1349 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * (C) Copyright 2022 ARM Limited > + * Abdellatif El Khlifi <abdellatif.elkhl...@arm.com> > + */ > + > +#include "arm_ffa_prv.h" > +#include <asm/global_data.h> > +#include <asm/io.h> > +#include <common.h> > +#include <dm.h> > +#include <dm/device-internal.h> > +#include <dm/root.h> > +#include <linux/errno.h> > +#include <linux/sizes.h> > +#include <log.h> > +#include <malloc.h> > +#include <mapmem.h> > +#include <string.h> > + > +DECLARE_GLOBAL_DATA_PTR; > + > +/** > + * The device private data structure containing all the resident > + * data read from secure world > + */ > +struct ffa_prvdata __ffa_runtime_data ffa_priv_data = {0}; > + > +/* > + * Driver functions > + */ > + > +/** > + * ffa_get_device - create, bind and probe the arm_ffa device > + * > + * This boot time function makes sure the arm_ffa device is > + * created, bound to this driver, probed and ready to use. > + * Arm FF-A transport is implemented through a single u-boot > + * device managing the FF-A bus (arm_ffa). > + * > + * Return: > + * > + * FFA_ERR_STAT_SUCCESS on success. Otherwise, failure > + */ > +int ffa_get_device(void) > +{ > + int ret; > + > + if (ffa_priv_data.dev) > + return FFA_ERR_STAT_SUCCESS; > + > + ret = device_bind(dm_root(), > + DM_DRIVER_GET(arm_ffa), > + FFA_DRV_NAME, > + NULL, > + ofnode_null(), > + &ffa_priv_data.dev); > + if (ret) { > + ffa_priv_data.dev = NULL; > + return ret; > + } > + > + /* The FF-A bus discovery succeeds when probing is successful */ > + ret = device_probe(ffa_priv_data.dev); > + if (ret) { > + ffa_err("can not probe the device"); > + device_unbind(ffa_priv_data.dev); > + ffa_priv_data.dev = NULL; > + return ret; > + } > + > + return FFA_ERR_STAT_SUCCESS; > +} > + > +/** > + * ffa_get_version - FFA_VERSION handler function > + * > + * This is the boot time function that implements FFA_VERSION FF-A function > + * to get from the secure world the FF-A framework version > + * > + * Return: > + * > + * FFA_ERR_STAT_SUCCESS on success. Otherwise, failure > + */ > +static int ffa_get_version(void) > +{ > + u16 major, minor; > + > + FFA_DECLARE_ARGS; > + > + if (!ffa_priv_data.invoke_ffa_fn) > + panic("[FFA] no private data found\n"); > + > + a0 = FFA_VERSION; > + a1 = FFA_VERSION_1_0; > + ffa_priv_data.invoke_ffa_fn(a0, a1, a2, a3, a4, a5, a6, a7, &res); > + > + if (res.a0 == FFA_ERR_STAT_NOT_SUPPORTED) { > + ffa_err("A Firmware Framework implementation does not exist"); > + return -EOPNOTSUPP; > + } > + > + major = GET_FFA_MAJOR_VERSION(res.a0); > + minor = GET_FFA_MINOR_VERSION(res.a0); > + > + ffa_info("FF-A driver %d.%d\nFF-A framework %d.%d", > + FFA_MAJOR_VERSION, FFA_MINOR_VERSION, major, minor); > + > + if ((major == FFA_MAJOR_VERSION && minor >= FFA_MINOR_VERSION)) { > + ffa_info("Versions are compatible "); > + > + ffa_priv_data.fwk_version = res.a0; > + > + return FFA_ERR_STAT_SUCCESS; > + } > + > + ffa_info("Versions are incompatible "); > + return -EPROTONOSUPPORT; > +} > + > +/** > + * ffa_get_endpoint_id - FFA_ID_GET handler function > + * > + * This is the boot time function that implements FFA_ID_GET FF-A function > + * to get from the secure world u-boot endpoint ID > + * > + * Return: > + * > + * FFA_ERR_STAT_SUCCESS on success. Otherwise, failure > + */ > +static int ffa_get_endpoint_id(void) > +{ > + FFA_DECLARE_ARGS; > + > + if (!ffa_priv_data.invoke_ffa_fn) > + panic("[FFA] no private data found\n"); > + > + a0 = FFA_ID_GET; > + > + ffa_priv_data.invoke_ffa_fn(a0, a1, a2, a3, a4, a5, a6, a7, &res); > + > + switch (res.a0) { > + case FFA_ERROR: > + { > + if (((int)res.a2) == FFA_ERR_STAT_NOT_SUPPORTED) { > + ffa_err("This function is not implemented at this FF-A > instance"); > + return -EOPNOTSUPP; > + } > + > + ffa_err("Undefined error code (%d)", ((int)res.a2)); > + return -EINVAL; > + } > + case FFA_SUCCESS: > + { > + ffa_priv_data.id = GET_SELF_ENDPOINT_ID(res.a2); > + ffa_info("endpoint ID is %u", ffa_priv_data.id); > + > + return FFA_ERR_STAT_SUCCESS; > + } > + default: > + { > + ffa_err("Undefined response function (0x%lx)", res.a0); > + return -EINVAL; > + } > + } > +} > + > +/** > + * ffa_get_features_desc - returns the features descriptor of the specified > + * FF-A function > + * @func_id: the FF-A function which the features are to be retrieved > + * > + * This is a boot time function that searches the features descriptor of the > + * specified FF-A function > + * > + * Return: > + * > + * When found, the address of the features descriptor is returned. > Otherwise, NULL. > + */ > +static struct ffa_features_desc *ffa_get_features_desc(u32 func_id) > +{ > + u32 desc_idx; > + > + /* > + * search for the descriptor of the selected FF-A interface > + */ > + for (desc_idx = 0; desc_idx < FFA_FEATURE_DESC_CNT ; desc_idx++) > + if (ffa_priv_data.features[desc_idx].func_id == func_id) > + return &ffa_priv_data.features[desc_idx]; > + > + return NULL; > +} > + > +/** > + * ffa_get_rxtx_map_features - FFA_FEATURES handler function with > FFA_RXTX_MAP > + * argument > + * > + * This is the boot time function that implements FFA_FEATURES FF-A function > + * to retrieve the FFA_RXTX_MAP features > + * > + * Return: > + * > + * FFA_ERR_STAT_SUCCESS on success. Otherwise, failure > + */ > +static int ffa_get_rxtx_map_features(void) > +{ > + FFA_DECLARE_ARGS; > + > + if (!ffa_priv_data.invoke_ffa_fn) > + panic("[FFA] no private data found\n"); > + > + a0 = FFA_FEATURES; > + a1 = FFA_RXTX_MAP; > + > + ffa_priv_data.invoke_ffa_fn(a0, a1, a2, a3, a4, a5, a6, a7, &res); > + > + switch (res.a0) { > + case FFA_ERROR: > + { > + if (((int)res.a2) == FFA_ERR_STAT_NOT_SUPPORTED) { > + ffa_err("FFA_RXTX_MAP is not implemented at this FF-A > instance"); > + return -EOPNOTSUPP; > + } > + > + ffa_err("Undefined error code (%d)", ((int)res.a2)); > + return -EINVAL;
This happens quite a few times throughout the code. Why is -EINVAL used here instead of -FFA_ERR_STAT_INVALID_PARAMETERS for example? We should either use FFA_* errors consistently of get rid of those entirely and stick to standard ones, but let's not mix and match > + } > + case FFA_SUCCESS: > + { > + u32 desc_idx; > + > + /* > + * search for an empty descriptor > + */ > + for (desc_idx = 0; desc_idx < FFA_FEATURE_DESC_CNT ; desc_idx++) > + if (!ffa_priv_data.features[desc_idx].func_id) { > + /* > + * populate the descriptor with > + * the interface features data > + */ > + ffa_priv_data.features[desc_idx].func_id = > + FFA_RXTX_MAP; > + ffa_priv_data.features[desc_idx].field1 = > + res.a2; > + > + ffa_info("FFA_RXTX_MAP features data 0x%lx", > + res.a2); > + > + return FFA_ERR_STAT_SUCCESS; > + } > + > + ffa_err("Cannot save FFA_RXTX_MAP features data. Descriptors > table full"); > + return -ENOBUFS; Similarly -FFA_ERR_STAT_NO_MEMORY or something? > + } > + default: > + { > + ffa_err("Undefined response function (0x%lx)", > + res.a0); > + return -EINVAL; > + } > + } > +} > + > +/** > + * ffa_get_rxtx_buffers_pages_cnt - reads from the features data descriptors > + * the minimum number of pages in > each of the RX/TX > + * buffers > + * @buf_4k_pages: Pointer to the minimum number of pages > + * > + * This is the boot time function that returns the minimum number of pages > + * in each of the RX/TX buffers > + * > + * Return: > + * > + * buf_4k_pages points to the returned number of pages > + * FFA_ERR_STAT_SUCCESS on success. Otherwise, failure > + */ > +static int ffa_get_rxtx_buffers_pages_cnt(size_t *buf_4k_pages) > +{ > + struct ffa_features_desc *desc = NULL; > + > + if (!buf_4k_pages) > + return -EINVAL; > + > + desc = ffa_get_features_desc(FFA_RXTX_MAP); > + if (!desc) > + return -EINVAL; > + > + switch (desc->field1) { > + case RXTX_4K: > + *buf_4k_pages = 1; > + break; > + case RXTX_16K: > + *buf_4k_pages = 4; > + break; > + case RXTX_64K: > + *buf_4k_pages = 16; > + break; > + default: > + ffa_err("RX/TX buffer size not supported"); > + return -EINVAL; > + } > + > + return FFA_ERR_STAT_SUCCESS; > +} > + > +/** > + * ffa_free_rxtx_buffers - frees the RX/TX buffers > + * @buf_4k_pages: the minimum number of pages in each of the RX/TX > + * buffers > + * > + * This is the boot time function used to free the RX/TX buffers > + * > + * Return: > + * > + * FFA_ERR_STAT_SUCCESS on success. Otherwise, failure > + */ > +static int ffa_free_rxtx_buffers(size_t buf_4k_pages) > +{ > + efi_status_t free_rxbuf_ret, free_txbuf_ret; > + > + ffa_info("Freeing RX/TX buffers"); > + > + free_rxbuf_ret = efi_free_pages(ffa_priv_data.pair.rxbuf, buf_4k_pages); > + free_txbuf_ret = efi_free_pages(ffa_priv_data.pair.txbuf, buf_4k_pages); > + > + if (free_rxbuf_ret != EFI_SUCCESS || free_txbuf_ret != EFI_SUCCESS) { > + ffa_err("Failed to free RX/TX buffers (rx: %lu , tx: %lu)", > + free_rxbuf_ret, > + free_txbuf_ret); > + return -EINVAL; > + } > + > + ffa_priv_data.pair.rxbuf = 0; > + ffa_priv_data.pair.txbuf = 0; Should those be set to 0 regardless of the efi_free_pages() outcome? If not then you probably need to handle those one by one. As is it right now one failure to free a buffer means both of those won't be set > + > + return FFA_ERR_STAT_SUCCESS; > +} > + > +/** > + * ffa_alloc_rxtx_buffers - allocates the RX/TX buffers > + * @buf_4k_pages: the minimum number of pages in each of the RX/TX > + * buffers > + * > + * This is the boot time function used by ffa_map_rxtx_buffers to allocate > + * the RX/TX buffers before mapping them > + * > + * Return: > + * > + * FFA_ERR_STAT_SUCCESS on success. Otherwise, failure > + */ > +static int ffa_alloc_rxtx_buffers(size_t buf_4k_pages) > +{ > +#if CONFIG_IS_ENABLED(EFI_LOADER) > + > + efi_status_t efi_ret; > + void *virt_txbuf; > + void *virt_rxbuf; > + > + ffa_info("Using %lu 4KB page(s) for RX/TX buffers size", > + buf_4k_pages); > + > + efi_ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, > + EFI_BOOT_SERVICES_DATA, > + buf_4k_pages, > + &ffa_priv_data.pair.rxbuf); > + Why are we specifically limiting this to EFI? I understand using efi allocated memory for potential runtime operations, were we need the memory preserved, but why here? There's also a side effect with this allocation. The EFI subsystem doesn't usually come up until u-boot is fully up and running. This call will force it to start way earlier > + if (efi_ret != EFI_SUCCESS) { > + ffa_priv_data.pair.rxbuf = 0; > + ffa_err("Failure to allocate RX buffer (EFI error: 0x%lx)", > + efi_ret); > + > + return -ENOBUFS; > + } > + > + ffa_info("RX buffer at virtual address 0x%llx", > + ffa_priv_data.pair.rxbuf); > + > + virt_rxbuf = (void *)ffa_priv_data.pair.rxbuf; > + > + /* > + * make sure the buffer is clean before use > + */ > + memset(virt_rxbuf, 0, buf_4k_pages * SZ_4K); > + > + efi_ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, > + EFI_RUNTIME_SERVICES_DATA, > + buf_4k_pages, > + &ffa_priv_data.pair.txbuf); > + > + if (efi_ret != EFI_SUCCESS) { > + efi_free_pages(ffa_priv_data.pair.rxbuf, buf_4k_pages); > + ffa_priv_data.pair.rxbuf = 0; > + ffa_priv_data.pair.txbuf = 0; > + ffa_err("Failure to allocate the TX buffer (EFI error: 0x%lx)" > + , efi_ret); > + > + return -ENOBUFS; > + } > + > + ffa_info("TX buffer at virtual address 0x%llx", > + ffa_priv_data.pair.txbuf); > + > + virt_txbuf = (void *)ffa_priv_data.pair.txbuf; > + > + /* > + * make sure the buffer is clean before use > + */ > + memset(virt_txbuf, 0, buf_4k_pages * SZ_4K); > + > + return FFA_ERR_STAT_SUCCESS; > + > +#else > + return -ENOBUFS; > +#endif > +} > + > +/** > + * ffa_map_rxtx_buffers - FFA_RXTX_MAP handler function > + * @buf_4k_pages: the minimum number of pages in each of the RX/TX > + * buffers > + * > + * This is the boot time function that implements FFA_RXTX_MAP FF-A function > + * to map the RX/TX buffers > + * > + * Return: > + * > + * FFA_ERR_STAT_SUCCESS on success. Otherwise, failure > + */ > +static int ffa_map_rxtx_buffers(size_t buf_4k_pages) > +{ > + int ret; > + > + FFA_DECLARE_ARGS; > + > + if (!ffa_priv_data.invoke_ffa_fn) > + panic("[FFA] no private data found\n"); > + > + ret = ffa_alloc_rxtx_buffers(buf_4k_pages); > + if (ret != FFA_ERR_STAT_SUCCESS) > + return ret; > + > + a0 = FFA_RXTX_MAP; > + a1 = ffa_priv_data.pair.txbuf; > + a2 = ffa_priv_data.pair.rxbuf; > + a3 = buf_4k_pages; > + > + ffa_priv_data.invoke_ffa_fn(a0, a1, a2, a3, a4, a5, a6, a7, &res); > + > + switch (res.a0) { > + case FFA_ERROR: > + { > + switch (((int)res.a2)) { > + case FFA_ERR_STAT_INVALID_PARAMETERS: > + ffa_err("One or more fields in input parameters is > incorrectly encoded"); > + ret = -EPERM; > + break; > + case FFA_ERR_STAT_NO_MEMORY: > + ffa_err("Not enough memory"); > + ret = -ENOMEM; > + break; > + case FFA_ERR_STAT_DENIED: > + ffa_err("Buffer pair already registered"); > + ret = -EACCES; > + break; > + case FFA_ERR_STAT_NOT_SUPPORTED: > + ffa_err("This function is not implemented at this FF-A > instance"); > + ret = -EOPNOTSUPP; > + break; > + default: > + ffa_err("Undefined error (%d)", > + ((int)res.a2)); > + ret = -EINVAL; > + } Can we have an array with string literals for the errors and do a lookup instead of nested case switches? > + break; > + } > + case FFA_SUCCESS: > + ffa_info("RX/TX buffers mapped"); > + return FFA_ERR_STAT_SUCCESS; > + default: > + ffa_err("Undefined response function (0x%lx)", > + res.a0); > + ret = -EINVAL; > + } > + > + ffa_free_rxtx_buffers(buf_4k_pages); > + > + return ret; > +} > + > +/** > + * ffa_unmap_rxtx_buffers - FFA_RXTX_UNMAP handler function > + * > + * This is the boot time function that implements FFA_RXTX_UNMAP FF-A > function > + * to unmap the RX/TX buffers > + * > + * Return: > + * > + * FFA_ERR_STAT_SUCCESS on success. Otherwise, failure > + */ > +static int ffa_unmap_rxtx_buffers(void) > +{ > + FFA_DECLARE_ARGS; > + > + if (!ffa_priv_data.invoke_ffa_fn) > + panic("[FFA] no private data found\n"); > + > + a0 = FFA_RXTX_UNMAP; > + a1 = PREP_SELF_ENDPOINT_ID(ffa_priv_data.id); > + > + ffa_priv_data.invoke_ffa_fn(a0, a1, a2, a3, a4, a5, a6, a7, &res); > + > + switch (res.a0) { > + case FFA_ERROR: > + { > + if (((int)res.a2) == FFA_ERR_STAT_NOT_SUPPORTED) > + panic("[FFA] FFA_RXTX_UNMAP is not implemented at this > FF-A instance\n"); > + else if (((int)res.a2) == FFA_ERR_STAT_INVALID_PARAMETERS) > + panic("[FFA] There is no buffer pair registered on > behalf of the caller\n"); > + else > + panic("[FFA] Undefined error (%d)\n", ((int)res.a2)); There's panics sprinkled around the code. Are all those cases really fatal? > + } > + case FFA_SUCCESS: > + { > + size_t buf_4k_pages = 0; > + int ret; > + > + ret = ffa_get_rxtx_buffers_pages_cnt(&buf_4k_pages); > + if (ret != FFA_ERR_STAT_SUCCESS) > + panic("[FFA] RX/TX buffers unmapped but failure in > getting pages count\n"); > + > + ret = ffa_free_rxtx_buffers(buf_4k_pages); > + if (ret != FFA_ERR_STAT_SUCCESS) > + panic("[FFA] RX/TX buffers unmapped but failure in > freeing the memory\n"); > + > + ffa_info("RX/TX buffers unmapped and memory freed"); > + > + return FFA_ERR_STAT_SUCCESS; > + } > + default: > + panic("[FFA] Undefined response function (0x%lx)", res.a0); > + } > +} > + > +/** > + * ffa_release_rx_buffer - FFA_RX_RELEASE handler function > + * > + * This is the boot time function that invokes FFA_RX_RELEASE FF-A function > + * to release the ownership of the RX buffer > + * > + * Return: > + * > + * FFA_ERR_STAT_SUCCESS on success. Otherwise, failure > + */ > +static int ffa_release_rx_buffer(void) > +{ > + FFA_DECLARE_ARGS; > + > + if (!ffa_priv_data.invoke_ffa_fn) > + panic("[FFA] no private data found\n"); > + > + a0 = FFA_RX_RELEASE; > + > + ffa_priv_data.invoke_ffa_fn(a0, a1, a2, a3, a4, a5, a6, a7, &res); > + > + switch (res.a0) { > + case FFA_ERROR: > + { > + if (((int)res.a2) == FFA_ERR_STAT_NOT_SUPPORTED) > + panic("[FFA] FFA_RX_RELEASE is not implemented at this > FF-A instance\n"); > + else if (((int)res.a2) == FFA_ERR_STAT_DENIED) > + panic("[FFA] Caller did not have ownership of the RX > buffer\n"); > + else > + panic("[FFA] Undefined error (%d)\n", ((int)res.a2)); > + } > + case FFA_SUCCESS: > + return FFA_ERR_STAT_SUCCESS; > + > + default: > + panic("[FFA] Undefined response function (0x%lx)\n", res.a0); > + } > +} > + > +/** > + * ffa_uuid_are_identical - checks whether two given UUIDs are identical > + * @uuid1: first UUID > + * @uuid2: second UUID > + * > + * This is a boot time function used by ffa_read_partitions_info to search > + * for a UUID in the partitions descriptors table > + * > + * Return: > + * > + * 1 when UUIDs match. Otherwise, 0 > + */ > +int ffa_uuid_are_identical(const union ffa_partition_uuid *uuid1, > + const union ffa_partition_uuid *uuid2) > +{ > + if (!uuid1 || !uuid2) > + return 0; > + > + return (!memcmp(uuid1, uuid2, sizeof(union ffa_partition_uuid))); > +} > + > +/** > + * ffa_read_partitions_info - reads the data queried by > FFA_PARTITION_INFO_GET > + * and saves it in the > private structure > + * @count: The number of partitions queried > + * @part_uuid: Pointer to the partition(s) UUID > + * > + * This is the boot time function that reads the partitions information > + * returned by the FFA_PARTITION_INFO_GET and saves it in the private > + * data structure. > + * > + * Return: > + * > + * The private data structure is updated with the partition(s) information > + * FFA_ERR_STAT_SUCCESS is returned on success. Otherwise, failure > + */ > +static int ffa_read_partitions_info(u32 count, union ffa_partition_uuid > *part_uuid) > +{ > + if (!count) { > + ffa_err("No partition detected"); > + return -ENODATA; > + } > + > + ffa_info("Reading partitions data from the RX buffer"); > + > +#if CONFIG_IS_ENABLED(EFI_LOADER) > + > + if (!part_uuid) { > + /* > + * querying information of all partitions > + */ > + u64 data_pages; > + u64 data_bytes; > + efi_status_t efi_ret; > + size_t buf_4k_pages = 0; > + u32 desc_idx; > + struct ffa_partition_info *parts_info; > + int ret; > + > + data_bytes = count * sizeof(struct ffa_partition_desc); > + data_pages = efi_size_in_pages(data_bytes); > + > + /* > + * get the RX buffer size in pages > + */ > + ret = ffa_get_rxtx_buffers_pages_cnt(&buf_4k_pages); > + if (ret != FFA_ERR_STAT_SUCCESS) { > + ffa_err("Can not get the RX buffer size (error %d)", > ret); > + return ret; > + } > + > + if (data_pages > buf_4k_pages) { > + ffa_err("Partitions data size exceeds the RX buffer > size:"); > + ffa_err(" Sizes in pages: data %llu , RX buffer %lu > ", > + data_pages, > + buf_4k_pages); > + > + return -ENOMEM; > + } > + > + efi_ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, > + EFI_RUNTIME_SERVICES_DATA, > + data_pages, > + (u64 > *)&ffa_priv_data.partitions.descs); > + Same questions here. Why are we using the EFI APIs to allocate buffers? > + if (efi_ret != EFI_SUCCESS) { > + ffa_priv_data.partitions.descs = NULL; > + > + ffa_err("Cannot allocate partitions data buffer (EFI > error 0x%lx)", > + efi_ret); > + > + return -ENOBUFS; > + } > + > + /* > + * make sure the buffer is clean before use > + */ > + memset(ffa_priv_data.partitions.descs, 0, > + data_pages * SZ_4K); > + > + parts_info = (struct ffa_partition_info > *)ffa_priv_data.pair.rxbuf; > + > + for (desc_idx = 0 ; desc_idx < count ; desc_idx++) { > + ffa_priv_data.partitions.descs[desc_idx].info = > + parts_info[desc_idx]; > + > + ffa_info("Partition ID %x : info cached", > + > ffa_priv_data.partitions.descs[desc_idx].info.id); > + } > + > + ffa_priv_data.partitions.count = count; > + > + ffa_info("%d partition(s) found and cached", count); > + > + } else { > + u32 rx_desc_idx, cached_desc_idx; > + struct ffa_partition_info *parts_info; > + u8 desc_found; > + > + parts_info = (struct ffa_partition_info > *)ffa_priv_data.pair.rxbuf; > + > + /* > + * search for the SP IDs read from the RX buffer > + * in the already cached SPs. > + * Update the UUID when ID found. > + */ > + for (rx_desc_idx = 0; rx_desc_idx < count ; rx_desc_idx++) { > + desc_found = 0; > + > + /* > + * search the current ID in the cached partitions > + */ > + for (cached_desc_idx = 0; > + cached_desc_idx < ffa_priv_data.partitions.count; > + cached_desc_idx++) { > + /* > + * save the UUID > + */ > + if > (ffa_priv_data.partitions.descs[cached_desc_idx].info.id == > + parts_info[rx_desc_idx].id) { > + > ffa_priv_data.partitions.descs[cached_desc_idx].UUID = > + *part_uuid; > + > + desc_found = 1; > + break; > + } > + } > + > + if (!desc_found) > + return -ENODATA; > + } > + } > +#else > +#warning "arm_ffa: reading FFA_PARTITION_INFO_GET data not implemented" > +#endif > + > + return FFA_ERR_STAT_SUCCESS; > +} > + > +/** > + * ffa_query_partitions_info - invokes FFA_PARTITION_INFO_GET > + * and saves partitions > data > + * @part_uuid: Pointer to the partition(s) UUID > + * @pcount: Pointer to the number of partitions variable filled when querying > + * > + * This is the boot time function that executes the FFA_PARTITION_INFO_GET > + * to query the partitions data. Then, it calls ffa_read_partitions_info > + * to save the data in the private data structure. > + * > + * After reading the data the RX buffer is released using > ffa_release_rx_buffer > + * > + * Return: > + * > + * When part_uuid is NULL, all partitions data are retrieved from secure > world > + * When part_uuid is non NULL, data for partitions matching the given UUID > are > + * retrieved and the number of partitions is returned > + * FFA_ERR_STAT_SUCCESS is returned on success. Otherwise, failure > + */ > +static int ffa_query_partitions_info(union ffa_partition_uuid *part_uuid, > + u32 *pcount) > +{ > + unsigned long a0 = 0; > + union ffa_partition_uuid query_uuid = {0}; > + unsigned long a5 = 0; > + unsigned long a6 = 0; > + unsigned long a7 = 0; > + struct arm_smccc_res res = {0}; > + > + if (!ffa_priv_data.invoke_ffa_fn) > + panic("[FFA] no private data found\n"); > + > + a0 = FFA_PARTITION_INFO_GET; > + > + /* > + * If a UUID is specified. Information for one or more > + * partitions in the system is queried. Otherwise, information > + * for all installed partitions is queried > + */ > + > + if (part_uuid) { > + if (!pcount) > + return -EINVAL; > + > + query_uuid = *part_uuid; > + } > + > + ffa_priv_data.invoke_ffa_fn(a0, query_uuid.words.a1, > query_uuid.words.a2, > + query_uuid.words.a3, query_uuid.words.a4, > + a5, a6, a7, &res); > + > + switch (res.a0) { > + case FFA_ERROR: > + { > + switch (((int)res.a2)) { > + case FFA_ERR_STAT_INVALID_PARAMETERS: > + ffa_err("Unrecognized UUID"); > + return -EPERM; > + case FFA_ERR_STAT_NO_MEMORY: > + ffa_err("Results cannot fit in RX buffer of the > caller"); > + return -ENOMEM; > + case FFA_ERR_STAT_DENIED: > + ffa_err("Callee is not in a state to handle this > request"); > + return -EACCES; > + case FFA_ERR_STAT_NOT_SUPPORTED: > + ffa_err("This function is not implemented at this FF-A > instance"); > + return -EOPNOTSUPP; > + case FFA_ERR_STAT_BUSY: > + ffa_err("RX buffer of the caller is not free"); > + return -EBUSY; > + default: > + ffa_err("Undefined error (%d)", ((int)res.a2)); > + return -EINVAL; > + } > + } Same cases as above. Please map those in an array or something and just do a lookup to print an error > + case FFA_SUCCESS: > + { > + int ret; > + > + /* > + * res.a2 contains the count of partition information > descriptors > + * populated in the RX buffer > + */ > + if (res.a2) { > + ret = ffa_read_partitions_info(res.a2, part_uuid); > + if (ret) > + ffa_err("Failed to read partition(s) data , > error (%d)", ret); > + } > + > + /* > + * return the SP count > + */ > + if (part_uuid) { > + if (!ret) > + *pcount = res.a2; > + else > + *pcount = 0; > + } If I am following the code correctly this can be called with (NULL, NULL), which means that the previous !pcount check won't apply. Is there any other check I am missing? > + /* > + * After calling FFA_PARTITION_INFO_GET the buffer ownership > + * is assigned to the consumer (u-boot). So, we need to give > + * the ownership back to the secure world > + */ > + ret = ffa_release_rx_buffer(); > + > + if (!part_uuid && !res.a2) { > + ffa_err("[FFA] no partition installed in the system"); > + return -ENODEV; > + } > + > + return ret; > + } > + default: > + ffa_err("Undefined response function (0x%lx)", res.a0); > + return -EINVAL; > + } > +} > + > +/** [...] Regards /Ilias