On 2026-04-28 12:14 -0600, Simon Glass wrote: > 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? > > > 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) > > The commit message body opens with 'Match boot-time FF-A cache > handling to runtime behavior', which restates the subject. Please drop > that line and start directly with the problem paragraph per the U-Boot > convention of leading with motivation. > > Regards, > Simon >
In v2, ffa_mm_communicate() itself handles both boot time and runtime capabilities, so this patch is no longer required. This patch has been removed in v2 but the review comments have been addressed. Link to v2: https://lore.kernel.org/u-boot/[email protected]/ Regards Harsimran Singh Tungal
