[U-Boot] [PATCH 1/2] armv8: caches: Disable dcache after flush

[Siva Durga Prasad Paladugu]

Hi Mark.

> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland at arm.com]
> Sent: Wednesday, April 15, 2015 6:41 PM
> To: Michal Simek
> Cc: u-boot at lists.denx.de; Tom Rini; Siva Durga Prasad Paladugu; Varun Sethi;
> Arnab Basu; York Sun
> Subject: Re: [U-Boot] [PATCH 1/2] armv8: caches: Disable dcache after flush
> 
> On Wed, Apr 15, 2015 at 12:33:00PM +0100, Michal Simek wrote:
> > From: Siva Durga Prasad Paladugu <siva.durga.paladugu at xilinx.com>
> >
> > Always disable dcache after the flush operation The following sequence
> > is advisable while disabling d-cache:
> > 1. disable_dcache() - flushes and disables d-cache 2.
> > invalidate_dcache_all() - invalid any entry that came to the cache
> >    in the short period after the cache was flushed but before the
> >    cache got disabled
> 
> For reasons I have described previously (see [1,2,3]), this is unsafe.
> The first cache flush may achieve nothing.
> 
> If you need data out at the PoC before disabling the cache, then you should
> first use maintenance by VA to push that data out.
> 
> Thanks,
> Mark.
> 
> [1] http://lists.denx.de/pipermail/u-boot/2015-February/204403.html
> [2] http://lists.denx.de/pipermail/u-boot/2015-February/204407.html
> [3] http://lists.denx.de/pipermail/u-boot/2015-February/204702.html
> 
> >
> > Signed-off-by: Siva Durga Prasad Paladugu <sivadur at xilinx.com>
> > Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> > ---
> >
> >  arch/arm/cpu/armv8/cache_v8.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/cpu/armv8/cache_v8.c
> > b/arch/arm/cpu/armv8/cache_v8.c index c5ec5297cd39..2a0492fbef52
> > 100644
> > --- a/arch/arm/cpu/armv8/cache_v8.c
> > +++ b/arch/arm/cpu/armv8/cache_v8.c
> > @@ -128,10 +128,10 @@ void dcache_disable(void)
> >  	if (!(sctlr & CR_C))
> >  		return;
> >
> > -	set_sctlr(sctlr & ~(CR_C|CR_M));
> > -
> >  	flush_dcache_all();
> >  	__asm_invalidate_tlb_all();
> > +
> > +	set_sctlr(sctlr & ~(CR_C|CR_M));

I got your point. But here in this scenario also there is an issue with disable first and then flush_dcache_all().
This is because when we disable the cache and invoke the c routine flush_dcache_all() then the return address of this is stored in a stack(in memory as dcache is disabled).
Now in the flush_dcache_all we are invoking the actual asm call to flush dcache which may wipeout the stored return value in stack with cahe contents(main memory). Hence the return from the flush_dcahe_all will fail.

To confirm this I modified the dcache_disable in the below way and it worked fine.
1. Disable the dcache.
2. Now I called the __asm_flush_dcache_all(); and then flush_l3_cache();  instead of calling the flush_dcache_all().

Regards,
Siva

> >  }
> >
> >  int dcache_status(void)
> > --
> > 2.3.5
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> >