Commit b284909a authored by Josh Poimboeuf's avatar Josh Poimboeuf Committed by Thomas Gleixner

cpu/hotplug: Fix "SMT disabled by BIOS" detection for KVM

With the following commit:

  73d5e2b4 ("cpu/hotplug: detect SMT disabled by BIOS")

... the hotplug code attempted to detect when SMT was disabled by BIOS,
in which case it reported SMT as permanently disabled.  However, that
code broke a virt hotplug scenario, where the guest is booted with only
primary CPU threads, and a sibling is brought online later.

The problem is that there doesn't seem to be a way to reliably
distinguish between the HW "SMT disabled by BIOS" case and the virt
"sibling not yet brought online" case.  So the above-mentioned commit
was a bit misguided, as it permanently disabled SMT for both cases,
preventing future virt sibling hotplugs.

Going back and reviewing the original problems which were attempted to
be solved by that commit, when SMT was disabled in BIOS:

  1) /sys/devices/system/cpu/smt/control showed "on" instead of
     "notsupported"; and

  2) vmx_vm_init() was incorrectly showing the L1TF_MSG_SMT warning.

I'd propose that we instead consider #1 above to not actually be a
problem.  Because, at least in the virt case, it's possible that SMT
wasn't disabled by BIOS and a sibling thread could be brought online
later.  So it makes sense to just always default the smt control to "on"
to allow for that possibility (assuming cpuid indicates that the CPU
supports SMT).

The real problem is #2, which has a simple fix: change vmx_vm_init() to
query the actual current SMT state -- i.e., whether any siblings are
currently online -- instead of looking at the SMT "control" sysfs value.

So fix it by:

  a) reverting the original "fix" and its followup fix:

     73d5e2b4 ("cpu/hotplug: detect SMT disabled by BIOS")
     bc2d8d26 ("cpu/hotplug: Fix SMT supported evaluation")

     and

  b) changing vmx_vm_init() to query the actual current SMT state --
     instead of the sysfs control value -- to determine whether the L1TF
     warning is needed.  This also requires the 'sched_smt_present'
     variable to exported, instead of 'cpu_smt_control'.

Fixes: 73d5e2b4 ("cpu/hotplug: detect SMT disabled by BIOS")
Reported-by: default avatarIgor Mammedov <imammedo@redhat.com>
Signed-off-by: default avatarJosh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
Cc: Joe Mario <jmario@redhat.com>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: kvm@vger.kernel.org
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/e3a85d585da28cc333ecbc1e78ee9216e6da9396.1548794349.git.jpoimboe@redhat.com
parent 34d66caf
...@@ -71,7 +71,7 @@ void __init check_bugs(void) ...@@ -71,7 +71,7 @@ void __init check_bugs(void)
* identify_boot_cpu() initialized SMT support information, let the * identify_boot_cpu() initialized SMT support information, let the
* core code know. * core code know.
*/ */
cpu_smt_check_topology_early(); cpu_smt_check_topology();
if (!IS_ENABLED(CONFIG_SMP)) { if (!IS_ENABLED(CONFIG_SMP)) {
pr_info("CPU: "); pr_info("CPU: ");
......
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
#include <linux/mod_devicetable.h> #include <linux/mod_devicetable.h>
#include <linux/mm.h> #include <linux/mm.h>
#include <linux/sched.h> #include <linux/sched.h>
#include <linux/sched/smt.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/tboot.h> #include <linux/tboot.h>
#include <linux/trace_events.h> #include <linux/trace_events.h>
...@@ -6823,7 +6824,7 @@ static int vmx_vm_init(struct kvm *kvm) ...@@ -6823,7 +6824,7 @@ static int vmx_vm_init(struct kvm *kvm)
* Warn upon starting the first VM in a potentially * Warn upon starting the first VM in a potentially
* insecure environment. * insecure environment.
*/ */
if (cpu_smt_control == CPU_SMT_ENABLED) if (sched_smt_active())
pr_warn_once(L1TF_MSG_SMT); pr_warn_once(L1TF_MSG_SMT);
if (l1tf_vmx_mitigation == VMENTER_L1D_FLUSH_NEVER) if (l1tf_vmx_mitigation == VMENTER_L1D_FLUSH_NEVER)
pr_warn_once(L1TF_MSG_L1D); pr_warn_once(L1TF_MSG_L1D);
......
...@@ -180,12 +180,10 @@ enum cpuhp_smt_control { ...@@ -180,12 +180,10 @@ enum cpuhp_smt_control {
#if defined(CONFIG_SMP) && defined(CONFIG_HOTPLUG_SMT) #if defined(CONFIG_SMP) && defined(CONFIG_HOTPLUG_SMT)
extern enum cpuhp_smt_control cpu_smt_control; extern enum cpuhp_smt_control cpu_smt_control;
extern void cpu_smt_disable(bool force); extern void cpu_smt_disable(bool force);
extern void cpu_smt_check_topology_early(void);
extern void cpu_smt_check_topology(void); extern void cpu_smt_check_topology(void);
#else #else
# define cpu_smt_control (CPU_SMT_ENABLED) # define cpu_smt_control (CPU_SMT_ENABLED)
static inline void cpu_smt_disable(bool force) { } static inline void cpu_smt_disable(bool force) { }
static inline void cpu_smt_check_topology_early(void) { }
static inline void cpu_smt_check_topology(void) { } static inline void cpu_smt_check_topology(void) { }
#endif #endif
......
...@@ -376,9 +376,6 @@ void __weak arch_smt_update(void) { } ...@@ -376,9 +376,6 @@ void __weak arch_smt_update(void) { }
#ifdef CONFIG_HOTPLUG_SMT #ifdef CONFIG_HOTPLUG_SMT
enum cpuhp_smt_control cpu_smt_control __read_mostly = CPU_SMT_ENABLED; enum cpuhp_smt_control cpu_smt_control __read_mostly = CPU_SMT_ENABLED;
EXPORT_SYMBOL_GPL(cpu_smt_control);
static bool cpu_smt_available __read_mostly;
void __init cpu_smt_disable(bool force) void __init cpu_smt_disable(bool force)
{ {
...@@ -397,25 +394,11 @@ void __init cpu_smt_disable(bool force) ...@@ -397,25 +394,11 @@ void __init cpu_smt_disable(bool force)
/* /*
* The decision whether SMT is supported can only be done after the full * The decision whether SMT is supported can only be done after the full
* CPU identification. Called from architecture code before non boot CPUs * CPU identification. Called from architecture code.
* are brought up.
*/
void __init cpu_smt_check_topology_early(void)
{
if (!topology_smt_supported())
cpu_smt_control = CPU_SMT_NOT_SUPPORTED;
}
/*
* If SMT was disabled by BIOS, detect it here, after the CPUs have been
* brought online. This ensures the smt/l1tf sysfs entries are consistent
* with reality. cpu_smt_available is set to true during the bringup of non
* boot CPUs when a SMT sibling is detected. Note, this may overwrite
* cpu_smt_control's previous setting.
*/ */
void __init cpu_smt_check_topology(void) void __init cpu_smt_check_topology(void)
{ {
if (!cpu_smt_available) if (!topology_smt_supported())
cpu_smt_control = CPU_SMT_NOT_SUPPORTED; cpu_smt_control = CPU_SMT_NOT_SUPPORTED;
} }
...@@ -428,18 +411,10 @@ early_param("nosmt", smt_cmdline_disable); ...@@ -428,18 +411,10 @@ early_param("nosmt", smt_cmdline_disable);
static inline bool cpu_smt_allowed(unsigned int cpu) static inline bool cpu_smt_allowed(unsigned int cpu)
{ {
if (topology_is_primary_thread(cpu)) if (cpu_smt_control == CPU_SMT_ENABLED)
return true; return true;
/* if (topology_is_primary_thread(cpu))
* If the CPU is not a 'primary' thread and the booted_once bit is
* set then the processor has SMT support. Store this information
* for the late check of SMT support in cpu_smt_check_topology().
*/
if (per_cpu(cpuhp_state, cpu).booted_once)
cpu_smt_available = true;
if (cpu_smt_control == CPU_SMT_ENABLED)
return true; return true;
/* /*
......
...@@ -5980,6 +5980,7 @@ static inline int find_idlest_cpu(struct sched_domain *sd, struct task_struct *p ...@@ -5980,6 +5980,7 @@ static inline int find_idlest_cpu(struct sched_domain *sd, struct task_struct *p
#ifdef CONFIG_SCHED_SMT #ifdef CONFIG_SCHED_SMT
DEFINE_STATIC_KEY_FALSE(sched_smt_present); DEFINE_STATIC_KEY_FALSE(sched_smt_present);
EXPORT_SYMBOL_GPL(sched_smt_present);
static inline void set_idle_cores(int cpu, int val) static inline void set_idle_cores(int cpu, int val)
{ {
......
...@@ -584,8 +584,6 @@ void __init smp_init(void) ...@@ -584,8 +584,6 @@ void __init smp_init(void)
num_nodes, (num_nodes > 1 ? "s" : ""), num_nodes, (num_nodes > 1 ? "s" : ""),
num_cpus, (num_cpus > 1 ? "s" : "")); num_cpus, (num_cpus > 1 ? "s" : ""));
/* Final decision about SMT support */
cpu_smt_check_topology();
/* Any cleanup work */ /* Any cleanup work */
smp_cpus_done(setup_max_cpus); smp_cpus_done(setup_max_cpus);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment