[Qemu-devel] [PATCH 0/6] cpu: add device_add foo-x86_64-cpu support

[Qemu-devel] [PATCH 0/6] cpu: add device_add foo-x86_64-cpu support

[Gu Zheng]

This series is based on the previous patchset from Chen Fan:
https://lists.nongnu.org/archive/html/qemu-devel/2014-05/msg02360.html

We try to make cpu hotplug with device_add, and make
"-device foo-x86_64-cpu" available，also we can set apic-id
property with command line, if without setting apic-id property,
we offer the first unoccupied apic id as the default new apic id.
When hotplug cpu with device_add, additional check of APIC ID will be
done after cpu object initialization which was different from
'cpu_add' command that check 'ids' at the beginning.

---
Changelog since RFC:
 -split out APIC vmstate/QMP-monitor changes into separate patches.
 -add the handle of the startup cpus(-device foo).
 -remove duplicated checking about env->cpuid_apic_id.
 -do actual APIC ID allocation at realize time if it is not set before.
 -remove the unneeded x86_cpu_cpudef_instance_init().
 -split off device_del support out here.
---

Chen Fan (2):
  cpu: introduce CpuTopoInfo structure for argument simplification
  cpu: add device_add foo-x86_64-cpu support

Gu Zheng (4):
  qom/cpu: move register_vmstate to common CPUClass.realizefn
  qom/cpu: move apic vmstate register into x86_cpu_apic_realize
  monitor: use cc->get_arch_id as the cpu index
  acpi:cpu hotplug: set pcmachine as icc bus' hotplug handler

 cpus.c                          |    4 ++-
 exec.c                          |   32 +++++++++++++---------
 hw/acpi/cpu_hotplug.c           |    5 +++-
 hw/cpu/icc_bus.c                |   15 -----------
 hw/i386/pc.c                    |    6 ----
 hw/i386/pc_piix.c               |    6 +++-
 hw/i386/pc_q35.c                |    5 +++
 hw/intc/apic_common.c           |    3 +-
 include/hw/cpu/icc_bus.h        |   14 ++++++++++
 include/hw/i386/apic_internal.h |    3 ++
 include/qom/cpu.h               |    2 +
 monitor.c                       |    4 ++-
 qom/cpu.c                       |    2 +
 target-i386/cpu.c               |   54 +++++++++++++++++++++++++++++++++++---
 target-i386/topology.h          |   51 +++++++++++++++++++++++++-----------
 15 files changed, 145 insertions(+), 61 deletions(-)

-- 
1.7.7

[Qemu-devel] [PATCH 1/6] cpu: introduce CpuTopoInfo structure for argument simplification

[Gu Zheng]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 target-i386/topology.h |   33 +++++++++++++++++----------------
 1 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/target-i386/topology.h b/target-i386/topology.h
index 07a6c5f..e9ff89c 100644
--- a/target-i386/topology.h
+++ b/target-i386/topology.h
@@ -47,6 +47,12 @@
  */
 typedef uint32_t apic_id_t;
 
+typedef struct X86CPUTopoInfo {
+    unsigned pkg_id;
+    unsigned core_id;
+    unsigned smt_id;
+} X86CPUTopoInfo;
+
 /* Return the bit width needed for 'count' IDs
  */
 static unsigned apicid_bitwidth_for_count(unsigned count)
@@ -92,13 +98,11 @@ static inline unsigned apicid_pkg_offset(unsigned nr_cores, unsigned nr_threads)
  */
 static inline apic_id_t apicid_from_topo_ids(unsigned nr_cores,
                                              unsigned nr_threads,
-                                             unsigned pkg_id,
-                                             unsigned core_id,
-                                             unsigned smt_id)
+                                             const X86CPUTopoInfo *topo)
 {
-    return (pkg_id  << apicid_pkg_offset(nr_cores, nr_threads)) |
-           (core_id << apicid_core_offset(nr_cores, nr_threads)) |
-           smt_id;
+    return (topo->pkg_id  << apicid_pkg_offset(nr_cores, nr_threads)) |
+           (topo->core_id << apicid_core_offset(nr_cores, nr_threads)) |
+           topo->smt_id;
 }
 
 /* Calculate thread/core/package IDs for a specific topology,
@@ -107,14 +111,12 @@ static inline apic_id_t apicid_from_topo_ids(unsigned nr_cores,
 static inline void x86_topo_ids_from_idx(unsigned nr_cores,
                                          unsigned nr_threads,
                                          unsigned cpu_index,
-                                         unsigned *pkg_id,
-                                         unsigned *core_id,
-                                         unsigned *smt_id)
+                                         X86CPUTopoInfo *topo)
 {
     unsigned core_index = cpu_index / nr_threads;
-    *smt_id = cpu_index % nr_threads;
-    *core_id = core_index % nr_cores;
-    *pkg_id = core_index / nr_cores;
+    topo->smt_id = cpu_index % nr_threads;
+    topo->core_id = core_index % nr_cores;
+    topo->pkg_id = core_index / nr_cores;
 }
 
 /* Make APIC ID for the CPU 'cpu_index'
@@ -125,10 +127,9 @@ static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_cores,
                                                 unsigned nr_threads,
                                                 unsigned cpu_index)
 {
-    unsigned pkg_id, core_id, smt_id;
-    x86_topo_ids_from_idx(nr_cores, nr_threads, cpu_index,
-                          &pkg_id, &core_id, &smt_id);
-    return apicid_from_topo_ids(nr_cores, nr_threads, pkg_id, core_id, smt_id);
+    X86CPUTopoInfo topo;
+    x86_topo_ids_from_idx(nr_cores, nr_threads, cpu_index, &topo);
+    return apicid_from_topo_ids(nr_cores, nr_threads, &topo);
 }
 
 #endif /* TARGET_I386_TOPOLOGY_H */
-- 
1.7.7

[Qemu-devel] [PATCH 2/6] qom/cpu: move register_vmstate to common CPUClass.realizefn

[Gu Zheng]

Move cpu vmstate register from cpu_exec_init into cpu_common_realizefn,
and use cc->get_arch_id as the instance id that suggested by Igor to
fix the migration issue.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 exec.c            |   32 +++++++++++++++++++-------------
 include/qom/cpu.h |    2 ++
 qom/cpu.c         |    2 ++
 3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/exec.c b/exec.c
index 759055d..6823f36 100644
--- a/exec.c
+++ b/exec.c
@@ -508,10 +508,28 @@ void tcg_cpu_address_space_init(CPUState *cpu, AddressSpace *as)
 }
 #endif
 
+void cpu_vmstate_register(CPUState *cpu)
+{
+    CPUClass *cc = CPU_GET_CLASS(cpu);
+    int cpu_index = cc->get_arch_id(cpu);
+
+    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
+        vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu);
+    }
+#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
+    register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
+                    cpu_save, cpu_load, cpu->env_ptr);
+    assert(cc->vmsd == NULL);
+    assert(qdev_get_vmsd(DEVICE(cpu)) == NULL);
+#endif
+    if (cc->vmsd != NULL) {
+        vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
+    }
+}
+
 void cpu_exec_init(CPUArchState *env)
 {
     CPUState *cpu = ENV_GET_CPU(env);
-    CPUClass *cc = CPU_GET_CLASS(cpu);
     CPUState *some_cpu;
     int cpu_index;
 
@@ -534,18 +552,6 @@ void cpu_exec_init(CPUArchState *env)
 #if defined(CONFIG_USER_ONLY)
     cpu_list_unlock();
 #endif
-    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
-        vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu);
-    }
-#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
-    register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
-                    cpu_save, cpu_load, env);
-    assert(cc->vmsd == NULL);
-    assert(qdev_get_vmsd(DEVICE(cpu)) == NULL);
-#endif
-    if (cc->vmsd != NULL) {
-        vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
-    }
 }
 
 #if defined(TARGET_HAS_ICE)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 2098f1c..936afcd 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -562,6 +562,8 @@ void cpu_interrupt(CPUState *cpu, int mask);
 
 #endif /* USER_ONLY */
 
+void cpu_vmstate_register(CPUState *cpu);
+
 #ifdef CONFIG_SOFTMMU
 static inline void cpu_unassigned_access(CPUState *cpu, hwaddr addr,
                                          bool is_write, bool is_exec,
diff --git a/qom/cpu.c b/qom/cpu.c
index 79d2228..00c1007 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -301,6 +301,8 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cpu = CPU(dev);
 
+    cpu_vmstate_register(cpu);
+
     if (dev->hotplugged) {
         cpu_synchronize_post_init(cpu);
         cpu_resume(cpu);
-- 
1.7.7

[Qemu-devel] [PATCH 3/6] qom/cpu: move apic vmstate register into x86_cpu_apic_realize

[Gu Zheng]

move apic vmstate register into x86_cpu_apic_realize, and use
cc->get_arch_id as the instance id to avoid using the auto-id which will
break the migration if we add device not in order.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 hw/intc/apic_common.c           |    3 +--
 include/hw/i386/apic_internal.h |    3 +++
 target-i386/cpu.c               |    8 +++++++-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index ce3d903..029f67d 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -345,7 +345,7 @@ static int apic_dispatch_post_load(void *opaque, int version_id)
     return 0;
 }
 
-static const VMStateDescription vmstate_apic_common = {
+const VMStateDescription vmstate_apic_common = {
     .name = "apic",
     .version_id = 3,
     .minimum_version_id = 3,
@@ -391,7 +391,6 @@ static void apic_common_class_init(ObjectClass *klass, void *data)
     ICCDeviceClass *idc = ICC_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    dc->vmsd = &vmstate_apic_common;
     dc->reset = apic_reset_common;
     dc->props = apic_properties_common;
     idc->realize = apic_common_realize;
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 83e2a42..61fddf6 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -23,6 +23,7 @@
 #include "exec/memory.h"
 #include "hw/cpu/icc_bus.h"
 #include "qemu/timer.h"
+#include "migration/vmstate.h"
 
 /* APIC Local Vector Table */
 #define APIC_LVT_TIMER                  0
@@ -137,6 +138,8 @@ typedef struct VAPICState {
 
 extern bool apic_report_tpr_access;
 
+extern const VMStateDescription vmstate_apic_common;
+
 void apic_report_irq_delivered(int delivered);
 bool apic_next_timer(APICCommonState *s, int64_t current_time);
 void apic_enable_tpr_access_reporting(DeviceState *d, bool enable);
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e4ccee1..0f23ba3 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2678,10 +2678,16 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
 
 static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
 {
-    if (cpu->apic_state == NULL) {
+    DeviceState *apic_state = cpu->apic_state;
+    CPUClass *cc = CPU_GET_CLASS(CPU(cpu));
+
+    if (apic_state == NULL) {
         return;
     }
 
+    vmstate_register(0, cc->get_arch_id(CPU(cpu)),
+            &vmstate_apic_common, apic_state);
+
     if (qdev_init(cpu->apic_state)) {
         error_setg(errp, "APIC device '%s' could not be initialized",
                    object_get_typename(OBJECT(cpu->apic_state)));
-- 
1.7.7

[Qemu-devel] [PATCH 4/6] monitor: use cc->get_arch_id as the cpu index

[Gu Zheng]

Use cc->get_arch_id as the cpu index to avoid the cpu index duplicated
issue in the QMP/HMP command output.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 cpus.c    |    4 +++-
 monitor.c |    4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index 0c33458..849d1bf 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1411,6 +1411,7 @@ CpuInfoList *qmp_query_cpus(Error **errp)
 
     CPU_FOREACH(cpu) {
         CpuInfoList *info;
+        CPUClass *cc;
 #if defined(TARGET_I386)
         X86CPU *x86_cpu = X86_CPU(cpu);
         CPUX86State *env = &x86_cpu->env;
@@ -1428,11 +1429,12 @@ CpuInfoList *qmp_query_cpus(Error **errp)
         CPUTriCoreState *env = &tricore_cpu->env;
 #endif
 
+        cc = CPU_GET_CLASS(cpu);
         cpu_synchronize_state(cpu);
 
         info = g_malloc0(sizeof(*info));
         info->value = g_malloc0(sizeof(*info->value));
-        info->value->CPU = cpu->cpu_index;
+        info->value->CPU = cc->get_arch_id(cpu);
         info->value->current = (cpu == first_cpu);
         info->value->halted = cpu->halted;
         info->value->thread_id = cpu->thread_id;
diff --git a/monitor.c b/monitor.c
index 1fc201a..fdd51fa 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1024,7 +1024,9 @@ static CPUArchState *mon_get_cpu(void)
 int monitor_get_cpu_index(void)
 {
     CPUState *cpu = ENV_GET_CPU(mon_get_cpu());
-    return cpu->cpu_index;
+    CPUClass *cc = CPU_GET_CLASS(cpu);
+
+    return cc->get_arch_id(cpu);
 }
 
 static void do_info_registers(Monitor *mon, const QDict *qdict)
-- 
1.7.7

[Qemu-devel] [PATCH 5/6] acpi:cpu hotplug: set pcmachine as icc bus' hotplug handler

[Gu Zheng]

As the pre-check in the qdev_device_add():
    if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) {
        qerror_report(QERR_BUS_NO_HOTPLUG, bus->name);
        return NULL;
    }
if device has parent bus, the bus must have valid hotplug_handler,
otherwise can not hot plug.
Currently cpu hotplug is based on the PCMachine's hotplug handler,
so when hot add cpu, the hotpluggable check of icc bus will be
rejected.
So we set pcmachine as icc bus' hotplug handler to avoid the rejetion.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 hw/cpu/icc_bus.c         |   15 ---------------
 hw/i386/pc_piix.c        |    6 +++++-
 hw/i386/pc_q35.c         |    5 +++++
 include/hw/cpu/icc_bus.h |   14 ++++++++++++++
 4 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index 9575fd6..1c55a07 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -20,7 +20,6 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>
  */
 #include "hw/cpu/icc_bus.h"
-#include "hw/sysbus.h"
 
 /* icc-bridge implementation */
 
@@ -61,20 +60,6 @@ static const TypeInfo icc_device_info = {
     .class_init = icc_device_class_init,
 };
 
-
-/*  icc-bridge implementation */
-
-typedef struct ICCBridgeState {
-    /*< private >*/
-    SysBusDevice parent_obj;
-    /*< public >*/
-
-    ICCBus icc_bus;
-    MemoryRegion apic_container;
-} ICCBridgeState;
-
-#define ICC_BRIGDE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE)
-
 static void icc_bridge_init(Object *obj)
 {
     ICCBridgeState *s = ICC_BRIGDE(obj);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 537bcf2..6bb9ccc 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -276,7 +276,7 @@ static void pc_init1(MachineState *machine,
     if (pci_enabled && acpi_enabled) {
         DeviceState *piix4_pm;
         I2CBus *smbus;
-
+        ICCBus *iccbus;
         smi_irq = qemu_allocate_irqs(pc_acpi_smi_interrupt, first_cpu, 1);
         /* TODO: Populate SPD eeprom data.  */
         smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
@@ -291,6 +291,10 @@ static void pc_init1(MachineState *machine,
                                  OBJ_PROP_LINK_UNREF_ON_RELEASE, &error_abort);
         object_property_set_link(OBJECT(machine), OBJECT(piix4_pm),
                                  PC_MACHINE_ACPI_DEVICE_PROP, &error_abort);
+
+        iccbus = &ICC_BRIGDE(icc_bridge)->icc_bus;
+        object_property_set_link(OBJECT(iccbus), OBJECT(pc_machine),
+                                 QDEV_HOTPLUG_HANDLER_PROPERTY, &error_abort);
     }
 
     if (pci_enabled) {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 296bdec..443603d 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -88,6 +88,7 @@ static void pc_q35_init(MachineState *machine)
     PcGuestInfo *guest_info;
     ram_addr_t lowmem;
     DriveInfo *hd[MAX_SATA_PORTS];
+    ICCBus *iccbus;
 
     /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
      * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
@@ -212,6 +213,10 @@ static void pc_q35_init(MachineState *machine)
     object_property_set_link(OBJECT(machine), OBJECT(lpc),
                              PC_MACHINE_ACPI_DEVICE_PROP, &error_abort);
 
+    iccbus = &ICC_BRIGDE(icc_bridge)->icc_bus;
+    object_property_set_link(OBJECT(iccbus), OBJECT(pc_machine),
+                             QDEV_HOTPLUG_HANDLER_PROPERTY, &error_abort);
+
     ich9_lpc = ICH9_LPC_DEVICE(lpc);
     ich9_lpc->pic = gsi;
     ich9_lpc->ioapic = gsi_state->ioapic_irq;
diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
index 98a979f..8354c96 100644
--- a/include/hw/cpu/icc_bus.h
+++ b/include/hw/cpu/icc_bus.h
@@ -24,6 +24,7 @@
 
 #include "exec/memory.h"
 #include "hw/qdev-core.h"
+#include "hw/sysbus.h"
 
 #define TYPE_ICC_BUS "icc-bus"
 
@@ -44,6 +45,19 @@ typedef struct ICCBus {
 
 #define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS)
 
+/*  icc-bridge implementation */
+
+typedef struct ICCBridgeState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    ICCBus icc_bus;
+    MemoryRegion apic_container;
+} ICCBridgeState;
+
+#define ICC_BRIGDE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE)
+
 /**
  * ICCDevice:
  *
-- 
1.7.7

[Qemu-devel] [PATCH 6/6] cpu: add device_add foo-x86_64-cpu support

[Gu Zheng]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

Add support to device_add foo-x86_64-cpu, and additional checks of
apic id are added into x86_cpuid_set_apic_id() to avoid duplicate.
Besides, in order to support "device/device_add foo-x86_64-cpu"
which without specified apic id, we assign cpuid_apic_id with a
default broadcast value (0xFFFFFFFF) in initfn, and a new function
get_free_apic_id() to provide a free apid id to cpuid_apic_id if
it still has the default at realize time (e.g. hot add foo-cpu without
a specified apic id) to avoid apic id duplicates.

Thanks very much for Igor's suggestion.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 hw/acpi/cpu_hotplug.c  |    5 ++++-
 hw/i386/pc.c           |    6 ------
 target-i386/cpu.c      |   46 ++++++++++++++++++++++++++++++++++++++++++----
 target-i386/topology.h |   18 ++++++++++++++++++
 4 files changed, 64 insertions(+), 11 deletions(-)

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index b8ebfad..3dbfd71 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -60,7 +60,10 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
     }
 
     ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS;
-    acpi_update_sci(ar, irq);
+
+    /* Only trigger sci if cpu is hotplugged */
+    if (dev->hotplugged)
+        acpi_update_sci(ar, irq);
 }
 
 void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 1205db8..08e169d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1615,13 +1615,7 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
     Error *local_err = NULL;
     PCMachineState *pcms = PC_MACHINE(hotplug_dev);
 
-    if (!dev->hotplugged) {
-        goto out;
-    }
-
     if (!pcms->acpi_dev) {
-        error_setg(&local_err,
-                   "cpu hotplug is not enabled: missing acpi device");
         goto out;
     }
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0f23ba3..d0eb3c9 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -49,7 +49,6 @@
 #include "hw/i386/apic_internal.h"
 #endif
 
-
 /* Cache topology CPUID constants: */
 
 /* CPUID Leaf 2 Descriptors */
@@ -1633,6 +1632,7 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
     const int64_t max = UINT32_MAX;
     Error *error = NULL;
     int64_t value;
+    X86CPUTopoInfo topo;
 
     if (dev->realized) {
         error_setg(errp, "Attempt to set property '%s' on '%s' after "
@@ -1652,6 +1652,19 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
         return;
     }
 
+    if (value > x86_cpu_apic_id_from_index(max_cpus - 1)) {
+        error_setg(errp, "CPU with APIC ID %" PRIi64
+                   " is more than MAX APIC ID limits", value);
+        return;
+    }
+
+    x86_topo_ids_from_apic_id(smp_cores, smp_threads, value, &topo);
+    if (topo.smt_id >= smp_threads || topo.core_id >= smp_cores) {
+        error_setg(errp, "CPU with APIC ID %" PRIi64 " does not match "
+                   "topology configuration.", value);
+        return;
+    }
+
     if ((value != cpu->env.cpuid_apic_id) && cpu_exists(value)) {
         error_setg(errp, "CPU with APIC ID %" PRIi64 " exists", value);
         return;
@@ -2106,8 +2119,10 @@ static void x86_cpu_cpudef_class_init(ObjectClass *oc, void *data)
 {
     X86CPUDefinition *cpudef = data;
     X86CPUClass *xcc = X86_CPU_CLASS(oc);
+    DeviceClass *dc = DEVICE_CLASS(oc);
 
     xcc->cpu_def = cpudef;
+    dc->cannot_instantiate_with_device_add_yet = false;
 }
 
 static void x86_register_cpudef_type(X86CPUDefinition *def)
@@ -2116,6 +2131,7 @@ static void x86_register_cpudef_type(X86CPUDefinition *def)
     TypeInfo ti = {
         .name = typename,
         .parent = TYPE_X86_CPU,
+        .instance_size = sizeof(X86CPU),
         .class_init = x86_cpu_cpudef_class_init,
         .class_data = def,
     };
@@ -2649,12 +2665,27 @@ static void mce_init(X86CPU *cpu)
 }
 
 #ifndef CONFIG_USER_ONLY
+static uint32_t get_free_apic_id(void)
+{
+    int i;
+
+    for (i = 0; i < max_cpus; i++) {
+        uint32_t id = x86_cpu_apic_id_from_index(i);
+
+        if (!cpu_exists(id)) {
+            return id;
+        }
+    }
+
+    return x86_cpu_apic_id_from_index(max_cpus);
+}
+
 static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
 {
-    CPUX86State *env = &cpu->env;
     DeviceState *dev = DEVICE(cpu);
     APICCommonState *apic;
     const char *apic_type = "apic";
+    uint32_t apic_id;
 
     if (kvm_irqchip_in_kernel()) {
         apic_type = "kvm-apic";
@@ -2670,7 +2701,14 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
 
     object_property_add_child(OBJECT(cpu), "apic",
                               OBJECT(cpu->apic_state), NULL);
-    qdev_prop_set_uint8(cpu->apic_state, "id", env->cpuid_apic_id);
+
+    apic_id = object_property_get_int(OBJECT(cpu), "apic-id", NULL);
+    if (apic_id == 0xffffffff) {
+        apic_id = get_free_apic_id();
+        object_property_set_int(OBJECT(cpu), apic_id, "apic-id", errp);
+    }
+
+    qdev_prop_set_uint8(cpu->apic_state, "id", apic_id);
     /* TODO: convert to link<> */
     apic = APIC_COMMON(cpu->apic_state);
     apic->cpu = cpu;
@@ -2859,7 +2897,7 @@ static void x86_cpu_initfn(Object *obj)
                         NULL, NULL, (void *)cpu->filtered_features, NULL);
 
     cpu->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY;
-    env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index);
+    env->cpuid_apic_id = 0xffffffff;
 
     x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort);
 
diff --git a/target-i386/topology.h b/target-i386/topology.h
index e9ff89c..dcb4988 100644
--- a/target-i386/topology.h
+++ b/target-i386/topology.h
@@ -132,4 +132,22 @@ static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_cores,
     return apicid_from_topo_ids(nr_cores, nr_threads, &topo);
 }
 
+/* Calculate CPU topology based on CPU APIC ID.
+ */
+static inline void x86_topo_ids_from_apic_id(unsigned nr_cores,
+                                             unsigned nr_threads,
+                                             apic_id_t apic_id,
+                                             X86CPUTopoInfo *topo)
+{
+    unsigned offset_mask;
+    topo->pkg_id = apic_id >> apicid_pkg_offset(nr_cores, nr_threads);
+
+    offset_mask = (1L << apicid_pkg_offset(nr_cores, nr_threads)) - 1;
+    topo->core_id = (apic_id & offset_mask)
+                     >> apicid_core_offset(nr_cores, nr_threads);
+
+    offset_mask = (1L << apicid_core_offset(nr_cores, nr_threads)) - 1;
+    topo->smt_id = apic_id & offset_mask;
+}
+
 #endif /* TARGET_I386_TOPOLOGY_H */
-- 
1.7.7

Re: [Qemu-devel] [PATCH 0/6] cpu: add device_add foo-x86_64-cpu support

[Gu Zheng]

ping...

On 11/13/2014 09:10 AM, Gu Zheng wrote:

> This series is based on the previous patchset from Chen Fan:
> https://lists.nongnu.org/archive/html/qemu-devel/2014-05/msg02360.html
> 
> We try to make cpu hotplug with device_add, and make
> "-device foo-x86_64-cpu" available，also we can set apic-id
> property with command line, if without setting apic-id property,
> we offer the first unoccupied apic id as the default new apic id.
> When hotplug cpu with device_add, additional check of APIC ID will be
> done after cpu object initialization which was different from
> 'cpu_add' command that check 'ids' at the beginning.
> 
> ---
> Changelog since RFC:
>  -split out APIC vmstate/QMP-monitor changes into separate patches.
>  -add the handle of the startup cpus(-device foo).
>  -remove duplicated checking about env->cpuid_apic_id.
>  -do actual APIC ID allocation at realize time if it is not set before.
>  -remove the unneeded x86_cpu_cpudef_instance_init().
>  -split off device_del support out here.
> ---
> 
> Chen Fan (2):
>   cpu: introduce CpuTopoInfo structure for argument simplification
>   cpu: add device_add foo-x86_64-cpu support
> 
> Gu Zheng (4):
>   qom/cpu: move register_vmstate to common CPUClass.realizefn
>   qom/cpu: move apic vmstate register into x86_cpu_apic_realize
>   monitor: use cc->get_arch_id as the cpu index
>   acpi:cpu hotplug: set pcmachine as icc bus' hotplug handler
> 
>  cpus.c                          |    4 ++-
>  exec.c                          |   32 +++++++++++++---------
>  hw/acpi/cpu_hotplug.c           |    5 +++-
>  hw/cpu/icc_bus.c                |   15 -----------
>  hw/i386/pc.c                    |    6 ----
>  hw/i386/pc_piix.c               |    6 +++-
>  hw/i386/pc_q35.c                |    5 +++
>  hw/intc/apic_common.c           |    3 +-
>  include/hw/cpu/icc_bus.h        |   14 ++++++++++
>  include/hw/i386/apic_internal.h |    3 ++
>  include/qom/cpu.h               |    2 +
>  monitor.c                       |    4 ++-
>  qom/cpu.c                       |    2 +
>  target-i386/cpu.c               |   54 +++++++++++++++++++++++++++++++++++---
>  target-i386/topology.h          |   51 +++++++++++++++++++++++++-----------
>  15 files changed, 145 insertions(+), 61 deletions(-)
>