Make ARMv6 behave with TLS, VFPv3, and NEON

The TLS register is only available on ARM1136 r1p0 and later. Set HWCAP_TLS flags if hardware TLS is available.

Note that we now use 0xffff0ff4 for flagging software TLS to __kuser_get_tls, and 0xffff0ff8 for storing the software TLS value.

__kuser_get_tls: @ 0xffff0fe0

arch/arm/kernel/entry-armv.S +1 -6

-
-#if !defined(CONFIG_HAS_TLS_REG) && !defined(CONFIG_TLS_REG_EMUL)
-	ldr	r0, [pc, #(16 - 8)]		@ TLS stored at 0xffff0ff0
-#else
-	mrc	p15, 0, r0, c13, c0, 3		@ read TLS register
-#endif
+	ldr     r0, [pc, #(20 - 8)]		@ software TLS set in 0xffff0ff4?

Quoted 15 lines · Tony Lindgren

__kuser_get_tls: @ 0xffff0fe0

arch/arm/kernel/entry-armv.S +4 -9

-
-#if !defined(CONFIG_HAS_TLS_REG) && !defined(CONFIG_TLS_REG_EMUL)
-	ldr	r0, [pc, #(16 - 8)]		@ TLS stored at 0xffff0ff0
-#else
-	mrc	p15, 0, r0, c13, c0, 3		@ read TLS register
-#endif
+	ldr     r0, [pc, #(20 - 8)]		@ software TLS set in 0xffff0ff4?
+	cmp	r0, #0				@ hardware TLS if flag not set
+	mrceq	p15, 0, r0, c13, c0, 3		@ read hardware TLS register
+	ldrne	r0, [pc, #(12 - 8)]		@ software TLS val at 0xffff0ff8
 	usr_ret	lr
-
-	.rep	5
-	.word	0			@ pad up to __kuser_helper_version

arch/arm/kernel/entry-armv.S kuser_cmpxchg_fixup: · line 1021

Quoted 3 lines

arch/arm/kernel/entry-armv.S +2 -1

-	.endr
+	.word	0				@ non-zero for software TLS
+	.word	0				@ software TLS value

It'd be nice not to waste instructions checking for HWCAP_TLS on archs which definitely don't have it. I guess it doesn't matter elsewhere; I'd expect this to be a warm path for some programs making extensive use of TLS (I haven't measured though).

As it's only a single instruction, and the code is in a writable page already (copied at init), how about just patching the instruction when ELF_HWCAP is set?

Yeah that can be done for __kuser_get_tls if it's always writable. But __switch_to is trickier because of the CONFIG_MMU ifdefs there.

What if we have optional __switch_to and __kuser_get_tls implementations in the mm/proc-*.S files that get copied over the current locations if implemented?

Quoted 2 lines · Tony Lindgren

Yeah that can be done for __kuser_get_tls if it's always writable. But __switch_to is trickier because of the CONFIG_MMU ifdefs there.

And impossible with XIP kernels.

Quoted 3 lines

What if we have optional __switch_to and __kuser_get_tls implementations in the mm/proc-*.S files that get copied over the current locations if implemented?

Also problematical with XIP - if we go down the route of implementing these by copying code fragments into the kernel, we need to strip out XIP support or implement a second way. Obviously having a second way adds maintainence burden, and the second way will probably lose out on updates.

Quoted 10 lines · Russell King - ARM Linux

Quoted 2 lines · Tony Lindgren

Yeah that can be done for __kuser_get_tls if it's always writable. But __switch_to is trickier because of the CONFIG_MMU ifdefs there.

And impossible with XIP kernels.

Quoted 3 lines

What if we have optional __switch_to and __kuser_get_tls implementations in the mm/proc-*.S files that get copied over the current locations if implemented?

Quoted 5 lines

Also problematical with XIP - if we go down the route of implementing these by copying code fragments into the kernel, we need to strip out XIP support or implement a second way. Obviously having a second way adds maintainence burden, and the second way will probably lose out on updates.

How about if we implement the default XIP-safe unoptimized functions, with minimal iffdeffery and then allow optional override for non-XIP kernels from mm/proc-*.S files?

Quoted 5 lines

Also problematical with XIP - if we go down the route of implementing these by copying code fragments into the kernel, we need to strip out XIP support or implement a second way. Obviously having a second way adds maintainence burden, and the second way will probably lose out on updates.

How about if we implement the default XIP-safe unoptimized functions, with minimal iffdeffery and then allow optional override for non-XIP kernels from mm/proc-*.S files?

Quoted 16 lines · Jamie Lokier

Quoted 15 lines · Tony Lindgren

__kuser_get_tls: @ 0xffff0fe0

arch/arm/kernel/entry-armv.S +4 -9

-
-#if !defined(CONFIG_HAS_TLS_REG) && !defined(CONFIG_TLS_REG_EMUL)
-	ldr	r0, [pc, #(16 - 8)]		@ TLS stored at 0xffff0ff0
-#else
-	mrc	p15, 0, r0, c13, c0, 3		@ read TLS register
-#endif
+	ldr     r0, [pc, #(20 - 8)]		@ software TLS set in 0xffff0ff4?
+	cmp	r0, #0				@ hardware TLS if flag not set
+	mrceq	p15, 0, r0, c13, c0, 3		@ read hardware TLS register
+	ldrne	r0, [pc, #(12 - 8)]		@ software TLS val at 0xffff0ff8
 	usr_ret	lr
-
-	.rep	5
-	.word	0			@ pad up to __kuser_helper_version

Quoted 8 lines

arch/arm/kernel/entry-armv.S kuser_cmpxchg_fixup: · line 1021

Quoted 3 lines

arch/arm/kernel/entry-armv.S +2 -1

-	.endr
+	.word	0				@ non-zero for software TLS
+	.word	0				@ software TLS value

It'd be nice not to waste instructions checking for HWCAP_TLS on archs which definitely don't have it. I guess it doesn't matter elsewhere; I'd expect this to be a warm path for some programs making extensive use of TLS (I haven't measured though).

OK, but let's try to figure out a way that does not add more ifdef else code as that makes it harder to build support for multiple ARM cores.

Quoted 3 lines

As it's only a single instruction, and the code is in a writable page already (copied at init), how about just patching the instruction when ELF_HWCAP is set?

Yeah that can be done for __kuser_get_tls if it's always writable. But __switch_to is trickier because of the CONFIG_MMU ifdefs there.

What if we have optional __switch_to and __kuser_get_tls implementations in the mm/proc-*.S files that get copied over the current locations if implemented?

For other things like __switch_to and maybe cache maintenance calls, dmb() etc, I'd suggest first doing a generic asm mechanism like x86's "alternatives", keeping in mind that one of the alternatives has to be XIP friendly.

Quoted 4 lines · Jamie Lokier

For other things like __switch_to and maybe cache maintenance calls, dmb() etc, I'd suggest first doing a generic asm mechanism like x86's "alternatives", keeping in mind that one of the alternatives has to be XIP friendly.

You cannot be XIP friendly unless you rewrite the concerned function(s) into a RAM page in order to modify it. And then of course you have to play tricks with the linker so that the rewritten functions are referenced with their final rewritten location. And that means of course that the RAM location has to be a constant unless we introduce some indirect function calls. And at that point we're not much better than the proposed runtime test.

Also, if you are interested by a XIP kernel, this usually means you have a fairly highly customized kernel config. In this context it is senseless to have runtime patching of the kernel. What you want in that case is as much stuff as possible selected and optimized at compile time, and anything you don't need configured out.

Quoted 29 lines · Tony Lindgren

Quoted 16 lines · Jamie Lokier

Quoted 15 lines · Tony Lindgren

__kuser_get_tls: @ 0xffff0fe0

arch/arm/kernel/entry-armv.S +4 -9

-
-#if !defined(CONFIG_HAS_TLS_REG) && !defined(CONFIG_TLS_REG_EMUL)
-	ldr	r0, [pc, #(16 - 8)]		@ TLS stored at 0xffff0ff0
-#else
-	mrc	p15, 0, r0, c13, c0, 3		@ read TLS register
-#endif
+	ldr     r0, [pc, #(20 - 8)]		@ software TLS set in 0xffff0ff4?
+	cmp	r0, #0				@ hardware TLS if flag not set
+	mrceq	p15, 0, r0, c13, c0, 3		@ read hardware TLS register
+	ldrne	r0, [pc, #(12 - 8)]		@ software TLS val at 0xffff0ff8
 	usr_ret	lr
-
-	.rep	5
-	.word	0			@ pad up to __kuser_helper_version

Quoted 8 lines

arch/arm/kernel/entry-armv.S kuser_cmpxchg_fixup: · line 1021

Quoted 3 lines

arch/arm/kernel/entry-armv.S +2 -1

-	.endr
+	.word	0				@ non-zero for software TLS
+	.word	0				@ software TLS value

It'd be nice not to waste instructions checking for HWCAP_TLS on archs which definitely don't have it. I guess it doesn't matter elsewhere; I'd expect this to be a warm path for some programs making extensive use of TLS (I haven't measured though).

OK, but let's try to figure out a way that does not add more ifdef else code as that makes it harder to build support for multiple ARM cores.

Quoted 6 lines

Quoted 3 lines

As it's only a single instruction, and the code is in a writable page already (copied at init), how about just patching the instruction when ELF_HWCAP is set?

Yeah that can be done for __kuser_get_tls if it's always writable. But __switch_to is trickier because of the CONFIG_MMU ifdefs there.

__kuser_get_tls must be writable in kernels where !HAS_TLS_REG is supported, because the TLS value is written to the same page.

I was thinking of changing *only* __kuser_get_tls, by the way. Out of all the different places, that's the only one I'd expect to be a hot path in some TLS-using programs.

OK. Sorry for the delay again. Here's an updated version that sets __kuser_get_tls instruction dynamically. Does this do what you were thinking, or did I miss something?

Also, can we detect somehow the hardware that uses CONFIG_TLS_REG_EMUL? Might be possible to remove that Kconfig option too later on..

arch/arm/include/asm/hwcap.h +1 -0

index f7bd52b..c1062c3 100644
@@ -19,6 +19,7 @@
 #define HWCAP_NEON	4096
 #define HWCAP_VFPv3	8192
 #define HWCAP_VFPv3D16	16384
+#define HWCAP_TLS	32768
 
 #if defined(__KERNEL__) && !defined(__ASSEMBLY__)
 /*

arch/arm/kernel/entry-armv.S +0 -3

index 7ee48e7..949df9b 100644
@@ -739,11 +739,13 @@ ENTRY(__switch_to)
 #ifdef CONFIG_MMU
 	ldr	r6, [r2, #TI_CPU_DOMAIN]
 #endif
-#if defined(CONFIG_HAS_TLS_REG)
-	mcr	p15, 0, r3, c13, c0, 3		@ set TLS register
-#elif !defined(CONFIG_TLS_REG_EMUL)

arch/arm/kernel/entry-armv.S +1 -6

 #ifdef CONFIG_MMU
 	mcr	p15, 0, r6, c3, c0, 0		@ Set domain register

@@ -1009,16 +1011,13 @@ kuser_cmpxchg_fixup:
  */
 
 __kuser_get_tls:				@ 0xffff0fe0
-
-#if !defined(CONFIG_HAS_TLS_REG) && !defined(CONFIG_TLS_REG_EMUL)
-	ldr	r0, [pc, #(16 - 8)]		@ TLS stored at 0xffff0ff0
-#else
-	mrc	p15, 0, r0, c13, c0, 3		@ read TLS register
-#endif
+	nop				@ read TLS, set in kuser_get_tls_init

arch/arm/kernel/entry-armv.S +0 -0

 /*

arch/arm/kernel/setup.c +4 -0

index 122d999..a675260 100644
@@ -269,6 +269,27 @@ static void __init cacheid_init(void)
 extern struct proc_info_list *lookup_processor_type(unsigned int);
 extern struct machine_desc *lookup_machine_type(unsigned int);
 
+#ifdef CONFIG_CPU_V6
+static void __init feat_v6_fixup(void)
+{
+	int id = read_cpuid_id();

arch/arm/kernel/setup.c +3 -0

+
 static void __init setup_processor(void)
 {
 	struct proc_info_list *list;

@@ -311,6 +332,8 @@ static void __init setup_processor(void)
 	elf_hwcap &= ~HWCAP_THUMB;
 #endif
 
+	feat_v6_fixup();
+
 	cacheid_init();
 	cpu_proc_init();
 }

arch/arm/kernel/traps.c +4 -10

index 1621e53..85dd001 100644
@@ -518,16 +518,19 @@ asmlinkage int arm_syscall(int no, struct pt_regs *regs)
 
 	case NR(set_tls):
 		thread->tp_value = regs->ARM_r0;
-#if defined(CONFIG_HAS_TLS_REG)
-		asm ("mcr p15, 0, %0, c13, c0, 3" : : "r" (regs->ARM_r0) );
-#elif !defined(CONFIG_TLS_REG_EMUL)
-		/*
-		 * User space must never try to access this directly.
-		 * Expect your app to break eventually if you do so.
-		 * The user helper at 0xffff0fe0 must be used instead.
-		 * (see entry-armv.S for details)
-		 */
-		*((unsigned int *)0xffff0ff0) = regs->ARM_r0;
+#if !defined(CONFIG_TLS_REG_EMUL)
+		if (elf_hwcap & HWCAP_TLS) {
+			asm ("mcr p15, 0, %0, c13, c0, 3"
+				: : "r" (regs->ARM_r0));

arch/arm/kernel/traps.c +4 -0

 		return 0;
 

@@ -743,6 +746,21 @@ void __init trap_init(void)
 	return;
 }
 
+#if defined(CONFIG_TLS_REG_EMUL)
+static void __init kuser_get_tls_init(unsigned long vectors)
+{
+	memcpy((void *)vectors + 0xfe0, (void *)vectors + 0xfe8, 4);

Quoted 2 lines · Tony Lindgren

Also, can we detect somehow the hardware that uses CONFIG_TLS_REG_EMUL? Might be possible to remove that Kconfig option too later on..

Well... I don't think we _should_ try to detect it as it is not widely available if at all.

And there is actually a "bug" with __kuser_get_tls because if ever CONFIG_TLS_REG_EMUL is selected then a call to __kuser_get_tls is currently doing nothing while it should actually use the mrc instruction.

Could this be a tristate instead? There are actually 3 cases:

1) We know the TLS reg is _always_ available i.e. ARMv6K, ARMV7 and above. In that case the test on elf_hwcap is redundant and wasteful.

2) We know the TLS reg is _never_ available i.e. pre-ARMv6. The test on elf_hwcap is again redundant and wasteful.

3) We're unsure as the actual CPU is known only at run time.

Case #1 will become the common case in the future. Case #2 is still widely relevant for deployed systems in the field, and some popular ARMv5TE based SOCs are still produced right now. So instead of replacing #1 and #2 with #3, I think you should add #3 to the other cases instead.

Quoted 21 lines

#ifdef CONFIG_MMU

arch/arm/kernel/entry-armv.S +5 -9

 	mcr	p15, 0, r6, c3, c0, 0		@ Set domain register

@@ -1009,16 +1011,13 @@ kuser_cmpxchg_fixup:
  */
 
 __kuser_get_tls:				@ 0xffff0fe0
-
-#if !defined(CONFIG_HAS_TLS_REG) && !defined(CONFIG_TLS_REG_EMUL)
-	ldr	r0, [pc, #(16 - 8)]		@ TLS stored at 0xffff0ff0
-#else
-	mrc	p15, 0, r0, c13, c0, 3		@ read TLS register
-#endif
+	nop				@ read TLS, set in kuser_get_tls_init
 	usr_ret	lr
-
-	.rep	5
-	.word	0			@ pad up to __kuser_helper_version
+	mrc	p15, 0, r0, c13, c0, 3	@ 0xffff0fe8 hardware TLS code
+	ldr	r0, [pc, #(16 - 8)]	@ 0xffff0fec software TLS code
+	.word	0			@ 0xffff0ff0 software TLS value
+	nop				@ pad up to __kuser_helper_version

Review kuser_cmpxchg_fixup: · line 1021

Quoted 2 lines

(quoted diff) +1 -0

+	nop
 	.endr

This looks obscur. The idea of patching the appropriate instruction at runtime here is a good one. However I'd simply keep the ldr version in place otherwise the pc displacement doesn't match anymore when disassembling. And simply overwrite it with the mrc version when necessary.

BTW you left a stray .endr here.

Review kuser_cmpxchg_fixup: · line 1023

Quoted 2 lines

(quoted diff) +2 -0

+	if ((((id >> 4) & 0xfff) == 0xb36) && (((id >> 20) & 3) == 0))
+		elf_hwcap &= ~HWCAP_TLS;

You should probably test for the vendor ID as well (ARM in this case).

arch/arm/kernel/setup.c static void __init cacheid_init(void) · line 288

Quoted 6 lines

arch/arm/kernel/setup.c +6 -0

+}
+#else
+static inline void feat_v6_fixup(void)
+{
+}
+#endif

I think the #ifdef is unnecessary here. This is __init code anyway, so this could as well be always compiled in.

Quoted 28 lines

arch/arm/kernel/traps.c +10 -10

index 1621e53..85dd001 100644
@@ -518,16 +518,19 @@ asmlinkage int arm_syscall(int no, struct pt_regs *regs)
 
 	case NR(set_tls):
 		thread->tp_value = regs->ARM_r0;
-#if defined(CONFIG_HAS_TLS_REG)
-		asm ("mcr p15, 0, %0, c13, c0, 3" : : "r" (regs->ARM_r0) );
-#elif !defined(CONFIG_TLS_REG_EMUL)
-		/*
-		 * User space must never try to access this directly.
-		 * Expect your app to break eventually if you do so.
-		 * The user helper at 0xffff0fe0 must be used instead.
-		 * (see entry-armv.S for details)
-		 */
-		*((unsigned int *)0xffff0ff0) = regs->ARM_r0;
+#if !defined(CONFIG_TLS_REG_EMUL)
+		if (elf_hwcap & HWCAP_TLS) {
+			asm ("mcr p15, 0, %0, c13, c0, 3"
+				: : "r" (regs->ARM_r0));
+		} else {
+			/*
+			 * User space must never try to access this directly.
+			 * Expect your app to break eventually if you do so.
+			 * The user helper at 0xffff0fe0 must be used instead.
+			 * (see entry-armv.S for details)

arch/arm/kernel/traps.c asmlinkage int arm_syscall(int no, struct pt_regs *regs) · line 534

Quoted 4 lines

arch/arm/kernel/traps.c +3 -0

+			 */
+			*((unsigned int *)0xffff0ff0) = regs->ARM_r0;
+		}
 #endif

The same comment as for __kuser_get_tls would apply here. However, instead of duplicating the code block, you could define a macro, such as has_tls_reg(), that would be either 0, 1, or ((elf_hwcap & HWCAP_TLS).

Quoted 15 lines

arch/arm/kernel/traps.c +9 -0

 		return 0;
 

@@ -743,6 +746,21 @@ void __init trap_init(void)
 	return;
 }
 
+#if defined(CONFIG_TLS_REG_EMUL)
+static void __init kuser_get_tls_init(unsigned long vectors)
+{
+	memcpy((void *)vectors + 0xfe0, (void *)vectors + 0xfe8, 4);
+}
+#else
+static void __init kuser_get_tls_init(unsigned long vectors)
+{
+	if (elf_hwcap & HWCAP_TLS)

arch/arm/kernel/traps.c void __init trap_init(void) · line 762

Quoted 5 lines

arch/arm/kernel/traps.c +5 -0

+		memcpy((void *)vectors + 0xfe0, (void *)vectors + 0xfe8, 4);
+	else
+		memcpy((void *)vectors + 0xfe0, (void *)vectors + 0xfec, 4);
+}
+#endif

Please move the #ifdef within the function body. Also it would be nice to add comments about what those magic offsets are.

* Nicolas Pitre <[email protected]> [100629 22:14]:

Updated patch below.

From 248ee34cd03cb4f0040445d0e07831f4227f13e3 Mon Sep 17 00:00:00 2001

Quoted 3 lines · Tony Lindgren

* Nicolas Pitre <[email protected]> [100629 22:14]:

Updated patch below.

And a bug crept in..

Review

Quoted 8 lines

(quoted diff) +8 -0

+	.macro set_tls_v6, tp, tmp1, tmp2
+	ldr	\tmp1, =elf_hwcap
+	ldr	\tmp1, [\tmp1, #0]
+	mov	\tmp2, #0xffff0fff
+	tst	\tmp2, #HWCAP_TLS		@ hardware TLS available?
+	mcrne	p15, 0, \tp, c13, c0, 3		@ yes, set TLS register
+	streq	\tp, [\tmp2, #-15]		@ set TLS value at 0xffff0ff0
+	.endm

The tst \tmp2, #HWCAP_TLS should of course use \tmp1.

Also fixed a warning about adding parentheses around comparison if (id & 0x410f0000) != 0x41070000).

Again, updated patch below.

Quoted 18 lines · Tony Lindgren

Quoted 3 lines · Tony Lindgren

* Nicolas Pitre <[email protected]> [100629 22:14]:

Updated patch below.

And a bug crept in..

Review

Quoted 8 lines

(quoted diff) +8 -0

+	.macro set_tls_v6, tp, tmp1, tmp2
+	ldr	\tmp1, =elf_hwcap
+	ldr	\tmp1, [\tmp1, #0]
+	mov	\tmp2, #0xffff0fff
+	tst	\tmp2, #HWCAP_TLS		@ hardware TLS available?
+	mcrne	p15, 0, \tp, c13, c0, 3		@ yes, set TLS register
+	streq	\tp, [\tmp2, #-15]		@ set TLS value at 0xffff0ff0
+	.endm

The tst \tmp2, #HWCAP_TLS should of course use \tmp1.

Quoted 2 lines

Also fixed a warning about adding parentheses around comparison if (id & 0x410f0000) != 0x41070000).

Here you probably want (id & 0xff0f0000) and not (id & 0x410f0000).

Quoted 1 line

Again, updated patch below.

I like it. However, in proc-v6.S, you don't need to add a reference to feat_v6_fixup() to the __pj4_v6_proc_info block. Simply adding HWCAP_TLS in that case should be fine as PJ4 always has the TLS reg.

With the above fixes, you can add

Reviewed-by Nicolas Pitre <[email protected]>

Review

Quoted 9 lines · Tony Lindgren

(quoted diff) +9 -0

+static void __init kuser_get_tls_init(unsigned long vectors)
+{
+	/*
+	 * vectors + 0xfe0 = __kuser_get_tls
+	 * vectors + 0xfe8 = hardware TLS instruction at 0xffff0fe8
+	 */
+	if (tls_emu || has_tls)
+		memcpy((void *)vectors + 0xfe0, (void *)vectors + 0xfe8, 4);
+}

Just a little opinion: Perhaps has_tls_reg would be a clearer name. All variants "have TLS" after all.

Also - and this isn't directly related to your change so feel free to ignore it - wouldn't it make more sense for the tls_emu case to use the memory version (and update the memory location), so that even on tls_emu systems, user programs which call the kuser code will run faster? With that change, there would be no real penalty to enabling tls_emu for any system that finds it useful.

Quoted 13 lines · Jamie Lokier

Review

Quoted 9 lines · Tony Lindgren

(quoted diff) +9 -0

+static void __init kuser_get_tls_init(unsigned long vectors)
+{
+	/*
+	 * vectors + 0xfe0 = __kuser_get_tls
+	 * vectors + 0xfe8 = hardware TLS instruction at 0xffff0fe8
+	 */
+	if (tls_emu || has_tls)
+		memcpy((void *)vectors + 0xfe0, (void *)vectors + 0xfe8, 4);
+}

Just a little opinion: Perhaps has_tls_reg would be a clearer name. All variants "have TLS" after all.

Good point.

Quoted 16 lines · Nicolas Pitre

Quoted 13 lines · Jamie Lokier

Review

Quoted 9 lines · Tony Lindgren

(quoted diff) +9 -0

+static void __init kuser_get_tls_init(unsigned long vectors)
+{
+	/*
+	 * vectors + 0xfe0 = __kuser_get_tls
+	 * vectors + 0xfe8 = hardware TLS instruction at 0xffff0fe8
+	 */
+	if (tls_emu || has_tls)
+		memcpy((void *)vectors + 0xfe0, (void *)vectors + 0xfe8, 4);
+}

Just a little opinion: Perhaps has_tls_reg would be a clearer name. All variants "have TLS" after all.

Quoted 1 line

Good point.

I like that too. Updated patch below.

Quoted 17 lines · Tony Lindgren

Quoted 16 lines · Nicolas Pitre

Quoted 13 lines · Jamie Lokier

Review

Quoted 9 lines · Tony Lindgren

(quoted diff) +9 -0

+static void __init kuser_get_tls_init(unsigned long vectors)
+{
+ ? ? ? /*
+ ? ? ? ?* vectors + 0xfe0 = __kuser_get_tls
+ ? ? ? ?* vectors + 0xfe8 = hardware TLS instruction at 0xffff0fe8
+ ? ? ? ?*/
+ ? ? ? if (tls_emu || has_tls)
+ ? ? ? ? ? ? ? memcpy((void *)vectors + 0xfe0, (void *)vectors + 0xfe8, 4);
+}

Just a little opinion: Perhaps has_tls_reg would be a clearer name. All variants "have TLS" after all.

Quoted 3 lines

Quoted 1 line

Good point.

I like that too. Updated patch below.

I've uploaded this patch into Russell's patch system:

http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6207/1

Jamie, assuming no more comments, do you care to Ack/Reviewed/Tested-by the patch in the link above?

Regards,

Tony

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Dears,

I cannot understand how TLS EMU ensure it's SMP safe, because get_tls helper (at 0xffff0fe0) just read the value from 0xffff0ff0. But all SMP cores should have the exact same mapping to the vectors page (at 0xffff0000). So various threads running on different SMP cores at the same time would get the same emulated TLS value (instead of thread specific value set via set_tls). Could anybody explain this a little bit?

Quoted 9 lines · Li Li

Dears,

I cannot understand how TLS EMU ensure it's SMP safe, because get_tls helper (at 0xffff0fe0) just read the value from 0xffff0ff0. But all SMP cores should have the exact same mapping to the vectors page (at 0xffff0000). So various threads running on different SMP cores at the same time would get the same emulated TLS value (instead of thread specific value set via set_tls). Could anybody explain this a little bit?

On SMP the hardware TLS register is always available, therefore the TLS value is not stored at 0xffff0ff0. ?The code actually installed at 0xffff0fe0 is modified at boot time by kuser_get_tls_init to select either the ldr or the mrc instruction to retrieve the TLS value.

Nicolas

Nicolas,

Thanks for the explanation. :)

I noticed what you mentioned. Actually, Kconfig says TLS EMU might be used in some "old" (e.g. pre-armv6) SMP platforms without TLS register. In such a case, could we still ensure it's SMP safe by a single ldr?

Quoted 8 lines · Li Li

Nicolas,

Thanks for the explanation. :)

I noticed what you mentioned. Actually, Kconfig says TLS EMU might be used in some "old" (e.g. pre-armv6) SMP platforms without TLS register. In such a case, could we still ensure it's SMP safe by a single ldr?

In that case, the hardware TLS 'mcr' is used, which traps and is emulated by the undefined instruction handler. ?It's not fast but presumably those platforms don't really matter.

See CONFIG_TLS_REG_EMUL in arch/arm/kernel/traps.c, and 'tls_emu'.

'tls_emu' is a constant, so if a kernel built for TLS emulation is run on something which has the TLS register, it will not work properly.

-- JAmie

arch/arm/vfp/vfpmodule.c +0 -1

index 315a540..19ba626 100644
@@ -549,10 +549,14 @@ static int __init vfp_init(void)
                /*
                 * Check for the presence of the Advanced SIMD
                 * load/store instructions, integer and single
-                * precision floating point operations.

Quoted 9 lines · Tony Lindgren

arch/arm/vfp/vfpmodule.c +0 -1

index 315a540..19ba626 100644
@@ -549,10 +549,14 @@ static int __init vfp_init(void)
                /*
                 * Check for the presence of the Advanced SIMD
                 * load/store instructions, integer and single
-                * precision floating point operations.

arch/arm/vfp/vfpmodule.c static int __init vfp_init(void) · line 559

Quoted 10 lines

arch/arm/vfp/vfpmodule.c +7 -2

+                * precision floating point operations. Only check
+                * for NEON if the hardware has TLS register as the
+                * MVFR registers got added in ARM1136 r1p0 with TLS.
                 */
-               if ((fmrx(MVFR1) & 0x000fff00) == 0x00011100)
-                       elf_hwcap |= HWCAP_NEON;
+               if (elf_hwcap & HWCAP_TLS) {
+                       if ((fmrx(MVFR1) & 0x000fff00) == 0x00011100)
+                               elf_hwcap |= HWCAP_NEON;
+               }

MVFR should be available with the new CPU id format:

	((read_cpuid_id() & 0x000f0000) == 0x000f0000)

I think that would be a cleaner test than relying on the TLS presence.

Quoted 23 lines · Catalin Marinas

Quoted 9 lines · Tony Lindgren

arch/arm/vfp/vfpmodule.c +0 -1

index 315a540..19ba626 100644
@@ -549,10 +549,14 @@ static int __init vfp_init(void)
                /*
                 * Check for the presence of the Advanced SIMD
                 * load/store instructions, integer and single
-                * precision floating point operations.

arch/arm/vfp/vfpmodule.c static int __init vfp_init(void) · line 559

Quoted 10 lines

arch/arm/vfp/vfpmodule.c +7 -2

+                * precision floating point operations. Only check
+                * for NEON if the hardware has TLS register as the
+                * MVFR registers got added in ARM1136 r1p0 with TLS.
                 */
-               if ((fmrx(MVFR1) & 0x000fff00) == 0x00011100)
-                       elf_hwcap |= HWCAP_NEON;
+               if (elf_hwcap & HWCAP_TLS) {
+                       if ((fmrx(MVFR1) & 0x000fff00) == 0x00011100)
+                               elf_hwcap |= HWCAP_NEON;
+               }

MVFR should be available with the new CPU id format:

Quoted 3 lines

	((read_cpuid_id() & 0x000f0000) == 0x000f0000)

I think that would be a cleaner test than relying on the TLS presence.

OK, thanks that's better. Will update accordingly and repost.

MVFR0 and MVFR1 are only available starting with ARM1136 r1p0 release according to "B.5 VFP changes" in DDI0211F_arm1136_r1p0_trm.pdf. This is also when TLS register got added, so we can use HAS_TLS also to test for MVFR0 and MVFR1.

Otherwise VFPFMRX and VFPFMXR access fails and we get:

Internal error: Oops - undefined instruction: 0 [#1]
PC is at no_old_VFP_process+0x8/0x3c
LR is at __und_svc+0x48/0x80

...

Signed-off-by Tony Lindgren <[email protected]>

Quoted 12 lines · Tony Lindgren

MVFR0 and MVFR1 are only available starting with ARM1136 r1p0 release according to "B.5 VFP changes" in DDI0211F_arm1136_r1p0_trm.pdf. This is also when TLS register got added, so we can use HAS_TLS also to test for MVFR0 and MVFR1.

Otherwise VFPFMRX and VFPFMXR access fails and we get:

Internal error: Oops - undefined instruction: 0 [#1]
PC is at no_old_VFP_process+0x8/0x3c
LR is at __und_svc+0x48/0x80

...

Quoted 1 line

Signed-off-by Tony Lindgren <[email protected]>

The new version looks fine to me.

Acked-by Catalin Marinas <[email protected]>