On Thu, Jul 21, 2011 at 01:06:41PM +0200, Vasilis Liaskovitis wrote: > Hi, > > On Wed, Jul 20, 2011 at 10:35 AM, Gleb Natapov <gleb@xxxxxxxxxx> wrote: > > On Tue, Jul 19, 2011 at 07:40:55PM +0200, Vasilis Liaskovitis wrote: > >> Hello, > >> > >> I have encountered a problem trying to hotplug a CPU in my x86_64 guest setup. > >> > > You do everything right. It's qemu who is buggy. Since qemu need a patch > > for cpu hotplug to not crash it nobody tests it, so code bit rots. > > thanks for your reply. > > As I mentioned in the original email, onlining a hotplugged-cpu with > qemu-kvm/master results in: > > >> echo 1 > /sys/devices/system/cpu/cpu1/online > >> bash: echo: write error: Input/output error > >> > >> in the guest, dmesg reports: > >> > >> [ 2325.376355] Booting Node 0 Processor 1 APIC 0x1 > >> [ 2325.376357] smpboot cpu 1: start_ip = 9a000 > >> [ 2330.821306] CPU1: Not responding. > > I tried to git-bisect between qemu-kvm-0.13.0 (last known version > where cpu hotplug works correctly > for me) and qemu-kvm/master. > > More precisely: To enable cpu-hotplug at each bisect stage, I apply > this patch derived from: > http://lists.gnu.org/archive/html/qemu-devel/2010-08/msg00850.html > > diff --git a/hw/qdev.c b/hw/qdev.c > index 1aa1ea0..aed48ce 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -327,6 +327,7 @@ BusState *sysbus_get_default(void) > if (!main_system_bus) { > main_system_bus = qbus_create(&system_bus_info, NULL, > "main-system-bus"); > + main_system_bus->allow_hotplug = 1; > } > return main_system_bus; > } > > and test cpu hotplug functionality. > The commit that appears to break CPU hotplug is: > Thank you for going through the pain of bisecting it! Bisects that require patch to be applied between each step are especially annoying. > commit f4de8c1451f2265148ff4d895a27e21c0a8788aa > Author: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> > Date: Mon Feb 21 12:28:07 2011 +0100 > qemu-kvm: Mark VCPU state dirty on creation > Jan can you look at this please? > Is it possible that kvm_vcpu_dirty should not be set to 1 for a CPU > that's being hot-plugged? > I.e. when kvm_cpu_exec() is called for the first time during > initialization of a hotplugged-CPU, > we shouldn't try to restore state with kvm_arch_put_registers(). > > The following patch enables hotplug and solves the non-responsive > hotplugged CPU problem, > by not setting the vcpu_dirty state when hotplugging. Enabling hotplug > is still done through > the main_system_bus (see above). > Tested on amd64host/amd64guest combination. Comments? > > Note that there is probably another bug in qemu-kvm/master regarding > acpi-udev event delivery for > a cpu-hotplug event (cpu_set in the qemu_monitor no longer triggers > the event in the guest, see > first issue in my original mail). This patch does not address that issue. > Did this work in qemu-0.13? > Signed-off-by: Vasilis Liaskovitis <vliaskov@xxxxxxxxx> > --- > > hw/acpi_piix4.c | 2 +- > hw/pc.c | 13 +++++++++++-- > hw/qdev.c | 1 + > kvm-all.c | 22 +++++++++++++++++++++- > kvm.h | 3 +++ > target-i386/cpu.h | 2 +- > 6 files changed, 38 insertions(+), 5 deletions(-) > > diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c > index c30a050..b0cac60 100644 > --- a/hw/acpi_piix4.c > +++ b/hw/acpi_piix4.c > @@ -584,7 +584,7 @@ void qemu_system_cpu_hot_add(int cpu, int state) > PIIX4PMState *s = global_piix4_pm_state; > > if (state && !qemu_get_cpu(cpu)) { > - env = pc_new_cpu(global_cpu_model); > + env = pc_new_cpu(global_cpu_model, 1); > if (!env) { > fprintf(stderr, "cpu %d creation failed\n", cpu); > return; > diff --git a/hw/pc.c b/hw/pc.c > index c0a88e1..8cfbf27 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -924,7 +924,7 @@ static void pc_cpu_reset(void *opaque) > env->halted = !cpu_is_bsp(env); > } > > -CPUState *pc_new_cpu(const char *cpu_model) > +CPUState *pc_new_cpu(const char *cpu_model, int hotplugged) > { > CPUState *env; > > @@ -936,7 +936,16 @@ CPUState *pc_new_cpu(const char *cpu_model) > #endif > } > > + if (hotplugged) { > + kvm_set_cpuhotplug_req(); > + } > + > env = cpu_init(cpu_model); > + > + if (hotplugged) { > + kvm_reset_cpuhotplug_req(); > + } > + > if (!env) { > fprintf(stderr, "Unable to find x86 CPU definition\n"); > exit(1); > @@ -956,7 +965,7 @@ void pc_cpus_init(const char *cpu_model) > > /* init CPUs */ > for(i = 0; i < smp_cpus; i++) { > - pc_new_cpu(cpu_model); > + pc_new_cpu(cpu_model, 0); > } > } > > diff --git a/hw/qdev.c b/hw/qdev.c > index 292b52f..f85ac0f 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -327,6 +327,7 @@ BusState *sysbus_get_default(void) > if (!main_system_bus) { > main_system_bus = qbus_create(&system_bus_info, NULL, > "main-system-bus"); > + main_system_bus->allow_hotplug = 1; > } > return main_system_bus; > } > diff --git a/kvm-all.c b/kvm-all.c > index 3ad2459..8aae1d7 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -85,6 +85,7 @@ struct KVMState > #endif > void *used_gsi_bitmap; > int max_gsi; > + int cpu_hotplug_req; > }; > > KVMState *kvm_state; > @@ -220,7 +221,9 @@ int kvm_init_vcpu(CPUState *env) > > env->kvm_fd = ret; > env->kvm_state = s; > - env->kvm_vcpu_dirty = 1; > + if (!kvm_has_cpuhotplug_req()) { > + env->kvm_vcpu_dirty = 1; > + } > > mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0); > if (mmap_size < 0) { > @@ -797,6 +800,8 @@ int kvm_init(void) > > s->pit_in_kernel = kvm_pit; > > + s->cpu_hotplug_req = 0; > + > ret = kvm_arch_init(s); > if (ret < 0) { > goto err; > @@ -1104,6 +1109,21 @@ int kvm_has_vcpu_events(void) > return kvm_state->vcpu_events; > } > > +int kvm_has_cpuhotplug_req(void) > +{ > + return kvm_state->cpu_hotplug_req; > +} > + > +void kvm_set_cpuhotplug_req(void) > +{ > + kvm_state->cpu_hotplug_req = 1; > +} > + > +void kvm_reset_cpuhotplug_req(void) > +{ > + kvm_state->cpu_hotplug_req = 0; > +} > + > int kvm_has_robust_singlestep(void) > { > return kvm_state->robust_singlestep; > diff --git a/kvm.h b/kvm.h > index b15e1dd..7fa72fd 100644 > --- a/kvm.h > +++ b/kvm.h > @@ -52,6 +52,9 @@ int kvm_has_xsave(void); > int kvm_has_xcrs(void); > int kvm_has_many_ioeventfds(void); > int kvm_has_pit_state2(void); > +int kvm_has_cpuhotplug_req(void); > +void kvm_set_cpuhotplug_req(void); > +void kvm_reset_cpuhotplug_req(void); > > #ifdef NEED_CPU_H > int kvm_init_vcpu(CPUState *env); > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index 935d08a..7e7839b 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -923,7 +923,7 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4); > /* hw/pc.c */ > void cpu_smm_update(CPUX86State *env); > uint64_t cpu_get_tsc(CPUX86State *env); > -CPUState *pc_new_cpu(const char *cpu_model); > +CPUState *pc_new_cpu(const char *cpu_model, int hotplugged); > > /* used to debug */ > #define X86_DUMP_FPU 0x0001 /* dump FPU state too */ -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
