linux-kernel - Re: [Patch v5 08/16] smt: Create cpu_smt_enabled static key for SMT specific code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <439a2d72-988c-d397-dc32-daf098af655c@linux.intel.com>
Date:   Mon, 19 Nov 2018 10:08:53 -0800
From:   Tim Chen <tim.c.chen@...ux.intel.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Jiri Kosina <jikos@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Tom Lendacky <thomas.lendacky@....com>,
        Ingo Molnar <mingo@...hat.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        David Woodhouse <dwmw@...zon.co.uk>,
        Andi Kleen <ak@...ux.intel.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Casey Schaufler <casey.schaufler@...el.com>,
        Asit Mallick <asit.k.mallick@...el.com>,
        Arjan van de Ven <arjan@...ux.intel.com>,
        Jon Masters <jcm@...hat.com>,
        Waiman Long <longman9394@...il.com>,
        linux-kernel@...r.kernel.org, x86@...nel.org
Subject: Re: [Patch v5 08/16] smt: Create cpu_smt_enabled static key for SMT
 specific code

On 11/19/2018 06:57 AM, Peter Zijlstra wrote:
> On Fri, Nov 16, 2018 at 05:53:51PM -0800, Tim Chen wrote:
>> In later code, STIBP will be turned on/off in the context switch code
>> path when SMT is enabled.  Checks for SMT is best
>> avoided on such hot paths.
>>
>> Create cpu_smt_enabled static key to turn on such SMT specific code
>> statically.
> 
> AFAICT this patch only follows the SMT control knob but not the actual
> topology state.
> 
> And, as I previously wrote, we already have sched_smt_present, which is
> supposed to do much the same.
> 
> All you need is the below to make it accurately track the topology.
> 
> ---
> Subject: sched/smt: Make sched_smt_present track topology
> 
> Currently the sched_smt_present static key is only enabled when we
> encounter SMT topology. However there is demand to also disable the key
> when the topology changes such that there is no SMT present anymore.
> 
> Implement this by making the key count the number of cores that have SMT
> enabled.
> 
> In particular, the SMT topology bits are set before we enable
> interrrupts and similarly, are cleared after we disable interrupts for
> the last time and die.


Peter & Thomas,

Any objection if I export sched_smt_present after including
Peter's patch and use it in spec_ctrl_update_msr instead.

Something like this?

Tim

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 943e90d..62fc3af 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -410,8 +410,7 @@ static __always_inline void spec_ctrl_update_msr(unsigned long tifn)
 	 * Need STIBP defense against Spectre v2 attack
 	 * if SMT is in use and enhanced IBRS is unsupported.
 	 */
-	if (static_branch_likely(&cpu_smt_enabled) &&
-	    !static_cpu_has(X86_FEATURE_USE_IBRS_ENHANCED))
+	if (cpu_smt_present() && !static_cpu_has(X86_FEATURE_USE_IBRS_ENHANCED))
 		msr |= stibp_tif_to_spec_ctrl(tifn);
 
 	wrmsrl(MSR_IA32_SPEC_CTRL, msr);
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 3d90155..e3d985e 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -68,6 +68,27 @@ struct device *cpu_device_create(struct device *parent, void *drvdata,
 extern ssize_t arch_cpu_release(const char *, size_t);
 #endif
 
+#ifdef CONFIG_SCHED_SMT
+
+extern struct static_key_false sched_smt_present;
+
+static inline bool cpu_smt_present(void)
+{
+	if (static_branch_unlikely(&sched_smt_present))
+		return true;
+	else
+		return false;
+}
+
+#else
+
+static inline bool cpu_smt_present(void)
+{
+	return false;
+}
+
+#endif
+
 /*
  * These states are not related to the core CPU hotplug mechanism. They are
  * used by various (sub)architectures to track internal state
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 618577f..e1e3f09 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -937,8 +937,6 @@ static inline int cpu_of(struct rq *rq)
 
 #ifdef CONFIG_SCHED_SMT
 
-extern struct static_key_false sched_smt_present;
-
 extern void __update_idle_core(struct rq *rq);
 
 static inline void update_idle_core(struct rq *rq)