cache flushing for armv3 in zImage

cache flushing for armv3 in zImage

[Uwe Kleine-König]

Hello,

I currently look into arch/arm/boot/compressed/head.S and wonder about
some cache functions:

 - __armv3_mpu_cache_flush does:
	mov     r1, #0
	mcr     p15, 0, r0, c7, c0, 0   @ invalidate whole cache v3
	mov     pc, lr

   why is r1 set to zero?  I assume the register used in the mcr
   instruction should be zero, but this uses r0, not r1.

 - ARMv5TEJ uses __armv4_mmu_cache_flush not __armv5tej_mmu_cache_flush?

 - __armv5tej_mmu_cache_flush calls
	mcr     p15, 0, r0, c7, c5, 0   @ flush I cache
	mcr     p15, 0, r0, c7, c10, 4  @ drain WB
   without asserting that r0 is zero.  ARM DDI 0198E (i.e. ARM926EJ-S
   TRM) specifies the register should be zero.  (I'm not sure this is
   the right document.)
   Ditto for __armv4_mmu_cache_flush.  I assume both should first zero
   r1 and then use r1 in the mcr instructions?
   Ditto __armv3_mpu_cache_on:
	mcr     p15, 0, r0, c7, c0, 0
   while r0 holds a value read by
	mrc     p15, 0, r0, c1, c0, 0 

 - __armv7_mmu_cache_flush corrupts r10, but this is not listed as
   corrupted register in the comment describing cache_clean_flush.
   As __armv7_mmu_cache_off calls __armv7_mmu_cache_flush there r10 is
   corrupted, too which isn't explicitly allowed according to the
   comment of cache_off either.

 - __armv4_mpu_cache_on corrupts r0
   __armv3_mpu_cache_on ditto

 - __armv3_mpu_cache_on has:

	mrc     p15, 0, r0, c1, c0, 0   @ read control reg
	                                @ .... .... .... WC.M
	orr     r0, r0, #0x000d         @ .... .... .... 11.1
	mov     r0, #0
	mcr     p15, 0, r0, c1, c0, 0   @ write control reg

   assuming that reading control reg and writing a recently read value
   has no side effects all four instructions can go away.

 - __armv3_mpu_cache_on does "invalidate whole cache v3" twice?

 - the comment that r9 holds the run-time address of "start" on entry of
   cache_on is wrong I think.  If the zImage isn't relocated r9 isn't
   touched until cacheon is called.
   (r9 is used in __setup_mmu and then holds something that should be
   the start of RAM.  (The actual value is 

	(zreladdr - 0x4000) & ~0x3ffff

   .))

Any thoughts on how to address these points?

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-K?nig            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

cache flushing for armv3 in zImage

[Russell King - ARM Linux]

On Tue, Jan 26, 2010 at 08:20:46PM +0100, Uwe Kleine-K?nig wrote:
> Hello,
> 
> I currently look into arch/arm/boot/compressed/head.S and wonder about
> some cache functions:
> 
>  - __armv3_mpu_cache_flush does:
> 	mov     r1, #0
> 	mcr     p15, 0, r0, c7, c0, 0   @ invalidate whole cache v3
> 	mov     pc, lr
> 
>    why is r1 set to zero?  I assume the register used in the mcr
>    instruction should be zero, but this uses r0, not r1.

The mcr should be using r1 - r0 must be preserved.

>  - ARMv5TEJ uses __armv4_mmu_cache_flush not __armv5tej_mmu_cache_flush?

I don't believe every ARMv5TEJ CPU supports the "test,clean,invalidate
D cache" instruction; you may notice only a very small subset of ARMv5
implementations use __armv5tej_mmu_cache_flush, and I suspect that it
was originally mis-named.

>  - __armv5tej_mmu_cache_flush calls
> 	mcr     p15, 0, r0, c7, c5, 0   @ flush I cache
> 	mcr     p15, 0, r0, c7, c10, 4  @ drain WB
>    without asserting that r0 is zero.  ARM DDI 0198E (i.e. ARM926EJ-S
>    TRM) specifies the register should be zero.  (I'm not sure this is
>    the right document.)

Over the years, the requirement for the register passed to 'mcr' has
varied between "should be zero" to "doesn't care" depending on the
document you read.  As a result, I no longer attach much belief to
statements requiring the register passed to cache flushing functions
to be zero.

Indeed, in real tests, it doesn't seem to make any difference on any
CPU I've come across.

>  - __armv7_mmu_cache_flush corrupts r10, but this is not listed as
>    corrupted register in the comment describing cache_clean_flush.

r10 can be listed as being corrupted.

>    As __armv7_mmu_cache_off calls __armv7_mmu_cache_flush there r10 is
>    corrupted, too which isn't explicitly allowed according to the
>    comment of cache_off either.

Ditto.  As the only place where cache_off is called is:

call_kernel:    bl      cache_clean_flush
                bl      cache_off

any registers which cache_clean_flush can corrupt can also be corrupted
by cache_off, so lets make the two lists of corrupted registers identical.

>  - __armv4_mpu_cache_on corrupts r0
>    __armv3_mpu_cache_on ditto

Allowed.

>  - __armv3_mpu_cache_on has:
> 
> 	mrc     p15, 0, r0, c1, c0, 0   @ read control reg
> 	                                @ .... .... .... WC.M
> 	orr     r0, r0, #0x000d         @ .... .... .... 11.1
> 	mov     r0, #0
> 	mcr     p15, 0, r0, c1, c0, 0   @ write control reg
> 
>    assuming that reading control reg and writing a recently read value
>    has no side effects all four instructions can go away.

Nice catch - no idea what the right thing is here, the ARM uclinux people
seem to have vanished from the planet.

ARMv3 MMU does not allow reading the control register, and I'd be surprised
if ARMv3 MPU allows it either.  I'd suggest leaving this as is until
someone who knows this hardware decides to fix.

>  - __armv3_mpu_cache_on does "invalidate whole cache v3" twice?

No idea - uclinux magic I assume.

>  - the comment that r9 holds the run-time address of "start" on entry of
>    cache_on is wrong I think.  If the zImage isn't relocated r9 isn't
>    touched until cacheon is called.

Comment should be removed.

cache flushing for armv3 in zImage

[Uwe Kleine-König]

Hi Russell,

thanks for your comments.  I will prepare a patch based on these.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-K?nig            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

[PATCH 1/4] zImage: fix comments for cache_on, cache_off and cache_clean_flush

[Uwe Kleine-König]

This adds missing registers to the list of corrupted registers and
removes a wrong comment about r9 on entry

While at it the formatting of the comment to cache_off is changed to
resemble the other two.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 arch/arm/boot/compressed/head.S |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 4fddc50..7cbab3c 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -339,9 +339,8 @@ params:		ldr	r0, =params_phys
  *  r6 = processor ID
  *  r7 = architecture number
  *  r8 = atags pointer
- *  r9 = run-time address of "start"  (???)
  * On exit,
- *  r1, r2, r3, r9, r10, r12 corrupted
+ *  r0, r1, r2, r3, r9, r10, r12 corrupted
  * This routine must preserve:
  *  r4, r5, r6, r7, r8
  */
@@ -778,9 +777,12 @@ proc_types:
  * Turn off the Cache and MMU.  ARMv3 does not support
  * reading the control register, but ARMv4 does.
  *
- * On entry,  r6 = processor ID
- * On exit,   r0, r1, r2, r3, r12 corrupted
- * This routine must preserve: r4, r6, r7
+ * On entry,
+ *  r6 = processor ID
+ * On exit,
+ *  r0, r1, r2, r3, r10, r12 corrupted
+ * This routine must preserve:
+ *  r4, r6, r7
  */
 		.align	5
 cache_off:	mov	r3, #12			@ cache_off function
@@ -855,7 +857,7 @@ __armv3_mmu_cache_off:
  * On entry,
  *  r6 = processor ID
  * On exit,
- *  r1, r2, r3, r11, r12 corrupted
+ *  r1, r2, r3, r10, r11, r12 corrupted
  * This routine must preserve:
  *  r0, r4, r5, r6, r7
  */
-- 
1.6.6

[PATCH 2/4] zImage: some comments for __armv3_mpu_cache_on

[Uwe Kleine-König]

__armv3_mpu_cache_on seems broken.  As there is noone around who knows
about these machines just keep the code as is but point out the strange
things.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 arch/arm/boot/compressed/head.S |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 7cbab3c..f6d665f 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -395,12 +395,15 @@ __armv3_mpu_cache_on:
 
 		mov	r0, #0
 		mcr	p15, 0, r0, c7, c0, 0	@ invalidate whole cache v3
+		/* ?? ARMv3 MMU doesn not allow reading the control register, does this really work on ARMv3 MPU? */
 		mrc	p15, 0, r0, c1, c0, 0	@ read control reg
 						@ .... .... .... WC.M
 		orr	r0, r0, #0x000d		@ .... .... .... 11.1
+		/* ?? this overwrites the value constructed above? */
 		mov	r0, #0
 		mcr	p15, 0, r0, c1, c0, 0	@ write control reg
 
+		/* ?? invalidate for the second time? */
 		mcr	p15, 0, r0, c7, c0, 0	@ invalidate whole cache v3
 		mov	pc, lr
 
-- 
1.6.6

[PATCH 3/4] zImage: __armv3_mpu_cache_flush: respect should-be-zero specification

[Uwe Kleine-König]

Probably the register content for cache operations is "don't care" in
practice, but as r1 is explicitly zeroed, use that one.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 arch/arm/boot/compressed/head.S |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index f6d665f..15534ea 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -1003,7 +1003,7 @@ no_cache_id:
 __armv3_mmu_cache_flush:
 __armv3_mpu_cache_flush:
 		mov	r1, #0
-		mcr	p15, 0, r0, c7, c0, 0	@ invalidate whole cache v3
+		mcr	p15, 0, r1, c7, c0, 0	@ invalidate whole cache v3
 		mov	pc, lr
 
 /*
-- 
1.6.6

[PATCH 4/4] zImage: annotate debug functions about corrupted registers

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 arch/arm/boot/compressed/head.S |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 15534ea..04bb6b4 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -1016,6 +1016,7 @@ __armv3_mpu_cache_flush:
 phexbuf:	.space	12
 		.size	phexbuf, . - phexbuf
 
+@ phex corrupts {r0, r1, r2, r3}
 phex:		adr	r3, phexbuf
 		mov	r2, #0
 		strb	r2, [r3, r1]
@@ -1030,6 +1031,7 @@ phex:		adr	r3, phexbuf
 		strb	r2, [r3, r1]
 		b	1b
 
+@ puts corrupts {r0, r1, r2, r3}
 puts:		loadsp	r3
 1:		ldrb	r2, [r0], #1
 		teq	r2, #0
@@ -1044,12 +1046,14 @@ puts:		loadsp	r3
 		teq	r0, #0
 		bne	1b
 		mov	pc, lr
+@ putc corrupts {r0, r1, r2, r3}
 putc:
 		mov	r2, r0
 		mov	r0, #0
 		loadsp	r3
 		b	2b
 
+@ memdump corrupts {r0, r1, r2, r3, r10, r11, r12, lr}
 memdump:	mov	r12, r0
 		mov	r10, lr
 		mov	r11, #0
-- 
1.6.6