[PATCH 1/4] powerpc/tm: Add commandline option to disable hardware transactional memory

[PATCH 1/4] powerpc/tm: Add commandline option to disable hardware transactional memory

[Michael Ellerman]

From: Cyril Bur <cyrilbur@gmail.com>

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.

Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
[mpe: Simplify to a bool, move to prom.c, put doco in the right place.
 Always disable, regardless of initial state, to avoid user confusion.]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 Documentation/admin-guide/kernel-parameters.txt |  4 ++++
 arch/powerpc/kernel/prom.c                      | 31 +++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 05496622b4ef..ef03e6e16bdb 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3185,6 +3185,10 @@
 			allowed (eg kernel_enable_fpu()/kernel_disable_fpu()).
 			There is some performance impact when enabling this.
 
+	ppc_tm=		[PPC]
+			Format: {"off"}
+			Disable Hardware Transactional Memory
+
 	print-fatal-signals=
 			[KNL] debug: print fatal signals
 
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index f83056297441..d9bd6555f980 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -658,6 +658,35 @@ static void __init early_reserve_mem(void)
 #endif
 }
 
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+static bool tm_disabled __initdata;
+
+static int __init parse_ppc_tm(char *str)
+{
+	bool res;
+
+	if (kstrtobool(str, &res))
+		return -EINVAL;
+
+	tm_disabled = !res;
+
+	return 0;
+}
+early_param("ppc_tm", parse_ppc_tm);
+
+static void __init tm_init(void)
+{
+	if (tm_disabled) {
+		pr_info("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 tm_init(void) { }
+#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+
 void __init early_init_devtree(void *params)
 {
 	phys_addr_t limit;
@@ -767,6 +796,8 @@ void __init early_init_devtree(void *params)
 		powerpc_firmware_features |= FW_FEATURE_PS3_POSSIBLE;
 #endif
 
+	tm_init();
+
 	DBG(" <- early_init_devtree()\n");
 }
 
-- 
2.7.4

[PATCH 2/4] powerpc: Add PPC_FEATURE2_HTM_NO_SUSPEND

[Michael Ellerman]

Some CPUs can operate in a mode where TM (Transactional Memory) is
enabled but the suspended state of TM is disabled. In this mode
tsuspend does not enter suspended state, instead the transaction is
aborted. Similarly any other event that would lead to suspended state
instead aborts the transaction.

There is also an ABI change, in that in this mode processes are not
allowed to sigreturn with an MSR that would lead to suspended state,
Linux will instead return an error to the sigreturn syscall.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/uapi/asm/cputable.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/include/uapi/asm/cputable.h b/arch/powerpc/include/uapi/asm/cputable.h
index 4d877144f377..b3b64cba71ec 100644
--- a/arch/powerpc/include/uapi/asm/cputable.h
+++ b/arch/powerpc/include/uapi/asm/cputable.h
@@ -48,6 +48,7 @@
 #define PPC_FEATURE2_HAS_IEEE128	0x00400000 /* VSX IEEE Binary Float 128-bit */
 #define PPC_FEATURE2_DARN		0x00200000 /* darn random number insn */
 #define PPC_FEATURE2_SCV		0x00100000 /* scv syscall */
+#define PPC_FEATURE2_HTM_NO_SUSPEND	0x00080000 /* TM w/out suspended state */
 
 /*
  * IMPORTANT!
-- 
2.7.4

[PATCH 3/4] powerpc/powernv: Enable TM without suspend if possible

[Michael Ellerman]

Some Power9 revisions can run in a mode where TM operates without
suspended state. If we find ourself on a CPU that might be in this
mode, we query OPAL to check, and if so we reenable TM in CPU
features, and enable a new user feature to signal to userspace that we
are in this mode.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/opal-api.h    |  2 ++
 arch/powerpc/include/asm/powernv.h     |  4 ++++
 arch/powerpc/include/asm/tm.h          |  2 ++
 arch/powerpc/kernel/process.c          |  7 +++++++
 arch/powerpc/kernel/prom.c             |  4 ++++
 arch/powerpc/platforms/powernv/setup.c | 19 +++++++++++++++++++
 6 files changed, 38 insertions(+)

diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index 9d191ebea706..233c7504b1f2 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -896,6 +896,8 @@ enum {
 	 */
 	OPAL_REINIT_CPUS_MMU_HASH	= (1 << 2),
 	OPAL_REINIT_CPUS_MMU_RADIX	= (1 << 3),
+
+	OPAL_REINIT_CPUS_TM_SUSPEND_DISABLED = (1 << 4),
 };
 
 typedef struct oppanel_line {
diff --git a/arch/powerpc/include/asm/powernv.h b/arch/powerpc/include/asm/powernv.h
index f62797702300..dc5f6a5d4575 100644
--- a/arch/powerpc/include/asm/powernv.h
+++ b/arch/powerpc/include/asm/powernv.h
@@ -22,6 +22,8 @@ extern void pnv_npu2_destroy_context(struct npu_context *context,
 extern int pnv_npu2_handle_fault(struct npu_context *context, uintptr_t *ea,
 				unsigned long *flags, unsigned long *status,
 				int count);
+
+void pnv_tm_init(void);
 #else
 static inline void powernv_set_nmmu_ptcr(unsigned long ptcr) { }
 static inline struct npu_context *pnv_npu2_init_context(struct pci_dev *gpdev,
@@ -36,6 +38,8 @@ static inline int pnv_npu2_handle_fault(struct npu_context *context,
 					unsigned long *status, int count) {
 	return -ENODEV;
 }
+
+static inline void pnv_tm_init(void) { }
 #endif
 
 #endif /* _ASM_POWERNV_H */
diff --git a/arch/powerpc/include/asm/tm.h b/arch/powerpc/include/asm/tm.h
index 82e06ca3a49b..ad19fe41931b 100644
--- a/arch/powerpc/include/asm/tm.h
+++ b/arch/powerpc/include/asm/tm.h
@@ -19,4 +19,6 @@ extern void tm_abort(uint8_t cause);
 extern void tm_save_sprs(struct thread_struct *thread);
 extern void tm_restore_sprs(struct thread_struct *thread);
 
+extern bool tm_suspend_disabled;
+
 #endif /* __ASSEMBLY__ */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 166145b18728..b02807ea54dc 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -77,6 +77,13 @@
 extern unsigned long _get_SP(void);
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+/*
+ * Are we running in "Suspend disabled" mode? If so we have to block any
+ * sigreturn that would get us into suspended state, and we also warn in some
+ * other paths that we should never reach with suspend disabled.
+ */
+bool tm_suspend_disabled __ro_after_init = false;
+
 static void check_if_tm_restore_required(struct task_struct *tsk)
 {
 	/*
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index d9bd6555f980..101822be525a 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -47,6 +47,7 @@
 #include <asm/mmu.h>
 #include <asm/paca.h>
 #include <asm/pgtable.h>
+#include <asm/powernv.h>
 #include <asm/iommu.h>
 #include <asm/btext.h>
 #include <asm/sections.h>
@@ -681,7 +682,10 @@ static void __init tm_init(void)
 		cur_cpu_spec->cpu_user_features2 &=
 			~(PPC_FEATURE2_HTM_NOSC | PPC_FEATURE2_HTM);
 		cur_cpu_spec->cpu_features &= ~CPU_FTR_TM;
+		return;
 	}
+
+	pnv_tm_init();
 }
 #else
 static void tm_init(void) { }
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index cf52d53da460..8096c3352e2b 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -36,6 +36,7 @@
 #include <asm/opal.h>
 #include <asm/kexec.h>
 #include <asm/smp.h>
+#include <asm/tm.h>
 
 #include "powernv.h"
 
@@ -304,6 +305,24 @@ static int __init pnv_probe(void)
 	return 1;
 }
 
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+void __init pnv_tm_init(void)
+{
+	if (!firmware_has_feature(FW_FEATURE_OPAL) ||
+	    !pvr_version_is(PVR_POWER9) ||
+	    early_cpu_has_feature(CPU_FTR_TM))
+		return;
+
+	if (opal_reinit_cpus(OPAL_REINIT_CPUS_TM_SUSPEND_DISABLED) != OPAL_SUCCESS)
+		return;
+
+	pr_info("Enabling TM (Transactional Memory) with Suspend Disabled\n");
+	cur_cpu_spec->cpu_features |= CPU_FTR_TM;
+	cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_HTM_NO_SUSPEND;
+	tm_suspend_disabled = true;
+}
+#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+
 /*
  * Returns the cpu frequency for 'cpu' in Hz. This is used by
  * /proc/cpuinfo
-- 
2.7.4

[PATCH 4/4] powerpc/tm: P9 disable transactionally suspended sigcontexts

[Michael Ellerman]

From: Michael Neuling <mikey@neuling.org>

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 by 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 ever enter
suspend when we shouldn't. This should catch systems that don't have
the firmware change and are running TM.

Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 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(+)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index b02807ea54dc..c051dc2b42ad 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -910,6 +910,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_disabled);
+
 	TM_DEBUG("--- tm_reclaim on pid %d (NIP=%lx, "
 		 "ccr=%lx, msr=%lx, trap=%lx)\n",
 		 tsk->pid, thr->regs->nip,
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 92fb1c8dbbd8..1dd5fa0f65fd 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -519,6 +519,8 @@ static int save_tm_user_regs(struct pt_regs *regs,
 {
 	unsigned long msr = regs->msr;
 
+	WARN_ON(tm_suspend_disabled);
+
 	/* 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_disabled)
+		return 1;
 	/*
 	 * restore general registers but not including MSR or SOFTE. Also
 	 * take care of keeping r2 (TLS) intact if not a signal.
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index c83c115858c1..4dfdd8e56836 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -214,6 +214,8 @@ static long setup_tm_sigcontexts(struct sigcontext __user *sc,
 
 	BUG_ON(!MSR_TM_ACTIVE(regs->msr));
 
+	WARN_ON(tm_suspend_disabled);
+
 	/* 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_disabled)
+		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,
-- 
2.7.4

[PATCH 5/4] KVM: PPC: Tie KVM_CAP_PPC_HTM to the user-visible TM feature

[Michael Ellerman]

Currently we use CPU_FTR_TM to decide if the CPU/kernel can support
TM (Transactional Memory), and if it's true we advertise that to
Qemu (or similar) via KVM_CAP_PPC_HTM.

PPC_FEATURE2_HTM is the user-visible feature bit, which indicates that
the CPU and kernel can support TM. Currently CPU_FTR_TM and
PPC_FEATURE2_HTM always have the same value, either true or false, so
using the former for KVM_CAP_PPC_HTM is correct.

However some Power9 CPUs can operate in a mode where TM is enabled but
TM suspended state is disabled. In this mode CPU_FTR_TM is true, but
PPC_FEATURE2_HTM is false. Instead a different PPC_FEATURE2 bit is
set, to indicate that this different mode of TM is available.

It is not safe to let guests use TM as-is, when the CPU is in this
mode. So to prevent that from happening, use PPC_FEATURE2_HTM to
determine the value of KVM_CAP_PPC_HTM.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kvm/powerpc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 3480faaf1ef8..a3746b98ec11 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -644,8 +644,8 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 		break;
 #endif
 	case KVM_CAP_PPC_HTM:
-		r = cpu_has_feature(CPU_FTR_TM_COMP) &&
-		    is_kvmppc_hv_enabled(kvm);
+		r = is_kvmppc_hv_enabled(kvm) &&
+		    (cur_cpu_spec->cpu_user_features2 & PPC_FEATURE2_HTM_COMP);
 		break;
 	default:
 		r = 0;
-- 
2.7.4

Re: [PATCH 3/4] powerpc/powernv: Enable TM without suspend if possible

[Florian Weimer]

On 10/12/2017 12:17 PM, Michael Ellerman wrote:
> +	pr_info("Enabling TM (Transactional Memory) with Suspend Disabled\n");
> +	cur_cpu_spec->cpu_features |= CPU_FTR_TM;
> +	cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_HTM_NO_SUSPEND;
> +	tm_suspend_disabled = true;

This doesn't look right because if suspend is not available, you need to 
clear the original PPC_FEATURE2_HTM flag because the semantics are not 
right, so that applications can use fallback code.  Otherwise, 
applications may incorrectly select the HTM code and break if running on 
a system which supports HTM, but without the suspend state.

The new flag should say that HTM is supported, but without the suspend 
state, and it should be always set if PPC_FEATURE2_HTM is set.

Thanks,
Florian

Re: [PATCH 3/4] powerpc/powernv: Enable TM without suspend if possible

[Tulio Magno Quites Machado Filho]

Florian Weimer <fweimer@redhat.com> writes:

> On 10/12/2017 12:17 PM, Michael Ellerman wrote:
>> +	pr_info("Enabling TM (Transactional Memory) with Suspend Disabled\n");
>> +	cur_cpu_spec->cpu_features |= CPU_FTR_TM;
>> +	cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_HTM_NO_SUSPEND;
>> +	tm_suspend_disabled = true;
>
> This doesn't look right because if suspend is not available, you need to 
> clear the original PPC_FEATURE2_HTM flag because the semantics are not 
> right, so that applications can use fallback code.  Otherwise, 
> applications may incorrectly select the HTM code and break if running on 
> a system which supports HTM, but without the suspend state.

Just clarifying: it won't break an application, but abort the transaction,
which can cause a performance hit.

> The new flag should say that HTM is supported, but without the suspend 
> state, and it should be always set if PPC_FEATURE2_HTM is set.

If we change the semantics of this bit, old applications that don't suspend
transactions won't run on these processors, even if they're safe to run.

If we adopt the current semantics, only applications that enter in suspend
state will have to be modified in order to not get a performance regression.

-- 
Tulio Magno

Re: [PATCH 3/4] powerpc/powernv: Enable TM without suspend if possible

[Florian Weimer]

On 10/19/2017 02:04 PM, Tulio Magno Quites Machado Filho wrote:
> Florian Weimer <fweimer@redhat.com> writes:
> 
>> On 10/12/2017 12:17 PM, Michael Ellerman wrote:
>>> +	pr_info("Enabling TM (Transactional Memory) with Suspend Disabled\n");
>>> +	cur_cpu_spec->cpu_features |= CPU_FTR_TM;
>>> +	cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_HTM_NO_SUSPEND;
>>> +	tm_suspend_disabled = true;
>>
>> This doesn't look right because if suspend is not available, you need to
>> clear the original PPC_FEATURE2_HTM flag because the semantics are not
>> right, so that applications can use fallback code.  Otherwise,
>> applications may incorrectly select the HTM code and break if running on
>> a system which supports HTM, but without the suspend state.
> 
> Just clarifying: it won't break an application, but abort the transaction,
> which can cause a performance hit.

And in this case, it is better not HTM at all.

>> The new flag should say that HTM is supported, but without the suspend
>> state, and it should be always set if PPC_FEATURE2_HTM is set.
> 
> If we change the semantics of this bit, old applications that don't suspend
> transactions won't run on these processors, even if they're safe to run.
> 
> If we adopt the current semantics, only applications that enter in suspend
> state will have to be modified in order to not get a performance regression.

In this light, the trade-off implied by the posted patch seems to be the 
right one.  But the other discussion of an ABI change still makes me a 
bit nervous.

Thanks,
Florian

Re: [PATCH 3/4] powerpc/powernv: Enable TM without suspend if possible

[Tulio Magno Quites Machado Filho]

Forwarding some comments from Adhemerval sent to libc-alpha [1]...

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>Florian Weimer <fweimer@redhat.com> writes:
>
>> On 10/12/2017 12:17 PM, Michael Ellerman wrote:
>>> +	pr_info("Enabling TM (Transactional Memory) with Suspend Disabled\n");
>>> +	cur_cpu_spec->cpu_features |= CPU_FTR_TM;
>>> +	cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_HTM_NO_SUSPEND;
>>> +	tm_suspend_disabled = true;
>>
>> This doesn't look right because if suspend is not available, you need to 
>> clear the original PPC_FEATURE2_HTM flag because the semantics are not 
>> right, so that applications can use fallback code.  Otherwise, 
>> applications may incorrectly select the HTM code and break if running on 
>> a system which supports HTM, but without the suspend state.
>>
>> The new flag should say that HTM is supported, but without the suspend 
>> state, and it should be always set if PPC_FEATURE2_HTM is set.
>
> Will it also change TEXARS with the abort information?

It should, with a permanent error cause so that old applications entering
suspended state can adopt another technique.
Michael, could you clarify if this is indeed happening, please?

> I completely agree with Florian here, this is as *ABI* change
> and the kernel need to advertise a different TM ABI instead
> of as an extension.

Adhemerval, could you elaborate which problems you're foreseeing, please?

-- 
Tulio Magno

[1] https://sourceware.org/ml/libc-alpha/2017-10/msg00893.html

Re: [PATCH 3/4] powerpc/powernv: Enable TM without suspend if possible

[Adhemerval Zanella]

On 19/10/2017 11:34, Tulio Magno Quites Machado Filho wrote:
> Forwarding some comments from Adhemerval sent to libc-alpha [1]...
> 
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>> Florian Weimer <fweimer@redhat.com> writes:
>>
>>> On 10/12/2017 12:17 PM, Michael Ellerman wrote:
>>>> +	pr_info("Enabling TM (Transactional Memory) with Suspend Disabled\n");
>>>> +	cur_cpu_spec->cpu_features |= CPU_FTR_TM;
>>>> +	cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_HTM_NO_SUSPEND;
>>>> +	tm_suspend_disabled = true;
>>>
>>> This doesn't look right because if suspend is not available, you need to 
>>> clear the original PPC_FEATURE2_HTM flag because the semantics are not 
>>> right, so that applications can use fallback code.  Otherwise, 
>>> applications may incorrectly select the HTM code and break if running on 
>>> a system which supports HTM, but without the suspend state.
>>>
>>> The new flag should say that HTM is supported, but without the suspend 
>>> state, and it should be always set if PPC_FEATURE2_HTM is set.
>>
>> Will it also change TEXARS with the abort information?
> 
> It should, with a permanent error cause so that old applications entering
> suspended state can adopt another technique.
> Michael, could you clarify if this is indeed happening, please?
> 
>> I completely agree with Florian here, this is as *ABI* change
>> and the kernel need to advertise a different TM ABI instead
>> of as an extension.
> 
> Adhemerval, could you elaborate which problems you're foreseeing, please?
> 

Pretty much the same Florian already stated: an application can not any
more assume for instance:

        tsr. 	0
        mfcr	r9,128
        andis. 	r10,r9,0x4000
        be 	cr0,L(suspend)
	andis.	r10,r9,0x2000
	be	cr0,L(transactional)

However thinking more about it I am not sure if this should be really a
problem: on default HTM mode the program must handle self-induced failures
as the tbegin. failure path and I assume trying to suspend/resume in this
case will trigger this.  For instance:

   if (__builtin_tbegin (0))
     {
       /* some transactional stuff.  */
       __builtin_tsuspend ();
       /* non transactional stuff.  */
       __builtin_tresume ();
       /* more transactional stuff.  */
     }
   else
     {
       /* fall-out code.  */
     }

So I assume for these chips without suspend/resume support the example
code will always run the fall-out code.

Re: [PATCH 3/4] powerpc/powernv: Enable TM without suspend if possible

[Michael Neuling]

On Thu, 2017-10-19 at 11:34 -0200, Tulio Magno Quites Machado Filho wrote:
> Forwarding some comments from Adhemerval sent to libc-alpha [1]...
>=20
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> > Florian Weimer <fweimer@redhat.com> writes:
> >=20
> > > On 10/12/2017 12:17 PM, Michael Ellerman wrote:
> > > > +	pr_info("Enabling TM (Transactional Memory) with Suspend
> > > > Disabled\n");
> > > > +	cur_cpu_spec->cpu_features |=3D CPU_FTR_TM;
> > > > +	cur_cpu_spec->cpu_user_features2 |=3D
> > > > PPC_FEATURE2_HTM_NO_SUSPEND;
> > > > +	tm_suspend_disabled =3D true;
> > >=20
> > > This doesn't look right because if suspend is not available, you need=
 to=C2=A0
> > > clear the original PPC_FEATURE2_HTM flag because the semantics are no=
t=C2=A0
> > > right, so that applications can use fallback code.=C2=A0=C2=A0Otherwi=
se,=C2=A0
> > > applications may incorrectly select the HTM code and break if running=
 on=C2=A0
> > > a system which supports HTM, but without the suspend state.
> > >=20
> > > The new flag should say that HTM is supported, but without the suspen=
d=C2=A0
> > > state, and it should be always set if PPC_FEATURE2_HTM is set.
> >=20
> > Will it also change TEXARS with the abort information?
>=20
> It should, with a permanent error cause so that old applications entering
> suspended state can adopt another technique.
> Michael, could you clarify if this is indeed happening, please?

It will.

If we hit a tsuspend and we are in this mode, the hardware will abort us an=
d set
the Failure Persistent bit (unless there is some other exception pending at=
 the
same time which causes the abort).

Mikey

Re: [PATCH 1/4] powerpc/tm: Add commandline option to disable hardware transactional memory

[Breno Leitao]

Mikey, Cyril,

On Thu, Oct 12, 2017 at 09:17:16PM +1100, Michael Ellerman wrote:
> From: Cyril Bur <cyrilbur@gmail.com>
> 
> 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.
> 
> Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
> [mpe: Simplify to a bool, move to prom.c, put doco in the right place.
>  Always disable, regardless of initial state, to avoid user confusion.]
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  4 ++++
>  arch/powerpc/kernel/prom.c                      | 31 +++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 05496622b4ef..ef03e6e16bdb 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3185,6 +3185,10 @@
>  			allowed (eg kernel_enable_fpu()/kernel_disable_fpu()).
>  			There is some performance impact when enabling this.
>  
> +	ppc_tm=		[PPC]
> +			Format: {"off"}
> +			Disable Hardware Transactional Memory
> +
>  	print-fatal-signals=
>  			[KNL] debug: print fatal signals
>  
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index f83056297441..d9bd6555f980 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -658,6 +658,35 @@ static void __init early_reserve_mem(void)
>  #endif
>  }
>  
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +static bool tm_disabled __initdata;

I think the name 'tm_disabled' might cause more confusion on the TM
code. Mainly because we already have tm_enable() and tm_enabled()
functions which are related to the MSR register and TM bit, and, with
your new variable, tm_enabled() and tm_disabled are not going to be
exclusionary. Neither tm_enable() with be able to toggle the tm_disabled
value.

Anyway, just my 2 cents.

RE: [PATCH 1/4] powerpc/tm: Add commandline option to disable hardware transactional memory

[David Laight]

> > This patch adds a simple commandline option so that HTM can be
> > disabled at boot time.

ISTM that being able to disable it after boot would be more useful.
(ie in a startup script)

	David

Re: [PATCH 1/4] powerpc/tm: Add commandline option to disable hardware transactional memory

[Michael Neuling]

On Fri, 2017-10-20 at 09:45 -0200, Breno Leitao wrote:
> Mikey, Cyril,
>=20
> On Thu, Oct 12, 2017 at 09:17:16PM +1100, Michael Ellerman wrote:
> > From: Cyril Bur <cyrilbur@gmail.com>
> >=20
> > 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=3Dy then it will allow userspace to make
> > use of the facility.
> >=20
> > 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=3Dn.
> >=20
> > This patch adds a simple commandline option so that HTM can be
> > disabled at boot time.
> >=20
> > Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
> > [mpe: Simplify to a bool, move to prom.c, put doco in the right place.
> > =C2=A0Always disable, regardless of initial state, to avoid user confus=
ion.]
> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> > ---
> > =C2=A0Documentation/admin-guide/kernel-parameters.txt |=C2=A0=C2=A04 ++=
++
> > =C2=A0arch/powerpc/kernel/prom.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0| 31
> > +++++++++++++++++++++++++
> > =C2=A02 files changed, 35 insertions(+)
> >=20
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt
> > b/Documentation/admin-guide/kernel-parameters.txt
> > index 05496622b4ef..ef03e6e16bdb 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -3185,6 +3185,10 @@
> > =C2=A0			allowed (eg
> > kernel_enable_fpu()/kernel_disable_fpu()).
> > =C2=A0			There is some performance impact when enabling
> > this.
> > =C2=A0
> > +	ppc_tm=3D		[PPC]
> > +			Format: {"off"}
> > +			Disable Hardware Transactional Memory
> > +
> > =C2=A0	print-fatal-signals=3D
> > =C2=A0			[KNL] debug: print fatal signals
> > =C2=A0
> > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> > index f83056297441..d9bd6555f980 100644
> > --- a/arch/powerpc/kernel/prom.c
> > +++ b/arch/powerpc/kernel/prom.c
> > @@ -658,6 +658,35 @@ static void __init early_reserve_mem(void)
> > =C2=A0#endif
> > =C2=A0}
> > =C2=A0
> > +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> > +static bool tm_disabled __initdata;
>=20
> I think the name 'tm_disabled' might cause more confusion on the TM
> code. Mainly because we already have tm_enable() and tm_enabled()
> functions which are related to the MSR register and TM bit, and, with
> your new variable, tm_enabled() and tm_disabled are not going to be
> exclusionary. Neither tm_enable() with be able to toggle the tm_disabled
> value.

Got a proposal for better names?

Mikey

Re: [PATCH 1/4] powerpc/tm: Add commandline option to disable hardware transactional memory

[Michael Neuling]

On Fri, 2017-10-20 at 12:58 +0000, David Laight wrote:
> > > This patch adds a simple commandline option so that HTM can be
> > > disabled at boot time.
>=20
> ISTM that being able to disable it after boot would be more useful.
> (ie in a startup script)

I agree bug unfortunately that's impossible.

If a process is already running in tm suspend, there is no way to stop it o=
ther
than killing the process.  At that point you may as well kexec with a new
cmdline option

Mikey

Re: [PATCH 3/4] powerpc/powernv: Enable TM without suspend if possible

[Michael Ellerman]

Hi all,

Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com> writes:
> Forwarding some comments from Adhemerval sent to libc-alpha [1]...
>
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>>Florian Weimer <fweimer@redhat.com> writes:
>>
>>> On 10/12/2017 12:17 PM, Michael Ellerman wrote:
>>>> +	pr_info("Enabling TM (Transactional Memory) with Suspend Disabled\n");
>>>> +	cur_cpu_spec->cpu_features |= CPU_FTR_TM;
>>>> +	cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_HTM_NO_SUSPEND;
>>>> +	tm_suspend_disabled = true;
>>>
>>> This doesn't look right because if suspend is not available, you need to 
>>> clear the original PPC_FEATURE2_HTM flag because the semantics are not 
>>> right, so that applications can use fallback code.  Otherwise, 
>>> applications may incorrectly select the HTM code and break if running on 
>>> a system which supports HTM, but without the suspend state.
...
>> I completely agree with Florian here, this is as *ABI* change
>> and the kernel need to advertise a different TM ABI instead
>> of as an extension.

Sorry for the slow reply, I'm travelling.

You're all misreading the patch, when we enter into this hunk of code TM
is disabled, so PPC_FEATURE2_HTM is *not set*.

If we can configure no suspend mode, then we turn on *only* the new
HTM_NO_SUSPEND bit.

So yes it is an ABI change, and we advertise it as such. User space code
that wants to use "no suspend mode" has to opt-in by checking for the
new feature bit.

I've updated the patch to make this clearer. I've also added HTM_NOSC
because that should still be set (it describes kernel behaviour not
hardware).

cheers

diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index cf52d53da460..d23f148a11f0 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -304,6 +305,28 @@ static int __init pnv_probe(void)
        return 1;
 }
 
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+void __init pnv_tm_init(void)
+{
+       if (!firmware_has_feature(FW_FEATURE_OPAL) ||
+           !pvr_version_is(PVR_POWER9) ||
+           early_cpu_has_feature(CPU_FTR_TM))
+               return;
+
+       if (opal_reinit_cpus(OPAL_REINIT_CPUS_TM_SUSPEND_DISABLED) != OPAL_SUCCESS)
+               return;
+
+       pr_info("Enabling TM (Transactional Memory) with Suspend Disabled\n");
+       cur_cpu_spec->cpu_features |= CPU_FTR_TM;
+       /* Make sure "normal" HTM is off (it should be) */
+       cur_cpu_spec->cpu_user_features2 &= ~PPC_FEATURE2_HTM;
+       /* Turn on no suspend mode, and HTM no SC */
+       cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_HTM_NO_SUSPEND | \
+                                           PPC_FEATURE2_HTM_NOSC;
+       tm_suspend_disabled = true;
+}
+#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+
 /*
  * Returns the cpu frequency for 'cpu' in Hz. This is used by
  * /proc/cpuinfo

Re: [PATCH 3/4] powerpc/powernv: Enable TM without suspend if possible

[Michael Ellerman]

Florian Weimer <fweimer@redhat.com> writes:

> On 10/12/2017 12:17 PM, Michael Ellerman wrote:
>> +	pr_info("Enabling TM (Transactional Memory) with Suspend Disabled\n");
>> +	cur_cpu_spec->cpu_features |= CPU_FTR_TM;
>> +	cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_HTM_NO_SUSPEND;
>> +	tm_suspend_disabled = true;
>
> This doesn't look right because if suspend is not available, you need to 
> clear the original PPC_FEATURE2_HTM flag because the semantics are not 
> right, so that applications can use fallback code.

Yes I agree, but the PPC_FEATURE2_HTM bit is already clear. I've updated
the patch to clear it explicitly, see my other mail.

> Otherwise, 
> applications may incorrectly select the HTM code and break if running on 
> a system which supports HTM, but without the suspend state.

Yes.

> The new flag should say that HTM is supported, but without the suspend 
> state, and it should be always set if PPC_FEATURE2_HTM is set.

I don't agree with that. HTM and HTM_NO_SUSPEND are mutually exclusive.

I don't understand how it would work if HTM_NO_SUSPEND was "always set
if PPC_FEATURE2_HTM is set"? That would tell apps that HTM with suspend
is supported and HTM without suspend is supported, which is meaningless
AFAICS.

cheers

Re: [PATCH 3/4] powerpc/powernv: Enable TM without suspend if possible

[Michael Ellerman]

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:

> On 19/10/2017 11:34, Tulio Magno Quites Machado Filho wrote:
>> Forwarding some comments from Adhemerval sent to libc-alpha [1]...
>> 
>> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>>> Florian Weimer <fweimer@redhat.com> writes:
>>>
>>>> On 10/12/2017 12:17 PM, Michael Ellerman wrote:
>>>>> +	pr_info("Enabling TM (Transactional Memory) with Suspend Disabled\n");
>>>>> +	cur_cpu_spec->cpu_features |= CPU_FTR_TM;
>>>>> +	cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_HTM_NO_SUSPEND;
>>>>> +	tm_suspend_disabled = true;
>>>>
>>>> This doesn't look right because if suspend is not available, you need to 
>>>> clear the original PPC_FEATURE2_HTM flag because the semantics are not 
>>>> right, so that applications can use fallback code.  Otherwise, 
>>>> applications may incorrectly select the HTM code and break if running on 
>>>> a system which supports HTM, but without the suspend state.
>>>>
>>>> The new flag should say that HTM is supported, but without the suspend 
>>>> state, and it should be always set if PPC_FEATURE2_HTM is set.
>>>
>>> Will it also change TEXARS with the abort information?
>> 
>> It should, with a permanent error cause so that old applications entering
>> suspended state can adopt another technique.
>> Michael, could you clarify if this is indeed happening, please?
>> 
>>> I completely agree with Florian here, this is as *ABI* change
>>> and the kernel need to advertise a different TM ABI instead
>>> of as an extension.
>> 
>> Adhemerval, could you elaborate which problems you're foreseeing, please?
>> 
>
> Pretty much the same Florian already stated: an application can not any
> more assume for instance:
>
>         tsr. 	0
>         mfcr	r9,128
>         andis. 	r10,r9,0x4000
>         be 	cr0,L(suspend)
> 	andis.	r10,r9,0x2000
> 	be	cr0,L(transactional)
>
> However thinking more about it I am not sure if this should be really a
> problem: on default HTM mode the program must handle self-induced failures
> as the tbegin. failure path and I assume trying to suspend/resume in this
> case will trigger this.

You're right, it *probably* shouldn't be a problem for most code that is
written well and doesn't use suspend mode for anything except
trace/debug etc.

But that's not a strong enough guarantee to just make the two modes
equivalent and punt the problems to userspace.

Disabling suspend changes the programming model in ways that are not
clearly documented, and which have not been widely tested, so it has to
be controlled by a new feature bit.

cheers

RE: [PATCH 1/4] powerpc/tm: Add commandline option to disable hardware transactional memory

[David Laight]

RnJvbTogTWljaGFlbCBOZXVsaW5nDQo+IFNlbnQ6IDIxIE9jdG9iZXIgMjAxNyAwMjowMA0KPiBU
bzogRGF2aWQgTGFpZ2h0OyAnQnJlbm8gTGVpdGFvJzsgTWljaGFlbCBFbGxlcm1hbg0KPiBDYzog
c3Rld2FydEBsaW51eC52bmV0LmlibS5jb207IGxpbnV4cHBjLWRldkBvemxhYnMub3JnOyBjeXJp
bGJ1ckBnbWFpbC5jb20NCj4gU3ViamVjdDogUmU6IFtQQVRDSCAxLzRdIHBvd2VycGMvdG06IEFk
ZCBjb21tYW5kbGluZSBvcHRpb24gdG8gZGlzYWJsZSBoYXJkd2FyZSB0cmFuc2FjdGlvbmFsIG1l
bW9yeQ0KPiANCj4gT24gRnJpLCAyMDE3LTEwLTIwIGF0IDEyOjU4ICswMDAwLCBEYXZpZCBMYWln
aHQgd3JvdGU6DQo+ID4gPiA+IFRoaXMgcGF0Y2ggYWRkcyBhIHNpbXBsZSBjb21tYW5kbGluZSBv
cHRpb24gc28gdGhhdCBIVE0gY2FuIGJlDQo+ID4gPiA+IGRpc2FibGVkIGF0IGJvb3QgdGltZS4N
Cj4gPg0KPiA+IElTVE0gdGhhdCBiZWluZyBhYmxlIHRvIGRpc2FibGUgaXQgYWZ0ZXIgYm9vdCB3
b3VsZCBiZSBtb3JlIHVzZWZ1bC4NCj4gPiAoaWUgaW4gYSBzdGFydHVwIHNjcmlwdCkNCj4gDQo+
IEkgYWdyZWUgYnVnIHVuZm9ydHVuYXRlbHkgdGhhdCdzIGltcG9zc2libGUuDQo+IA0KPiBJZiBh
IHByb2Nlc3MgaXMgYWxyZWFkeSBydW5uaW5nIGluIHRtIHN1c3BlbmQsIHRoZXJlIGlzIG5vIHdh
eSB0byBzdG9wIGl0IG90aGVyDQo+IHRoYW4ga2lsbGluZyB0aGUgcHJvY2Vzcy4gIEF0IHRoYXQg
cG9pbnQgeW91IG1heSBhcyB3ZWxsIGtleGVjIHdpdGggYSBuZXcNCj4gY21kbGluZSBvcHRpb24N
Cg0KSXNuJ3QgdGhhdCB1bmxpa2VseSB3aGVuIHRoZSBmaXJzdCByYyBzY3JpcHRzIGFyZSBiZWlu
ZyBydW4/DQpTZXR0aW5nIGFuIGVhcmx5IHJjIHNjcmlwdCBpcyBnZW5lcmFsbHkgZWFzaWVyIHRo
YW4gYWRkaW5nIGEgY29tbWFuZCBsaW5lDQpwYXJhbWV0ZXIuDQoNCkkgZG9uJ3Qga25vdyBhYm91
dCBwcGMsIGJ1dCBncnViIG9uIHg4NiBtYWtlcyBpdCBwYWluZnVsbHkgYWxtb3N0DQppbXBvc3Np
YmxlIHRvIGhhdmUgdHdvIGJvb3QgZW50cmllcyB0aGF0IGhhdmUgZGlmZmVyZW50IGNvbW1hbmQg
bGluZQ0Kb3B0aW9ucy4NCg0KCURhdmlkDQoNCg==

Re: [PATCH 1/4] powerpc/tm: Add commandline option to disable hardware transactional memory

[Michael Neuling]

On Mon, 2017-10-23 at 09:01 +0000, David Laight wrote:
> From: Michael Neuling
> > Sent: 21 October 2017 02:00
> > To: David Laight; 'Breno Leitao'; Michael Ellerman
> > Cc: stewart@linux.vnet.ibm.com; linuxppc-dev@ozlabs.org; cyrilbur@gmail=
.com
> > Subject: Re: [PATCH 1/4] powerpc/tm: Add commandline option to disable
> > hardware transactional memory
> >=20
> > On Fri, 2017-10-20 at 12:58 +0000, David Laight wrote:
> > > > > This patch adds a simple commandline option so that HTM can be
> > > > > disabled at boot time.
> > >=20
> > > ISTM that being able to disable it after boot would be more useful.
> > > (ie in a startup script)
> >=20
> > I agree bug unfortunately that's impossible.
> >=20
> > If a process is already running in tm suspend, there is no way to stop =
it
> > other
> > than killing the process.  At that point you may as well kexec with a n=
ew
> > cmdline option
>=20
> Isn't that unlikely when the first rc scripts are being run?
> Setting an early rc script is generally easier than adding a command line
> parameter.

Unlikely or not, it's a case we'd have to handle that would add significant
complexity compared to what's being proposed for (IMHO) little gain.

Also, this doesn't exclude that option later if we decided we could do it.

> I don't know about ppc, but grub on x86 makes it painfully almost
> impossible to have two boot entries that have different command line
> options.

Ok.

Mikey

Re: [PATCH 1/4] powerpc/tm: Add commandline option to disable hardware transactional memory

[Breno Leitao]

On Sat, Oct 21, 2017 at 11:58:47AM +1100, Michael Neuling wrote:
> On Fri, 2017-10-20 at 09:45 -0200, Breno Leitao wrote:
> > Mikey, Cyril,
> > 
> > On Thu, Oct 12, 2017 at 09:17:16PM +1100, Michael Ellerman wrote:
> > > From: Cyril Bur <cyrilbur@gmail.com>
> > > 
> > > 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.
> > > 
> > > Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
> > > [mpe: Simplify to a bool, move to prom.c, put doco in the right place.
> > >  Always disable, regardless of initial state, to avoid user confusion.]
> > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> > > ---
> > >  Documentation/admin-guide/kernel-parameters.txt |  4 ++++
> > >  arch/powerpc/kernel/prom.c                      | 31
> > > +++++++++++++++++++++++++
> > >  2 files changed, 35 insertions(+)
> > > 
> > > diff --git a/Documentation/admin-guide/kernel-parameters.txt
> > > b/Documentation/admin-guide/kernel-parameters.txt
> > > index 05496622b4ef..ef03e6e16bdb 100644
> > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > @@ -3185,6 +3185,10 @@
> > >  			allowed (eg
> > > kernel_enable_fpu()/kernel_disable_fpu()).
> > >  			There is some performance impact when enabling
> > > this.
> > >  
> > > +	ppc_tm=		[PPC]
> > > +			Format: {"off"}
> > > +			Disable Hardware Transactional Memory
> > > +
> > >  	print-fatal-signals=
> > >  			[KNL] debug: print fatal signals
> > >  
> > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> > > index f83056297441..d9bd6555f980 100644
> > > --- a/arch/powerpc/kernel/prom.c
> > > +++ b/arch/powerpc/kernel/prom.c
> > > @@ -658,6 +658,35 @@ static void __init early_reserve_mem(void)
> > >  #endif
> > >  }
> > >  
> > > +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> > > +static bool tm_disabled __initdata;
> > 
> > I think the name 'tm_disabled' might cause more confusion on the TM
> > code. Mainly because we already have tm_enable() and tm_enabled()
> > functions which are related to the MSR register and TM bit, and, with
> > your new variable, tm_enabled() and tm_disabled are not going to be
> > exclusionary. Neither tm_enable() with be able to toggle the tm_disabled
> > value.
> 
> Got a proposal for better names?

That is the hardest part, but I thought about something as:

 * tm_disabled_on_boot
 * tm_off
 * tm_explicit_disabled
 * tm_feature_disabled
 * tm_enablement_disabled

I think these names, although a bit longer, might avoid the confusion
with tm_enable/tm_enabled nomenclature.

Re: [1/4] powerpc/tm: Add commandline option to disable hardware transactional memory

[Michael Ellerman]

On Thu, 2017-10-12 at 10:17:16 UTC, Michael Ellerman wrote:
> From: Cyril Bur <cyrilbur@gmail.com>
> 
> 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.
> 
> Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
> [mpe: Simplify to a bool, move to prom.c, put doco in the right place.
>  Always disable, regardless of initial state, to avoid user confusion.]
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Series applied to powerpc next.

https://git.kernel.org/powerpc/c/07fd1761e1cd2b5ecdb470fc228d52

cheers

Re: [5/4] KVM: PPC: Tie KVM_CAP_PPC_HTM to the user-visible TM feature

[Michael Ellerman]

On Thu, 2017-10-12 at 11:58:54 UTC, Michael Ellerman wrote:
> Currently we use CPU_FTR_TM to decide if the CPU/kernel can support
> TM (Transactional Memory), and if it's true we advertise that to
> Qemu (or similar) via KVM_CAP_PPC_HTM.
> 
> PPC_FEATURE2_HTM is the user-visible feature bit, which indicates that
> the CPU and kernel can support TM. Currently CPU_FTR_TM and
> PPC_FEATURE2_HTM always have the same value, either true or false, so
> using the former for KVM_CAP_PPC_HTM is correct.
> 
> However some Power9 CPUs can operate in a mode where TM is enabled but
> TM suspended state is disabled. In this mode CPU_FTR_TM is true, but
> PPC_FEATURE2_HTM is false. Instead a different PPC_FEATURE2 bit is
> set, to indicate that this different mode of TM is available.
> 
> It is not safe to let guests use TM as-is, when the CPU is in this
> mode. So to prevent that from happening, use PPC_FEATURE2_HTM to
> determine the value of KVM_CAP_PPC_HTM.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Applied to powerpc next.

https://git.kernel.org/powerpc/c/2a3d6553cbd791da4fb624c2500b45

cheers

Re: [PATCH 1/4] powerpc/tm: Add commandline option to disable hardware transactional memory

[Michael Ellerman]

Breno Leitao <leitao@debian.org> writes:
> On Sat, Oct 21, 2017 at 11:58:47AM +1100, Michael Neuling wrote:
>> On Fri, 2017-10-20 at 09:45 -0200, Breno Leitao wrote:
>> > On Thu, Oct 12, 2017 at 09:17:16PM +1100, Michael Ellerman wrote:
>> > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> > > index f83056297441..d9bd6555f980 100644
>> > > --- a/arch/powerpc/kernel/prom.c
>> > > +++ b/arch/powerpc/kernel/prom.c
>> > > @@ -658,6 +658,35 @@ static void __init early_reserve_mem(void)
>> > > =C2=A0#endif
>> > > =C2=A0}
>> > > =C2=A0
>> > > +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>> > > +static bool tm_disabled __initdata;
>> >=20
>> > I think the name 'tm_disabled' might cause more confusion on the TM
>> > code. Mainly because we already have tm_enable() and tm_enabled()
>> > functions which are related to the MSR register and TM bit, and, with
>> > your new variable, tm_enabled() and tm_disabled are not going to be
>> > exclusionary. Neither tm_enable() with be able to toggle the tm_disabl=
ed
>> > value.

I've merged it with that name, but I'm happy to take an incremental
patch to give it a better name.

>> Got a proposal for better names?
>
> That is the hardest part, but I thought about something as:
>
>  * tm_disabled_on_boot

Maybe.

>  * tm_off

Nah.

>  * tm_explicit_disabled

Maybe.

>  * tm_feature_disabled

No that's not quite accurate.

>  * tm_enablement_disabled

I refuse to use "enablement" ;)

> I think these names, although a bit longer, might avoid the confusion
> with tm_enable/tm_enabled nomenclature.

tm_cmdline_disabled would be OK. Or tm_force_disable, or ...

cheers