Skip to content

Commit 8547cfe

Browse files
yamahatabonzini
authored andcommitted
KVM: Make hardware_enable_failed a local variable in the "enable all" path
Rework detecting hardware enabling errors to use a local variable in the "enable all" path to track whether or not enabling was successful across all CPUs. Using a global variable complicates paths that enable hardware only on the current CPU, e.g. kvm_resume() and kvm_online_cpu(). Opportunistically add a WARN if hardware enabling fails during kvm_resume(), KVM is all kinds of hosed if CPU0 fails to enable hardware. The WARN is largely futile in the current code, as KVM BUG()s on spurious faults on VMX instructions, e.g. attempting to run a vCPU on CPU if hardware enabling fails will explode. ------------[ cut here ]------------ kernel BUG at arch/x86/kvm/x86.c:508! invalid opcode: 0000 [#1] SMP CPU: 3 PID: 1009 Comm: CPU 4/KVM Not tainted 6.1.0-rc1+ #11 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 RIP: 0010:kvm_spurious_fault+0xa/0x10 Call Trace: vmx_vcpu_load_vmcs+0x192/0x230 [kvm_intel] vmx_vcpu_load+0x16/0x60 [kvm_intel] kvm_arch_vcpu_load+0x32/0x1f0 vcpu_load+0x2f/0x40 kvm_arch_vcpu_ioctl_run+0x19/0x9d0 kvm_vcpu_ioctl+0x271/0x660 __x64_sys_ioctl+0x80/0xb0 do_syscall_64+0x2b/0x50 entry_SYSCALL_64_after_hwframe+0x46/0xb0 But, the WARN may provide a breadcrumb to understand what went awry, and someday KVM may fix one or both of those bugs, e.g. by finding a way to eat spurious faults no matter the context (easier said than done due to side effects of certain operations, e.g. Intel's VMCLEAR). Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> [sean: rebase, WARN on failure in kvm_resume()] Signed-off-by: Sean Christopherson <seanjc@google.com> Message-Id: <20221130230934.1014142-48-seanjc@google.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
1 parent 1e567e4 commit 8547cfe

File tree

1 file changed

+16
-19
lines changed

1 file changed

+16
-19
lines changed

virt/kvm/kvm_main.c

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ LIST_HEAD(vm_list);
104104

105105
static DEFINE_PER_CPU(bool, hardware_enabled);
106106
static int kvm_usage_count;
107-
static atomic_t hardware_enable_failed;
108107

109108
static struct kmem_cache *kvm_vcpu_cache;
110109

@@ -5062,19 +5061,25 @@ static struct miscdevice kvm_dev = {
50625061
&kvm_chardev_ops,
50635062
};
50645063

5065-
static void hardware_enable_nolock(void *junk)
5064+
static int __hardware_enable_nolock(void)
50665065
{
50675066
if (__this_cpu_read(hardware_enabled))
5068-
return;
5067+
return 0;
50695068

50705069
if (kvm_arch_hardware_enable()) {
5071-
atomic_inc(&hardware_enable_failed);
50725070
pr_info("kvm: enabling virtualization on CPU%d failed\n",
50735071
raw_smp_processor_id());
5074-
return;
5072+
return -EIO;
50755073
}
50765074

50775075
__this_cpu_write(hardware_enabled, true);
5076+
return 0;
5077+
}
5078+
5079+
static void hardware_enable_nolock(void *failed)
5080+
{
5081+
if (__hardware_enable_nolock())
5082+
atomic_inc(failed);
50785083
}
50795084

50805085
static int kvm_online_cpu(unsigned int cpu)
@@ -5087,16 +5092,8 @@ static int kvm_online_cpu(unsigned int cpu)
50875092
* errors when scheduled to this CPU.
50885093
*/
50895094
mutex_lock(&kvm_lock);
5090-
if (kvm_usage_count) {
5091-
WARN_ON_ONCE(atomic_read(&hardware_enable_failed));
5092-
5093-
hardware_enable_nolock(NULL);
5094-
5095-
if (atomic_read(&hardware_enable_failed)) {
5096-
atomic_set(&hardware_enable_failed, 0);
5097-
ret = -EIO;
5098-
}
5099-
}
5095+
if (kvm_usage_count)
5096+
ret = __hardware_enable_nolock();
51005097
mutex_unlock(&kvm_lock);
51015098
return ret;
51025099
}
@@ -5144,6 +5141,7 @@ static void hardware_disable_all(void)
51445141

51455142
static int hardware_enable_all(void)
51465143
{
5144+
atomic_t failed = ATOMIC_INIT(0);
51475145
int r = 0;
51485146

51495147
/*
@@ -5159,10 +5157,9 @@ static int hardware_enable_all(void)
51595157

51605158
kvm_usage_count++;
51615159
if (kvm_usage_count == 1) {
5162-
atomic_set(&hardware_enable_failed, 0);
5163-
on_each_cpu(hardware_enable_nolock, NULL, 1);
5160+
on_each_cpu(hardware_enable_nolock, &failed, 1);
51645161

5165-
if (atomic_read(&hardware_enable_failed)) {
5162+
if (atomic_read(&failed)) {
51665163
hardware_disable_all_nolock();
51675164
r = -EBUSY;
51685165
}
@@ -5796,7 +5793,7 @@ static void kvm_resume(void)
57965793
lockdep_assert_irqs_disabled();
57975794

57985795
if (kvm_usage_count)
5799-
hardware_enable_nolock(NULL);
5796+
WARN_ON_ONCE(__hardware_enable_nolock());
58005797
}
58015798

58025799
static struct syscore_ops kvm_syscore_ops = {

0 commit comments

Comments
 (0)