[PATCH v2 1/6] arm_ffa: introduce Arm FF-A low-level driver
Abdellatif El Khlifi
abdellatif.elkhlifi at arm.com
Mon Sep 26 12:42:16 CEST 2022
On Fri, May 13, 2022 at 05:23:19PM +0300, Ilias Apalodimas wrote:
> Hi Abdellatif
> On Fri, Apr 15, 2022 at 01:27:58PM +0100, abdellatif.elkhlifi at arm.com wrote:
> > From: Abdellatif El Khlifi <abdellatif.elkhlifi at 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 at 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 at 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 at 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 at 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 at 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 at 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
Thanks for all your feedbacks. Error handling and panics removal have been addressed in v4 patchset. Now we use a mapping mechanism and no panics anymore.
please check: https://lore.kernel.org/all/20220926101723.9965-5-abdellatif.elkhlifi@arm.com/
>
> > + }
> > + 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
>
addressed in v4 patchset, please check: https://lore.kernel.org/all/20220926101723.9965-5-abdellatif.elkhlifi@arm.com/
> > +
> > + 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
>
In v4 the core driver is no longer dependent on EFI. The intent is to be able to compile it
without EFI support. The use of EFI APIs has been dropped and replaced with generic u-boot
APIs.
When using EFI, FF-A core driver data needs to be copied to the EFI runtime section so it can be
accessed at runtime. For that specific case, a new CONFIG_ARM_FFA_EFI_RUNTIME_MODE
has been created to enable the feature of copying the data to the runtime section.
The code that copies the data is provided by drivers/firmware/arm-ffa/efi_ffa_runtime_data_mgr.c
When EFI is needed CONFIG_ARM_FFA_EFI_RUNTIME_MODE should be turned on. If no EFI needed
the FF-A driver can still be used independently.
> > + 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?
>
addressed in v4 patchset, please check: https://lore.kernel.org/all/20220926101723.9965-5-abdellatif.elkhlifi@arm.com/
> > + /*
> > + * 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
More information about the U-Boot
mailing list