[RFC 0/3] Enable MSR_TM lazily

[RFC 0/3] Enable MSR_TM lazily

[Cyril Bur]

Currently the kernel checks to see if the hardware is transactional
memory capable and always enables the MSR_TM bit. The problem with
this is that the TM related SPRs become available to userspace,
requiring them to be switched between processes. It turns out these
SPRs are expensive to read and write and if a thread doesn't use TM
(or worse yet isn't even TM aware) then context switching incurs this
penalty for nothing.

The solution here is to leave the MSR_TM bit disabled and enable it
more 'on demand'. Leaving MSR_TM disabled cause a thread to take a
facility unavailable fault if and when it does decide to use TM. As
with recent updates to the FPU, VMX and VSX units the MSR_TM bit will
be enabled upon taking the fault and left on for some time afterwards
as the assumption is that if a thread used TM ones it may well use it
again. The kernel will turn the MSR_TM bit off after some number of
context switches of that thread.

Performance numbers haven't been completely gathered as yet but early
runs of tools/testing/selftests/powerpc/benchmarks/context_switch
(which doesn't use TM) yields a jump from ~160000 switches per second
to ~180000 switches per second with patch 3/3 applied.

These patches will need to be applied on top of my recent rework of
TM: http://patchwork.ozlabs.org/patch/631959/
I have pushed a branch to github to help with reviews:
https://github.com/cyrilbur-ibm/linux/tree/tm_lazy

Cyril Bur (3):
  selftests/powerpc: Add test to check TM ucontext creation
  powerpc: tm: Add TM Unavailable Exception
  powerpc: tm: Enable transactional memory (TM) lazily for userspace

 arch/powerpc/include/asm/processor.h               |   1 +
 arch/powerpc/kernel/process.c                      |  30 ++++--
 arch/powerpc/kernel/traps.c                        |  33 +++++++
 .../selftests/powerpc/tm/tm-signal-context-chk.c   | 102 +++++++++++++++++++++
 4 files changed, 158 insertions(+), 8 deletions(-)
 create mode 100644 tools/testing/selftests/powerpc/tm/tm-signal-context-chk.c

-- 
2.9.0

[RFC 1/3] selftests/powerpc: Add test to check TM ucontext creation

[Cyril Bur]

Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
---
 .../selftests/powerpc/tm/tm-signal-context-chk.c   | 102 +++++++++++++++++++++
 1 file changed, 102 insertions(+)
 create mode 100644 tools/testing/selftests/powerpc/tm/tm-signal-context-chk.c

diff --git a/tools/testing/selftests/powerpc/tm/tm-signal-context-chk.c b/tools/testing/selftests/powerpc/tm/tm-signal-context-chk.c
new file mode 100644
index 0000000..4c906cf
--- /dev/null
+++ b/tools/testing/selftests/powerpc/tm/tm-signal-context-chk.c
@@ -0,0 +1,102 @@
+/*
+ * Copyright 2016, Cyril Bur, IBM Corp.
+ * Licensed under GPLv2.
+ *
+ * Test the kernel's signal frame code.
+ *
+ * The kernel sets up two sets of ucontexts if the signal was to be delivered
+ * while the thread was in a transaction. Expected behaviour is that the
+ * currently executing code is in the first and the checkpointed state (the
+ * state that will be rolled back to) is in the uc_link ucontext.
+ *
+ * The reason for this is that code which is not TM aware and installs a signal
+ * handler will expect to see/modify its currently running state in the uc,
+ * this code may have dynamicially linked against code which is TM aware and is
+ * doing HTM under the hood.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <signal.h>
+#include <unistd.h>
+
+#include "utils.h"
+#include "tm.h"
+
+#define TBEGIN          ".long 0x7C00051D ;"
+#define TSUSPEND        ".long 0x7C0005DD ;"
+#define TRESUME         ".long 0x7C2005DD ;"
+#define MAX_ATTEMPT 100
+
+static double fps[] = { 1, 2, 3, 4, 5, 6, 7, 8,
+						-1, -2, -3, -4, -5, -6, -7, -8 };
+
+extern long tm_signal_self(pid_t pid, double *fps);
+
+static int signaled;
+static int fail;
+
+static void signal_usr1(int signum, siginfo_t *info, void *uc)
+{
+	int i;
+	ucontext_t *ucp = uc;
+	ucontext_t *tm_ucp = ucp->uc_link;
+
+	signaled = 1;
+
+	/* Always be 64bit, don't really care about 32bit */
+	for (i = 0; i < 8 && !fail; i++) {
+		fail = (ucp->uc_mcontext.gp_regs[i + 14] != i);
+		fail |= (tm_ucp->uc_mcontext.gp_regs[i + 14] != 0xFF - i);
+	}
+	if (fail) {
+		printf("Failed on %d gpr %lu or %lu\n", i - 1, ucp->uc_mcontext.gp_regs[i + 13], tm_ucp->uc_mcontext.gp_regs[i + 13]);
+		return;
+	}
+	for (i = 0; i < 8 && !fail; i++) {
+		fail = (ucp->uc_mcontext.fp_regs[i + 14] != fps[i]);
+		fail |= (tm_ucp->uc_mcontext.fp_regs[i + 14] != fps[i + 8]);
+	}
+	if (fail) {
+		printf("Failed on %d FP %g or %g\n", i - 1, ucp->uc_mcontext.fp_regs[i + 13], tm_ucp->uc_mcontext.fp_regs[i + 13]);
+	}
+}
+
+static int tm_signal_context_chk()
+{
+	struct sigaction act;
+	int i;
+	long rc;
+	pid_t pid = getpid();
+
+	SKIP_IF(!have_htm());
+
+	act.sa_sigaction = signal_usr1;
+	sigemptyset(&act.sa_mask);
+	act.sa_flags = SA_SIGINFO;
+	if (sigaction(SIGUSR1, &act, NULL) < 0) {
+		perror("sigaction sigusr1");
+		exit(1);
+	}
+
+	i = 0;
+	while (!signaled && i < MAX_ATTEMPT) {
+		rc = tm_signal_self(pid, fps);
+		if (!rc) {
+			fprintf(stderr, "Transaction was not doomed...\n");
+			FAIL_IF(!rc);
+		}
+		i++;
+	}
+
+	if (i == MAX_ATTEMPT) {
+		fprintf(stderr, "Tried to signal %d times and didn't work, failing!\n", MAX_ATTEMPT);
+		fail = 1;
+	}
+	return fail;
+}
+
+int main(void)
+{
+	return test_harness(tm_signal_context_chk, "tm_signal_context_chk");
+}
-- 
2.9.0

[RFC 2/3] powerpc: tm: Add TM Unavailable Exception

[Cyril Bur]

If the kernel disables transactional memory (TM) and userspace still
tries TM related actions (TM instructions or TM SPR accesses) TM aware
hardware will cause the kernel to take a facility unavailable
exception.

Add checks for the exception being caused by illegal TM access in
userspace.

Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
---
 arch/powerpc/kernel/traps.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 3e4c84d..29260ee 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1364,6 +1364,13 @@ void vsx_unavailable_exception(struct pt_regs *regs)
 	die("Unrecoverable VSX Unavailable Exception", regs, SIGABRT);
 }
 
+static void tm_unavailable(struct pt_regs *regs)
+{
+	pr_emerg("Unrecoverable TM Unavailable Exception "
+			"%lx at %lx\n", regs->trap, regs->nip);
+	die("Unrecoverable TM Unavailable Exception", regs, SIGABRT);
+}
+
 #ifdef CONFIG_PPC64
 void facility_unavailable_exception(struct pt_regs *regs)
 {
@@ -1434,6 +1441,23 @@ void facility_unavailable_exception(struct pt_regs *regs)
 		return;
 	}
 
+	/*
+	 * TM Unavailable
+	 *
+	 * If
+	 *  - firmware bits say don't do TM or
+	 *  - CONFIG_PPC_TRANSACTIONAL_MEM was not set and
+	 *  - hardware is actually TM aware
+	 * Then userspace can spam the console (even with the use of
+	 * _ratelimited), just send the SIGILL.
+	 */
+	if (status == FSCR_TM_LG) {
+		if (!cpu_has_feature(CPU_FTR_TM))
+			goto out;
+		tm_unavailable(regs);
+		return;
+	}
+
 	if ((status < ARRAY_SIZE(facility_strings)) &&
 	    facility_strings[status])
 		facility = facility_strings[status];
@@ -1446,6 +1470,7 @@ void facility_unavailable_exception(struct pt_regs *regs)
 		"%sFacility '%s' unavailable, exception at 0x%lx, MSR=%lx\n",
 		hv ? "Hypervisor " : "", facility, regs->nip, regs->msr);
 
+out:
 	if (user_mode(regs)) {
 		_exception(SIGILL, regs, ILL_ILLOPC, regs->nip);
 		return;
-- 
2.9.0

[RFC 3/3] powerpc: tm: Enable transactional memory (TM) lazily for userspace

[Cyril Bur]

Currently the MSR TM bit is always set if the hardware is TM capable.
This adds extra overhead as it means the TM SPRS (TFHAR, TEXASR and
TFAIR) must be swapped for each process regardless of if they use TM.

For processes that don't use TM the TM MSR bit can be turned off
allowing the kernel to avoid the expensive swap of the TM registers.

A TM unavailable exception will occur if a thread does use TM and the
kernel will enable MSR_TM and leave it so for some time afterwards.

Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
---
 arch/powerpc/include/asm/processor.h |  1 +
 arch/powerpc/kernel/process.c        | 30 ++++++++++++++++++++++--------
 arch/powerpc/kernel/traps.c          |  8 ++++++++
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 5ff1e4c..9d4363c 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -257,6 +257,7 @@ struct thread_struct {
 	int		used_spe;	/* set if process has used spe */
 #endif /* CONFIG_SPE */
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	u8	load_tm;
 	u64		tm_tfhar;	/* Transaction fail handler addr */
 	u64		tm_texasr;	/* Transaction exception & summary */
 	u64		tm_tfiar;	/* Transaction fail instr address reg */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 2e903c6..8abecda 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -870,6 +870,9 @@ void tm_recheckpoint(struct thread_struct *thread,
 {
 	unsigned long flags;
 
+	if (!(thread->regs->msr & MSR_TM))
+		return;
+
 	/* We really can't be interrupted here as the TEXASR registers can't
 	 * change and later in the trecheckpoint code, we have a userspace R1.
 	 * So let's hard disable over this region.
@@ -905,6 +908,9 @@ static inline void tm_recheckpoint_new_task(struct task_struct *new)
 	if (!new->thread.regs)
 		return;
 
+	if (!(new->thread.regs->msr & MSR_TM))
+		return;
+
 	if (!MSR_TM_ACTIVE(new->thread.regs->msr)){
 		tm_restore_sprs(&new->thread);
 		return;
@@ -925,11 +931,18 @@ static inline void tm_recheckpoint_new_task(struct task_struct *new)
 		 new->pid, mfmsr());
 }
 
-static inline void __switch_to_tm(struct task_struct *prev)
+static inline void __switch_to_tm(struct task_struct *prev, struct task_struct *new)
 {
 	if (cpu_has_feature(CPU_FTR_TM)) {
-		tm_enable();
-		tm_reclaim_task(prev);
+		if (prev->thread.regs && (prev->thread.regs->msr & MSR_TM)) {
+			prev->thread.load_tm++;
+			tm_enable();
+			tm_reclaim_task(prev);
+			if (!MSR_TM_ACTIVE(prev->thread.regs->msr) && prev->thread.load_tm == 0)
+				prev->thread.regs->msr |= ~MSR_TM;
+		} else if (new && new->thread.regs && (new->thread.regs->msr & MSR_TM)) {
+			tm_enable();
+		}
 	}
 }
 
@@ -965,7 +978,7 @@ void restore_tm_state(struct pt_regs *regs)
 
 #else
 #define tm_recheckpoint_new_task(new)
-#define __switch_to_tm(prev)
+#define __switch_to_tm(prev, new)
 #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
 
 static inline void save_sprs(struct thread_struct *t)
@@ -1095,7 +1108,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
 	/* Save FPU, Altivec, VSX and SPE state */
 	giveup_all(prev);
 
-	__switch_to_tm(prev);
+	__switch_to_tm(prev, new);
 
 	/*
 	 * We can't take a PMU exception inside _switch() since there is a
@@ -1340,8 +1353,11 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
 	 * transitions the CPU out of TM mode.  Hence we need to call
 	 * tm_recheckpoint_new_task() (on the same task) to restore the
 	 * checkpointed state back and the TM mode.
+	 *
+	 * Can't pass dst because it isn't ready. Doesn't matter, passing
+	 * dst is only important for __switch_to()
 	 */
-	__switch_to_tm(src);
+	__switch_to_tm(src, NULL);
 	tm_recheckpoint_new_task(src);
 
 	*dst = *src;
@@ -1574,8 +1590,6 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)
 	current->thread.used_spe = 0;
 #endif /* CONFIG_SPE */
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-	if (cpu_has_feature(CPU_FTR_TM))
-		regs->msr |= MSR_TM;
 	current->thread.tm_tfhar = 0;
 	current->thread.tm_texasr = 0;
 	current->thread.tm_tfiar = 0;
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 29260ee..141b953 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1366,6 +1366,14 @@ void vsx_unavailable_exception(struct pt_regs *regs)
 
 static void tm_unavailable(struct pt_regs *regs)
 {
+	if (user_mode(regs)) {
+		current->thread.load_tm++;
+		regs->msr |= MSR_TM;
+		tm_enable();
+		tm_restore_sprs(&current->thread);
+		return;
+	}
+
 	pr_emerg("Unrecoverable TM Unavailable Exception "
 			"%lx at %lx\n", regs->trap, regs->nip);
 	die("Unrecoverable TM Unavailable Exception", regs, SIGABRT);
-- 
2.9.0

Re: [RFC 3/3] powerpc: tm: Enable transactional memory (TM) lazily for userspace

[Laurent Dufour]

On 29/06/2016 08:34, Cyril Bur wrote:
> Currently the MSR TM bit is always set if the hardware is TM capable.
> This adds extra overhead as it means the TM SPRS (TFHAR, TEXASR and
> TFAIR) must be swapped for each process regardless of if they use TM.
> 
> For processes that don't use TM the TM MSR bit can be turned off
> allowing the kernel to avoid the expensive swap of the TM registers.
> 
> A TM unavailable exception will occur if a thread does use TM and the
> kernel will enable MSR_TM and leave it so for some time afterwards.
> 
> Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
> ---
>  arch/powerpc/include/asm/processor.h |  1 +
>  arch/powerpc/kernel/process.c        | 30 ++++++++++++++++++++++--------
>  arch/powerpc/kernel/traps.c          |  8 ++++++++
>  3 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 5ff1e4c..9d4363c 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -257,6 +257,7 @@ struct thread_struct {
>  	int		used_spe;	/* set if process has used spe */
>  #endif /* CONFIG_SPE */
>  #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +	u8	load_tm;
>  	u64		tm_tfhar;	/* Transaction fail handler addr */
>  	u64		tm_texasr;	/* Transaction exception & summary */
>  	u64		tm_tfiar;	/* Transaction fail instr address reg */
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 2e903c6..8abecda 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -870,6 +870,9 @@ void tm_recheckpoint(struct thread_struct *thread,
>  {
>  	unsigned long flags;
>  
> +	if (!(thread->regs->msr & MSR_TM))
> +		return;
> +
>  	/* We really can't be interrupted here as the TEXASR registers can't
>  	 * change and later in the trecheckpoint code, we have a userspace R1.
>  	 * So let's hard disable over this region.
> @@ -905,6 +908,9 @@ static inline void tm_recheckpoint_new_task(struct task_struct *new)
>  	if (!new->thread.regs)
>  		return;
>  
> +	if (!(new->thread.regs->msr & MSR_TM))
> +		return;
> +
>  	if (!MSR_TM_ACTIVE(new->thread.regs->msr)){
>  		tm_restore_sprs(&new->thread);
>  		return;
> @@ -925,11 +931,18 @@ static inline void tm_recheckpoint_new_task(struct task_struct *new)
>  		 new->pid, mfmsr());
>  }
>  
> -static inline void __switch_to_tm(struct task_struct *prev)
> +static inline void __switch_to_tm(struct task_struct *prev, struct task_struct *new)
>  {
>  	if (cpu_has_feature(CPU_FTR_TM)) {
> -		tm_enable();
> -		tm_reclaim_task(prev);
> +		if (prev->thread.regs && (prev->thread.regs->msr & MSR_TM)) {
> +			prev->thread.load_tm++;
> +			tm_enable();
> +			tm_reclaim_task(prev);
> +			if (!MSR_TM_ACTIVE(prev->thread.regs->msr) && prev->thread.load_tm == 0)
> +				prev->thread.regs->msr |= ~MSR_TM;

Hi Cyrill,

I guess the idea is to clear MSR_TM here, so why "or-ing" here ?
I'd rather see :
 +				prev->thread.regs->msr &= ~MSR_TM;

Cheers,
Laurent.

> +		} else if (new && new->thread.regs && (new->thread.regs->msr & MSR_TM)) {
> +			tm_enable();
> +		}
>  	}
>  }
>  
> @@ -965,7 +978,7 @@ void restore_tm_state(struct pt_regs *regs)
>  
>  #else
>  #define tm_recheckpoint_new_task(new)
> -#define __switch_to_tm(prev)
> +#define __switch_to_tm(prev, new)
>  #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
>  
>  static inline void save_sprs(struct thread_struct *t)
> @@ -1095,7 +1108,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
>  	/* Save FPU, Altivec, VSX and SPE state */
>  	giveup_all(prev);
>  
> -	__switch_to_tm(prev);
> +	__switch_to_tm(prev, new);
>  
>  	/*
>  	 * We can't take a PMU exception inside _switch() since there is a
> @@ -1340,8 +1353,11 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
>  	 * transitions the CPU out of TM mode.  Hence we need to call
>  	 * tm_recheckpoint_new_task() (on the same task) to restore the
>  	 * checkpointed state back and the TM mode.
> +	 *
> +	 * Can't pass dst because it isn't ready. Doesn't matter, passing
> +	 * dst is only important for __switch_to()
>  	 */
> -	__switch_to_tm(src);
> +	__switch_to_tm(src, NULL);
>  	tm_recheckpoint_new_task(src);
>  
>  	*dst = *src;
> @@ -1574,8 +1590,6 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)
>  	current->thread.used_spe = 0;
>  #endif /* CONFIG_SPE */
>  #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> -	if (cpu_has_feature(CPU_FTR_TM))
> -		regs->msr |= MSR_TM;
>  	current->thread.tm_tfhar = 0;
>  	current->thread.tm_texasr = 0;
>  	current->thread.tm_tfiar = 0;
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 29260ee..141b953 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1366,6 +1366,14 @@ void vsx_unavailable_exception(struct pt_regs *regs)
>  
>  static void tm_unavailable(struct pt_regs *regs)
>  {
> +	if (user_mode(regs)) {
> +		current->thread.load_tm++;
> +		regs->msr |= MSR_TM;
> +		tm_enable();
> +		tm_restore_sprs(&current->thread);
> +		return;
> +	}
> +
>  	pr_emerg("Unrecoverable TM Unavailable Exception "
>  			"%lx at %lx\n", regs->trap, regs->nip);
>  	die("Unrecoverable TM Unavailable Exception", regs, SIGABRT);
>