Re: [PATCH 12/12] efi_loader: align FF-A cache maintenance with runtime path

[Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>]

Hi Harsimran,

> On 2026-04-24T17:31:50, Harsimran Singh Tungal
> <harsimransingh.tungal@arm.com> 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 <abdellatif.elkhlifi@arm.com>
> > Signed-off-by: Harsimran Singh Tungal <harsimransingh.tungal@arm.com>
> >
> > 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