qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support
@ 2013-08-29  2:09 Chen Fan
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 1/6] piix4: implement function 'cpu_status_write' for vcpu ejection Chen Fan
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Chen Fan @ 2013-08-29  2:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

via implementing ACPI standard methods _EJ0, after Guest OS hot remove
one vcpu, it is able to send a signal to QEMU, then QEMU could notify 
the assigned vcpu to exit.

this series patches must be used with seabios patch and KVM patch together.

for KVM patches:
 http://comments.gmane.org/gmane.comp.emulators.kvm.devel/114347

for seabios patches:
 http://comments.gmane.org/gmane.comp.emulators.qemu/230460

Chen Fan (6):
  piix4: implement function 'cpu_status_write' for vcpu ejection
  cpus: release allocated vcpu objects and exit vcpu thread
  qom cpu: rename variable 'cpu_added_notifier' to
    'cpu_hotplug_notifier'
  qmp: add 'cpu-del' command support
  qom cpu: add struct CPUNotifier for supporting PLUG and UNPLUG cpu
    notifier
  i386: implement cpu interface 'cpu_common_unrealizefn'

 cpus.c                  | 36 +++++++++++++++++++++++++++++
 hw/acpi/piix4.c         | 61 +++++++++++++++++++++++++++++++++++++++----------
 hw/i386/pc.c            | 24 ++++++++++++++++++-
 hw/i386/pc_piix.c       |  1 +
 include/hw/boards.h     |  2 ++
 include/hw/i386/pc.h    |  1 +
 include/qom/cpu.h       | 19 +++++++++++++++
 include/sysemu/kvm.h    |  1 +
 include/sysemu/sysemu.h |  2 +-
 kvm-all.c               | 26 +++++++++++++++++++++
 qapi-schema.json        | 12 ++++++++++
 qmp-commands.hx         | 23 +++++++++++++++++++
 qmp.c                   |  9 ++++++++
 qom/cpu.c               | 27 ++++++++++++++++++----
 14 files changed, 225 insertions(+), 19 deletions(-)

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC][PATCH 1/6] piix4: implement function 'cpu_status_write' for vcpu ejection
  2013-08-29  2:09 [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support Chen Fan
@ 2013-08-29  2:09 ` Chen Fan
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 2/6] cpus: release allocated vcpu objects and exit vcpu thread Chen Fan
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-08-29  2:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

When OS eject a vcpu (like: echo 1 > /sys/bus/acpi/devices/LNXCPUXX/eject),
it will call acpi EJ0 method, the firmware will write the new cpumap, QEMU
will know which vcpu need to be ejected.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/acpi/piix4.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index c885690..1aaa7a4 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -61,6 +61,7 @@ struct pci_status {
 
 typedef struct CPUStatus {
     uint8_t sts[PIIX4_PROC_LEN];
+    uint8_t old_sts[PIIX4_PROC_LEN];
 } CPUStatus;
 
 typedef struct PIIX4PMState {
@@ -610,6 +611,12 @@ static const MemoryRegionOps piix4_pci_ops = {
     },
 };
 
+static void acpi_piix_eject_vcpu(int64_t cpuid)
+{
+    /* TODO: eject a vcpu, release allocated vcpu and exit the vcpu pthread.  */
+    PIIX4_DPRINTF("vcpu: %" PRIu64 " need to be ejected.\n", cpuid);
+}
+
 static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
 {
     PIIX4PMState *s = opaque;
@@ -622,7 +629,26 @@ static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
 static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data,
                              unsigned int size)
 {
-    /* TODO: implement VCPU removal on guest signal that CPU can be removed */
+    PIIX4PMState *s = opaque;
+    CPUStatus *cpus = &s->gpe_cpu;
+    uint8_t val;
+    int i;
+    int64_t cpuid = 0;
+
+    val = cpus->old_sts[addr] ^ data;
+
+    if (val == 0)
+        return;
+
+    for (i = 0; i < 8; i++) {
+        if (val & 1 << i) {
+            cpuid = 8 * addr + i;
+        }
+    }
+
+    if (cpuid != 0) {
+        acpi_piix_eject_vcpu(cpuid);
+    }
 }
 
 static const MemoryRegionOps cpu_hotplug_ops = {
@@ -647,11 +673,17 @@ static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
     ACPIGPE *gpe = &s->ar.gpe;
     CPUClass *k = CPU_GET_CLASS(cpu);
     int64_t cpu_id;
+    int i;
 
     assert(s != NULL);
 
     *gpe->sts = *gpe->sts | PIIX4_CPU_HOTPLUG_STATUS;
     cpu_id = k->get_arch_id(CPU(cpu));
+
+    for (i = 0; i < PIIX4_PROC_LEN; i++) {
+        g->old_sts[i] = g->sts[i];
+    }
+
     if (action == PLUG) {
         g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
     } else {
@@ -675,6 +707,7 @@ static void piix4_init_cpu_status(CPUState *cpu, void *data)
 
     g_assert((id / 8) < PIIX4_PROC_LEN);
     g->sts[id / 8] |= (1 << (id % 8));
+    g->old_sts[id / 8] |= (1 << (id % 8));
 }
 
 static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC][PATCH 2/6] cpus: release allocated vcpu objects and exit vcpu thread
  2013-08-29  2:09 [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support Chen Fan
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 1/6] piix4: implement function 'cpu_status_write' for vcpu ejection Chen Fan
@ 2013-08-29  2:09 ` Chen Fan
  2013-08-29  5:10   ` Andreas Färber
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 3/6] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier' Chen Fan
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Chen Fan @ 2013-08-29  2:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

After ACPI get a signal to eject a vcpu, then it will notify
the vcpu thread of needing to exit, before the vcpu exiting,
will release the vcpu related objects.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 cpus.c               | 36 ++++++++++++++++++++++++++++++++++++
 hw/acpi/piix4.c      | 16 ++++++++++++----
 include/qom/cpu.h    |  9 +++++++++
 include/sysemu/kvm.h |  1 +
 kvm-all.c            | 26 ++++++++++++++++++++++++++
 5 files changed, 84 insertions(+), 4 deletions(-)

diff --git a/cpus.c b/cpus.c
index 70cc617..6b793cb 100644
--- a/cpus.c
+++ b/cpus.c
@@ -697,6 +697,30 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
     qemu_cpu_kick(cpu);
 }
 
+static void qemu_kvm_destroy_vcpu(CPUState *cpu)
+{
+    CPUState *pcpu, *pcpu1;
+
+    pcpu = first_cpu;
+    pcpu1 = NULL;
+
+    while (pcpu) {
+        if (pcpu == cpu && pcpu1) {
+            pcpu1->next_cpu = cpu->next_cpu;
+            break;
+        }
+        pcpu1 = pcpu;
+        pcpu = pcpu->next_cpu;
+    }
+
+    if (kvm_destroy_vcpu(cpu) < 0) {
+        fprintf(stderr, "kvm_destroy_vcpu failed.\n");
+        exit(1);
+    }
+
+    qdev_free(DEVICE(X86_CPU(cpu)));
+}
+
 static void flush_queued_work(CPUState *cpu)
 {
     struct qemu_work_item *wi;
@@ -788,6 +812,11 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
             }
         }
         qemu_kvm_wait_io_event(cpu);
+        if (cpu->exit && !cpu_can_run(cpu)) {
+            qemu_kvm_destroy_vcpu(cpu);
+            qemu_mutex_unlock(&qemu_global_mutex);
+            return NULL;
+        }
     }
 
     return NULL;
@@ -1080,6 +1109,13 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
     }
 }
 
+void qemu_down_vcpu(CPUState *cpu)
+{
+    cpu->stop = true;
+    cpu->exit = true;
+    qemu_cpu_kick(cpu);
+}
+
 void qemu_init_vcpu(CPUState *cpu)
 {
     cpu->nr_cores = smp_cores;
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 1aaa7a4..44bc809 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -611,10 +611,18 @@ static const MemoryRegionOps piix4_pci_ops = {
     },
 };
 
-static void acpi_piix_eject_vcpu(int64_t cpuid)
+static void acpi_piix_eject_vcpu(PIIX4PMState *s, int64_t cpuid)
 {
-    /* TODO: eject a vcpu, release allocated vcpu and exit the vcpu pthread.  */
-    PIIX4_DPRINTF("vcpu: %" PRIu64 " need to be ejected.\n", cpuid);
+    CPUStatus *cpus = &s->gpe_cpu;
+    CPUState *cs = NULL;
+
+    cs = qemu_get_cpu(cpuid);
+    if (cs == NULL) {
+        return;
+    }
+
+    cpus->old_sts[cpuid / 8] &= ~(1 << (cpuid % 8));
+    qemu_down_vcpu(cs);
 }
 
 static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
@@ -647,7 +655,7 @@ static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data,
     }
 
     if (cpuid != 0) {
-        acpi_piix_eject_vcpu(cpuid);
+        acpi_piix_eject_vcpu(s, cpuid);
     }
 }
 
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 3e49936..fa8ec8a 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -180,6 +180,7 @@ struct CPUState {
     bool created;
     bool stop;
     bool stopped;
+    bool exit;
     volatile sig_atomic_t exit_request;
     volatile sig_atomic_t tcg_exit_req;
     uint32_t interrupt_request;
@@ -489,6 +490,14 @@ void cpu_exit(CPUState *cpu);
 void cpu_resume(CPUState *cpu);
 
 /**
+ * qemu_down_vcpu:
+ * @cpu: The vCPU will to down.
+ *
+ * Down a vCPU.
+ */
+void qemu_down_vcpu(CPUState *cpu);
+
+/**
  * qemu_init_vcpu:
  * @cpu: The vCPU to initialize.
  *
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index de74411..fd85605 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -158,6 +158,7 @@ int kvm_has_intx_set_mask(void);
 
 int kvm_init_vcpu(CPUState *cpu);
 int kvm_cpu_exec(CPUState *cpu);
+int kvm_destroy_vcpu(CPUState *cpu);
 
 #ifdef NEED_CPU_H
 
diff --git a/kvm-all.c b/kvm-all.c
index 716860f..fda3601 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -225,6 +225,32 @@ static void kvm_reset_vcpu(void *opaque)
     kvm_arch_reset_vcpu(cpu);
 }
 
+int kvm_destroy_vcpu(CPUState *cpu)
+{
+    KVMState *s = kvm_state;
+    long mmap_size;
+    int ret = 0;
+
+    DPRINTF("kvm_destroy_vcpu\n");
+
+    mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
+    if (mmap_size < 0) {
+        ret = mmap_size;
+        DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n");
+        goto err;
+    }
+
+    ret = munmap(cpu->kvm_run, mmap_size);
+    if (ret < 0) {
+        goto err;
+    }
+
+    close(cpu->kvm_fd);
+
+err:
+    return ret;
+}
+
 int kvm_init_vcpu(CPUState *cpu)
 {
     KVMState *s = kvm_state;
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC][PATCH 3/6] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier'
  2013-08-29  2:09 [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support Chen Fan
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 1/6] piix4: implement function 'cpu_status_write' for vcpu ejection Chen Fan
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 2/6] cpus: release allocated vcpu objects and exit vcpu thread Chen Fan
@ 2013-08-29  2:09 ` Chen Fan
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 4/6] qmp: add 'cpu-del' command support Chen Fan
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-08-29  2:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

Rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier', for
adding vcpu-remove notifier support.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/acpi/piix4.c         | 10 +++++-----
 hw/i386/pc.c            |  2 +-
 include/sysemu/sysemu.h |  2 +-
 qom/cpu.c               | 10 +++++-----
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 44bc809..0a58ff7 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -96,7 +96,7 @@ typedef struct PIIX4PMState {
     uint8_t s4_val;
 
     CPUStatus gpe_cpu;
-    Notifier cpu_added_notifier;
+    Notifier cpu_hotplug_notifier;
 } PIIX4PMState;
 
 #define TYPE_PIIX4_PM "PIIX4_PM"
@@ -700,9 +700,9 @@ static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
     pm_update_sci(s);
 }
 
-static void piix4_cpu_added_req(Notifier *n, void *opaque)
+static void piix4_cpu_hotplug(Notifier *n, void *opaque)
 {
-    PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_added_notifier);
+    PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_hotplug_notifier);
 
     piix4_cpu_hotplug_req(s, CPU(opaque), PLUG);
 }
@@ -738,8 +738,8 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
     memory_region_init_io(&s->io_cpu, OBJECT(s), &cpu_hotplug_ops, s,
                           "acpi-cpu-hotplug", PIIX4_PROC_LEN);
     memory_region_add_subregion(parent, PIIX4_PROC_BASE, &s->io_cpu);
-    s->cpu_added_notifier.notify = piix4_cpu_added_req;
-    qemu_register_cpu_added_notifier(&s->cpu_added_notifier);
+    s->cpu_hotplug_notifier.notify = piix4_cpu_hotplug;
+    qemu_register_cpu_hotplug_notifier(&s->cpu_hotplug_notifier);
 }
 
 static void enable_device(PIIX4PMState *s, int slot)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e8bc8ce..c0e7cbd 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -408,7 +408,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
     /* init CPU hotplug notifier */
     cpu_hotplug_cb.rtc_state = s;
     cpu_hotplug_cb.cpu_added_notifier.notify = rtc_notify_cpu_added;
-    qemu_register_cpu_added_notifier(&cpu_hotplug_cb.cpu_added_notifier);
+    qemu_register_cpu_hotplug_notifier(&cpu_hotplug_cb.cpu_added_notifier);
 
     if (set_boot_dev(s, boot_device)) {
         exit(1);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d7a77b6..a7384c0 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -153,7 +153,7 @@ void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
 void drive_hot_add(Monitor *mon, const QDict *qdict);
 
 /* CPU hotplug */
-void qemu_register_cpu_added_notifier(Notifier *notifier);
+void qemu_register_cpu_hotplug_notifier(Notifier *notifier);
 
 /* pcie aer error injection */
 void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
diff --git a/qom/cpu.c b/qom/cpu.c
index e71e57b..e3e75de 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -79,12 +79,12 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,
 }
 
 /* CPU hot-plug notifiers */
-static NotifierList cpu_added_notifiers =
-    NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
+static NotifierList cpu_hotplug_notifiers =
+    NOTIFIER_LIST_INITIALIZER(cpu_hotplug_notifiers);
 
-void qemu_register_cpu_added_notifier(Notifier *notifier)
+void qemu_register_cpu_hotplug_notifier(Notifier *notifier)
 {
-    notifier_list_add(&cpu_added_notifiers, notifier);
+    notifier_list_add(&cpu_hotplug_notifiers, notifier);
 }
 
 void cpu_reset_interrupt(CPUState *cpu, int mask)
@@ -230,7 +230,7 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
 
     if (dev->hotplugged) {
         cpu_synchronize_post_init(cpu);
-        notifier_list_notify(&cpu_added_notifiers, dev);
+        notifier_list_notify(&cpu_hotplug_notifiers, dev);
         cpu_resume(cpu);
     }
 }
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC][PATCH 4/6] qmp: add 'cpu-del' command support
  2013-08-29  2:09 [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support Chen Fan
                   ` (2 preceding siblings ...)
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 3/6] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier' Chen Fan
@ 2013-08-29  2:09 ` Chen Fan
  2013-08-29 12:28   ` Eric Blake
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 5/6] qom cpu: add struct CPUNotifier for supporting PLUG and UNPLUG cpu notifier Chen Fan
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 6/6] i386: implement cpu interface 'cpu_common_unrealizefn' Chen Fan
  5 siblings, 1 reply; 11+ messages in thread
From: Chen Fan @ 2013-08-29  2:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/i386/pc.c         |  5 +++++
 hw/i386/pc_piix.c    |  1 +
 include/hw/boards.h  |  2 ++
 include/hw/i386/pc.h |  1 +
 qapi-schema.json     | 12 ++++++++++++
 qmp-commands.hx      | 23 +++++++++++++++++++++++
 qmp.c                |  9 +++++++++
 7 files changed, 53 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c0e7cbd..75fc9bb 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -958,6 +958,11 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
     pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp);
 }
 
+void pc_hot_del_cpu(const int64_t id, Error **errp)
+{
+    /* TODO: hot remove VCPU. */
+}
+
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
 {
     int i;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 6e1e654..d779b75 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -347,6 +347,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
     .desc = "Standard PC (i440FX + PIIX, 1996)",
     .init = pc_init_pci_1_6,
     .hot_add_cpu = pc_hot_add_cpu,
+    .hot_del_cpu = pc_hot_del_cpu,
     .max_cpus = 255,
     .is_default = 1,
     DEFAULT_MACHINE_OPTIONS,
diff --git a/include/hw/boards.h b/include/hw/boards.h
index fb7c6f1..fea3737 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -23,6 +23,7 @@ typedef void QEMUMachineInitFunc(QEMUMachineInitArgs *args);
 typedef void QEMUMachineResetFunc(void);
 
 typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp);
+typedef void QEMUMachineHotDelCPUFunc(const int64_t id, Error **errp);
 
 typedef struct QEMUMachine {
     const char *name;
@@ -31,6 +32,7 @@ typedef struct QEMUMachine {
     QEMUMachineInitFunc *init;
     QEMUMachineResetFunc *reset;
     QEMUMachineHotAddCPUFunc *hot_add_cpu;
+    QEMUMachineHotDelCPUFunc *hot_del_cpu;
     BlockInterfaceType block_default_type;
     int max_cpus;
     unsigned int no_serial:1,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index f79d478..b7e66f4 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -96,6 +96,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
 void pc_hot_add_cpu(const int64_t id, Error **errp);
+void pc_hot_del_cpu(const int64_t id, Error **errp);
 void pc_acpi_init(const char *default_dsdt);
 
 PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
diff --git a/qapi-schema.json b/qapi-schema.json
index a51f7d2..6d3bf5f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1432,6 +1432,18 @@
 ##
 { 'command': 'cpu-add', 'data': {'id': 'int'} }
 
+# @cpu-del
+
+# Deletes CPU with specified ID
+#
+# @id: ID of CPU to be deleted, valid values [0..max_cpus)
+#
+# Returns: Nothing on success
+#
+# Since 1.6
+##
+{ 'command': 'cpu-del', 'data': {'id': 'int'} }
+
 ##
 # @memsave:
 #
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cf47e3f..16b54fd 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -411,6 +411,29 @@ Example:
 EQMP
 
     {
+        .name       = "cpu-del",
+        .args_type  = "id:i",
+        .mhandler.cmd_new = qmp_marshal_input_cpu_del,
+    },
+
+SQMP
+cpu-del
+-------
+
+Deletes virtual cpu
+
+Arguments:
+
+- "id": cpu id (json-int)
+
+Example:
+
+-> { "execute": "cpu-del", "arguments": { "id": 2 } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "memsave",
         .args_type  = "val:l,size:i,filename:s,cpu:i?",
         .mhandler.cmd_new = qmp_marshal_input_memsave,
diff --git a/qmp.c b/qmp.c
index 4c149b3..84dc873 100644
--- a/qmp.c
+++ b/qmp.c
@@ -118,6 +118,15 @@ void qmp_cpu_add(int64_t id, Error **errp)
     }
 }
 
+void qmp_cpu_del(int64_t id, Error **errp)
+{
+    if (current_machine->hot_del_cpu) {
+        current_machine->hot_del_cpu(id, errp);
+    } else {
+        error_setg(errp, "Not supported");
+    }
+}
+
 #ifndef CONFIG_VNC
 /* If VNC support is enabled, the "true" query-vnc command is
    defined in the VNC subsystem */
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC][PATCH 5/6] qom cpu: add struct CPUNotifier for supporting PLUG and UNPLUG cpu notifier
  2013-08-29  2:09 [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support Chen Fan
                   ` (3 preceding siblings ...)
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 4/6] qmp: add 'cpu-del' command support Chen Fan
@ 2013-08-29  2:09 ` Chen Fan
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 6/6] i386: implement cpu interface 'cpu_common_unrealizefn' Chen Fan
  5 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-08-29  2:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

Move struct HotplugEventType from file piix4.c to file qom/cpu.c,
and add struct CPUNotifier for supporting PLUG and UNPLUG cpu notifier.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/acpi/piix4.c   |  8 ++------
 include/qom/cpu.h | 10 ++++++++++
 qom/cpu.c         |  6 +++++-
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 0a58ff7..fa82768 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -669,11 +669,6 @@ static const MemoryRegionOps cpu_hotplug_ops = {
     },
 };
 
-typedef enum {
-    PLUG,
-    UNPLUG,
-} HotplugEventType;
-
 static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
                                   HotplugEventType action)
 {
@@ -703,8 +698,9 @@ static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
 static void piix4_cpu_hotplug(Notifier *n, void *opaque)
 {
     PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_hotplug_notifier);
+    CPUNotifier *notifier = opaque;
 
-    piix4_cpu_hotplug_req(s, CPU(opaque), PLUG);
+    piix4_cpu_hotplug_req(s, CPU(notifier->dev), notifier->type);
 }
 
 static void piix4_init_cpu_status(CPUState *cpu, void *data)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index fa8ec8a..c6f612d 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -518,6 +518,16 @@ void qemu_init_vcpu(CPUState *cpu);
  */
 void cpu_single_step(CPUState *cpu, int enabled);
 
+typedef enum {
+    PLUG,
+    UNPLUG,
+} HotplugEventType;
+
+typedef struct CPUNotifier {
+    DeviceState *dev;
+    HotplugEventType type;
+} CPUNotifier;
+
 #ifdef CONFIG_SOFTMMU
 extern const struct VMStateDescription vmstate_cpu_common;
 #else
diff --git a/qom/cpu.c b/qom/cpu.c
index e3e75de..3439c5d 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -227,10 +227,14 @@ static ObjectClass *cpu_common_class_by_name(const char *cpu_model)
 static void cpu_common_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cpu = CPU(dev);
+    CPUNotifier notifier;
+
+    notifier.dev = dev;
+    notifier.type = PLUG;
 
     if (dev->hotplugged) {
         cpu_synchronize_post_init(cpu);
-        notifier_list_notify(&cpu_hotplug_notifiers, dev);
+        notifier_list_notify(&cpu_hotplug_notifiers, &notifier);
         cpu_resume(cpu);
     }
 }
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC][PATCH 6/6] i386: implement cpu interface 'cpu_common_unrealizefn'
  2013-08-29  2:09 [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support Chen Fan
                   ` (4 preceding siblings ...)
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 5/6] qom cpu: add struct CPUNotifier for supporting PLUG and UNPLUG cpu notifier Chen Fan
@ 2013-08-29  2:09 ` Chen Fan
  2013-08-29  5:21   ` Andreas Färber
  5 siblings, 1 reply; 11+ messages in thread
From: Chen Fan @ 2013-08-29  2:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

Implement cpu interface 'cpu_common_unrealizefn' for emiting vcpu-remove
notifier to ACPI, then ACPI could send sci interrupt to OS for hot-remove
vcpu.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/i386/pc.c | 19 ++++++++++++++++++-
 qom/cpu.c    | 13 +++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 75fc9bb..9a87ac0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -960,7 +960,24 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
 
 void pc_hot_del_cpu(const int64_t id, Error **errp)
 {
-    /* TODO: hot remove VCPU. */
+    CPUState *s = NULL;
+    X86CPU *cpu = NULL;
+    DeviceState *ds = NULL;
+    DeviceClass *dc = NULL;
+
+    s = qemu_get_cpu(id);
+    if (s == NULL) {
+        error_setg(errp, "Unable to find cpu-index: %" PRIi64
+                   ",it non-exists or has been deleted.", id);
+        return;
+    }
+
+    cpu = X86_CPU(s);
+    ds = DEVICE(cpu);
+    dc = DEVICE_GET_CLASS(ds);
+    if (dc->unrealize) {
+        dc->unrealize(ds, errp);
+    }
 }
 
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
diff --git a/qom/cpu.c b/qom/cpu.c
index 3439c5d..d2b0c9e 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -239,6 +239,18 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
     }
 }
 
+static void cpu_common_unrealizefn(DeviceState *dev, Error **errp)
+{
+    CPUNotifier notifier;
+
+    notifier.dev = dev;
+    notifier.type = UNPLUG;
+
+    if (dev->hotplugged) {
+        notifier_list_notify(&cpu_hotplug_notifiers, &notifier);
+     }
+}
+
 static void cpu_common_initfn(Object *obj)
 {
     CPUState *cpu = CPU(obj);
@@ -269,6 +281,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
     k->gdb_read_register = cpu_common_gdb_read_register;
     k->gdb_write_register = cpu_common_gdb_write_register;
     dc->realize = cpu_common_realizefn;
+    dc->unrealize = cpu_common_unrealizefn;
     dc->no_user = 1;
 }
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC][PATCH 2/6] cpus: release allocated vcpu objects and exit vcpu thread
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 2/6] cpus: release allocated vcpu objects and exit vcpu thread Chen Fan
@ 2013-08-29  5:10   ` Andreas Färber
  2013-08-29 10:08     ` chenfan
  0 siblings, 1 reply; 11+ messages in thread
From: Andreas Färber @ 2013-08-29  5:10 UTC (permalink / raw)
  To: Chen Fan; +Cc: Igor Mammedov, qemu-devel, kvm@vger.kernel.org list

Am 29.08.2013 04:09, schrieb Chen Fan:
> After ACPI get a signal to eject a vcpu, then it will notify
> the vcpu thread of needing to exit, before the vcpu exiting,
> will release the vcpu related objects.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
>  cpus.c               | 36 ++++++++++++++++++++++++++++++++++++
>  hw/acpi/piix4.c      | 16 ++++++++++++----
>  include/qom/cpu.h    |  9 +++++++++
>  include/sysemu/kvm.h |  1 +
>  kvm-all.c            | 26 ++++++++++++++++++++++++++
>  5 files changed, 84 insertions(+), 4 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 70cc617..6b793cb 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -697,6 +697,30 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
>      qemu_cpu_kick(cpu);
>  }
>  
> +static void qemu_kvm_destroy_vcpu(CPUState *cpu)
> +{
> +    CPUState *pcpu, *pcpu1;
> +
> +    pcpu = first_cpu;
> +    pcpu1 = NULL;
> +
> +    while (pcpu) {
> +        if (pcpu == cpu && pcpu1) {
> +            pcpu1->next_cpu = cpu->next_cpu;
> +            break;
> +        }
> +        pcpu1 = pcpu;
> +        pcpu = pcpu->next_cpu;
> +    }

No, no, no. :) I specifically posted the "QOM CPUState, part 12" series
early to avoid exactly such code appearing! Give me a few minutes to
apply that to qom-cpu and then please rebase your work on
git://github.com/afaerber/qemu-cpu.git qom-cpu
using QTAILQ macro and --subject-prefix="RFC qom-cpu v2" for the next
version of the series.

Also, why is this only in the KVM code path? Isn't this needed for TCG
as well?

> +
> +    if (kvm_destroy_vcpu(cpu) < 0) {
> +        fprintf(stderr, "kvm_destroy_vcpu failed.\n");
> +        exit(1);
> +    }
> +
> +    qdev_free(DEVICE(X86_CPU(cpu)));

DEVICE(cpu) should be sufficient.

> +}
> +
>  static void flush_queued_work(CPUState *cpu)
>  {
>      struct qemu_work_item *wi;
> @@ -788,6 +812,11 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
>              }
>          }
>          qemu_kvm_wait_io_event(cpu);
> +        if (cpu->exit && !cpu_can_run(cpu)) {
> +            qemu_kvm_destroy_vcpu(cpu);
> +            qemu_mutex_unlock(&qemu_global_mutex);
> +            return NULL;
> +        }
>      }
>  
>      return NULL;
> @@ -1080,6 +1109,13 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
>      }
>  }
>  
> +void qemu_down_vcpu(CPUState *cpu)
> +{
> +    cpu->stop = true;
> +    cpu->exit = true;
> +    qemu_cpu_kick(cpu);
> +}
> +
>  void qemu_init_vcpu(CPUState *cpu)
>  {
>      cpu->nr_cores = smp_cores;
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 1aaa7a4..44bc809 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -611,10 +611,18 @@ static const MemoryRegionOps piix4_pci_ops = {
>      },
>  };
>  
> -static void acpi_piix_eject_vcpu(int64_t cpuid)
> +static void acpi_piix_eject_vcpu(PIIX4PMState *s, int64_t cpuid)
>  {
> -    /* TODO: eject a vcpu, release allocated vcpu and exit the vcpu pthread.  */
> -    PIIX4_DPRINTF("vcpu: %" PRIu64 " need to be ejected.\n", cpuid);
> +    CPUStatus *cpus = &s->gpe_cpu;
> +    CPUState *cs = NULL;
> +
> +    cs = qemu_get_cpu(cpuid);

Are you sure this is correct as 0-based index? Igor?

> +    if (cs == NULL) {
> +        return;
> +    }
> +
> +    cpus->old_sts[cpuid / 8] &= ~(1 << (cpuid % 8));
> +    qemu_down_vcpu(cs);
>  }
>  
>  static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
> @@ -647,7 +655,7 @@ static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data,
>      }
>  
>      if (cpuid != 0) {
> -        acpi_piix_eject_vcpu(cpuid);
> +        acpi_piix_eject_vcpu(s, cpuid);
>      }
>  }
>  
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 3e49936..fa8ec8a 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -180,6 +180,7 @@ struct CPUState {
>      bool created;
>      bool stop;
>      bool stopped;
> +    bool exit;
>      volatile sig_atomic_t exit_request;
>      volatile sig_atomic_t tcg_exit_req;
>      uint32_t interrupt_request;
> @@ -489,6 +490,14 @@ void cpu_exit(CPUState *cpu);
>  void cpu_resume(CPUState *cpu);
>  
>  /**
> + * qemu_down_vcpu:
> + * @cpu: The vCPU will to down.
> + *
> + * Down a vCPU.
> + */
> +void qemu_down_vcpu(CPUState *cpu);

The naming and documentation sounds wrong language-wise to me, but I am
not a native speaker either. Maybe "tear down" instead of "down"? Or
simply qemu_request_vcpu_removal() or something like that?

> +
> +/**
>   * qemu_init_vcpu:
>   * @cpu: The vCPU to initialize.
>   *
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index de74411..fd85605 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -158,6 +158,7 @@ int kvm_has_intx_set_mask(void);
>  
>  int kvm_init_vcpu(CPUState *cpu);
>  int kvm_cpu_exec(CPUState *cpu);
> +int kvm_destroy_vcpu(CPUState *cpu);
>  
>  #ifdef NEED_CPU_H
>  
> diff --git a/kvm-all.c b/kvm-all.c
> index 716860f..fda3601 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -225,6 +225,32 @@ static void kvm_reset_vcpu(void *opaque)
>      kvm_arch_reset_vcpu(cpu);
>  }
>  
> +int kvm_destroy_vcpu(CPUState *cpu)
> +{
> +    KVMState *s = kvm_state;
> +    long mmap_size;
> +    int ret = 0;
> +
> +    DPRINTF("kvm_destroy_vcpu\n");
> +
> +    mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
> +    if (mmap_size < 0) {
> +        ret = mmap_size;
> +        DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n");
> +        goto err;
> +    }
> +
> +    ret = munmap(cpu->kvm_run, mmap_size);
> +    if (ret < 0) {
> +        goto err;
> +    }
> +
> +    close(cpu->kvm_fd);
> +
> +err:
> +    return ret;
> +}

I always assumed we'd need a new ioctl to inform the kernel about our
intent to remove a vCPU before doing such cleanups? CC'ing kvm list.

Regards,
Andreas

> +
>  int kvm_init_vcpu(CPUState *cpu)
>  {
>      KVMState *s = kvm_state;
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC][PATCH 6/6] i386: implement cpu interface 'cpu_common_unrealizefn'
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 6/6] i386: implement cpu interface 'cpu_common_unrealizefn' Chen Fan
@ 2013-08-29  5:21   ` Andreas Färber
  0 siblings, 0 replies; 11+ messages in thread
From: Andreas Färber @ 2013-08-29  5:21 UTC (permalink / raw)
  To: Chen Fan; +Cc: Igor Mammedov, qemu-devel, Paolo Bonzini

Am 29.08.2013 04:09, schrieb Chen Fan:
> Implement cpu interface 'cpu_common_unrealizefn' for emiting vcpu-remove
> notifier to ACPI, then ACPI could send sci interrupt to OS for hot-remove
> vcpu.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
>  hw/i386/pc.c | 19 ++++++++++++++++++-
>  qom/cpu.c    | 13 +++++++++++++
>  2 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 75fc9bb..9a87ac0 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -960,7 +960,24 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
>  
>  void pc_hot_del_cpu(const int64_t id, Error **errp)
>  {
> -    /* TODO: hot remove VCPU. */
> +    CPUState *s = NULL;

"cs" since this is in PC code, to free the "s" identifier.

> +    X86CPU *cpu = NULL;
> +    DeviceState *ds = NULL;

"dev" please for consistency.

> +    DeviceClass *dc = NULL;
> +
> +    s = qemu_get_cpu(id);

Same question as on 2/6, whether id == cpu_index here.

> +    if (s == NULL) {
> +        error_setg(errp, "Unable to find cpu-index: %" PRIi64
> +                   ",it non-exists or has been deleted.", id);

Space after comma please in English language and "doesn't exist".

> +        return;
> +    }
> +
> +    cpu = X86_CPU(s);
> +    ds = DEVICE(cpu);
> +    dc = DEVICE_GET_CLASS(ds);
> +    if (dc->unrealize) {
> +        dc->unrealize(ds, errp);
> +    }

Never call this directly, this is missing important device cleanups such
as VMState. What you want is:
object_property_set_bool(OBJECT(cs), false, "realized", errp);

>  }
>  
>  void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 3439c5d..d2b0c9e 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -239,6 +239,18 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
>      }
>  }
>  
> +static void cpu_common_unrealizefn(DeviceState *dev, Error **errp)
> +{
> +    CPUNotifier notifier;
> +
> +    notifier.dev = dev;
> +    notifier.type = UNPLUG;
> +
> +    if (dev->hotplugged) {
> +        notifier_list_notify(&cpu_hotplug_notifiers, &notifier);
> +     }

Apart from the misindentation this looks wrong. If we initialize a
machine with -smp 2 we should be allowed to hot-unplug CPU 1 IMO. Are
there any reasons not to allow that?

I would simply drop the dev->hotplugged check and always emit the
notification. When QEMU is shut down, the unrealizefn and finalizers are
not called, I believe.

Regards,
Andreas

> +}
> +
>  static void cpu_common_initfn(Object *obj)
>  {
>      CPUState *cpu = CPU(obj);
> @@ -269,6 +281,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
>      k->gdb_read_register = cpu_common_gdb_read_register;
>      k->gdb_write_register = cpu_common_gdb_write_register;
>      dc->realize = cpu_common_realizefn;
> +    dc->unrealize = cpu_common_unrealizefn;
>      dc->no_user = 1;
>  }
>  
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC][PATCH 2/6] cpus: release allocated vcpu objects and exit vcpu thread
  2013-08-29  5:10   ` Andreas Färber
@ 2013-08-29 10:08     ` chenfan
  0 siblings, 0 replies; 11+ messages in thread
From: chenfan @ 2013-08-29 10:08 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Igor Mammedov, qemu-devel, kvm@vger.kernel.org list

On Thu, 2013-08-29 at 07:10 +0200, Andreas Färber wrote:
> Am 29.08.2013 04:09, schrieb Chen Fan:
> > After ACPI get a signal to eject a vcpu, then it will notify
> > the vcpu thread of needing to exit, before the vcpu exiting,
> > will release the vcpu related objects.
> > 
> > Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> > ---
> >  cpus.c               | 36 ++++++++++++++++++++++++++++++++++++
> >  hw/acpi/piix4.c      | 16 ++++++++++++----
> >  include/qom/cpu.h    |  9 +++++++++
> >  include/sysemu/kvm.h |  1 +
> >  kvm-all.c            | 26 ++++++++++++++++++++++++++
> >  5 files changed, 84 insertions(+), 4 deletions(-)
> > 
> > diff --git a/cpus.c b/cpus.c
> > index 70cc617..6b793cb 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -697,6 +697,30 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
> >      qemu_cpu_kick(cpu);
> >  }
> >  
> > +static void qemu_kvm_destroy_vcpu(CPUState *cpu)
> > +{
> > +    CPUState *pcpu, *pcpu1;
> > +
> > +    pcpu = first_cpu;
> > +    pcpu1 = NULL;
> > +
> > +    while (pcpu) {
> > +        if (pcpu == cpu && pcpu1) {
> > +            pcpu1->next_cpu = cpu->next_cpu;
> > +            break;
> > +        }
> > +        pcpu1 = pcpu;
> > +        pcpu = pcpu->next_cpu;
> > +    }
> 
> No, no, no. :) I specifically posted the "QOM CPUState, part 12" series
> early to avoid exactly such code appearing! Give me a few minutes to
> apply that to qom-cpu and then please rebase your work on
> git://github.com/afaerber/qemu-cpu.git qom-cpu
> using QTAILQ macro and --subject-prefix="RFC qom-cpu v2" for the next
> version of the series.
OK. Thanks.
> 
> Also, why is this only in the KVM code path? Isn't this needed for TCG
> as well?
I will add work for TCG support later.
> 
> > +
> > +    if (kvm_destroy_vcpu(cpu) < 0) {
> > +        fprintf(stderr, "kvm_destroy_vcpu failed.\n");
> > +        exit(1);
> > +    }
> > +
> > +    qdev_free(DEVICE(X86_CPU(cpu)));
> 
> DEVICE(cpu) should be sufficient.
> 
> > +}
> > +
> >  static void flush_queued_work(CPUState *cpu)
> >  {
> >      struct qemu_work_item *wi;
> > @@ -788,6 +812,11 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
> >              }
> >          }
> >          qemu_kvm_wait_io_event(cpu);
> > +        if (cpu->exit && !cpu_can_run(cpu)) {
> > +            qemu_kvm_destroy_vcpu(cpu);
> > +            qemu_mutex_unlock(&qemu_global_mutex);
> > +            return NULL;
> > +        }
> >      }
> >  
> >      return NULL;
> > @@ -1080,6 +1109,13 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
> >      }
> >  }
> >  
> > +void qemu_down_vcpu(CPUState *cpu)
> > +{
> > +    cpu->stop = true;
> > +    cpu->exit = true;
> > +    qemu_cpu_kick(cpu);
> > +}
> > +
> >  void qemu_init_vcpu(CPUState *cpu)
> >  {
> >      cpu->nr_cores = smp_cores;
> > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > index 1aaa7a4..44bc809 100644
> > --- a/hw/acpi/piix4.c
> > +++ b/hw/acpi/piix4.c
> > @@ -611,10 +611,18 @@ static const MemoryRegionOps piix4_pci_ops = {
> >      },
> >  };
> >  
> > -static void acpi_piix_eject_vcpu(int64_t cpuid)
> > +static void acpi_piix_eject_vcpu(PIIX4PMState *s, int64_t cpuid)
> >  {
> > -    /* TODO: eject a vcpu, release allocated vcpu and exit the vcpu pthread.  */
> > -    PIIX4_DPRINTF("vcpu: %" PRIu64 " need to be ejected.\n", cpuid);
> > +    CPUStatus *cpus = &s->gpe_cpu;
> > +    CPUState *cs = NULL;
> > +
> > +    cs = qemu_get_cpu(cpuid);
> 
> Are you sure this is correct as 0-based index? Igor?
> 
> > +    if (cs == NULL) {
> > +        return;
> > +    }
> > +
> > +    cpus->old_sts[cpuid / 8] &= ~(1 << (cpuid % 8));
> > +    qemu_down_vcpu(cs);
> >  }
> >  
> >  static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
> > @@ -647,7 +655,7 @@ static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data,
> >      }
> >  
> >      if (cpuid != 0) {
> > -        acpi_piix_eject_vcpu(cpuid);
> > +        acpi_piix_eject_vcpu(s, cpuid);
> >      }
> >  }
> >  
> > diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> > index 3e49936..fa8ec8a 100644
> > --- a/include/qom/cpu.h
> > +++ b/include/qom/cpu.h
> > @@ -180,6 +180,7 @@ struct CPUState {
> >      bool created;
> >      bool stop;
> >      bool stopped;
> > +    bool exit;
> >      volatile sig_atomic_t exit_request;
> >      volatile sig_atomic_t tcg_exit_req;
> >      uint32_t interrupt_request;
> > @@ -489,6 +490,14 @@ void cpu_exit(CPUState *cpu);
> >  void cpu_resume(CPUState *cpu);
> >  
> >  /**
> > + * qemu_down_vcpu:
> > + * @cpu: The vCPU will to down.
> > + *
> > + * Down a vCPU.
> > + */
> > +void qemu_down_vcpu(CPUState *cpu);
> 
> The naming and documentation sounds wrong language-wise to me, but I am
> not a native speaker either. Maybe "tear down" instead of "down"? Or
> simply qemu_request_vcpu_removal() or something like that?
> 
> > +
> > +/**
> >   * qemu_init_vcpu:
> >   * @cpu: The vCPU to initialize.
> >   *
> > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> > index de74411..fd85605 100644
> > --- a/include/sysemu/kvm.h
> > +++ b/include/sysemu/kvm.h
> > @@ -158,6 +158,7 @@ int kvm_has_intx_set_mask(void);
> >  
> >  int kvm_init_vcpu(CPUState *cpu);
> >  int kvm_cpu_exec(CPUState *cpu);
> > +int kvm_destroy_vcpu(CPUState *cpu);
> >  
> >  #ifdef NEED_CPU_H
> >  
> > diff --git a/kvm-all.c b/kvm-all.c
> > index 716860f..fda3601 100644
> > --- a/kvm-all.c
> > +++ b/kvm-all.c
> > @@ -225,6 +225,32 @@ static void kvm_reset_vcpu(void *opaque)
> >      kvm_arch_reset_vcpu(cpu);
> >  }
> >  
> > +int kvm_destroy_vcpu(CPUState *cpu)
> > +{
> > +    KVMState *s = kvm_state;
> > +    long mmap_size;
> > +    int ret = 0;
> > +
> > +    DPRINTF("kvm_destroy_vcpu\n");
> > +
> > +    mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
> > +    if (mmap_size < 0) {
> > +        ret = mmap_size;
> > +        DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n");
> > +        goto err;
> > +    }
> > +
> > +    ret = munmap(cpu->kvm_run, mmap_size);
> > +    if (ret < 0) {
> > +        goto err;
> > +    }
> > +
> > +    close(cpu->kvm_fd);
> > +
> > +err:
> > +    return ret;
> > +}
> 
> I always assumed we'd need a new ioctl to inform the kernel about our
> intent to remove a vCPU before doing such cleanups? CC'ing kvm list.
I think, In kvm.h, there is not one '_IO' number reserved for removing a
vCPU. like KVM_REMOVE_VCPU or other. maybe the original idea for
releasing a vCPU does not need to use a new ioctl to free the allocated
vCPU.

Thanks for your review.
Chen

> 
> Regards,
> Andreas
> 
> > +
> >  int kvm_init_vcpu(CPUState *cpu)
> >  {
> >      KVMState *s = kvm_state;
> > 
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC][PATCH 4/6] qmp: add 'cpu-del' command support
  2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 4/6] qmp: add 'cpu-del' command support Chen Fan
@ 2013-08-29 12:28   ` Eric Blake
  0 siblings, 0 replies; 11+ messages in thread
From: Eric Blake @ 2013-08-29 12:28 UTC (permalink / raw)
  To: Chen Fan; +Cc: Igor Mammedov, qemu-devel, Andreas Färber

[-- Attachment #1: Type: text/plain, Size: 1287 bytes --]

On 08/28/2013 08:09 PM, Chen Fan wrote:
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
>  hw/i386/pc.c         |  5 +++++
>  hw/i386/pc_piix.c    |  1 +
>  include/hw/boards.h  |  2 ++
>  include/hw/i386/pc.h |  1 +
>  qapi-schema.json     | 12 ++++++++++++
>  qmp-commands.hx      | 23 +++++++++++++++++++++++
>  qmp.c                |  9 +++++++++
>  7 files changed, 53 insertions(+)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c0e7cbd..75fc9bb 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -958,6 +958,11 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
>      pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp);
>  }
>  
> +void pc_hot_del_cpu(const int64_t id, Error **errp)
> +{
> +    /* TODO: hot remove VCPU. */
> +}

That seems fishy.


> +++ b/qapi-schema.json
> @@ -1432,6 +1432,18 @@
>  ##
>  { 'command': 'cpu-add', 'data': {'id': 'int'} }
>  
> +# @cpu-del
> +
> +# Deletes CPU with specified ID
> +#
> +# @id: ID of CPU to be deleted, valid values [0..max_cpus)
> +#
> +# Returns: Nothing on success
> +#
> +# Since 1.6

1.6 is already out the door.  This must be 1.7.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-08-29 12:29 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-29  2:09 [Qemu-devel] [RFC][PATCH 0/6] i386: add cpu hot remove support Chen Fan
2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 1/6] piix4: implement function 'cpu_status_write' for vcpu ejection Chen Fan
2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 2/6] cpus: release allocated vcpu objects and exit vcpu thread Chen Fan
2013-08-29  5:10   ` Andreas Färber
2013-08-29 10:08     ` chenfan
2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 3/6] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier' Chen Fan
2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 4/6] qmp: add 'cpu-del' command support Chen Fan
2013-08-29 12:28   ` Eric Blake
2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 5/6] qom cpu: add struct CPUNotifier for supporting PLUG and UNPLUG cpu notifier Chen Fan
2013-08-29  2:09 ` [Qemu-devel] [RFC][PATCH 6/6] i386: implement cpu interface 'cpu_common_unrealizefn' Chen Fan
2013-08-29  5:21   ` Andreas Färber

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).