On 2026-07-01 16:56 +0300, Ilias Apalodimas wrote: > 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. > Thanks Ilias.
I have split the larger patches into smaller independently reviewable commits in the v4 patchset. Regards Harsimran Singh Tungal
