[Qemu-devel] [RFC qom-cpu v2 0/8] i386: add cpu hot remove support

[Qemu-devel] [RFC qom-cpu v2 0/8] i386: add cpu hot remove support

[Chen Fan]

Via implementing ACPI standard methods _EJ0 in bios, after Guest OS hot remove
one vCPU, it is able to send a signal to QEMU, then QEMU could notify
the assigned vCPU of exiting.

this work is based on Andreas Färber's qom-cpu branch tree.
    git://github.com/afaerber/qemu-cpu.git

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 (8):
  apic: remove apic_no from apic_init_common()
  x86: add x86_cpu_unrealizefn() for cpu apic remove
  qmp: add 'cpu-del' command support
  qom cpu: rename variable 'cpu_added_notifier' to
    'cpu_hotplug_notifier'
  qom cpu: add UNPLUG cpu notifier support
  i386: implement pc interface pc_hot_del_cpu()
  piix4: implement function cpu_status_write() for vcpu ejection
  cpus: release allocated vCPU objects

 cpus.c                          | 46 ++++++++++++++++++++++++++++
 hw/acpi/piix4.c                 | 66 +++++++++++++++++++++++++++++++++--------
 hw/cpu/icc_bus.c                | 11 +++++++
 hw/i386/kvm/apic.c              |  6 ++++
 hw/i386/pc.c                    | 34 ++++++++++++++++++++-
 hw/i386/pc_piix.c               |  1 +
 hw/intc/apic.c                  |  7 +++++
 hw/intc/apic_common.c           | 15 ++++++++--
 include/hw/boards.h             |  2 ++
 include/hw/cpu/icc_bus.h        |  1 +
 include/hw/i386/apic_internal.h |  1 +
 include/hw/i386/pc.h            |  1 +
 include/qom/cpu.h               | 20 +++++++++++++
 include/sysemu/kvm.h            |  1 +
 include/sysemu/sysemu.h         |  2 +-
 kvm-all.c                       | 25 ++++++++++++++++
 qapi-schema.json                | 12 ++++++++
 qmp-commands.hx                 | 23 ++++++++++++++
 qmp.c                           |  9 ++++++
 qom/cpu.c                       | 25 ++++++++++++----
 target-i386/cpu-qom.h           |  1 +
 target-i386/cpu.c               | 36 ++++++++++++++++++++++
 22 files changed, 323 insertions(+), 22 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [RFC qom-cpu v2 1/8] apic: remove apic_no from apic_init_common()

[Chen Fan]

the 'apic_no' is increased by one when initialize/create a vCPU each time,
which causes APICCommonState s->idx always is increased.
but if we want to re-add a vCPU after removing a vCPU, we need to re-use the
vacant s->idx which the corresponding vCPU had been removed. 
so we could use the unique cpu apic_id instead of the progressive s->idx.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/intc/apic_common.c | 4 +---
 target-i386/cpu.c     | 1 +
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index a0beb10..5568621 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -289,13 +289,11 @@ static int apic_init_common(ICCDevice *dev)
     APICCommonState *s = APIC_COMMON(dev);
     APICCommonClass *info;
     static DeviceState *vapic;
-    static int apic_no;
     static bool mmio_registered;
 
-    if (apic_no >= MAX_APICS) {
+    if (s->idx >= MAX_APICS) {
         return -1;
     }
-    s->idx = apic_no++;
 
     info = APIC_COMMON_GET_CLASS(s);
     info->init(s);
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 42c5de0..2b99683 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2322,6 +2322,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
     /* TODO: convert to link<> */
     apic = APIC_COMMON(env->apic_state);
     apic->cpu = cpu;
+    apic->idx = env->cpuid_apic_id;
 }
 
 static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
-- 
1.8.1.4

[Qemu-devel] [RFC qom-cpu v2 2/8] x86: add x86_cpu_unrealizefn() for cpu apic remove

[Chen Fan]

Implement x86_cpu_unrealizefn() for corresponding x86_cpu_realizefn(),
which is mostly used to clear the apic related information at here.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/cpu/icc_bus.c                | 11 +++++++++++
 hw/i386/kvm/apic.c              |  6 ++++++
 hw/intc/apic.c                  |  7 +++++++
 hw/intc/apic_common.c           | 11 +++++++++++
 include/hw/cpu/icc_bus.h        |  1 +
 include/hw/i386/apic_internal.h |  1 +
 target-i386/cpu-qom.h           |  1 +
 target-i386/cpu.c               | 35 +++++++++++++++++++++++++++++++++++
 8 files changed, 73 insertions(+)

diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index 8748cc5..45e87d1 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -54,11 +54,22 @@ static void icc_device_realize(DeviceState *dev, Error **errp)
     }
 }
 
+static void icc_device_unrealize(DeviceState *dev, Error **errp)
+{
+    ICCDevice *id = ICC_DEVICE(dev);
+    ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(id);
+
+    if (idc->exit) {
+        idc->exit(id);
+    }
+}
+
 static void icc_device_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
 
     dc->realize = icc_device_realize;
+    dc->unrealize = icc_device_unrealize;
     dc->bus_type = TYPE_ICC_BUS;
 }
 
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 5609063..8f028a1 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -181,11 +181,17 @@ static void kvm_apic_init(APICCommonState *s)
     }
 }
 
+static void kvm_apic_exit(APICCommonState *s)
+{
+    memory_region_destroy(&s->io_memory);
+}
+
 static void kvm_apic_class_init(ObjectClass *klass, void *data)
 {
     APICCommonClass *k = APIC_COMMON_CLASS(klass);
 
     k->init = kvm_apic_init;
+    k->exit = kvm_apic_exit;
     k->set_base = kvm_apic_set_base;
     k->set_tpr = kvm_apic_set_tpr;
     k->get_tpr = kvm_apic_get_tpr;
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index a913186..23488b4 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -882,11 +882,18 @@ static void apic_init(APICCommonState *s)
     msi_supported = true;
 }
 
+static void apic_uninit(APICCommonState *s)
+{
+    memory_region_destroy(&s->io_memory);
+    local_apics[s->idx] = NULL;
+}
+
 static void apic_class_init(ObjectClass *klass, void *data)
 {
     APICCommonClass *k = APIC_COMMON_CLASS(klass);
 
     k->init = apic_init;
+    k->exit = apic_uninit;
     k->set_base = apic_set_base;
     k->set_tpr = apic_set_tpr;
     k->get_tpr = apic_get_tpr;
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 5568621..32c2f74 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -316,6 +316,16 @@ static int apic_init_common(ICCDevice *dev)
     return 0;
 }
 
+static void apic_exit_common(ICCDevice *dev)
+{
+    APICCommonState *s = APIC_COMMON(dev);
+    APICCommonClass *info;
+
+    info = APIC_COMMON_GET_CLASS(s);
+    if (info->exit)
+        info->exit(s);
+}
+
 static void apic_dispatch_pre_save(void *opaque)
 {
     APICCommonState *s = APIC_COMMON(opaque);
@@ -387,6 +397,7 @@ static void apic_common_class_init(ObjectClass *klass, void *data)
     dc->no_user = 1;
     dc->props = apic_properties_common;
     idc->init = apic_init_common;
+    idc->exit = apic_exit_common;
 }
 
 static const TypeInfo apic_common_type = {
diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
index b550070..15d5374 100644
--- a/include/hw/cpu/icc_bus.h
+++ b/include/hw/cpu/icc_bus.h
@@ -67,6 +67,7 @@ typedef struct ICCDeviceClass {
     /*< public >*/
 
     int (*init)(ICCDevice *dev); /* TODO replace with QOM realize */
+    void (*exit)(ICCDevice *dev);
 } ICCDeviceClass;
 
 #define TYPE_ICC_DEVICE "icc-device"
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 1b0a7fb..87d5248 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -81,6 +81,7 @@ typedef struct APICCommonClass
     ICCDeviceClass parent_class;
 
     void (*init)(APICCommonState *s);
+    void (*exit)(APICCommonState *s);
     void (*set_base)(APICCommonState *s, uint64_t val);
     void (*set_tpr)(APICCommonState *s, uint8_t val);
     uint8_t (*get_tpr)(APICCommonState *s);
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index c4447c2..1e520be 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -50,6 +50,7 @@ typedef struct X86CPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
     void (*parent_reset)(CPUState *cpu);
 } X86CPUClass;
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2b99683..6f9154d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2339,10 +2339,31 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
         return;
     }
 }
+
+static void x86_cpu_apic_unrealize(X86CPU *cpu, Error **errp)
+{
+    CPUX86State *env = &cpu->env;
+    Error *local_err = NULL;
+
+    if (env->apic_state == NULL) {
+        return;
+    }
+
+    object_property_set_bool(OBJECT(env->apic_state), false, "realized", &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    qdev_free(env->apic_state);
+}
 #else
 static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
 {
 }
+static void x86_cpu_apic_unrealize(X86CPU *cpu, Error **errp)
+{
+}
 #endif
 
 static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
@@ -2418,6 +2439,18 @@ out:
     }
 }
 
+static void x86_cpu_unrealizefn(DeviceState *dev, Error **errp)
+{
+    X86CPU *cpu = X86_CPU(dev);
+    Error *local_err = NULL;
+
+    x86_cpu_apic_unrealize(cpu, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
+}
+
 /* Enables contiguous-apic-ID mode, for compatibility */
 static bool compat_apic_id_mode;
 
@@ -2549,7 +2582,9 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
 
     xcc->parent_realize = dc->realize;
+    xcc->parent_unrealize = dc->unrealize;
     dc->realize = x86_cpu_realizefn;
+    dc->unrealize = x86_cpu_unrealizefn;
     dc->bus_type = TYPE_ICC_BUS;
     dc->props = x86_cpu_properties;
 
-- 
1.8.1.4

[Qemu-devel] [RFC qom-cpu v2 3/8] qmp: add 'cpu-del' command support

[Chen Fan]

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 0c313fe..3de9c51 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -957,6 +957,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..6052aa9 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.7
+##
+{ '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

[Qemu-devel] [RFC qom-cpu v2 4/8] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier'

[Chen Fan]

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 0b8d1d9..c8f4182 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -95,7 +95,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"
@@ -660,9 +660,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);
 }
@@ -695,8 +695,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 3de9c51..f36903f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -407,7 +407,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 b1aa059..e1c1120 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 fa7ec6b..7992fe1 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -67,12 +67,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)
@@ -218,7 +218,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

[Qemu-devel] [RFC qom-cpu v2 5/8] qom cpu: add UNPLUG cpu notifier support

[Chen Fan]

Move struct HotplugEventType from file piix4.c to file qom/cpu.c,
and add struct CPUNotifier for supporting 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 c8f4182..2ddc9a8 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -635,11 +635,6 @@ static const MemoryRegionOps cpu_hotplug_ops = {
     },
 };
 
-typedef enum {
-    PLUG,
-    UNPLUG,
-} HotplugEventType;
-
 static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
                                   HotplugEventType action)
 {
@@ -663,8 +658,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 int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7739e00..0238532 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -507,6 +507,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 7992fe1..c6d7ebc 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -215,10 +215,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

[Qemu-devel] [RFC qom-cpu v2 6/8] i386: implement pc interface pc_hot_del_cpu()

[Chen Fan]

Implement cpu interface pc_hot_del_cpu() for unrealizing device vCPU.
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 | 29 ++++++++++++++++++++++++++++-
 qom/cpu.c    | 11 +++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f36903f..6f88e41 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -959,7 +959,34 @@ 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 *cpu;
+    bool found = false;
+    X86CPUClass *xcc;
+
+    CPU_FOREACH(cpu) {
+        CPUClass *cc = CPU_GET_CLASS(cpu);
+        int64_t cpuid = cc->get_arch_id(cpu);
+
+        if (cpuid == id) {
+            found = true;
+            break;
+        }
+    }
+
+    if (!found) {
+        error_setg(errp, "Unable to find cpu-index: %" PRIi64
+                   ", it doesn't exist or has been deleted.", id);
+        return;
+    }
+
+    if (cpu == first_cpu && !CPU_NEXT(cpu)) {
+        error_setg(errp, "Unable to delete the last"
+                   " cpu when VM running.");
+        return;
+    }
+
+    xcc = X86_CPU_GET_CLASS(DEVICE(cpu));
+    xcc->parent_unrealize(DEVICE(cpu), errp);
 }
 
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
diff --git a/qom/cpu.c b/qom/cpu.c
index c6d7ebc..9cd7fcd 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -227,6 +227,16 @@ 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;
+
+    notifier_list_notify(&cpu_hotplug_notifiers, &notifier);
+}
+
 static void cpu_common_initfn(Object *obj)
 {
     CPUState *cpu = CPU(obj);
@@ -257,6 +267,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

[Qemu-devel] [RFC qom-cpu v2 7/8] piix4: implement function cpu_status_write() for vcpu ejection

[Chen Fan]

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 | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 2ddc9a8..0e9b5bd 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,27 @@ 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 = {
@@ -642,13 +669,20 @@ 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));
+        g->old_sts[cpu_id / 8] |= (1 << (cpu_id % 8));
     } else {
         g->sts[cpu_id / 8] &= ~(1 << (cpu_id % 8));
     }
@@ -687,6 +721,7 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
 
         g_assert((id / 8) < PIIX4_PROC_LEN);
         s->gpe_cpu.sts[id / 8] |= (1 << (id % 8));
+        s->gpe_cpu.old_sts[id / 8] |= (1 << (id % 8));
     }
     memory_region_init_io(&s->io_cpu, OBJECT(s), &cpu_hotplug_ops, s,
                           "acpi-cpu-hotplug", PIIX4_PROC_LEN);
-- 
1.8.1.4

[Qemu-devel] [RFC qom-cpu v2 8/8] cpus: release allocated vCPU objects

[Chen Fan]

After ACPI get a signal to eject a vCPU, then it will notify
the vCPU thread to exit when using KVM, and the vCPU must be removed from CPU list，
before the vCPU really removed, there will release the all related vCPU objects and
apic device.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 cpus.c               | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/acpi/piix4.c      | 23 +++++++++++++++++------
 include/qom/cpu.h    | 10 ++++++++++
 include/sysemu/kvm.h |  1 +
 kvm-all.c            | 25 +++++++++++++++++++++++++
 5 files changed, 99 insertions(+), 6 deletions(-)

diff --git a/cpus.c b/cpus.c
index 980697e..10dded3 100644
--- a/cpus.c
+++ b/cpus.c
@@ -714,6 +714,26 @@ 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)
+{
+    CPU_REMOVE(cpu);
+
+    if (kvm_destroy_vcpu(cpu) < 0) {
+        fprintf(stderr, "kvm_destroy_vcpu failed.\n");
+        exit(1);
+    }
+
+    object_property_set_bool(OBJECT(cpu), false, "realized", NULL);
+    qdev_free(DEVICE(cpu));
+}
+
+static void qemu_tcg_destroy_vcpu(CPUState *cpu)
+{
+    CPU_REMOVE(cpu);
+    object_property_set_bool(OBJECT(cpu), false, "realized", NULL);
+    qdev_free(DEVICE(cpu));
+}
+
 static void flush_queued_work(CPUState *cpu)
 {
     struct qemu_work_item *wi;
@@ -805,6 +825,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;
@@ -857,6 +882,7 @@ static void tcg_exec_all(void);
 static void *qemu_tcg_cpu_thread_fn(void *arg)
 {
     CPUState *cpu = arg;
+    CPUState *remove_cpu = NULL;
 
     qemu_tcg_init_cpu_signals();
     qemu_thread_get_self(cpu->thread);
@@ -889,6 +915,16 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
             }
         }
         qemu_tcg_wait_io_event();
+        CPU_FOREACH(cpu) {
+            if (cpu->exit && !cpu_can_run(cpu)) {
+                remove_cpu = cpu;
+                break;
+            }
+        }
+        if (remove_cpu) {
+            qemu_tcg_destroy_vcpu(remove_cpu);
+            remove_cpu = NULL;
+        }
     }
 
     return NULL;
@@ -1045,6 +1081,13 @@ void resume_all_vcpus(void)
     }
 }
 
+void cpu_remove(CPUState *cpu)
+{
+    cpu->stop = true;
+    cpu->exit = true;
+    qemu_cpu_kick(cpu);
+}
+
 static void qemu_tcg_init_vcpu(CPUState *cpu)
 {
     /* share a single thread for all cpus with TCG */
@@ -1219,6 +1262,9 @@ static void tcg_exec_all(void)
                 break;
             }
         } else if (cpu->stop || cpu->stopped) {
+            if (cpu->exit) {
+                next_cpu = CPU_NEXT(cpu);
+            }
             break;
         }
     }
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 0e9b5bd..c2cf519 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -611,10 +611,21 @@ 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 *g = &s->gpe_cpu;
+    CPUState *cpu;
+
+    CPU_FOREACH(cpu) {
+        CPUClass *cc = CPU_GET_CLASS(cpu);
+        int64_t id = cc->get_arch_id(cpu);
+
+        if (cpuid == id) {
+            g->old_sts[cpuid / 8] &= ~(1 << (cpuid % 8));
+            cpu_remove(cpu);
+            break;
+        }
+    }
 }
 
 static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
@@ -633,7 +644,7 @@ static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data,
     CPUStatus *cpus = &s->gpe_cpu;
     uint8_t val;
     int i;
-    int64_t cpuid = 0;
+    int64_t cpuid = -1;
 
     val = cpus->old_sts[addr] ^ data;
 
@@ -647,8 +658,8 @@ static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data,
         }
     }
 
-    if (cpuid != 0) {
-        acpi_piix_eject_vcpu(cpuid);
+    if (cpuid != -1) {
+        acpi_piix_eject_vcpu(s, cpuid);
     }
 }
 
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 0238532..eb8d32b 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -181,6 +181,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;
@@ -206,6 +207,7 @@ struct CPUState {
 QTAILQ_HEAD(CPUTailQ, CPUState);
 extern struct CPUTailQ cpus;
 #define CPU_NEXT(cpu) QTAILQ_NEXT(cpu, node)
+#define CPU_REMOVE(cpu) QTAILQ_REMOVE(&cpus, cpu, node)
 #define CPU_FOREACH(cpu) QTAILQ_FOREACH(cpu, &cpus, node)
 #define CPU_FOREACH_SAFE(cpu, next_cpu) \
     QTAILQ_FOREACH_SAFE(cpu, &cpus, node, next_cpu)
@@ -487,6 +489,14 @@ void cpu_exit(CPUState *cpu);
 void cpu_resume(CPUState *cpu);
 
 /**
+ * qemu_remove_vcpu:
+ * @cpu: The vCPU to remove.
+ *
+ * Requests the CPU @cpu to be removed.
+ */
+void cpu_remove(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 52df2a4..dbe56d7 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -225,6 +225,31 @@ 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

Re: [Qemu-devel] [RFC qom-cpu v2 1/8] apic: remove apic_no from apic_init_common()

[Igor Mammedov]

On Tue, 10 Sep 2013 17:43:41 +0800
Chen Fan <chen.fan.fnst@cn.fujitsu.com> wrote:

> the 'apic_no' is increased by one when initialize/create a vCPU each time,
> which causes APICCommonState s->idx always is increased.
> but if we want to re-add a vCPU after removing a vCPU, we need to re-use the
> vacant s->idx which the corresponding vCPU had been removed. 
> so we could use the unique cpu apic_id instead of the progressive s->idx.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
>  hw/intc/apic_common.c | 4 +---
>  target-i386/cpu.c     | 1 +
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index a0beb10..5568621 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -289,13 +289,11 @@ static int apic_init_common(ICCDevice *dev)
>      APICCommonState *s = APIC_COMMON(dev);
>      APICCommonClass *info;
>      static DeviceState *vapic;
> -    static int apic_no;
>      static bool mmio_registered;
>  
> -    if (apic_no >= MAX_APICS) {
> +    if (s->idx >= MAX_APICS) {
>          return -1;
>      }
> -    s->idx = apic_no++;
>  
>      info = APIC_COMMON_GET_CLASS(s);
>      info->init(s);
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 42c5de0..2b99683 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2322,6 +2322,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
>      /* TODO: convert to link<> */
>      apic = APIC_COMMON(env->apic_state);
>      apic->cpu = cpu;
> +    apic->idx = env->cpuid_apic_id;
earlier here we set:
 qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
so apic->idx = env->cpuid_apic_id is redundant.

it would be better to search by apic->id and preferably replace O(MAX_APIC) scans with
a faster approach since for TCG iqr delivery might be a hot path, dropping MAX_APIC
altogether and using dynamic present APICs list.

>  }
>  
>  static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)

Re: [Qemu-devel] [RFC qom-cpu v2 1/8] apic: remove apic_no from apic_init_common()

[Andreas Färber]

Am 10.09.2013 14:09, schrieb Igor Mammedov:
> On Tue, 10 Sep 2013 17:43:41 +0800
> Chen Fan <chen.fan.fnst@cn.fujitsu.com> wrote:
> 
>> the 'apic_no' is increased by one when initialize/create a vCPU each time,
>> which causes APICCommonState s->idx always is increased.
>> but if we want to re-add a vCPU after removing a vCPU, we need to re-use the
>> vacant s->idx which the corresponding vCPU had been removed. 
>> so we could use the unique cpu apic_id instead of the progressive s->idx.
>>
>> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>> ---
>>  hw/intc/apic_common.c | 4 +---
>>  target-i386/cpu.c     | 1 +
>>  2 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
>> index a0beb10..5568621 100644
>> --- a/hw/intc/apic_common.c
>> +++ b/hw/intc/apic_common.c
>> @@ -289,13 +289,11 @@ static int apic_init_common(ICCDevice *dev)
>>      APICCommonState *s = APIC_COMMON(dev);
>>      APICCommonClass *info;
>>      static DeviceState *vapic;
>> -    static int apic_no;
>>      static bool mmio_registered;
>>  
>> -    if (apic_no >= MAX_APICS) {
>> +    if (s->idx >= MAX_APICS) {
>>          return -1;
>>      }
>> -    s->idx = apic_no++;
>>  
>>      info = APIC_COMMON_GET_CLASS(s);
>>      info->init(s);
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index 42c5de0..2b99683 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -2322,6 +2322,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
>>      /* TODO: convert to link<> */
>>      apic = APIC_COMMON(env->apic_state);
>>      apic->cpu = cpu;
>> +    apic->idx = env->cpuid_apic_id;
> earlier here we set:
>  qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
> so apic->idx = env->cpuid_apic_id is redundant.
> 
> it would be better to search by apic->id and preferably replace O(MAX_APIC) scans with
> a faster approach since for TCG iqr delivery might be a hot path, dropping MAX_APIC
> altogether and using dynamic present APICs list.

Independent of that, the recent removal of X86_CPU() cast from
x86_env_get_cpu() should allow us to finally tackle the TODO above,
moving apic_state field from CPUX86State to X86CPU.

Andreas

> 
>>  }
>>  
>>  static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
> 


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

Re: [Qemu-devel] [RFC qom-cpu v2 2/8] x86: add x86_cpu_unrealizefn() for cpu apic remove

[Andreas Färber]

Am 10.09.2013 11:43, schrieb Chen Fan:
> Implement x86_cpu_unrealizefn() for corresponding x86_cpu_realizefn(),
> which is mostly used to clear the apic related information at here.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
>  hw/cpu/icc_bus.c                | 11 +++++++++++
>  hw/i386/kvm/apic.c              |  6 ++++++
>  hw/intc/apic.c                  |  7 +++++++
>  hw/intc/apic_common.c           | 11 +++++++++++
>  include/hw/cpu/icc_bus.h        |  1 +
>  include/hw/i386/apic_internal.h |  1 +
>  target-i386/cpu-qom.h           |  1 +
>  target-i386/cpu.c               | 35 +++++++++++++++++++++++++++++++++++
>  8 files changed, 73 insertions(+)

Some nitpicks below, mostly about adopting the latest concepts.

> diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
> index 8748cc5..45e87d1 100644
> --- a/hw/cpu/icc_bus.c
> +++ b/hw/cpu/icc_bus.c
> @@ -54,11 +54,22 @@ static void icc_device_realize(DeviceState *dev, Error **errp)
>      }
>  }
>  
> +static void icc_device_unrealize(DeviceState *dev, Error **errp)
> +{
> +    ICCDevice *id = ICC_DEVICE(dev);
> +    ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(id);
> +
> +    if (idc->exit) {
> +        idc->exit(id);

->unrealize

> +    }
> +}
> +
>  static void icc_device_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
>  
>      dc->realize = icc_device_realize;
> +    dc->unrealize = icc_device_unrealize;
>      dc->bus_type = TYPE_ICC_BUS;
>  }
>  
> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> index 5609063..8f028a1 100644
> --- a/hw/i386/kvm/apic.c
> +++ b/hw/i386/kvm/apic.c
> @@ -181,11 +181,17 @@ static void kvm_apic_init(APICCommonState *s)
>      }
>  }
>  
> +static void kvm_apic_exit(APICCommonState *s)

kvm_apic_unrealize

> +{
> +    memory_region_destroy(&s->io_memory);
> +}
> +
>  static void kvm_apic_class_init(ObjectClass *klass, void *data)
>  {
>      APICCommonClass *k = APIC_COMMON_CLASS(klass);
>  
>      k->init = kvm_apic_init;
> +    k->exit = kvm_apic_exit;
>      k->set_base = kvm_apic_set_base;
>      k->set_tpr = kvm_apic_set_tpr;
>      k->get_tpr = kvm_apic_get_tpr;
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index a913186..23488b4 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -882,11 +882,18 @@ static void apic_init(APICCommonState *s)
>      msi_supported = true;
>  }
>  
> +static void apic_uninit(APICCommonState *s)

apic_unrealize

> +{
> +    memory_region_destroy(&s->io_memory);
> +    local_apics[s->idx] = NULL;
> +}
> +
>  static void apic_class_init(ObjectClass *klass, void *data)
>  {
>      APICCommonClass *k = APIC_COMMON_CLASS(klass);
>  
>      k->init = apic_init;
> +    k->exit = apic_uninit;
>      k->set_base = apic_set_base;
>      k->set_tpr = apic_set_tpr;
>      k->get_tpr = apic_get_tpr;
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 5568621..32c2f74 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -316,6 +316,16 @@ static int apic_init_common(ICCDevice *dev)
>      return 0;
>  }
>  
> +static void apic_exit_common(ICCDevice *dev)
> +{
> +    APICCommonState *s = APIC_COMMON(dev);
> +    APICCommonClass *info;

acc please

> +
> +    info = APIC_COMMON_GET_CLASS(s);
> +    if (info->exit)
> +        info->exit(s);

Braces missing -> checkpatch.pl

> +}
> +
>  static void apic_dispatch_pre_save(void *opaque)
>  {
>      APICCommonState *s = APIC_COMMON(opaque);
> @@ -387,6 +397,7 @@ static void apic_common_class_init(ObjectClass *klass, void *data)
>      dc->no_user = 1;
>      dc->props = apic_properties_common;
>      idc->init = apic_init_common;
> +    idc->exit = apic_exit_common;
>  }
>  
>  static const TypeInfo apic_common_type = {
> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
> index b550070..15d5374 100644
> --- a/include/hw/cpu/icc_bus.h
> +++ b/include/hw/cpu/icc_bus.h
> @@ -67,6 +67,7 @@ typedef struct ICCDeviceClass {
>      /*< public >*/
>  
>      int (*init)(ICCDevice *dev); /* TODO replace with QOM realize */
> +    void (*exit)(ICCDevice *dev);

DeviceUnrealize unrealize;

>  } ICCDeviceClass;
>  
>  #define TYPE_ICC_DEVICE "icc-device"
> diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
> index 1b0a7fb..87d5248 100644
> --- a/include/hw/i386/apic_internal.h
> +++ b/include/hw/i386/apic_internal.h
> @@ -81,6 +81,7 @@ typedef struct APICCommonClass
>      ICCDeviceClass parent_class;
>  
>      void (*init)(APICCommonState *s);
> +    void (*exit)(APICCommonState *s);

DeviceUnrealize unrealize;

>      void (*set_base)(APICCommonState *s, uint64_t val);
>      void (*set_tpr)(APICCommonState *s, uint8_t val);
>      uint8_t (*get_tpr)(APICCommonState *s);
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index c4447c2..1e520be 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -50,6 +50,7 @@ typedef struct X86CPUClass {
>      /*< public >*/
>  
>      DeviceRealize parent_realize;
> +    DeviceUnrealize parent_unrealize;
>      void (*parent_reset)(CPUState *cpu);
>  } X86CPUClass;
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 2b99683..6f9154d 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2339,10 +2339,31 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
>          return;
>      }
>  }
> +
> +static void x86_cpu_apic_unrealize(X86CPU *cpu, Error **errp)
> +{
> +    CPUX86State *env = &cpu->env;
> +    Error *local_err = NULL;
> +
> +    if (env->apic_state == NULL) {
> +        return;
> +    }
> +
> +    object_property_set_bool(OBJECT(env->apic_state), false, "realized", &local_err);
> +    if (local_err != NULL) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    qdev_free(env->apic_state);
> +}
>  #else
>  static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
>  {
>  }
> +static void x86_cpu_apic_unrealize(X86CPU *cpu, Error **errp)
> +{
> +}
>  #endif
>  
>  static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> @@ -2418,6 +2439,18 @@ out:
>      }
>  }
>  
> +static void x86_cpu_unrealizefn(DeviceState *dev, Error **errp)
> +{
> +    X86CPU *cpu = X86_CPU(dev);
> +    Error *local_err = NULL;
> +
> +    x86_cpu_apic_unrealize(cpu, &local_err);
> +    if (local_err != NULL) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +}
> +
>  /* Enables contiguous-apic-ID mode, for compatibility */
>  static bool compat_apic_id_mode;
>  
> @@ -2549,7 +2582,9 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>      DeviceClass *dc = DEVICE_CLASS(oc);
>  
>      xcc->parent_realize = dc->realize;
> +    xcc->parent_unrealize = dc->unrealize;
>      dc->realize = x86_cpu_realizefn;
> +    dc->unrealize = x86_cpu_unrealizefn;
>      dc->bus_type = TYPE_ICC_BUS;
>      dc->props = x86_cpu_properties;
>  

Regards,
Andreas

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

Re: [Qemu-devel] [RFC qom-cpu v2 3/8] qmp: add 'cpu-del' command support

[Eric Blake]

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

On 09/10/2013 03:43 AM, 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 0c313fe..3de9c51 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -957,6 +957,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. */

Even if you implement things later, it's better to at least set errp
here rather than having a command that silently does nothing but
succeeds.  Or squash the two patches together so that you are providing
functionality at the time you expose the QMP.

Otherwise, looks fine from the QMP perspective.

-- 
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 --]

Re: [Qemu-devel] [RFC qom-cpu v2 3/8] qmp: add 'cpu-del' command support

[chenfan]

On Tue, 2013-09-10 at 09:52 -0600, Eric Blake wrote:
> On 09/10/2013 03:43 AM, 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 0c313fe..3de9c51 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -957,6 +957,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. */
> 
> Even if you implement things later, it's better to at least set errp
> here rather than having a command that silently does nothing but
> succeeds.  Or squash the two patches together so that you are providing
> functionality at the time you expose the QMP.
I should temporarily set some errp here in this patch. Thanks for you
comment.

> 
> Otherwise, looks fine from the QMP perspective.
> 

Re: [Qemu-devel] [RFC qom-cpu v2 1/8] apic: remove apic_no from apic_init_common()

[chenfan]

On Tue, 2013-09-10 at 14:16 +0200, Andreas Färber wrote:
> Am 10.09.2013 14:09, schrieb Igor Mammedov:
> > On Tue, 10 Sep 2013 17:43:41 +0800
> > Chen Fan <chen.fan.fnst@cn.fujitsu.com> wrote:
> > 
> >> the 'apic_no' is increased by one when initialize/create a vCPU each time,
> >> which causes APICCommonState s->idx always is increased.
> >> but if we want to re-add a vCPU after removing a vCPU, we need to re-use the
> >> vacant s->idx which the corresponding vCPU had been removed. 
> >> so we could use the unique cpu apic_id instead of the progressive s->idx.
> >>
> >> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> >> ---
> >>  hw/intc/apic_common.c | 4 +---
> >>  target-i386/cpu.c     | 1 +
> >>  2 files changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> >> index a0beb10..5568621 100644
> >> --- a/hw/intc/apic_common.c
> >> +++ b/hw/intc/apic_common.c
> >> @@ -289,13 +289,11 @@ static int apic_init_common(ICCDevice *dev)
> >>      APICCommonState *s = APIC_COMMON(dev);
> >>      APICCommonClass *info;
> >>      static DeviceState *vapic;
> >> -    static int apic_no;
> >>      static bool mmio_registered;
> >>  
> >> -    if (apic_no >= MAX_APICS) {
> >> +    if (s->idx >= MAX_APICS) {
> >>          return -1;
> >>      }
> >> -    s->idx = apic_no++;
> >>  
> >>      info = APIC_COMMON_GET_CLASS(s);
> >>      info->init(s);
> >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> >> index 42c5de0..2b99683 100644
> >> --- a/target-i386/cpu.c
> >> +++ b/target-i386/cpu.c
> >> @@ -2322,6 +2322,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
> >>      /* TODO: convert to link<> */
> >>      apic = APIC_COMMON(env->apic_state);
> >>      apic->cpu = cpu;
> >> +    apic->idx = env->cpuid_apic_id;
> > earlier here we set:
> >  qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
> > so apic->idx = env->cpuid_apic_id is redundant.
> > 
> > it would be better to search by apic->id and preferably replace O(MAX_APIC) scans with
> > a faster approach since for TCG iqr delivery might be a hot path, dropping MAX_APIC
> > altogether and using dynamic present APICs list.
> 
> Independent of that, the recent removal of X86_CPU() cast from
> x86_env_get_cpu() should allow us to finally tackle the TODO above,
> moving apic_state field from CPUX86State to X86CPU.
> 
I will send V3 patches later including modification pointed out above. 

Thanks

> Andreas
> 
> > 
> >>  }
> >>  
> >>  static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
> > 
> 
>