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

Ilias Apalodimas
2022-05-13T14:23:32Z

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