[MODERATED] Re: [PATCH v7 03/10] TAAv7 3

[Josh Poimboeuf <jpoimboe@redhat.com>]

On Mon, Oct 21, 2019 at 01:25:02PM -0700, speck for Pawan Gupta wrote:
> From: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> Subject: [PATCH v7 03/10] x86/tsx: Add TSX cmdline option with TSX disabled by
>  default
> 
> Add kernel cmdline parameter "tsx" to control the Transactional
> Synchronization Extensions (TSX) feature.  On CPUs that support TSX
> control, use "tsx=on|off" to enable or disable TSX.  Not specifying this
> option is equivalent to "tsx=off". This is because on certain processors
> TSX may be used as a part of a speculative side channel attack.
> 
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> Reviewed-by: Mark Gross <mgross@linux.intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Tested-by: Neelima Krishnan <neelima.krishnan@intel.com>
> ---
>  .../admin-guide/kernel-parameters.txt         |  11 ++
>  arch/x86/kernel/cpu/Makefile                  |   2 +-
>  arch/x86/kernel/cpu/common.c                  |   2 +
>  arch/x86/kernel/cpu/cpu.h                     |  18 +++
>  arch/x86/kernel/cpu/intel.c                   |   5 +
>  arch/x86/kernel/cpu/tsx.c                     | 114 ++++++++++++++++++
>  6 files changed, 151 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/kernel/cpu/tsx.c
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index a84a83f8881e..ad6b69057bb0 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4848,6 +4848,17 @@
>  			interruptions from clocksource watchdog are not
>  			acceptable).
>  
> +	tsx=		[X86] Control Transactional Synchronization
> +			Extensions (TSX) feature in Intel processors that
> +			support TSX control.
> +
> +			This parameter controls the TSX feature. The options are:
> +
> +			on	- Enable TSX on the system.
> +			off	- Disable TSX on the system.
> +
> +			Not specifying this option is equivalent to tsx=off.

In addition to the previous comments about clarifying the
functionalities of 'off' and 'auto', I think this really needs to
describe the risks associated with 'on' and 'auto', so the user can have
more guidance about what to do.

It should state that while there are mitigations for all known issues
(i.e., the tsx_async_abort= option), TSX has been known to be an
accelerator for several previous speculation-related CVEs, and so there
may be unknown security risks associated with leaving it enabled.

-- 
Josh