armv7: Fix to mistake clean the memory space

PATCH New Zhong Hongbo 3 participants 7 messages Jul 5, 2012 – Jul 6, 2012 Arm (32-bit)

lore

Share & tools

lore

Zhong Hongbo

author

2012-07-05 14:35 UTC #0 lore raw

PATCH 1/1 armv7: Fix to mistake clean the memory space

From: Zhong Hongbo <[email protected]>

In currently, when __bss_start is equal to __bss_end__, The bss loop will clear all the things in memory space.

But just only when __bss_end__ greater than __bss_start__, we do the clear bss section operation.

Signed-off-by Hongbo Zhong <[email protected]>

---
 arch/arm/cpu/armv7/start.S |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

arch/arm/cpu/armv7/start.S +6 -3

index 22a3ced..2e583ee 100644
@@ -259,10 +259,13 @@ clear_bss:
 #endif
 	mov	r2, #0x00000000		/* clear			    */
 
-clbss_l:str	r2, [r0]		/* clear loop...		    */
+clbss_l:
+	cmp	r1, r0
+	bls	clbss_end
+	str	r2, [r0]		/* clear loop...		    */
 	add	r0, r0, #4
-	cmp	r0, r1
-	bne	clbss_l
+	b	clbss_l
+clbss_end:
 
 /*
  * We are done. Do not return, instead branch to second part of board

Signature · 1 line

1.7.5.4

Andreas Bießmann

2012-07-05 15:38 UTC #1 lore raw

PATCH 1/1 armv7: Fix to mistake clean the memory space

Dear Zhong Hongbo,

this is arm related and should therefore apply to all the different arch/arm/cpu/*/start.S (as long as they have SPL support).

Quoted 4 lines · Zhong Hongbo

From: Zhong Hongbo <[email protected]>
In currently, when __bss_start is equal to __bss_end__, The bss loop will clear all the things in memory space.

I ask myself why there is no bss at all. Have you just initialized data in your spl code?

Quoted 17 lines

But just only when __bss_end__ greater than __bss_start__, we do the clear bss section operation.
Signed-off-by Hongbo Zhong <[email protected]>
---
 arch/arm/cpu/armv7/start.S |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)
arch/arm/cpu/armv7/start.S +0 -0
index 22a3ced..2e583ee 100644
@@ -259,10 +259,13 @@ clear_bss:
 #endif
 	mov	r2, #0x00000000		/* clear			    */
 

arch/arm/cpu/armv7/start.S clear_bss: · line 268

Quoted 10 lines

arch/arm/cpu/armv7/start.S +6 -3

-clbss_l:str	r2, [r0]		/* clear loop...		    */
+clbss_l:
+	cmp	r1, r0
+	bls	clbss_end
+	str	r2, [r0]		/* clear loop...		    */
 	add	r0, r0, #4
-	cmp	r0, r1
-	bne	clbss_l
+	b	clbss_l
+clbss_end:

Looks sane, I maybe had checked before clbss_l, jumped over clear loop if r0 and r1 equal (to emphasize the fact that this is a check for entry correctness) and wouldn't change the loop at all. To be more precise we could also check if end is after start.

arch/arm/cpu/armv7/start.S clear_bss:

Quoted 4 lines

arch/arm/cpu/armv7/start.S +0 -0

 /*
  * We are done. Do not return, instead branch to second part of board

Best regards

Andreas Bie?mann

Andreas Bießmann

2012-07-05 15:52 UTC #2 lore raw

PATCH 1/1 armv7: Fix to mistake clean the memory space

On 05.07.2012 17:38, Andreas Bie?mann wrote:

<snip>

Quoted 7 lines

arch/arm/cpu/armv7/start.S +4 -2

-clbss_l:str	r2, [r0]		/* clear loop...		    */
+clbss_l:
+	cmp	r1, r0
+	bls	clbss_end
+	str	r2, [r0]		/* clear loop...		    */
 	add	r0, r0, #4
-	cmp	r0, r1

Quoted 8 lines

arch/arm/cpu/armv7/start.S clear_bss: · line 268
Quoted 3 lines
arch/arm/cpu/armv7/start.S +2 -1
-	bne	clbss_l
+	b	clbss_l
+clbss_end:
Looks sane, I maybe had checked before clbss_l, jumped over clear loop if r0 and r1 equal (to emphasize the fact that this is a check for entry correctness) and wouldn't change the loop at all. To be more precise we could also check if end is after start.

Well, I just realized you do check for less ;)

Zhong Hongbo

author

2012-07-06 11:31 UTC #3 lore raw

PATCH 1/1 armv7: Fix to mistake clean the memory space

Quoted 20 lines · Andreas Bie?mann

On 05.07.2012 17:38, Andreas Bie?mann wrote:
<snip>
Quoted 7 lines
Quoted 7 lines
arch/arm/cpu/armv7/start.S +4 -2
-clbss_l:str	r2, [r0]		/* clear loop...		    */
+clbss_l:
+	cmp	r1, r0
+	bls	clbss_end
+	str	r2, [r0]		/* clear loop...		    */
 	add	r0, r0, #4
-	cmp	r0, r1
Quoted 8 lines
arch/arm/cpu/armv7/start.S clear_bss: · line 268
Quoted 3 lines
arch/arm/cpu/armv7/start.S +2 -1
-	bne	clbss_l
+	b	clbss_l
+clbss_end:
Looks sane, I maybe had checked before clbss_l, jumped over clear loop if r0 and r1 equal (to emphasize the fact that this is a check for entry correctness) and wouldn't change the loop at all. To be more precise we could also check if end is after start.

Quoted 2 lines

Well, I just realized you do check for less ;)

Yes,

Thanks,
hongbo

Zhong Hongbo

author

2012-07-06 11:30 UTC #4 lore raw

PATCH 1/1 armv7: Fix to mistake clean the memory space

In reply to Andreas Bießmann

Hi Andreas,

Quoted 4 lines · Andreas Bie?mann

Dear Zhong Hongbo,
this is arm related and should therefore apply to all the different arch/arm/cpu/*/start.S (as long as they have SPL support).

Ok, I will do it. Thanks for your advise.

Quoted 9 lines · Zhong Hongbo

Quoted 4 lines
From: Zhong Hongbo <[email protected]>
In currently, when __bss_start is equal to __bss_end__, The bss loop will clear all the things in memory space.
I ask myself why there is no bss at all. Have you just initialized data in your spl code?

Yes

Quoted 18 lines

Quoted 18 lines
But just only when __bss_end__ greater than __bss_start__, we do the clear bss section operation.
Signed-off-by Hongbo Zhong <[email protected]>
---
 arch/arm/cpu/armv7/start.S |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)
arch/arm/cpu/armv7/start.S +0 -0
index 22a3ced..2e583ee 100644
@@ -259,10 +259,13 @@ clear_bss:
 #endif
 	mov	r2, #0x00000000		/* clear			    */
 

Quoted 15 lines

arch/arm/cpu/armv7/start.S clear_bss: · line 268
Quoted 10 lines
arch/arm/cpu/armv7/start.S +6 -3
-clbss_l:str	r2, [r0]		/* clear loop...		    */
+clbss_l:
+	cmp	r1, r0
+	bls	clbss_end
+	str	r2, [r0]		/* clear loop...		    */
 	add	r0, r0, #4
-	cmp	r0, r1
-	bne	clbss_l
+	b	clbss_l
+clbss_end:
Looks sane, I maybe had checked before clbss_l, jumped over clear loop if r0 and r1 equal (to emphasize the fact that this is a check for entry correctness) and wouldn't change the loop at all. To be more precise we could also check if end is after start.

I check the starts of arm92ejs, I found the issue display in other arm platform. As following:

commit 8f1da53508c78789ebeea98a92a3f55c3f84dc5d

Author: Christian Riesch <[email protected]>
Date:   Wed Nov 30 22:27:37 2011 +0000

arm, arm926ejs: Fix clear bss loop for zero length bss

This patch fixes the clear bss loop for bss sections that have zero length, i.e., where __bss_start == __bss_end__.

Signed-off-by Christian Riesch <[email protected]>

Cc Albert Aribaud <[email protected]>

arch/arm/cpu/arm926ejs/start.S +5 -3

index 339c5ed..bb4d00b 100644
@@ -301,10 +301,12 @@ clear_bss:
 #endif
        mov     r2, #0x00000000         /* clear
    */
 
-clbss_l:str    r2, [r0]                /* clear loop...
    */
+clbss_l:cmp    r0, r1                  /* clear loop... */
+       bhs     clbss_e                 /* if reached end of bss, exit */
+       str     r2, [r0]
        add     r0, r0, #4
-       cmp     r0, r1
-       bne     clbss_l
+       b       clbss_l
+clbss_e:

Thanks,
hongbo

Quoted 10 lines

arch/arm/cpu/armv7/start.S clear_bss: · line 271
Quoted 5 lines
arch/arm/cpu/armv7/start.S +0 -0
 /*
  * We are done. Do not return, instead branch to second part of board
 
Best regards
Andreas Bie?mann

Albert ARIBAUD

2012-07-06 05:44 UTC #5 lore raw

PATCH 1/1 armv7: Fix to mistake clean the memory space

In reply to Zhong Hongbo

Hi Zhong,

Quoted 24 lines · Zhong Hongbo

From: Zhong Hongbo <[email protected]>
In currently, when __bss_start is equal to __bss_end__, The bss loop will clear all the things in memory space.
But just only when __bss_end__ greater than __bss_start__, we do the clear bss section operation.
Signed-off-by Hongbo Zhong <[email protected]>
---
 arch/arm/cpu/armv7/start.S |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)
arch/arm/cpu/armv7/start.S +2 -1
index 22a3ced..2e583ee 100644
@@ -259,10 +259,13 @@ clear_bss:
 #endif
 	mov	r2, #0x00000000		/* clear			    */
 
-clbss_l:str	r2, [r0]		/* clear loop...		    */
+clbss_l:
+	cmp	r1, r0

arch/arm/cpu/armv7/start.S clear_bss: · line 271

Quoted 10 lines

arch/arm/cpu/armv7/start.S +4 -2

+	bls	clbss_end
+	str	r2, [r0]		/* clear loop...		    */
 	add	r0, r0, #4
-	cmp	r0, r1
-	bne	clbss_l
+	b	clbss_l
+clbss_end:
 
 /*
  * We are done. Do not return, instead branch to second part of board

Just checking: that's the same as your 'infinite' patch, right?

Amicalement,

Signature · 1 line

Albert.

Zhong Hongbo

author

2012-07-06 11:35 UTC #6 lore raw

PATCH 1/1 armv7: Fix to mistake clean the memory space

Quoted 35 lines · Albert ARIBAUD

Hi Zhong,
Quoted 32 lines · Zhong Hongbo
From: Zhong Hongbo <[email protected]>
In currently, when __bss_start is equal to __bss_end__, The bss loop will clear all the things in memory space.
But just only when __bss_end__ greater than __bss_start__, we do the clear bss section operation.
Signed-off-by Hongbo Zhong <[email protected]>
---
 arch/arm/cpu/armv7/start.S |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)
arch/arm/cpu/armv7/start.S +6 -3
index 22a3ced..2e583ee 100644
@@ -259,10 +259,13 @@ clear_bss:
 #endif
 	mov	r2, #0x00000000		/* clear			    */
 
-clbss_l:str	r2, [r0]		/* clear loop...		    */
+clbss_l:
+	cmp	r1, r0
+	bls	clbss_end
+	str	r2, [r0]		/* clear loop...		    */
 	add	r0, r0, #4
-	cmp	r0, r1
-	bne	clbss_l
+	b	clbss_l
+clbss_end:
 

Quoted 4 lines

arch/arm/cpu/armv7/start.S clear_bss: · line 271
Quoted 2 lines
arch/arm/cpu/armv7/start.S +0 -0
 /*
  * We are done. Do not return, instead branch to second part of board
Just checking: that's the same as your 'infinite' patch, right?

You are right. I think the description(V1 patch) have not very clear, So I modify it.

Thanks,
hongbo

Quoted 3 lines

Amicalement,