[Qemu-devel] [PATCH 0/7] pc/spapr: unify handling of possible CPUs

[Qemu-devel] [PATCH 0/7] pc/spapr: unify handling of possible CPUs

[Igor Mammedov]

Series is prerequsite split of from NUMA cpu mapping
refactoring and includes only possible_cpus[] generalization
to make it reusable for SPAPR and virt-arm targets as
has been requested by Eduardo [1].

Since virt-arm doesn't support -device based CPUs, ARM
patches are not included here and will be posted separately as
 'hw/arm/virt: -device cpu support'
series with dependency on this series.

This series doesn't include any of NUMA refactoring patches,
which will be posted on top once SPAPR and virt-arm would use
possible_cpus[].

Hence goal of this series is to make x86 specific possible_cpus[]
more generic, move it to MachineState and reuse it in SPAPR
machine so we could use it as common storage for topology
information when parsing -smp/-numa and then follow
the same/similar CPU creation pattern across the affected targets.

As result of this series:
  * possible_cpus[] becomes part of MachineState
  * SPAPR machine reuses it for persistent topololy storage
    and core bookkeeping
  * target specific query_hotpluggable_cpus() callbacks are
    generalized into common machine_query_hotpluggable_cpus()

Series depends on:
  [PATCH 0/3] spapr: fix cpu core hotunplug call flow
  https://www.mail-archive.com/qemu-devel@nongnu.org/msg427214.html
which is staged for next pull req in SPAPR tree.

git tree for testing:
  git@github.com:imammedo/qemu.git unify_poissible_cpus_v1

Regression tested CPU hotplug with ppc fedora25 guest on ppc host
with kvm enabled.

 1) "[RFC 00/13] numa: add '-numa cpu' option"
    https://www.mail-archive.com/qemu-devel@nongnu.org/msg423883.html

CC: David Gibson <david@gibson.dropbear.id.au> (supporter:sPAPR)
CC: Alexander Graf <agraf@suse.de> (supporter:sPAPR)
CC: qemu-ppc@nongnu.org 
CC: Bharata B Rao <bharata@linux.vnet.ibm.com>
CC: ehabkost@redhat.com
CC: drjones@redhat.com
CC: Marcel Apfelbaum <marcel@redhat.com>


Igor Mammedov (7):
  machine: move possible_cpus to MachineState
  pc: move pcms->possible_cpus init out of pc_cpus_init()
  pc: calculate topology only once when possible_cpus is initialised
  pc: pass apic_id to pc_find_cpu_slot() directly so lookup could be
    done without CPU object
  change CPUArchId.cpu type to Object*
  spapr: reuse machine->possible_cpus instead of cores[]
  machine: unify [pc_|spapr_]query_hotpluggable_cpus() callbacks

 include/hw/boards.h    |   8 +++-
 include/hw/i386/pc.h   |   1 -
 include/hw/ppc/spapr.h |   1 -
 hw/acpi/cpu.c          |   2 +-
 hw/core/machine.c      |  31 +++++++++++++
 hw/i386/pc.c           | 124 +++++++++++++++++++++----------------------------
 hw/ppc/spapr.c         | 117 +++++++++++++++++++++++++++-------------------
 7 files changed, 162 insertions(+), 122 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/7] machine: move possible_cpus to MachineState

[Igor Mammedov]

so that it would be possible to reuse it with
spapr/virt-aarch64 targets.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/boards.h  |  1 +
 include/hw/i386/pc.h |  1 -
 hw/i386/pc.c         | 57 ++++++++++++++++++++++++++--------------------------
 3 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index ac891a8..64e8c07 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -178,6 +178,7 @@ struct MachineState {
     char *initrd_filename;
     const char *cpu_model;
     AccelState *accelerator;
+    CPUArchIdList *possible_cpus;
 };
 
 #define DEFINE_MACHINE(namestr, machine_initfn) \
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 079e8d9..d1f4554 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -73,7 +73,6 @@ struct PCMachineState {
     /* CPU and apic information: */
     bool apic_xrupt_override;
     unsigned apic_id_limit;
-    CPUArchIdList *possible_cpus;
     uint16_t boot_cpus;
 
     /* NUMA information: */
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e3fcd51..cf2bec4 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -707,7 +707,8 @@ static void pc_build_smbios(PCMachineState *pcms)
     size_t smbios_tables_len, smbios_anchor_len;
     struct smbios_phys_mem_area *mem_array;
     unsigned i, array_count;
-    X86CPU *cpu = X86_CPU(pcms->possible_cpus->cpus[0].cpu);
+    MachineState *ms = MACHINE(pcms);
+    X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
 
     /* tell smbios about cpuid version and features */
     smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
@@ -1111,7 +1112,7 @@ static void pc_new_cpu(const char *typename, int64_t apic_id, Error **errp)
 void pc_hot_add_cpu(const int64_t id, Error **errp)
 {
     ObjectClass *oc;
-    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+    MachineState *ms = MACHINE(qdev_get_machine());
     int64_t apic_id = x86_cpu_apic_id_from_index(id);
     Error *local_err = NULL;
 
@@ -1127,8 +1128,8 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
         return;
     }
 
-    assert(pcms->possible_cpus->cpus[0].cpu); /* BSP is always present */
-    oc = OBJECT_CLASS(CPU_GET_CLASS(pcms->possible_cpus->cpus[0].cpu));
+    assert(ms->possible_cpus->cpus[0].cpu); /* BSP is always present */
+    oc = OBJECT_CLASS(CPU_GET_CLASS(ms->possible_cpus->cpus[0].cpu));
     pc_new_cpu(object_class_get_name(oc), apic_id, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
@@ -1178,11 +1179,11 @@ void pc_cpus_init(PCMachineState *pcms)
      * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
      */
     pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
-    pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
-                                    sizeof(CPUArchId) * max_cpus);
+    machine->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
+                                       sizeof(CPUArchId) * max_cpus);
     for (i = 0; i < max_cpus; i++) {
-        pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
-        pcms->possible_cpus->len++;
+        machine->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
+        machine->possible_cpus->len++;
         if (i < smp_cpus) {
             pc_new_cpu(typename, x86_cpu_apic_id_from_index(i), &error_fatal);
         }
@@ -1191,7 +1192,8 @@ void pc_cpus_init(PCMachineState *pcms)
 
 static void pc_build_feature_control_file(PCMachineState *pcms)
 {
-    X86CPU *cpu = X86_CPU(pcms->possible_cpus->cpus[0].cpu);
+    MachineState *ms = MACHINE(pcms);
+    X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
     CPUX86State *env = &cpu->env;
     uint32_t unused, ecx, edx;
     uint64_t feature_control_bits = 0;
@@ -1781,21 +1783,20 @@ static int pc_apic_cmp(const void *a, const void *b)
 }
 
 /* returns pointer to CPUArchId descriptor that matches CPU's apic_id
- * in pcms->possible_cpus->cpus, if pcms->possible_cpus->cpus has no
+ * in ms->possible_cpus->cpus, if ms->possible_cpus->cpus has no
  * entry corresponding to CPU's apic_id returns NULL.
  */
-static CPUArchId *pc_find_cpu_slot(PCMachineState *pcms, CPUState *cpu,
-                                   int *idx)
+static CPUArchId *pc_find_cpu_slot(MachineState *ms, CPUState *cpu, int *idx)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
     CPUArchId apic_id, *found_cpu;
 
     apic_id.arch_id = cc->get_arch_id(CPU(cpu));
-    found_cpu = bsearch(&apic_id, pcms->possible_cpus->cpus,
-        pcms->possible_cpus->len, sizeof(*pcms->possible_cpus->cpus),
+    found_cpu = bsearch(&apic_id, ms->possible_cpus->cpus,
+        ms->possible_cpus->len, sizeof(*ms->possible_cpus->cpus),
         pc_apic_cmp);
     if (found_cpu && idx) {
-        *idx = found_cpu - pcms->possible_cpus->cpus;
+        *idx = found_cpu - ms->possible_cpus->cpus;
     }
     return found_cpu;
 }
@@ -1825,7 +1826,7 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
         fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
     }
 
-    found_cpu = pc_find_cpu_slot(pcms, CPU(dev), NULL);
+    found_cpu = pc_find_cpu_slot(MACHINE(pcms), CPU(dev), NULL);
     found_cpu->cpu = CPU(dev);
 out:
     error_propagate(errp, local_err);
@@ -1838,7 +1839,7 @@ static void pc_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
     Error *local_err = NULL;
     PCMachineState *pcms = PC_MACHINE(hotplug_dev);
 
-    pc_find_cpu_slot(pcms, CPU(dev), &idx);
+    pc_find_cpu_slot(MACHINE(pcms), CPU(dev), &idx);
     assert(idx != -1);
     if (idx == 0) {
         error_setg(&local_err, "Boot CPU is unpluggable");
@@ -1872,7 +1873,7 @@ static void pc_cpu_unplug_cb(HotplugHandler *hotplug_dev,
         goto out;
     }
 
-    found_cpu = pc_find_cpu_slot(pcms, CPU(dev), NULL);
+    found_cpu = pc_find_cpu_slot(MACHINE(pcms), CPU(dev), NULL);
     found_cpu->cpu = NULL;
     object_unparent(OBJECT(dev));
 
@@ -1930,13 +1931,15 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         cpu->apic_id = apicid_from_topo_ids(smp_cores, smp_threads, &topo);
     }
 
-    cpu_slot = pc_find_cpu_slot(pcms, CPU(dev), &idx);
+    cpu_slot = pc_find_cpu_slot(MACHINE(pcms), CPU(dev), &idx);
     if (!cpu_slot) {
+        MachineState *ms = MACHINE(pcms);
+
         x86_topo_ids_from_apicid(cpu->apic_id, smp_cores, smp_threads, &topo);
         error_setg(errp, "Invalid CPU [socket: %u, core: %u, thread: %u] with"
                   " APIC ID %" PRIu32 ", valid index range 0:%d",
                    topo.pkg_id, topo.core_id, topo.smt_id, cpu->apic_id,
-                   pcms->possible_cpus->len - 1);
+                   ms->possible_cpus->len - 1);
         return;
     }
 
@@ -2247,9 +2250,8 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
 
 static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine)
 {
-    PCMachineState *pcms = PC_MACHINE(machine);
-    assert(pcms->possible_cpus);
-    return pcms->possible_cpus;
+    assert(machine->possible_cpus);
+    return machine->possible_cpus;
 }
 
 static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
@@ -2257,19 +2259,18 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
     int i;
     CPUState *cpu;
     HotpluggableCPUList *head = NULL;
-    PCMachineState *pcms = PC_MACHINE(machine);
     const char *cpu_type;
 
-    cpu = pcms->possible_cpus->cpus[0].cpu;
+    cpu = machine->possible_cpus->cpus[0].cpu;
     assert(cpu); /* BSP is always present */
     cpu_type = object_class_get_name(OBJECT_CLASS(CPU_GET_CLASS(cpu)));
 
-    for (i = 0; i < pcms->possible_cpus->len; i++) {
+    for (i = 0; i < machine->possible_cpus->len; i++) {
         X86CPUTopoInfo topo;
         HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
         HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
         CpuInstanceProperties *cpu_props = g_new0(typeof(*cpu_props), 1);
-        const uint32_t apic_id = pcms->possible_cpus->cpus[i].arch_id;
+        const uint32_t apic_id = machine->possible_cpus->cpus[i].arch_id;
 
         x86_topo_ids_from_apicid(apic_id, smp_cores, smp_threads, &topo);
 
@@ -2283,7 +2284,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
         cpu_props->thread_id = topo.smt_id;
         cpu_item->props = cpu_props;
 
-        cpu = pcms->possible_cpus->cpus[i].cpu;
+        cpu = machine->possible_cpus->cpus[i].cpu;
         if (cpu) {
             cpu_item->has_qom_path = true;
             cpu_item->qom_path = object_get_canonical_path(OBJECT(cpu));
-- 
2.7.4

[Qemu-devel] [PATCH 2/7] pc: move pcms->possible_cpus init out of pc_cpus_init()

[Igor Mammedov]

possible_cpus could be initialized earlier then cpu objects,
i.e. when -smp is parsed so move init code to possible_cpu_arch_ids()
interface func and do initialization on the first call.

it should help later with making -numa cpu/-smp parsing a machine state
properties.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index cf2bec4..a6cfc97 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1144,7 +1144,9 @@ void pc_cpus_init(PCMachineState *pcms)
     ObjectClass *oc;
     const char *typename;
     gchar **model_pieces;
+    const CPUArchIdList *possible_cpus;
     MachineState *machine = MACHINE(pcms);
+    MachineClass *mc = MACHINE_GET_CLASS(pcms);
 
     /* init CPUs */
     if (machine->cpu_model == NULL) {
@@ -1179,14 +1181,9 @@ void pc_cpus_init(PCMachineState *pcms)
      * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
      */
     pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
-    machine->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
-                                       sizeof(CPUArchId) * max_cpus);
-    for (i = 0; i < max_cpus; i++) {
-        machine->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
-        machine->possible_cpus->len++;
-        if (i < smp_cpus) {
-            pc_new_cpu(typename, x86_cpu_apic_id_from_index(i), &error_fatal);
-        }
+    possible_cpus = mc->possible_cpu_arch_ids(machine);
+    for (i = 0; i < smp_cpus; i++) {
+        pc_new_cpu(typename, possible_cpus->cpus[i].arch_id, &error_fatal);
     }
 }
 
@@ -2248,10 +2245,26 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
     return topo.pkg_id;
 }
 
-static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine)
+static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
 {
-    assert(machine->possible_cpus);
-    return machine->possible_cpus;
+    int i;
+
+    if (ms->possible_cpus) {
+        /*
+         * make sure that max_cpus hasn't changed since the first use, i.e.
+         * -smp hasn't been parsed after it
+        */
+        assert(ms->possible_cpus->len == max_cpus);
+        return ms->possible_cpus;
+    }
+
+    ms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
+                                  sizeof(CPUArchId) * max_cpus);
+    ms->possible_cpus->len = max_cpus;
+    for (i = 0; i < ms->possible_cpus->len; i++) {
+        ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
+    }
+    return ms->possible_cpus;
 }
 
 static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
-- 
2.7.4

[Qemu-devel] [PATCH 3/7] pc: calculate topology only once when possible_cpus is initialised

[Igor Mammedov]

Fill in CpuInstanceProperties once at board init time and
just copy them whenever query_hotpluggable_cpus() is called.
It will keep topology info always available without need
to recalculate it every time it's needed.
Considering it has NUMA node id, it will be used to keep
NUMA node to cpu mapping instead of numa_info[i].node_cpu
bitmasks.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/boards.h |  2 ++
 hw/i386/pc.c        | 24 ++++++++++++------------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 64e8c07..4023b38 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -46,9 +46,11 @@ void machine_register_compat_props(MachineState *machine);
  * CPUArchId:
  * @arch_id - architecture-dependent CPU ID of present or possible CPU
  * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise
+ * @props - CPU object properties, initialized by board
  */
 typedef struct {
     uint64_t arch_id;
+    CpuInstanceProperties props;
     struct CPUState *cpu;
 } CPUArchId;
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a6cfc97..f03a555 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2262,7 +2262,17 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
                                   sizeof(CPUArchId) * max_cpus);
     ms->possible_cpus->len = max_cpus;
     for (i = 0; i < ms->possible_cpus->len; i++) {
+        X86CPUTopoInfo topo;
+
         ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
+        x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
+                                 smp_cores, smp_threads, &topo);
+        ms->possible_cpus->cpus[i].props.has_socket_id = true;
+        ms->possible_cpus->cpus[i].props.socket_id = topo.pkg_id;
+        ms->possible_cpus->cpus[i].props.has_core_id = true;
+        ms->possible_cpus->cpus[i].props.core_id = topo.core_id;
+        ms->possible_cpus->cpus[i].props.has_thread_id = true;
+        ms->possible_cpus->cpus[i].props.thread_id = topo.smt_id;
     }
     return ms->possible_cpus;
 }
@@ -2279,23 +2289,13 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
     cpu_type = object_class_get_name(OBJECT_CLASS(CPU_GET_CLASS(cpu)));
 
     for (i = 0; i < machine->possible_cpus->len; i++) {
-        X86CPUTopoInfo topo;
         HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
         HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
-        CpuInstanceProperties *cpu_props = g_new0(typeof(*cpu_props), 1);
-        const uint32_t apic_id = machine->possible_cpus->cpus[i].arch_id;
-
-        x86_topo_ids_from_apicid(apic_id, smp_cores, smp_threads, &topo);
 
         cpu_item->type = g_strdup(cpu_type);
         cpu_item->vcpus_count = 1;
-        cpu_props->has_socket_id = true;
-        cpu_props->socket_id = topo.pkg_id;
-        cpu_props->has_core_id = true;
-        cpu_props->core_id = topo.core_id;
-        cpu_props->has_thread_id = true;
-        cpu_props->thread_id = topo.smt_id;
-        cpu_item->props = cpu_props;
+        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
+                                   sizeof(*cpu_item->props));
 
         cpu = machine->possible_cpus->cpus[i].cpu;
         if (cpu) {
-- 
2.7.4

[Qemu-devel] [PATCH 4/7] pc: pass apic_id to pc_find_cpu_slot() directly so lookup could be done without CPU object

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f03a555..ec6dded 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1783,12 +1783,11 @@ static int pc_apic_cmp(const void *a, const void *b)
  * in ms->possible_cpus->cpus, if ms->possible_cpus->cpus has no
  * entry corresponding to CPU's apic_id returns NULL.
  */
-static CPUArchId *pc_find_cpu_slot(MachineState *ms, CPUState *cpu, int *idx)
+static CPUArchId *pc_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
 {
-    CPUClass *cc = CPU_GET_CLASS(cpu);
     CPUArchId apic_id, *found_cpu;
 
-    apic_id.arch_id = cc->get_arch_id(CPU(cpu));
+    apic_id.arch_id = id;
     found_cpu = bsearch(&apic_id, ms->possible_cpus->cpus,
         ms->possible_cpus->len, sizeof(*ms->possible_cpus->cpus),
         pc_apic_cmp);
@@ -1804,6 +1803,7 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
     CPUArchId *found_cpu;
     HotplugHandlerClass *hhc;
     Error *local_err = NULL;
+    X86CPU *cpu = X86_CPU(dev);
     PCMachineState *pcms = PC_MACHINE(hotplug_dev);
 
     if (pcms->acpi_dev) {
@@ -1823,7 +1823,7 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
         fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
     }
 
-    found_cpu = pc_find_cpu_slot(MACHINE(pcms), CPU(dev), NULL);
+    found_cpu = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, NULL);
     found_cpu->cpu = CPU(dev);
 out:
     error_propagate(errp, local_err);
@@ -1834,9 +1834,10 @@ static void pc_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
     int idx = -1;
     HotplugHandlerClass *hhc;
     Error *local_err = NULL;
+    X86CPU *cpu = X86_CPU(dev);
     PCMachineState *pcms = PC_MACHINE(hotplug_dev);
 
-    pc_find_cpu_slot(MACHINE(pcms), CPU(dev), &idx);
+    pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx);
     assert(idx != -1);
     if (idx == 0) {
         error_setg(&local_err, "Boot CPU is unpluggable");
@@ -1861,6 +1862,7 @@ static void pc_cpu_unplug_cb(HotplugHandler *hotplug_dev,
     CPUArchId *found_cpu;
     HotplugHandlerClass *hhc;
     Error *local_err = NULL;
+    X86CPU *cpu = X86_CPU(dev);
     PCMachineState *pcms = PC_MACHINE(hotplug_dev);
 
     hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
@@ -1870,7 +1872,7 @@ static void pc_cpu_unplug_cb(HotplugHandler *hotplug_dev,
         goto out;
     }
 
-    found_cpu = pc_find_cpu_slot(MACHINE(pcms), CPU(dev), NULL);
+    found_cpu = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, NULL);
     found_cpu->cpu = NULL;
     object_unparent(OBJECT(dev));
 
@@ -1928,7 +1930,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         cpu->apic_id = apicid_from_topo_ids(smp_cores, smp_threads, &topo);
     }
 
-    cpu_slot = pc_find_cpu_slot(MACHINE(pcms), CPU(dev), &idx);
+    cpu_slot = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx);
     if (!cpu_slot) {
         MachineState *ms = MACHINE(pcms);
 
-- 
2.7.4

[Qemu-devel] [PATCH 5/7] change CPUArchId.cpu type to Object*

[Igor Mammedov]

so it could be reused for SPAPR cores as well

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/boards.h | 2 +-
 hw/acpi/cpu.c       | 2 +-
 hw/i386/pc.c        | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 4023b38..60209df 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -51,7 +51,7 @@ void machine_register_compat_props(MachineState *machine);
 typedef struct {
     uint64_t arch_id;
     CpuInstanceProperties props;
-    struct CPUState *cpu;
+    Object *cpu;
 } CPUArchId;
 
 /**
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 6017ca0..8c719d3 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -198,7 +198,7 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
     state->dev_count = id_list->len;
     state->devs = g_new0(typeof(*state->devs), state->dev_count);
     for (i = 0; i < id_list->len; i++) {
-        state->devs[i].cpu =  id_list->cpus[i].cpu;
+        state->devs[i].cpu =  CPU(id_list->cpus[i].cpu);
         state->devs[i].arch_id = id_list->cpus[i].arch_id;
     }
     memory_region_init_io(&state->ctrl_reg, owner, &cpu_hotplug_ops, state,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index ec6dded..afaae15 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1824,7 +1824,7 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
     }
 
     found_cpu = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, NULL);
-    found_cpu->cpu = CPU(dev);
+    found_cpu->cpu = OBJECT(dev);
 out:
     error_propagate(errp, local_err);
 }
@@ -2282,13 +2282,13 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
 static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
 {
     int i;
-    CPUState *cpu;
+    Object *cpu;
     HotpluggableCPUList *head = NULL;
     const char *cpu_type;
 
     cpu = machine->possible_cpus->cpus[0].cpu;
     assert(cpu); /* BSP is always present */
-    cpu_type = object_class_get_name(OBJECT_CLASS(CPU_GET_CLASS(cpu)));
+    cpu_type = object_get_typename(cpu);
 
     for (i = 0; i < machine->possible_cpus->len; i++) {
         HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
@@ -2302,7 +2302,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
         cpu = machine->possible_cpus->cpus[i].cpu;
         if (cpu) {
             cpu_item->has_qom_path = true;
-            cpu_item->qom_path = object_get_canonical_path(OBJECT(cpu));
+            cpu_item->qom_path = object_get_canonical_path(cpu);
         }
 
         list_item->value = cpu_item;
-- 
2.7.4

[Qemu-devel] [PATCH 6/7] spapr: reuse machine->possible_cpus instead of cores[]

[Igor Mammedov]

Replace SPAPR specific cores[] array with generic
machine->possible_cpus and store core objects there.
It makes cores bookkeeping similar to x86 cpus and
will allow to unify similar code.
It would allow to replace cpu_index based NUMA node
mapping with iproperty based one (for -device created
cores) since possible_cpus carries board defined
topology/layout.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/ppc/spapr.h |   1 -
 hw/ppc/spapr.c         | 129 ++++++++++++++++++++++++++++++++++---------------
 2 files changed, 90 insertions(+), 40 deletions(-)

diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index a2d8964..f9b17d8 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -94,7 +94,6 @@ struct sPAPRMachineState {
     /*< public >*/
     char *kvm_type;
     MemoryHotplugState hotplug_memory;
-    Object **cores;
 };
 
 #define H_SUCCESS         0
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 37cb338..8d039ae 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1751,13 +1751,28 @@ static void spapr_validate_node_memory(MachineState *machine, Error **errp)
     }
 }
 
+/* find cpu slot in machine->possible_cpus by core_id */
+static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
+{
+    int index = id / smp_threads;
+
+    if (index >= ms->possible_cpus->len) {
+        return NULL;
+    }
+    if (idx) {
+        *idx = index;
+    }
+    return &ms->possible_cpus->cpus[index];
+}
+
 static void spapr_init_cpus(sPAPRMachineState *spapr)
 {
     MachineState *machine = MACHINE(spapr);
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     char *type = spapr_get_cpu_core_type(machine->cpu_model);
     int smt = kvmppc_smt_threads();
-    int spapr_max_cores, spapr_cores;
+    const CPUArchIdList *possible_cpus;
+    int boot_cores_nr = smp_cpus / smp_threads;;
     int i;
 
     if (!type) {
@@ -1765,6 +1780,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
         exit(1);
     }
 
+    possible_cpus = mc->possible_cpu_arch_ids(machine);
     if (mc->query_hotpluggable_cpus) {
         if (smp_cpus % smp_threads) {
             error_report("smp_cpus (%u) must be multiple of threads (%u)",
@@ -1776,21 +1792,15 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
                          max_cpus, smp_threads);
             exit(1);
         }
-
-        spapr_max_cores = max_cpus / smp_threads;
-        spapr_cores = smp_cpus / smp_threads;
     } else {
         if (max_cpus != smp_cpus) {
             error_report("This machine version does not support CPU hotplug");
             exit(1);
         }
-
-        spapr_max_cores = QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_threads;
-        spapr_cores = spapr_max_cores;
+        boot_cores_nr = possible_cpus->len;
     }
 
-    spapr->cores = g_new0(Object *, spapr_max_cores);
-    for (i = 0; i < spapr_max_cores; i++) {
+    for (i = 0; i < possible_cpus->len; i++) {
         int core_id = i * smp_threads;
 
         if (mc->query_hotpluggable_cpus) {
@@ -1802,7 +1812,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
             qemu_register_reset(spapr_drc_reset, drc);
         }
 
-        if (i < spapr_cores) {
+        if (i < boot_cores_nr) {
             Object *core  = object_new(type);
             int nr_threads = smp_threads;
 
@@ -2491,10 +2501,11 @@ void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
 static void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
                               Error **errp)
 {
-    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+    MachineState *ms = MACHINE(qdev_get_machine());
     CPUCore *cc = CPU_CORE(dev);
+    CPUArchId *core_slot = spapr_find_cpu_slot(ms, cc->core_id, NULL);
 
-    spapr->cores[cc->core_id / smp_threads] = NULL;
+    core_slot->cpu = NULL;
     object_unparent(OBJECT(dev));
 }
 
@@ -2510,19 +2521,24 @@ static
 void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev,
                                Error **errp)
 {
-    CPUCore *cc = CPU_CORE(dev);
-    int smt = kvmppc_smt_threads();
-    int index = cc->core_id / smp_threads;
-    sPAPRDRConnector *drc =
-        spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
+    int index;
+    sPAPRDRConnector *drc;
     sPAPRDRConnectorClass *drck;
     Error *local_err = NULL;
+    CPUCore *cc = CPU_CORE(dev);
+    int smt = kvmppc_smt_threads();
 
+    if (!spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index)) {
+        error_setg(errp, "Unable to find CPU core with core-id: %d",
+                   cc->core_id);
+        return;
+    }
     if (index == 0) {
         error_setg(errp, "Boot CPU core may not be unplugged");
         return;
     }
 
+    drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
     g_assert(drc);
 
     drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
@@ -2547,11 +2563,17 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
     Error *local_err = NULL;
     void *fdt = NULL;
     int fdt_offset = 0;
-    int index = cc->core_id / smp_threads;
     int smt = kvmppc_smt_threads();
+    CPUArchId *core_slot;
+    int index;
 
+    core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index);
+    if (!core_slot) {
+        error_setg(errp, "Unable to find CPU core with core-id: %d",
+                   cc->core_id);
+        return;
+    }
     drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
-    spapr->cores[index] = OBJECT(dev);
 
     g_assert(drc || !mc->query_hotpluggable_cpus);
 
@@ -2568,7 +2590,6 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
         drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
         if (local_err) {
             g_free(fdt);
-            spapr->cores[index] = NULL;
             error_propagate(errp, local_err);
             return;
         }
@@ -2590,6 +2611,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
             drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED);
         }
     }
+    core_slot->cpu = OBJECT(dev);
 }
 
 static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
@@ -2597,13 +2619,12 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
 {
     MachineState *machine = MACHINE(OBJECT(hotplug_dev));
     MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev);
-    sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
-    int spapr_max_cores = max_cpus / smp_threads;
-    int index;
     Error *local_err = NULL;
     CPUCore *cc = CPU_CORE(dev);
     char *base_core_type = spapr_get_cpu_core_type(machine->cpu_model);
     const char *type = object_get_typename(OBJECT(dev));
+    CPUArchId *core_slot;
+    int index;
 
     if (dev->hotplugged && !mc->query_hotpluggable_cpus) {
         error_setg(&local_err, "CPU hotplug not supported for this machine");
@@ -2620,13 +2641,13 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
         goto out;
     }
 
-    index = cc->core_id / smp_threads;
-    if (index < 0 || index >= spapr_max_cores) {
+    core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index);
+    if (!core_slot) {
         error_setg(&local_err, "core id %d out of range", cc->core_id);
         goto out;
     }
 
-    if (spapr->cores[index]) {
+    if (core_slot->cpu) {
         error_setg(&local_err, "core %d already populated", cc->core_id);
         goto out;
     }
@@ -2758,29 +2779,58 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
     return cpu_index / smp_threads / smp_cores;
 }
 
+static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine)
+{
+    int i;
+    int spapr_max_cores = max_cpus / smp_threads;
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
+
+    if (!mc->query_hotpluggable_cpus) {
+        spapr_max_cores = QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_threads;
+    }
+    if (machine->possible_cpus) {
+        assert(machine->possible_cpus->len == spapr_max_cores);
+        return machine->possible_cpus;
+    }
+
+    machine->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
+                             sizeof(CPUArchId) * spapr_max_cores);
+    machine->possible_cpus->len = spapr_max_cores;
+    for (i = 0; i < machine->possible_cpus->len; i++) {
+        int core_id = i * smp_threads;
+
+        machine->possible_cpus->cpus[i].arch_id = core_id;
+        machine->possible_cpus->cpus[i].props.has_core_id = true;
+        machine->possible_cpus->cpus[i].props.core_id = core_id;
+        /* TODO: add 'has_node/node' here to describe
+           to which node core belongs */
+    }
+    return machine->possible_cpus;
+}
+
 static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
 {
     int i;
+    Object *cpu;
     HotpluggableCPUList *head = NULL;
-    sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
-    int spapr_max_cores = max_cpus / smp_threads;
+    const char *cpu_type;
 
-    for (i = 0; i < spapr_max_cores; i++) {
+    cpu = machine->possible_cpus->cpus[0].cpu;
+    assert(cpu); /* Boot cpu is always present */
+    cpu_type = object_get_typename(cpu);
+    for (i = 0; i < machine->possible_cpus->len; i++) {
         HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
         HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
-        CpuInstanceProperties *cpu_props = g_new0(typeof(*cpu_props), 1);
 
-        cpu_item->type = spapr_get_cpu_core_type(machine->cpu_model);
-        cpu_item->vcpus_count = smp_threads;
-        cpu_props->has_core_id = true;
-        cpu_props->core_id = i * smp_threads;
-        /* TODO: add 'has_node/node' here to describe
-           to which node core belongs */
+        cpu_item->type = g_strdup(cpu_type);
+        cpu_item->vcpus_count = smp_threads; // TODO: ??? generalize
+        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
+                                   sizeof(*cpu_item->props));
 
-        cpu_item->props = cpu_props;
-        if (spapr->cores[i]) {
+        cpu = machine->possible_cpus->cpus[i].cpu;
+        if (cpu) {
             cpu_item->has_qom_path = true;
-            cpu_item->qom_path = object_get_canonical_path(spapr->cores[i]);
+            cpu_item->qom_path = object_get_canonical_path(cpu);
         }
         list_item->value = cpu_item;
         list_item->next = head;
@@ -2872,6 +2922,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     hc->plug = spapr_machine_device_plug;
     hc->unplug = spapr_machine_device_unplug;
     mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
+    mc->possible_cpu_arch_ids = spapr_possible_cpu_arch_ids;
     hc->unplug_request = spapr_machine_device_unplug_request;
 
     smc->dr_lmb_enabled = true;
-- 
2.7.4

[Qemu-devel] [PATCH 7/7] machine: unify [pc_|spapr_]query_hotpluggable_cpus() callbacks

[Igor Mammedov]

All callbacks FOO_query_hotpluggable_cpus() are practically
the same except of setting vcpus_count to different values.
Convert them to a generic machine_query_hotpluggable_cpus()
callback by moving vcpus_count initialization to per machine
specific callback possible_cpu_arch_ids().

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/boards.h |  3 +++
 hw/core/machine.c   | 31 +++++++++++++++++++++++++++++++
 hw/i386/pc.c        | 36 ++----------------------------------
 hw/ppc/spapr.c      | 34 ++--------------------------------
 4 files changed, 38 insertions(+), 66 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 60209df..9040dbb 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -41,15 +41,18 @@ int machine_phandle_start(MachineState *machine);
 bool machine_dump_guest_core(MachineState *machine);
 bool machine_mem_merge(MachineState *machine);
 void machine_register_compat_props(MachineState *machine);
+HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine);
 
 /**
  * CPUArchId:
  * @arch_id - architecture-dependent CPU ID of present or possible CPU
  * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise
  * @props - CPU object properties, initialized by board
+ * #vcpus_count - number of threads provided by @cpu object
  */
 typedef struct {
     uint64_t arch_id;
+    int64_t vcpus_count;
     CpuInstanceProperties props;
     Object *cpu;
 } CPUArchId;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index b0fd91f..0699750 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -357,6 +357,37 @@ static void machine_init_notify(Notifier *notifier, void *data)
     foreach_dynamic_sysbus_device(error_on_sysbus_device, NULL);
 }
 
+HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine)
+{
+    int i;
+    Object *cpu;
+    HotpluggableCPUList *head = NULL;
+    const char *cpu_type;
+
+    cpu = machine->possible_cpus->cpus[0].cpu;
+    assert(cpu); /* Boot cpu is always present */
+    cpu_type = object_get_typename(cpu);
+    for (i = 0; i < machine->possible_cpus->len; i++) {
+        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
+        HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
+
+        cpu_item->type = g_strdup(cpu_type);
+        cpu_item->vcpus_count = machine->possible_cpus->cpus[i].vcpus_count;
+        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
+                                   sizeof(*cpu_item->props));
+
+        cpu = machine->possible_cpus->cpus[i].cpu;
+        if (cpu) {
+            cpu_item->has_qom_path = true;
+            cpu_item->qom_path = object_get_canonical_path(cpu);
+        }
+        list_item->value = cpu_item;
+        list_item->next = head;
+        head = list_item;
+    }
+    return head;
+}
+
 static void machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index afaae15..548628f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2266,6 +2266,7 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
     for (i = 0; i < ms->possible_cpus->len; i++) {
         X86CPUTopoInfo topo;
 
+        ms->possible_cpus->cpus[i].vcpus_count = 1;
         ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
         x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
                                  smp_cores, smp_threads, &topo);
@@ -2279,39 +2280,6 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
     return ms->possible_cpus;
 }
 
-static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
-{
-    int i;
-    Object *cpu;
-    HotpluggableCPUList *head = NULL;
-    const char *cpu_type;
-
-    cpu = machine->possible_cpus->cpus[0].cpu;
-    assert(cpu); /* BSP is always present */
-    cpu_type = object_get_typename(cpu);
-
-    for (i = 0; i < machine->possible_cpus->len; i++) {
-        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
-        HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
-
-        cpu_item->type = g_strdup(cpu_type);
-        cpu_item->vcpus_count = 1;
-        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
-                                   sizeof(*cpu_item->props));
-
-        cpu = machine->possible_cpus->cpus[i].cpu;
-        if (cpu) {
-            cpu_item->has_qom_path = true;
-            cpu_item->qom_path = object_get_canonical_path(cpu);
-        }
-
-        list_item->value = cpu_item;
-        list_item->next = head;
-        head = list_item;
-    }
-    return head;
-}
-
 static void x86_nmi(NMIState *n, int cpu_index, Error **errp)
 {
     /* cpu index isn't used */
@@ -2352,7 +2320,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     mc->get_hotplug_handler = pc_get_hotpug_handler;
     mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
     mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids;
-    mc->query_hotpluggable_cpus = pc_query_hotpluggable_cpus;
+    mc->query_hotpluggable_cpus = machine_query_hotpluggable_cpus;
     mc->default_boot_order = "cad";
     mc->hot_add_cpu = pc_hot_add_cpu;
     mc->max_cpus = 255;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 8d039ae..3c4d632 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2799,6 +2799,7 @@ static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine)
     for (i = 0; i < machine->possible_cpus->len; i++) {
         int core_id = i * smp_threads;
 
+        machine->possible_cpus->cpus[i].vcpus_count = smp_threads;
         machine->possible_cpus->cpus[i].arch_id = core_id;
         machine->possible_cpus->cpus[i].props.has_core_id = true;
         machine->possible_cpus->cpus[i].props.core_id = core_id;
@@ -2808,37 +2809,6 @@ static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine)
     return machine->possible_cpus;
 }
 
-static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
-{
-    int i;
-    Object *cpu;
-    HotpluggableCPUList *head = NULL;
-    const char *cpu_type;
-
-    cpu = machine->possible_cpus->cpus[0].cpu;
-    assert(cpu); /* Boot cpu is always present */
-    cpu_type = object_get_typename(cpu);
-    for (i = 0; i < machine->possible_cpus->len; i++) {
-        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
-        HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
-
-        cpu_item->type = g_strdup(cpu_type);
-        cpu_item->vcpus_count = smp_threads; // TODO: ??? generalize
-        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
-                                   sizeof(*cpu_item->props));
-
-        cpu = machine->possible_cpus->cpus[i].cpu;
-        if (cpu) {
-            cpu_item->has_qom_path = true;
-            cpu_item->qom_path = object_get_canonical_path(cpu);
-        }
-        list_item->value = cpu_item;
-        list_item->next = head;
-        head = list_item;
-    }
-    return head;
-}
-
 static void spapr_phb_placement(sPAPRMachineState *spapr, uint32_t index,
                                 uint64_t *buid, hwaddr *pio,
                                 hwaddr *mmio32, hwaddr *mmio64,
@@ -2927,7 +2897,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
 
     smc->dr_lmb_enabled = true;
     smc->tcg_default_cpu = "POWER8";
-    mc->query_hotpluggable_cpus = spapr_query_hotpluggable_cpus;
+    mc->query_hotpluggable_cpus = machine_query_hotpluggable_cpus;
     fwc->get_dev_path = spapr_get_fw_dev_path;
     nc->nmi_monitor_handler = spapr_nmi;
     smc->phb_placement = spapr_phb_placement;
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/7] machine: move possible_cpus to MachineState

[David Gibson]

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

On Thu, Feb 09, 2017 at 12:08:32PM +0100, Igor Mammedov wrote:
> so that it would be possible to reuse it with
> spapr/virt-aarch64 targets.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/hw/boards.h  |  1 +
>  include/hw/i386/pc.h |  1 -
>  hw/i386/pc.c         | 57 ++++++++++++++++++++++++++--------------------------
>  3 files changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index ac891a8..64e8c07 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -178,6 +178,7 @@ struct MachineState {
>      char *initrd_filename;
>      const char *cpu_model;
>      AccelState *accelerator;
> +    CPUArchIdList *possible_cpus;
>  };
>  
>  #define DEFINE_MACHINE(namestr, machine_initfn) \
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 079e8d9..d1f4554 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -73,7 +73,6 @@ struct PCMachineState {
>      /* CPU and apic information: */
>      bool apic_xrupt_override;
>      unsigned apic_id_limit;
> -    CPUArchIdList *possible_cpus;
>      uint16_t boot_cpus;
>  
>      /* NUMA information: */
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index e3fcd51..cf2bec4 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -707,7 +707,8 @@ static void pc_build_smbios(PCMachineState *pcms)
>      size_t smbios_tables_len, smbios_anchor_len;
>      struct smbios_phys_mem_area *mem_array;
>      unsigned i, array_count;
> -    X86CPU *cpu = X86_CPU(pcms->possible_cpus->cpus[0].cpu);
> +    MachineState *ms = MACHINE(pcms);
> +    X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
>  
>      /* tell smbios about cpuid version and features */
>      smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
> @@ -1111,7 +1112,7 @@ static void pc_new_cpu(const char *typename, int64_t apic_id, Error **errp)
>  void pc_hot_add_cpu(const int64_t id, Error **errp)
>  {
>      ObjectClass *oc;
> -    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      int64_t apic_id = x86_cpu_apic_id_from_index(id);
>      Error *local_err = NULL;
>  
> @@ -1127,8 +1128,8 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
>          return;
>      }
>  
> -    assert(pcms->possible_cpus->cpus[0].cpu); /* BSP is always present */
> -    oc = OBJECT_CLASS(CPU_GET_CLASS(pcms->possible_cpus->cpus[0].cpu));
> +    assert(ms->possible_cpus->cpus[0].cpu); /* BSP is always present */
> +    oc = OBJECT_CLASS(CPU_GET_CLASS(ms->possible_cpus->cpus[0].cpu));
>      pc_new_cpu(object_class_get_name(oc), apic_id, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> @@ -1178,11 +1179,11 @@ void pc_cpus_init(PCMachineState *pcms)
>       * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
>       */
>      pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
> -    pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
> -                                    sizeof(CPUArchId) * max_cpus);
> +    machine->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
> +                                       sizeof(CPUArchId) * max_cpus);
>      for (i = 0; i < max_cpus; i++) {
> -        pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
> -        pcms->possible_cpus->len++;
> +        machine->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
> +        machine->possible_cpus->len++;
>          if (i < smp_cpus) {
>              pc_new_cpu(typename, x86_cpu_apic_id_from_index(i), &error_fatal);
>          }
> @@ -1191,7 +1192,8 @@ void pc_cpus_init(PCMachineState *pcms)
>  
>  static void pc_build_feature_control_file(PCMachineState *pcms)
>  {
> -    X86CPU *cpu = X86_CPU(pcms->possible_cpus->cpus[0].cpu);
> +    MachineState *ms = MACHINE(pcms);
> +    X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
>      CPUX86State *env = &cpu->env;
>      uint32_t unused, ecx, edx;
>      uint64_t feature_control_bits = 0;
> @@ -1781,21 +1783,20 @@ static int pc_apic_cmp(const void *a, const void *b)
>  }
>  
>  /* returns pointer to CPUArchId descriptor that matches CPU's apic_id
> - * in pcms->possible_cpus->cpus, if pcms->possible_cpus->cpus has no
> + * in ms->possible_cpus->cpus, if ms->possible_cpus->cpus has no
>   * entry corresponding to CPU's apic_id returns NULL.
>   */
> -static CPUArchId *pc_find_cpu_slot(PCMachineState *pcms, CPUState *cpu,
> -                                   int *idx)
> +static CPUArchId *pc_find_cpu_slot(MachineState *ms, CPUState *cpu, int *idx)
>  {
>      CPUClass *cc = CPU_GET_CLASS(cpu);
>      CPUArchId apic_id, *found_cpu;
>  
>      apic_id.arch_id = cc->get_arch_id(CPU(cpu));
> -    found_cpu = bsearch(&apic_id, pcms->possible_cpus->cpus,
> -        pcms->possible_cpus->len, sizeof(*pcms->possible_cpus->cpus),
> +    found_cpu = bsearch(&apic_id, ms->possible_cpus->cpus,
> +        ms->possible_cpus->len, sizeof(*ms->possible_cpus->cpus),
>          pc_apic_cmp);
>      if (found_cpu && idx) {
> -        *idx = found_cpu - pcms->possible_cpus->cpus;
> +        *idx = found_cpu - ms->possible_cpus->cpus;
>      }
>      return found_cpu;
>  }
> @@ -1825,7 +1826,7 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
>          fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
>      }
>  
> -    found_cpu = pc_find_cpu_slot(pcms, CPU(dev), NULL);
> +    found_cpu = pc_find_cpu_slot(MACHINE(pcms), CPU(dev), NULL);
>      found_cpu->cpu = CPU(dev);
>  out:
>      error_propagate(errp, local_err);
> @@ -1838,7 +1839,7 @@ static void pc_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
>      Error *local_err = NULL;
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>  
> -    pc_find_cpu_slot(pcms, CPU(dev), &idx);
> +    pc_find_cpu_slot(MACHINE(pcms), CPU(dev), &idx);
>      assert(idx != -1);
>      if (idx == 0) {
>          error_setg(&local_err, "Boot CPU is unpluggable");
> @@ -1872,7 +1873,7 @@ static void pc_cpu_unplug_cb(HotplugHandler *hotplug_dev,
>          goto out;
>      }
>  
> -    found_cpu = pc_find_cpu_slot(pcms, CPU(dev), NULL);
> +    found_cpu = pc_find_cpu_slot(MACHINE(pcms), CPU(dev), NULL);
>      found_cpu->cpu = NULL;
>      object_unparent(OBJECT(dev));
>  
> @@ -1930,13 +1931,15 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>          cpu->apic_id = apicid_from_topo_ids(smp_cores, smp_threads, &topo);
>      }
>  
> -    cpu_slot = pc_find_cpu_slot(pcms, CPU(dev), &idx);
> +    cpu_slot = pc_find_cpu_slot(MACHINE(pcms), CPU(dev), &idx);
>      if (!cpu_slot) {
> +        MachineState *ms = MACHINE(pcms);
> +
>          x86_topo_ids_from_apicid(cpu->apic_id, smp_cores, smp_threads, &topo);
>          error_setg(errp, "Invalid CPU [socket: %u, core: %u, thread: %u] with"
>                    " APIC ID %" PRIu32 ", valid index range 0:%d",
>                     topo.pkg_id, topo.core_id, topo.smt_id, cpu->apic_id,
> -                   pcms->possible_cpus->len - 1);
> +                   ms->possible_cpus->len - 1);
>          return;
>      }
>  
> @@ -2247,9 +2250,8 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
>  
>  static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine)
>  {
> -    PCMachineState *pcms = PC_MACHINE(machine);
> -    assert(pcms->possible_cpus);
> -    return pcms->possible_cpus;
> +    assert(machine->possible_cpus);
> +    return machine->possible_cpus;
>  }
>  
>  static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
> @@ -2257,19 +2259,18 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
>      int i;
>      CPUState *cpu;
>      HotpluggableCPUList *head = NULL;
> -    PCMachineState *pcms = PC_MACHINE(machine);
>      const char *cpu_type;
>  
> -    cpu = pcms->possible_cpus->cpus[0].cpu;
> +    cpu = machine->possible_cpus->cpus[0].cpu;
>      assert(cpu); /* BSP is always present */
>      cpu_type = object_class_get_name(OBJECT_CLASS(CPU_GET_CLASS(cpu)));
>  
> -    for (i = 0; i < pcms->possible_cpus->len; i++) {
> +    for (i = 0; i < machine->possible_cpus->len; i++) {
>          X86CPUTopoInfo topo;
>          HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
>          HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
>          CpuInstanceProperties *cpu_props = g_new0(typeof(*cpu_props), 1);
> -        const uint32_t apic_id = pcms->possible_cpus->cpus[i].arch_id;
> +        const uint32_t apic_id = machine->possible_cpus->cpus[i].arch_id;
>  
>          x86_topo_ids_from_apicid(apic_id, smp_cores, smp_threads, &topo);
>  
> @@ -2283,7 +2284,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
>          cpu_props->thread_id = topo.smt_id;
>          cpu_item->props = cpu_props;
>  
> -        cpu = pcms->possible_cpus->cpus[i].cpu;
> +        cpu = machine->possible_cpus->cpus[i].cpu;
>          if (cpu) {
>              cpu_item->has_qom_path = true;
>              cpu_item->qom_path = object_get_canonical_path(OBJECT(cpu));

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 2/7] pc: move pcms->possible_cpus init out of pc_cpus_init()

[David Gibson]

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

On Thu, Feb 09, 2017 at 12:08:33PM +0100, Igor Mammedov wrote:
> possible_cpus could be initialized earlier then cpu objects,
> i.e. when -smp is parsed so move init code to possible_cpu_arch_ids()
> interface func and do initialization on the first call.
> 
> it should help later with making -numa cpu/-smp parsing a machine state
> properties.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/i386/pc.c | 35 ++++++++++++++++++++++++-----------
>  1 file changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index cf2bec4..a6cfc97 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1144,7 +1144,9 @@ void pc_cpus_init(PCMachineState *pcms)
>      ObjectClass *oc;
>      const char *typename;
>      gchar **model_pieces;
> +    const CPUArchIdList *possible_cpus;
>      MachineState *machine = MACHINE(pcms);
> +    MachineClass *mc = MACHINE_GET_CLASS(pcms);
>  
>      /* init CPUs */
>      if (machine->cpu_model == NULL) {
> @@ -1179,14 +1181,9 @@ void pc_cpus_init(PCMachineState *pcms)
>       * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
>       */
>      pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
> -    machine->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
> -                                       sizeof(CPUArchId) * max_cpus);
> -    for (i = 0; i < max_cpus; i++) {
> -        machine->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
> -        machine->possible_cpus->len++;
> -        if (i < smp_cpus) {
> -            pc_new_cpu(typename, x86_cpu_apic_id_from_index(i), &error_fatal);
> -        }
> +    possible_cpus = mc->possible_cpu_arch_ids(machine);
> +    for (i = 0; i < smp_cpus; i++) {
> +        pc_new_cpu(typename, possible_cpus->cpus[i].arch_id, &error_fatal);
>      }
>  }
>  
> @@ -2248,10 +2245,26 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
>      return topo.pkg_id;
>  }
>  
> -static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine)
> +static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
>  {
> -    assert(machine->possible_cpus);
> -    return machine->possible_cpus;
> +    int i;
> +
> +    if (ms->possible_cpus) {
> +        /*
> +         * make sure that max_cpus hasn't changed since the first use, i.e.
> +         * -smp hasn't been parsed after it
> +        */
> +        assert(ms->possible_cpus->len == max_cpus);
> +        return ms->possible_cpus;
> +    }
> +
> +    ms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
> +                                  sizeof(CPUArchId) * max_cpus);
> +    ms->possible_cpus->len = max_cpus;
> +    for (i = 0; i < ms->possible_cpus->len; i++) {
> +        ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
> +    }
> +    return ms->possible_cpus;
>  }
>  
>  static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 3/7] pc: calculate topology only once when possible_cpus is initialised

[David Gibson]

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

On Thu, Feb 09, 2017 at 12:08:34PM +0100, Igor Mammedov wrote:
> Fill in CpuInstanceProperties once at board init time and
> just copy them whenever query_hotpluggable_cpus() is called.
> It will keep topology info always available without need
> to recalculate it every time it's needed.
> Considering it has NUMA node id, it will be used to keep
> NUMA node to cpu mapping instead of numa_info[i].node_cpu
> bitmasks.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/hw/boards.h |  2 ++
>  hw/i386/pc.c        | 24 ++++++++++++------------
>  2 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 64e8c07..4023b38 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -46,9 +46,11 @@ void machine_register_compat_props(MachineState *machine);
>   * CPUArchId:
>   * @arch_id - architecture-dependent CPU ID of present or possible CPU
>   * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise
> + * @props - CPU object properties, initialized by board
>   */
>  typedef struct {
>      uint64_t arch_id;
> +    CpuInstanceProperties props;
>      struct CPUState *cpu;
>  } CPUArchId;
>  
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index a6cfc97..f03a555 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2262,7 +2262,17 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
>                                    sizeof(CPUArchId) * max_cpus);
>      ms->possible_cpus->len = max_cpus;
>      for (i = 0; i < ms->possible_cpus->len; i++) {
> +        X86CPUTopoInfo topo;
> +
>          ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
> +        x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
> +                                 smp_cores, smp_threads, &topo);
> +        ms->possible_cpus->cpus[i].props.has_socket_id = true;
> +        ms->possible_cpus->cpus[i].props.socket_id = topo.pkg_id;
> +        ms->possible_cpus->cpus[i].props.has_core_id = true;
> +        ms->possible_cpus->cpus[i].props.core_id = topo.core_id;
> +        ms->possible_cpus->cpus[i].props.has_thread_id = true;
> +        ms->possible_cpus->cpus[i].props.thread_id = topo.smt_id;
>      }
>      return ms->possible_cpus;
>  }
> @@ -2279,23 +2289,13 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
>      cpu_type = object_class_get_name(OBJECT_CLASS(CPU_GET_CLASS(cpu)));
>  
>      for (i = 0; i < machine->possible_cpus->len; i++) {
> -        X86CPUTopoInfo topo;
>          HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
>          HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> -        CpuInstanceProperties *cpu_props = g_new0(typeof(*cpu_props), 1);
> -        const uint32_t apic_id = machine->possible_cpus->cpus[i].arch_id;
> -
> -        x86_topo_ids_from_apicid(apic_id, smp_cores, smp_threads, &topo);
>  
>          cpu_item->type = g_strdup(cpu_type);
>          cpu_item->vcpus_count = 1;
> -        cpu_props->has_socket_id = true;
> -        cpu_props->socket_id = topo.pkg_id;
> -        cpu_props->has_core_id = true;
> -        cpu_props->core_id = topo.core_id;
> -        cpu_props->has_thread_id = true;
> -        cpu_props->thread_id = topo.smt_id;
> -        cpu_item->props = cpu_props;
> +        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
> +                                   sizeof(*cpu_item->props));
>  
>          cpu = machine->possible_cpus->cpus[i].cpu;
>          if (cpu) {

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 4/7] pc: pass apic_id to pc_find_cpu_slot() directly so lookup could be done without CPU object

[David Gibson]

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

On Thu, Feb 09, 2017 at 12:08:35PM +0100, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/i386/pc.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index f03a555..ec6dded 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1783,12 +1783,11 @@ static int pc_apic_cmp(const void *a, const void *b)
>   * in ms->possible_cpus->cpus, if ms->possible_cpus->cpus has no
>   * entry corresponding to CPU's apic_id returns NULL.
>   */
> -static CPUArchId *pc_find_cpu_slot(MachineState *ms, CPUState *cpu, int *idx)
> +static CPUArchId *pc_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
>  {
> -    CPUClass *cc = CPU_GET_CLASS(cpu);
>      CPUArchId apic_id, *found_cpu;
>  
> -    apic_id.arch_id = cc->get_arch_id(CPU(cpu));
> +    apic_id.arch_id = id;
>      found_cpu = bsearch(&apic_id, ms->possible_cpus->cpus,
>          ms->possible_cpus->len, sizeof(*ms->possible_cpus->cpus),
>          pc_apic_cmp);
> @@ -1804,6 +1803,7 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
>      CPUArchId *found_cpu;
>      HotplugHandlerClass *hhc;
>      Error *local_err = NULL;
> +    X86CPU *cpu = X86_CPU(dev);
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>  
>      if (pcms->acpi_dev) {
> @@ -1823,7 +1823,7 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
>          fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
>      }
>  
> -    found_cpu = pc_find_cpu_slot(MACHINE(pcms), CPU(dev), NULL);
> +    found_cpu = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, NULL);
>      found_cpu->cpu = CPU(dev);
>  out:
>      error_propagate(errp, local_err);
> @@ -1834,9 +1834,10 @@ static void pc_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
>      int idx = -1;
>      HotplugHandlerClass *hhc;
>      Error *local_err = NULL;
> +    X86CPU *cpu = X86_CPU(dev);
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>  
> -    pc_find_cpu_slot(MACHINE(pcms), CPU(dev), &idx);
> +    pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx);
>      assert(idx != -1);
>      if (idx == 0) {
>          error_setg(&local_err, "Boot CPU is unpluggable");
> @@ -1861,6 +1862,7 @@ static void pc_cpu_unplug_cb(HotplugHandler *hotplug_dev,
>      CPUArchId *found_cpu;
>      HotplugHandlerClass *hhc;
>      Error *local_err = NULL;
> +    X86CPU *cpu = X86_CPU(dev);
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>  
>      hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
> @@ -1870,7 +1872,7 @@ static void pc_cpu_unplug_cb(HotplugHandler *hotplug_dev,
>          goto out;
>      }
>  
> -    found_cpu = pc_find_cpu_slot(MACHINE(pcms), CPU(dev), NULL);
> +    found_cpu = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, NULL);
>      found_cpu->cpu = NULL;
>      object_unparent(OBJECT(dev));
>  
> @@ -1928,7 +1930,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>          cpu->apic_id = apicid_from_topo_ids(smp_cores, smp_threads, &topo);
>      }
>  
> -    cpu_slot = pc_find_cpu_slot(MACHINE(pcms), CPU(dev), &idx);
> +    cpu_slot = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx);
>      if (!cpu_slot) {
>          MachineState *ms = MACHINE(pcms);
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 5/7] change CPUArchId.cpu type to Object*

[David Gibson]

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

On Thu, Feb 09, 2017 at 12:08:36PM +0100, Igor Mammedov wrote:
> so it could be reused for SPAPR cores as well
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/hw/boards.h | 2 +-
>  hw/acpi/cpu.c       | 2 +-
>  hw/i386/pc.c        | 8 ++++----
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 4023b38..60209df 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -51,7 +51,7 @@ void machine_register_compat_props(MachineState *machine);
>  typedef struct {
>      uint64_t arch_id;
>      CpuInstanceProperties props;
> -    struct CPUState *cpu;
> +    Object *cpu;
>  } CPUArchId;
>  
>  /**
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index 6017ca0..8c719d3 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -198,7 +198,7 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
>      state->dev_count = id_list->len;
>      state->devs = g_new0(typeof(*state->devs), state->dev_count);
>      for (i = 0; i < id_list->len; i++) {
> -        state->devs[i].cpu =  id_list->cpus[i].cpu;
> +        state->devs[i].cpu =  CPU(id_list->cpus[i].cpu);
>          state->devs[i].arch_id = id_list->cpus[i].arch_id;
>      }
>      memory_region_init_io(&state->ctrl_reg, owner, &cpu_hotplug_ops, state,
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index ec6dded..afaae15 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1824,7 +1824,7 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
>      }
>  
>      found_cpu = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, NULL);
> -    found_cpu->cpu = CPU(dev);
> +    found_cpu->cpu = OBJECT(dev);
>  out:
>      error_propagate(errp, local_err);
>  }
> @@ -2282,13 +2282,13 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
>  static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
>  {
>      int i;
> -    CPUState *cpu;
> +    Object *cpu;
>      HotpluggableCPUList *head = NULL;
>      const char *cpu_type;
>  
>      cpu = machine->possible_cpus->cpus[0].cpu;
>      assert(cpu); /* BSP is always present */
> -    cpu_type = object_class_get_name(OBJECT_CLASS(CPU_GET_CLASS(cpu)));
> +    cpu_type = object_get_typename(cpu);
>  
>      for (i = 0; i < machine->possible_cpus->len; i++) {
>          HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> @@ -2302,7 +2302,7 @@ static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
>          cpu = machine->possible_cpus->cpus[i].cpu;
>          if (cpu) {
>              cpu_item->has_qom_path = true;
> -            cpu_item->qom_path = object_get_canonical_path(OBJECT(cpu));
> +            cpu_item->qom_path = object_get_canonical_path(cpu);
>          }
>  
>          list_item->value = cpu_item;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 6/7] spapr: reuse machine->possible_cpus instead of cores[]

[David Gibson]

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

On Thu, Feb 09, 2017 at 12:08:37PM +0100, Igor Mammedov wrote:
> Replace SPAPR specific cores[] array with generic
> machine->possible_cpus and store core objects there.
> It makes cores bookkeeping similar to x86 cpus and
> will allow to unify similar code.
> It would allow to replace cpu_index based NUMA node
> mapping with iproperty based one (for -device created
> cores) since possible_cpus carries board defined
> topology/layout.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Apart from one type noted below,

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/hw/ppc/spapr.h |   1 -
>  hw/ppc/spapr.c         | 129 ++++++++++++++++++++++++++++++++++---------------
>  2 files changed, 90 insertions(+), 40 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index a2d8964..f9b17d8 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -94,7 +94,6 @@ struct sPAPRMachineState {
>      /*< public >*/
>      char *kvm_type;
>      MemoryHotplugState hotplug_memory;
> -    Object **cores;
>  };
>  
>  #define H_SUCCESS         0
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 37cb338..8d039ae 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1751,13 +1751,28 @@ static void spapr_validate_node_memory(MachineState *machine, Error **errp)
>      }
>  }
>  
> +/* find cpu slot in machine->possible_cpus by core_id */
> +static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
> +{
> +    int index = id / smp_threads;
> +
> +    if (index >= ms->possible_cpus->len) {
> +        return NULL;
> +    }
> +    if (idx) {
> +        *idx = index;
> +    }
> +    return &ms->possible_cpus->cpus[index];
> +}
> +
>  static void spapr_init_cpus(sPAPRMachineState *spapr)
>  {
>      MachineState *machine = MACHINE(spapr);
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>      char *type = spapr_get_cpu_core_type(machine->cpu_model);
>      int smt = kvmppc_smt_threads();
> -    int spapr_max_cores, spapr_cores;
> +    const CPUArchIdList *possible_cpus;
> +    int boot_cores_nr = smp_cpus / smp_threads;;

Stray extra ';' above.

>      int i;
>  
>      if (!type) {
> @@ -1765,6 +1780,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
>          exit(1);
>      }
>  
> +    possible_cpus = mc->possible_cpu_arch_ids(machine);
>      if (mc->query_hotpluggable_cpus) {
>          if (smp_cpus % smp_threads) {
>              error_report("smp_cpus (%u) must be multiple of threads (%u)",
> @@ -1776,21 +1792,15 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
>                           max_cpus, smp_threads);
>              exit(1);
>          }
> -
> -        spapr_max_cores = max_cpus / smp_threads;
> -        spapr_cores = smp_cpus / smp_threads;
>      } else {
>          if (max_cpus != smp_cpus) {
>              error_report("This machine version does not support CPU hotplug");
>              exit(1);
>          }
> -
> -        spapr_max_cores = QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_threads;
> -        spapr_cores = spapr_max_cores;
> +        boot_cores_nr = possible_cpus->len;
>      }
>  
> -    spapr->cores = g_new0(Object *, spapr_max_cores);
> -    for (i = 0; i < spapr_max_cores; i++) {
> +    for (i = 0; i < possible_cpus->len; i++) {
>          int core_id = i * smp_threads;
>  
>          if (mc->query_hotpluggable_cpus) {
> @@ -1802,7 +1812,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
>              qemu_register_reset(spapr_drc_reset, drc);
>          }
>  
> -        if (i < spapr_cores) {
> +        if (i < boot_cores_nr) {
>              Object *core  = object_new(type);
>              int nr_threads = smp_threads;
>  
> @@ -2491,10 +2501,11 @@ void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
>  static void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>                                Error **errp)
>  {
> -    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      CPUCore *cc = CPU_CORE(dev);
> +    CPUArchId *core_slot = spapr_find_cpu_slot(ms, cc->core_id, NULL);
>  
> -    spapr->cores[cc->core_id / smp_threads] = NULL;
> +    core_slot->cpu = NULL;
>      object_unparent(OBJECT(dev));
>  }
>  
> @@ -2510,19 +2521,24 @@ static
>  void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev,
>                                 Error **errp)
>  {
> -    CPUCore *cc = CPU_CORE(dev);
> -    int smt = kvmppc_smt_threads();
> -    int index = cc->core_id / smp_threads;
> -    sPAPRDRConnector *drc =
> -        spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
> +    int index;
> +    sPAPRDRConnector *drc;
>      sPAPRDRConnectorClass *drck;
>      Error *local_err = NULL;
> +    CPUCore *cc = CPU_CORE(dev);
> +    int smt = kvmppc_smt_threads();
>  
> +    if (!spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index)) {
> +        error_setg(errp, "Unable to find CPU core with core-id: %d",
> +                   cc->core_id);
> +        return;
> +    }
>      if (index == 0) {
>          error_setg(errp, "Boot CPU core may not be unplugged");
>          return;
>      }
>  
> +    drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
>      g_assert(drc);
>  
>      drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> @@ -2547,11 +2563,17 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>      Error *local_err = NULL;
>      void *fdt = NULL;
>      int fdt_offset = 0;
> -    int index = cc->core_id / smp_threads;
>      int smt = kvmppc_smt_threads();
> +    CPUArchId *core_slot;
> +    int index;
>  
> +    core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index);
> +    if (!core_slot) {
> +        error_setg(errp, "Unable to find CPU core with core-id: %d",
> +                   cc->core_id);
> +        return;
> +    }
>      drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
> -    spapr->cores[index] = OBJECT(dev);
>  
>      g_assert(drc || !mc->query_hotpluggable_cpus);
>  
> @@ -2568,7 +2590,6 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>          drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
>          if (local_err) {
>              g_free(fdt);
> -            spapr->cores[index] = NULL;
>              error_propagate(errp, local_err);
>              return;
>          }
> @@ -2590,6 +2611,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>              drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED);
>          }
>      }
> +    core_slot->cpu = OBJECT(dev);
>  }
>  
>  static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> @@ -2597,13 +2619,12 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>  {
>      MachineState *machine = MACHINE(OBJECT(hotplug_dev));
>      MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev);
> -    sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
> -    int spapr_max_cores = max_cpus / smp_threads;
> -    int index;
>      Error *local_err = NULL;
>      CPUCore *cc = CPU_CORE(dev);
>      char *base_core_type = spapr_get_cpu_core_type(machine->cpu_model);
>      const char *type = object_get_typename(OBJECT(dev));
> +    CPUArchId *core_slot;
> +    int index;
>  
>      if (dev->hotplugged && !mc->query_hotpluggable_cpus) {
>          error_setg(&local_err, "CPU hotplug not supported for this machine");
> @@ -2620,13 +2641,13 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>          goto out;
>      }
>  
> -    index = cc->core_id / smp_threads;
> -    if (index < 0 || index >= spapr_max_cores) {
> +    core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index);
> +    if (!core_slot) {
>          error_setg(&local_err, "core id %d out of range", cc->core_id);
>          goto out;
>      }
>  
> -    if (spapr->cores[index]) {
> +    if (core_slot->cpu) {
>          error_setg(&local_err, "core %d already populated", cc->core_id);
>          goto out;
>      }
> @@ -2758,29 +2779,58 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
>      return cpu_index / smp_threads / smp_cores;
>  }
>  
> +static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine)
> +{
> +    int i;
> +    int spapr_max_cores = max_cpus / smp_threads;
> +    MachineClass *mc = MACHINE_GET_CLASS(machine);
> +
> +    if (!mc->query_hotpluggable_cpus) {
> +        spapr_max_cores = QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_threads;
> +    }
> +    if (machine->possible_cpus) {
> +        assert(machine->possible_cpus->len == spapr_max_cores);
> +        return machine->possible_cpus;
> +    }
> +
> +    machine->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
> +                             sizeof(CPUArchId) * spapr_max_cores);
> +    machine->possible_cpus->len = spapr_max_cores;
> +    for (i = 0; i < machine->possible_cpus->len; i++) {
> +        int core_id = i * smp_threads;
> +
> +        machine->possible_cpus->cpus[i].arch_id = core_id;
> +        machine->possible_cpus->cpus[i].props.has_core_id = true;
> +        machine->possible_cpus->cpus[i].props.core_id = core_id;
> +        /* TODO: add 'has_node/node' here to describe
> +           to which node core belongs */
> +    }
> +    return machine->possible_cpus;
> +}
> +
>  static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
>  {
>      int i;
> +    Object *cpu;
>      HotpluggableCPUList *head = NULL;
> -    sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
> -    int spapr_max_cores = max_cpus / smp_threads;
> +    const char *cpu_type;
>  
> -    for (i = 0; i < spapr_max_cores; i++) {
> +    cpu = machine->possible_cpus->cpus[0].cpu;
> +    assert(cpu); /* Boot cpu is always present */
> +    cpu_type = object_get_typename(cpu);
> +    for (i = 0; i < machine->possible_cpus->len; i++) {
>          HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
>          HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> -        CpuInstanceProperties *cpu_props = g_new0(typeof(*cpu_props), 1);
>  
> -        cpu_item->type = spapr_get_cpu_core_type(machine->cpu_model);
> -        cpu_item->vcpus_count = smp_threads;
> -        cpu_props->has_core_id = true;
> -        cpu_props->core_id = i * smp_threads;
> -        /* TODO: add 'has_node/node' here to describe
> -           to which node core belongs */
> +        cpu_item->type = g_strdup(cpu_type);
> +        cpu_item->vcpus_count = smp_threads; // TODO: ??? generalize
> +        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
> +                                   sizeof(*cpu_item->props));
>  
> -        cpu_item->props = cpu_props;
> -        if (spapr->cores[i]) {
> +        cpu = machine->possible_cpus->cpus[i].cpu;
> +        if (cpu) {
>              cpu_item->has_qom_path = true;
> -            cpu_item->qom_path = object_get_canonical_path(spapr->cores[i]);
> +            cpu_item->qom_path = object_get_canonical_path(cpu);
>          }
>          list_item->value = cpu_item;
>          list_item->next = head;
> @@ -2872,6 +2922,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      hc->plug = spapr_machine_device_plug;
>      hc->unplug = spapr_machine_device_unplug;
>      mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
> +    mc->possible_cpu_arch_ids = spapr_possible_cpu_arch_ids;
>      hc->unplug_request = spapr_machine_device_unplug_request;
>  
>      smc->dr_lmb_enabled = true;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 7/7] machine: unify [pc_|spapr_]query_hotpluggable_cpus() callbacks

[David Gibson]

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

On Thu, Feb 09, 2017 at 12:08:38PM +0100, Igor Mammedov wrote:
> All callbacks FOO_query_hotpluggable_cpus() are practically
> the same except of setting vcpus_count to different values.
> Convert them to a generic machine_query_hotpluggable_cpus()
> callback by moving vcpus_count initialization to per machine
> specific callback possible_cpu_arch_ids().
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

It seems to be sensible to go further after this and remove the
query_hotpluggable_cpus callback from the MachineClass entirely,
replacing it with just a boolean 'cpu_hotplug_allowed'.

> ---
>  include/hw/boards.h |  3 +++
>  hw/core/machine.c   | 31 +++++++++++++++++++++++++++++++
>  hw/i386/pc.c        | 36 ++----------------------------------
>  hw/ppc/spapr.c      | 34 ++--------------------------------
>  4 files changed, 38 insertions(+), 66 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 60209df..9040dbb 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -41,15 +41,18 @@ int machine_phandle_start(MachineState *machine);
>  bool machine_dump_guest_core(MachineState *machine);
>  bool machine_mem_merge(MachineState *machine);
>  void machine_register_compat_props(MachineState *machine);
> +HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine);
>  
>  /**
>   * CPUArchId:
>   * @arch_id - architecture-dependent CPU ID of present or possible CPU
>   * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise
>   * @props - CPU object properties, initialized by board
> + * #vcpus_count - number of threads provided by @cpu object
>   */
>  typedef struct {
>      uint64_t arch_id;
> +    int64_t vcpus_count;
>      CpuInstanceProperties props;
>      Object *cpu;
>  } CPUArchId;
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index b0fd91f..0699750 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -357,6 +357,37 @@ static void machine_init_notify(Notifier *notifier, void *data)
>      foreach_dynamic_sysbus_device(error_on_sysbus_device, NULL);
>  }
>  
> +HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine)
> +{
> +    int i;
> +    Object *cpu;
> +    HotpluggableCPUList *head = NULL;
> +    const char *cpu_type;
> +
> +    cpu = machine->possible_cpus->cpus[0].cpu;
> +    assert(cpu); /* Boot cpu is always present */
> +    cpu_type = object_get_typename(cpu);
> +    for (i = 0; i < machine->possible_cpus->len; i++) {
> +        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> +        HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> +
> +        cpu_item->type = g_strdup(cpu_type);
> +        cpu_item->vcpus_count = machine->possible_cpus->cpus[i].vcpus_count;
> +        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
> +                                   sizeof(*cpu_item->props));
> +
> +        cpu = machine->possible_cpus->cpus[i].cpu;
> +        if (cpu) {
> +            cpu_item->has_qom_path = true;
> +            cpu_item->qom_path = object_get_canonical_path(cpu);
> +        }
> +        list_item->value = cpu_item;
> +        list_item->next = head;
> +        head = list_item;
> +    }
> +    return head;
> +}
> +
>  static void machine_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index afaae15..548628f 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2266,6 +2266,7 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
>      for (i = 0; i < ms->possible_cpus->len; i++) {
>          X86CPUTopoInfo topo;
>  
> +        ms->possible_cpus->cpus[i].vcpus_count = 1;
>          ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
>          x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
>                                   smp_cores, smp_threads, &topo);
> @@ -2279,39 +2280,6 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
>      return ms->possible_cpus;
>  }
>  
> -static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
> -{
> -    int i;
> -    Object *cpu;
> -    HotpluggableCPUList *head = NULL;
> -    const char *cpu_type;
> -
> -    cpu = machine->possible_cpus->cpus[0].cpu;
> -    assert(cpu); /* BSP is always present */
> -    cpu_type = object_get_typename(cpu);
> -
> -    for (i = 0; i < machine->possible_cpus->len; i++) {
> -        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> -        HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> -
> -        cpu_item->type = g_strdup(cpu_type);
> -        cpu_item->vcpus_count = 1;
> -        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
> -                                   sizeof(*cpu_item->props));
> -
> -        cpu = machine->possible_cpus->cpus[i].cpu;
> -        if (cpu) {
> -            cpu_item->has_qom_path = true;
> -            cpu_item->qom_path = object_get_canonical_path(cpu);
> -        }
> -
> -        list_item->value = cpu_item;
> -        list_item->next = head;
> -        head = list_item;
> -    }
> -    return head;
> -}
> -
>  static void x86_nmi(NMIState *n, int cpu_index, Error **errp)
>  {
>      /* cpu index isn't used */
> @@ -2352,7 +2320,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>      mc->get_hotplug_handler = pc_get_hotpug_handler;
>      mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
>      mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids;
> -    mc->query_hotpluggable_cpus = pc_query_hotpluggable_cpus;
> +    mc->query_hotpluggable_cpus = machine_query_hotpluggable_cpus;
>      mc->default_boot_order = "cad";
>      mc->hot_add_cpu = pc_hot_add_cpu;
>      mc->max_cpus = 255;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 8d039ae..3c4d632 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2799,6 +2799,7 @@ static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine)
>      for (i = 0; i < machine->possible_cpus->len; i++) {
>          int core_id = i * smp_threads;
>  
> +        machine->possible_cpus->cpus[i].vcpus_count = smp_threads;
>          machine->possible_cpus->cpus[i].arch_id = core_id;
>          machine->possible_cpus->cpus[i].props.has_core_id = true;
>          machine->possible_cpus->cpus[i].props.core_id = core_id;
> @@ -2808,37 +2809,6 @@ static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine)
>      return machine->possible_cpus;
>  }
>  
> -static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
> -{
> -    int i;
> -    Object *cpu;
> -    HotpluggableCPUList *head = NULL;
> -    const char *cpu_type;
> -
> -    cpu = machine->possible_cpus->cpus[0].cpu;
> -    assert(cpu); /* Boot cpu is always present */
> -    cpu_type = object_get_typename(cpu);
> -    for (i = 0; i < machine->possible_cpus->len; i++) {
> -        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> -        HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> -
> -        cpu_item->type = g_strdup(cpu_type);
> -        cpu_item->vcpus_count = smp_threads; // TODO: ??? generalize
> -        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
> -                                   sizeof(*cpu_item->props));
> -
> -        cpu = machine->possible_cpus->cpus[i].cpu;
> -        if (cpu) {
> -            cpu_item->has_qom_path = true;
> -            cpu_item->qom_path = object_get_canonical_path(cpu);
> -        }
> -        list_item->value = cpu_item;
> -        list_item->next = head;
> -        head = list_item;
> -    }
> -    return head;
> -}
> -
>  static void spapr_phb_placement(sPAPRMachineState *spapr, uint32_t index,
>                                  uint64_t *buid, hwaddr *pio,
>                                  hwaddr *mmio32, hwaddr *mmio64,
> @@ -2927,7 +2897,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>  
>      smc->dr_lmb_enabled = true;
>      smc->tcg_default_cpu = "POWER8";
> -    mc->query_hotpluggable_cpus = spapr_query_hotpluggable_cpus;
> +    mc->query_hotpluggable_cpus = machine_query_hotpluggable_cpus;
>      fwc->get_dev_path = spapr_get_fw_dev_path;
>      nc->nmi_monitor_handler = spapr_nmi;
>      smc->phb_placement = spapr_phb_placement;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 7/7] machine: unify [pc_|spapr_]query_hotpluggable_cpus() callbacks

[Igor Mammedov]

On Fri, 10 Feb 2017 10:53:39 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Feb 09, 2017 at 12:08:38PM +0100, Igor Mammedov wrote:
> > All callbacks FOO_query_hotpluggable_cpus() are practically
> > the same except of setting vcpus_count to different values.
> > Convert them to a generic machine_query_hotpluggable_cpus()
> > callback by moving vcpus_count initialization to per machine
> > specific callback possible_cpu_arch_ids().
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>  
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Thanks!
 
> It seems to be sensible to go further after this and remove the
> query_hotpluggable_cpus callback from the MachineClass entirely,
> replacing it with just a boolean 'cpu_hotplug_allowed'.
I'll post additional [8/7] patch to do it.

> 
> > ---
> >  include/hw/boards.h |  3 +++
> >  hw/core/machine.c   | 31 +++++++++++++++++++++++++++++++
> >  hw/i386/pc.c        | 36 ++----------------------------------
> >  hw/ppc/spapr.c      | 34 ++--------------------------------
> >  4 files changed, 38 insertions(+), 66 deletions(-)
> > 
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 60209df..9040dbb 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -41,15 +41,18 @@ int machine_phandle_start(MachineState *machine);
> >  bool machine_dump_guest_core(MachineState *machine);
> >  bool machine_mem_merge(MachineState *machine);
> >  void machine_register_compat_props(MachineState *machine);
> > +HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine);
> >  
> >  /**
> >   * CPUArchId:
> >   * @arch_id - architecture-dependent CPU ID of present or possible CPU
> >   * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise
> >   * @props - CPU object properties, initialized by board
> > + * #vcpus_count - number of threads provided by @cpu object
> >   */
> >  typedef struct {
> >      uint64_t arch_id;
> > +    int64_t vcpus_count;
> >      CpuInstanceProperties props;
> >      Object *cpu;
> >  } CPUArchId;
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index b0fd91f..0699750 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -357,6 +357,37 @@ static void machine_init_notify(Notifier *notifier, void *data)
> >      foreach_dynamic_sysbus_device(error_on_sysbus_device, NULL);
> >  }
> >  
> > +HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine)
> > +{
> > +    int i;
> > +    Object *cpu;
> > +    HotpluggableCPUList *head = NULL;
> > +    const char *cpu_type;
> > +
> > +    cpu = machine->possible_cpus->cpus[0].cpu;
> > +    assert(cpu); /* Boot cpu is always present */
> > +    cpu_type = object_get_typename(cpu);
> > +    for (i = 0; i < machine->possible_cpus->len; i++) {
> > +        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> > +        HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> > +
> > +        cpu_item->type = g_strdup(cpu_type);
> > +        cpu_item->vcpus_count = machine->possible_cpus->cpus[i].vcpus_count;
> > +        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
> > +                                   sizeof(*cpu_item->props));
> > +
> > +        cpu = machine->possible_cpus->cpus[i].cpu;
> > +        if (cpu) {
> > +            cpu_item->has_qom_path = true;
> > +            cpu_item->qom_path = object_get_canonical_path(cpu);
> > +        }
> > +        list_item->value = cpu_item;
> > +        list_item->next = head;
> > +        head = list_item;
> > +    }
> > +    return head;
> > +}
> > +
> >  static void machine_class_init(ObjectClass *oc, void *data)
> >  {
> >      MachineClass *mc = MACHINE_CLASS(oc);
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index afaae15..548628f 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -2266,6 +2266,7 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
> >      for (i = 0; i < ms->possible_cpus->len; i++) {
> >          X86CPUTopoInfo topo;
> >  
> > +        ms->possible_cpus->cpus[i].vcpus_count = 1;
> >          ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
> >          x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
> >                                   smp_cores, smp_threads, &topo);
> > @@ -2279,39 +2280,6 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
> >      return ms->possible_cpus;
> >  }
> >  
> > -static HotpluggableCPUList *pc_query_hotpluggable_cpus(MachineState *machine)
> > -{
> > -    int i;
> > -    Object *cpu;
> > -    HotpluggableCPUList *head = NULL;
> > -    const char *cpu_type;
> > -
> > -    cpu = machine->possible_cpus->cpus[0].cpu;
> > -    assert(cpu); /* BSP is always present */
> > -    cpu_type = object_get_typename(cpu);
> > -
> > -    for (i = 0; i < machine->possible_cpus->len; i++) {
> > -        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> > -        HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> > -
> > -        cpu_item->type = g_strdup(cpu_type);
> > -        cpu_item->vcpus_count = 1;
> > -        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
> > -                                   sizeof(*cpu_item->props));
> > -
> > -        cpu = machine->possible_cpus->cpus[i].cpu;
> > -        if (cpu) {
> > -            cpu_item->has_qom_path = true;
> > -            cpu_item->qom_path = object_get_canonical_path(cpu);
> > -        }
> > -
> > -        list_item->value = cpu_item;
> > -        list_item->next = head;
> > -        head = list_item;
> > -    }
> > -    return head;
> > -}
> > -
> >  static void x86_nmi(NMIState *n, int cpu_index, Error **errp)
> >  {
> >      /* cpu index isn't used */
> > @@ -2352,7 +2320,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
> >      mc->get_hotplug_handler = pc_get_hotpug_handler;
> >      mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
> >      mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids;
> > -    mc->query_hotpluggable_cpus = pc_query_hotpluggable_cpus;
> > +    mc->query_hotpluggable_cpus = machine_query_hotpluggable_cpus;
> >      mc->default_boot_order = "cad";
> >      mc->hot_add_cpu = pc_hot_add_cpu;
> >      mc->max_cpus = 255;
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 8d039ae..3c4d632 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2799,6 +2799,7 @@ static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine)
> >      for (i = 0; i < machine->possible_cpus->len; i++) {
> >          int core_id = i * smp_threads;
> >  
> > +        machine->possible_cpus->cpus[i].vcpus_count = smp_threads;
> >          machine->possible_cpus->cpus[i].arch_id = core_id;
> >          machine->possible_cpus->cpus[i].props.has_core_id = true;
> >          machine->possible_cpus->cpus[i].props.core_id = core_id;
> > @@ -2808,37 +2809,6 @@ static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine)
> >      return machine->possible_cpus;
> >  }
> >  
> > -static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
> > -{
> > -    int i;
> > -    Object *cpu;
> > -    HotpluggableCPUList *head = NULL;
> > -    const char *cpu_type;
> > -
> > -    cpu = machine->possible_cpus->cpus[0].cpu;
> > -    assert(cpu); /* Boot cpu is always present */
> > -    cpu_type = object_get_typename(cpu);
> > -    for (i = 0; i < machine->possible_cpus->len; i++) {
> > -        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> > -        HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> > -
> > -        cpu_item->type = g_strdup(cpu_type);
> > -        cpu_item->vcpus_count = smp_threads; // TODO: ??? generalize
> > -        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
> > -                                   sizeof(*cpu_item->props));
> > -
> > -        cpu = machine->possible_cpus->cpus[i].cpu;
> > -        if (cpu) {
> > -            cpu_item->has_qom_path = true;
> > -            cpu_item->qom_path = object_get_canonical_path(cpu);
> > -        }
> > -        list_item->value = cpu_item;
> > -        list_item->next = head;
> > -        head = list_item;
> > -    }
> > -    return head;
> > -}
> > -
> >  static void spapr_phb_placement(sPAPRMachineState *spapr, uint32_t index,
> >                                  uint64_t *buid, hwaddr *pio,
> >                                  hwaddr *mmio32, hwaddr *mmio64,
> > @@ -2927,7 +2897,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> >  
> >      smc->dr_lmb_enabled = true;
> >      smc->tcg_default_cpu = "POWER8";
> > -    mc->query_hotpluggable_cpus = spapr_query_hotpluggable_cpus;
> > +    mc->query_hotpluggable_cpus = machine_query_hotpluggable_cpus;
> >      fwc->get_dev_path = spapr_get_fw_dev_path;
> >      nc->nmi_monitor_handler = spapr_nmi;
> >      smc->phb_placement = spapr_phb_placement;  
> 

[Qemu-devel] [PATCH v2 6/7] spapr: reuse machine->possible_cpus instead of cores[]

[Igor Mammedov]

Replace SPAPR specific cores[] array with generic
machine->possible_cpus and store core objects there.
It makes cores bookkeeping similar to x86 cpus and
will allow to unify similar code.
It would allow to replace cpu_index based NUMA node
mapping with iproperty based one (for -device created
cores) since possible_cpus carries board defined
topology/layout.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
---
v2:
  - remove extra ';' at the end of expression (David Gibson)

CC: David Gibson <david@gibson.dropbear.id.au> (supporter:sPAPR)
CC: Alexander Graf <agraf@suse.de> (supporter:sPAPR)
CC: qemu-ppc@nongnu.org 
CC: Bharata B Rao <bharata@linux.vnet.ibm.com>
CC: ehabkost@redhat.com
CC: drjones@redhat.com
CC: Marcel Apfelbaum <marcel@redhat.com>
---
 include/hw/ppc/spapr.h |   1 -
 hw/ppc/spapr.c         | 129 ++++++++++++++++++++++++++++++++++---------------
 2 files changed, 90 insertions(+), 40 deletions(-)

diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index a2d8964..f9b17d8 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -94,7 +94,6 @@ struct sPAPRMachineState {
     /*< public >*/
     char *kvm_type;
     MemoryHotplugState hotplug_memory;
-    Object **cores;
 };
 
 #define H_SUCCESS         0
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 37cb338..a0aa69e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1751,13 +1751,28 @@ static void spapr_validate_node_memory(MachineState *machine, Error **errp)
     }
 }
 
+/* find cpu slot in machine->possible_cpus by core_id */
+static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
+{
+    int index = id / smp_threads;
+
+    if (index >= ms->possible_cpus->len) {
+        return NULL;
+    }
+    if (idx) {
+        *idx = index;
+    }
+    return &ms->possible_cpus->cpus[index];
+}
+
 static void spapr_init_cpus(sPAPRMachineState *spapr)
 {
     MachineState *machine = MACHINE(spapr);
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     char *type = spapr_get_cpu_core_type(machine->cpu_model);
     int smt = kvmppc_smt_threads();
-    int spapr_max_cores, spapr_cores;
+    const CPUArchIdList *possible_cpus;
+    int boot_cores_nr = smp_cpus / smp_threads;
     int i;
 
     if (!type) {
@@ -1765,6 +1780,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
         exit(1);
     }
 
+    possible_cpus = mc->possible_cpu_arch_ids(machine);
     if (mc->query_hotpluggable_cpus) {
         if (smp_cpus % smp_threads) {
             error_report("smp_cpus (%u) must be multiple of threads (%u)",
@@ -1776,21 +1792,15 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
                          max_cpus, smp_threads);
             exit(1);
         }
-
-        spapr_max_cores = max_cpus / smp_threads;
-        spapr_cores = smp_cpus / smp_threads;
     } else {
         if (max_cpus != smp_cpus) {
             error_report("This machine version does not support CPU hotplug");
             exit(1);
         }
-
-        spapr_max_cores = QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_threads;
-        spapr_cores = spapr_max_cores;
+        boot_cores_nr = possible_cpus->len;
     }
 
-    spapr->cores = g_new0(Object *, spapr_max_cores);
-    for (i = 0; i < spapr_max_cores; i++) {
+    for (i = 0; i < possible_cpus->len; i++) {
         int core_id = i * smp_threads;
 
         if (mc->query_hotpluggable_cpus) {
@@ -1802,7 +1812,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
             qemu_register_reset(spapr_drc_reset, drc);
         }
 
-        if (i < spapr_cores) {
+        if (i < boot_cores_nr) {
             Object *core  = object_new(type);
             int nr_threads = smp_threads;
 
@@ -2491,10 +2501,11 @@ void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
 static void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
                               Error **errp)
 {
-    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+    MachineState *ms = MACHINE(qdev_get_machine());
     CPUCore *cc = CPU_CORE(dev);
+    CPUArchId *core_slot = spapr_find_cpu_slot(ms, cc->core_id, NULL);
 
-    spapr->cores[cc->core_id / smp_threads] = NULL;
+    core_slot->cpu = NULL;
     object_unparent(OBJECT(dev));
 }
 
@@ -2510,19 +2521,24 @@ static
 void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev,
                                Error **errp)
 {
-    CPUCore *cc = CPU_CORE(dev);
-    int smt = kvmppc_smt_threads();
-    int index = cc->core_id / smp_threads;
-    sPAPRDRConnector *drc =
-        spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
+    int index;
+    sPAPRDRConnector *drc;
     sPAPRDRConnectorClass *drck;
     Error *local_err = NULL;
+    CPUCore *cc = CPU_CORE(dev);
+    int smt = kvmppc_smt_threads();
 
+    if (!spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index)) {
+        error_setg(errp, "Unable to find CPU core with core-id: %d",
+                   cc->core_id);
+        return;
+    }
     if (index == 0) {
         error_setg(errp, "Boot CPU core may not be unplugged");
         return;
     }
 
+    drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
     g_assert(drc);
 
     drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
@@ -2547,11 +2563,17 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
     Error *local_err = NULL;
     void *fdt = NULL;
     int fdt_offset = 0;
-    int index = cc->core_id / smp_threads;
     int smt = kvmppc_smt_threads();
+    CPUArchId *core_slot;
+    int index;
 
+    core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index);
+    if (!core_slot) {
+        error_setg(errp, "Unable to find CPU core with core-id: %d",
+                   cc->core_id);
+        return;
+    }
     drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
-    spapr->cores[index] = OBJECT(dev);
 
     g_assert(drc || !mc->query_hotpluggable_cpus);
 
@@ -2568,7 +2590,6 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
         drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
         if (local_err) {
             g_free(fdt);
-            spapr->cores[index] = NULL;
             error_propagate(errp, local_err);
             return;
         }
@@ -2590,6 +2611,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
             drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED);
         }
     }
+    core_slot->cpu = OBJECT(dev);
 }
 
 static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
@@ -2597,13 +2619,12 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
 {
     MachineState *machine = MACHINE(OBJECT(hotplug_dev));
     MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev);
-    sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
-    int spapr_max_cores = max_cpus / smp_threads;
-    int index;
     Error *local_err = NULL;
     CPUCore *cc = CPU_CORE(dev);
     char *base_core_type = spapr_get_cpu_core_type(machine->cpu_model);
     const char *type = object_get_typename(OBJECT(dev));
+    CPUArchId *core_slot;
+    int index;
 
     if (dev->hotplugged && !mc->query_hotpluggable_cpus) {
         error_setg(&local_err, "CPU hotplug not supported for this machine");
@@ -2620,13 +2641,13 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
         goto out;
     }
 
-    index = cc->core_id / smp_threads;
-    if (index < 0 || index >= spapr_max_cores) {
+    core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index);
+    if (!core_slot) {
         error_setg(&local_err, "core id %d out of range", cc->core_id);
         goto out;
     }
 
-    if (spapr->cores[index]) {
+    if (core_slot->cpu) {
         error_setg(&local_err, "core %d already populated", cc->core_id);
         goto out;
     }
@@ -2758,29 +2779,58 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
     return cpu_index / smp_threads / smp_cores;
 }
 
+static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine)
+{
+    int i;
+    int spapr_max_cores = max_cpus / smp_threads;
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
+
+    if (!mc->query_hotpluggable_cpus) {
+        spapr_max_cores = QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_threads;
+    }
+    if (machine->possible_cpus) {
+        assert(machine->possible_cpus->len == spapr_max_cores);
+        return machine->possible_cpus;
+    }
+
+    machine->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
+                             sizeof(CPUArchId) * spapr_max_cores);
+    machine->possible_cpus->len = spapr_max_cores;
+    for (i = 0; i < machine->possible_cpus->len; i++) {
+        int core_id = i * smp_threads;
+
+        machine->possible_cpus->cpus[i].arch_id = core_id;
+        machine->possible_cpus->cpus[i].props.has_core_id = true;
+        machine->possible_cpus->cpus[i].props.core_id = core_id;
+        /* TODO: add 'has_node/node' here to describe
+           to which node core belongs */
+    }
+    return machine->possible_cpus;
+}
+
 static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
 {
     int i;
+    Object *cpu;
     HotpluggableCPUList *head = NULL;
-    sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
-    int spapr_max_cores = max_cpus / smp_threads;
+    const char *cpu_type;
 
-    for (i = 0; i < spapr_max_cores; i++) {
+    cpu = machine->possible_cpus->cpus[0].cpu;
+    assert(cpu); /* Boot cpu is always present */
+    cpu_type = object_get_typename(cpu);
+    for (i = 0; i < machine->possible_cpus->len; i++) {
         HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
         HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
-        CpuInstanceProperties *cpu_props = g_new0(typeof(*cpu_props), 1);
 
-        cpu_item->type = spapr_get_cpu_core_type(machine->cpu_model);
-        cpu_item->vcpus_count = smp_threads;
-        cpu_props->has_core_id = true;
-        cpu_props->core_id = i * smp_threads;
-        /* TODO: add 'has_node/node' here to describe
-           to which node core belongs */
+        cpu_item->type = g_strdup(cpu_type);
+        cpu_item->vcpus_count = smp_threads; // TODO: ??? generalize
+        cpu_item->props = g_memdup(&machine->possible_cpus->cpus[i].props,
+                                   sizeof(*cpu_item->props));
 
-        cpu_item->props = cpu_props;
-        if (spapr->cores[i]) {
+        cpu = machine->possible_cpus->cpus[i].cpu;
+        if (cpu) {
             cpu_item->has_qom_path = true;
-            cpu_item->qom_path = object_get_canonical_path(spapr->cores[i]);
+            cpu_item->qom_path = object_get_canonical_path(cpu);
         }
         list_item->value = cpu_item;
         list_item->next = head;
@@ -2872,6 +2922,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     hc->plug = spapr_machine_device_plug;
     hc->unplug = spapr_machine_device_unplug;
     mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
+    mc->possible_cpu_arch_ids = spapr_possible_cpu_arch_ids;
     hc->unplug_request = spapr_machine_device_unplug_request;
 
     smc->dr_lmb_enabled = true;
-- 
2.7.4

[Qemu-devel] [PATCH v2 8/7] machine: replace query_hotpluggable_cpus() callback with has_hotpluggable_cpus flag

[Igor Mammedov]

Generic helper machine_query_hotpluggable_cpus() replaced
target specific query_hotpluggable_cpus() callbacks so
there is no need in it anymore. However inon NULL callback
value is used to detect/report hotpluggable cpus support,
therefore it can be removed completely.
Replace it with MachineClass.has_hotpluggable_cpus boolean
which is sufficient for the task.

Suggested-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
CC: David Gibson <david@gibson.dropbear.id.au> (supporter:sPAPR)
CC: Alexander Graf <agraf@suse.de> (supporter:sPAPR)
CC: qemu-ppc@nongnu.org 
CC: Bharata B Rao <bharata@linux.vnet.ibm.com>
CC: ehabkost@redhat.com
CC: drjones@redhat.com
CC: Marcel Apfelbaum <marcel@redhat.com>

---
 include/hw/boards.h |  8 +++-----
 hw/i386/pc.c        |  4 ++--
 hw/ppc/spapr.c      | 20 ++++++++++----------
 monitor.c           |  4 ++--
 vl.c                |  2 +-
 5 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 9040dbb..269d0ba 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -87,10 +87,8 @@ typedef struct {
  *    Returns an array of @CPUArchId architecture-dependent CPU IDs
  *    which includes CPU IDs for present and possible to hotplug CPUs.
  *    Caller is responsible for freeing returned list.
- * @query_hotpluggable_cpus:
- *    Returns a @HotpluggableCPUList, which describes CPUs objects which
- *    could be added with -device/device_add.
- *    Caller is responsible for freeing returned list.
+ * @has_hotpluggable_cpus:
+ *    If true, board supports CPUs creation with -device/device_add.
  * @minimum_page_bits:
  *    If non-zero, the board promises never to create a CPU with a page size
  *    smaller than this, so QEMU can use a more efficient larger page
@@ -136,12 +134,12 @@ struct MachineClass {
     bool option_rom_has_mr;
     bool rom_file_has_mr;
     int minimum_page_bits;
+    bool has_hotpluggable_cpus;
 
     HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
                                            DeviceState *dev);
     unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
     const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
-    HotpluggableCPUList *(*query_hotpluggable_cpus)(MachineState *machine);
 };
 
 /**
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 548628f..a8660d4 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1949,7 +1949,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
     }
 
     /* if 'address' properties socket-id/core-id/thread-id are not set, set them
-     * so that query_hotpluggable_cpus would show correct values
+     * so that machine_query_hotpluggable_cpus would show correct values
      */
     /* TODO: move socket_id/core_id/thread_id checks into x86_cpu_realizefn()
      * once -smp refactoring is complete and there will be CPU private
@@ -2320,7 +2320,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     mc->get_hotplug_handler = pc_get_hotpug_handler;
     mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
     mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids;
-    mc->query_hotpluggable_cpus = machine_query_hotpluggable_cpus;
+    mc->has_hotpluggable_cpus = true;
     mc->default_boot_order = "cad";
     mc->hot_add_cpu = pc_hot_add_cpu;
     mc->max_cpus = 255;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 49768eb..6f37288 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -958,7 +958,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
         _FDT(spapr_drc_populate_dt(fdt, 0, NULL, SPAPR_DR_CONNECTOR_TYPE_LMB));
     }
 
-    if (mc->query_hotpluggable_cpus) {
+    if (mc->has_hotpluggable_cpus) {
         int offset = fdt_path_offset(fdt, "/cpus");
         ret = spapr_drc_populate_dt(fdt, offset, NULL,
                                     SPAPR_DR_CONNECTOR_TYPE_CPU);
@@ -1781,7 +1781,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
     }
 
     possible_cpus = mc->possible_cpu_arch_ids(machine);
-    if (mc->query_hotpluggable_cpus) {
+    if (mc->has_hotpluggable_cpus) {
         if (smp_cpus % smp_threads) {
             error_report("smp_cpus (%u) must be multiple of threads (%u)",
                          smp_cpus, smp_threads);
@@ -1803,7 +1803,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
     for (i = 0; i < possible_cpus->len; i++) {
         int core_id = i * smp_threads;
 
-        if (mc->query_hotpluggable_cpus) {
+        if (mc->has_hotpluggable_cpus) {
             sPAPRDRConnector *drc =
                 spapr_dr_connector_new(OBJECT(spapr),
                                        SPAPR_DR_CONNECTOR_TYPE_CPU,
@@ -2575,7 +2575,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
     }
     drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
 
-    g_assert(drc || !mc->query_hotpluggable_cpus);
+    g_assert(drc || !mc->has_hotpluggable_cpus);
 
     /*
      * Setup CPU DT entries only for hotplugged CPUs. For boot time or
@@ -2626,7 +2626,7 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
     CPUArchId *core_slot;
     int index;
 
-    if (dev->hotplugged && !mc->query_hotpluggable_cpus) {
+    if (dev->hotplugged && !mc->has_hotpluggable_cpus) {
         error_setg(&local_err, "CPU hotplug not supported for this machine");
         goto out;
     }
@@ -2719,7 +2719,7 @@ static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev,
             error_setg(errp, "Memory hot unplug not supported for this guest");
         }
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
-        if (!mc->query_hotpluggable_cpus) {
+        if (!mc->has_hotpluggable_cpus) {
             error_setg(errp, "CPU hot unplug not supported on this machine");
             return;
         }
@@ -2746,7 +2746,7 @@ static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
             error_setg(errp, "Memory hot unplug not supported for this guest");
         }
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
-        if (!mc->query_hotpluggable_cpus) {
+        if (!mc->has_hotpluggable_cpus) {
             error_setg(errp, "CPU hot unplug not supported on this machine");
             return;
         }
@@ -2785,7 +2785,7 @@ static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine)
     int spapr_max_cores = max_cpus / smp_threads;
     MachineClass *mc = MACHINE_GET_CLASS(machine);
 
-    if (!mc->query_hotpluggable_cpus) {
+    if (!mc->has_hotpluggable_cpus) {
         spapr_max_cores = QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_threads;
     }
     if (machine->possible_cpus) {
@@ -2897,7 +2897,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
 
     smc->dr_lmb_enabled = true;
     smc->tcg_default_cpu = "POWER8";
-    mc->query_hotpluggable_cpus = machine_query_hotpluggable_cpus;
+    mc->has_hotpluggable_cpus = true;
     fwc->get_dev_path = spapr_get_fw_dev_path;
     nc->nmi_monitor_handler = spapr_nmi;
     smc->phb_placement = spapr_phb_placement;
@@ -3097,7 +3097,7 @@ static void spapr_machine_2_6_instance_options(MachineState *machine)
 static void spapr_machine_2_6_class_options(MachineClass *mc)
 {
     spapr_machine_2_7_class_options(mc);
-    mc->query_hotpluggable_cpus = NULL;
+    mc->has_hotpluggable_cpus = false;
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_6);
 }
 
diff --git a/monitor.c b/monitor.c
index 3cd72a9..18bf2f8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4155,10 +4155,10 @@ HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
     MachineState *ms = MACHINE(qdev_get_machine());
     MachineClass *mc = MACHINE_GET_CLASS(ms);
 
-    if (!mc->query_hotpluggable_cpus) {
+    if (!mc->has_hotpluggable_cpus) {
         error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
         return NULL;
     }
 
-    return mc->query_hotpluggable_cpus(ms);
+    return machine_query_hotpluggable_cpus(ms);
 }
diff --git a/vl.c b/vl.c
index b4eaf03..93406ba 100644
--- a/vl.c
+++ b/vl.c
@@ -1519,7 +1519,7 @@ MachineInfoList *qmp_query_machines(Error **errp)
 
         info->name = g_strdup(mc->name);
         info->cpu_max = !mc->max_cpus ? 1 : mc->max_cpus;
-        info->hotpluggable_cpus = !!mc->query_hotpluggable_cpus;
+        info->hotpluggable_cpus = mc->has_hotpluggable_cpus;
 
         entry = g_malloc0(sizeof(*entry));
         entry->value = info;
-- 
2.7.4

Re: [Qemu-devel] [PATCH 0/7] pc/spapr: unify handling of possible CPUs

[Igor Mammedov]

On Thu,  9 Feb 2017 12:08:31 +0100
Igor Mammedov <imammedo@redhat.com> wrote:

[...]
> 
> Series depends on:
>   [PATCH 0/3] spapr: fix cpu core hotunplug call flow
>   https://www.mail-archive.com/qemu-devel@nongnu.org/msg427214.html
> which is staged for next pull req in SPAPR tree.

David,

could you merge it through your ppc tree as this series
depends on above patches that are there but not in master yet.


[...]

Re: [Qemu-devel] [PATCH 0/7] pc/spapr: unify handling of possible CPUs

[David Gibson]

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

On Fri, Feb 10, 2017 at 11:31:08AM +0100, Igor Mammedov wrote:
> On Thu,  9 Feb 2017 12:08:31 +0100
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> [...]
> > 
> > Series depends on:
> >   [PATCH 0/3] spapr: fix cpu core hotunplug call flow
> >   https://www.mail-archive.com/qemu-devel@nongnu.org/msg427214.html
> > which is staged for next pull req in SPAPR tree.
> 
> David,
> 
> could you merge it through your ppc tree as this series
> depends on above patches that are there but not in master yet.

Ok, I've merged it into ppc-for-2.9, including the v2 6/7 and 8/7.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]