fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

PATCH New Rabin Vincent 4 participants 10 messages Jul 30, 2010 – Aug 3, 2010 Arm (32-bit)

Share & tools

Rabin Vincent

author

2010-07-30 19:35 UTC #0 lore raw

PATCH 1/1 fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

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

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

arch/arm/boot/compressed/head.S +2 -2

index abf4d65..47924fc 100644
@@ -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

Signature · 1 line

1.7.1

Uwe Kleine-König

2010-07-30 20:57 UTC #1 lore raw

PATCH 1/1 fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

Quoted 7 lines · 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.

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

Signature · 2 lines

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

Rabin Vincent

author

2010-07-30 23:44 UTC #2 lore raw

PATCH 1/1 fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

2010/7/31 Uwe Kleine-K?nig <[email protected]>:

Quoted 6 lines · Rabin Vincent

Quoted 3 lines
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:

Quoted 7 lines

? ? ? ?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.

Russell King - ARM Linux

2010-07-31 09:04 UTC #3 lore raw

PATCH 1/1 fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

In reply to Uwe Kleine-König

Quoted 10 lines · Uwe Kleine-K?nig

Quoted 7 lines · 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.
I only found:

Quoted 7 lines

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."

Uwe Kleine-König

2010-07-31 12:08 UTC #4 lore raw

PATCH 1/1 fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

Quoted 17 lines · Russell King - ARM Linux

Quoted 16 lines · Uwe Kleine-K?nig
Quoted 7 lines · 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.
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.

Quoted 5 lines

Quoted 1 line
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.

Quoted 6 lines

- 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]

Quoted 5 lines

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

Signature · 2 lines

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

Russell King - ARM Linux

2010-07-31 12:31 UTC #5 lore raw

PATCH 1/1 fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

Quoted 11 lines · Uwe Kleine-K?nig

Quoted 3 lines · Russell King - ARM Linux
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.
Quoted 6 lines
- 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]

Quoted 6 lines

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.

Uwe Kleine-König

2010-07-31 12:50 UTC #6 lore raw

PATCH 1/1 fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

Quoted 12 lines · Russell King - ARM Linux

Quoted 11 lines · Uwe Kleine-K?nig
Quoted 3 lines · Russell King - ARM Linux
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.
Quoted 6 lines
- 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]

Quoted 8 lines

Quoted 6 lines

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

Signature · 2 lines

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

Rabin Vincent

author

2010-08-03 00:40 UTC #7 lore raw

PATCH v2 1/1 fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

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

---
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(-)

arch/arm/boot/compressed/head.S +2 -3

index abf4d65..6af9907 100644
@@ -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

Signature · 1 line

1.7.1

Uwe Kleine-König

2010-08-03 07:36 UTC #8 lore raw

PATCH v2 1/1 fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

Hello Rabin,

Quoted 10 lines · 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 <[email protected]>

Acked-and-regretted-by Uwe Kleine-K?nig <[email protected]>

Thanks
Uwe

Quoted 27 lines

---
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(-)

arch/arm/boot/compressed/head.S +2 -3

index abf4d65..6af9907 100644
@@ -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

Signature · 1 line

1.7.1

Signature · 2 lines

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

Eric Miao

2010-08-03 07:47 UTC #9 lore raw

PATCH v2 1/1 fix Thumb-2 decompressor broken by "Auto calculate ZRELADDR"

2010/8/3 Uwe Kleine-K?nig <[email protected]>:

Quoted 8 lines

Hello Rabin,
Quoted 5 lines · 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.

Quoted 6 lines

Quoted 5 lines
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 <[email protected]>
Acked-and-regretted-by Uwe Kleine-K?nig <[email protected]>

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