[MODERATED] Re: ***UNCHECKED*** Re: [PATCH v5 08/11] TAAv5 8

[Michal Hocko <mhocko@suse.com>]

On Tue 15-10-19 10:01:40, speck for Thomas Gleixner wrote:
> On Mon, 14 Oct 2019, speck for Jiri Kosina wrote:
> 
> > On Mon, 14 Oct 2019, speck for Borislav Petkov wrote:
> > 
> > > > 	not everybody (by far) is reading those. If this regressess on a=20
> > > > 	handful of platforms, we (together with the OS vendor) could=20
> > > > 	probably handle the followup reports, but if it happens all across=20
> > > > 	the board, it'd surely be overwhelming, to put it mildly. It'll"
> > > > 	basically promptly escalate to a support disaster."
> 
> I feared that :)
> 
> > > Ok, how about we make it a config option: CONFIG_TSX_DEFAULT_ON
> > >
> > > and we - distro kernels - enable it. Mainline leaves it off.
> > > 
> > > Problem solved.
> > > 
> > > Hmmm?
> > 
> > If noone would strongly object to that config option, I'll happily send a 
> > followup patch on top of Pawan's series that'd be adding it (defaulting to 
> > 'n', as that appears to be the common consensus for upstream for totally 
> > understandable reasons).
> > 
> > Otherwise, some distros might just have to flip the default in the sources 
> > by non-upstream patch, I am afraid (even though we haven't made it to the 
> > point where are are making that decision yet).
> 
> The config option is fine.

OK, so what about this patch on top of Pawan's series? I have to say I am not
really entirely happy about yet another config option. In principle this
is not much different from the HT where we decided to stay enabled even
though it is vulnerable to side channels. But I do understand that much
more people will notice HT off than TSX off.

Anyway here is the patch
---
From 9666e91b63cd6213d362d04289e1bcbbe2050bc3 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Tue, 15 Oct 2019 11:21:01 +0200
Subject: [PATCH] x86, tsx: allow to set tsx=auto by a config option

There is a general consensus that TSX usage is not largely spread while
the history shows there is a non trivial space for side channel attacks
possible. Therefore the tsx is disabled by default even on platforms
that might have a safe implementation of TSX according to the current
knowledge. This is a fair trade off to make.

There are, however, workloads that really do benefit from using TSX and
updating to a newer kernel with TSX disabled might introduce a
noticeable regressions. This would be especially a problem for Linux
distributions which will provide TAA mitigations.

Introduce X86_INTEL_ENABLE_SAFE_TSX config option to override the
default tsx=off semantic and make tsx=auto a default which is more
update friendly.

Suggested-by: Borislav Petkov <bpetkov@suse.de>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 arch/x86/Kconfig          | 22 ++++++++++++++++++++++
 arch/x86/kernel/cpu/tsx.c | 20 ++++++++++++++------
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d6e1faa28c58..9823e34b81ce 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1940,6 +1940,28 @@ config X86_INTEL_MEMORY_PROTECTION_KEYS
 
 	  If unsure, say y.
 
+config X86_INTEL_ENABLE_SAFE_TSX
+	prompt ""
+	def_bool n
+	depends on CPU_SUP_INTEL
+	---help---
+	  Intel's TSX (Transactional Synchronization Extensions) feature
+	  allows to optimize locking protocols through lock elision which
+	  can lead to a noticeable performance boost.
+
+	  On the other hand it has been shown that TSX can be exploited
+	  to form side channel attacks (e.g. TAA) and chances are there
+	  will be more of those attacks discovered in the future.
+
+	  Therefore the TSX is not enabled by default. An admin might override
+	  this decision by tsx=on command line parameter. This has a risk that
+	  TSX will get enabled also on platforms which are known to be vulnerable
+	  to attacks like TAA and a safer option is to use tsx=auto command line
+	  parameter. Enabling this config option will make tsx=auto the default.
+	  See Documentation/admin-guide/kernel-parameters.txt for more details.
+
+	  If you really benefit from TSX then enable this option, otherwise say n.
+
 config EFI
 	bool "EFI runtime service support"
 	depends on ACPI
diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
index 96320449abb7..d3dc1ce5cd4b 100644
--- a/arch/x86/kernel/cpu/tsx.c
+++ b/arch/x86/kernel/cpu/tsx.c
@@ -69,6 +69,14 @@ static bool __init tsx_ctrl_is_supported(void)
 	return !!(ia32_cap & ARCH_CAP_TSX_CTRL_MSR);
 }
 
+static enum tsx_ctrl_states x86_safe_tsx_mode(void)
+{
+	if (boot_cpu_has_bug(X86_BUG_TAA))
+		return TSX_CTRL_DISABLE;
+
+	return TSX_CTRL_ENABLE;
+}
+
 void __init tsx_init(void)
 {
 	char arg[20];
@@ -84,17 +92,17 @@ void __init tsx_init(void)
 		} else if (!strcmp(arg, "off")) {
 			tsx_ctrl_state = TSX_CTRL_DISABLE;
 		} else if (!strcmp(arg, "auto")) {
-			if (boot_cpu_has_bug(X86_BUG_TAA))
-				tsx_ctrl_state = TSX_CTRL_DISABLE;
-			else
-				tsx_ctrl_state = TSX_CTRL_ENABLE;
+			tsx_ctrl_state = x86_safe_tsx_mode();
 		} else {
 			tsx_ctrl_state = TSX_CTRL_DISABLE;
 			pr_info("tsx: invalid option, defaulting to off\n");
 		}
 	} else {
-		/* tsx= not provided, defaulting to off */
-		tsx_ctrl_state = TSX_CTRL_DISABLE;
+		/* tsx= not provided */
+		if (IS_ENABLED(X86_INTEL_ENABLE_SAFE_TSX))
+			tsx_ctrl_state = x86_safe_tsx_mode();
+		else
+			tsx_ctrl_state = TSX_CTRL_DISABLE;
 	}
 
 	if (tsx_ctrl_state == TSX_CTRL_DISABLE) {
-- 
2.20.1

-- 
Michal Hocko
SUSE Labs