[PATCH v5 09/10] arm_ffa: introduce FF-A MM communication
Ilias Apalodimas
ilias.apalodimas at linaro.org
Thu Sep 29 11:32:42 CEST 2022
Hi Abdellatif,
> --- a/arch/arm/cpu/armv8/cache.S
> +++ b/arch/arm/cpu/armv8/cache.S
> @@ -3,6 +3,9 @@
> * (C) Copyright 2013
> * David Feng <fenghua at phytium.com.cn>
> *
> + * (C) Copyright 2022 ARM Limited
> + * Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> + *
> * This file is based on sample code from ARMv8 ARM.
> */
>
> @@ -21,7 +24,11 @@
> * x1: 0 clean & invalidate, 1 invalidate only
> * x2~x9: clobbered
> */
> +#ifdef CONFIG_EFI_LOADER
> +.pushsection .text.efi_runtime, "ax"
Maybe we discussed this in the past and I forgot, but why would you need
__asm_dcache_level, __asm_dcache_all, __asm_invalidate_dcache_alla etc in
the runtime section ?
> +#else
> .pushsection .text.__asm_dcache_level, "ax"
> +#endif
> ENTRY(__asm_dcache_level)
> lsl x12, x0, #1
> msr csselr_el1, x12 /* select cache level */
> @@ -65,7 +72,11 @@ ENDPROC(__asm_dcache_level)
> *
> * flush or invalidate all data cache by SET/WAY.
> */
> +#ifdef CONFIG_EFI_LOADER
> +.pushsection .text.efi_runtime, "ax"
> +#else
> .pushsection .text.__asm_dcache_all, "ax"
> +#endif
> ENTRY(__asm_dcache_all)
> mov x1, x0
> dsb sy
> @@ -109,7 +120,11 @@ ENTRY(__asm_flush_dcache_all)
> ENDPROC(__asm_flush_dcache_all)
> .popsection
>
> +#ifdef CONFIG_EFI_LOADER
> +.pushsection .text.efi_runtime, "ax"
> +#else
> .pushsection .text.__asm_invalidate_dcache_all, "ax"
> +#endif
> ENTRY(__asm_invalidate_dcache_all)
> mov x0, #0x1
> b __asm_dcache_all
> @@ -182,7 +197,11 @@ ENTRY(__asm_invalidate_icache_all)
> ENDPROC(__asm_invalidate_icache_all)
> .popsection
>
> +#ifdef CONFIG_EFI_LOADER
> +.pushsection .text.efi_runtime, "ax"
> +#else
> .pushsection .text.__asm_invalidate_l3_dcache, "ax"
> +#endif
> WEAK(__asm_invalidate_l3_dcache)
> mov x0, #0 /* return status as success */
> ret
> diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
> index e4736e5643..45f57372c2 100644
> --- a/arch/arm/cpu/armv8/cache_v8.c
> +++ b/arch/arm/cpu/armv8/cache_v8.c
> @@ -5,10 +5,14 @@
> *
> * (C) Copyright 2016
> * Alexander Graf <agraf at suse.de>
> + *
> + * (C) Copyright 2022 ARM Limited
> + * Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> */
>
> #include <common.h>
> #include <cpu_func.h>
> +#include <efi_loader.h>
> #include <hang.h>
> #include <log.h>
> #include <asm/cache.h>
> @@ -445,7 +449,7 @@ __weak void mmu_setup(void)
> /*
> * Performs a invalidation of the entire data cache at all levels
> */
> -void invalidate_dcache_all(void)
> +void __efi_runtime invalidate_dcache_all(void)
> {
> __asm_invalidate_dcache_all();
> __asm_invalidate_l3_dcache();
> diff --git a/include/mm_communication.h b/include/mm_communication.h
> index e65fbde60d..fe9104c56d 100644
> --- a/include/mm_communication.h
[...]
> * always begin with efi_mm_communicate_header.
> */
> -struct __packed efi_mm_communicate_header {
> +struct efi_mm_communicate_header {
> efi_guid_t header_guid;
> size_t message_len;
> u8 data[];
> @@ -145,7 +150,7 @@ struct smm_variable_communicate_header {
> * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE.
> *
> */
> -struct smm_variable_access {
> +struct __packed smm_variable_access {
You are randomly adding and deleting __packed cwin both structs. But you can't do that.
Those structs are defined in StandAloneMM. This is the reason each struct
description has the corresponding EDK2 definition.
> efi_guid_t guid;
> efi_uintn_t data_size;
> efi_uintn_t name_size;
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index b8fb2701a7..d292f57244 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -61,13 +61,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
I think we discussed this in the past. I don't want ifdefs in the code for
that. FF-A is a discoverable bus. So I think what we should do here, is
fire up the variable access driver -> probe for FF-A -> decide what to use.
So it looks something like
**************************
**** Variable Driver *****
**************************
|
|
|
***************************
****** Discover FF-A ******
***************************
| |
| yes | No
| |
Use SP Use OP-TEE
> 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_boottime.c b/lib/efi_loader/efi_boottime.c
> index d404343a7d..8a397ea21b 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -2193,6 +2193,13 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
> debug("[efi_boottime][INFO]: FF-A RX/TX buffers unmapped\n");
> #endif
>
> +#if CONFIG_IS_ENABLED(ARM_FFA_EFI_RUNTIME_MODE) && !CONFIG_IS_ENABLED(SANDBOX_FFA)
> + if (ffa_copy_runtime_data())
> + printf("ERROR: EFI: FFA: copying runtime data\n");
> + else
> + printf("INFO: EFI: FFA: runtime data copied\n");
> +#endif
> +
> /* Patch out unsupported runtime function */
> efi_runtime_detach();
>
> diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> index dfef18435d..d6f24f85bd 100644
> --- a/lib/efi_loader/efi_variable_tee.c
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -4,6 +4,8 @@
> *
> * Copyright (C) 2019 Linaro Ltd. <sughosh.ganu at linaro.org>
> * Copyright (C) 2019 Linaro Ltd. <ilias.apalodimas at linaro.org>
> + * Copyright (C) 2022 ARM Limited
> + * Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> */
>
> #include <common.h>
> @@ -15,6 +17,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
> +#warning "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
> +#warning "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
> +#warning "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)
> +
> +const char *mm_sp_svc_uuid = MM_SP_UUID;
> +
> +static __efi_runtime_data 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 +56,7 @@ struct mm_connection {
> u32 session;
> };
>
> +#if (IS_ENABLED(CONFIG_OPTEE))
> /**
> * get_connection() - Retrieve OP-TEE session for a specific UUID.
> *
> @@ -143,16 +176,227 @@ static efi_status_t optee_mm_communicate(void *comm_buf, ulong dsize)
>
> return ret;
> }
> +#endif
> +
> +#if (IS_ENABLED(CONFIG_ARM_FFA_TRANSPORT))
>
> /**
> - * mm_communicate() - Adjust the cmonnucation buffer to StandAlonneMM and send
> + * ffa_notify_mm_sp() - Announce there is data in the shared buffer
> + *
> + * Notifies the MM partition in the trusted world that
> + * data is available in the shared buffer.
> + * This is a blocking call during which trusted world has exclusive access
> + * to the MM shared buffer.
> + *
> + * Return:
> + *
> + * 0 on success
> + */
> +static int __efi_runtime ffa_notify_mm_sp(void)
> +{
> + struct ffa_send_direct_data msg = {0};
> + int ret;
> + int sp_event_ret = -1;
> +
> + if (!ffa_bus_ops_get())
> + return -EINVAL;
> +
> + msg.data0 = FFA_SHARED_MM_BUFFER_OFFSET; /* x3 */
> +
> + ret = ffa_bus_ops_get()->sync_send_receive(mm_sp_id, &msg);
> + if (ret != 0)
> + return ret;
> +
> + sp_event_ret = msg.data0; /* x3 */
> +
> + if (sp_event_ret == MM_SUCCESS)
> + return 0;
> +
> + /*
> + * Failure to notify the MM SP
> + */
> +
> + return -EACCES;
> +}
> +
> +/**
> + * ffa_discover_mm_sp_id() - Query the MM partition ID
> + *
> + * Use the FF-A driver to get the MM partition ID.
> + * If multiple partitions are found, use the first one.
> + * This is a boot time function.
> + *
> + * Return:
> + *
> + * 0 on success
> + */
> +static int ffa_discover_mm_sp_id(void)
> +{
> + u32 count = 0, size = 0;
> + int ret;
> + struct ffa_partition_info *parts_info;
> +
> + if (!ffa_bus_ops_get())
> + return -EINVAL;
> +
> + /*
> + * get from the driver the count of the SPs matching the UUID
> + */
> + ret = ffa_bus_ops_get()->partition_info_get(mm_sp_svc_uuid, &count, NULL);
> + if (ret != 0) {
> + log_err("EFI: Failure in querying partitions count (error code: %d)\n", ret);
> + return ret;
> + }
> +
> + if (!count) {
> + log_info("EFI: No MM partition found\n");
> + return ret;
> + }
> +
> + /*
> + * pre-allocate a buffer to be filled by the driver
> + * with ffa_partition_info structs
> + */
> +
> + log_info("EFI: Pre-allocating %d partition(s) info structures\n", count);
> +
> + parts_info = calloc(count, sizeof(struct ffa_partition_info));
> + if (!parts_info)
> + return -EINVAL;
> +
> + size = count * sizeof(struct ffa_partition_info);
> +
> + /*
> + * ask the driver to fill the
> + * buffer with the SPs info
> + */
> + ret = ffa_bus_ops_get()->partition_info_get(mm_sp_svc_uuid, &size, parts_info);
> + if (ret != 0) {
> + log_err("EFI: Failure in querying partition(s) info (error code: %d)\n", ret);
> + free(parts_info);
> + return ret;
> + }
> +
> + /*
> + * MM SPs found , use the first one
> + */
> +
> + mm_sp_id = parts_info[0].id;
> +
> + log_info("EFI: MM partition ID 0x%x\n", mm_sp_id);
> +
> + free(parts_info);
> +
> + return 0;
> +}
> +
> +/**
> + * ffa_mm_communicate() - Exchange EFI services data with the MM partition using FF-A
> + * @comm_buf: locally allocated communication buffer used for rx/tx
> + * @dsize: communication buffer size
> + *
> + * Issues a door bell event to notify the MM partition (SP) running in OP-TEE
> + * that there is data to read from the shared buffer.
> + * Communication with the MM SP is performed using FF-A transport.
> + * On the event, MM SP can read the data from the buffer and
> + * update the MM shared buffer with response data.
> + * The response data is copied back to the communication buffer.
> + *
> + * Return:
> + *
> + * EFI status code
> + */
> +static efi_status_t __efi_runtime ffa_mm_communicate(void *comm_buf, ulong comm_buf_size)
> +{
If you are registering this as a *runtime* service you should also tweak
the RT_PROPERTIES TABLE and enable runtime services (e.g SetVariable).
Otherwise having as runtime makes little sense. To be honest I'd much
prefer this whole patchset without the runtime parts. It's much easier to
review and reason about and we can always add runtime later.
> + ulong tx_data_size;
> + int ffa_ret;
> + struct efi_mm_communicate_header *mm_hdr;
> + void *virt_shared_buf;
> +
> + if (!comm_buf)
> + return EFI_INVALID_PARAMETER;
> +
> + /* Discover MM partition ID at boot time */
> + if (!mm_sp_id && ffa_discover_mm_sp_id() != 0) {
> + log_err("EFI: Failure to discover MM partition ID at boot time\n");
> + return EFI_UNSUPPORTED;
> + }
> +
> + mm_hdr = (struct efi_mm_communicate_header *)comm_buf;
> + tx_data_size = mm_hdr->message_len + sizeof(efi_guid_t) + sizeof(size_t);
> +
> + if (comm_buf_size != tx_data_size || tx_data_size > FFA_SHARED_MM_BUFFER_SIZE)
> + return EFI_INVALID_PARAMETER;
> +
> + /* Copy the data to the shared buffer */
> +
> + virt_shared_buf = (void *)map_sysmem((phys_addr_t)FFA_SHARED_MM_BUFFER_ADDR, 0);
> + efi_memcpy_runtime(virt_shared_buf, comm_buf, tx_data_size);
> +
> + /*
> + * The secure world might have cache disabled for
> + * the device region used for shared buffer (which is the case for Optee).
> + * In this case, the secure world reads the data from DRAM.
> + * Let's flush the cache so the DRAM is updated with the latest data.
> + */
> + #ifdef CONFIG_ARM64
> + invalidate_dcache_all();
> + #endif
> +
> + /* Announce there is data in the shared buffer */
> +
> + ffa_ret = ffa_notify_mm_sp();
> + if (ffa_ret)
> + unmap_sysmem(virt_shared_buf);
> +
> + switch (ffa_ret) {
> + case 0:
> + {
> + ulong rx_data_size;
> + /* Copy the MM SP response from the shared buffer to the communication buffer */
> + rx_data_size = ((struct efi_mm_communicate_header *)virt_shared_buf)->message_len +
> + sizeof(efi_guid_t) +
> + sizeof(size_t);
> +
> + if (rx_data_size > comm_buf_size) {
> + unmap_sysmem(virt_shared_buf);
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + efi_memcpy_runtime(comm_buf, virt_shared_buf, rx_data_size);
> + unmap_sysmem(virt_shared_buf);
> +
> + return EFI_SUCCESS;
> + }
> + case -EINVAL:
> + return EFI_DEVICE_ERROR;
> + case -EPERM:
> + return EFI_INVALID_PARAMETER;
> + case -EACCES:
> + return EFI_ACCESS_DENIED;
> + case -EBUSY:
> + return EFI_OUT_OF_RESOURCES;
> + default:
> + return EFI_ACCESS_DENIED;
> + }
> +}
> +#endif
> +
> +/**
> + * mm_communicate() - Adjust the communication buffer to the MM SP and send
> * it to OP-TEE
> *
> - * @comm_buf: locally allocted communcation buffer
> + * @comm_buf: locally allocated communication buffer
> * @dsize: buffer size
> + *
> + * The MM SP (also called partition) can be StandAlonneMM or smm-gateway.
> + * The comm_buf format is the same for both partitions.
> + * When using the u-boot OP-TEE driver, StandAlonneMM is supported.
> + * When using the u-boot FF-A driver, StandAlonneMM and smm-gateway are supported.
> + *
> * Return: status code
> */
> -static efi_status_t mm_communicate(u8 *comm_buf, efi_uintn_t dsize)
> +static efi_status_t __efi_runtime mm_communicate(u8 *comm_buf, efi_uintn_t dsize)
The optee version of the function can't be on the efi_runtime section. The
reason is that almost the entire of the OP-TEE subsystem is not marked as
runtime, which will lead to a crash if we ever take that path.
> {
> efi_status_t ret;
> struct efi_mm_communicate_header *mm_hdr;
> @@ -162,7 +406,11 @@ static efi_status_t mm_communicate(u8 *comm_buf, efi_uintn_t dsize)
> mm_hdr = (struct efi_mm_communicate_header *)comm_buf;
> var_hdr = (struct smm_variable_communicate_header *)mm_hdr->data;
>
> + #if (IS_ENABLED(CONFIG_OPTEE))
> ret = optee_mm_communicate(comm_buf, dsize);
> + #elif (IS_ENABLED(CONFIG_ARM_FFA_TRANSPORT))
> + ret = ffa_mm_communicate(comm_buf, dsize);
If we enable runtime discovery of optee vs ffa, we can get rid of that and
just pass an extra argument to mm_communicate
Thanks
/Ilias
> + #endif
> if (ret != EFI_SUCCESS) {
> log_err("%s failed!\n", __func__);
> return ret;
> @@ -258,6 +506,13 @@ efi_status_t EFIAPI get_max_payload(efi_uintn_t *size)
> goto out;
> }
> *size = var_payload->size;
> +
> + #if (IS_ENABLED(CONFIG_ARM_FFA_TRANSPORT))
> + if (*size > FFA_SHARED_MM_BUFFER_SIZE)
> + *size = FFA_SHARED_MM_BUFFER_SIZE - MM_COMMUNICATE_HEADER_SIZE -
> + MM_VARIABLE_COMMUNICATE_SIZE;
> + #endif
> +
> /*
> * There seems to be a bug in EDK2 miscalculating the boundaries and
> * size checks, so deduct 2 more bytes to fulfill this requirement. Fix
> @@ -697,7 +952,7 @@ void efi_variables_boot_exit_notify(void)
> ret = EFI_NOT_FOUND;
>
> if (ret != EFI_SUCCESS)
> - log_err("Unable to notify StMM for ExitBootServices\n");
> + log_err("Unable to notify the MM partition for ExitBootServices\n");
> free(comm_buf);
>
> /*
> --
> 2.17.1
>
More information about the U-Boot
mailing list