| Message ID | 1337248374-23252-1-git-send-email-r.sricharan@ti.com |
|---|---|
| State | Accepted |
| Commit | d702b0811df53a1fc2d8049e35431e4591d093c6 |
| Delegated to: | Albert ARIBAUD |
| Headers | show |
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
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
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
Hi Marek Vasut, >> Ping.. > > +1 Thanks.. Thanks, Sricharan
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,
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
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(-)