Patchwork [U-Boot,1/5] ARM: cache: Move the cp15 CR register read before flushing the cache.

login
register
mail settings
Submitter SRICHARAN R
Date May 17, 2012, 9:52 a.m.
Message ID <1337248374-23252-1-git-send-email-r.sricharan@ti.com>
Download mbox | patch
Permalink /patch/159877/
State Accepted
Commit d702b0811df53a1fc2d8049e35431e4591d093c6
Delegated to: Albert ARIBAUD
Headers show

Comments

SRICHARAN R - May 17, 2012, 9:52 a.m.
The following is the cleanup sequence in arch/arm/cpu/armv7/cpu.c

int cleanup_before_linux(void)
{
 ...
 ...
 dcache_disable();
 v7_outer_cache_disable();
 invalidate_dcache_all();
}

 1) invalidate_dcache_all call expects that all the caches has been
 flushed, invalidated and there are no dirty entries prior to its
 execution.  In the above sequence dcache_disable() flushes, invalidates
 the caches and turns off the  mmu. But after it cleanups the cache
 and before the mmu is disabled  there is a cp_delay() function which
 has STR instruction. On certain cores like the cortex-a15, cache hit
 and a write can happen to a cache line even when the dcache is
 disabled. So the above mentioned STR instruction creates a dirty entry
 after cleaning. The mmu gets disabled after this.

 2) invalidate_dcache_all invalidates the cache lines. Again on
 cores like cortex-a15, invalidate instruction flushes the dirty
 line as well. So some times the dirty line from sequence 1
 can corrupt the memory resulting in a crash.

 Fixing this by moving the get_cr() and cp_delay() calls before
 cleaning up the cache, thus avoiding the dirty entry.

Signed-off-by: R Sricharan <r.sricharan@ti.com>
---
 arch/arm/lib/cache-cp15.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
SRICHARAN R - May 29, 2012, 2:54 p.m.
Hi Albert,
  Are you planning to take up the below patch ?

Thanks,
 Sricharan

On Thu, May 17, 2012 at 3:22 PM, R Sricharan <r.sricharan@ti.com> wrote:
> The following is the cleanup sequence in arch/arm/cpu/armv7/cpu.c
>
> int cleanup_before_linux(void)
> {
>  ...
>  ...
>  dcache_disable();
>  v7_outer_cache_disable();
>  invalidate_dcache_all();
> }
>
>  1) invalidate_dcache_all call expects that all the caches has been
>  flushed, invalidated and there are no dirty entries prior to its
>  execution.  In the above sequence dcache_disable() flushes, invalidates
>  the caches and turns off the  mmu. But after it cleanups the cache
>  and before the mmu is disabled  there is a cp_delay() function which
>  has STR instruction. On certain cores like the cortex-a15, cache hit
>  and a write can happen to a cache line even when the dcache is
>  disabled. So the above mentioned STR instruction creates a dirty entry
>  after cleaning. The mmu gets disabled after this.
>
>  2) invalidate_dcache_all invalidates the cache lines. Again on
>  cores like cortex-a15, invalidate instruction flushes the dirty
>  line as well. So some times the dirty line from sequence 1
>  can corrupt the memory resulting in a crash.
>
>  Fixing this by moving the get_cr() and cp_delay() calls before
>  cleaning up the cache, thus avoiding the dirty entry.
>
> Signed-off-by: R Sricharan <r.sricharan@ti.com>
> ---
>  arch/arm/lib/cache-cp15.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
> index e6c3eae..939de10 100644
> --- a/arch/arm/lib/cache-cp15.c
> +++ b/arch/arm/lib/cache-cp15.c
> @@ -115,17 +115,17 @@ static void cache_disable(uint32_t cache_bit)
>  {
>        uint32_t reg;
>
> +       reg = get_cr();
> +       cp_delay();
> +
>        if (cache_bit == CR_C) {
>                /* if cache isn;t enabled no need to disable */
> -               reg = get_cr();
>                if ((reg & CR_C) != CR_C)
>                        return;
>                /* if disabling data cache, disable mmu too */
>                cache_bit |= CR_M;
>                flush_dcache_all();
>        }
> -       reg = get_cr();
> -       cp_delay();
>        set_cr(reg & ~cache_bit);
>  }
>  #endif
> --
> 1.7.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
SRICHARAN R - June 5, 2012, 8:58 a.m.
Ping..

Thanks,
 Sricharan


> -----Original Message-----
> From: R, Sricharan [mailto:r.sricharan@ti.com]
> Sent: Tuesday, May 29, 2012 8:24 PM
> To: U-Boot@lists.denx.de; albert.u.boot@aribaud.net
> Cc: trini@ti.com
> Subject: Re: [U-Boot] [PATCH 1/5] ARM: cache: Move the cp15 CR register
> read before flushing the cache.
>
> Hi Albert,
>   Are you planning to take up the below patch ?
>
> Thanks,
>  Sricharan
>
> On Thu, May 17, 2012 at 3:22 PM, R Sricharan <r.sricharan@ti.com>
> wrote:
> > The following is the cleanup sequence in arch/arm/cpu/armv7/cpu.c
> >
> > int cleanup_before_linux(void)
> > {
> >  ...
> >  ...
> >  dcache_disable();
> >  v7_outer_cache_disable();
> >  invalidate_dcache_all();
> > }
> >
> >  1) invalidate_dcache_all call expects that all the caches has been
> >  flushed, invalidated and there are no dirty entries prior to its
> >  execution.  In the above sequence dcache_disable() flushes,
> invalidates
> >  the caches and turns off the  mmu. But after it cleanups the cache
> >  and before the mmu is disabled  there is a cp_delay() function which
> >  has STR instruction. On certain cores like the cortex-a15, cache hit
> >  and a write can happen to a cache line even when the dcache is
> >  disabled. So the above mentioned STR instruction creates a dirty
> entry
> >  after cleaning. The mmu gets disabled after this.
> >
> >  2) invalidate_dcache_all invalidates the cache lines. Again on
> >  cores like cortex-a15, invalidate instruction flushes the dirty
> >  line as well. So some times the dirty line from sequence 1
> >  can corrupt the memory resulting in a crash.
> >
> >  Fixing this by moving the get_cr() and cp_delay() calls before
> >  cleaning up the cache, thus avoiding the dirty entry.
> >
> > Signed-off-by: R Sricharan <r.sricharan@ti.com>
> > ---
> >  arch/arm/lib/cache-cp15.c |    6 +++---
> >  1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
> > index e6c3eae..939de10 100644
> > --- a/arch/arm/lib/cache-cp15.c
> > +++ b/arch/arm/lib/cache-cp15.c
> > @@ -115,17 +115,17 @@ static void cache_disable(uint32_t cache_bit)
> >  {
> >        uint32_t reg;
> >
> > +       reg = get_cr();
> > +       cp_delay();
> > +
> >        if (cache_bit == CR_C) {
> >                /* if cache isn;t enabled no need to disable */
> > -               reg = get_cr();
> >                if ((reg & CR_C) != CR_C)
> >                        return;
> >                /* if disabling data cache, disable mmu too */
> >                cache_bit |= CR_M;
> >                flush_dcache_all();
> >        }
> > -       reg = get_cr();
> > -       cp_delay();
> >        set_cr(reg & ~cache_bit);
> >  }
> >  #endif
> > --
> > 1.7.1
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
Marek Vasut - June 5, 2012, 8:25 p.m.
Dear Sricharan R,

> Ping..

+1

> Thanks,
>  Sricharan
> 
> > -----Original Message-----
> > From: R, Sricharan [mailto:r.sricharan@ti.com]
> > Sent: Tuesday, May 29, 2012 8:24 PM
> > To: U-Boot@lists.denx.de; albert.u.boot@aribaud.net
> > Cc: trini@ti.com
> > Subject: Re: [U-Boot] [PATCH 1/5] ARM: cache: Move the cp15 CR register
> > read before flushing the cache.
> > 
> > Hi Albert,
> > 
> >   Are you planning to take up the below patch ?
> > 
> > Thanks,
> > 
> >  Sricharan
> > 
> > On Thu, May 17, 2012 at 3:22 PM, R Sricharan <r.sricharan@ti.com>
> > 
> > wrote:
> > > The following is the cleanup sequence in arch/arm/cpu/armv7/cpu.c
> > > 
> > > int cleanup_before_linux(void)
> > > {
> > >  ...
> > >  ...
> > >  dcache_disable();
> > >  v7_outer_cache_disable();
> > >  invalidate_dcache_all();
> > > }
> > > 
> > >  1) invalidate_dcache_all call expects that all the caches has been
> > >  flushed, invalidated and there are no dirty entries prior to its
> > >  execution.  In the above sequence dcache_disable() flushes,
> > 
> > invalidates
> > 
> > >  the caches and turns off the  mmu. But after it cleanups the cache
> > >  and before the mmu is disabled  there is a cp_delay() function which
> > >  has STR instruction. On certain cores like the cortex-a15, cache hit
> > >  and a write can happen to a cache line even when the dcache is
> > >  disabled. So the above mentioned STR instruction creates a dirty
> > 
> > entry
> > 
> > >  after cleaning. The mmu gets disabled after this.
> > > 
> > >  2) invalidate_dcache_all invalidates the cache lines. Again on
> > >  cores like cortex-a15, invalidate instruction flushes the dirty
> > >  line as well. So some times the dirty line from sequence 1
> > >  can corrupt the memory resulting in a crash.
> > > 
> > >  Fixing this by moving the get_cr() and cp_delay() calls before
> > >  cleaning up the cache, thus avoiding the dirty entry.
> > > 
> > > Signed-off-by: R Sricharan <r.sricharan@ti.com>
> > > ---
> > >  arch/arm/lib/cache-cp15.c |    6 +++---
> > >  1 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
> > > index e6c3eae..939de10 100644
> > > --- a/arch/arm/lib/cache-cp15.c
> > > +++ b/arch/arm/lib/cache-cp15.c
> > > @@ -115,17 +115,17 @@ static void cache_disable(uint32_t cache_bit)
> > >  {
> > >        uint32_t reg;
> > > 
> > > +       reg = get_cr();
> > > +       cp_delay();
> > > +
> > >        if (cache_bit == CR_C) {
> > >                /* if cache isn;t enabled no need to disable */
> > > -               reg = get_cr();
> > >                if ((reg & CR_C) != CR_C)
> > >                        return;
> > >                /* if disabling data cache, disable mmu too */
> > >                cache_bit |= CR_M;
> > >                flush_dcache_all();
> > >        }
> > > -       reg = get_cr();
> > > -       cp_delay();
> > >        set_cr(reg & ~cache_bit);
> > >  }
> > >  #endif
> > > --
> > > 1.7.1
> > > 
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot@lists.denx.de
> > > http://lists.denx.de/mailman/listinfo/u-boot
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Best regards,
Marek Vasut
SRICHARAN R - June 7, 2012, 5:05 a.m.
Hi Marek Vasut,

>> Ping..
>
> +1

Thanks..

Thanks,
 Sricharan
Albert ARIBAUD - July 7, 2012, noon
Hi R Sricharan,

On Thu, 17 May 2012 15:22:54 +0530, R Sricharan <r.sricharan@ti.com>
wrote:

> The following is the cleanup sequence in arch/arm/cpu/armv7/cpu.c
> 
> int cleanup_before_linux(void)
> {
>  ...
>  ...
>  dcache_disable();
>  v7_outer_cache_disable();
>  invalidate_dcache_all();
> }
> 
>  1) invalidate_dcache_all call expects that all the caches has been
>  flushed, invalidated and there are no dirty entries prior to its
>  execution.  In the above sequence dcache_disable() flushes,
> invalidates the caches and turns off the  mmu. But after it cleanups
> the cache and before the mmu is disabled  there is a cp_delay()
> function which has STR instruction. On certain cores like the
> cortex-a15, cache hit and a write can happen to a cache line even
> when the dcache is disabled. So the above mentioned STR instruction
> creates a dirty entry after cleaning. The mmu gets disabled after
> this.
> 
>  2) invalidate_dcache_all invalidates the cache lines. Again on
>  cores like cortex-a15, invalidate instruction flushes the dirty
>  line as well. So some times the dirty line from sequence 1
>  can corrupt the memory resulting in a crash.
> 
>  Fixing this by moving the get_cr() and cp_delay() calls before
>  cleaning up the cache, thus avoiding the dirty entry.
> 
> Signed-off-by: R Sricharan <r.sricharan@ti.com>
> ---

Applied to u-boot-arm/master, thanks.

Amicalement,

Patch

diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index e6c3eae..939de10 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -115,17 +115,17 @@  static void cache_disable(uint32_t cache_bit)
 {
 	uint32_t reg;
 
+	reg = get_cr();
+	cp_delay();
+
 	if (cache_bit == CR_C) {
 		/* if cache isn;t enabled no need to disable */
-		reg = get_cr();
 		if ((reg & CR_C) != CR_C)
 			return;
 		/* if disabling data cache, disable mmu too */
 		cache_bit |= CR_M;
 		flush_dcache_all();
 	}
-	reg = get_cr();
-	cp_delay();
 	set_cr(reg & ~cache_bit);
 }
 #endif