ARM: Fix SMP_ON_UP for non ARM ltd. implementations

PATCH In review Stephen Boyd 3 participants 7 messages Jan 26, 2011 – Jan 31, 2011

Subsystems (2)

Arm (32-bit) Arm Qualcomm mailing list

lore

Share & tools

lore

Stephen Boyd

author

2011-01-26 23:04 UTC #0 lore raw

PATCH 1/1 ARM: Fix SMP_ON_UP for non ARM ltd. implementations

The SMP_ON_UP checks restrict the feature to ARM limited designs, when other implementers support SMP designs. Instead of checking for an ARM CPU and ARMv6/v7 just check for a v6 or v7 and then rely on the MPIDR for non ARM 11MPCore designs.

Signed-off-by Stephen Boyd <[email protected]>

---
 arch/arm/kernel/head.S |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

arch/arm/kernel/head.S +9 -4

index f17d9a0..ede5b70 100644
@@ -393,20 +393,25 @@ ENDPROC(__turn_mmu_on)
 #ifdef CONFIG_SMP_ON_UP
 __fixup_smp:
 	mov	r4, #0x00070000
-	orr	r3, r4, #0xff000000	@ mask 0xff070000
-	orr	r4, r4, #0x41000000	@ val 0x41070000
-	and	r0, r9, r3
-	teq	r0, r4			@ ARM CPU and ARMv6/v7?
+	and	r0, r9, r4
+	teq	r0, r4			@ ARMv6/v7?
 	bne	__fixup_smp_on_up	@ no, assume UP
 
+	orr	r3, r4, #0xff000000	@ mask 0xff070000
 	orr	r3, r3, #0x0000ff00
 	orr	r3, r3, #0x000000f0	@ mask 0xff07fff0
+	orr	r4, r4, #0x41000000	@ val 0x41070000
 	orr	r4, r4, #0x0000b000
 	orr	r4, r4, #0x00000020	@ val 0x4107b020
 	and	r0, r9, r3
 	teq	r0, r4			@ ARM 11MPCore?
 	moveq	pc, lr			@ yes, assume SMP
 
+	mov	r4, #0x00070000
+	and	r0, r9, #0x000f0000
+	teq	r0, r4			@ ARMv6?
+	beq	__fixup_smp_on_up	@ yes, assume UP
+
 	mrc	p15, 0, r0, c0, c0, 5	@ read MPIDR
 	tst	r0, #1 << 31
 	movne	pc, lr			@ bit 31 => SMP

Signature · 2 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Russell King - ARM Linux

2011-01-30 12:20 UTC #1 lore raw

Quoted 17 lines · Stephen Boyd

(quoted diff) +4 -4

@@ -393,20 +393,25 @@ ENDPROC(__turn_mmu_on)
 #ifdef CONFIG_SMP_ON_UP
 __fixup_smp:
 	mov	r4, #0x00070000
-	orr	r3, r4, #0xff000000	@ mask 0xff070000
-	orr	r4, r4, #0x41000000	@ val 0x41070000
-	and	r0, r9, r3
-	teq	r0, r4			@ ARM CPU and ARMv6/v7?
+	and	r0, r9, r4
+	teq	r0, r4			@ ARMv6/v7?
 	bne	__fixup_smp_on_up	@ no, assume UP
 
+	orr	r3, r4, #0xff000000	@ mask 0xff070000
 	orr	r3, r3, #0x0000ff00
 	orr	r3, r3, #0x000000f0	@ mask 0xff07fff0
+	orr	r4, r4, #0x41000000	@ val 0x41070000
 	orr	r4, r4, #0x0000b000

Review ENDPROC(__turn_mmu_on) · line 414

Quoted 10 lines

(quoted diff) +5 -0

 	orr	r4, r4, #0x00000020	@ val 0x4107b020
 	and	r0, r9, r3
 	teq	r0, r4			@ ARM 11MPCore?
 	moveq	pc, lr			@ yes, assume SMP
 
+	mov	r4, #0x00070000
+	and	r0, r9, #0x000f0000
+	teq	r0, r4			@ ARMv6?
+	beq	__fixup_smp_on_up	@ yes, assume UP
+

Wouldn't it be better to check for CPUID presence first, then ARM11MPcore, and lastly preserve of MPIDR-flagged extensions?

Will - can you check whether the below is correct?

 arch/arm/kernel/head.S |   17 +++++++----------
 1 files changed, 7 insertions(+), 10 deletions(-)

arch/arm/kernel/head.S +7 ~~-10~~

index 17a97b5..f6b31c4 100644
@@ -390,19 +390,16 @@ ENDPROC(__turn_mmu_on)
 
 #ifdef CONFIG_SMP_ON_UP
 __fixup_smp:
-	mov	r4, #0x00070000
-	orr	r3, r4, #0xff000000	@ mask 0xff070000
-	orr	r4, r4, #0x41000000	@ val 0x41070000
-	and	r0, r9, r3
-	teq	r0, r4			@ ARM CPU and ARMv6/v7?
+	and	r3, r9, #0x000f0000	@ architecture version
+	teq	r3, #0x000f00000	@ CPU ID supported?
 	bne	__fixup_smp_on_up	@ no, assume UP
 
-	orr	r3, r3, #0x0000ff00
-	orr	r3, r3, #0x000000f0	@ mask 0xff07fff0
+	bic	r3, r9, #0x00ff0000
+	bic	r3, r3, #0x0000000f	@ mask 0xff00fff0
+	mov	r4, #0x41000000
 	orr	r4, r4, #0x0000b000
-	orr	r4, r4, #0x00000020	@ val 0x4107b020
-	and	r0, r9, r3
-	teq	r0, r4			@ ARM 11MPCore?
+	orr	r4, r4, #0x00000020	@ val 0x4100b020
+	teq	r3, r4			@ ARM 11MPCore?
 	moveq	pc, lr			@ yes, assume SMP
 
 	mrc	p15, 0, r0, c0, c0, 5	@ read MPIDR

Will Deacon

Acked-by

2011-01-30 16:27 UTC #2 lore raw

Hi Russell,

Quoted 23 lines · Russell King - ARM Linux

Quoted 22 lines · Stephen Boyd

arch/arm/kernel/head.S +4 -4

@@ -393,20 +393,25 @@ ENDPROC(__turn_mmu_on)
 #ifdef CONFIG_SMP_ON_UP
 __fixup_smp:
      mov     r4, #0x00070000
-     orr     r3, r4, #0xff000000     @ mask 0xff070000
-     orr     r4, r4, #0x41000000     @ val 0x41070000
-     and     r0, r9, r3
-     teq     r0, r4                  @ ARM CPU and ARMv6/v7?
+     and     r0, r9, r4
+     teq     r0, r4                  @ ARMv6/v7?
      bne     __fixup_smp_on_up       @ no, assume UP
 
+     orr     r3, r4, #0xff000000     @ mask 0xff070000
      orr     r3, r3, #0x0000ff00
      orr     r3, r3, #0x000000f0     @ mask 0xff07fff0
+     orr     r4, r4, #0x41000000     @ val 0x41070000
      orr     r4, r4, #0x0000b000
      orr     r4, r4, #0x00000020     @ val 0x4107b020
      and     r0, r9, r3
      teq     r0, r4                  @ ARM 11MPCore?
      moveq   pc, lr                  @ yes, assume SMP

Quoted 9 lines

Review ENDPROC(__turn_mmu_on) · line 414
Quoted 5 lines
(quoted diff) +5 -0
+     mov     r4, #0x00070000
+     and     r0, r9, #0x000f0000
+     teq     r0, r4                  @ ARMv6?
+     beq     __fixup_smp_on_up       @ yes, assume UP
+
Wouldn't it be better to check for CPUID presence first, then ARM11MPcore, and lastly preserve of MPIDR-flagged extensions?

Yes, the v6 catch-all at the end is pretty horrible.

Quoted 10 lines

Will - can you check whether the below is correct?
 arch/arm/kernel/head.S |   17 +++++++----------
 1 files changed, 7 insertions(+), 10 deletions(-)
arch/arm/kernel/head.S +0 -0
index 17a97b5..f6b31c4 100644

arch/arm/kernel/head.S ENDPROC(__turn_mmu_on) · line 394

Quoted 10 lines

arch/arm/kernel/head.S +2 -5

@@ -390,19 +390,16 @@ ENDPROC(__turn_mmu_on)
 
 #ifdef CONFIG_SMP_ON_UP
 __fixup_smp:
-       mov     r4, #0x00070000
-       orr     r3, r4, #0xff000000     @ mask 0xff070000
-       orr     r4, r4, #0x41000000     @ val 0x41070000
-       and     r0, r9, r3
-       teq     r0, r4                  @ ARM CPU and ARMv6/v7?
+       and     r3, r9, #0x000f0000     @ architecture version
+       teq     r3, #0x000f00000        @ CPU ID supported?

You've got an extra 0x0 on the end of that constant.

Quoted 5 lines

arch/arm/kernel/head.S +1 -2

        bne     __fixup_smp_on_up       @ no, assume UP
 
-       orr     r3, r3, #0x0000ff00
-       orr     r3, r3, #0x000000f0     @ mask 0xff07fff0
+       bic     r3, r9, #0x00ff0000

arch/arm/kernel/head.S ENDPROC(__turn_mmu_on) · line 404

Quoted 10 lines

arch/arm/kernel/head.S +4 -3

+       bic     r3, r3, #0x0000000f     @ mask 0xff00fff0
+       mov     r4, #0x41000000
        orr     r4, r4, #0x0000b000
-       orr     r4, r4, #0x00000020     @ val 0x4107b020
-       and     r0, r9, r3
-       teq     r0, r4                  @ ARM 11MPCore?
+       orr     r4, r4, #0x00000020     @ val 0x4100b020
+       teq     r3, r4                  @ ARM 11MPCore?
        moveq   pc, lr                  @ yes, assume SMP

Yup, that looks correct to me. 11MPCore has 0xf for the architecture field so we'll end up identifying it correctly here.

arch/arm/kernel/head.S ENDPROC(__turn_mmu_on) · line 405

Quoted 1 line

arch/arm/kernel/head.S +0 -0

        mrc     p15, 0, r0, c0, c0, 5   @ read MPIDR

The code following this does:

tst r0, #1 << 31
movne pc, lr @ bit 31 => SMP

As an optimisation, we could also check that bit 30 is zero so that we patch out the SMP stuff for single-core A5/A9/A15. Up to you.

With the typo fixed:

Acked-by Will Deacon <[email protected]>

Will

Russell King - ARM Linux

2011-01-30 16:40 UTC #3 lore raw

Quoted 10 lines · Will Deacon

Quoted 1 line
        mrc     p15, 0, r0, c0, c0, 5   @ read MPIDR
The code following this does:
tst r0, #1 << 31
movne pc, lr @ bit 31 => SMP
As an optimisation, we could also check that bit 30 is zero so that we patch out the SMP stuff for single-core A5/A9/A15. Up to you.

Ok, so with that added, it becomes:

 arch/arm/kernel/head.S |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

arch/arm/kernel/head.S +10 ~~-12~~

index f17d9a0..c0225da 100644
@@ -392,24 +392,22 @@ ENDPROC(__turn_mmu_on)
 
 #ifdef CONFIG_SMP_ON_UP
 __fixup_smp:
-	mov	r4, #0x00070000
-	orr	r3, r4, #0xff000000	@ mask 0xff070000
-	orr	r4, r4, #0x41000000	@ val 0x41070000
-	and	r0, r9, r3
-	teq	r0, r4			@ ARM CPU and ARMv6/v7?
+	and	r3, r9, #0x000f0000	@ architecture version
+	teq	r3, #0x000f0000		@ CPU ID supported?
 	bne	__fixup_smp_on_up	@ no, assume UP
 
-	orr	r3, r3, #0x0000ff00
-	orr	r3, r3, #0x000000f0	@ mask 0xff07fff0
+	bic	r3, r9, #0x00ff0000
+	bic	r3, r3, #0x0000000f	@ mask 0xff00fff0
+	mov	r4, #0x41000000
 	orr	r4, r4, #0x0000b000
-	orr	r4, r4, #0x00000020	@ val 0x4107b020
-	and	r0, r9, r3
-	teq	r0, r4			@ ARM 11MPCore?
+	orr	r4, r4, #0x00000020	@ val 0x4100b020
+	teq	r3, r4			@ ARM 11MPCore?
 	moveq	pc, lr			@ yes, assume SMP
 
 	mrc	p15, 0, r0, c0, c0, 5	@ read MPIDR
-	tst	r0, #1 << 31
-	movne	pc, lr			@ bit 31 => SMP
+	and	r0, r0, #0xc0000000	@ multiprocessing extensions and
+	teq	r0, #0x80000000		@ not part of a uniprocessor system?
+	moveq	pc, lr			@ yes, assume SMP
 
 __fixup_smp_on_up:
 	adr	r0, 1f

Will Deacon

2011-01-31 10:32 UTC #4 lore raw

PATCH 1/1 ARM: Fix SMP_ON_UP for non ARM ltd. implementations

Quoted 37 lines

Quoted 2 lines

As an optimisation, we could also check that bit 30 is zero so that we patch out the SMP stuff for single-core A5/A9/A15. Up to you.

Ok, so with that added, it becomes:

 arch/arm/kernel/head.S |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

arch/arm/kernel/head.S +7 ~~-10~~

index f17d9a0..c0225da 100644
@@ -392,24 +392,22 @@ ENDPROC(__turn_mmu_on)
 
 #ifdef CONFIG_SMP_ON_UP
 __fixup_smp:
-	mov	r4, #0x00070000
-	orr	r3, r4, #0xff000000	@ mask 0xff070000
-	orr	r4, r4, #0x41000000	@ val 0x41070000
-	and	r0, r9, r3
-	teq	r0, r4			@ ARM CPU and ARMv6/v7?
+	and	r3, r9, #0x000f0000	@ architecture version
+	teq	r3, #0x000f0000		@ CPU ID supported?
 	bne	__fixup_smp_on_up	@ no, assume UP
 
-	orr	r3, r3, #0x0000ff00
-	orr	r3, r3, #0x000000f0	@ mask 0xff07fff0
+	bic	r3, r9, #0x00ff0000
+	bic	r3, r3, #0x0000000f	@ mask 0xff00fff0
+	mov	r4, #0x41000000
 	orr	r4, r4, #0x0000b000
-	orr	r4, r4, #0x00000020	@ val 0x4107b020
-	and	r0, r9, r3
-	teq	r0, r4			@ ARM 11MPCore?
+	orr	r4, r4, #0x00000020	@ val 0x4100b020
+	teq	r3, r4			@ ARM 11MPCore?
 	moveq	pc, lr			@ yes, assume SMP

arch/arm/kernel/head.S ENDPROC(__turn_mmu_on) · line 413

Quoted 10 lines

arch/arm/kernel/head.S +3 -2

 
 	mrc	p15, 0, r0, c0, c0, 5	@ read MPIDR
-	tst	r0, #1 << 31
-	movne	pc, lr			@ bit 31 => SMP
+	and	r0, r0, #0xc0000000	@ multiprocessing extensions and
+	teq	r0, #0x80000000		@ not part of a uniprocessor system?
+	moveq	pc, lr			@ yes, assume SMP
 
 __fixup_smp_on_up:
 	adr	r0, 1f

That should do the trick.

Acked-by Will Deacon <[email protected]>

Thanks,

Will

Will Deacon

Acked-by

2011-01-31 10:32 UTC #5 lore raw

In reply to Russell King - ARM Linux

Quoted 37 lines

Quoted 2 lines

As an optimisation, we could also check that bit 30 is zero so that we patch out the SMP stuff for single-core A5/A9/A15. Up to you.

Ok, so with that added, it becomes:

 arch/arm/kernel/head.S |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

arch/arm/kernel/head.S +7 ~~-10~~

index f17d9a0..c0225da 100644
@@ -392,24 +392,22 @@ ENDPROC(__turn_mmu_on)
 
 #ifdef CONFIG_SMP_ON_UP
 __fixup_smp:
-	mov	r4, #0x00070000
-	orr	r3, r4, #0xff000000	@ mask 0xff070000
-	orr	r4, r4, #0x41000000	@ val 0x41070000
-	and	r0, r9, r3
-	teq	r0, r4			@ ARM CPU and ARMv6/v7?
+	and	r3, r9, #0x000f0000	@ architecture version
+	teq	r3, #0x000f0000		@ CPU ID supported?
 	bne	__fixup_smp_on_up	@ no, assume UP
 
-	orr	r3, r3, #0x0000ff00
-	orr	r3, r3, #0x000000f0	@ mask 0xff07fff0
+	bic	r3, r9, #0x00ff0000
+	bic	r3, r3, #0x0000000f	@ mask 0xff00fff0
+	mov	r4, #0x41000000
 	orr	r4, r4, #0x0000b000
-	orr	r4, r4, #0x00000020	@ val 0x4107b020
-	and	r0, r9, r3
-	teq	r0, r4			@ ARM 11MPCore?
+	orr	r4, r4, #0x00000020	@ val 0x4100b020
+	teq	r3, r4			@ ARM 11MPCore?
 	moveq	pc, lr			@ yes, assume SMP

arch/arm/kernel/head.S ENDPROC(__turn_mmu_on) · line 413

Quoted 10 lines

arch/arm/kernel/head.S +3 -2

 
 	mrc	p15, 0, r0, c0, c0, 5	@ read MPIDR
-	tst	r0, #1 << 31
-	movne	pc, lr			@ bit 31 => SMP
+	and	r0, r0, #0xc0000000	@ multiprocessing extensions and
+	teq	r0, #0x80000000		@ not part of a uniprocessor system?
+	moveq	pc, lr			@ yes, assume SMP
 
 __fixup_smp_on_up:
 	adr	r0, 1f

That should do the trick.

Acked-by Will Deacon <[email protected]>

Thanks,

Will

Stephen Boyd

author

2011-01-31 21:33 UTC #6 lore raw

In reply to Russell King - ARM Linux

Quoted 34 lines · Russell King - ARM Linux

Ok, so with that added, it becomes:

 arch/arm/kernel/head.S |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

arch/arm/kernel/head.S +7 ~~-10~~

index f17d9a0..c0225da 100644
@@ -392,24 +392,22 @@ ENDPROC(__turn_mmu_on)
 
 #ifdef CONFIG_SMP_ON_UP
 __fixup_smp:
-	mov	r4, #0x00070000
-	orr	r3, r4, #0xff000000	@ mask 0xff070000
-	orr	r4, r4, #0x41000000	@ val 0x41070000
-	and	r0, r9, r3
-	teq	r0, r4			@ ARM CPU and ARMv6/v7?
+	and	r3, r9, #0x000f0000	@ architecture version
+	teq	r3, #0x000f0000		@ CPU ID supported?
 	bne	__fixup_smp_on_up	@ no, assume UP
 
-	orr	r3, r3, #0x0000ff00
-	orr	r3, r3, #0x000000f0	@ mask 0xff07fff0
+	bic	r3, r9, #0x00ff0000
+	bic	r3, r3, #0x0000000f	@ mask 0xff00fff0
+	mov	r4, #0x41000000
 	orr	r4, r4, #0x0000b000
-	orr	r4, r4, #0x00000020	@ val 0x4107b020
-	and	r0, r9, r3
-	teq	r0, r4			@ ARM 11MPCore?
+	orr	r4, r4, #0x00000020	@ val 0x4100b020
+	teq	r3, r4			@ ARM 11MPCore?
 	moveq	pc, lr			@ yes, assume SMP

arch/arm/kernel/head.S ENDPROC(__turn_mmu_on) · line 413

Quoted 10 lines

arch/arm/kernel/head.S +3 -2

 
 	mrc	p15, 0, r0, c0, c0, 5	@ read MPIDR
-	tst	r0, #1 << 31
-	movne	pc, lr			@ bit 31 => SMP
+	and	r0, r0, #0xc0000000	@ multiprocessing extensions and
+	teq	r0, #0x80000000		@ not part of a uniprocessor system?
+	moveq	pc, lr			@ yes, assume SMP
 
 __fixup_smp_on_up:
 	adr	r0, 1f

Great. Looks good to me too. If I had know that ARM 11MPCore had an 0xf in the architecture bits I would have suggested this fix instead.

Also, I wasn't sure if we should send fixes for EXPERIMENTAL features to the stable tree. If its appropriate, please Cc stable when the patch goes in.

Reported-and-Tested-by Stephen Boyd <[email protected]>

Signature · 2 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.