Re: [PATCH v2 1/6] arm_ffa: introduce Arm FF-A low-level driver

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Abdellatif
On Fri, Apr 15, 2022 at 01:27:58PM +0100, abdellatif.elkhlifi@arm.com wrote:
> From: Abdellatif El Khlifi <abdellatif.elkhlifi@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.elkhlifi@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.elkhlifi@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.elkhlifi@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.elkhlifi@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.elkhlifi@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.elkhlifi@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