Re: [PATCH] KVM breaks CPU hotplug

Avi Kivity
2007-03-26T09:11:08Z

Shaohua Li wrote:
> When testing CPU hotplug, I found cpu can't be onlined with kvm enabled
> sometimes. The reason is smp_call_function_single is a nop if the thread
> is running on the target cpu. I think CPU_ONLINE case doesn't require
> the fix as the online CPU isn't plugged into sheduler yet.
>
>

I think this is not enough, because:

- this path is preemptible code, so the test (this_cpu == cpu) can run
one on cpu and execute later code on another
- a virtual machine might be scheduled later on this cpu and oops when
executing a vmx/svm instruction

I'm not sure how to fix. Perhaps have _cpu_down() remove the cpu it is
downing from its affinity mask earlier? the code is already there, a
few lines below.

> Signed-off-by: Shaohua Li <shaohua.li-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
> diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
> index dc7a8c7..806a931 100644
> --- a/drivers/kvm/kvm_main.c
> +++ b/drivers/kvm/kvm_main.c
> @@ -2360,6 +2360,7 @@ static int kvm_cpu_hotplug(struct notifier_block *notifier, unsigned long val,
> void *v)
> {
> int cpu = (long)v;
> + int this_cpu;
>
> switch (val) {
> case CPU_DOWN_PREPARE:
> @@ -2367,8 +2368,13 @@ static int kvm_cpu_hotplug(struct notifier_block *notifier, unsigned long val,
> printk(KERN_INFO "kvm: disabling virtualization on CPU%d\n",
> cpu);
> decache_vcpus_on_cpu(cpu);
> - smp_call_function_single(cpu, kvm_arch_ops->hardware_disable,
> - NULL, 0, 1);
> + this_cpu = get_cpu();
> + if (this_cpu == cpu)
> + kvm_arch_ops->hardware_disable(NULL);
> + else
> + smp_call_function_single(cpu,
> + kvm_arch_ops->hardware_disable, NULL, 0, 1);
> + put_cpu();
> break;
> case CPU_ONLINE:
> printk(KERN_INFO "kvm: enabling virtualization on CPU%d\n",
>

--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.