[Qemu-devel] [PATCH 0/8] ppc: compute cpu_dt_id in the machine code

[Qemu-devel] [PATCH 0/8] ppc: compute cpu_dt_id in the machine code

[Greg Kurz]

This series is a sequel to the discussion on a patch from Ben's powernv
patchset:

        http://patchwork.ozlabs.org/patch/597153/

Indeed, since the DT is a machine abstraction, it should definitely sit
under hw/ppc and not in the target code:
- all machine types are forced to share the same numbering logic
- user mode does not need that => there are #ifdef everywhere

So this series moves all the current numbering logic to the machine
code. It also provides the ability for each machine type to provide
its own numbering logic instead of using the legacy one.

The current code computes cpu_dt_id out of cpu_index, both getting
their values during cpu realization.

The idea is to compute cpu_index at cpu initialization time, so
that the machine can compute cpu_dt_id before realizing the cpu.

I had to fix some issues and to do some more structural changes to have
this working, hence the RFC tag. The first 4 patches are bug fixes. The
next ones do what the subject says.

This patchset is based on David's ppc-for-2.7 branch. I could test it plays
nicely with Bharata's CPU hotplug code for sPAPR.

There are some more places where the cpu_dt_id logic is open coded in the
sPAPR code. It may be worth to consolidate that in follow-up patches. Also
maybe other machine types may provide their own numbering logic if the
current one is not appropriate.

Please comment.

---

Greg Kurz (8):
      spapr: drop reference on child object during core realization
      spapr: do proper error propagation in spapr_cpu_core_realize_child()
      spapr: drop duplicate variable in spapr_core_release()
      exec: add missing conditional compilation
      exec: move cpu_index init and exit to their own function
      ppc: move cpu index setup to instance_init/finalize
      cpu: add initialization helper without realize
      hw/ppc: move DT cpu id generation to machine code


 exec.c                      |   72 +++++++++++++++++++++++++++++++++----------
 hw/ppc/e500.c               |    2 +
 hw/ppc/mac_newworld.c       |    2 +
 hw/ppc/mac_oldworld.c       |    2 +
 hw/ppc/ppc.c                |   60 ++++++++++++++++++++++++++++++++++++
 hw/ppc/ppc440_bamboo.c      |    2 +
 hw/ppc/ppc4xx_devs.c        |    2 +
 hw/ppc/prep.c               |    2 +
 hw/ppc/spapr.c              |    2 +
 hw/ppc/spapr_cpu_core.c     |   23 ++++++++++----
 hw/ppc/virtex_ml507.c       |    2 +
 include/exec/exec-all.h     |    2 +
 include/hw/ppc/ppc.h        |    4 ++
 include/qom/cpu.h           |   14 ++++++++
 qom/cpu.c                   |   19 ++++++++++-
 target-ppc/translate_init.c |   42 ++++++-------------------
 16 files changed, 185 insertions(+), 67 deletions(-)

--
Greg

[Qemu-devel] [PATCH 1/8] spapr: drop reference on child object during core realization

[Greg Kurz]

When a core is being realized, we create a child object for each thread
of the core.

The child is first initialized with object_initialize() which sets its ref
count to 1, and then added to the core with object_property_add_child()
which bumps the ref count to 2.

When the core gets released, object_unparent() decreases the ref count to 1,
and we g_free() the object: we hence loose the reference on an unfinalized
object. This is likely to cause random crashes.

Let's drop the extra reference as soon as we don't need it, after the
thread is added to the core.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_cpu_core.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 2aa0dc523374..789eb2e6f206 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -299,8 +299,9 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
         object_property_add_child(OBJECT(sc), id, obj, &local_err);
         if (local_err) {
             goto err;
         }
+        object_unref(obj);
     }
     object_child_foreach(OBJECT(dev), spapr_cpu_core_realize_child, &local_err);
     if (local_err) {
         goto err;

Re: [Qemu-devel] [PATCH 1/8] spapr: drop reference on child object during core realization

[Bharata B Rao]

On Wed, Jun 29, 2016 at 10:50:20PM +0200, Greg Kurz wrote:
> When a core is being realized, we create a child object for each thread
> of the core.
> 
> The child is first initialized with object_initialize() which sets its ref
> count to 1, and then added to the core with object_property_add_child()
> which bumps the ref count to 2.
> 
> When the core gets released, object_unparent() decreases the ref count to 1,
> and we g_free() the object: we hence loose the reference on an unfinalized
> object. This is likely to cause random crashes.
> 
> Let's drop the extra reference as soon as we don't need it, after the
> thread is added to the core.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Reviewed-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

[Qemu-devel] [PATCH 2/8] spapr: do proper error propagation in spapr_cpu_core_realize_child()

[Greg Kurz]

This patch changes spapr_cpu_core_realize_child() to have a local error
pointer and use error_propagate() as it is supposed to be done.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_cpu_core.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 789eb2e6f206..2615058da745 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -261,20 +261,22 @@ out:
 }
 
 static int spapr_cpu_core_realize_child(Object *child, void *opaque)
 {
-    Error **errp = opaque;
+    Error **errp = opaque, *local_err = NULL;
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
     CPUState *cs = CPU(child);
     PowerPCCPU *cpu = POWERPC_CPU(cs);
 
-    object_property_set_bool(child, true, "realized", errp);
-    if (*errp) {
+    object_property_set_bool(child, true, "realized", &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         return 1;
     }
 
-    spapr_cpu_init(spapr, cpu, errp);
-    if (*errp) {
+    spapr_cpu_init(spapr, cpu, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         return 1;
     }
     return 0;
 }

Re: [Qemu-devel] [PATCH 2/8] spapr: do proper error propagation in spapr_cpu_core_realize_child()

[Bharata B Rao]

On Wed, Jun 29, 2016 at 10:50:32PM +0200, Greg Kurz wrote:
> This patch changes spapr_cpu_core_realize_child() to have a local error
> pointer and use error_propagate() as it is supposed to be done.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Reviewed-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

[Qemu-devel] [PATCH 3/8] spapr: drop duplicate variable in spapr_core_release()

[Greg Kurz]

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_cpu_core.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 2615058da745..a15dc010626d 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -101,9 +101,8 @@ static void spapr_core_release(DeviceState *dev, void *opaque)
     sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
     const char *typename = object_class_get_name(sc->cpu_class);
     size_t size = object_type_get_instance_size(typename);
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
-    sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev));
     CPUCore *cc = CPU_CORE(dev);
     int smt = kvmppc_smt_threads();
     int i;
 
@@ -119,9 +118,9 @@ static void spapr_core_release(DeviceState *dev, void *opaque)
     }
 
     spapr->cores[cc->core_id / smt] = NULL;
 
-    g_free(core->threads);
+    g_free(sc->threads);
     object_unparent(OBJECT(dev));
 }
 
 void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,

Re: [Qemu-devel] [PATCH 3/8] spapr: drop duplicate variable in spapr_core_release()

[Bharata B Rao]

On Wed, Jun 29, 2016 at 10:50:45PM +0200, Greg Kurz wrote:
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/ppc/spapr_cpu_core.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 2615058da745..a15dc010626d 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -101,9 +101,8 @@ static void spapr_core_release(DeviceState *dev, void *opaque)
>      sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
>      const char *typename = object_class_get_name(sc->cpu_class);
>      size_t size = object_type_get_instance_size(typename);
>      sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> -    sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev));
>      CPUCore *cc = CPU_CORE(dev);
>      int smt = kvmppc_smt_threads();
>      int i;
> 
> @@ -119,9 +118,9 @@ static void spapr_core_release(DeviceState *dev, void *opaque)
>      }
> 
>      spapr->cores[cc->core_id / smt] = NULL;
> 
> -    g_free(core->threads);
> +    g_free(sc->threads);
>      object_unparent(OBJECT(dev));
>  }
> 
>  void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,

Reviewed-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

[Qemu-devel] [PATCH 4/8] exec: add missing conditional compilation

[Greg Kurz]

Commit 741da0d38 "hw: cannot include hw/hw.h from user emulation" also
switched off compilation of the vmstate bits in cpu_exec_init() for user
mode.

This patch does the same in cpu_exec_exit() because user mode shouldn't
call vmstate_unregister() either.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 exec.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index 0122ef76de5d..a528a1e6bbf6 100644
--- a/exec.c
+++ b/exec.c
@@ -654,17 +654,18 @@ void cpu_exec_exit(CPUState *cpu)
     QTAILQ_REMOVE(&cpus, cpu, node);
     cpu_release_index(cpu);
     cpu->cpu_index = -1;
 #if defined(CONFIG_USER_ONLY)
+    (void) cc;
     cpu_list_unlock();
-#endif
-
+#else
     if (cc->vmsd != NULL) {
         vmstate_unregister(NULL, cc->vmsd, cpu);
     }
     if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
         vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
     }
+#endif
 }
 
 void cpu_exec_init(CPUState *cpu, Error **errp)
 {

[Qemu-devel] [PATCH 5/8] exec: move cpu_index init and exit to their own function

[Greg Kurz]

This patch splits the cpu_index bits from the rest of the cpu_exec
init and exit code, so that they may be called separately. The goal
is to be able to initialize cpu_index during cpu initialization and
keep the rest for cpu realization.

The cpu_exec_init() and cpu_exec_exit() functions are kept since most
callers will stick to the current behaviour.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 exec.c                  |   77 +++++++++++++++++++++++++++++++++++------------
 include/exec/exec-all.h |    2 +
 include/qom/cpu.h       |    2 +
 3 files changed, 61 insertions(+), 20 deletions(-)

diff --git a/exec.c b/exec.c
index a528a1e6bbf6..a9b3b4c2ca45 100644
--- a/exec.c
+++ b/exec.c
@@ -635,26 +635,17 @@ static void cpu_release_index(CPUState *cpu)
     return;
 }
 #endif
 
-void cpu_exec_exit(CPUState *cpu)
+void cpu_exec_unrealize(CPUState *cpu)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
 
 #if defined(CONFIG_USER_ONLY)
     cpu_list_lock();
 #endif
-    if (cpu->cpu_index == -1) {
-        /* cpu_index was never allocated by this @cpu or was already freed. */
-#if defined(CONFIG_USER_ONLY)
-        cpu_list_unlock();
-#endif
-        return;
-    }
 
     QTAILQ_REMOVE(&cpus, cpu, node);
-    cpu_release_index(cpu);
-    cpu->cpu_index = -1;
 #if defined(CONFIG_USER_ONLY)
     (void) cc;
     cpu_list_unlock();
 #else
@@ -666,13 +657,54 @@ void cpu_exec_exit(CPUState *cpu)
     }
 #endif
 }
 
-void cpu_exec_init(CPUState *cpu, Error **errp)
+void cpu_exec_exit_cpu_index(CPUState *cpu)
+{
+#if defined(CONFIG_USER_ONLY)
+    cpu_list_lock();
+#endif
+    if (cpu->cpu_index == -1) {
+        /* cpu_index was never allocated by this @cpu or was already freed. */
+#if defined(CONFIG_USER_ONLY)
+        cpu_list_unlock();
+#endif
+        return;
+    }
+
+    cpu_release_index(cpu);
+    cpu->cpu_index = -1;
+#if defined(CONFIG_USER_ONLY)
+    cpu_list_unlock();
+#endif
+}
+
+void cpu_exec_exit(CPUState *cpu)
+{
+    cpu_exec_unrealize(cpu);
+    cpu_exec_exit_cpu_index(cpu);
+}
+
+void cpu_exec_init_cpu_index(CPUState *cpu, Error **errp)
 {
-    CPUClass *cc = CPU_GET_CLASS(cpu);
     Error *local_err = NULL;
 
+#if defined(CONFIG_USER_ONLY)
+    cpu_list_lock();
+#endif
+    cpu->cpu_index = cpu_get_free_index(&local_err);
+#if defined(CONFIG_USER_ONLY)
+    cpu_list_unlock();
+#endif
+    if (local_err) {
+        error_propagate(errp, local_err);
+    }
+}
+
+void cpu_exec_realize(CPUState *cpu, Error **errp)
+{
+    CPUClass *cc = CPU_GET_CLASS(cpu);
+
     cpu->as = NULL;
     cpu->num_ases = 0;
 
 #ifndef CONFIG_USER_ONLY
@@ -695,16 +727,8 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
 
 #if defined(CONFIG_USER_ONLY)
     cpu_list_lock();
 #endif
-    cpu->cpu_index = cpu_get_free_index(&local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-#if defined(CONFIG_USER_ONLY)
-        cpu_list_unlock();
-#endif
-        return;
-    }
     QTAILQ_INSERT_TAIL(&cpus, cpu, node);
 #if defined(CONFIG_USER_ONLY)
     (void) cc;
     cpu_list_unlock();
@@ -717,8 +741,21 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
     }
 #endif
 }
 
+void cpu_exec_init(CPUState *cpu, Error **errp)
+{
+    Error *local_err = NULL;
+
+    cpu_exec_init_cpu_index(cpu, &local_err);
+    if (local_err == NULL) {
+        cpu_exec_realize(cpu, &local_err);
+    }
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+    }
+}
+
 #if defined(CONFIG_USER_ONLY)
 static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
 {
     tb_invalidate_phys_page_range(pc, pc + 1, 0);
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index c1f59fa59d2c..0f4cdf368945 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -56,8 +56,10 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
                               target_ulong pc, target_ulong cs_base,
                               uint32_t flags,
                               int cflags);
 void cpu_exec_init(CPUState *cpu, Error **errp);
+void cpu_exec_init_cpu_index(CPUState *cpu, Error **errp);
+void cpu_exec_realize(CPUState *cpu, Error **errp);
 void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
 void QEMU_NORETURN cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc);
 
 #if !defined(CONFIG_USER_ONLY)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 32f3af3e1c63..63e77607f644 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -854,8 +854,10 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx);
 
 void QEMU_NORETURN cpu_abort(CPUState *cpu, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
 void cpu_exec_exit(CPUState *cpu);
+bool cpu_exec_exit_cpu_index(CPUState *cpu);
+void cpu_exec_unrealize(CPUState *cpu);
 
 #ifdef CONFIG_SOFTMMU
 extern const struct VMStateDescription vmstate_cpu_common;
 #else

[Qemu-devel] [PATCH 6/8] ppc: move cpu index setup to instance_init/finalize

[Greg Kurz]

This patch moves cpu_index initialization to instance init: it will
allow machine code to compute cpu_dt_id between cpu initialization and
cpu realization.

It also adds the related exit code to be called at instance finalize, for
symmetry.

This doesn't change behaviour when the cpu is setup with cpu_ppc_init(),
which handles initialization and realization sequentially. This is
currently the case for all configurations in both user and system mode.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 target-ppc/translate_init.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index d7860fd7f8ee..f7571f34d6ac 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9540,9 +9540,9 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 #endif
 
-    cpu_exec_init(cs, &local_err);
+    cpu_exec_realize(cs, &local_err);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
         return;
     }
@@ -9772,9 +9772,9 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
     CPUPPCState *env = &cpu->env;
     opc_handler_t **table;
     int i, j;
 
-    cpu_exec_exit(CPU(dev));
+    cpu_exec_unrealize(CPU(dev));
 
     for (i = 0; i < PPC_CPU_OPCODES_LEN; i++) {
         if (env->opcodes[i] == &invalid_handler) {
             continue;
@@ -10336,8 +10336,15 @@ static void ppc_cpu_initfn(Object *obj)
 
     if (tcg_enabled()) {
         ppc_translate_init();
     }
+
+    cpu_exec_init_cpu_index(cs, &error_abort);
+}
+
+static void ppc_cpu_finalizefn(Object *obj)
+{
+    cpu_exec_exit_cpu_index(CPU(obj));
 }
 
 static bool ppc_pvr_match_default(PowerPCCPUClass *pcc, uint32_t pvr)
 {
@@ -10413,8 +10420,9 @@ static const TypeInfo ppc_cpu_type_info = {
     .name = TYPE_POWERPC_CPU,
     .parent = TYPE_CPU,
     .instance_size = sizeof(PowerPCCPU),
     .instance_init = ppc_cpu_initfn,
+    .instance_finalize = ppc_cpu_finalizefn,
     .abstract = true,
     .class_size = sizeof(PowerPCCPUClass),
     .class_init = ppc_cpu_class_init,
 };

[Qemu-devel] [PATCH 7/8] cpu: add initialization helper without realize

[Greg Kurz]

This will allow PowerPC machines to compute the device-tree cpu id
between initialization and realization of the cpu.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 include/qom/cpu.h |   12 ++++++++++++
 qom/cpu.c         |   19 ++++++++++++++++---
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 63e77607f644..8b3adbbff060 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -557,8 +557,20 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model);
  */
 CPUState *cpu_generic_init(const char *typename, const char *cpu_model);
 
 /**
+ * cpu_generic_init_no_realize:
+ * @typename: The CPU base type.
+ * @cpu_model: The model string including optional parameters.
+ *
+ * Instantiates a CPU, processes optional parameters.
+ *
+ * Returns: A #CPUState or %NULL if an error occurred.
+ */
+CPUState *cpu_generic_init_no_realize(const char *typename,
+                                      const char *cpu_model);
+
+/**
  * cpu_has_work:
  * @cpu: The vCPU to check.
  *
  * Checks whether the CPU has work to do.
diff --git a/qom/cpu.c b/qom/cpu.c
index 751e992de882..dfe3289991ab 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -42,9 +42,10 @@ bool cpu_exists(int64_t id)
     }
     return false;
 }
 
-CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
+static CPUState *cpu_generic_init_common(const char *typename,
+                                         const char *cpu_model, bool realize)
 {
     char *str, *name, *featurestr;
     CPUState *cpu;
     ObjectClass *oc;
@@ -69,10 +70,11 @@ CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
     if (err != NULL) {
         goto out;
     }
 
-    object_property_set_bool(OBJECT(cpu), true, "realized", &err);
-
+    if (realize) {
+        object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+    }
 out:
     if (err != NULL) {
         error_report_err(err);
         object_unref(OBJECT(cpu));
@@ -81,8 +83,19 @@ out:
 
     return cpu;
 }
 
+CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
+{
+    return cpu_generic_init_common(typename, cpu_model, true);
+}
+
+CPUState *cpu_generic_init_no_realize(const char *typename,
+                                      const char *cpu_model)
+{
+    return cpu_generic_init_common(typename, cpu_model, false);
+}
+
 bool cpu_paging_enabled(const CPUState *cpu)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
 

[Qemu-devel] [PATCH 8/8] hw/ppc: move DT cpu id generation to machine code

[Greg Kurz]

Now that cpu_index is computed at cpu initialization time, we can
compute cpu_dt_id in the machine code.

All the logic moves from ppc_cpu_realizefn() to a generic function
in the machine code, that serves as the default for all machine types.

A new ppc_cpu_init() helper is also added to be used instead of the
current cpu_ppc_init() from the target code. It allows each machine
type to use the default numbering logic or to provide its own one,
which will be called just before realizing the cpu.

This has no impact on user mode since all of this is for system mode
only.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/e500.c               |    2 +
 hw/ppc/mac_newworld.c       |    2 +
 hw/ppc/mac_oldworld.c       |    2 +
 hw/ppc/ppc.c                |   60 +++++++++++++++++++++++++++++++++++++++++++
 hw/ppc/ppc440_bamboo.c      |    2 +
 hw/ppc/ppc4xx_devs.c        |    2 +
 hw/ppc/prep.c               |    2 +
 hw/ppc/spapr.c              |    2 +
 hw/ppc/spapr_cpu_core.c     |    7 +++++
 hw/ppc/virtex_ml507.c       |    2 +
 include/hw/ppc/ppc.h        |    4 +++
 target-ppc/translate_init.c |   30 ----------------------
 12 files changed, 79 insertions(+), 38 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 0cd534df55f8..a8153448c4d4 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -820,9 +820,9 @@ void ppce500_init(MachineState *machine, PPCE500Params *params)
         PowerPCCPU *cpu;
         CPUState *cs;
         qemu_irq *input;
 
-        cpu = cpu_ppc_init(machine->cpu_model);
+        cpu = ppc_cpu_init(machine->cpu_model, NULL);
         if (cpu == NULL) {
             fprintf(stderr, "Unable to initialize CPU!\n");
             exit(1);
         }
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 32e88b378687..5d4c28cdb5b2 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -192,9 +192,9 @@ static void ppc_core99_init(MachineState *machine)
         machine->cpu_model = "G4";
 #endif
     }
     for (i = 0; i < smp_cpus; i++) {
-        cpu = cpu_ppc_init(machine->cpu_model);
+        cpu = ppc_cpu_init(machine->cpu_model, NULL);
         if (cpu == NULL) {
             fprintf(stderr, "Unable to find PowerPC CPU definition\n");
             exit(1);
         }
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 447948746b1a..3ebabd7f203b 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -112,9 +112,9 @@ static void ppc_heathrow_init(MachineState *machine)
     /* init CPUs */
     if (machine->cpu_model == NULL)
         machine->cpu_model = "G3";
     for (i = 0; i < smp_cpus; i++) {
-        cpu = cpu_ppc_init(machine->cpu_model);
+        cpu = ppc_cpu_init(machine->cpu_model, NULL);
         if (cpu == NULL) {
             fprintf(stderr, "Unable to find PowerPC CPU definition\n");
             exit(1);
         }
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index e4252528a69d..c234b7adfd43 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -36,8 +36,9 @@
 #include "hw/loader.h"
 #include "sysemu/kvm.h"
 #include "kvm_ppc.h"
 #include "trace.h"
+#include "qapi/error.h"
 
 //#define PPC_DEBUG_IRQ
 //#define PPC_DEBUG_TB
 
@@ -1349,4 +1350,63 @@ PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id)
     }
 
     return NULL;
 }
+
+void ppc_cpu_compute_dt_id_generic(PowerPCCPU *cpu, Error **errp)
+{
+    CPUState *cs = CPU(cpu);
+    int max_smt = kvm_enabled() ? kvmppc_smt_threads() : 1;
+
+    if (smp_threads > max_smt) {
+        error_setg(errp, "Cannot support more than %d threads on PPC with %s",
+                   max_smt, kvm_enabled() ? "KVM" : "TCG");
+        return;
+    }
+    if (!is_power_of_2(smp_threads)) {
+        error_setg(errp, "Cannot support %d threads on PPC with %s, "
+                   "threads count must be a power of 2.",
+                   smp_threads, kvm_enabled() ? "KVM" : "TCG");
+        return;
+    }
+
+    if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->cpu_dt_id)) {
+        error_setg(errp, "Can't create CPU with id %d in KVM", cpu->cpu_dt_id);
+        error_append_hint(errp, "Adjust the number of cpus to %d "
+                          "or try to raise the number of threads per core\n",
+                          cpu->cpu_dt_id * smp_threads / max_smt);
+        return;
+    }
+
+    cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt
+        + (cs->cpu_index % smp_threads);
+}
+
+PowerPCCPU *ppc_cpu_init(const char *cpu_model, cpu_compute_dt_id_fn compute_fn)
+{
+    PowerPCCPU *cpu;
+    Error *err = NULL;
+
+    cpu = POWERPC_CPU(cpu_generic_init_no_realize(TYPE_POWERPC_CPU, cpu_model));
+    if (cpu == NULL) {
+        return NULL;
+    }
+
+    if (compute_fn) {
+        compute_fn(cpu, &err);
+    } else {
+        ppc_cpu_compute_dt_id_generic(cpu, &err);
+    }
+    if (err != NULL) {
+        goto out;
+    }
+
+    object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+out:
+    if (err != NULL) {
+        error_report_err(err);
+        object_unref(OBJECT(cpu));
+        return NULL;
+    }
+
+    return cpu;
+}
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 5c535b18a20d..c5ea15b1f732 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -185,9 +185,9 @@ static void bamboo_init(MachineState *machine)
     /* Setup CPU. */
     if (machine->cpu_model == NULL) {
         machine->cpu_model = "440EP";
     }
-    cpu = cpu_ppc_init(machine->cpu_model);
+    cpu = ppc_cpu_init(machine->cpu_model, NULL);
     if (cpu == NULL) {
         fprintf(stderr, "Unable to initialize CPU!\n");
         exit(1);
     }
diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
index e7f413e49d08..b28bca1b797b 100644
--- a/hw/ppc/ppc4xx_devs.c
+++ b/hw/ppc/ppc4xx_devs.c
@@ -55,9 +55,9 @@ PowerPCCPU *ppc4xx_init(const char *cpu_model,
     PowerPCCPU *cpu;
     CPUPPCState *env;
 
     /* init CPUs */
-    cpu = cpu_ppc_init(cpu_model);
+    cpu = ppc_cpu_init(cpu_model, NULL);
     if (cpu == NULL) {
         fprintf(stderr, "Unable to find PowerPC %s CPU definition\n",
                 cpu_model);
         exit(1);
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 054af1e8b481..1bd3ede62114 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -508,9 +508,9 @@ static void ppc_prep_init(MachineState *machine)
     /* init CPUs */
     if (machine->cpu_model == NULL)
         machine->cpu_model = "602";
     for (i = 0; i < smp_cpus; i++) {
-        cpu = cpu_ppc_init(machine->cpu_model);
+        cpu = ppc_cpu_init(machine->cpu_model, NULL);
         if (cpu == NULL) {
             fprintf(stderr, "Unable to find PowerPC CPU definition\n");
             exit(1);
         }
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0771c93f6550..040c9158cce4 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1834,9 +1834,9 @@ static void ppc_spapr_init(MachineState *machine)
         }
         g_free(type);
     } else {
         for (i = 0; i < smp_cpus; i++) {
-            PowerPCCPU *cpu = cpu_ppc_init(machine->cpu_model);
+            PowerPCCPU *cpu = ppc_cpu_init(machine->cpu_model, NULL);
             if (cpu == NULL) {
                 error_report("Unable to find PowerPC CPU definition");
                 exit(1);
             }
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index a15dc010626d..56f360cb6c7f 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -12,8 +12,9 @@
 #include "hw/ppc/spapr.h"
 #include "hw/boards.h"
 #include "qapi/error.h"
 #include <sysemu/cpus.h>
+#include <sysemu/kvm.h>
 #include "target-ppc/kvm_ppc.h"
 #include "hw/ppc/ppc.h"
 #include "target-ppc/mmu-hash64.h"
 #include <sysemu/numa.h>
@@ -265,8 +266,14 @@ static int spapr_cpu_core_realize_child(Object *child, void *opaque)
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
     CPUState *cs = CPU(child);
     PowerPCCPU *cpu = POWERPC_CPU(cs);
 
+    ppc_cpu_compute_dt_id_generic(cpu, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return 1;
+    }
+
     object_property_set_bool(child, true, "realized", &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return 1;
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index b97d96685cf1..cf3e71b9f2cc 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -95,9 +95,9 @@ static PowerPCCPU *ppc440_init_xilinx(ram_addr_t *ram_size,
     PowerPCCPU *cpu;
     CPUPPCState *env;
     qemu_irq *irqs;
 
-    cpu = cpu_ppc_init(cpu_model);
+    cpu = ppc_cpu_init(cpu_model, NULL);
     if (cpu == NULL) {
         fprintf(stderr, "Unable to initialize CPU!\n");
         exit(1);
     }
diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
index 5617dc4a2c04..3e6aa7e7720f 100644
--- a/include/hw/ppc/ppc.h
+++ b/include/hw/ppc/ppc.h
@@ -105,5 +105,9 @@ enum {
 
 /* ppc_booke.c */
 void ppc_booke_timers_init(PowerPCCPU *cpu, uint32_t freq, uint32_t flags);
 
+typedef void (*cpu_compute_dt_id_fn)(PowerPCCPU *cpu, Error **errp);
+PowerPCCPU *ppc_cpu_init(const char *cpu_model,
+                         cpu_compute_dt_id_fn compute_fn);
+void ppc_cpu_compute_dt_id_generic(PowerPCCPU *cpu, Error **errp);
 #endif
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index f7571f34d6ac..f9d1ee8b12f9 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9522,45 +9522,15 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
     CPUState *cs = CPU(dev);
     PowerPCCPU *cpu = POWERPC_CPU(dev);
     PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
     Error *local_err = NULL;
-#if !defined(CONFIG_USER_ONLY)
-    int max_smt = kvm_enabled() ? kvmppc_smt_threads() : 1;
-#endif
-
-#if !defined(CONFIG_USER_ONLY)
-    if (smp_threads > max_smt) {
-        error_setg(errp, "Cannot support more than %d threads on PPC with %s",
-                   max_smt, kvm_enabled() ? "KVM" : "TCG");
-        return;
-    }
-    if (!is_power_of_2(smp_threads)) {
-        error_setg(errp, "Cannot support %d threads on PPC with %s, "
-                   "threads count must be a power of 2.",
-                   smp_threads, kvm_enabled() ? "KVM" : "TCG");
-        return;
-    }
-#endif
 
     cpu_exec_realize(cs, &local_err);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
         return;
     }
 
-#if !defined(CONFIG_USER_ONLY)
-    cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt
-        + (cs->cpu_index % smp_threads);
-
-    if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->cpu_dt_id)) {
-        error_setg(errp, "Can't create CPU with id %d in KVM", cpu->cpu_dt_id);
-        error_append_hint(errp, "Adjust the number of cpus to %d "
-                          "or try to raise the number of threads per core\n",
-                          cpu->cpu_dt_id * smp_threads / max_smt);
-        return;
-    }
-#endif
-
     if (tcg_enabled()) {
         if (ppc_fixup_cpu(cpu) != 0) {
             error_setg(errp, "Unable to emulate selected CPU with TCG");
             return;

Re: [Qemu-devel] [Qemu-ppc] [PATCH 0/8] ppc: compute cpu_dt_id in the machine code

[Greg Kurz]

On Wed, 29 Jun 2016 22:50:06 +0200
Greg Kurz <groug@kaod.org> wrote:

> This series is a sequel to the discussion on a patch from Ben's powernv
> patchset:
> 
>         http://patchwork.ozlabs.org/patch/597153/
> 
> Indeed, since the DT is a machine abstraction, it should definitely sit
> under hw/ppc and not in the target code:
> - all machine types are forced to share the same numbering logic
> - user mode does not need that => there are #ifdef everywhere
> 
> So this series moves all the current numbering logic to the machine
> code. It also provides the ability for each machine type to provide
> its own numbering logic instead of using the legacy one.
> 
> The current code computes cpu_dt_id out of cpu_index, both getting
> their values during cpu realization.
> 
> The idea is to compute cpu_index at cpu initialization time, so
> that the machine can compute cpu_dt_id before realizing the cpu.
> 
> I had to fix some issues and to do some more structural changes to have
> this working, hence the RFC tag. The first 4 patches are bug fixes. The
> next ones do what the subject says.
> 

Of course I forgot to add the RFC tag in the subject :) but yes, the 4
last patches especially deserve some feedback and are definitely not
2.7 material.

> This patchset is based on David's ppc-for-2.7 branch. I could test it plays
> nicely with Bharata's CPU hotplug code for sPAPR.
> 
> There are some more places where the cpu_dt_id logic is open coded in the
> sPAPR code. It may be worth to consolidate that in follow-up patches. Also
> maybe other machine types may provide their own numbering logic if the
> current one is not appropriate.
> 
> Please comment.
> 
> ---
> 
> Greg Kurz (8):
>       spapr: drop reference on child object during core realization
>       spapr: do proper error propagation in spapr_cpu_core_realize_child()
>       spapr: drop duplicate variable in spapr_core_release()
>       exec: add missing conditional compilation
>       exec: move cpu_index init and exit to their own function
>       ppc: move cpu index setup to instance_init/finalize
>       cpu: add initialization helper without realize
>       hw/ppc: move DT cpu id generation to machine code
> 
> 
>  exec.c                      |   72 +++++++++++++++++++++++++++++++++----------
>  hw/ppc/e500.c               |    2 +
>  hw/ppc/mac_newworld.c       |    2 +
>  hw/ppc/mac_oldworld.c       |    2 +
>  hw/ppc/ppc.c                |   60 ++++++++++++++++++++++++++++++++++++
>  hw/ppc/ppc440_bamboo.c      |    2 +
>  hw/ppc/ppc4xx_devs.c        |    2 +
>  hw/ppc/prep.c               |    2 +
>  hw/ppc/spapr.c              |    2 +
>  hw/ppc/spapr_cpu_core.c     |   23 ++++++++++----
>  hw/ppc/virtex_ml507.c       |    2 +
>  include/exec/exec-all.h     |    2 +
>  include/hw/ppc/ppc.h        |    4 ++
>  include/qom/cpu.h           |   14 ++++++++
>  qom/cpu.c                   |   19 ++++++++++-
>  target-ppc/translate_init.c |   42 ++++++-------------------
>  16 files changed, 185 insertions(+), 67 deletions(-)
> 
> --
> Greg
> 
> 

Re: [Qemu-devel] [PATCH 0/8] ppc: compute cpu_dt_id in the machine code

[David Gibson]

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

On Wed, Jun 29, 2016 at 10:50:06PM +0200, Greg Kurz wrote:
> This series is a sequel to the discussion on a patch from Ben's powernv
> patchset:
> 
>         http://patchwork.ozlabs.org/patch/597153/
> 
> Indeed, since the DT is a machine abstraction, it should definitely sit
> under hw/ppc and not in the target code:
> - all machine types are forced to share the same numbering logic
> - user mode does not need that => there are #ifdef everywhere
> 
> So this series moves all the current numbering logic to the machine
> code. It also provides the ability for each machine type to provide
> its own numbering logic instead of using the legacy one.
> 
> The current code computes cpu_dt_id out of cpu_index, both getting
> their values during cpu realization.
> 
> The idea is to compute cpu_index at cpu initialization time, so
> that the machine can compute cpu_dt_id before realizing the cpu.
> 
> I had to fix some issues and to do some more structural changes to have
> this working, hence the RFC tag. The first 4 patches are bug fixes. The
> next ones do what the subject says.
> 
> This patchset is based on David's ppc-for-2.7 branch. I could test it plays
> nicely with Bharata's CPU hotplug code for sPAPR.
> 
> There are some more places where the cpu_dt_id logic is open coded in the
> sPAPR code. It may be worth to consolidate that in follow-up patches. Also
> maybe other machine types may provide their own numbering logic if the
> current one is not appropriate.

Since they're clear bugfixes, I've applied 1-3/8 to ppc-for-2.7.  As
you say the rest of the series needs more discussion.

-- 
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 0/8] ppc: compute cpu_dt_id in the machine code

[Igor Mammedov]

On Wed, 29 Jun 2016 22:50:06 +0200
Greg Kurz <groug@kaod.org> wrote:

> This series is a sequel to the discussion on a patch from Ben's powernv
> patchset:
> 
>         http://patchwork.ozlabs.org/patch/597153/
> 
> Indeed, since the DT is a machine abstraction, it should definitely sit
> under hw/ppc and not in the target code:
> - all machine types are forced to share the same numbering logic
> - user mode does not need that => there are #ifdef everywhere
> 
> So this series moves all the current numbering logic to the machine
> code. It also provides the ability for each machine type to provide
> its own numbering logic instead of using the legacy one.
> 
> The current code computes cpu_dt_id out of cpu_index, both getting
> their values during cpu realization.
>
> The idea is to compute cpu_index at cpu initialization time, so
> that the machine can compute cpu_dt_id before realizing the cpu.
Series goes to considerable lengths to reuse cpu_index,
maybe other way around would be better (like we did for x86) where
the end goal was the same as here i.e. make machine code assign/manage
apic_id (x86 equivalent of cpu_dt_id). So what we did to make it happen.

 - do not try to reuse cpu_index and leave it alone
 - do not use cpu_init() in machine code, leave it for creating a CPU only for usermode
 - make target code use dt_id_property instead of cpu_index internally
 - make machine code to construct cpu manually
     cpu = object_new(CPU)
     set_dt_id_property(cpu)
     set other properties if needed
     realize(cpu)
   see pc_new_cpu() as example
 - make machine code do mgmt/bookkeeping of DT_IDs
     possibly you could reuse CPUArchIdList for it,
     see pc_cpus_init() + usage of pcms->possible_cpus

> I had to fix some issues and to do some more structural changes to have
> this working, hence the RFC tag. The first 4 patches are bug fixes. The
> next ones do what the subject says.
> 
> This patchset is based on David's ppc-for-2.7 branch. I could test it plays
> nicely with Bharata's CPU hotplug code for sPAPR.
> 
> There are some more places where the cpu_dt_id logic is open coded in the
> sPAPR code. It may be worth to consolidate that in follow-up patches. Also
> maybe other machine types may provide their own numbering logic if the
> current one is not appropriate.
> 
> Please comment.
> 
> ---
> 
> Greg Kurz (8):
>       spapr: drop reference on child object during core realization
>       spapr: do proper error propagation in spapr_cpu_core_realize_child()
>       spapr: drop duplicate variable in spapr_core_release()
>       exec: add missing conditional compilation
>       exec: move cpu_index init and exit to their own function
>       ppc: move cpu index setup to instance_init/finalize
>       cpu: add initialization helper without realize
>       hw/ppc: move DT cpu id generation to machine code
> 
> 
>  exec.c                      |   72 +++++++++++++++++++++++++++++++++----------
>  hw/ppc/e500.c               |    2 +
>  hw/ppc/mac_newworld.c       |    2 +
>  hw/ppc/mac_oldworld.c       |    2 +
>  hw/ppc/ppc.c                |   60 ++++++++++++++++++++++++++++++++++++
>  hw/ppc/ppc440_bamboo.c      |    2 +
>  hw/ppc/ppc4xx_devs.c        |    2 +
>  hw/ppc/prep.c               |    2 +
>  hw/ppc/spapr.c              |    2 +
>  hw/ppc/spapr_cpu_core.c     |   23 ++++++++++----
>  hw/ppc/virtex_ml507.c       |    2 +
>  include/exec/exec-all.h     |    2 +
>  include/hw/ppc/ppc.h        |    4 ++
>  include/qom/cpu.h           |   14 ++++++++
>  qom/cpu.c                   |   19 ++++++++++-
>  target-ppc/translate_init.c |   42 ++++++-------------------
>  16 files changed, 185 insertions(+), 67 deletions(-)
> 
> --
> Greg
> 
> 

Re: [Qemu-devel] [PATCH 0/8] ppc: compute cpu_dt_id in the machine code

[Greg Kurz]

On Thu, 30 Jun 2016 09:34:35 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Wed, 29 Jun 2016 22:50:06 +0200
> Greg Kurz <groug@kaod.org> wrote:
> 
> > This series is a sequel to the discussion on a patch from Ben's powernv
> > patchset:
> > 
> >         http://patchwork.ozlabs.org/patch/597153/
> > 
> > Indeed, since the DT is a machine abstraction, it should definitely sit
> > under hw/ppc and not in the target code:
> > - all machine types are forced to share the same numbering logic
> > - user mode does not need that => there are #ifdef everywhere
> > 
> > So this series moves all the current numbering logic to the machine
> > code. It also provides the ability for each machine type to provide
> > its own numbering logic instead of using the legacy one.
> > 
> > The current code computes cpu_dt_id out of cpu_index, both getting
> > their values during cpu realization.
> >
> > The idea is to compute cpu_index at cpu initialization time, so
> > that the machine can compute cpu_dt_id before realizing the cpu.  
> Series goes to considerable lengths to reuse cpu_index,

I fully agree ! 

> maybe other way around would be better (like we did for x86) where
> the end goal was the same as here i.e. make machine code assign/manage
> apic_id (x86 equivalent of cpu_dt_id). So what we did to make it happen.
> 
>  - do not try to reuse cpu_index and leave it alone

Looking again at patches 5/8 and 6/8, I'm more than convinced, yeah :)

>  - do not use cpu_init() in machine code, leave it for creating a CPU only for usermode

Makes sense, cpu_ppc_init() should only be used by usermode. I'll start with
that I guess.

>  - make target code use dt_id_property instead of cpu_index internally

Agreed.

>  - make machine code to construct cpu manually
>      cpu = object_new(CPU)
>      set_dt_id_property(cpu)
>      set other properties if needed
>      realize(cpu)
>    see pc_new_cpu() as example

With the recent addition of CPU hotplug, the latest pseries machine type is
now constructing cpus that way. The idea in patches 7-8/8 was to provide a
helper to do the same for other machines.

>  - make machine code do mgmt/bookkeeping of DT_IDs
>      possibly you could reuse CPUArchIdList for it,
>      see pc_cpus_init() + usage of pcms->possible_cpus
> 

I'll have look.

Thanks a lot for your feedback !

Cheers.

--
Greg

> > I had to fix some issues and to do some more structural changes to have
> > this working, hence the RFC tag. The first 4 patches are bug fixes. The
> > next ones do what the subject says.
> > 
> > This patchset is based on David's ppc-for-2.7 branch. I could test it plays
> > nicely with Bharata's CPU hotplug code for sPAPR.
> > 
> > There are some more places where the cpu_dt_id logic is open coded in the
> > sPAPR code. It may be worth to consolidate that in follow-up patches. Also
> > maybe other machine types may provide their own numbering logic if the
> > current one is not appropriate.
> > 
> > Please comment.
> > 
> > ---
> > 
> > Greg Kurz (8):
> >       spapr: drop reference on child object during core realization
> >       spapr: do proper error propagation in spapr_cpu_core_realize_child()
> >       spapr: drop duplicate variable in spapr_core_release()
> >       exec: add missing conditional compilation
> >       exec: move cpu_index init and exit to their own function
> >       ppc: move cpu index setup to instance_init/finalize
> >       cpu: add initialization helper without realize
> >       hw/ppc: move DT cpu id generation to machine code
> > 
> > 
> >  exec.c                      |   72 +++++++++++++++++++++++++++++++++----------
> >  hw/ppc/e500.c               |    2 +
> >  hw/ppc/mac_newworld.c       |    2 +
> >  hw/ppc/mac_oldworld.c       |    2 +
> >  hw/ppc/ppc.c                |   60 ++++++++++++++++++++++++++++++++++++
> >  hw/ppc/ppc440_bamboo.c      |    2 +
> >  hw/ppc/ppc4xx_devs.c        |    2 +
> >  hw/ppc/prep.c               |    2 +
> >  hw/ppc/spapr.c              |    2 +
> >  hw/ppc/spapr_cpu_core.c     |   23 ++++++++++----
> >  hw/ppc/virtex_ml507.c       |    2 +
> >  include/exec/exec-all.h     |    2 +
> >  include/hw/ppc/ppc.h        |    4 ++
> >  include/qom/cpu.h           |   14 ++++++++
> >  qom/cpu.c                   |   19 ++++++++++-
> >  target-ppc/translate_init.c |   42 ++++++-------------------
> >  16 files changed, 185 insertions(+), 67 deletions(-)
> > 
> > --
> > Greg
> > 
> >   
>