arm: Preserve TPIDRURW on context switch

PATCH Superseded André Hentschel 4 participants 11 messages Feb 6, 2013 – Feb 21, 2013

Subsystems (3)

Arm (32-bit) Generic include/asm header files ptrace support

lore

Share & tools

lore

André Hentschel

author

2013-02-06 22:43 UTC #0 lore raw

PATCH 1/1 arm: Preserve TPIDRURW on context switch

There are more and more applications coming to WinRT, Wine could support them, but mostly they expect to have the thread environment block (TEB) in TPIDRURW. This register must be preserved per thread instead of being cleared.

This patch is against 0944c0a03465718909ba8e800a5230528aeabafb

I could only test it with kernel 3.4.6

Signed-off-by Andr? Hentschel <[email protected]>

From: =?UTF-8?q?Andr=C3=A9=20Hentschel?= <[email protected]>
Date: Wed, 6 Feb 2013 23:24:54 +0100
Subject: [PATCH] arm: Preserve TPIDRURW on context switch

---
 arch/arm/include/asm/thread_info.h |  1 +
 arch/arm/include/asm/tls.h         | 43 ++++++++++++++++++++++++++++++++++----
 arch/arm/kernel/asm-offsets.c      |  1 +
 arch/arm/kernel/entry-armv.S       |  7 +++++++
 4 files changed, 48 insertions(+), 4 deletions(-)

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

index cddda1f..948525c 100644
@@ -59,6 +59,7 @@ struct thread_info {
 	__u32			syscall;	/* syscall number */
 	__u8			used_cp[16];	/* thread used copro */
 	unsigned long		tp_value;
+	unsigned long		tp2_value;	/* userspace read/write thread id register (2) */
 #ifdef CONFIG_CRUNCH
 	struct crunch_state	crunchstate;
 #endif

arch/arm/include/asm/tls.h +39 -4

79 lines not on this page · raw message

arch/arm/kernel/asm-offsets.c +1 -0

index c985b48..2563f29 100644
@@ -58,6 +58,7 @@ int main(void)
   DEFINE(TI_CPU_SAVE,		offsetof(struct thread_info, cpu_context));
   DEFINE(TI_USED_CP,		offsetof(struct thread_info, used_cp));
   DEFINE(TI_TP_VALUE,		offsetof(struct thread_info, tp_value));
+  DEFINE(TI_TP2_VALUE,		offsetof(struct thread_info, tp2_value));
   DEFINE(TI_FPSTATE,		offsetof(struct thread_info, fpstate));
 #ifdef CONFIG_VFP
   DEFINE(TI_VFPSTATE,		offsetof(struct thread_info, vfpstate));

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

index 0f82098..39de843 100644
@@ -737,6 +737,13 @@ ENTRY(__switch_to)
 	ldr	r6, [r2, #TI_CPU_DOMAIN]
 #endif
 	set_tls	r3, r4, r5
+
+	@ preserve TPIDRURW register state
+	get_tls2	r3, r4, r5
+	str	r3, [r1, #TI_TP2_VALUE]
+	ldr	r3, [r2, #TI_TP2_VALUE]
+	set_tls2	r3, r4, r5
+
 #if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
 	ldr	r7, [r2, #TI_TASK]
 	ldr	r8, =__stack_chk_guard

Signature · 3 lines

1.8.0

Best Regards, Andr? Hentschel

Russell King - ARM Linux

2013-02-06 22:51 UTC #1 lore raw

Quoted 3 lines · Andr? Hentschel

There are more and more applications coming to WinRT, Wine could support them, but mostly they expect to have the thread environment block (TEB) in TPIDRURW. This register must be preserved per thread instead of being cleared.

I'd prefer this was done a little more sensitively to those CPUs where loads/stores are expensive, namely:

arch/arm/kernel/entry-armv.S ENTRY(__switch_to) · line 745

Quoted 6 lines

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

+
+	@ preserve TPIDRURW register state
+	get_tls2	r3, r4, r5
+	str	r3, [r1, #TI_TP2_VALUE]
+	ldr	r3, [r2, #TI_TP2_VALUE]
+	set_tls2	r3, r4, r5

those two loads/stores get omitted from the thread switching if the CPU doesn't support it. Do you think that's something you could do?

André Hentschel

author

2013-02-06 23:01 UTC #2 lore raw

Am 06.02.2013 23:51, schrieb Russell King - ARM Linux:

Quoted 8 lines · André Hentschel

Quoted 3 lines
There are more and more applications coming to WinRT, Wine could support them, but mostly they expect to have the thread environment block (TEB) in TPIDRURW. This register must be preserved per thread instead of being cleared.
I'd prefer this was done a little more sensitively to those CPUs where loads/stores are expensive, namely:

Quoted 9 lines

arch/arm/kernel/entry-armv.S ENTRY(__switch_to) · line 745
Quoted 6 lines
arch/arm/kernel/entry-armv.S +6 -0
+
+	@ preserve TPIDRURW register state
+	get_tls2	r3, r4, r5
+	str	r3, [r1, #TI_TP2_VALUE]
+	ldr	r3, [r2, #TI_TP2_VALUE]
+	set_tls2	r3, r4, r5
those two loads/stores get omitted from the thread switching if the CPU doesn't support it. Do you think that's something you could do?

No, i'm not sure how to improve this. How does the process can continue, can you or someone else fix that and add his Signed-off-by?

Signature · 1 line

Best Regards, André Hentschel

Will Deacon

2013-02-08 15:48 UTC #3 lore raw

Quoted 19 lines · Andr? Hentschel

Am 06.02.2013 23:51, schrieb Russell King - ARM Linux:
Quoted 8 lines · Andr? Hentschel
Quoted 3 lines
There are more and more applications coming to WinRT, Wine could support them, but mostly they expect to have the thread environment block (TEB) in TPIDRURW. This register must be preserved per thread instead of being cleared.
I'd prefer this was done a little more sensitively to those CPUs where loads/stores are expensive, namely:
Quoted 9 lines
Review
Quoted 6 lines
(quoted diff) +6 -0
+
+	@ preserve TPIDRURW register state
+	get_tls2	r3, r4, r5
+	str	r3, [r1, #TI_TP2_VALUE]
+	ldr	r3, [r2, #TI_TP2_VALUE]
+	set_tls2	r3, r4, r5
those two loads/stores get omitted from the thread switching if the CPU doesn't support it. Do you think that's something you could do?

Quoted 1 line

No, i'm not sure how to improve this. How does the process can continue, can you or someone else fix that and add his Signed-off-by?

How about something like the (completely untested) diff below?

Andre: if this works for you, I'm happy to write a commit message etc.

Cheers,

Will

--->8

arch/arm/include/asm/thread_info.h +1 -1

index cddda1f..d90be6d 100644
@@ -58,7 +58,7 @@ struct thread_info {
 	struct cpu_context_save	cpu_context;	/* cpu context */
 	__u32			syscall;	/* syscall number */
 	__u8			used_cp[16];	/* thread used copro */
-	unsigned long		tp_value;
+	unsigned long		tp_value[2];	/* TLS registers */
 #ifdef CONFIG_CRUNCH
 	struct crunch_state	crunchstate;
 #endif

arch/arm/include/asm/tls.h +11 -9

index 73409e6..e1b09d32 100644
@@ -6,9 +6,9 @@
 	.endm
 
 	.macro set_tls_v6k, tp, tmp1, tmp2
-	mcr	p15, 0, \tp, c13, c0, 3		@ set TLS register
-	mov	\tmp1, #0
-	mcr	p15, 0, \tmp1, c13, c0, 2	@ clear user r/w TLS register
+	ldrd	\tmp1, \tmp2, [\tp]
+	mcr	p15, 0, \tmp1, c13, c0, 3	@ set user r/o TLS register
+	mcr	p15, 0, \tmp2, c13, c0, 2	@ set user r/w TLS register
 	.endm
 
 	.macro set_tls_v6, tp, tmp1, tmp2

@@ -16,15 +16,17 @@
 	ldr	\tmp1, [\tmp1, #0]
 	mov	\tmp2, #0xffff0fff
 	tst	\tmp1, #HWCAP_TLS		@ hardware TLS available?
-	mcrne	p15, 0, \tp, c13, c0, 3		@ yes, set TLS register
-	movne	\tmp1, #0
-	mcrne	p15, 0, \tmp1, c13, c0, 2	@ clear user r/w TLS register
-	streq	\tp, [\tmp2, #-15]		@ set TLS value at 0xffff0ff0
+	ldrdne	\tmp1, \tmp2, [\tp]
+	ldreq	\tmp1, [\tp]
+	mcrne	p15, 0, \tmp1, c13, c0, 3	@ yes, set user r/o TLS register
+	mcrne	p15, 0, \tmp2, c13, c0, 2	@ set user r/w TLS register
+	streq	\tmp1, [\tmp2, #-15]		@ set TLS value at 0xffff0ff0
 	.endm
 
 	.macro set_tls_software, tp, tmp1, tmp2
-	mov	\tmp1, #0xffff0fff
-	str	\tp, [\tmp1, #-15]		@ set TLS value at 0xffff0ff0
+	ldr	\tmp1, [\tp]
+	mov	\tmp2, #0xffff0fff
+	str	\tmp1, [\tmp2, #-15]		@ set TLS value at 0xffff0ff0
 	.endm
 #endif

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

index 0f82098..a0a8fe3 100644
@@ -728,7 +728,7 @@ ENTRY(__switch_to)
  UNWIND(.fnstart	)
  UNWIND(.cantunwind	)
 	add	ip, r1, #TI_CPU_SAVE
-	ldr	r3, [r2, #TI_TP_VALUE]
+	add	r3, r2, #TI_TP_VALUE
  ARM(	stmia	ip!, {r4 - sl, fp, sp, lr} )	@ Store most regs on stack
  THUMB(	stmia	ip!, {r4 - sl, fp}	   )	@ Store most regs on stack
  THUMB(	str	sp, [ip], #4		   )

arch/arm/kernel/process.c +1 -1

index c6dec5f..ea298d2 100644
@@ -400,7 +400,7 @@ copy_thread(unsigned long clone_flags, unsigned long stack_start,
 	clear_ptrace_hw_breakpoint(p);
 
 	if (clone_flags & CLONE_SETTLS)
-		thread->tp_value = childregs->ARM_r3;
+		thread->tp_value[0] = childregs->ARM_r3;
 
 	thread_notify(THREAD_NOTIFY_COPY, thread);

arch/arm/kernel/ptrace.c +1 -1

index 03deeff..2bc1514 100644
@@ -849,7 +849,7 @@ long arch_ptrace(struct task_struct *child, long request,
 #endif
 
 		case PTRACE_GET_THREAD_AREA:
-			ret = put_user(task_thread_info(child)->tp_value,
+			ret = put_user(task_thread_info(child)->tp_value[0],
 				       datap);
 			break;

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

index b0179b8..08b0db9 100644
@@ -588,7 +588,7 @@ asmlinkage int arm_syscall(int no, struct pt_regs *regs)
 		return regs->ARM_r0;
 
 	case NR(set_tls):
-		thread->tp_value = regs->ARM_r0;
+		thread->tp_value[0] = regs->ARM_r0;
 		if (tls_emu)
 			return 0;
 		if (has_tls_reg) {

@@ -706,7 +706,7 @@ static int get_tp_trap(struct pt_regs *regs, unsigned int instr)
 	int reg = (instr >> 12) & 15;
 	if (reg == 15)
 		return 1;
-	regs->uregs[reg] = current_thread_info()->tp_value;
+	regs->uregs[reg] = current_thread_info()->tp_value[0];
 	regs->ARM_pc += 4;
 	return 0;
 }

André Hentschel

author

2013-02-09 16:44 UTC #4 lore raw

Am 08.02.2013 16:48, schrieb Will Deacon:

Quoted 24 lines · Andr? Hentschel

Quoted 19 lines
Am 06.02.2013 23:51, schrieb Russell King - ARM Linux:
Quoted 8 lines · Andr? Hentschel
Quoted 3 lines
There are more and more applications coming to WinRT, Wine could support them, but mostly they expect to have the thread environment block (TEB) in TPIDRURW. This register must be preserved per thread instead of being cleared.
I'd prefer this was done a little more sensitively to those CPUs where loads/stores are expensive, namely:
Quoted 9 lines
Review
Quoted 6 lines
(quoted diff) +6 -0
+
+	@ preserve TPIDRURW register state
+	get_tls2	r3, r4, r5
+	str	r3, [r1, #TI_TP2_VALUE]
+	ldr	r3, [r2, #TI_TP2_VALUE]
+	set_tls2	r3, r4, r5
those two loads/stores get omitted from the thread switching if the CPU doesn't support it. Do you think that's something you could do?
Quoted 1 line
No, i'm not sure how to improve this. How does the process can continue, can you or someone else fix that and add his Signed-off-by?
How about something like the (completely untested) diff below?

Quoted 7 lines

Andre: if this works for you, I'm happy to write a commit message etc.
Cheers,
Will
--->8

I'll try the next days and report back, thx.

Signature · 1 line

Best Regards, Andr? Hentschel

André Hentschel

author

2013-02-12 14:02 UTC #5 lore raw

In reply to Will Deacon

Am 08.02.2013 16:48, schrieb Will Deacon:

Quoted 22 lines · Andr? Hentschel

Quoted 19 lines
Am 06.02.2013 23:51, schrieb Russell King - ARM Linux:
Quoted 8 lines · Andr? Hentschel
Quoted 3 lines
There are more and more applications coming to WinRT, Wine could support them, but mostly they expect to have the thread environment block (TEB) in TPIDRURW. This register must be preserved per thread instead of being cleared.
I'd prefer this was done a little more sensitively to those CPUs where loads/stores are expensive, namely:
Quoted 9 lines
Review
Quoted 6 lines
(quoted diff) +6 -0
+
+	@ preserve TPIDRURW register state
+	get_tls2	r3, r4, r5
+	str	r3, [r1, #TI_TP2_VALUE]
+	ldr	r3, [r2, #TI_TP2_VALUE]
+	set_tls2	r3, r4, r5
those two loads/stores get omitted from the thread switching if the CPU doesn't support it. Do you think that's something you could do?
Quoted 1 line
No, i'm not sure how to improve this. How does the process can continue, can you or someone else fix that and add his Signed-off-by?

Quoted 7 lines

How about something like the (completely untested) diff below?
Andre: if this works for you, I'm happy to write a commit message etc.
Cheers,
Will

Thx for your effort, but it doesn't work, at least not with my 3.4.6 suse kernel. Be free to have another try, i'll be happy to test it.

Quoted 118 lines

--->8

arch/arm/include/asm/thread_info.h +1 -1

index cddda1f..d90be6d 100644
@@ -58,7 +58,7 @@ struct thread_info {
 	struct cpu_context_save	cpu_context;	/* cpu context */
 	__u32			syscall;	/* syscall number */
 	__u8			used_cp[16];	/* thread used copro */
-	unsigned long		tp_value;
+	unsigned long		tp_value[2];	/* TLS registers */
 #ifdef CONFIG_CRUNCH
 	struct crunch_state	crunchstate;
 #endif

arch/arm/include/asm/tls.h +11 -9

index 73409e6..e1b09d32 100644
@@ -6,9 +6,9 @@
 	.endm
 
 	.macro set_tls_v6k, tp, tmp1, tmp2
-	mcr	p15, 0, \tp, c13, c0, 3		@ set TLS register
-	mov	\tmp1, #0
-	mcr	p15, 0, \tmp1, c13, c0, 2	@ clear user r/w TLS register
+	ldrd	\tmp1, \tmp2, [\tp]
+	mcr	p15, 0, \tmp1, c13, c0, 3	@ set user r/o TLS register
+	mcr	p15, 0, \tmp2, c13, c0, 2	@ set user r/w TLS register
 	.endm
 
 	.macro set_tls_v6, tp, tmp1, tmp2

@@ -16,15 +16,17 @@
 	ldr	\tmp1, [\tmp1, #0]
 	mov	\tmp2, #0xffff0fff
 	tst	\tmp1, #HWCAP_TLS		@ hardware TLS available?
-	mcrne	p15, 0, \tp, c13, c0, 3		@ yes, set TLS register
-	movne	\tmp1, #0
-	mcrne	p15, 0, \tmp1, c13, c0, 2	@ clear user r/w TLS register
-	streq	\tp, [\tmp2, #-15]		@ set TLS value at 0xffff0ff0
+	ldrdne	\tmp1, \tmp2, [\tp]
+	ldreq	\tmp1, [\tp]
+	mcrne	p15, 0, \tmp1, c13, c0, 3	@ yes, set user r/o TLS register
+	mcrne	p15, 0, \tmp2, c13, c0, 2	@ set user r/w TLS register
+	streq	\tmp1, [\tmp2, #-15]		@ set TLS value at 0xffff0ff0
 	.endm
 
 	.macro set_tls_software, tp, tmp1, tmp2
-	mov	\tmp1, #0xffff0fff
-	str	\tp, [\tmp1, #-15]		@ set TLS value at 0xffff0ff0
+	ldr	\tmp1, [\tp]
+	mov	\tmp2, #0xffff0fff
+	str	\tmp1, [\tmp2, #-15]		@ set TLS value at 0xffff0ff0
 	.endm
 #endif

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

index 0f82098..a0a8fe3 100644
@@ -728,7 +728,7 @@ ENTRY(__switch_to)
  UNWIND(.fnstart	)
  UNWIND(.cantunwind	)
 	add	ip, r1, #TI_CPU_SAVE
-	ldr	r3, [r2, #TI_TP_VALUE]
+	add	r3, r2, #TI_TP_VALUE
  ARM(	stmia	ip!, {r4 - sl, fp, sp, lr} )	@ Store most regs on stack
  THUMB(	stmia	ip!, {r4 - sl, fp}	   )	@ Store most regs on stack
  THUMB(	str	sp, [ip], #4		   )

arch/arm/kernel/process.c +1 -1

index c6dec5f..ea298d2 100644
@@ -400,7 +400,7 @@ copy_thread(unsigned long clone_flags, unsigned long stack_start,
 	clear_ptrace_hw_breakpoint(p);
 
 	if (clone_flags & CLONE_SETTLS)
-		thread->tp_value = childregs->ARM_r3;
+		thread->tp_value[0] = childregs->ARM_r3;
 
 	thread_notify(THREAD_NOTIFY_COPY, thread);

arch/arm/kernel/ptrace.c +1 -1

index 03deeff..2bc1514 100644
@@ -849,7 +849,7 @@ long arch_ptrace(struct task_struct *child, long request,
 #endif
 
 		case PTRACE_GET_THREAD_AREA:
-			ret = put_user(task_thread_info(child)->tp_value,
+			ret = put_user(task_thread_info(child)->tp_value[0],
 				       datap);
 			break;

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

index b0179b8..08b0db9 100644
@@ -588,7 +588,7 @@ asmlinkage int arm_syscall(int no, struct pt_regs *regs)
 		return regs->ARM_r0;
 
 	case NR(set_tls):
-		thread->tp_value = regs->ARM_r0;
+		thread->tp_value[0] = regs->ARM_r0;
 		if (tls_emu)
 			return 0;
 		if (has_tls_reg) {

@@ -706,7 +706,7 @@ static int get_tp_trap(struct pt_regs *regs, unsigned int instr)
 	int reg = (instr >> 12) & 15;
 	if (reg == 15)
 		return 1;
-	regs->uregs[reg] = current_thread_info()->tp_value;
+	regs->uregs[reg] = current_thread_info()->tp_value[0];
 	regs->ARM_pc += 4;
 	return 0;
 }

Signature · 1 line

Best Regards, Andr? Hentschel

Will Deacon

2013-02-12 14:09 UTC #6 lore raw

Hi Andre,

Quoted 10 lines · Andr? Hentschel

Am 08.02.2013 16:48, schrieb Will Deacon:
Quoted 6 lines · Andr? Hentschel
Quoted 1 line
No, i'm not sure how to improve this. How does the process can continue, can you or someone else fix that and add his Signed-off-by?
How about something like the (completely untested) diff below?
Andre: if this works for you, I'm happy to write a commit message etc.
Thx for your effort, but it doesn't work, at least not with my 3.4.6 suse kernel. Be free to have another try, i'll be happy to test it.

Well, cheers for testing. Can you be more specific about what goes wrong please? What do you see in TPIDRURW?

Will

André Hentschel

author

2013-02-12 14:14 UTC #7 lore raw

Am 12.02.2013 15:09, schrieb Will Deacon:

Quoted 14 lines

Hi Andre,
Quoted 10 lines · Andr? Hentschel
Am 08.02.2013 16:48, schrieb Will Deacon:
Quoted 6 lines · Andr? Hentschel
Quoted 1 line
No, i'm not sure how to improve this. How does the process can continue, can you or someone else fix that and add his Signed-off-by?
How about something like the (completely untested) diff below?
Andre: if this works for you, I'm happy to write a commit message etc.
Thx for your effort, but it doesn't work, at least not with my 3.4.6 suse kernel. Be free to have another try, i'll be happy to test it.

Quoted 9 lines

Well, cheers for testing. Can you be more specific about what goes wrong please? What do you see in TPIDRURW?
Will
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi Will,

it's 0x00000000 again.

Signature · 1 line

Best Regards, Andr? Hentschel

Matthieu CASTET

2013-02-12 15:02 UTC #8 lore raw

In reply to Will Deacon

Will Deacon a ?crit :

Quoted 6 lines

Hi Andre,
Quoted 3 lines · Andr? Hentschel
Am 08.02.2013 16:48, schrieb Will Deacon:
Quoted 2 lines · Andr? Hentschel
Quoted 1 line
No, i'm not sure how to improve this. How does the process can continue, can you or someone else fix that and add his Signed-off-by?

Quoted 9 lines

Quoted 5 lines
Quoted 3 lines
How about something like the (completely untested) diff below?
Andre: if this works for you, I'm happy to write a commit message etc.
Thx for your effort, but it doesn't work, at least not with my 3.4.6 suse kernel. Be free to have another try, i'll be happy to test it.
Well, cheers for testing. Can you be more specific about what goes wrong please? What do you see in TPIDRURW?

The tls value is never saved to : thread->tp_value[1].

Also I don't know if you can avoid the extra ldr in the software case.

Matthieu

André Hentschel

author

2013-02-20 19:34 UTC #9 lore raw

Am 12.02.2013 16:02, schrieb Matthieu CASTET:

Quoted 18 lines

Will Deacon a ?crit :
Quoted 6 lines
Hi Andre,
Quoted 3 lines · Andr? Hentschel
Am 08.02.2013 16:48, schrieb Will Deacon:
Quoted 2 lines · Andr? Hentschel
Quoted 1 line
No, i'm not sure how to improve this. How does the process can continue, can you or someone else fix that and add his Signed-off-by?
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
How about something like the (completely untested) diff below?
Andre: if this works for you, I'm happy to write a commit message etc.
Thx for your effort, but it doesn't work, at least not with my 3.4.6 suse kernel. Be free to have another try, i'll be happy to test it.
Well, cheers for testing. Can you be more specific about what goes wrong please? What do you see in TPIDRURW?
The tls value is never saved to : thread->tp_value[1].

Quoted 8 lines

Also I don't know if you can avoid the extra ldr in the software case.
Matthieu
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi Will,
seems like Matthieu is right, any update on this?

Signature · 1 line

Best Regards, Andr? Hentschel

Will Deacon

2013-02-21 10:33 UTC #10 lore raw

Hi Andre,

Quoted 6 lines · André Hentschel

Am 12.02.2013 16:02, schrieb Matthieu CASTET:
Quoted 3 lines
The tls value is never saved to : thread->tp_value[1].
Also I don't know if you can avoid the extra ldr in the software case.
seems like Matthieu is right, any update on this?

Yeah, I'll get round to this after the merge window and send a v2. I have a nice long flight to Hong Kong on the horizon, which may well drive me to writing patches :)

Cheers,

Will