[MODERATED] Re: [PATCH v5 03/11] TAAv5 3

On 10/4/19 11:28 PM, speck for Pawan Gupta wrote:
> Disable TSX by default on bootup. If IA32_TSX_CTRL MSR is not present,
> TSX state stays NOT_SUPPORTED which is the compile time default,
> otherwise change TSX state to DISABLE. This is because on certain
> processsors TSX may be used as a part of a speculative side channel
> attack.
> 
> Signed-off-by: Pawan Gupta <[email protected]>
> Reviewed-by: Mark Gross <[email protected]>
> Reviewed-by: Tony Luck <[email protected]>
> Tested-by: Neelima Krishnan <[email protected]>
> ---
>  arch/x86/kernel/cpu/Makefile |  2 +-
>  arch/x86/kernel/cpu/cpu.h    |  4 ++
>  arch/x86/kernel/cpu/intel.c  |  2 +
>  arch/x86/kernel/cpu/tsx.c    | 72 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 79 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/kernel/cpu/tsx.c
> 
> diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
> index d7a1e5a9331c..890f60083eca 100644
> --- a/arch/x86/kernel/cpu/Makefile
> +++ b/arch/x86/kernel/cpu/Makefile
> @@ -30,7 +30,7 @@ obj-$(CONFIG_PROC_FS)	+= proc.o
>  obj-$(CONFIG_X86_FEATURE_NAMES) += capflags.o powerflags.o
>  
>  ifdef CONFIG_CPU_SUP_INTEL
> -obj-y			+= intel.o intel_pconfig.o
> +obj-y			+= intel.o intel_pconfig.o tsx.o
>  obj-$(CONFIG_PM)	+= intel_epb.o
>  endif
>  obj-$(CONFIG_CPU_SUP_AMD)		+= amd.o
> diff --git a/arch/x86/kernel/cpu/cpu.h b/arch/x86/kernel/cpu/cpu.h
> index c0e2407abdd6..d864ec4180cc 100644
> --- a/arch/x86/kernel/cpu/cpu.h
> +++ b/arch/x86/kernel/cpu/cpu.h
> @@ -62,4 +62,8 @@ unsigned int aperfmperf_get_khz(int cpu);
>  
>  extern void x86_spec_ctrl_setup_ap(void);
>  
> +extern void tsx_init(struct cpuinfo_x86 *c);
> +
> +extern u64 read_ia32_arch_cap(void);
> +
>  #endif /* ARCH_X86_CPU_H */
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 8d6d92ebeb54..b1d6c96f6b88 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -761,6 +761,8 @@ static void init_intel(struct cpuinfo_x86 *c)
>  		detect_tme(c);
>  
>  	init_intel_misc_features(c);
> +
> +	tsx_init(c);
>  }
>  
>  #ifdef CONFIG_X86_32
> diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
> new file mode 100644
> index 000000000000..c549750dd7c8
> --- /dev/null
> +++ b/arch/x86/kernel/cpu/tsx.c
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Intel Transactional Synchronization Extensions (TSX) control.
> + *
> + * Copyright (C) 2019 Intel Corporation
> + *
> + * Author:
> + *	Pawan Gupta <[email protected]>
> + */
> +
> +#include <linux/processor.h>
> +#include <linux/cpufeature.h>
> +
> +#include "cpu.h"
> +
> +static enum tsx_ctrl_states {
> +	TSX_CTRL_ENABLE,
> +	TSX_CTRL_DISABLE,
> +	TSX_CTRL_NOT_SUPPORTED,
> +} tsx_ctrl_state = TSX_CTRL_NOT_SUPPORTED;
> +
> +static void tsx_disable(void)
> +{
> +	u64 tsx;
> +
> +	rdmsrl(MSR_IA32_TSX_CTRL, tsx);
> +
> +	/* Force all transactions to immediately abort */
> +	tsx |= TSX_CTRL_RTM_DISABLE;
> +	/*
> +	 * Ensure TSX support is not enumerated in CPUID.
> +	 * This is visible to userspace and will ensure they
> +	 * do not waste resources trying TSX transactions that
> +	 * will always abort.
> +	 */
> +	tsx |= TSX_CTRL_CPUID_CLEAR;
> +
> +	wrmsrl(MSR_IA32_TSX_CTRL, tsx);
> +}
> +
> +static bool tsx_ctrl_is_supported(void)
> +{
> +	u64 ia32_cap = read_ia32_arch_cap();
> +
> +	/*
> +	 * TSX is controlled via MSR_IA32_TSX_CTRL.  However,
> +	 * support for this MSR is enumerated by ARCH_CAP_TSX_MSR bit
> +	 * in MSR_IA32_ARCH_CAPABILITIES.
> +	 */
> +	return !!(ia32_cap & ARCH_CAP_TSX_CTRL_MSR);
> +}
> +
> +void tsx_init(struct cpuinfo_x86 *c)
> +{
> +	if (!tsx_ctrl_is_supported())
> +		return;
> +
> +	/*
> +	 * Default to TSX_CTRL_DISABLE. This is because on certain processors
> +	 * TSX may be used as part of a speculative side channel attack.
> +	 */
> +	tsx_ctrl_state = TSX_CTRL_DISABLE;
> +
> +	tsx_disable();
> +	/*
> +	 * tsx_disable() will change the state of the
> +	 * RTM CPUID bit.  Clear it here since it is now
> +	 * expected to be not set.
> +	 */
> +	clear_cpu_cap(c, X86_FEATURE_RTM);
> +	setup_clear_cpu_cap(X86_FEATURE_RTM);

You shouldn't be doing both clear_cpu_cap *and* setup_clear_cpu_cap().

Also, please fold patch 4 into this patch or at least make the contents
of tsx_init() in this patch make some sense.