Hi Harsimran, On Sat, 27 Jun 2026 at 17:44, Harsimran Singh Tungal <[email protected]> wrote: > > Enable MM variable services over FF-A after ExitBootServices > > Extend lib/efi_loader/efi_variable_tee.c to support FF-A > communication with the secure world during EFI runtime. Reuse the > statically reserved FF-A shared buffer after ExitBootServices(), > make the MM communication path runtime-safe so runtime variable > operations continue to reach the secure partition. > > Share the MM communication and MM SP notification helpers between the > boot and runtime paths instead of maintaining separate runtime-only > variants. Select dynamic allocation during boot and the fixed FF-A > shared buffer at runtime, and reject requests that would exceed the > shared buffer size. > > Mark the required code and data with __efi_runtime and > __efi_runtime_data, use range-based cache maintenance on the shared > buffer for the runtime FF-A path, and add the shared buffer to the EFI > runtime memory map. Document the FF-A shared MM buffer > cacheline-alignment requirement in Kconfig and add BUILD_BUG_ON() > checks in ffa_mm_communicate() for the FF-A shared buffer alignment > used by the arm64 cache-maintenance path. > > Reviewed-by: Simon Glass <[email protected]> > Signed-off-by: Harsimran Singh Tungal <[email protected]> > --- > arch/arm/cpu/armv8/cache.S | 8 + > arch/arm/cpu/armv8/cache_v8.c | 13 +- > lib/efi_loader/Kconfig | 4 + > lib/efi_loader/efi_variable_tee.c | 382 ++++++++++++++++++++++-------- > 4 files changed, 306 insertions(+), 101 deletions(-) > > diff --git a/arch/arm/cpu/armv8/cache.S b/arch/arm/cpu/armv8/cache.S > index c9e46859b4f..916558fe477 100644 > --- a/arch/arm/cpu/armv8/cache.S > +++ b/arch/arm/cpu/armv8/cache.S > @@ -169,7 +169,11 @@ ENDPROC(__asm_flush_l3_dcache) > * x0: start address > * x1: end address > */ > +#ifdef CONFIG_EFI_LOADER > +.pushsection .text.efi_runtime.__asm_flush_dcache_range, "ax" > +#else > .pushsection .text.__asm_flush_dcache_range, "ax" > +#endif > ENTRY(__asm_flush_dcache_range) > mrs x3, ctr_el0 > ubfx x3, x3, #16, #4 > @@ -195,7 +199,11 @@ ENDPROC(__asm_flush_dcache_range) > * x0: start address > * x1: end address > */ > +#ifdef CONFIG_EFI_LOADER > +.pushsection .text.efi_runtime.__asm_invalidate_dcache_range, "ax" > +#else > .pushsection .text.__asm_invalidate_dcache_range, "ax" > +#endif > ENTRY(__asm_invalidate_dcache_range) > mrs x3, ctr_el0 > ubfx x3, x3, #16, #4 > diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c > index 7c0e3f6d055..d150da4778e 100644 > --- a/arch/arm/cpu/armv8/cache_v8.c > +++ b/arch/arm/cpu/armv8/cache_v8.c > @@ -8,6 +8,7 @@ > */ > > #include <cpu_func.h> > +#include <efi_loader.h> > #include <hang.h> > #include <log.h> > #include <asm/cache.h> > @@ -855,7 +856,8 @@ inline void flush_dcache_all(void) > /* > * Invalidates range in all levels of D-cache/unified cache > */ > -void invalidate_dcache_range(unsigned long start, unsigned long stop) > +void __efi_runtime invalidate_dcache_range(unsigned long start, > + unsigned long stop) > { > __asm_invalidate_dcache_range(start, stop); > } > @@ -863,16 +865,19 @@ void invalidate_dcache_range(unsigned long start, > unsigned long stop) > /* > * Flush range(clean & invalidate) from all levels of D-cache/unified cache > */ > -void flush_dcache_range(unsigned long start, unsigned long stop) > +void __efi_runtime flush_dcache_range(unsigned long start, > + unsigned long stop) > { > __asm_flush_dcache_range(start, stop); > } > #else > -void invalidate_dcache_range(unsigned long start, unsigned long stop) > +void __efi_runtime invalidate_dcache_range(unsigned long start, > + unsigned long stop) > { > } > > -void flush_dcache_range(unsigned long start, unsigned long stop) > +void __efi_runtime flush_dcache_range(unsigned long start, > + unsigned long stop) > { > } > #endif /* CONFIG_SYS_DISABLE_DCACHE_OPS */
This needs to be a patch of it's own explaining *why* having these available at runtime is a needed. [...] > @@ -4,7 +4,7 @@ > * > * Copyright (C) 2019 Linaro Ltd. <[email protected]> > * Copyright (C) 2019 Linaro Ltd. <[email protected]> > - * Copyright 2022-2023 Arm Limited and/or its affiliates > <[email protected]> > + * Copyright 2022-2026 Arm Limited and/or its affiliates > <[email protected]> > * > * Authors: > * Abdellatif El Khlifi <[email protected]> > @@ -14,6 +14,7 @@ > > #if CONFIG_IS_ENABLED(ARM_FFA_TRANSPORT) > #include <arm_ffa.h> > +#include <arm_ffa_runtime.h> > #endif > #include <cpu_func.h> > #include <dm.h> > @@ -21,6 +22,8 @@ > #include <efi_api.h> > #include <efi_loader.h> > #include <efi_variable.h> > +#include <linux/build_bug.h> > +#include <linux/kernel.h> > #include <malloc.h> > #include <mapmem.h> > #include <mm_communication.h> > @@ -34,20 +37,49 @@ > #define MM_DENIED (-3) > #define MM_NO_MEMORY (-5) These need to be removed now with the enum > > +/* > + * MM_* return codes are negative. Use -MM_* as sparse positive indices so > + * ffa_map_sp_event() can look up mm_sp_errmap[-sp_event_ret]. Unassigned > + * slots remain 0 and are treated as unmapped MM return codes. > + */ > +static const int __efi_runtime_rodata mm_sp_errmap[] = { > + [-MM_NOT_SUPPORTED] = -EINVAL, > + [-MM_INVALID_PARAMETER] = -EPERM, > + [-MM_DENIED] = -EACCES, > + [-MM_NO_MEMORY] = -EBUSY, > +}; [...] > +static void *__efi_runtime_data ffa_shared_buf; > 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 */ > +static efi_uintn_t __efi_runtime_data max_buffer_size; /* comm + var + func > + data */ > +static efi_uintn_t __efi_runtime_data max_payload_size; /* func + > data */ > static const u16 __efi_runtime_rodata pk[] = u"PK"; > +static bool __efi_runtime_data ebs_called; > > struct mm_connection { > struct udevice *tee; > u32 session; > }; > > +/** > + * efi_at_runtime() - Indicate whether the system is in the UEFI runtime > phase > + * > + * This helper returns whether the firmware has transitioned into the > + * UEFI runtime phase, meaning that ExitBootServices() has been invoked. > + * > + * Return: > + * true - The system is operating in UEFI runtime mode. > + * false - The system is still in the boot services phase. > + */ > +static bool __efi_runtime efi_at_runtime(void) > +{ > + return ebs_called; > +} > + > /** > * get_connection() - Retrieve OP-TEE session for a specific UUID. > * > @@ -169,6 +201,28 @@ static efi_status_t optee_mm_communicate(void *comm_buf, > ulong dsize) > } > [...] > - > - virt_shared_buf = > map_sysmem((phys_addr_t)CONFIG_FFA_SHARED_MM_BUF_ADDR, 0); > - memcpy(virt_shared_buf, comm_buf, tx_data_size); > + if (at_runtime) { > + shared_buf = comm_buf; > + } else { > + /* Copy the data to the shared buffer */ > + shared_buf = > map_sysmem((phys_addr_t)CONFIG_FFA_SHARED_MM_BUF_ADDR, 0); > + memcpy(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. > + * Shared buffer cache maintenance for FF-A / OP-TEE communication: > + * > + * NS -> S (request path): > + * > + * The non-secure side populates the shared buffer. If the buffer is > cached > + * in NS, the updated bytes may reside in dirty D-cache lines and not > yet be > + * visible in DDR. Since the secure world typically reads the shared > buffer > + * directly from DDR (e.g. with caches disabled / non-coherent > mapping), we > + * must clean the corresponding cache lines to the Point of Coherency > (PoC) > + * before entering secure world. > + * > + * S -> NS (response path): > + * > + * The secure world may update the same shared buffer in DDR. After > returning > + * to non-secure, any cached copies of that region in NS may be > stale. We > + * therefore invalidate the shared buffer range after the FF-A call > to drop > + * those lines and force subsequent reads to fetch the latest data > from DDR. > + * > + * Note: Whole-cache invalidation must not be used in EFI runtime > context. > + * After ExitBootServices(), the OS owns the cache hierarchy; global > + * invalidation could drop OS dirty lines and violate the OS coherency > + * model. Always operate on the shared buffer range only. > */ > -#ifdef CONFIG_ARM64 > - invalidate_dcache_all(); > -#endif > + if (IS_ENABLED(CONFIG_ARM64)) { > + BUILD_BUG_ON(CONFIG_FFA_SHARED_MM_BUF_ADDR % > + CONFIG_SYS_CACHELINE_SIZE); > + BUILD_BUG_ON(CONFIG_FFA_SHARED_MM_BUF_SIZE % > + CONFIG_SYS_CACHELINE_SIZE); > + flush_dcache_range((unsigned long)shared_buf, > + (unsigned long)(shared_buf + > + tx_cache_size)); > + } > > /* Announce there is data in the shared buffer */ > - > ffa_ret = ffa_notify_mm_sp(); > > 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 + > + ulong rx_cache_size; > + > + if (IS_ENABLED(CONFIG_ARM64)) > + invalidate_dcache_range((unsigned long)shared_buf, > + (unsigned long)(shared_buf + > + hdr_cache_size)); > + > + rx_data_size = ((struct efi_mm_communicate_header > *)shared_buf)->message_len + > sizeof(efi_guid_t) + > sizeof(size_t); > > - if (rx_data_size > comm_buf_size) { > + if (rx_data_size > comm_buf_size || > + rx_data_size > CONFIG_FFA_SHARED_MM_BUF_SIZE) { > efi_ret = EFI_OUT_OF_RESOURCES; > break; > } > > - memcpy(comm_buf, virt_shared_buf, rx_data_size); > + if (IS_ENABLED(CONFIG_ARM64)) { > + rx_cache_size = ALIGN(rx_data_size, > + CONFIG_SYS_CACHELINE_SIZE); > + if (rx_cache_size > hdr_cache_size) > + invalidate_dcache_range((unsigned > long)(shared_buf + > + hdr_cache_size), > + (unsigned long)(shared_buf + > + rx_cache_size)); > + } > + > + if (!at_runtime) > + memcpy(comm_buf, shared_buf, rx_data_size); > efi_ret = EFI_SUCCESS; > break; > } > @@ -356,41 +443,45 @@ static efi_status_t ffa_mm_communicate(void *comm_buf, > ulong comm_buf_size) > efi_ret = EFI_ACCESS_DENIED; > } > > - unmap_sysmem(virt_shared_buf); > + if (!at_runtime) > + unmap_sysmem(shared_buf); > return efi_ret; > } > > /** > * get_mm_comms() - detect the available MM transport > * > - * Make sure the FF-A bus is probed successfully > - * which means FF-A communication with secure world works and ready > - * for use. > + * Make sure the FF-A bus is probed successfully during the boot phase, > + * which means FF-A communication with secure world works and is ready for > + * use. During the runtime phase, only the FF-A runtime transport can be > + * selected. > * > - * If FF-A bus is not ready, use OPTEE comms. > + * If FF-A bus is not ready at boot, use OP-TEE comms. > * > - * Return: > - * > - * MM_COMMS_FFA or MM_COMMS_OPTEE > + * Return: MM_COMMS_FFA, MM_COMMS_OPTEE, or MM_COMMS_UNDEFINED > */ > -static enum mm_comms_select get_mm_comms(void) > +static enum mm_comms_select __efi_runtime get_mm_comms(void) > { > struct udevice *dev; > int ret; > > + if (efi_at_runtime()) { > + if (IS_ENABLED(CONFIG_ARM_FFA_RT_MODE)) > + return MM_COMMS_FFA; > + return MM_COMMS_UNDEFINED; Why undefined? It's either backed by OP-TEE or an FF-A SP. Later down the road you are changing the efi_runtime_services.get_variable to point to this efi_query_variable_info_int_runtime(). if we are at runtime ARM_FFA_TRANSPORT is enabled and CONFIG_ARM_FFA_RT_MODE is not enabled the op-tee path will never run. > + > + /* Record that ExitBootServices() has been called */ > + ebs_called = true; > +} > + > +/** > + * ffa_shared_buf_notify_virtual_address_map() - SetVirtualAddressMap > callback > + * > + * @event: callback event > + * @context: callback context > + */ > +static void EFIAPI __efi_runtime > +ffa_shared_buf_notify_virtual_address_map(struct efi_event *event, void > *context) > +{ > + efi_convert_pointer(0, (void **)&ffa_shared_buf); > } > > /** > @@ -992,6 +1149,7 @@ void efi_variables_boot_exit_notify(void) > efi_status_t efi_init_variables(void) > { > efi_status_t ret; > + struct efi_event *event; > > /* Create a cached copy of the variables that will be enabled on > ExitBootServices() */ > ret = efi_var_mem_init(); > @@ -1010,5 +1168,35 @@ efi_status_t efi_init_variables(void) > if (ret != EFI_SUCCESS) > return ret; > > + if (IS_ENABLED(CONFIG_ARM_FFA_RT_MODE)) { > + /* > + * The FF-A shared buffer is accessed by EFI runtime > services, so > + * keep the resident pointer convertible across > + * SetVirtualAddressMap() and mark the region as runtime > memory. > + * > + * CONFIG_FFA_SHARED_MM_BUF_ADDR is expected to be EFI-page > aligned. > + */ > + BUILD_BUG_ON(CONFIG_FFA_SHARED_MM_BUF_ADDR & EFI_PAGE_MASK); > + ffa_shared_buf = (void *)CONFIG_FFA_SHARED_MM_BUF_ADDR; > + ret = efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, > + TPL_CALLBACK, > + > ffa_shared_buf_notify_virtual_address_map, > + NULL, NULL, &event); > + if (ret != EFI_SUCCESS) > + return ret; > + ret = efi_add_memory_map(CONFIG_FFA_SHARED_MM_BUF_ADDR, > + CONFIG_FFA_SHARED_MM_BUF_SIZE, > + EFI_RUNTIME_SERVICES_DATA); > + if (ret != EFI_SUCCESS) { > + efi_close_event(event); > + log_err("EFI: failed to add FF-A shared buffer to > runtime map (%lu)\n", > + ret); > + return ret; > + } > + log_info("EFI: FF-A shared buffer runtime map: addr=0x%lx > size=0x%lx\n", > + (ulong)CONFIG_FFA_SHARED_MM_BUF_ADDR, > + (ulong)CONFIG_FFA_SHARED_MM_BUF_SIZE); > + } > + > return EFI_SUCCESS; > } > -- > 2.34.1 > I am trying to go throughh the patches, but they are way too big for proper review. Please split in a number of independently reviewable patches.
