[PATCH] fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

[PATCH] fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

[Rabin Vincent]

"ARM: Auto calculate ZRELADDR and provide option for exceptions" broke the
Thumb-2 decompressor because it removed an entry in the LC0 table but didn't
adjust the offset the Thumb-2 code uses to load the SP from that table.

Fix it, and also change the ARM code to use the separate SP-load since
ARM instructions that include the SP in the LDM register list are
deprecated.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 arch/arm/boot/compressed/head.S |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index abf4d65..47924fc 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -170,9 +170,9 @@ not_angel:
 
 		.text
 		adr	r0, LC0
- ARM(		ldmia	r0, {r1, r2, r3, r5, r6, r11, ip, sp})
+ ARM(		ldmia	r0, {r1, r2, r3, r5, r6, r11, ip})
  THUMB(		ldmia	r0, {r1, r2, r3, r5, r6, r11, ip}	)
- THUMB(		ldr	sp, [r0, #32]				)
+		ldr	sp, [r0, #28]
 #ifdef CONFIG_AUTO_ZRELADDR
 		@ determine final kernel image address
 		and	r4, pc, #0xf8000000
-- 
1.7.1

[PATCH] fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

[Uwe Kleine-König]

On Sat, Jul 31, 2010 at 01:05:23AM +0530, Rabin Vincent wrote:
> "ARM: Auto calculate ZRELADDR and provide option for exceptions" broke the
> Thumb-2 decompressor because it removed an entry in the LC0 table but didn't
> adjust the offset the Thumb-2 code uses to load the SP from that table.
> 
> Fix it, and also change the ARM code to use the separate SP-load since
> ARM instructions that include the SP in the LDM register list are
> deprecated.
I only found:

	LDM{<cond>}<addressing_mode> <Rn>{!}, <registers>
	...
	If the base register <Rn> is specified in <registers>, and base
	register write-back is specified,
	the final value of <Rn> is UNPREDICTABLE.

Are you really sure about this?  Where do you know it from?
 
Best regards
Uwe

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

[PATCH] fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

[Rabin Vincent]

2010/7/31 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> On Sat, Jul 31, 2010 at 01:05:23AM +0530, Rabin Vincent wrote:
>> Fix it, and also change the ARM code to use the separate SP-load since
>> ARM instructions that include the SP in the LDM register list are
>> deprecated.
> I only found:
>
> ? ? ? ?LDM{<cond>}<addressing_mode> <Rn>{!}, <registers>
> ? ? ? ?...
> ? ? ? ?If the base register <Rn> is specified in <registers>, and base
> ? ? ? ?register write-back is specified,
> ? ? ? ?the final value of <Rn> is UNPREDICTABLE.
>
> Are you really sure about this? ?Where do you know it from?

This is in section "A8.6.53 LDM / LDMIA / LDMFD" of the
ARM ARM (v7 edition):

  LDM<c><q> <Rn>{!}, <registers>

  <registers> The SP can be in the list in ARM code, but not
  in Thumb code. However, ARM instructions that include the
  SP in the list are deprecated.

[PATCH] fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

[Russell King - ARM Linux]

On Fri, Jul 30, 2010 at 10:57:56PM +0200, Uwe Kleine-K?nig wrote:
> On Sat, Jul 31, 2010 at 01:05:23AM +0530, Rabin Vincent wrote:
> > "ARM: Auto calculate ZRELADDR and provide option for exceptions" broke the
> > Thumb-2 decompressor because it removed an entry in the LC0 table but didn't
> > adjust the offset the Thumb-2 code uses to load the SP from that table.
> > 
> > Fix it, and also change the ARM code to use the separate SP-load since
> > ARM instructions that include the SP in the LDM register list are
> > deprecated.
> I only found:
> 
> 	LDM{<cond>}<addressing_mode> <Rn>{!}, <registers>
> 	...
> 	If the base register <Rn> is specified in <registers>, and base
> 	register write-back is specified,
> 	the final value of <Rn> is UNPREDICTABLE.
> 
> Are you really sure about this?  Where do you know it from?

It doesn't matter.  The point is that you deleted an entry from the
table which the ldmia/ldmia+ldr loads from and didn't adjust the
resulting code properly.

- ARM(          ldmia   r0, {r1, r2, r3, r5, r6, r11, ip, sp})
+ ARM(          ldmia   r0, {r1, r2, r3, r5, r6, r11, ip})
  THUMB(                ldmia   r0, {r1, r2, r3, r5, r6, r11, ip}       )
- THUMB(                ldr     sp, [r0, #32]                           )
+               ldr     sp, [r0, #28]

In case you're wondering, it's this part, found in the "Instruction
details" section of "A8.6.53 LDM / LDMIA / LDMFD":

"The SP can be in the list in ARM code, but not in Thumb code. However,
ARM instructions that include the SP in the list are deprecated."

[PATCH] fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

[Uwe Kleine-König]

On Sat, Jul 31, 2010 at 10:04:45AM +0100, Russell King - ARM Linux wrote:
> On Fri, Jul 30, 2010 at 10:57:56PM +0200, Uwe Kleine-K?nig wrote:
> > On Sat, Jul 31, 2010 at 01:05:23AM +0530, Rabin Vincent wrote:
> > > "ARM: Auto calculate ZRELADDR and provide option for exceptions" broke the
> > > Thumb-2 decompressor because it removed an entry in the LC0 table but didn't
> > > adjust the offset the Thumb-2 code uses to load the SP from that table.
> > > 
> > > Fix it, and also change the ARM code to use the separate SP-load since
> > > ARM instructions that include the SP in the LDM register list are
> > > deprecated.
> > I only found:
> > 
> > 	LDM{<cond>}<addressing_mode> <Rn>{!}, <registers>
> > 	...
> > 	If the base register <Rn> is specified in <registers>, and base
> > 	register write-back is specified,
> > 	the final value of <Rn> is UNPREDICTABLE.
> > 
> > Are you really sure about this?  Where do you know it from?
> 
> It doesn't matter.  The point is that you deleted an entry from the
> table which the ldmia/ldmia+ldr loads from and didn't adjust the
> resulting code properly.
Yes, got that.
> 
> - ARM(          ldmia   r0, {r1, r2, r3, r5, r6, r11, ip, sp})
> + ARM(          ldmia   r0, {r1, r2, r3, r5, r6, r11, ip})
>   THUMB(                ldmia   r0, {r1, r2, r3, r5, r6, r11, ip}       )
> - THUMB(                ldr     sp, [r0, #32]                           )
> +               ldr     sp, [r0, #28]
We can simpify this further to just

	@ Thumb code doesn't allow sp to be in the list, for ARM it's
	@ deprecated, so use a seperate ldr for it.
	ldmia   r0, {r1, r2, r3, r5, r6, r11, ip}
	ldr     sp, [r0, #32]

> In case you're wondering, it's this part, found in the "Instruction
> details" section of "A8.6.53 LDM / LDMIA / LDMFD":
> 
> "The SP can be in the list in ARM code, but not in Thumb code. However,
> ARM instructions that include the SP in the list are deprecated."
OK, I didn't have the v7 ARMARM.

Thanks
Uwe

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

[PATCH] fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

[Russell King - ARM Linux]

On Sat, Jul 31, 2010 at 02:08:27PM +0200, Uwe Kleine-K?nig wrote:
> On Sat, Jul 31, 2010 at 10:04:45AM +0100, Russell King - ARM Linux wrote:
> > It doesn't matter.  The point is that you deleted an entry from the
> > table which the ldmia/ldmia+ldr loads from and didn't adjust the
> > resulting code properly.
> Yes, got that.
> > 
> > - ARM(          ldmia   r0, {r1, r2, r3, r5, r6, r11, ip, sp})
> > + ARM(          ldmia   r0, {r1, r2, r3, r5, r6, r11, ip})
> >   THUMB(                ldmia   r0, {r1, r2, r3, r5, r6, r11, ip}       )
> > - THUMB(                ldr     sp, [r0, #32]                           )
> > +               ldr     sp, [r0, #28]
> We can simpify this further to just
> 
> 	@ Thumb code doesn't allow sp to be in the list, for ARM it's
> 	@ deprecated, so use a seperate ldr for it.
> 	ldmia   r0, {r1, r2, r3, r5, r6, r11, ip}
> 	ldr     sp, [r0, #32]

No, you mean #28.

[PATCH] fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

[Uwe Kleine-König]

On Sat, Jul 31, 2010 at 01:31:42PM +0100, Russell King - ARM Linux wrote:
> On Sat, Jul 31, 2010 at 02:08:27PM +0200, Uwe Kleine-K?nig wrote:
> > On Sat, Jul 31, 2010 at 10:04:45AM +0100, Russell King - ARM Linux wrote:
> > > It doesn't matter.  The point is that you deleted an entry from the
> > > table which the ldmia/ldmia+ldr loads from and didn't adjust the
> > > resulting code properly.
> > Yes, got that.
> > > 
> > > - ARM(          ldmia   r0, {r1, r2, r3, r5, r6, r11, ip, sp})
> > > + ARM(          ldmia   r0, {r1, r2, r3, r5, r6, r11, ip})
> > >   THUMB(                ldmia   r0, {r1, r2, r3, r5, r6, r11, ip}       )
> > > - THUMB(                ldr     sp, [r0, #32]                           )
> > > +               ldr     sp, [r0, #28]
> > We can simpify this further to just
> > 
> > 	@ Thumb code doesn't allow sp to be in the list, for ARM it's
> > 	@ deprecated, so use a seperate ldr for it.
> > 	ldmia   r0, {r1, r2, r3, r5, r6, r11, ip}
> > 	ldr     sp, [r0, #32]
> 
> No, you mean #28.
oops, yes of course.

Best regards
Uwe

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

[PATCHv2] fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

[Rabin Vincent]

"ARM: Auto calculate ZRELADDR and provide option for exceptions" broke
the Thumb-2 decompressor because it removed an entry in the LC0 table
but didn't adjust the offset the Thumb-2 code uses to load the SP from
that table.

Fix it, and also change the ARM code to use the separate SP-load since
ARM instructions that include the SP in the LDM register list are
deprecated.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
v2: don't need separate code for ARM and THUMB since they're now
    the same.

 arch/arm/boot/compressed/head.S |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index abf4d65..6af9907 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -170,9 +170,8 @@ not_angel:
 
 		.text
 		adr	r0, LC0
- ARM(		ldmia	r0, {r1, r2, r3, r5, r6, r11, ip, sp})
- THUMB(		ldmia	r0, {r1, r2, r3, r5, r6, r11, ip}	)
- THUMB(		ldr	sp, [r0, #32]				)
+		ldmia	r0, {r1, r2, r3, r5, r6, r11, ip}
+		ldr	sp, [r0, #28]
 #ifdef CONFIG_AUTO_ZRELADDR
 		@ determine final kernel image address
 		and	r4, pc, #0xf8000000
-- 
1.7.1

[PATCHv2] fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

[Uwe Kleine-König]

Hello Rabin,

On Tue, Aug 03, 2010 at 06:10:06AM +0530, Rabin Vincent wrote:
> "ARM: Auto calculate ZRELADDR and provide option for exceptions" broke
> the Thumb-2 decompressor because it removed an entry in the LC0 table
> but didn't adjust the offset the Thumb-2 code uses to load the SP from
> that table.
> 
> Fix it, and also change the ARM code to use the separate SP-load since
> ARM instructions that include the SP in the LDM register list are
> deprecated.
> 
> Signed-off-by: Rabin Vincent <rabin@rab.in>
Acked-and-regretted-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

> ---
> v2: don't need separate code for ARM and THUMB since they're now
>     the same.
> 
>  arch/arm/boot/compressed/head.S |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index abf4d65..6af9907 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -170,9 +170,8 @@ not_angel:
>  
>  		.text
>  		adr	r0, LC0
> - ARM(		ldmia	r0, {r1, r2, r3, r5, r6, r11, ip, sp})
> - THUMB(		ldmia	r0, {r1, r2, r3, r5, r6, r11, ip}	)
> - THUMB(		ldr	sp, [r0, #32]				)
> +		ldmia	r0, {r1, r2, r3, r5, r6, r11, ip}
> +		ldr	sp, [r0, #28]
>  #ifdef CONFIG_AUTO_ZRELADDR
>  		@ determine final kernel image address
>  		and	r4, pc, #0xf8000000
> -- 
> 1.7.1
> 
> 

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

[PATCHv2] fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

[Eric Miao]

2010/8/3 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> Hello Rabin,
>
> On Tue, Aug 03, 2010 at 06:10:06AM +0530, Rabin Vincent wrote:
>> "ARM: Auto calculate ZRELADDR and provide option for exceptions" broke
>> the Thumb-2 decompressor because it removed an entry in the LC0 table
>> but didn't adjust the offset the Thumb-2 code uses to load the SP from
>> that table.
>>
>> Fix it, and also change the ARM code to use the separate SP-load since
>> ARM instructions that include the SP in the LDM register list are
>> deprecated.
>>
>> Signed-off-by: Rabin Vincent <rabin@rab.in>
> Acked-and-regretted-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

Nah, I seemed to introduce that so you don't have to feel regret :-)