Re: [PATCH v3 3/4] arm_ffa: introduce Arm FF-A low-level driver

[Simon Glass <sjg@chromium.org>]

Hi,

On Mon, 1 Aug 2022 at 11:21, Abdellatif El Khlifi
<abdellatif.elkhlifi@arm.com> wrote:
>
> 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 64-bit registers as per SMCCCv1.2 spec and comes
> on top of the SMCCC layer. The driver provides the FF-A ABIs needed for
> querying the FF-A framework from the secure world.
>
> 32-bit version of the ABIs is supported and 64-bit version of FFA_RXTX_MAP
> and FFA_MSG_SEND_DIRECT_{REQ, RESP}.
>
> 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 exports its operations to be used by upper layers.
>
> Exported operations:
>
> - partition_info_get
> - sync_send_receive
> - rxtx_unmap
>
> This implementation provides an optional feature to copy the driver data
> to EFI runtime area.
>
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Jens Wiklander <jens.wiklander@linaro.org>
> ---
>  MAINTAINERS                                |    6 +
>  common/board_r.c                           |    7 +
>  drivers/Kconfig                            |    2 +
>  drivers/Makefile                           |    1 +
>  drivers/arm-ffa/Kconfig                    |   33 +
>  drivers/arm-ffa/Makefile                   |    7 +
>  drivers/arm-ffa/arm-ffa-uclass.c           |   16 +
>  drivers/arm-ffa/arm_ffa_prv.h              |  219 ++++
>  drivers/arm-ffa/core.c                     | 1338 ++++++++++++++++++++
>  drivers/arm-ffa/efi_ffa_runtime_data_mgr.c |   94 ++
>  include/arm_ffa.h                          |  132 ++
>  include/dm/uclass-id.h                     |    1 +
>  include/uuid.h                             |    8 +
>  lib/efi_loader/efi_boottime.c              |   17 +
>  lib/uuid.c                                 |   65 +
>  15 files changed, 1946 insertions(+)
>  create mode 100644 drivers/arm-ffa/Kconfig
>  create mode 100644 drivers/arm-ffa/Makefile
>  create mode 100644 drivers/arm-ffa/arm-ffa-uclass.c
>  create mode 100644 drivers/arm-ffa/arm_ffa_prv.h
>  create mode 100644 drivers/arm-ffa/core.c
>  create mode 100644 drivers/arm-ffa/.c
>  create mode 100644 include/arm_ffa.h

Please add something to doc/ so people know what this is.

Since you are adding a new uclass you need a sandbox driver and tests.

The driver appears to have no operations, but there is a bus_ops. The
ops should go in the driver, I suspect, and should pass the device as
the first arg.

Can FFA_ERR_STAT_SUCCESS be 0 so you don't have to sprinkle the code with it?

Why is it using EFI things? Can this driver only be used with UEFI? I
hope not, if it is an official way of updating firmware.

Please don't add more things to board_r.c - we are trying to remove
this init over time. If it is a device it should be probed as needed.

Is there a device tree binding?

Also should this go in drivers/misc instead of creating a whole new subdir?

Regards,
Simon