Messages in this thread | | | From | Thomas Gleixner <> | Subject | RE: Q. about KVM and CPU hotplug | Date | Wed, 01 Dec 2021 11:30:57 +0100 |
| |
On Wed, Dec 01 2021 at 06:59, Kevin Tian wrote: >> From: Paolo Bonzini <paolo.bonzini@gmail.com> >> It should fail the first vmptrld instruction. It will result in a few >> WARN_ONCE and pr_warn_ratelimited (see vmx_insn_failed). For VMX this >> should be a pretty bad firmware bug, and it has never been reported. >> KVM did find some undocumented errata but not this one! >> > > or it may be caused by incompatible CPU capabilities, which is currently > missing a check in kvm_starting_cpu(). So far the compatibility check is > done only once before registering cpu hotplug state machine: > > for_each_online_cpu(cpu) { > smp_call_function_single(cpu, check_processor_compat, &c, 1); > if (r < 0) > goto out_free_2; > } > > r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_STARTING, "kvm/cpu:starting", > kvm_starting_cpu, kvm_dying_cpu);
Duh. This is silly _and_ broken.
Using for_each_inline_cpu() without holding cpus_read_lock() is racy against concurrent hotplug. But even if the locking is added then nothing prevents a CPU from being plugged _after_ the lock is dropped.
The right solution is to move the hotplug state into the threaded section as I pointed out and do:
r = cpuhp_setup_state(CPUHP_AP_KVM_STARTING, "kvm/cpu:starting", kvm_starting_cpu, kvm_dying_cpu);
which will do the right thing automatically. Checking for compatibility would just be part of the kvm_starting_cpu() callback.
Thanks,
tglx
| |