| From b284909abad48b07d3071a9fc9b5692b3e64914b Mon Sep 17 00:00:00 2001 |
| From: Josh Poimboeuf <jpoimboe@redhat.com> |
| Date: Wed, 30 Jan 2019 07:13:58 -0600 |
| Subject: cpu/hotplug: Fix "SMT disabled by BIOS" detection for KVM |
| |
| From: Josh Poimboeuf <jpoimboe@redhat.com> |
| |
| commit b284909abad48b07d3071a9fc9b5692b3e64914b upstream. |
| |
| With the following commit: |
| |
| 73d5e2b47264 ("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: |
| |
| 73d5e2b47264 ("cpu/hotplug: detect SMT disabled by BIOS") |
| bc2d8d262cba ("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: 73d5e2b47264 ("cpu/hotplug: detect SMT disabled by BIOS") |
| Reported-by: Igor Mammedov <imammedo@redhat.com> |
| Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> |
| Signed-off-by: Thomas 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 |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| |
| --- |
| arch/x86/kernel/cpu/bugs.c | 2 +- |
| arch/x86/kvm/vmx.c | 3 ++- |
| include/linux/cpu.h | 2 -- |
| kernel/cpu.c | 33 ++++----------------------------- |
| kernel/sched/fair.c | 1 + |
| kernel/smp.c | 2 -- |
| 6 files changed, 8 insertions(+), 35 deletions(-) |
| |
| --- a/arch/x86/kernel/cpu/bugs.c |
| +++ b/arch/x86/kernel/cpu/bugs.c |
| @@ -68,7 +68,7 @@ void __init check_bugs(void) |
| * identify_boot_cpu() initialized SMT support information, let the |
| * core code know. |
| */ |
| - cpu_smt_check_topology_early(); |
| + cpu_smt_check_topology(); |
| |
| if (!IS_ENABLED(CONFIG_SMP)) { |
| pr_info("CPU: "); |
| --- a/arch/x86/kvm/vmx.c |
| +++ b/arch/x86/kvm/vmx.c |
| @@ -27,6 +27,7 @@ |
| #include <linux/mm.h> |
| #include <linux/highmem.h> |
| #include <linux/sched.h> |
| +#include <linux/sched/smt.h> |
| #include <linux/moduleparam.h> |
| #include <linux/mod_devicetable.h> |
| #include <linux/trace_events.h> |
| @@ -10120,7 +10121,7 @@ static int vmx_vm_init(struct kvm *kvm) |
| * Warn upon starting the first VM in a potentially |
| * insecure environment. |
| */ |
| - if (cpu_smt_control == CPU_SMT_ENABLED) |
| + if (sched_smt_active()) |
| pr_warn_once(L1TF_MSG_SMT); |
| if (l1tf_vmx_mitigation == VMENTER_L1D_FLUSH_NEVER) |
| pr_warn_once(L1TF_MSG_L1D); |
| --- a/include/linux/cpu.h |
| +++ b/include/linux/cpu.h |
| @@ -188,12 +188,10 @@ enum cpuhp_smt_control { |
| #if defined(CONFIG_SMP) && defined(CONFIG_HOTPLUG_SMT) |
| extern enum cpuhp_smt_control cpu_smt_control; |
| extern void cpu_smt_disable(bool force); |
| -extern void cpu_smt_check_topology_early(void); |
| extern void cpu_smt_check_topology(void); |
| #else |
| # define cpu_smt_control (CPU_SMT_ENABLED) |
| 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) { } |
| #endif |
| |
| --- a/kernel/cpu.c |
| +++ b/kernel/cpu.c |
| @@ -356,9 +356,6 @@ void __weak arch_smt_update(void) { } |
| |
| #ifdef CONFIG_HOTPLUG_SMT |
| 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) |
| { |
| @@ -376,25 +373,11 @@ void __init cpu_smt_disable(bool force) |
| |
| /* |
| * The decision whether SMT is supported can only be done after the full |
| - * CPU identification. Called from architecture code before non boot CPUs |
| - * 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. |
| + * CPU identification. Called from architecture code. |
| */ |
| void __init cpu_smt_check_topology(void) |
| { |
| - if (!cpu_smt_available) |
| + if (!topology_smt_supported()) |
| cpu_smt_control = CPU_SMT_NOT_SUPPORTED; |
| } |
| |
| @@ -407,18 +390,10 @@ early_param("nosmt", smt_cmdline_disable |
| |
| static inline bool cpu_smt_allowed(unsigned int cpu) |
| { |
| - if (topology_is_primary_thread(cpu)) |
| + if (cpu_smt_control == CPU_SMT_ENABLED) |
| return true; |
| |
| - /* |
| - * 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) |
| + if (topology_is_primary_thread(cpu)) |
| return true; |
| |
| /* |
| --- a/kernel/sched/fair.c |
| +++ b/kernel/sched/fair.c |
| @@ -5651,6 +5651,7 @@ find_idlest_cpu(struct sched_group *grou |
| |
| #ifdef CONFIG_SCHED_SMT |
| DEFINE_STATIC_KEY_FALSE(sched_smt_present); |
| +EXPORT_SYMBOL_GPL(sched_smt_present); |
| |
| static inline void set_idle_cores(int cpu, int val) |
| { |
| --- a/kernel/smp.c |
| +++ b/kernel/smp.c |
| @@ -584,8 +584,6 @@ void __init smp_init(void) |
| num_nodes, (num_nodes > 1 ? "s" : ""), |
| num_cpus, (num_cpus > 1 ? "s" : "")); |
| |
| - /* Final decision about SMT support */ |
| - cpu_smt_check_topology(); |
| /* Any cleanup work */ |
| smp_cpus_done(setup_max_cpus); |
| } |