Hi Harsimran, > On 2026-04-24T17:31:50, Harsimran Singh Tungal > <[email protected]> wrote: > > efi_loader: align FF-A cache maintenance with runtime path > > > > Match boot-time FF-A cache handling to runtime behavior > > > > The boot-time FF-A MM communication path used invalidate_dcache_all() > > after copying the message into the shared buffer. This differs from the > > runtime path, which performs range-based maintenance to avoid global cache > > operations. > > > > Update ffa_mm_communicate() to use the same pattern as the runtime helper: > > clean the shared buffer range before the SMC and invalidate the same range > > after the response. This keeps boot-time and runtime behavior consistent > > and avoids whole-cache invalidation. > > > > Signed-off-by: Abdellatif El Khlifi <[email protected]> > > Signed-off-by: Harsimran Singh Tungal <[email protected]> > > > > lib/efi_loader/efi_variable_tee.c | 33 ++++++++++++++++++++++++++------- > > 1 file changed, 26 insertions(+), 7 deletions(-) > > > diff --git a/lib/efi_loader/efi_variable_tee.c > > b/lib/efi_loader/efi_variable_tee.c > > @@ -389,19 +389,38 @@ static efi_status_t ffa_mm_communicate(void > > *comm_buf, ulong comm_buf_size) > > memcpy(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. > > + * 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. > > */ > > This 20-line comment is now duplicated verbatim with the one in > ffa_mm_communicate_runtime() introduced earlier in the series. Please > factor the clean-before / invalidate-after sequence into a small > helper (e.g. ffa_mm_buf_pre_call() / ffa_mm_buf_post_call()) so the > commentary lives in one place and the two paths cannot drift. The > runtime helper would add the extra 'no whole-cache invalidation after > ExitBootServices()' note at the call site. > > > diff --git a/lib/efi_loader/efi_variable_tee.c > > b/lib/efi_loader/efi_variable_tee.c > > @@ -389,19 +389,38 @@ static efi_status_t ffa_mm_communicate(void > > *comm_buf, ulong comm_buf_size) > > -#ifdef CONFIG_ARM64 > > - invalidate_dcache_all(); > > -#endif > > + if (IS_ENABLED(CONFIG_ARM64)) > > + flush_dcache_range((unsigned long)virt_shared_buf, > > + (unsigned long)virt_shared_buf + > > + CONFIG_FFA_SHARED_MM_BUF_SIZE); > > Just to check - flush_dcache_range() and invalidate_dcache_range() on > arm64 require start and end to be cache-line aligned, otherwise the > arch code warns and falls back to clean+invalidate of the partial > lines. CONFIG_FFA_SHARED_MM_BUF_ADDR and _SIZE need to be at least > CONFIG_SYS_CACHELINE_SIZE aligned - is that documented or enforced > anywhere? You could use BUILD_BUG_ON() near the call, perhaps?
Let's make sure the CONFIG_FFA_SHARED_MM_BUF_ADDR and _SIZE are CONFIG_SYS_CACHELINE_SIZE aligned and document that in the Kconfig file. It is also a good idea to add the BUILD_BUG_ON() check. Regards, Abdellatif
