Re: [PATCH] cpu hotplug issue

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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:

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

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.

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 */
--
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


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux