ARM: bitops: Fix low-level code to be Thumb-2 compatible
PATCH 1/1 ARM: bitops: Fix low-level code to be Thumb-2 compatible
The new bitops code cunningly str <Rt>, [r1, -r1] to trigger a fault by attempting to store to address zero.
This code doesn't assemble in Thumb-2, since Thumb-2 doesn't allow negative register offsets at all for loads and stores.
The patch loads 0 into r2 and uses that as a base register instead for the Thumb-2 case: r2 seems non-live at every instance of this problem.
The ARM case is unaffected.
--- arch/arm/lib/bitops.h | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-)
arch/arm/lib/bitops.h +12 -4
@@ -1,7 +1,9 @@#if __LINUX_ARM_ARCH__ >=6 .macro bitop, instr tst r1, #3 - strne r1, [r1, -r1] @ assert word-aligned + ARM( strne r1, [r1, -r1] ) @ assert word-aligned + THUMB( movne r2, #0 ) + THUMB( strne r1, [r2] ) mov r2, #1 and r3, r0, #31 @ Get bit offset mov r0, r0, lsr #5
@@ -17,7 +19,9 @@ .macro testop, instr, store tst r1, #3 - strne r1, [r1, -r1] @ assert word-aligned + ARM( strne r1, [r1, -r1] ) @ assert word-aligned + THUMB( movne r2, #0 ) + THUMB( strne r1, [r2] ) mov r2, #1 and r3, r0, #31 @ Get bit offset mov r0, r0, lsr #5
@@ -38,7 +42,9 @@#else .macro bitop, instr tst r1, #3 - strne r1, [r1, -r1] @ assert word-aligned + ARM( strne r1, [r1, -r1] ) @ assert word-aligned + THUMB( movne r2, #0 ) + THUMB( strne r1, [r2] ) and r2, r0, #31 mov r0, r0, lsr #5 mov r3, #1
@@ -61,7 +67,9 @@*/ .macro testop, instr, store tst r1, #3 - strne r1, [r1, -r1] @ assert word-aligned + ARM( strne r1, [r1, -r1] ) @ assert word-aligned + THUMB( movne r2, #0 ) + THUMB( strne r1, [r2] ) and r3, r0, #31 mov r0, r0, lsr #5 save_and_disable_irqs ip
Signature · 1 line
1.7.1
PATCH 1/1 ARM: bitops: Fix low-level code to be Thumb-2 compatible
Quoted 6 lines · Dave Martin
The new bitops code cunningly str <Rt>, [r1, -r1] to trigger a fault by attempting to store to address zero.
This code doesn't assemble in Thumb-2, since Thumb-2 doesn't allow negative register offsets at all for loads and stores.
Quoted 7 lines
The patch loads 0 into r2 and uses that as a base register instead for the Thumb-2 case: r2 seems non-live at every instance of this problem.
The ARM case is unaffected.
Signed-off-by Dave Martin <[email protected]>
Russell proposed a better solution already:
http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home
No idea why this wasn't folded in his series yet though.
Nicolas
Quoted 61 lines
--- arch/arm/lib/bitops.h | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-)arch/arm/lib/bitops.h +12
-4@@ -1,7 +1,9 @@#if __LINUX_ARM_ARCH__ >=6 .macro bitop, instr tst r1, #3 - strne r1, [r1, -r1] @ assert word-aligned + ARM( strne r1, [r1, -r1] ) @ assert word-aligned + THUMB( movne r2, #0 ) + THUMB( strne r1, [r2] ) mov r2, #1 and r3, r0, #31 @ Get bit offset mov r0, r0, lsr #5@@ -17,7 +19,9 @@ .macro testop, instr, store tst r1, #3 - strne r1, [r1, -r1] @ assert word-aligned + ARM( strne r1, [r1, -r1] ) @ assert word-aligned + THUMB( movne r2, #0 ) + THUMB( strne r1, [r2] ) mov r2, #1 and r3, r0, #31 @ Get bit offset mov r0, r0, lsr #5@@ -38,7 +42,9 @@#else .macro bitop, instr tst r1, #3 - strne r1, [r1, -r1] @ assert word-aligned + ARM( strne r1, [r1, -r1] ) @ assert word-aligned + THUMB( movne r2, #0 ) + THUMB( strne r1, [r2] ) and r2, r0, #31 mov r0, r0, lsr #5 mov r3, #1@@ -61,7 +67,9 @@*/ .macro testop, instr, store tst r1, #3 - strne r1, [r1, -r1] @ assert word-aligned + ARM( strne r1, [r1, -r1] ) @ assert word-aligned + THUMB( movne r2, #0 ) + THUMB( strne r1, [r2] ) and r3, r0, #31 mov r0, r0, lsr #5 save_and_disable_irqs ipSignature · 7 lines
1.7.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 ARM: bitops: Fix low-level code to be Thumb-2 compatible
Quoted 14 lines · Nicolas Pitre
Quoted 12 lines · Dave Martin
The new bitops code cunningly str <Rt>, [r1, -r1] to trigger a fault by attempting to store to address zero.
This code doesn't assemble in Thumb-2, since Thumb-2 doesn't allow negative register offsets at all for loads and stores.
The patch loads 0 into r2 and uses that as a base register instead for the Thumb-2 case: r2 seems non-live at every instance of this problem.
The ARM case is unaffected.
Quoted 7 lines
Quoted 1 line
Signed-off-by Dave Martin <[email protected]>Russell proposed a better solution already:
http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home
No idea why this wasn't folded in his series yet though.
Because I haven't got a round tuit.
http://www.quantumenterprises.co.uk/roundtuit/traditional_round_tuits.htm
PATCH 1/1 ARM: bitops: Fix low-level code to be Thumb-2 compatible
Quoted 19 lines · Russell King - ARM Linux
Quoted 18 lines · Nicolas Pitre
Quoted 6 lines · Dave Martin
The new bitops code cunningly str <Rt>, [r1, -r1] to trigger a fault by attempting to store to address zero.
This code doesn't assemble in Thumb-2, since Thumb-2 doesn't allow negative register offsets at all for loads and stores.
Quoted 7 lines
The patch loads 0 into r2 and uses that as a base register instead for the Thumb-2 case: r2 seems non-live at every instance of this problem.
The ARM case is unaffected.
Signed-off-by Dave Martin <[email protected]>Russell proposed a better solution already:
Quoted 7 lines
Quoted 3 lines
http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home
No idea why this wasn't folded in his series yet though.
Because I haven't got a round tuit.
http://www.quantumenterprises.co.uk/roundtuit/traditional_round_tuits.htm
LOL! No problem, here's a virtual one. :-)
Nicolas
PATCH 1/1 ARM: bitops: Fix low-level code to be Thumb-2 compatible
Quoted 23 lines · Nicolas Pitre
Quoted 21 lines · Russell King - ARM Linux
Quoted 20 lines · Nicolas Pitre
Quoted 6 lines · Dave Martin
The new bitops code cunningly str <Rt>, [r1, -r1] to trigger a fault by attempting to store to address zero.
This code doesn't assemble in Thumb-2, since Thumb-2 doesn't allow negative register offsets at all for loads and stores.
Quoted 7 lines
The patch loads 0 into r2 and uses that as a base register instead for the Thumb-2 case: r2 seems non-live at every instance of this problem.
The ARM case is unaffected.
Signed-off-by Dave Martin <[email protected]>Russell proposed a better solution already:
http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
No idea why this wasn't folded in his series yet though.
Because I haven't got a round tuit.
http://www.quantumenterprises.co.uk/roundtuit/traditional_round_tuits.htm
LOL! No problem, here's a virtual one. :-)
While you're waiting, you might like to get involved with the firestorm which David Brown has started with GregKH over the visibility of my git tree. I'm beyond participating in such arguments.
Oh, and if I follow what GregKH is saying, I have to refuse to fold this change in.
PATCH 1/1 ARM: bitops: Fix low-level code to be Thumb-2 compatible
In reply to Nicolas PitreQuoted 14 lines · Nicolas Pitre
Quoted 12 lines · Dave Martin
The new bitops code cunningly str <Rt>, [r1, -r1] to trigger a fault by attempting to store to address zero.
This code doesn't assemble in Thumb-2, since Thumb-2 doesn't allow negative register offsets at all for loads and stores.
The patch loads 0 into r2 and uses that as a base register instead for the Thumb-2 case: r2 seems non-live at every instance of this problem.
The ARM case is unaffected.
Quoted 7 lines
Quoted 1 line
Signed-off-by Dave Martin <[email protected]>Russell proposed a better solution already:
http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home
No idea why this wasn't folded in his series yet though.
Now added, and original tested-by's dropped. It now needs re-testing.
8<----
Subject: [PATCH] ARM: bitops: ensure set/clear/change bitops take a word-aligned pointer
Add additional instructions to our assembly bitops functions to ensure that they only operate on word-aligned pointers. This will be necessary when we switch these operations to use the word-based exclusive operations.
--- arch/arm/lib/bitops.h | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
arch/arm/lib/bitops.h +8 -0
@@ -1,6 +1,8 @@#if __LINUX_ARM_ARCH__ >=6 && defined(CONFIG_CPU_32v6K) .macro bitop, instr + ands ip, r1, #3 + strneb r1, [ip] @ assert word-aligned mov r2, #1 and r3, r0, #7 @ Get bit offset add r1, r1, r0, lsr #3 @ Get byte offset
@@ -14,6 +16,8 @@ .endm .macro testop, instr, store + ands ip, r1, #3 + strneb r1, [ip] @ assert word-aligned and r3, r0, #7 @ Get bit offset mov r2, #1 add r1, r1, r0, lsr #3 @ Get byte offset
@@ -32,6 +36,8 @@ .endm #else .macro bitop, instr + ands ip, r1, #3 + strneb r1, [ip] @ assert word-aligned and r2, r0, #7 mov r3, #1 mov r3, r3, lsl r2
@@ -52,6 +58,8 @@* to avoid dirtying the data cache. */ .macro testop, instr, store + ands ip, r1, #3 + strneb r1, [ip] @ assert word-aligned add r1, r1, r0, lsr #3 and r3, r0, #7 mov r0, #1
Signature · 1 line
1.6.2.5
PATCH 1/1 ARM: bitops: Fix low-level code to be Thumb-2 compatible
Quoted 28 lines · Russell King - ARM Linux
Quoted 14 lines · Nicolas Pitre
Quoted 12 lines · Dave Martin
The new bitops code cunningly str <Rt>, [r1, -r1] to trigger a fault by attempting to store to address zero.
This code doesn't assemble in Thumb-2, since Thumb-2 doesn't allow negative register offsets at all for loads and stores.
The patch loads 0 into r2 and uses that as a base register instead for the Thumb-2 case: r2 seems non-live at every instance of this problem.
The ARM case is unaffected.
Quoted 7 lines
Quoted 1 line
Signed-off-by Dave Martin <[email protected]>Russell proposed a better solution already:
http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home
No idea why this wasn't folded in his series yet though.
Now added, and original tested-by's dropped. It now needs re-testing.
8<----
Subject: [PATCH] ARM: bitops: ensure set/clear/change bitops take a word-aligned pointer
Quoted 6 lines
Add additional instructions to our assembly bitops functions to ensure that they only operate on word-aligned pointers. This will be necessary when we switch these operations to use the word-based exclusive operations.
Signed-off-by Russell King <[email protected]>
And this has been tested by many people already as this is the version I've been carrying in the Linaro kernel tree for a while.
Quoted 47 lines
--- arch/arm/lib/bitops.h | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)arch/arm/lib/bitops.h +8
-0@@ -1,6 +1,8 @@#if __LINUX_ARM_ARCH__ >=6 && defined(CONFIG_CPU_32v6K) .macro bitop, instr + ands ip, r1, #3 + strneb r1, [ip] @ assert word-aligned mov r2, #1 and r3, r0, #7 @ Get bit offset add r1, r1, r0, lsr #3 @ Get byte offset@@ -14,6 +16,8 @@ .endm .macro testop, instr, store + ands ip, r1, #3 + strneb r1, [ip] @ assert word-aligned and r3, r0, #7 @ Get bit offset mov r2, #1 add r1, r1, r0, lsr #3 @ Get byte offset@@ -32,6 +36,8 @@ .endm #else .macro bitop, instr + ands ip, r1, #3 + strneb r1, [ip] @ assert word-aligned and r2, r0, #7 mov r3, #1 mov r3, r3, lsl r2@@ -52,6 +58,8 @@* to avoid dirtying the data cache. */ .macro testop, instr, store + ands ip, r1, #3 + strneb r1, [ip] @ assert word-aligned add r1, r1, r0, lsr #3 and r3, r0, #7 mov r0, #1Signature · 1 line
1.6.2.5
PATCH 1/1 ARM: bitops: Fix low-level code to be Thumb-2 compatible
In reply to Russell King - ARM LinuxQuoted 72 lines · Russell King - ARM Linux
Quoted 14 lines · Nicolas Pitre
Quoted 12 lines · Dave Martin
The new bitops code cunningly str <Rt>, [r1, -r1] to trigger a fault by attempting to store to address zero.
This code doesn't assemble in Thumb-2, since Thumb-2 doesn't allow negative register offsets at all for loads and stores.
The patch loads 0 into r2 and uses that as a base register instead for the Thumb-2 case: r2 seems non-live at every instance of this problem.
The ARM case is unaffected.
Quoted 7 lines
Quoted 1 line
Signed-off-by Dave Martin <[email protected]>Russell proposed a better solution already:
http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home
No idea why this wasn't folded in his series yet though.
Now added, and original tested-by's dropped. ?It now needs re-testing.
8<----
Subject: [PATCH] ARM: bitops: ensure set/clear/change bitops take a word-aligned pointerAdd additional instructions to our assembly bitops functions to ensure that they only operate on word-aligned pointers. ?This will be necessary when we switch these operations to use the word-based exclusive operations.
Signed-off-by Russell King <[email protected]>---
?arch/arm/lib/bitops.h | ? ?8 ++++++++
?1 files changed, 8 insertions(+), 0 deletions(-)arch/arm/lib/bitops.h +0
-0@@ -1,6 +1,8 @@ ?#if __LINUX_ARM_ARCH__ >= 6 && defined(CONFIG_CPU_32v6K)? ? ? ?.macro ?bitop, instr
+ ? ? ? ands ? ?ip, r1, #3
+ ? ? ? strneb ?r1, [ip] ? ? ? ? ? ? ? ?@ assert word-aligned ? ? ? ?mov ? ? r2, #1
? ? ? ?and ? ? r3, r0, #7 ? ? ? ? ? ? ?@ Get bit offset ? ? ? ?add ? ? r1, r1, r0, lsr #3 ? ? ?@ Get byte offsetarch/arm/lib/bitops.h +0
-0@@ -14,6 +16,8 @@? ? ? ?.endm
? ? ? ?.macro ?testop, instr, store
+ ? ? ? ands ? ?ip, r1, #3
+ ? ? ? strneb ?r1, [ip] ? ? ? ? ? ? ? ?@ assert word-aligned ? ? ? ?and ? ? r3, r0, #7 ? ? ? ? ? ? ?@ Get bit offset ? ? ? ?mov ? ? r2, #1
? ? ? ?add ? ? r1, r1, r0, lsr #3 ? ? ?@ Get byte offsetarch/arm/lib/bitops.h +0
-0@@ -32,6 +36,8 @@? ? ? ?.endm
?#else
? ? ? ?.macro ?bitop, instr
+ ? ? ? ands ? ?ip, r1, #3
+ ? ? ? strneb ?r1, [ip] ? ? ? ? ? ? ? ?@ assert word-aligned ? ? ? ?and ? ? r2, r0, #7
? ? ? ?mov ? ? r3, #1
? ? ? ?mov ? ? r3, r3, lsl r2arch/arm/lib/bitops.h +0
-0@@ -52,6 +58,8 @@?* to avoid dirtying the data cache.
?*/
Quoted 10 lines
? ? ? ?.macro ?testop, instr, store
+ ? ? ? ands ? ?ip, r1, #3
+ ? ? ? strneb ?r1, [ip] ? ? ? ? ? ? ? ?@ assert word-aligned ? ? ? ?add ? ? r1, r1, r0, lsr #3
? ? ? ?and ? ? r3, r0, #7
? ? ? ?mov ? ? r0, #1
--
1.6.2.5
Heh, yes, that's a nicer fix. I'll give it a try.
Thanks
---Dave
PATCH 1/1 ARM: bitops: Use BX instead of MOV PC,LR
In reply to Russell King - ARM LinuxThe kernel doesn't officially need to interwork, but using BX wherever appropriate will help educate people into good assembler coding habits.
BX is appropriate here because this code is predicated on
---
A bit of a pedantic one, but is there any chance of switching to using BX here?
I spend a significant amount of my life trying to educate people not to do things like mov pc,lr ... though in interworking userspace is admittedly a different world from the kernel.
Cheers
Dave
arch/arm/lib/bitops.h | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
arch/arm/lib/bitops.h +4 -4
@@ -12,7 +12,7 @@ strex r0, r2, [r1] cmp r0, #0 bne 1b - mov pc, lr + bx lr .endm .macro testop, instr, store
@@ -33,7 +33,7 @@ smp_dmb cmp r0, #0 movne r0, #1 -2: mov pc, lr +2: bx lr .endm #else .macro bitop, instr
@@ -48,7 +48,7 @@ \instr r2, r2, r3 str r2, [r1, r0, lsl #2] restore_irqs ip - mov pc, lr + bx lr .endm/**
@@ -72,6 +72,6 @@ \store r2, [r1] moveq r0, #0 restore_irqs ip -mov pc, lr + bx lr .endm#endif
Signature · 1 line
1.7.1
PATCH 1/1 ARM: bitops: Use BX instead of MOV PC,LR
The kernel doesn't officially need to interwork, but using BX wherever appropriate will help educate people into good assembler coding habits.
BX is appropriate here because this code is predicated on __LINUX_ARM_ARCH__ >= 6.
---
Reposting ... git ate my commit message :/
arch/arm/lib/bitops.h | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
arch/arm/lib/bitops.h +4 -4
@@ -12,7 +12,7 @@ strex r0, r2, [r1] cmp r0, #0 bne 1b - mov pc, lr + bx lr .endm .macro testop, instr, store
@@ -33,7 +33,7 @@ smp_dmb cmp r0, #0 movne r0, #1 -2: mov pc, lr +2: bx lr .endm #else .macro bitop, instr
@@ -48,7 +48,7 @@ \instr r2, r2, r3 str r2, [r1, r0, lsl #2] restore_irqs ip - mov pc, lr + bx lr .endm/**
@@ -72,6 +72,6 @@ \store r2, [r1] moveq r0, #0 restore_irqs ip -mov pc, lr + bx lr .endm#endif
Signature · 1 line
1.7.1
PATCH 1/1 ARM: bitops: Use BX instead of MOV PC,LR
Quoted 2 lines · Dave Martin
BX is appropriate here because this code is predicated on
__LINUX_ARM_ARCH__>= 6.
But you're modifying the #else portion too?
PATCH 1/1 ARM: bitops: Use BX instead of MOV PC,LR
Quoted 6 lines · Rabin Vincent
Quoted 2 lines · Dave Martin
BX is appropriate here because this code is predicated on
__LINUX_ARM_ARCH__>= 6.But you're modifying the #else portion too?
Darn, you're quite right... thanks for spotting that!
Will repost.
Cheers
---Dave