From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linutronix.de (146.0.238.70:993) by crypto-ml.lab.linutronix.de with IMAP4-SSL for ; 22 Jan 2019 04:34:04 -0000 Received: from aserp2130.oracle.com ([141.146.126.79]) by Galois.linutronix.de with esmtps (TLS1.2:RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1glnlD-0003cc-CG for speck@linutronix.de; Tue, 22 Jan 2019 05:34:03 +0100 Received: from pps.filterd (aserp2130.oracle.com [127.0.0.1]) by aserp2130.oracle.com (8.16.0.22/8.16.0.22) with SMTP id x0M4Xi5N035252 for ; Tue, 22 Jan 2019 04:33:56 GMT Received: from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74]) by aserp2130.oracle.com with ESMTP id 2q3sde9fyj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Tue, 22 Jan 2019 04:33:56 +0000 Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id x0M4XswP001310 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Tue, 22 Jan 2019 04:33:55 GMT Received: from abhmp0001.oracle.com (abhmp0001.oracle.com [141.146.116.7]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id x0M4XstF029311 for ; Tue, 22 Jan 2019 04:33:54 GMT Date: Mon, 21 Jan 2019 23:33:53 -0500 From: Konrad Rzeszutek Wilk Subject: [MODERATED] Re: [PATCH v5 04/27] MDSv5 15 Message-ID: <20190122043353.GF12859@char.us.oracle.com> References: <21244f88a3fad17a0bdbc48e085083590bb31ab0.1547858934.git.ak@linux.intel.com> MIME-Version: 1.0 In-Reply-To: <21244f88a3fad17a0bdbc48e085083590bb31ab0.1547858934.git.ak@linux.intel.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Fri, Jan 18, 2019 at 04:50:19PM -0800, speck for Andi Kleen wrote: > From: Andi Kleen > Subject: x86/speculation/mds: Support mds=full > > Support a new command line option to support unconditional flushing > on each kernel exit. This is not enabled by default. > > Signed-off-by: Andi Kleen > > --- > > v2: Don't enable mds=full for MDS_NO because it will be a nop. > --- > Documentation/admin-guide/kernel-parameters.txt | 5 +++++ > arch/x86/entry/common.c | 7 ++++++- > arch/x86/include/asm/clearcpu.h | 2 ++ > arch/x86/kernel/cpu/bugs.c | 5 +++++ > 4 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index 9c967d0caeca..5f5a8808c475 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -2360,6 +2360,11 @@ > mds=off [X86, Intel] > Disable workarounds for Micro-architectural Data Sampling. > > + mds=full [X86, Intel] > + Always flush cpu buffers when exiting kernel for MDS. .. which implies that the microcode must be loaded. But right now you could do 'mds=full' on a machine _without_ the microcode and it would just do 'verw'. And that unpatched 'verw' would most certainly _not_ flush CPU buffers. See below in mds_select_mitigation > + Normally the kernel decides dynamically when flushing is > + needed or not. Can you follow the same standard as 'ssbd' and 'l1tf' - which is that this turns in 'mds=[off,full] and then each one has an explanation please? > + > mem=nn[KMG] [KNL,BOOT] Force usage of a specific amount of memory > Amount of memory to be used when the kernel is not able > to see the whole system memory or for test. > diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c > index 924f8dab2068..66c08e1d493a 100644 > --- a/arch/x86/entry/common.c > +++ b/arch/x86/entry/common.c > @@ -173,7 +173,9 @@ static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags) > > if (cached_flags & _TIF_CLEAR_CPU) { > clear_thread_flag(TIF_CLEAR_CPU); > - clear_cpu(); > + /* Don't do it twice if forced */ > + if (!static_key_enabled(&force_cpu_clear)) > + clear_cpu(); > } > > /* Disable IRQs and retry */ > @@ -217,6 +219,9 @@ __visible inline void prepare_exit_to_usermode(struct pt_regs *regs) > ti->status &= ~(TS_COMPAT|TS_I386_REGS_POKED); > #endif > > + if (static_key_enabled(&force_cpu_clear)) > + clear_cpu(); > + > user_enter_irqoff(); > } > > diff --git a/arch/x86/include/asm/clearcpu.h b/arch/x86/include/asm/clearcpu.h > index 530ef619ac1b..3b8ee76b9c07 100644 > --- a/arch/x86/include/asm/clearcpu.h > +++ b/arch/x86/include/asm/clearcpu.h > @@ -20,4 +20,6 @@ static inline void clear_cpu(void) > [kernelds] "m" (kernel_ds)); > } > > +DECLARE_STATIC_KEY_FALSE(force_cpu_clear); 'force_cpu_clear' sounds quite vague. As in in three months I will not remember the name of this. Perhaps 'force_verw' ? Or 'force_mds_verw'? > + > #endif > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c > index 2fd8faa7e23a..ce0e367753ff 100644 > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -1061,11 +1061,16 @@ early_param("l1tf", l1tf_cmdline); > > #undef pr_fmt > > +DEFINE_STATIC_KEY_FALSE(force_cpu_clear); > + > static void mds_select_mitigation(void) > { > if (cmdline_find_option_bool(boot_command_line, "mds=off") || > !boot_cpu_has_bug(X86_BUG_MDS)) > setup_force_cpu_cap(X86_FEATURE_NO_VERW); > + if (cmdline_find_option_bool(boot_command_line, "mds=full") && > + boot_cpu_has_bug(X86_BUG_MDS)) > + static_branch_enable(&force_cpu_clear); The 'mds=full' can be done on machines without the new microcode and it sets MDS (twice) and also does 'VERW' without any benefit. Why not make this: if (!boot_cpu_has_bug(X86_BUG_MDS) { setup_force_cpu_cap(X86_FEATURE_NO_VERW); return; } else { if (cmdline_find_option_bool(boot_command_line, "mds=off")) setup_force_cpu_cap(X86_FEATURE_NO_VERW); if (cmdline_find_option_bool(boot_command_line, "mds=full") && boot_cpu_has_bug(X86_BUG_MDS)) static_branch_enable(&force_cpu_clear); } ? > } > > #ifdef CONFIG_SYSFS > -- > 2.17.2