[PATCH v12 09/10] arm_ffa: efi: introduce FF-A MM communication

[Ilias Apalodimas]

Hi Abdellatif

I still have some concerns on this

In the past [0] I asking why this needs to be a Kconfig option.  Since FF-A
is a mechanism we can use to discover SPs, in theory we dont need the
ifdefery.  We might need something if the code difference grows too much
but I think we are fine for now. 

On top of that I am nissing how was this tested?  did you test the current
SMC to optee and the stmm for the RPMB backend?

> +++ b/include/mm_communication.h
> @@ -6,6 +6,9 @@
>   *  Copyright (c) 2017, Intel Corporation. All rights reserved.
>   *  Copyright (C) 2020 Linaro Ltd. <sughosh.ganu at linaro.org>
>   *  Copyright (C) 2020 Linaro Ltd. <ilias.apalodimas at linaro.org>
> + *  Copyright 2022-2023 Arm Limited and/or its affiliates <open-source-office at arm.com>
> + *    Authors:
> + *      Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
>   */
>  
>  #ifndef _MM_COMMUNICATION_H_
> @@ -13,6 +16,9 @@
>  
>  #include <part_efi.h>
>  
> +/* MM service UUID string (big-endian format). This UUID is  common across all MM SPs */
> +#define MM_SP_UUID	"33d532ed-e699-0942-c09c-a798d9cd722d"
> +
>  /*
>   * Interface to the pseudo Trusted Application (TA), which provides a
>   * communication channel with the Standalone MM (Management Mode)
> @@ -248,4 +254,11 @@ struct smm_variable_var_check_property {
>  	u16                       name[];
>  };
>  
> +/* supported MM transports */
> +enum mm_comms_select {
> +	MM_COMMS_UNDEFINED,
> +	MM_COMMS_FFA,
> +	MM_COMMS_OPTEE
> +};
> +
>  #endif /* _MM_COMMUNICATION_H_ */
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index c5835e6ef6..08a6b84101 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -55,13 +55,23 @@ config EFI_VARIABLE_FILE_STORE
>  	  stored as file /ubootefi.var on the EFI system partition.
>  
>  config EFI_MM_COMM_TEE
> -	bool "UEFI variables storage service via OP-TEE"
> -	depends on OPTEE
> +	bool "UEFI variables storage service via the trusted world"
> +	depends on OPTEE || ARM_FFA_TRANSPORT
>  	help
> +	  Allowing access to the MM SP services (SPs such as  StandAlonneMM, smm-gateway).
> +	  When using the u-boot OP-TEE driver, StandAlonneMM is supported.
> +	  When using the u-boot FF-A  driver any MM SP is supported.
> +
>  	  If OP-TEE is present and running StandAloneMM, dispatch all UEFI
>  	  variable related operations to that. The application will verify,
>  	  authenticate and store the variables on an RPMB.
>  
> +	  When ARM_FFA_TRANSPORT is used, dispatch all UEFI variable related
> +	  operations to the MM SP running in the secure world.
> +	  A door bell mechanism is used to notify the SP when there is data in the shared
> +	  MM buffer. The data is copied by u-boot to the shared buffer before issuing
> +	  the door bell event.
> +
>  config EFI_VARIABLE_NO_STORE
>  	bool "Don't persist non-volatile UEFI variables"
>  	help
> diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> index dfef18435d..2effb705d7 100644
> --- a/lib/efi_loader/efi_variable_tee.c
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -4,9 +4,14 @@
>   *
>   *  Copyright (C) 2019 Linaro Ltd. <sughosh.ganu at linaro.org>
>   *  Copyright (C) 2019 Linaro Ltd. <ilias.apalodimas at linaro.org>
> + *  Copyright 2022-2023 Arm Limited and/or its affiliates <open-source-office at arm.com>
> + *
> + *  Authors:
> + *    Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
>   */
>  
>  #include <common.h>
> +#include <dm.h>
>  #include <efi.h>
>  #include <efi_api.h>
>  #include <efi_loader.h>
> @@ -15,6 +20,36 @@
>  #include <malloc.h>
>  #include <mm_communication.h>
>  
> +#if IS_ENABLED(CONFIG_ARM_FFA_TRANSPORT)
> +
> +#include <arm_ffa.h>
> +#include <cpu_func.h>
> +#include <mapmem.h>
> +
> +#ifndef FFA_SHARED_MM_BUFFER_SIZE
> +#error "FFA_SHARED_MM_BUFFER_SIZE must be defined in include/configs/<board>.h"
> +#define FFA_SHARED_MM_BUFFER_SIZE 0
> +#endif
> +
> +#ifndef FFA_SHARED_MM_BUFFER_OFFSET
> +#error "FFA_SHARED_MM_BUFFER_OFFSET must be defined in include/configs/<board>.h"
> +#define FFA_SHARED_MM_BUFFER_OFFSET 0
> +#endif
> +
> +#ifndef FFA_SHARED_MM_BUFFER_ADDR
> +#error "FFA_SHARED_MM_BUFFER_ADDR must be defined in include/configs/<board>.h"
> +#define FFA_SHARED_MM_BUFFER_ADDR 0
> +#endif
> +
> +/* MM return codes */
> +#define MM_SUCCESS (0)
> +
> +static const char *mm_sp_svc_uuid = MM_SP_UUID;
> +
> +static u16 mm_sp_id;
> +
> +#endif
> +
>  extern struct efi_var_file __efi_runtime_data *efi_var_buf;
>  static efi_uintn_t max_buffer_size;	/* comm + var + func + data */
>  static efi_uintn_t max_payload_size;	/* func + data */
> @@ -24,6 +59,7 @@ struct mm_connection {
>  	u32 session;
>  };
>  
> +#if (IS_ENABLED(CONFIG_OPTEE))

>  /**
>   * get_connection() - Retrieve OP-TEE session for a specific UUID.
>   *

[...]

Isn't this problematic?  At the moment there's not > 8.4 hardware out
there.  As a result the SPMC *is* implemented in OP-TEE.  So how do we
expect FF-A to work?

[...]

[0] https://lore.kernel.org/u-boot/Y3NV991bKRgas1zZ@hera/

Thanks
/Ilias