[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 at 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 at 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 at 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.