powerpc/tm: Add commandline option to disable hardware transactional memory
Subsystems (2)
PATCH 1/3 powerpc/tm: Add commandline option to disable hardware transactional memory
Currently the kernel relies on firmware to inform it whether or not the CPU supports HTM and as long as the kernel was built with CONFIG_PPC_TRANSACTIONAL_MEM=y then it will allow userspace to make use of the facility.
There may be situations where it would be advantageous for the kernel to not allow userspace to use HTM, currently the only way to achieve this is to recompile the kernel with CONFIG_PPC_TRANSACTIONAL_MEM=n.
This patch adds a simple commandline option so that HTM can be disabled at boot time.
--- Documentation/admin-guide/kernel-parameters.txt | 4 ++++ arch/powerpc/include/asm/tm.h | 3 +++ arch/powerpc/kernel/setup_64.c | 28 +++++++++++++++++++++++++ 3 files changed, 35 insertions(+)
Documentation/admin-guide/kernel-parameters.txt +4 -0
@@ -804,6 +804,10 @@ disable_radix [PPC] Disable RADIX MMU mode on POWER9 + ppc_tm= [PPC] + Format: {"off"} + Disable Hardware Transactional Memory + disable_cpu_apicid= [X86,APIC,SMP] Format: <int> The number of initial APIC ID for the
arch/powerpc/include/asm/tm.h +3 -0
@@ -9,6 +9,9 @@#ifndef __ASSEMBLY__ +#define TM_STATE_ON 0 +#define TM_STATE_OFF 1 +extern void tm_enable (void );extern void tm_reclaim (struct thread_struct *thread,unsigned long orig_msr,uint8_t cause);
arch/powerpc/kernel/setup_64.c +28 -0
@@ -68,6 +68,7 @@#include <asm/livepatch.h> #include <asm/opal.h> #include <asm/cputhreads.h> +#include <asm/tm.h> #ifdef DEBUG #define DBG(fmt...) udbg_printf(fmt)
@@ -250,6 +251,31 @@ static void cpu_ready_for_interrupts(void)get_paca ()->kernel_msr =MSR_KERNEL ; } +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM +static int ppc_tm_state; +static int __init parse_ppc_tm (char *p) +{ +if (strcmp (p,"off" ) ==0 ) + ppc_tm_state =TM_STATE_OFF ; +else +printk (KERN_NOTICE "Unknown value to cmdline ppc_tm '%s'\n" , p); +return 0 ; +} +early_param ("ppc_tm" , parse_ppc_tm); + +static void check_disable_tm (void ) +{ +if (cpu_has_feature (CPU_FTR_TM ) && ppc_tm_state ==TM_STATE_OFF ) { +printk (KERN_NOTICE "Disabling hardware transactional memory (HTM)\n" ); + cur_cpu_spec->cpu_user_features2 &= + ~(PPC_FEATURE2_HTM_NOSC |PPC_FEATURE2_HTM ); + cur_cpu_spec->cpu_features &= ~CPU_FTR_TM ; + } +} +#else +static void check_disable_tm (void ) { } +#endif +/* * Early initialization entry point. This is called by head.S * with MMU translation disabled. We rely on the "feature" of
@@ -299,6 +325,8 @@ void __init early_setup(unsigned long dt_ptr)*/ early_init_devtree (__va (dt_ptr)); +check_disable_tm (); +/* Now we know the logical id of our boot cpu, setup the paca. */ setup_paca (&paca[boot_cpuid]);fixup_boot_paca ();
Signature · 1 line
2.14.2
PATCH 2/3 powerpc/tm: P9 disabled suspend mode workaround
[from Michael Neulings original patch]
Each POWER9 core is made of two super slices. Each super slice can only have one thread at a time in TM suspend mode. The super slice restricts ever entering a state where both threads are in suspend by aborting transactions on tsuspend or exceptions into the kernel.
Unfortunately for context switch we need trechkpt which forces suspend mode. If a thread is already in suspend and a second thread needs to be restored that was suspended, the trechkpt must be executed. Currently the trechkpt will hang in this case until the other thread exits suspend. This causes problems for Linux resulting in hang and RCU stall detectors going off.
To workaround this, we disable suspend in the core. This is done via a firmware change which stops the hardware ever getting into suspend. The hardware will always rollback a transaction on any tsuspend or entry into the kernel.
[added by Cyril Bur]
As the no-suspend firmware change is novel and untested using it should be opt in by users. Furthumore, currently the kernel has no method to know if the firmware has applied the no-suspend workaround. This patch extends the ppc_tm commandline option to allow users to opt-in if they are sure that their firmware has been updated and they understand the risks involed.
--- Documentation/admin-guide/kernel-parameters.txt | 7 +++++-- arch/powerpc/include/asm/cputable.h | 6 ++++++ arch/powerpc/include/asm/tm.h | 6 ++++-- arch/powerpc/kernel/cputable.c | 12 ++++++++++++ arch/powerpc/kernel/setup_64.c | 16 ++++++++++------ 5 files changed, 37 insertions(+), 10 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +5 -2
@@ -805,8 +805,11 @@ Disable RADIX MMU mode on POWER9 ppc_tm= [PPC] - Format: {"off"} - Disable Hardware Transactional Memory + Format: {"off" | "no-suspend"} + "Off" Will disable Hardware Transactional Memory. + "no-suspend" Informs the kernel that the + hardware will not transition into the kernel + with a suspended transaction. disable_cpu_apicid= [X86,APIC,SMP] Format: <int>
arch/powerpc/include/asm/cputable.h +6 -0
@@ -124,6 +124,12 @@ extern void identify_cpu_name(unsigned int pvr);extern void do_feature_fixups (unsigned long value,void *fixup_start,void *fixup_end); +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM +extern bool tm_suspend_supported (void ); +#else +static inline bool tm_suspend_supported (void ) {return false; } +#endif +extern const char *powerpc_base_platform;#ifdef CONFIG_JUMP_LABEL_FEATURE_CHECKS
arch/powerpc/include/asm/tm.h +4 -2
@@ -9,9 +9,11 @@#ifndef __ASSEMBLY__ -#define TM_STATE_ON 0 -#define TM_STATE_OFF 1 +#define TM_STATE_ON 0 +#define TM_STATE_OFF 1 +#define TM_STATE_NO_SUSPEND 2 +extern int ppc_tm_state;extern void tm_enable (void );extern void tm_reclaim (struct thread_struct *thread,unsigned long orig_msr,uint8_t cause);
arch/powerpc/kernel/cputable.c +12 -0
@@ -22,6 +22,7 @@#include <asm/prom.h> /* for PTRRELOC on ARCH=ppc */ #include <asm/mmu.h> #include <asm/setup.h> +#include <asm/tm.h> static struct cpu_spec the_cpu_spec__read_mostly ;
@@ -2301,6 +2302,17 @@ void __init identify_cpu_name(unsigned int pvr) } } +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM +bool tm_suspend_supported (void ) +{ +if (cpu_has_feature (CPU_FTR_TM )) { +if (pvr_version_is (PVR_POWER9 ) && ppc_tm_state !=TM_STATE_NO_SUSPEND ) +return false; +return true; + } +return false; +} +#endif #ifdef CONFIG_JUMP_LABEL_FEATURE_CHECKS struct static_key_true cpu_feature_keys[NUM_CPU_FTR_KEYS] = {
arch/powerpc/kernel/setup_64.c +10 -6
@@ -251,12 +251,14 @@ static void cpu_ready_for_interrupts(void)get_paca ()->kernel_msr =MSR_KERNEL ; } +int ppc_tm_state;#ifdef CONFIG_PPC_TRANSACTIONAL_MEM -static int ppc_tm_state;static int __init parse_ppc_tm (char *p) {if (strcmp (p,"off" ) ==0 ) ppc_tm_state =TM_STATE_OFF ; +else if (strcmp (p,"no-suspend" ) ==0 ) + ppc_tm_state =TM_STATE_NO_SUSPEND ;else printk (KERN_NOTICE "Unknown value to cmdline ppc_tm '%s'\n" , p);return 0 ;
@@ -265,11 +267,13 @@ early_param("ppc_tm", parse_ppc_tm);static void check_disable_tm (void ) { -if (cpu_has_feature (CPU_FTR_TM ) && ppc_tm_state ==TM_STATE_OFF ) { -printk (KERN_NOTICE "Disabling hardware transactional memory (HTM)\n" ); - cur_cpu_spec->cpu_user_features2 &= - ~(PPC_FEATURE2_HTM_NOSC |PPC_FEATURE2_HTM ); - cur_cpu_spec->cpu_features &= ~CPU_FTR_TM ; +if (cpu_has_feature (CPU_FTR_TM )) { +if (ppc_tm_state ==TM_STATE_OFF || (!tm_suspend_supported ())) { +printk (KERN_NOTICE "Disabling hardware transactional memory (HTM)\n" ); + cur_cpu_spec->cpu_user_features2 &= + ~(PPC_FEATURE2_HTM_NOSC |PPC_FEATURE2_HTM ); + cur_cpu_spec->cpu_features &= ~CPU_FTR_TM ; + } } }#else
Signature · 1 line
2.14.2
Re: [PATCH 2/3] powerpc/tm: P9 disabled suspend mode workaround
Quoted 18 lines · Cyril Bur
[from Michael Neulings original patch]
Each POWER9 core is made of two super slices. Each super slice can only have one thread at a time in TM suspend mode. The super slice restricts ever entering a state where both threads are in suspend by aborting transactions on tsuspend or exceptions into the kernel.Unfortunately for context switch we need trechkpt which forces suspend mode. If a thread is already in suspend and a second thread needs to be restored that was suspended, the trechkpt must be executed. Currently the trechkpt will hang in this case until the other thread exits suspend. This causes problems for Linux resulting in hang and RCU stall detectors going off.
To workaround this, we disable suspend in the core. This is done via a firmware change which stops the hardware ever getting into suspend. The hardware will always rollback a transaction on any tsuspend or entry into the kernel.
Quoted 7 lines
[added by Cyril Bur]
As the no-suspend firmware change is novel and untested using it should be opt in by users. Furthumore, currently the kernel has no method to know if the firmware has applied the no-suspend workaround. This patch extends theppc_tmcommandline option to allow users to opt-in if they are sure that their firmware has been updated and they understand the risks involed.
Is this what the patch actually does ? ...
Quoted 76 lines
Signed-off-by Cyril Bur <[email protected]>--- Documentation/admin-guide/kernel-parameters.txt | 7 +++++-- arch/powerpc/include/asm/cputable.h | 6 ++++++ arch/powerpc/include/asm/tm.h | 6 ++++-- arch/powerpc/kernel/cputable.c | 12 ++++++++++++ arch/powerpc/kernel/setup_64.c | 16 ++++++++++------ 5 files changed, 37 insertions(+), 10 deletions(-)Documentation/admin-guide/kernel-parameters.txt +5
-2@@ -805,8 +805,11 @@ Disable RADIX MMU mode on POWER9 ppc_tm= [PPC] - Format: {"off"} - Disable Hardware Transactional Memory + Format: {"off" | "no-suspend"} + "Off" Will disable Hardware Transactional Memory. + "no-suspend" Informs the kernel that the + hardware will not transition into the kernel + with a suspended transaction. disable_cpu_apicid= [X86,APIC,SMP] Format: <int>arch/powerpc/include/asm/cputable.h +6
-0@@ -124,6 +124,12 @@ extern void identify_cpu_name(unsigned int pvr);extern void do_feature_fixups (unsigned long value,void *fixup_start,void *fixup_end); +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM +extern bool tm_suspend_supported (void ); +#else +static inline bool tm_suspend_supported (void ) {return false; } +#endif +extern const char *powerpc_base_platform;#ifdef CONFIG_JUMP_LABEL_FEATURE_CHECKS arch/powerpc/include/asm/tm.h +4
-2@@ -9,9 +9,11 @@#ifndef __ASSEMBLY__ -#define TM_STATE_ON 0 -#define TM_STATE_OFF 1 +#define TM_STATE_ON 0 +#define TM_STATE_OFF 1 +#define TM_STATE_NO_SUSPEND 2 +extern int ppc_tm_state;extern void tm_enable (void );extern void tm_reclaim (struct thread_struct *thread,unsigned long orig_msr,uint8_t cause);arch/powerpc/kernel/cputable.c +1
-0@@ -22,6 +22,7 @@#include <asm/prom.h> /* for PTRRELOC on ARCH=ppc */ #include <asm/mmu.h> #include <asm/setup.h> +#include <asm/tm.h> static struct cpu_spec the_cpu_spec__read_mostly ;@@ -2301,6 +2302,17 @@ void __init identify_cpu_name(unsigned int pvr) }
arch/powerpc/kernel/cputable.c void __init identify_cpu_name(unsigned int pvr) · line 2312
Quoted 10 lines
arch/powerpc/kernel/cputable.c +8
-0} +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM +bool tm_suspend_supported (void ) +{ +if (cpu_has_feature (CPU_FTR_TM )) { +if (pvr_version_is (PVR_POWER9 ) && ppc_tm_state !=TM_STATE_NO_SUSPEND ) +return false; +return true; + }
Hrm... so if state is "NO SUSPEND" you return "true" ? Isn't this backward ? Or I don't understand what this is about...
Quoted 32 lines
arch/powerpc/kernel/cputable.c +3
-0+return false; +} +#endif #ifdef CONFIG_JUMP_LABEL_FEATURE_CHECKS struct static_key_true cpu_feature_keys[NUM_CPU_FTR_KEYS] = {arch/powerpc/kernel/setup_64.c +3
-3@@ -251,12 +251,14 @@ static void cpu_ready_for_interrupts(void)get_paca ()->kernel_msr =MSR_KERNEL ; } +int ppc_tm_state;#ifdef CONFIG_PPC_TRANSACTIONAL_MEM -static int ppc_tm_state;static int __init parse_ppc_tm (char *p) {if (strcmp (p,"off" ) ==0 ) ppc_tm_state =TM_STATE_OFF ; +else if (strcmp (p,"no-suspend" ) ==0 ) + ppc_tm_state =TM_STATE_NO_SUSPEND ;else printk (KERN_NOTICE "Unknown value to cmdline ppc_tm '%s'\n" , p);return 0 ;@@ -265,11 +267,13 @@ early_param("ppc_tm", parse_ppc_tm);static void check_disable_tm (void ) { -if (cpu_has_feature (CPU_FTR_TM ) && ppc_tm_state ==TM_STATE_OFF ) { -printk (KERN_NOTICE "Disabling hardware transactional memory (HTM)\n" );
arch/powerpc/kernel/setup_64.c early_param("ppc_tm", parse_ppc_tm); · line 276
Quoted 10 lines
arch/powerpc/kernel/setup_64.c +7
-3- cur_cpu_spec->cpu_user_features2 &= - ~(PPC_FEATURE2_HTM_NOSC |PPC_FEATURE2_HTM ); - cur_cpu_spec->cpu_features &= ~CPU_FTR_TM ; +if (cpu_has_feature (CPU_FTR_TM )) { +if (ppc_tm_state ==TM_STATE_OFF || (!tm_suspend_supported ())) { +printk (KERN_NOTICE "Disabling hardware transactional memory (HTM)\n" ); + cur_cpu_spec->cpu_user_features2 &= + ~(PPC_FEATURE2_HTM_NOSC |PPC_FEATURE2_HTM ); + cur_cpu_spec->cpu_features &= ~CPU_FTR_TM ; + }
So that code translates to if TM is off or doesn't support suspend, disable TM. Are we sure that's really what we meant here ?
I suspect this makes more sense if that function was called tm_supported() ...
Quoted 3 lines
arch/powerpc/kernel/setup_64.c +0
-0} }#else
Michael Ellerman
reviewerRe: [PATCH 2/3] powerpc/tm: P9 disabled suspend mode workaround
Quoted 1 line · Benjamin Herrenschmidt
On Fri, 2017-10-06 at 18:46 +1100, Cyril Bur wrote:
...
Quoted 8 lines
Quoted 8 lines
arch/powerpc/kernel/cputable.c +0
-0@@ -2301,6 +2302,17 @@ void __init identify_cpu_name(unsigned int pvr) } }
Quoted 11 lines
arch/powerpc/kernel/cputable.c void __init identify_cpu_name(unsigned int pvr) · line 2312
Quoted 8 lines
arch/powerpc/kernel/cputable.c +8
-0+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM +bool tm_suspend_supported (void ) +{ +if (cpu_has_feature (CPU_FTR_TM )) { +if (pvr_version_is (PVR_POWER9 ) && ppc_tm_state !=TM_STATE_NO_SUSPEND ) +return false; +return true; + }Hrm... so if state is "NO SUSPEND" you return "true" ? Isn't this backward ? Or I don't understand what this is about...
Yeah it's a bit confuzzled.
I literally wrote it for you on a post-it Cyril! >:D
tm_suspend_supported() should be called tm_no_suspend_mode(). Where "no suspend mode" is the new "mode" we're adding where suspend is not supported.
Then tm_no_suspend_mode() should be:
+ if (pvr_version_is(PVR_POWER9) && ppc_tm_state == TM_STATE_NO_SUSPEND)
+ return true;
+ return false;
And then all the extra checks and warnings in patch 3 just use it like:
WARN_ON(tm_no_suspend_mode());
Because they're in paths where we shouldn't get to if suspend is disabled.
I don't think we need to check CPU_FTR_TM because it's only called from TM paths anyway. But we could add that to be paranoid. Or probably better, when TM is forced off (below) we set ppc_tm_state to off.
Quoted 15 lines
Quoted 15 lines
arch/powerpc/kernel/setup_64.c +2
-5@@ -265,11 +267,13 @@ early_param("ppc_tm", parse_ppc_tm);static void check_disable_tm (void ) { -if (cpu_has_feature (CPU_FTR_TM ) && ppc_tm_state ==TM_STATE_OFF ) { -printk (KERN_NOTICE "Disabling hardware transactional memory (HTM)\n" ); - cur_cpu_spec->cpu_user_features2 &= - ~(PPC_FEATURE2_HTM_NOSC |PPC_FEATURE2_HTM ); - cur_cpu_spec->cpu_features &= ~CPU_FTR_TM ; +if (cpu_has_feature (CPU_FTR_TM )) { +if (ppc_tm_state ==TM_STATE_OFF || (!tm_suspend_supported ())) {
Quoted 8 lines
arch/powerpc/kernel/setup_64.c early_param("ppc_tm", parse_ppc_tm); · line 276
Quoted 5 lines
arch/powerpc/kernel/setup_64.c +5
-0+printk (KERN_NOTICE "Disabling hardware transactional memory (HTM)\n" ); + cur_cpu_spec->cpu_user_features2 &= + ~(PPC_FEATURE2_HTM_NOSC |PPC_FEATURE2_HTM ); + cur_cpu_spec->cpu_features &= ~CPU_FTR_TM ; + }So that code translates to if TM is off or doesn't support suspend, disable TM. Are we sure that's really what we meant here ?
It should be:
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return;
+
+ if (ppc_tm_state == TM_STATE_OFF || \
+ (pvr_version_is(PVR_POWER9) && ppc_tm_state != TM_STATE_NO_SUSPEND)) {
+ printk(KERN_NOTICE "Disabling hardware transactional memory (HTM)\n");
+ cur_cpu_spec->cpu_user_features2 &= ~(PPC_FEATURE2_HTM_NOSC | PPC_FEATURE2_HTM);
+ cur_cpu_spec->cpu_features &= ~CPU_FTR_TM;
+ }
And as I mentioned above perhaps we should also do:
+ ppc_tm_state = TM_STATE_OFF;
cheers
Re: [PATCH 2/3] powerpc/tm: P9 disabled suspend mode workaround
In reply to Cyril BurHi Cyril,
Quoted 3 lines · Cyril Bur
[added by Cyril Bur]
As the no-suspend firmware change is novel and untested using it should be opt in by users. Furthumore, currently the kernel has no method to
I forgot to mention on my last reply, but should s/Furthumore/Furthermore/ ?
Regards,
Gustavo
PATCH 3/3 powerpc/tm: P9 disable transactionally suspended sigcontexts
In reply to Cyril BurFrom: Michael Neuling <[email protected]>
Unfortunately userspace can construct a sigcontext which enables suspend. Thus userspace can force Linux into a path where trechkpt is executed.
This patch blocks this from happening on POWER9 but sanity checking sigcontexts passed in.
ptrace doesn't have this problem as only MSR SE and BE can be changed via ptrace.
This patch also adds a number of WARN_ON() in case we every enter suspend when we shouldn't. This should catch systems that don't have the firmware change and are running TM.
A future firmware change will allow suspend mode on POWER9 but that is going to require additional Linux changes to support. In the interim, this allows TM to continue to (partially) work while stopping userspace from crashing Linux.
--- arch/powerpc/kernel/process.c | 2 ++ arch/powerpc/kernel/signal_32.c | 4 ++++ arch/powerpc/kernel/signal_64.c | 5 +++++ 3 files changed, 11 insertions(+)
arch/powerpc/kernel/process.c +2 -0
@@ -903,6 +903,8 @@ static inline void tm_reclaim_task(struct task_struct *tsk)if (!MSR_TM_ACTIVE (thr->regs->msr)) goto out_and_saveregs; +WARN_ON (!tm_suspend_supported ()); + TM_DEBUG("--- tm_reclaim on pid %d (NIP=%lx, " "ccr=%lx, msr=%lx, trap=%lx)\n", tsk->pid, thr->regs->nip,
arch/powerpc/kernel/signal_32.c +4 -0
@@ -519,6 +519,8 @@ static int save_tm_user_regs(struct pt_regs *regs, {unsigned long msr = regs->msr; +WARN_ON (!tm_suspend_supported ()); +/* Remove TM bits from thread's MSR. The MSR in the sigcontext * just indicates to userland that we were doing a transaction, but we * don't want to return in transactional state. This also ensures
@@ -769,6 +771,8 @@ static long restore_tm_user_regs(struct pt_regs *regs,int i;#endif +if (!tm_suspend_supported ()) +return 1 ;/* * restore general registers but not including MSR or SOFTE. Also * take care of keeping r2 (TLS) intact if not a signal.
arch/powerpc/kernel/signal_64.c +5 -0
@@ -214,6 +214,8 @@ static long setup_tm_sigcontexts(struct sigcontext __user *sc,BUG_ON (!MSR_TM_ACTIVE (regs->msr)); +WARN_ON (!tm_suspend_supported ()); +/* Remove TM bits from thread's MSR. The MSR in the sigcontext * just indicates to userland that we were doing a transaction, but we * don't want to return in transactional state. This also ensures
@@ -430,6 +432,9 @@ static long restore_tm_sigcontexts(struct task_struct *tsk,BUG_ON (tsk != current); +if (!tm_suspend_supported ()) +return -EINVAL ; +/* copy the GPRs */ err |=__copy_from_user (regs->gpr, tm_sc->gp_regs,sizeof (regs->gpr)); err |= __copy_from_user(&tsk->thread.ckpt_regs, sc->gp_regs,
Signature · 1 line
2.14.2
Re: [PATCH 3/3] powerpc/tm: P9 disable transactionally suspended sigcontexts
Quoted 34 lines · Cyril Bur
From: Michael Neuling <[email protected]>
Unfortunately userspace can construct a sigcontext which enables suspend. Thus userspace can force Linux into a path where trechkpt is executed.
This patch blocks this from happening on POWER9 but sanity checking sigcontexts passed in.
ptrace doesn't have this problem as only MSR SE and BE can be changed via ptrace.
This patch also adds a number of
WARN_ON()in case we every enter suspend when we shouldn't. This should catch systems that don't have the firmware change and are running TM.A future firmware change will allow suspend mode on POWER9 but that is going to require additional Linux changes to support. In the interim, this allows TM to continue to (partially) work while stopping userspace from crashing Linux.
Signed-off-by Michael Neuling <[email protected]>Signed-off-by Cyril Bur <[email protected]>--- arch/powerpc/kernel/process.c | 2 ++ arch/powerpc/kernel/signal_32.c | 4 ++++ arch/powerpc/kernel/signal_64.c | 5 +++++ 3 files changed, 11 insertions(+)arch/powerpc/kernel/process.c +0
-0
arch/powerpc/kernel/process.c static inline void tm_reclaim_task(struct task_struct *tsk) · line 907
Quoted 5 lines
arch/powerpc/kernel/process.c +2
-0@@ -903,6 +903,8 @@ static inline void tm_reclaim_task(struct task_struct *tsk)if (!MSR_TM_ACTIVE (thr->regs->msr)) goto out_and_saveregs; +WARN_ON (!tm_suspend_supported ()); +
What does this function really says ? That TM is supported or that TM supports suspend ? Because the implementation in the previous patch seems to indicate that what it actually indicates is that TM is supported, period.
Quoted 48 lines
arch/powerpc/kernel/process.c +0
-0TM_DEBUG("--- tm_reclaim on pid %d (NIP=%lx, " "ccr=%lx, msr=%lx, trap=%lx)\n", tsk->pid, thr->regs->nip,arch/powerpc/kernel/signal_32.c +4
-0@@ -519,6 +519,8 @@ static int save_tm_user_regs(struct pt_regs *regs, {unsigned long msr = regs->msr; +WARN_ON (!tm_suspend_supported ()); +/* Remove TM bits from thread's MSR. The MSR in the sigcontext * just indicates to userland that we were doing a transaction, but we * don't want to return in transactional state. This also ensures @@ -769,6 +771,8 @@ static long restore_tm_user_regs(struct pt_regs *regs,int i;#endif +if (!tm_suspend_supported ()) +return 1 ;/* * restore general registers but not including MSR or SOFTE. Also * take care of keeping r2 (TLS) intact if not a signal. arch/powerpc/kernel/signal_64.c +5
-0@@ -214,6 +214,8 @@ static long setup_tm_sigcontexts(struct sigcontext __user *sc,BUG_ON (!MSR_TM_ACTIVE (regs->msr)); +WARN_ON (!tm_suspend_supported ()); +/* Remove TM bits from thread's MSR. The MSR in the sigcontext * just indicates to userland that we were doing a transaction, but we * don't want to return in transactional state. This also ensures @@ -430,6 +432,9 @@ static long restore_tm_sigcontexts(struct task_struct *tsk,BUG_ON (tsk != current); +if (!tm_suspend_supported ()) +return -EINVAL ; +/* copy the GPRs */ err |=__copy_from_user (regs->gpr, tm_sc->gp_regs,sizeof (regs->gpr)); err |= __copy_from_user(&tsk->thread.ckpt_regs, sc->gp_regs,
Re: [PATCH 3/3] powerpc/tm: P9 disable transactionally suspended sigcontexts
In reply to Cyril BurHi Cyril,
Quoted 8 lines · Cyril Bur
From: Michael Neuling <[email protected]>
Unfortunately userspace can construct a sigcontext which enables suspend. Thus userspace can force Linux into a path where trechkpt is executed.
This patch blocks this from happening on POWER9 but sanity checking sigcontexts passed in.
I think "but" should say "by" as pointed out by Joel and acked by Mikey previously.
Regards,
Gustavo