ARM: cache: Move the cp15 CR register read before flushing the cache.

PATCH New R Sricharan 4 participants 6 messages May 17, 2012 – Jul 7, 2012 Arm (32-bit)

lore

Share & tools

lore

R Sricharan

author

2012-05-17 09:52 UTC #0 lore raw

PATCH 1/5 ARM: cache: Move the cp15 CR register read before flushing the cache.

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 <[email protected]>

---
 arch/arm/lib/cache-cp15.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

arch/arm/lib/cache-cp15.c +3 -3

index e6c3eae..939de10 100644
@@ -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

Signature · 1 line

1.7.1

R

2012-05-29 14:54 UTC #1 lore raw

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

Quoted 66 lines · R Sricharan

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 <[email protected]>
---
?arch/arm/lib/cache-cp15.c | ? ?6 +++---
?1 files changed, 3 insertions(+), 3 deletions(-)
arch/arm/lib/cache-cp15.c +3 -0
index e6c3eae..939de10 100644
@@ -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 at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Sricharan R

author

2012-06-05 08:58 UTC #2 lore raw

PATCH 1/5 ARM: cache: Move the cp15 CR register read before flushing the cache.

Ping..

Thanks,
Sricharan

Quoted 84 lines

-----Original Message-----
From: R, Sricharan [mailto:r.sricharan at ti.com]
Sent: Tuesday, May 29, 2012 8:24 PM
To: U-Boot at lists.denx.de; albert.u.boot at aribaud.net
Cc: trini at 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
Quoted 11 lines · R Sricharan
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();
}
Quoted 3 lines
?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
Quoted 5 lines
?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
Quoted 47 lines
?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 <[email protected]>
---
?arch/arm/lib/cache-cp15.c | ? ?6 +++---
?1 files changed, 3 insertions(+), 3 deletions(-)
arch/arm/lib/cache-cp15.c +3 -0
index e6c3eae..939de10 100644
@@ -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 at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Marek Vasut

2012-06-05 20:25 UTC #3 lore raw

PATCH 1/5 ARM: cache: Move the cp15 CR register read before flushing the cache.

Dear Sricharan R,

Quoted 1 line

Ping..

Quoted 90 lines

Thanks,
Sricharan
Quoted 87 lines
-----Original Message-----
From: R, Sricharan [mailto:r.sricharan at ti.com]
Sent: Tuesday, May 29, 2012 8:24 PM
To: U-Boot at lists.denx.de; albert.u.boot at aribaud.net
Cc: trini at 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 <[email protected]>
Quoted 11 lines · 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();
}
Quoted 3 lines
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
Quoted 5 lines
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
Quoted 43 lines
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 <[email protected]>
---
 arch/arm/lib/cache-cp15.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
arch/arm/lib/cache-cp15.c +3 -4
index e6c3eae..939de10 100644
@@ -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

Quoted 9 lines

Quoted 4 lines
Quoted 4 lines
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Best regards,
Marek Vasut

R

2012-06-07 05:05 UTC #4 lore raw

PATCH 1/5 ARM: cache: Move the cp15 CR register read before flushing the cache.

Hi Marek Vasut,

Quoted 3 lines

Quoted 1 line
Ping..
+1

Thanks..

Thanks,
Sricharan

Albert ARIBAUD

2012-07-07 12:00 UTC #5 lore raw

PATCH 1/5 ARM: cache: Move the cp15 CR register read before flushing the cache.

In reply to R Sricharan

Hi R Sricharan,

Quoted 27 lines · R Sricharan

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.

Quoted 5 lines

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 <[email protected]>
---

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

Amicalement,

Signature · 1 line

Albert.