[Qemu-devel] [PATCH v2 00/20] Split cpu_exec_init() into an init and a realize part

[Qemu-devel] [PATCH v2 00/20] Split cpu_exec_init() into an init and a realize part

[Laurent Vivier]

Since commit 42ecaba ("target-i386: Call cpu_exec_init() on realize"),
, commit 6dd0f83 ("target-ppc: Move cpu_exec_init() call to realize function"),
and commit c6644fc ("s390x/cpu: Get rid of side effects when creating a vcpu"),
cpu_exec_init() has been moved to realize function for some architectures
to implement CPU htoplug. This allows any failures from cpu_exec_init() to be
handled appropriately.

This series tries to do the same work for all the other CPUs.

But as the ARM Virtual Machine ("virt") needs the "memory" property of the CPU
in the machine init function (the "memory" property is created in
cpu_exec_init() we want to move to the realize part), split cpu_exec_init() in
two parts: a realize part (cpu_exec_realize(), adding the CPU in the
environment) and an init part (cpu_exec_init(), initializing the CPU, like
adding the "memory" property).

For target-i386, target-s390 and target-ppc, we move back the cpu_exec_init()
into the init part, and put the cpu_exec_realize() into the realize part.
For all the other CPUs, we add the cpu_exec_realize() function into the
realize part.

This also allows to remove all the "cannot_destroy_with_object_finalize_yet"
properties from the CPU device class.

v2:
- rename cpu_exec_exit() as cpu_exec_unrealize(),
  as it un-does what cpu_exec_realize() does,
- remove cpu_exec_exit() from ppc_cpu_unrealizefn() as
  it is called from cpu_common_finalize(),
- add a patch to move all cpu_exec_init() from
  all XX_cpu_initfn() to cpu_common_initfn(),
- arm: move setting of cpu->mp_affinity to
  arm_cpu_realizefn() as the cpu_index is now set in
  cpu_exec_realizefn().
- update some commit messages

Laurent Vivier (20):
  exec: split cpu_exec_init()
  target-i386: move back cpu_exec_init() to init
  target-ppc: move back cpu_exec_init() to init
  target-s390: move back cpu_exec_init() to init
  target-arm: move cpu_exec_realize() to realize function
  target-alpha: move cpu_exec_realize() to realize function
  target-cris: move cpu_exec_realize() to realize function
  target-lm32: move cpu_exec_realize() to realize function
  target-m68k: move cpu_exec_realize() to realize function
  target-microblaze: move cpu_exec_realize() to realize function
  target-mips: move cpu_exec_realize() to realize function
  target-moxie: move cpu_exec_realize() to realize function
  target-openrisc: move cpu_exec_realize() to realize function
  target-sh4: move cpu_exec_realize() to realize function
  target-sparc: move cpu_exec_realize() to realize function
  target-tilegx: move cpu_exec_realize() to realize function
  target-tricore: move cpu_exec_realize() to realize function
  target-unicore32: move cpu_exec_realize() to realize function
  target-xtensa: move cpu_exec_realize() to realize function
  exec: move cpu_exec_init() to cpu_common_initfn()

 exec.c                      | 12 +++++++-----
 include/exec/exec-all.h     |  2 +-
 include/qom/cpu.h           |  3 ++-
 qom/cpu.c                   |  4 +++-
 target-alpha/cpu.c          | 15 +++++++--------
 target-arm/cpu.c            | 39 +++++++++++++++++----------------------
 target-cris/cpu.c           | 15 +++++++--------
 target-i386/cpu.c           | 11 +++++------
 target-lm32/cpu.c           | 15 +++++++--------
 target-m68k/cpu.c           | 15 +++++++--------
 target-microblaze/cpu.c     | 14 +++++++-------
 target-mips/cpu.c           | 15 +++++++--------
 target-moxie/cpu.c          | 15 +++++++--------
 target-openrisc/cpu.c       | 15 +++++++--------
 target-ppc/translate_init.c |  4 +---
 target-s390x/cpu.c          |  8 +-------
 target-sh4/cpu.c            | 15 +++++++--------
 target-sparc/cpu.c          | 18 +++++++++---------
 target-tilegx/cpu.c         | 15 +++++++--------
 target-tricore/cpu.c        | 15 +++++++--------
 target-unicore32/cpu.c      | 18 +++++++++---------
 target-xtensa/cpu.c         | 15 +++++++--------
 22 files changed, 139 insertions(+), 159 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v2 01/20] exec: split cpu_exec_init()

[Laurent Vivier]

Extract the realize part to cpu_exec_realize(), update all
calls to cpu_exec_init() to add cpu_exec_realize() to
have no functionnal change.

Put in cpu_exec_init() what initializes the CPU,
in cpu_exec_realize() what adds it to the environment.

Remove error parameter from cpu_exec_init() as it can't fail.

Rename cpu_exec_exit() with cpu_exec_unrealize():
cpu_exec_exit() is undoing what it has been done by cpu_exec_realize(), so
call it cpu_exec_unrealize().

CC: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 exec.c                      | 12 +++++++-----
 include/exec/exec-all.h     |  3 ++-
 include/qom/cpu.h           |  2 +-
 qom/cpu.c                   |  2 +-
 target-alpha/cpu.c          |  3 ++-
 target-arm/cpu.c            |  3 ++-
 target-cris/cpu.c           |  3 ++-
 target-i386/cpu.c           |  3 ++-
 target-lm32/cpu.c           |  3 ++-
 target-m68k/cpu.c           |  3 ++-
 target-microblaze/cpu.c     |  3 ++-
 target-mips/cpu.c           |  3 ++-
 target-moxie/cpu.c          |  3 ++-
 target-openrisc/cpu.c       |  3 ++-
 target-ppc/translate_init.c |  5 +++--
 target-s390x/cpu.c          |  3 ++-
 target-sh4/cpu.c            |  3 ++-
 target-sparc/cpu.c          |  3 ++-
 target-tilegx/cpu.c         |  3 ++-
 target-tricore/cpu.c        |  3 ++-
 target-unicore32/cpu.c      |  3 ++-
 target-xtensa/cpu.c         |  3 ++-
 22 files changed, 48 insertions(+), 27 deletions(-)

diff --git a/exec.c b/exec.c
index 374c364..885dc79 100644
--- a/exec.c
+++ b/exec.c
@@ -596,7 +596,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
 }
 #endif
 
-void cpu_exec_exit(CPUState *cpu)
+void cpu_exec_unrealize(CPUState *cpu)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
 
@@ -610,11 +610,8 @@ void cpu_exec_exit(CPUState *cpu)
     }
 }
 
-void cpu_exec_init(CPUState *cpu, Error **errp)
+void cpu_exec_init(CPUState *cpu)
 {
-    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
-    Error *local_err ATTRIBUTE_UNUSED = NULL;
-
     cpu->as = NULL;
     cpu->num_ases = 0;
 
@@ -635,6 +632,11 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
     cpu->memory = system_memory;
     object_ref(OBJECT(cpu->memory));
 #endif
+}
+
+void cpu_exec_realize(CPUState *cpu, Error **errp)
+{
+    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
 
     cpu_list_add(cpu);
 
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 336a57c..b42533e 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -57,7 +57,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
                               uint32_t flags,
                               int cflags);
 
-void cpu_exec_init(CPUState *cpu, Error **errp);
+void cpu_exec_init(CPUState *cpu);
+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);
 
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 6d481a1..4962980 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -946,7 +946,7 @@ 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);
+void cpu_exec_unrealize(CPUState *cpu);
 
 #ifdef CONFIG_SOFTMMU
 extern const struct VMStateDescription vmstate_cpu_common;
diff --git a/qom/cpu.c b/qom/cpu.c
index c40f774..39590e1 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -367,7 +367,7 @@ static void cpu_common_initfn(Object *obj)
 static void cpu_common_finalize(Object *obj)
 {
     CPUState *cpu = CPU(obj);
-    cpu_exec_exit(cpu);
+    cpu_exec_unrealize(CPU(obj));
     g_free(cpu->trace_dstate);
 }
 
diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
index 6d01d7f..98761d7 100644
--- a/target-alpha/cpu.c
+++ b/target-alpha/cpu.c
@@ -266,7 +266,8 @@ static void alpha_cpu_initfn(Object *obj)
     CPUAlphaState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
     tlb_flush(cs, 1);
 
     alpha_translate_init();
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 1b9540e..7e58134 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -444,7 +444,8 @@ static void arm_cpu_initfn(Object *obj)
     uint32_t Aff1, Aff0;
 
     cs->env_ptr = &cpu->env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
     cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
                                          g_free, g_free);
 
diff --git a/target-cris/cpu.c b/target-cris/cpu.c
index d680cfb..e28abc1 100644
--- a/target-cris/cpu.c
+++ b/target-cris/cpu.c
@@ -187,7 +187,8 @@ static void cris_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
     env->pregs[PR_VR] = ccc->vr;
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1c57fce..b977130 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3158,7 +3158,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
             cpu->phys_bits = 32;
         }
     }
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled()) {
         tcg_x86_init();
diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
index a783d46..147cc60 100644
--- a/target-lm32/cpu.c
+++ b/target-lm32/cpu.c
@@ -160,7 +160,8 @@ static void lm32_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
     env->flags = 0;
 
diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c
index 116b784..2768bbb 100644
--- a/target-m68k/cpu.c
+++ b/target-m68k/cpu.c
@@ -176,7 +176,8 @@ static void m68k_cpu_initfn(Object *obj)
     static bool inited;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled() && !inited) {
         inited = true;
diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
index 8edc00a..71fd5fc 100644
--- a/target-microblaze/cpu.c
+++ b/target-microblaze/cpu.c
@@ -199,7 +199,8 @@ static void mb_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
     set_float_rounding_mode(float_round_nearest_even, &env->fp_status);
 
diff --git a/target-mips/cpu.c b/target-mips/cpu.c
index 64ad112..1154d11 100644
--- a/target-mips/cpu.c
+++ b/target-mips/cpu.c
@@ -138,7 +138,8 @@ static void mips_cpu_initfn(Object *obj)
     CPUMIPSState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled()) {
         mips_tcg_init();
diff --git a/target-moxie/cpu.c b/target-moxie/cpu.c
index 50a0899..c9eed19 100644
--- a/target-moxie/cpu.c
+++ b/target-moxie/cpu.c
@@ -75,7 +75,8 @@ static void moxie_cpu_initfn(Object *obj)
     static int inited;
 
     cs->env_ptr = &cpu->env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled() && !inited) {
         inited = 1;
diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
index 155913f..74d52bf 100644
--- a/target-openrisc/cpu.c
+++ b/target-openrisc/cpu.c
@@ -95,7 +95,8 @@ static void openrisc_cpu_initfn(Object *obj)
     static int inited;
 
     cs->env_ptr = &cpu->env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
 #ifndef CONFIG_USER_ONLY
     cpu_openrisc_mmu_init(cpu);
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index b66b40b..094f28a 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9678,7 +9678,8 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 #endif
 
-    cpu_exec_init(cs, &local_err);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &local_err);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
         return;
@@ -9910,7 +9911,7 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
     opc_handler_t **table, **table_2;
     int i, j, k;
 
-    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) {
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 35ae2ce..4339da1 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -207,7 +207,8 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
         goto out;
     }
 
-    cpu_exec_init(cs, &err);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &err);
     if (err != NULL) {
         goto out;
     }
diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
index f589532..60689c7 100644
--- a/target-sh4/cpu.c
+++ b/target-sh4/cpu.c
@@ -258,7 +258,8 @@ static void superh_cpu_initfn(Object *obj)
     CPUSH4State *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
     env->movcal_backup_tail = &(env->movcal_backup);
 
diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
index 800a25a..135f30c 100644
--- a/target-sparc/cpu.c
+++ b/target-sparc/cpu.c
@@ -814,7 +814,8 @@ static void sparc_cpu_initfn(Object *obj)
     CPUSPARCState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled()) {
         gen_intermediate_code_init(env);
diff --git a/target-tilegx/cpu.c b/target-tilegx/cpu.c
index 7017cb6..ae6ea6e 100644
--- a/target-tilegx/cpu.c
+++ b/target-tilegx/cpu.c
@@ -107,7 +107,8 @@ static void tilegx_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled() && !tcg_initialized) {
         tcg_initialized = true;
diff --git a/target-tricore/cpu.c b/target-tricore/cpu.c
index 35d4ee4..43f444f 100644
--- a/target-tricore/cpu.c
+++ b/target-tricore/cpu.c
@@ -95,7 +95,8 @@ static void tricore_cpu_initfn(Object *obj)
     CPUTriCoreState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled()) {
         tricore_tcg_init();
diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
index e7a4984..ffc01bd 100644
--- a/target-unicore32/cpu.c
+++ b/target-unicore32/cpu.c
@@ -116,7 +116,8 @@ static void uc32_cpu_initfn(Object *obj)
     static bool inited;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
 #ifdef CONFIG_USER_ONLY
     env->uncached_asr = ASR_MODE_USER;
diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c
index 5ad08a2..2719b73 100644
--- a/target-xtensa/cpu.c
+++ b/target-xtensa/cpu.c
@@ -117,7 +117,8 @@ static void xtensa_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     env->config = xcc->config;
-    cpu_exec_init(cs, &error_abort);
+    cpu_exec_init(cs);
+    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled() && !tcg_inited) {
         tcg_inited = true;
-- 
2.7.4

[Qemu-devel] [PATCH v2 02/20] target-i386: move back cpu_exec_init() to init

[Laurent Vivier]

We have now the cpu_exec_realize() in realize,
so the init part must be in init.

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
"athlon-x86_64-cpu")

CC: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-i386/cpu.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b977130..4741dd6 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3158,8 +3158,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
             cpu->phys_bits = 32;
         }
     }
-    cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     if (tcg_enabled()) {
         tcg_x86_init();
@@ -3365,6 +3368,7 @@ static void x86_cpu_initfn(Object *obj)
     FeatureWord w;
 
     cs->env_ptr = env;
+    cpu_exec_init(cs);
 
     object_property_add(obj, "family", "int",
                         x86_cpuid_version_get_family,
@@ -3538,11 +3542,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     cc->cpu_exec_exit = x86_cpu_exec_exit;
 
     dc->cannot_instantiate_with_device_add_yet = false;
-    /*
-     * Reason: x86_cpu_initfn() calls cpu_exec_init(), which saves the
-     * object in cpus -> dangling pointer after final object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static const TypeInfo x86_cpu_type_info = {
-- 
2.7.4

[Qemu-devel] [PATCH v2 03/20] target-ppc: move back cpu_exec_init() to init

[Laurent Vivier]

We have now the cpu_exec_realize() in realize,
so the init part must be in init.

As cpu_exec_unrealize() is called from cpu_common_finalize(),
remove the call from ppc_cpu_unrealizefn().

CC: Bharata B Rao <bharata@linux.vnet.ibm.com>
CC: Alexander Graf <agraf@suse.de>
CC: qemu-ppc@nongnu.org
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-ppc/translate_init.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 094f28a..bbca8b5 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9678,7 +9678,6 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 #endif
 
-    cpu_exec_init(cs);
     cpu_exec_realize(cs, &local_err);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
@@ -9911,8 +9910,6 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
     opc_handler_t **table, **table_2;
     int i, j, k;
 
-    cpu_exec_unrealize(CPU(dev));
-
     for (i = 0; i < PPC_CPU_OPCODES_LEN; i++) {
         if (env->opcodes[i] == &invalid_handler) {
             continue;
@@ -10435,6 +10432,7 @@ static void ppc_cpu_initfn(Object *obj)
     CPUPPCState *env = &cpu->env;
 
     cs->env_ptr = env;
+    cpu_exec_init(cs);
 
     env->msr_mask = pcc->msr_mask;
     env->mmu_model = pcc->mmu_model;
-- 
2.7.4

[Qemu-devel] [PATCH v2 04/20] target-s390: move back cpu_exec_init() to init

[Laurent Vivier]

We have now the cpu_exec_realize() in realize,
so the init part must be in init.

I've removed the cannot_destroy_with_object_finalize_yet field as
it should be removed by commit c6644fc.
(tested with QOM command provided by commit 4c315c27 with
"z890.3-s390-cpu")

CC: Richard Henderson <rth@twiddle.net>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-s390x/cpu.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 4339da1..f92adad 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -207,7 +207,6 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
         goto out;
     }
 
-    cpu_exec_init(cs);
     cpu_exec_realize(cs, &err);
     if (err != NULL) {
         goto out;
@@ -290,6 +289,7 @@ static void s390_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     cs->halted = 1;
     cs->exception_index = EXCP_HLT;
+    cpu_exec_init(cs);
     object_property_add(OBJECT(cpu), "id", "int64_t", s390x_cpu_get_id,
                         s390x_cpu_set_id, NULL, NULL, NULL);
     s390_cpu_model_register_props(obj);
@@ -441,12 +441,6 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
     cc->gdb_core_xml_file = "s390x-core64.xml";
     cc->gdb_arch_name = s390_gdb_arch_name;
 
-    /*
-     * Reason: s390_cpu_realizefn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
     s390_cpu_model_class_register_props(oc);
 }
 
-- 
2.7.4

[Qemu-devel] [PATCH v2 05/20] target-arm: move cpu_exec_realize() to realize function

[Laurent Vivier]

Setting of cpu->mp_affinity is moved from arm_cpu_initfn()
to arm_cpu_realizefn() as setting of cpu_index is now done
in cpu_exec_realize().

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "arm1026-arm-cpu")

CC: Peter Maydell <peter.maydell@linaro.org>
CC: qemu-arm@nongnu.org
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-arm/cpu.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 7e58134..54e7991 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -441,23 +441,12 @@ static void arm_cpu_initfn(Object *obj)
     CPUState *cs = CPU(obj);
     ARMCPU *cpu = ARM_CPU(obj);
     static bool inited;
-    uint32_t Aff1, Aff0;
 
     cs->env_ptr = &cpu->env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
     cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
                                          g_free, g_free);
 
-    /* This cpu-id-to-MPIDR affinity is used only for TCG; KVM will override it.
-     * We don't support setting cluster ID ([16..23]) (known as Aff2
-     * in later ARM ARM versions), or any of the higher affinity level fields,
-     * so these bits always RAZ.
-     */
-    Aff1 = cs->cpu_index / ARM_CPUS_PER_CLUSTER;
-    Aff0 = cs->cpu_index % ARM_CPUS_PER_CLUSTER;
-    cpu->mp_affinity = (Aff1 << ARM_AFF1_SHIFT) | Aff0;
-
 #ifndef CONFIG_USER_ONLY
     /* Our inbound IRQ and FIQ lines */
     if (kvm_enabled()) {
@@ -577,6 +566,14 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
     ARMCPU *cpu = ARM_CPU(dev);
     ARMCPUClass *acc = ARM_CPU_GET_CLASS(dev);
     CPUARMState *env = &cpu->env;
+    Error *local_err = NULL;
+    uint32_t Aff1, Aff0;
+
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     /* Some features automatically imply others: */
     if (arm_feature(env, ARM_FEATURE_V8)) {
@@ -632,6 +629,15 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         set_feature(env, ARM_FEATURE_THUMB_DSP);
     }
 
+    /* This cpu-id-to-MPIDR affinity is used only for TCG; KVM will override it.
+     * We don't support setting cluster ID ([16..23]) (known as Aff2
+     * in later ARM ARM versions), or any of the higher affinity level fields,
+     * so these bits always RAZ.
+     */
+    Aff1 = cs->cpu_index / ARM_CPUS_PER_CLUSTER;
+    Aff0 = cs->cpu_index % ARM_CPUS_PER_CLUSTER;
+    cpu->mp_affinity = (Aff1 << ARM_AFF1_SHIFT) | Aff0;
+
     if (cpu->reset_hivecs) {
             cpu->reset_sctlr |= (1 << 13);
     }
@@ -1534,17 +1540,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
     cc->debug_check_watchpoint = arm_debug_check_watchpoint;
 
     cc->disas_set_info = arm_disas_set_info;
-
-    /*
-     * Reason: arm_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     *
-     * Once this is fixed, the devices that create ARM CPUs should be
-     * updated not to set cannot_destroy_with_object_finalize_yet,
-     * unless they still screw up something else.
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static void cpu_register(const ARMCPUInfo *info)
-- 
2.7.4

[Qemu-devel] [PATCH v2 06/20] target-alpha: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "pca56-alpha-cpu")

CC: Richard Henderson <rth@twiddle.net>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-alpha/cpu.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
index 98761d7..e0d21fe 100644
--- a/target-alpha/cpu.c
+++ b/target-alpha/cpu.c
@@ -59,6 +59,13 @@ static void alpha_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
     AlphaCPUClass *acc = ALPHA_CPU_GET_CLASS(dev);
+    Error *local_err = NULL;
+
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     qemu_init_vcpu(cs);
 
@@ -267,7 +274,6 @@ static void alpha_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
     tlb_flush(cs, 1);
 
     alpha_translate_init();
@@ -310,13 +316,6 @@ static void alpha_cpu_class_init(ObjectClass *oc, void *data)
     cc->disas_set_info = alpha_cpu_disas_set_info;
 
     cc->gdb_num_core_regs = 67;
-
-    /*
-     * Reason: alpha_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static const TypeInfo alpha_cpu_type_info = {
-- 
2.7.4

[Qemu-devel] [PATCH v2 07/20] target-cris: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "crisv17-cris-cpu")

CC: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-cris/cpu.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/target-cris/cpu.c b/target-cris/cpu.c
index e28abc1..d50ead5 100644
--- a/target-cris/cpu.c
+++ b/target-cris/cpu.c
@@ -142,6 +142,13 @@ static void cris_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
     CRISCPUClass *ccc = CRIS_CPU_GET_CLASS(dev);
+    Error *local_err = NULL;
+
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     cpu_reset(cs);
     qemu_init_vcpu(cs);
@@ -188,7 +195,6 @@ static void cris_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
 
     env->pregs[PR_VR] = ccc->vr;
 
@@ -327,13 +333,6 @@ static void cris_cpu_class_init(ObjectClass *oc, void *data)
     cc->gdb_stop_before_watchpoint = true;
 
     cc->disas_set_info = cris_disas_set_info;
-
-    /*
-     * Reason: cris_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static const TypeInfo cris_cpu_type_info = {
-- 
2.7.4

[Qemu-devel] [PATCH v2 08/20] target-lm32: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "lm32-full-lm32-cpu")

CC: Michael Walle <michael@walle.cc>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-lm32/cpu.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
index 147cc60..e021236 100644
--- a/target-lm32/cpu.c
+++ b/target-lm32/cpu.c
@@ -144,6 +144,13 @@ static void lm32_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
     LM32CPUClass *lcc = LM32_CPU_GET_CLASS(dev);
+    Error *local_err = NULL;
+
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     cpu_reset(cs);
 
@@ -161,7 +168,6 @@ static void lm32_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
 
     env->flags = 0;
 
@@ -286,13 +292,6 @@ static void lm32_cpu_class_init(ObjectClass *oc, void *data)
     cc->gdb_stop_before_watchpoint = true;
     cc->debug_excp_handler = lm32_debug_excp_handler;
     cc->disas_set_info = lm32_cpu_disas_set_info;
-
-    /*
-     * Reason: lm32_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static void lm32_register_cpu_type(const LM32CPUInfo *info)
-- 
2.7.4

[Qemu-devel] [PATCH v2 09/20] target-m68k: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "any-m68k-cpu")

CC: Greg Ungerer <gerg@uclinux.org>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-m68k/cpu.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c
index 2768bbb..f60a7a5 100644
--- a/target-m68k/cpu.c
+++ b/target-m68k/cpu.c
@@ -159,6 +159,13 @@ static void m68k_cpu_realizefn(DeviceState *dev, Error **errp)
     CPUState *cs = CPU(dev);
     M68kCPU *cpu = M68K_CPU(dev);
     M68kCPUClass *mcc = M68K_CPU_GET_CLASS(dev);
+    Error *local_err = NULL;
+
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     m68k_cpu_init_gdb(cpu);
 
@@ -177,7 +184,6 @@ static void m68k_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled() && !inited) {
         inited = true;
@@ -223,13 +229,6 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data)
     cc->gdb_core_xml_file = "cf-core.xml";
 
     dc->vmsd = &vmstate_m68k_cpu;
-
-    /*
-     * Reason: m68k_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static void register_cpu_type(const M68kCPUInfo *info)
-- 
2.7.4

[Qemu-devel] [PATCH v2 10/20] target-microblaze: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "microblaze-cpu")

CC: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-microblaze/cpu.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
index 71fd5fc..7a662b9 100644
--- a/target-microblaze/cpu.c
+++ b/target-microblaze/cpu.c
@@ -138,6 +138,13 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
     CPUMBState *env = &cpu->env;
     uint8_t version_code = 0;
     int i = 0;
+    Error *local_err = NULL;
+
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     qemu_init_vcpu(cs);
 
@@ -200,7 +207,6 @@ static void mb_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
 
     set_float_rounding_mode(float_round_nearest_even, &env->fp_status);
 
@@ -268,12 +274,6 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
     cc->gdb_num_core_regs = 32 + 5;
 
     cc->disas_set_info = mb_disas_set_info;
-
-    /*
-     * Reason: mb_cpu_initfn() calls cpu_exec_init(), which saves the
-     * object in cpus -> dangling pointer after final object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static const TypeInfo mb_cpu_type_info = {
-- 
2.7.4

[Qemu-devel] [PATCH v2 11/20] target-mips: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "mips-cpu")

CC: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-mips/cpu.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/target-mips/cpu.c b/target-mips/cpu.c
index 1154d11..d16685e 100644
--- a/target-mips/cpu.c
+++ b/target-mips/cpu.c
@@ -124,6 +124,13 @@ static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
     MIPSCPUClass *mcc = MIPS_CPU_GET_CLASS(dev);
+    Error *local_err = NULL;
+
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     cpu_reset(cs);
     qemu_init_vcpu(cs);
@@ -139,7 +146,6 @@ static void mips_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled()) {
         mips_tcg_init();
@@ -178,13 +184,6 @@ static void mips_cpu_class_init(ObjectClass *c, void *data)
 
     cc->gdb_num_core_regs = 73;
     cc->gdb_stop_before_watchpoint = true;
-
-    /*
-     * Reason: mips_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static const TypeInfo mips_cpu_type_info = {
-- 
2.7.4

[Qemu-devel] [PATCH v2 12/20] target-moxie: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "moxie-cpu")

CC: Anthony Green <green@moxielogic.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-moxie/cpu.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/target-moxie/cpu.c b/target-moxie/cpu.c
index c9eed19..a4003aa 100644
--- a/target-moxie/cpu.c
+++ b/target-moxie/cpu.c
@@ -61,6 +61,13 @@ static void moxie_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
     MoxieCPUClass *mcc = MOXIE_CPU_GET_CLASS(dev);
+    Error *local_err = NULL;
+
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     qemu_init_vcpu(cs);
     cpu_reset(cs);
@@ -76,7 +83,6 @@ static void moxie_cpu_initfn(Object *obj)
 
     cs->env_ptr = &cpu->env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled() && !inited) {
         inited = 1;
@@ -125,13 +131,6 @@ static void moxie_cpu_class_init(ObjectClass *oc, void *data)
     cc->vmsd = &vmstate_moxie_cpu;
 #endif
     cc->disas_set_info = moxie_cpu_disas_set_info;
-
-    /*
-     * Reason: moxie_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static void moxielite_initfn(Object *obj)
-- 
2.7.4

[Qemu-devel] [PATCH v2 13/20] target-openrisc: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "or1200-or32-cpu")

CC: Jia Liu <proljc@gmail.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-openrisc/cpu.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
index 74d52bf..710ff5e 100644
--- a/target-openrisc/cpu.c
+++ b/target-openrisc/cpu.c
@@ -81,6 +81,13 @@ static void openrisc_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
     OpenRISCCPUClass *occ = OPENRISC_CPU_GET_CLASS(dev);
+    Error *local_err = NULL;
+
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     qemu_init_vcpu(cs);
     cpu_reset(cs);
@@ -96,7 +103,6 @@ static void openrisc_cpu_initfn(Object *obj)
 
     cs->env_ptr = &cpu->env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
 
 #ifndef CONFIG_USER_ONLY
     cpu_openrisc_mmu_init(cpu);
@@ -181,13 +187,6 @@ static void openrisc_cpu_class_init(ObjectClass *oc, void *data)
     dc->vmsd = &vmstate_openrisc_cpu;
 #endif
     cc->gdb_num_core_regs = 32 + 3;
-
-    /*
-     * Reason: openrisc_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static void cpu_register(const OpenRISCCPUInfo *info)
-- 
2.7.4

[Qemu-devel] [PATCH v2 14/20] target-sh4: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "sh7750r-superh-cpu")

CC: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-sh4/cpu.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
index 60689c7..e31233e 100644
--- a/target-sh4/cpu.c
+++ b/target-sh4/cpu.c
@@ -244,6 +244,13 @@ static void superh_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
     SuperHCPUClass *scc = SUPERH_CPU_GET_CLASS(dev);
+    Error *local_err = NULL;
+
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     cpu_reset(cs);
     qemu_init_vcpu(cs);
@@ -259,7 +266,6 @@ static void superh_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
 
     env->movcal_backup_tail = &(env->movcal_backup);
 
@@ -304,13 +310,6 @@ static void superh_cpu_class_init(ObjectClass *oc, void *data)
     cc->gdb_num_core_regs = 59;
 
     dc->vmsd = &vmstate_sh_cpu;
-
-    /*
-     * Reason: superh_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static const TypeInfo superh_cpu_type_info = {
-- 
2.7.4

[Qemu-devel] [PATCH v2 15/20] target-sparc: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "sparc-cpu")

CC: Artyom Tarasenko <atar4qemu@gmail.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-sparc/cpu.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
index 135f30c..d60cc65 100644
--- a/target-sparc/cpu.c
+++ b/target-sparc/cpu.c
@@ -792,7 +792,9 @@ static bool sparc_cpu_has_work(CPUState *cs)
 
 static void sparc_cpu_realizefn(DeviceState *dev, Error **errp)
 {
+    CPUState *cs = CPU(dev);
     SPARCCPUClass *scc = SPARC_CPU_GET_CLASS(dev);
+    Error *local_err = NULL;
 #if defined(CONFIG_USER_ONLY)
     SPARCCPU *cpu = SPARC_CPU(dev);
     CPUSPARCState *env = &cpu->env;
@@ -802,7 +804,13 @@ static void sparc_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 #endif
 
-    qemu_init_vcpu(CPU(dev));
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    qemu_init_vcpu(cs);
 
     scc->parent_realize(dev, errp);
 }
@@ -815,7 +823,6 @@ static void sparc_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled()) {
         gen_intermediate_code_init(env);
@@ -868,13 +875,6 @@ static void sparc_cpu_class_init(ObjectClass *oc, void *data)
 #else
     cc->gdb_num_core_regs = 72;
 #endif
-
-    /*
-     * Reason: sparc_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static const TypeInfo sparc_cpu_type_info = {
-- 
2.7.4

[Qemu-devel] [PATCH v2 16/20] target-tilegx: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(not tested with QOM command as tilegx is only available in linux-user
 mode)

CC: Chen Gang <gang.chen.5i5j@gmail.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-tilegx/cpu.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/target-tilegx/cpu.c b/target-tilegx/cpu.c
index ae6ea6e..d00c86c 100644
--- a/target-tilegx/cpu.c
+++ b/target-tilegx/cpu.c
@@ -92,6 +92,13 @@ static void tilegx_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
     TileGXCPUClass *tcc = TILEGX_CPU_GET_CLASS(dev);
+    Error *local_err = NULL;
+
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     cpu_reset(cs);
     qemu_init_vcpu(cs);
@@ -108,7 +115,6 @@ static void tilegx_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled() && !tcg_initialized) {
         tcg_initialized = true;
@@ -163,13 +169,6 @@ static void tilegx_cpu_class_init(ObjectClass *oc, void *data)
     cc->set_pc = tilegx_cpu_set_pc;
     cc->handle_mmu_fault = tilegx_cpu_handle_mmu_fault;
     cc->gdb_num_core_regs = 0;
-
-    /*
-     * Reason: tilegx_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static const TypeInfo tilegx_cpu_type_info = {
-- 
2.7.4

[Qemu-devel] [PATCH v2 17/20] target-tricore: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "tc1796-tricore-cpu")

CC: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-tricore/cpu.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/target-tricore/cpu.c b/target-tricore/cpu.c
index 43f444f..8eb8aae 100644
--- a/target-tricore/cpu.c
+++ b/target-tricore/cpu.c
@@ -69,6 +69,13 @@ static void tricore_cpu_realizefn(DeviceState *dev, Error **errp)
     TriCoreCPU *cpu = TRICORE_CPU(dev);
     TriCoreCPUClass *tcc = TRICORE_CPU_GET_CLASS(dev);
     CPUTriCoreState *env = &cpu->env;
+    Error *local_err = NULL;
+
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     /* Some features automatically imply others */
     if (tricore_feature(env, TRICORE_FEATURE_161)) {
@@ -96,7 +103,6 @@ static void tricore_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled()) {
         tricore_tcg_init();
@@ -173,13 +179,6 @@ static void tricore_cpu_class_init(ObjectClass *c, void *data)
     cc->dump_state = tricore_cpu_dump_state;
     cc->set_pc = tricore_cpu_set_pc;
     cc->synchronize_from_tb = tricore_cpu_synchronize_from_tb;
-
-    /*
-     * Reason: tricore_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static void cpu_register(const TriCoreCPUInfo *info)
-- 
2.7.4

[Qemu-devel] [PATCH v2 18/20] target-unicore32: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "any-unicore32-cpu")

CC: Guan Xuetao <gxt@mprc.pku.edu.cn>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-unicore32/cpu.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
index ffc01bd..14fef43 100644
--- a/target-unicore32/cpu.c
+++ b/target-unicore32/cpu.c
@@ -101,9 +101,17 @@ static const UniCore32CPUInfo uc32_cpus[] = {
 
 static void uc32_cpu_realizefn(DeviceState *dev, Error **errp)
 {
+    CPUState *cs = CPU(dev);
     UniCore32CPUClass *ucc = UNICORE32_CPU_GET_CLASS(dev);
+    Error *local_err = NULL;
 
-    qemu_init_vcpu(CPU(dev));
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    qemu_init_vcpu(cs);
 
     ucc->parent_realize(dev, errp);
 }
@@ -117,7 +125,6 @@ static void uc32_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
 
 #ifdef CONFIG_USER_ONLY
     env->uncached_asr = ASR_MODE_USER;
@@ -161,13 +168,6 @@ static void uc32_cpu_class_init(ObjectClass *oc, void *data)
     cc->get_phys_page_debug = uc32_cpu_get_phys_page_debug;
 #endif
     dc->vmsd = &vmstate_uc32_cpu;
-
-    /*
-     * Reason: uc32_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static void uc32_register_cpu_type(const UniCore32CPUInfo *info)
-- 
2.7.4

[Qemu-devel] [PATCH v2 19/20] target-xtensa: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
unsafe references have been moved to cpu_exec_realize().
(tested with QOM command provided by commit 4c315c27 with
 "dc233c-xtensa-cpu")

CC: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-xtensa/cpu.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c
index 2719b73..85208b3 100644
--- a/target-xtensa/cpu.c
+++ b/target-xtensa/cpu.c
@@ -99,6 +99,13 @@ static void xtensa_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
     XtensaCPUClass *xcc = XTENSA_CPU_GET_CLASS(dev);
+    Error *local_err = NULL;
+
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     cs->gdb_num_regs = xcc->config->gdb_regmap.num_regs;
 
@@ -118,7 +125,6 @@ static void xtensa_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     env->config = xcc->config;
     cpu_exec_init(cs);
-    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled() && !tcg_inited) {
         tcg_inited = true;
@@ -159,13 +165,6 @@ static void xtensa_cpu_class_init(ObjectClass *oc, void *data)
 #endif
     cc->debug_excp_handler = xtensa_breakpoint_handler;
     dc->vmsd = &vmstate_xtensa_cpu;
-
-    /*
-     * Reason: xtensa_cpu_initfn() calls cpu_exec_init(), which saves
-     * the object in cpus -> dangling pointer after final
-     * object_unref().
-     */
-    dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static const TypeInfo xtensa_cpu_type_info = {
-- 
2.7.4

[Qemu-devel] [PATCH v2 20/20] exec: move cpu_exec_init() to cpu_common_initfn()

[Laurent Vivier]

As all XX_cpu_initfn() call cpu_exec_init(), we
can move it to cpu_common_initfn().

CC: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 include/exec/exec-all.h     | 1 -
 include/qom/cpu.h           | 1 +
 qom/cpu.c                   | 2 ++
 target-alpha/cpu.c          | 1 -
 target-arm/cpu.c            | 1 -
 target-cris/cpu.c           | 1 -
 target-i386/cpu.c           | 1 -
 target-lm32/cpu.c           | 1 -
 target-m68k/cpu.c           | 1 -
 target-microblaze/cpu.c     | 1 -
 target-mips/cpu.c           | 1 -
 target-moxie/cpu.c          | 1 -
 target-openrisc/cpu.c       | 1 -
 target-ppc/translate_init.c | 1 -
 target-s390x/cpu.c          | 1 -
 target-sh4/cpu.c            | 1 -
 target-sparc/cpu.c          | 1 -
 target-tilegx/cpu.c         | 1 -
 target-tricore/cpu.c        | 1 -
 target-unicore32/cpu.c      | 1 -
 target-xtensa/cpu.c         | 1 -
 21 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index b42533e..c7a3b65 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -57,7 +57,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
                               uint32_t flags,
                               int cflags);
 
-void cpu_exec_init(CPUState *cpu);
 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);
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 4962980..f45b967 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -946,6 +946,7 @@ 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_init(CPUState *cpu);
 void cpu_exec_unrealize(CPUState *cpu);
 
 #ifdef CONFIG_SOFTMMU
diff --git a/qom/cpu.c b/qom/cpu.c
index 39590e1..26989a2 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -362,6 +362,8 @@ static void cpu_common_initfn(Object *obj)
     QTAILQ_INIT(&cpu->watchpoints);
 
     cpu->trace_dstate = bitmap_new(trace_get_vcpu_event_count());
+
+    cpu_exec_init(cpu);
 }
 
 static void cpu_common_finalize(Object *obj)
diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
index e0d21fe..995bf79 100644
--- a/target-alpha/cpu.c
+++ b/target-alpha/cpu.c
@@ -273,7 +273,6 @@ static void alpha_cpu_initfn(Object *obj)
     CPUAlphaState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs);
     tlb_flush(cs, 1);
 
     alpha_translate_init();
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 54e7991..d521a74 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -443,7 +443,6 @@ static void arm_cpu_initfn(Object *obj)
     static bool inited;
 
     cs->env_ptr = &cpu->env;
-    cpu_exec_init(cs);
     cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
                                          g_free, g_free);
 
diff --git a/target-cris/cpu.c b/target-cris/cpu.c
index d50ead5..034666f 100644
--- a/target-cris/cpu.c
+++ b/target-cris/cpu.c
@@ -194,7 +194,6 @@ static void cris_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs);
 
     env->pregs[PR_VR] = ccc->vr;
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4741dd6..c3b192d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3368,7 +3368,6 @@ static void x86_cpu_initfn(Object *obj)
     FeatureWord w;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs);
 
     object_property_add(obj, "family", "int",
                         x86_cpuid_version_get_family,
diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
index e021236..273d881 100644
--- a/target-lm32/cpu.c
+++ b/target-lm32/cpu.c
@@ -167,7 +167,6 @@ static void lm32_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs);
 
     env->flags = 0;
 
diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c
index f60a7a5..5012c26 100644
--- a/target-m68k/cpu.c
+++ b/target-m68k/cpu.c
@@ -183,7 +183,6 @@ static void m68k_cpu_initfn(Object *obj)
     static bool inited;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs);
 
     if (tcg_enabled() && !inited) {
         inited = true;
diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
index 7a662b9..a4fabe4 100644
--- a/target-microblaze/cpu.c
+++ b/target-microblaze/cpu.c
@@ -206,7 +206,6 @@ static void mb_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs);
 
     set_float_rounding_mode(float_round_nearest_even, &env->fp_status);
 
diff --git a/target-mips/cpu.c b/target-mips/cpu.c
index d16685e..ef59c66 100644
--- a/target-mips/cpu.c
+++ b/target-mips/cpu.c
@@ -145,7 +145,6 @@ static void mips_cpu_initfn(Object *obj)
     CPUMIPSState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs);
 
     if (tcg_enabled()) {
         mips_tcg_init();
diff --git a/target-moxie/cpu.c b/target-moxie/cpu.c
index a4003aa..d5015ae 100644
--- a/target-moxie/cpu.c
+++ b/target-moxie/cpu.c
@@ -82,7 +82,6 @@ static void moxie_cpu_initfn(Object *obj)
     static int inited;
 
     cs->env_ptr = &cpu->env;
-    cpu_exec_init(cs);
 
     if (tcg_enabled() && !inited) {
         inited = 1;
diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
index 710ff5e..f0a5ca2 100644
--- a/target-openrisc/cpu.c
+++ b/target-openrisc/cpu.c
@@ -102,7 +102,6 @@ static void openrisc_cpu_initfn(Object *obj)
     static int inited;
 
     cs->env_ptr = &cpu->env;
-    cpu_exec_init(cs);
 
 #ifndef CONFIG_USER_ONLY
     cpu_openrisc_mmu_init(cpu);
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index bbca8b5..1cee88d 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -10432,7 +10432,6 @@ static void ppc_cpu_initfn(Object *obj)
     CPUPPCState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs);
 
     env->msr_mask = pcc->msr_mask;
     env->mmu_model = pcc->mmu_model;
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index f92adad..d405947 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -289,7 +289,6 @@ static void s390_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     cs->halted = 1;
     cs->exception_index = EXCP_HLT;
-    cpu_exec_init(cs);
     object_property_add(OBJECT(cpu), "id", "int64_t", s390x_cpu_get_id,
                         s390x_cpu_set_id, NULL, NULL, NULL);
     s390_cpu_model_register_props(obj);
diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
index e31233e..73d889a 100644
--- a/target-sh4/cpu.c
+++ b/target-sh4/cpu.c
@@ -265,7 +265,6 @@ static void superh_cpu_initfn(Object *obj)
     CPUSH4State *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs);
 
     env->movcal_backup_tail = &(env->movcal_backup);
 
diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
index d60cc65..cf27949 100644
--- a/target-sparc/cpu.c
+++ b/target-sparc/cpu.c
@@ -822,7 +822,6 @@ static void sparc_cpu_initfn(Object *obj)
     CPUSPARCState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs);
 
     if (tcg_enabled()) {
         gen_intermediate_code_init(env);
diff --git a/target-tilegx/cpu.c b/target-tilegx/cpu.c
index d00c86c..985a12b 100644
--- a/target-tilegx/cpu.c
+++ b/target-tilegx/cpu.c
@@ -114,7 +114,6 @@ static void tilegx_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs);
 
     if (tcg_enabled() && !tcg_initialized) {
         tcg_initialized = true;
diff --git a/target-tricore/cpu.c b/target-tricore/cpu.c
index 8eb8aae..93594e6 100644
--- a/target-tricore/cpu.c
+++ b/target-tricore/cpu.c
@@ -102,7 +102,6 @@ static void tricore_cpu_initfn(Object *obj)
     CPUTriCoreState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs);
 
     if (tcg_enabled()) {
         tricore_tcg_init();
diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
index 14fef43..181033f 100644
--- a/target-unicore32/cpu.c
+++ b/target-unicore32/cpu.c
@@ -124,7 +124,6 @@ static void uc32_cpu_initfn(Object *obj)
     static bool inited;
 
     cs->env_ptr = env;
-    cpu_exec_init(cs);
 
 #ifdef CONFIG_USER_ONLY
     env->uncached_asr = ASR_MODE_USER;
diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c
index 85208b3..e3099db 100644
--- a/target-xtensa/cpu.c
+++ b/target-xtensa/cpu.c
@@ -124,7 +124,6 @@ static void xtensa_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     env->config = xcc->config;
-    cpu_exec_init(cs);
 
     if (tcg_enabled() && !tcg_inited) {
         tcg_inited = true;
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 01/20] exec: split cpu_exec_init()

[David Gibson]

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

On Thu, Oct 13, 2016 at 06:24:43PM +0200, Laurent Vivier wrote:
> Extract the realize part to cpu_exec_realize(), update all
> calls to cpu_exec_init() to add cpu_exec_realize() to
> have no functionnal change.
> 
> Put in cpu_exec_init() what initializes the CPU,
> in cpu_exec_realize() what adds it to the environment.
> 
> Remove error parameter from cpu_exec_init() as it can't fail.
> 
> Rename cpu_exec_exit() with cpu_exec_unrealize():
> cpu_exec_exit() is undoing what it has been done by cpu_exec_realize(), so
> call it cpu_exec_unrealize().
> 
> CC: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

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

> ---
>  exec.c                      | 12 +++++++-----
>  include/exec/exec-all.h     |  3 ++-
>  include/qom/cpu.h           |  2 +-
>  qom/cpu.c                   |  2 +-
>  target-alpha/cpu.c          |  3 ++-
>  target-arm/cpu.c            |  3 ++-
>  target-cris/cpu.c           |  3 ++-
>  target-i386/cpu.c           |  3 ++-
>  target-lm32/cpu.c           |  3 ++-
>  target-m68k/cpu.c           |  3 ++-
>  target-microblaze/cpu.c     |  3 ++-
>  target-mips/cpu.c           |  3 ++-
>  target-moxie/cpu.c          |  3 ++-
>  target-openrisc/cpu.c       |  3 ++-
>  target-ppc/translate_init.c |  5 +++--
>  target-s390x/cpu.c          |  3 ++-
>  target-sh4/cpu.c            |  3 ++-
>  target-sparc/cpu.c          |  3 ++-
>  target-tilegx/cpu.c         |  3 ++-
>  target-tricore/cpu.c        |  3 ++-
>  target-unicore32/cpu.c      |  3 ++-
>  target-xtensa/cpu.c         |  3 ++-
>  22 files changed, 48 insertions(+), 27 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 374c364..885dc79 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -596,7 +596,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
>  }
>  #endif
>  
> -void cpu_exec_exit(CPUState *cpu)
> +void cpu_exec_unrealize(CPUState *cpu)
>  {
>      CPUClass *cc = CPU_GET_CLASS(cpu);
>  
> @@ -610,11 +610,8 @@ void cpu_exec_exit(CPUState *cpu)
>      }
>  }
>  
> -void cpu_exec_init(CPUState *cpu, Error **errp)
> +void cpu_exec_init(CPUState *cpu)
>  {
> -    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
> -    Error *local_err ATTRIBUTE_UNUSED = NULL;
> -
>      cpu->as = NULL;
>      cpu->num_ases = 0;
>  
> @@ -635,6 +632,11 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
>      cpu->memory = system_memory;
>      object_ref(OBJECT(cpu->memory));
>  #endif
> +}
> +
> +void cpu_exec_realize(CPUState *cpu, Error **errp)
> +{
> +    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
>  
>      cpu_list_add(cpu);
>  
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 336a57c..b42533e 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -57,7 +57,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>                                uint32_t flags,
>                                int cflags);
>  
> -void cpu_exec_init(CPUState *cpu, Error **errp);
> +void cpu_exec_init(CPUState *cpu);
> +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);
>  
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 6d481a1..4962980 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -946,7 +946,7 @@ 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);
> +void cpu_exec_unrealize(CPUState *cpu);
>  
>  #ifdef CONFIG_SOFTMMU
>  extern const struct VMStateDescription vmstate_cpu_common;
> diff --git a/qom/cpu.c b/qom/cpu.c
> index c40f774..39590e1 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -367,7 +367,7 @@ static void cpu_common_initfn(Object *obj)
>  static void cpu_common_finalize(Object *obj)
>  {
>      CPUState *cpu = CPU(obj);
> -    cpu_exec_exit(cpu);
> +    cpu_exec_unrealize(CPU(obj));
>      g_free(cpu->trace_dstate);
>  }
>  
> diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
> index 6d01d7f..98761d7 100644
> --- a/target-alpha/cpu.c
> +++ b/target-alpha/cpu.c
> @@ -266,7 +266,8 @@ static void alpha_cpu_initfn(Object *obj)
>      CPUAlphaState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>      tlb_flush(cs, 1);
>  
>      alpha_translate_init();
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 1b9540e..7e58134 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -444,7 +444,8 @@ static void arm_cpu_initfn(Object *obj)
>      uint32_t Aff1, Aff0;
>  
>      cs->env_ptr = &cpu->env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>      cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
>                                           g_free, g_free);
>  
> diff --git a/target-cris/cpu.c b/target-cris/cpu.c
> index d680cfb..e28abc1 100644
> --- a/target-cris/cpu.c
> +++ b/target-cris/cpu.c
> @@ -187,7 +187,8 @@ static void cris_cpu_initfn(Object *obj)
>      static bool tcg_initialized;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      env->pregs[PR_VR] = ccc->vr;
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 1c57fce..b977130 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -3158,7 +3158,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>              cpu->phys_bits = 32;
>          }
>      }
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled()) {
>          tcg_x86_init();
> diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
> index a783d46..147cc60 100644
> --- a/target-lm32/cpu.c
> +++ b/target-lm32/cpu.c
> @@ -160,7 +160,8 @@ static void lm32_cpu_initfn(Object *obj)
>      static bool tcg_initialized;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      env->flags = 0;
>  
> diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c
> index 116b784..2768bbb 100644
> --- a/target-m68k/cpu.c
> +++ b/target-m68k/cpu.c
> @@ -176,7 +176,8 @@ static void m68k_cpu_initfn(Object *obj)
>      static bool inited;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled() && !inited) {
>          inited = true;
> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
> index 8edc00a..71fd5fc 100644
> --- a/target-microblaze/cpu.c
> +++ b/target-microblaze/cpu.c
> @@ -199,7 +199,8 @@ static void mb_cpu_initfn(Object *obj)
>      static bool tcg_initialized;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      set_float_rounding_mode(float_round_nearest_even, &env->fp_status);
>  
> diff --git a/target-mips/cpu.c b/target-mips/cpu.c
> index 64ad112..1154d11 100644
> --- a/target-mips/cpu.c
> +++ b/target-mips/cpu.c
> @@ -138,7 +138,8 @@ static void mips_cpu_initfn(Object *obj)
>      CPUMIPSState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled()) {
>          mips_tcg_init();
> diff --git a/target-moxie/cpu.c b/target-moxie/cpu.c
> index 50a0899..c9eed19 100644
> --- a/target-moxie/cpu.c
> +++ b/target-moxie/cpu.c
> @@ -75,7 +75,8 @@ static void moxie_cpu_initfn(Object *obj)
>      static int inited;
>  
>      cs->env_ptr = &cpu->env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled() && !inited) {
>          inited = 1;
> diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
> index 155913f..74d52bf 100644
> --- a/target-openrisc/cpu.c
> +++ b/target-openrisc/cpu.c
> @@ -95,7 +95,8 @@ static void openrisc_cpu_initfn(Object *obj)
>      static int inited;
>  
>      cs->env_ptr = &cpu->env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>  #ifndef CONFIG_USER_ONLY
>      cpu_openrisc_mmu_init(cpu);
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index b66b40b..094f28a 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9678,7 +9678,8 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
>      }
>  #endif
>  
> -    cpu_exec_init(cs, &local_err);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &local_err);
>      if (local_err != NULL) {
>          error_propagate(errp, local_err);
>          return;
> @@ -9910,7 +9911,7 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
>      opc_handler_t **table, **table_2;
>      int i, j, k;
>  
> -    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) {
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 35ae2ce..4339da1 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -207,7 +207,8 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
>          goto out;
>      }
>  
> -    cpu_exec_init(cs, &err);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &err);
>      if (err != NULL) {
>          goto out;
>      }
> diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
> index f589532..60689c7 100644
> --- a/target-sh4/cpu.c
> +++ b/target-sh4/cpu.c
> @@ -258,7 +258,8 @@ static void superh_cpu_initfn(Object *obj)
>      CPUSH4State *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      env->movcal_backup_tail = &(env->movcal_backup);
>  
> diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
> index 800a25a..135f30c 100644
> --- a/target-sparc/cpu.c
> +++ b/target-sparc/cpu.c
> @@ -814,7 +814,8 @@ static void sparc_cpu_initfn(Object *obj)
>      CPUSPARCState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled()) {
>          gen_intermediate_code_init(env);
> diff --git a/target-tilegx/cpu.c b/target-tilegx/cpu.c
> index 7017cb6..ae6ea6e 100644
> --- a/target-tilegx/cpu.c
> +++ b/target-tilegx/cpu.c
> @@ -107,7 +107,8 @@ static void tilegx_cpu_initfn(Object *obj)
>      static bool tcg_initialized;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled() && !tcg_initialized) {
>          tcg_initialized = true;
> diff --git a/target-tricore/cpu.c b/target-tricore/cpu.c
> index 35d4ee4..43f444f 100644
> --- a/target-tricore/cpu.c
> +++ b/target-tricore/cpu.c
> @@ -95,7 +95,8 @@ static void tricore_cpu_initfn(Object *obj)
>      CPUTriCoreState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled()) {
>          tricore_tcg_init();
> diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
> index e7a4984..ffc01bd 100644
> --- a/target-unicore32/cpu.c
> +++ b/target-unicore32/cpu.c
> @@ -116,7 +116,8 @@ static void uc32_cpu_initfn(Object *obj)
>      static bool inited;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>  #ifdef CONFIG_USER_ONLY
>      env->uncached_asr = ASR_MODE_USER;
> diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c
> index 5ad08a2..2719b73 100644
> --- a/target-xtensa/cpu.c
> +++ b/target-xtensa/cpu.c
> @@ -117,7 +117,8 @@ static void xtensa_cpu_initfn(Object *obj)
>  
>      cs->env_ptr = env;
>      env->config = xcc->config;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled() && !tcg_inited) {
>          tcg_inited = 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 v2 03/20] target-ppc: move back cpu_exec_init() to init

[David Gibson]

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

On Thu, Oct 13, 2016 at 06:24:45PM +0200, Laurent Vivier wrote:
> We have now the cpu_exec_realize() in realize,
> so the init part must be in init.
> 
> As cpu_exec_unrealize() is called from cpu_common_finalize(),
> remove the call from ppc_cpu_unrealizefn().
> 
> CC: Bharata B Rao <bharata@linux.vnet.ibm.com>
> CC: Alexander Graf <agraf@suse.de>
> CC: qemu-ppc@nongnu.org
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  target-ppc/translate_init.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 094f28a..bbca8b5 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9678,7 +9678,6 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
>      }
>  #endif
>  
> -    cpu_exec_init(cs);
>      cpu_exec_realize(cs, &local_err);
>      if (local_err != NULL) {
>          error_propagate(errp, local_err);
> @@ -9911,8 +9910,6 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
>      opc_handler_t **table, **table_2;
>      int i, j, k;
>  
> -    cpu_exec_unrealize(CPU(dev));
> -

This doesn't seem right.  As you said in 0/20, cpu_exec_unrealize() is
called from cpu_common_finalize().  But finalize should mirror init,
not unrealize().  So it seems that unrealize() really should belong
here, not in finalize.

>      for (i = 0; i < PPC_CPU_OPCODES_LEN; i++) {
>          if (env->opcodes[i] == &invalid_handler) {
>              continue;
> @@ -10435,6 +10432,7 @@ static void ppc_cpu_initfn(Object *obj)
>      CPUPPCState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> +    cpu_exec_init(cs);
>  
>      env->msr_mask = pcc->msr_mask;
>      env->mmu_model = pcc->mmu_model;

-- 
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 v2 03/20] target-ppc: move back cpu_exec_init() to init

[Laurent Vivier]

On 14/10/2016 06:07, David Gibson wrote:
> On Thu, Oct 13, 2016 at 06:24:45PM +0200, Laurent Vivier wrote:
>> We have now the cpu_exec_realize() in realize,
>> so the init part must be in init.
>>
>> As cpu_exec_unrealize() is called from cpu_common_finalize(),
>> remove the call from ppc_cpu_unrealizefn().
>>
>> CC: Bharata B Rao <bharata@linux.vnet.ibm.com>
>> CC: Alexander Graf <agraf@suse.de>
>> CC: qemu-ppc@nongnu.org
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>  target-ppc/translate_init.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 094f28a..bbca8b5 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -9678,7 +9678,6 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
>>      }
>>  #endif
>>  
>> -    cpu_exec_init(cs);
>>      cpu_exec_realize(cs, &local_err);
>>      if (local_err != NULL) {
>>          error_propagate(errp, local_err);
>> @@ -9911,8 +9910,6 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
>>      opc_handler_t **table, **table_2;
>>      int i, j, k;
>>  
>> -    cpu_exec_unrealize(CPU(dev));
>> -
> 
> This doesn't seem right.  As you said in 0/20, cpu_exec_unrealize() is
> called from cpu_common_finalize().  But finalize should mirror init,
> not unrealize().  So it seems that unrealize() really should belong
> here, not in finalize.

OK, I was not sure for this part.

So I guess I have to add an cpu_common_unrealize().

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2 01/20] exec: split cpu_exec_init()

[Greg Kurz]

On Thu, 13 Oct 2016 18:24:43 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> Extract the realize part to cpu_exec_realize(), update all
> calls to cpu_exec_init() to add cpu_exec_realize() to
> have no functionnal change.
> 
> Put in cpu_exec_init() what initializes the CPU,
> in cpu_exec_realize() what adds it to the environment.
> 
> Remove error parameter from cpu_exec_init() as it can't fail.
> 
> Rename cpu_exec_exit() with cpu_exec_unrealize():
> cpu_exec_exit() is undoing what it has been done by cpu_exec_realize(), so
> call it cpu_exec_unrealize().
> 
> CC: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---

Just one question: is there a reason that prevents cpu_exec_unrealize() to be
declared in include/exec/exec-all.h next to cpu_exec_realize() ?

Apart from that, FWIW:

Reviewed-by: Greg Kurz <groug@kaod.org>

>  exec.c                      | 12 +++++++-----
>  include/exec/exec-all.h     |  3 ++-
>  include/qom/cpu.h           |  2 +-
>  qom/cpu.c                   |  2 +-
>  target-alpha/cpu.c          |  3 ++-
>  target-arm/cpu.c            |  3 ++-
>  target-cris/cpu.c           |  3 ++-
>  target-i386/cpu.c           |  3 ++-
>  target-lm32/cpu.c           |  3 ++-
>  target-m68k/cpu.c           |  3 ++-
>  target-microblaze/cpu.c     |  3 ++-
>  target-mips/cpu.c           |  3 ++-
>  target-moxie/cpu.c          |  3 ++-
>  target-openrisc/cpu.c       |  3 ++-
>  target-ppc/translate_init.c |  5 +++--
>  target-s390x/cpu.c          |  3 ++-
>  target-sh4/cpu.c            |  3 ++-
>  target-sparc/cpu.c          |  3 ++-
>  target-tilegx/cpu.c         |  3 ++-
>  target-tricore/cpu.c        |  3 ++-
>  target-unicore32/cpu.c      |  3 ++-
>  target-xtensa/cpu.c         |  3 ++-
>  22 files changed, 48 insertions(+), 27 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 374c364..885dc79 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -596,7 +596,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
>  }
>  #endif
>  
> -void cpu_exec_exit(CPUState *cpu)
> +void cpu_exec_unrealize(CPUState *cpu)
>  {
>      CPUClass *cc = CPU_GET_CLASS(cpu);
>  
> @@ -610,11 +610,8 @@ void cpu_exec_exit(CPUState *cpu)
>      }
>  }
>  
> -void cpu_exec_init(CPUState *cpu, Error **errp)
> +void cpu_exec_init(CPUState *cpu)
>  {
> -    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
> -    Error *local_err ATTRIBUTE_UNUSED = NULL;
> -
>      cpu->as = NULL;
>      cpu->num_ases = 0;
>  
> @@ -635,6 +632,11 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
>      cpu->memory = system_memory;
>      object_ref(OBJECT(cpu->memory));
>  #endif
> +}
> +
> +void cpu_exec_realize(CPUState *cpu, Error **errp)
> +{
> +    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
>  
>      cpu_list_add(cpu);
>  
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 336a57c..b42533e 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -57,7 +57,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>                                uint32_t flags,
>                                int cflags);
>  
> -void cpu_exec_init(CPUState *cpu, Error **errp);
> +void cpu_exec_init(CPUState *cpu);
> +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);
>  
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 6d481a1..4962980 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -946,7 +946,7 @@ 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);
> +void cpu_exec_unrealize(CPUState *cpu);
>  
>  #ifdef CONFIG_SOFTMMU
>  extern const struct VMStateDescription vmstate_cpu_common;
> diff --git a/qom/cpu.c b/qom/cpu.c
> index c40f774..39590e1 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -367,7 +367,7 @@ static void cpu_common_initfn(Object *obj)
>  static void cpu_common_finalize(Object *obj)
>  {
>      CPUState *cpu = CPU(obj);
> -    cpu_exec_exit(cpu);
> +    cpu_exec_unrealize(CPU(obj));
>      g_free(cpu->trace_dstate);
>  }
>  
> diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
> index 6d01d7f..98761d7 100644
> --- a/target-alpha/cpu.c
> +++ b/target-alpha/cpu.c
> @@ -266,7 +266,8 @@ static void alpha_cpu_initfn(Object *obj)
>      CPUAlphaState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>      tlb_flush(cs, 1);
>  
>      alpha_translate_init();
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 1b9540e..7e58134 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -444,7 +444,8 @@ static void arm_cpu_initfn(Object *obj)
>      uint32_t Aff1, Aff0;
>  
>      cs->env_ptr = &cpu->env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>      cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
>                                           g_free, g_free);
>  
> diff --git a/target-cris/cpu.c b/target-cris/cpu.c
> index d680cfb..e28abc1 100644
> --- a/target-cris/cpu.c
> +++ b/target-cris/cpu.c
> @@ -187,7 +187,8 @@ static void cris_cpu_initfn(Object *obj)
>      static bool tcg_initialized;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      env->pregs[PR_VR] = ccc->vr;
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 1c57fce..b977130 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -3158,7 +3158,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>              cpu->phys_bits = 32;
>          }
>      }
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled()) {
>          tcg_x86_init();
> diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
> index a783d46..147cc60 100644
> --- a/target-lm32/cpu.c
> +++ b/target-lm32/cpu.c
> @@ -160,7 +160,8 @@ static void lm32_cpu_initfn(Object *obj)
>      static bool tcg_initialized;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      env->flags = 0;
>  
> diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c
> index 116b784..2768bbb 100644
> --- a/target-m68k/cpu.c
> +++ b/target-m68k/cpu.c
> @@ -176,7 +176,8 @@ static void m68k_cpu_initfn(Object *obj)
>      static bool inited;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled() && !inited) {
>          inited = true;
> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
> index 8edc00a..71fd5fc 100644
> --- a/target-microblaze/cpu.c
> +++ b/target-microblaze/cpu.c
> @@ -199,7 +199,8 @@ static void mb_cpu_initfn(Object *obj)
>      static bool tcg_initialized;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      set_float_rounding_mode(float_round_nearest_even, &env->fp_status);
>  
> diff --git a/target-mips/cpu.c b/target-mips/cpu.c
> index 64ad112..1154d11 100644
> --- a/target-mips/cpu.c
> +++ b/target-mips/cpu.c
> @@ -138,7 +138,8 @@ static void mips_cpu_initfn(Object *obj)
>      CPUMIPSState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled()) {
>          mips_tcg_init();
> diff --git a/target-moxie/cpu.c b/target-moxie/cpu.c
> index 50a0899..c9eed19 100644
> --- a/target-moxie/cpu.c
> +++ b/target-moxie/cpu.c
> @@ -75,7 +75,8 @@ static void moxie_cpu_initfn(Object *obj)
>      static int inited;
>  
>      cs->env_ptr = &cpu->env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled() && !inited) {
>          inited = 1;
> diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
> index 155913f..74d52bf 100644
> --- a/target-openrisc/cpu.c
> +++ b/target-openrisc/cpu.c
> @@ -95,7 +95,8 @@ static void openrisc_cpu_initfn(Object *obj)
>      static int inited;
>  
>      cs->env_ptr = &cpu->env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>  #ifndef CONFIG_USER_ONLY
>      cpu_openrisc_mmu_init(cpu);
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index b66b40b..094f28a 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9678,7 +9678,8 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
>      }
>  #endif
>  
> -    cpu_exec_init(cs, &local_err);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &local_err);
>      if (local_err != NULL) {
>          error_propagate(errp, local_err);
>          return;
> @@ -9910,7 +9911,7 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
>      opc_handler_t **table, **table_2;
>      int i, j, k;
>  
> -    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) {
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 35ae2ce..4339da1 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -207,7 +207,8 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
>          goto out;
>      }
>  
> -    cpu_exec_init(cs, &err);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &err);
>      if (err != NULL) {
>          goto out;
>      }
> diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
> index f589532..60689c7 100644
> --- a/target-sh4/cpu.c
> +++ b/target-sh4/cpu.c
> @@ -258,7 +258,8 @@ static void superh_cpu_initfn(Object *obj)
>      CPUSH4State *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      env->movcal_backup_tail = &(env->movcal_backup);
>  
> diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
> index 800a25a..135f30c 100644
> --- a/target-sparc/cpu.c
> +++ b/target-sparc/cpu.c
> @@ -814,7 +814,8 @@ static void sparc_cpu_initfn(Object *obj)
>      CPUSPARCState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled()) {
>          gen_intermediate_code_init(env);
> diff --git a/target-tilegx/cpu.c b/target-tilegx/cpu.c
> index 7017cb6..ae6ea6e 100644
> --- a/target-tilegx/cpu.c
> +++ b/target-tilegx/cpu.c
> @@ -107,7 +107,8 @@ static void tilegx_cpu_initfn(Object *obj)
>      static bool tcg_initialized;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled() && !tcg_initialized) {
>          tcg_initialized = true;
> diff --git a/target-tricore/cpu.c b/target-tricore/cpu.c
> index 35d4ee4..43f444f 100644
> --- a/target-tricore/cpu.c
> +++ b/target-tricore/cpu.c
> @@ -95,7 +95,8 @@ static void tricore_cpu_initfn(Object *obj)
>      CPUTriCoreState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled()) {
>          tricore_tcg_init();
> diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
> index e7a4984..ffc01bd 100644
> --- a/target-unicore32/cpu.c
> +++ b/target-unicore32/cpu.c
> @@ -116,7 +116,8 @@ static void uc32_cpu_initfn(Object *obj)
>      static bool inited;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>  #ifdef CONFIG_USER_ONLY
>      env->uncached_asr = ASR_MODE_USER;
> diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c
> index 5ad08a2..2719b73 100644
> --- a/target-xtensa/cpu.c
> +++ b/target-xtensa/cpu.c
> @@ -117,7 +117,8 @@ static void xtensa_cpu_initfn(Object *obj)
>  
>      cs->env_ptr = env;
>      env->config = xcc->config;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled() && !tcg_inited) {
>          tcg_inited = true;

Re: [Qemu-devel] [PATCH v2 01/20] exec: split cpu_exec_init()

[Laurent Vivier]

On 14/10/2016 09:56, Greg Kurz wrote:
> On Thu, 13 Oct 2016 18:24:43 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
>> Extract the realize part to cpu_exec_realize(), update all
>> calls to cpu_exec_init() to add cpu_exec_realize() to
>> have no functionnal change.
>>
>> Put in cpu_exec_init() what initializes the CPU,
>> in cpu_exec_realize() what adds it to the environment.
>>
>> Remove error parameter from cpu_exec_init() as it can't fail.
>>
>> Rename cpu_exec_exit() with cpu_exec_unrealize():
>> cpu_exec_exit() is undoing what it has been done by cpu_exec_realize(), so
>> call it cpu_exec_unrealize().
>>
>> CC: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
> 
> Just one question: is there a reason that prevents cpu_exec_unrealize() to be
> declared in include/exec/exec-all.h next to cpu_exec_realize() ?

because qom/cpu.c doesn't include exec-all.h (and we can't as exec-all.h
is target specific and qom/cpu.c is common code).

Laurent

Re: [Qemu-devel] [PATCH v2 02/20] target-i386: move back cpu_exec_init() to init

[Eduardo Habkost]

On Thu, Oct 13, 2016 at 06:24:44PM +0200, Laurent Vivier wrote:
> We have now the cpu_exec_realize() in realize,
> so the init part must be in init.
> 
> I've removed the cannot_destroy_with_object_finalize_yet field as
> unsafe references have been moved to cpu_exec_realize().
> (tested with QOM command provided by commit 4c315c27 with
> "athlon-x86_64-cpu")
> 
> CC: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Resending the question I asked in my reply to v1:

Instead of creating requiring each subclass to manually call
cpu_exec_init()) on instance_init, why don't we move parts of
cpu_exec_init()/cpu_exec_realize() code to cpu_common_initfn()?
(TYPE_CPU's instance_init)

(And if there's any code that needs to be run after the
subclasses instance_init functions, we can just add a
instance_post_init function to TYPE_CPU).

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 02/20] target-i386: move back cpu_exec_init() to init

[Laurent Vivier]

On 14/10/2016 15:33, Eduardo Habkost wrote:
> On Thu, Oct 13, 2016 at 06:24:44PM +0200, Laurent Vivier wrote:
>> We have now the cpu_exec_realize() in realize,
>> so the init part must be in init.
>>
>> I've removed the cannot_destroy_with_object_finalize_yet field as
>> unsafe references have been moved to cpu_exec_realize().
>> (tested with QOM command provided by commit 4c315c27 with
>> "athlon-x86_64-cpu")
>>
>> CC: Eduardo Habkost <ehabkost@redhat.com>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> 
> Resending the question I asked in my reply to v1:
> 
> Instead of creating requiring each subclass to manually call
> cpu_exec_init()) on instance_init, why don't we move parts of
> cpu_exec_init()/cpu_exec_realize() code to cpu_common_initfn()?
> (TYPE_CPU's instance_init)
> 
> (And if there's any code that needs to be run after the
> subclasses instance_init functions, we can just add a
> instance_post_init function to TYPE_CPU).
> 

It's done in PATCH 20/20.

Is that what you want?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2 02/20] target-i386: move back cpu_exec_init() to init

[Eduardo Habkost]

On Fri, Oct 14, 2016 at 03:34:17PM +0200, Laurent Vivier wrote:
> On 14/10/2016 15:33, Eduardo Habkost wrote:
> > On Thu, Oct 13, 2016 at 06:24:44PM +0200, Laurent Vivier wrote:
> >> We have now the cpu_exec_realize() in realize,
> >> so the init part must be in init.
> >>
> >> I've removed the cannot_destroy_with_object_finalize_yet field as
> >> unsafe references have been moved to cpu_exec_realize().
> >> (tested with QOM command provided by commit 4c315c27 with
> >> "athlon-x86_64-cpu")
> >>
> >> CC: Eduardo Habkost <ehabkost@redhat.com>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > 
> > Resending the question I asked in my reply to v1:
> > 
> > Instead of creating requiring each subclass to manually call
> > cpu_exec_init()) on instance_init, why don't we move parts of
> > cpu_exec_init()/cpu_exec_realize() code to cpu_common_initfn()?
> > (TYPE_CPU's instance_init)
> > 
> > (And if there's any code that needs to be run after the
> > subclasses instance_init functions, we can just add a
> > instance_post_init function to TYPE_CPU).
> > 
> 
> It's done in PATCH 20/20.
> 
> Is that what you want?

Yes (except that I would have inlined the the cpu_exec_init()
code inside cpu_common_init()).

I think I expected this to be done in a single step, that
wouldn't require touching code for all architectures three times.
Something like:

1) Move cpu->as, cpu->num_ases, cpu->thread_id, cpu->memory
   initialization, and "memory" property registration from
   cpu_exec_init() to cpu_common_init() (no architecture code
   touched).
2) (optional) Rename cpu_exec_init() to cpu_exec_realize() (only
   trivial changes in architecture code)

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 01/20] exec: split cpu_exec_init()

[Igor Mammedov]

On Thu, 13 Oct 2016 18:24:43 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

[...]
> Rename cpu_exec_exit() with cpu_exec_unrealize():
> cpu_exec_exit() is undoing what it has been done by cpu_exec_realize(), so
> call it cpu_exec_unrealize().
a separate patch???

[...]

> diff --git a/exec.c b/exec.c
> index 374c364..885dc79 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -596,7 +596,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
>  }
>  #endif
>  
> -void cpu_exec_exit(CPUState *cpu)
> +void cpu_exec_unrealize(CPUState *cpu)
[...]

>  static void cpu_common_finalize(Object *obj)
>  {
>      CPUState *cpu = CPU(obj);
> -    cpu_exec_exit(cpu);
> +    cpu_exec_unrealize(CPU(obj));
if it's unrealize then it should be called at cpu_unrealize() time
and not at _finalize().

We've skipped this change during previous release merge window
because it was too late and would touch all targets and make already
huge hotplug series even bigger.
Now since you are doing all targets sweep/cleanup anyway,
it's good time to move it to unrealize() time.

Re: [Qemu-devel] [PATCH v2 01/20] exec: split cpu_exec_init()

[Laurent Vivier]

On 14/10/2016 15:55, Igor Mammedov wrote:
> On Thu, 13 Oct 2016 18:24:43 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
> [...]
>> Rename cpu_exec_exit() with cpu_exec_unrealize():
>> cpu_exec_exit() is undoing what it has been done by cpu_exec_realize(), so
>> call it cpu_exec_unrealize().
> a separate patch???
> 
> [...]
> 
>> diff --git a/exec.c b/exec.c
>> index 374c364..885dc79 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -596,7 +596,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
>>  }
>>  #endif
>>  
>> -void cpu_exec_exit(CPUState *cpu)
>> +void cpu_exec_unrealize(CPUState *cpu)
> [...]
> 
>>  static void cpu_common_finalize(Object *obj)
>>  {
>>      CPUState *cpu = CPU(obj);
>> -    cpu_exec_exit(cpu);
>> +    cpu_exec_unrealize(CPU(obj));
> if it's unrealize then it should be called at cpu_unrealize() time
> and not at _finalize().
> 
> We've skipped this change during previous release merge window
> because it was too late and would touch all targets and make already
> huge hotplug series even bigger.
> Now since you are doing all targets sweep/cleanup anyway,
> it's good time to move it to unrealize() time.

Yes, v3 will do that.

Laurent

Re: [Qemu-devel] [PATCH v2 01/20] exec: split cpu_exec_init()

[Igor Mammedov]

On Thu, 13 Oct 2016 18:24:43 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> Extract the realize part to cpu_exec_realize(), update all
> calls to cpu_exec_init() to add cpu_exec_realize() to
> have no functionnal change.
> 
> Put in cpu_exec_init() what initializes the CPU,
> in cpu_exec_realize() what adds it to the environment.
now since it's split, why not call cpu_exec_init()
from common cpu_common_initfn() and do
s/cpu_exec_init/cpu_exec_realize/ on individual targets
so that they don't have to call it from their _initfn()s.


> 
> Remove error parameter from cpu_exec_init() as it can't fail.
> 
> Rename cpu_exec_exit() with cpu_exec_unrealize():
> cpu_exec_exit() is undoing what it has been done by cpu_exec_realize(), so
> call it cpu_exec_unrealize().
> 
> CC: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  exec.c                      | 12 +++++++-----
>  include/exec/exec-all.h     |  3 ++-
>  include/qom/cpu.h           |  2 +-
>  qom/cpu.c                   |  2 +-
>  target-alpha/cpu.c          |  3 ++-
>  target-arm/cpu.c            |  3 ++-
>  target-cris/cpu.c           |  3 ++-
>  target-i386/cpu.c           |  3 ++-
>  target-lm32/cpu.c           |  3 ++-
>  target-m68k/cpu.c           |  3 ++-
>  target-microblaze/cpu.c     |  3 ++-
>  target-mips/cpu.c           |  3 ++-
>  target-moxie/cpu.c          |  3 ++-
>  target-openrisc/cpu.c       |  3 ++-
>  target-ppc/translate_init.c |  5 +++--
>  target-s390x/cpu.c          |  3 ++-
>  target-sh4/cpu.c            |  3 ++-
>  target-sparc/cpu.c          |  3 ++-
>  target-tilegx/cpu.c         |  3 ++-
>  target-tricore/cpu.c        |  3 ++-
>  target-unicore32/cpu.c      |  3 ++-
>  target-xtensa/cpu.c         |  3 ++-
>  22 files changed, 48 insertions(+), 27 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 374c364..885dc79 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -596,7 +596,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
>  }
>  #endif
>  
> -void cpu_exec_exit(CPUState *cpu)
> +void cpu_exec_unrealize(CPUState *cpu)
>  {
>      CPUClass *cc = CPU_GET_CLASS(cpu);
>  
> @@ -610,11 +610,8 @@ void cpu_exec_exit(CPUState *cpu)
>      }
>  }
>  
> -void cpu_exec_init(CPUState *cpu, Error **errp)
> +void cpu_exec_init(CPUState *cpu)
>  {
> -    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
> -    Error *local_err ATTRIBUTE_UNUSED = NULL;
> -
>      cpu->as = NULL;
>      cpu->num_ases = 0;
>  
> @@ -635,6 +632,11 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
>      cpu->memory = system_memory;
>      object_ref(OBJECT(cpu->memory));
>  #endif
> +}
> +
> +void cpu_exec_realize(CPUState *cpu, Error **errp)
> +{
> +    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
>  
>      cpu_list_add(cpu);
>  
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 336a57c..b42533e 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -57,7 +57,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>                                uint32_t flags,
>                                int cflags);
>  
> -void cpu_exec_init(CPUState *cpu, Error **errp);
> +void cpu_exec_init(CPUState *cpu);
> +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);
>  
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 6d481a1..4962980 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -946,7 +946,7 @@ 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);
> +void cpu_exec_unrealize(CPUState *cpu);
>  
>  #ifdef CONFIG_SOFTMMU
>  extern const struct VMStateDescription vmstate_cpu_common;
> diff --git a/qom/cpu.c b/qom/cpu.c
> index c40f774..39590e1 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -367,7 +367,7 @@ static void cpu_common_initfn(Object *obj)
>  static void cpu_common_finalize(Object *obj)
>  {
>      CPUState *cpu = CPU(obj);
> -    cpu_exec_exit(cpu);
> +    cpu_exec_unrealize(CPU(obj));
>      g_free(cpu->trace_dstate);
>  }
>  
> diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
> index 6d01d7f..98761d7 100644
> --- a/target-alpha/cpu.c
> +++ b/target-alpha/cpu.c
> @@ -266,7 +266,8 @@ static void alpha_cpu_initfn(Object *obj)
>      CPUAlphaState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>      tlb_flush(cs, 1);
>  
>      alpha_translate_init();
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 1b9540e..7e58134 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -444,7 +444,8 @@ static void arm_cpu_initfn(Object *obj)
>      uint32_t Aff1, Aff0;
>  
>      cs->env_ptr = &cpu->env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>      cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
>                                           g_free, g_free);
>  
> diff --git a/target-cris/cpu.c b/target-cris/cpu.c
> index d680cfb..e28abc1 100644
> --- a/target-cris/cpu.c
> +++ b/target-cris/cpu.c
> @@ -187,7 +187,8 @@ static void cris_cpu_initfn(Object *obj)
>      static bool tcg_initialized;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      env->pregs[PR_VR] = ccc->vr;
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 1c57fce..b977130 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -3158,7 +3158,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>              cpu->phys_bits = 32;
>          }
>      }
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled()) {
>          tcg_x86_init();
> diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
> index a783d46..147cc60 100644
> --- a/target-lm32/cpu.c
> +++ b/target-lm32/cpu.c
> @@ -160,7 +160,8 @@ static void lm32_cpu_initfn(Object *obj)
>      static bool tcg_initialized;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      env->flags = 0;
>  
> diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c
> index 116b784..2768bbb 100644
> --- a/target-m68k/cpu.c
> +++ b/target-m68k/cpu.c
> @@ -176,7 +176,8 @@ static void m68k_cpu_initfn(Object *obj)
>      static bool inited;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled() && !inited) {
>          inited = true;
> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
> index 8edc00a..71fd5fc 100644
> --- a/target-microblaze/cpu.c
> +++ b/target-microblaze/cpu.c
> @@ -199,7 +199,8 @@ static void mb_cpu_initfn(Object *obj)
>      static bool tcg_initialized;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      set_float_rounding_mode(float_round_nearest_even, &env->fp_status);
>  
> diff --git a/target-mips/cpu.c b/target-mips/cpu.c
> index 64ad112..1154d11 100644
> --- a/target-mips/cpu.c
> +++ b/target-mips/cpu.c
> @@ -138,7 +138,8 @@ static void mips_cpu_initfn(Object *obj)
>      CPUMIPSState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled()) {
>          mips_tcg_init();
> diff --git a/target-moxie/cpu.c b/target-moxie/cpu.c
> index 50a0899..c9eed19 100644
> --- a/target-moxie/cpu.c
> +++ b/target-moxie/cpu.c
> @@ -75,7 +75,8 @@ static void moxie_cpu_initfn(Object *obj)
>      static int inited;
>  
>      cs->env_ptr = &cpu->env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled() && !inited) {
>          inited = 1;
> diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
> index 155913f..74d52bf 100644
> --- a/target-openrisc/cpu.c
> +++ b/target-openrisc/cpu.c
> @@ -95,7 +95,8 @@ static void openrisc_cpu_initfn(Object *obj)
>      static int inited;
>  
>      cs->env_ptr = &cpu->env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>  #ifndef CONFIG_USER_ONLY
>      cpu_openrisc_mmu_init(cpu);
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index b66b40b..094f28a 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9678,7 +9678,8 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
>      }
>  #endif
>  
> -    cpu_exec_init(cs, &local_err);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &local_err);
>      if (local_err != NULL) {
>          error_propagate(errp, local_err);
>          return;
> @@ -9910,7 +9911,7 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
>      opc_handler_t **table, **table_2;
>      int i, j, k;
>  
> -    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) {
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 35ae2ce..4339da1 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -207,7 +207,8 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
>          goto out;
>      }
>  
> -    cpu_exec_init(cs, &err);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &err);
>      if (err != NULL) {
>          goto out;
>      }
> diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
> index f589532..60689c7 100644
> --- a/target-sh4/cpu.c
> +++ b/target-sh4/cpu.c
> @@ -258,7 +258,8 @@ static void superh_cpu_initfn(Object *obj)
>      CPUSH4State *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      env->movcal_backup_tail = &(env->movcal_backup);
>  
> diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
> index 800a25a..135f30c 100644
> --- a/target-sparc/cpu.c
> +++ b/target-sparc/cpu.c
> @@ -814,7 +814,8 @@ static void sparc_cpu_initfn(Object *obj)
>      CPUSPARCState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled()) {
>          gen_intermediate_code_init(env);
> diff --git a/target-tilegx/cpu.c b/target-tilegx/cpu.c
> index 7017cb6..ae6ea6e 100644
> --- a/target-tilegx/cpu.c
> +++ b/target-tilegx/cpu.c
> @@ -107,7 +107,8 @@ static void tilegx_cpu_initfn(Object *obj)
>      static bool tcg_initialized;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled() && !tcg_initialized) {
>          tcg_initialized = true;
> diff --git a/target-tricore/cpu.c b/target-tricore/cpu.c
> index 35d4ee4..43f444f 100644
> --- a/target-tricore/cpu.c
> +++ b/target-tricore/cpu.c
> @@ -95,7 +95,8 @@ static void tricore_cpu_initfn(Object *obj)
>      CPUTriCoreState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled()) {
>          tricore_tcg_init();
> diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
> index e7a4984..ffc01bd 100644
> --- a/target-unicore32/cpu.c
> +++ b/target-unicore32/cpu.c
> @@ -116,7 +116,8 @@ static void uc32_cpu_initfn(Object *obj)
>      static bool inited;
>  
>      cs->env_ptr = env;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>  #ifdef CONFIG_USER_ONLY
>      env->uncached_asr = ASR_MODE_USER;
> diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c
> index 5ad08a2..2719b73 100644
> --- a/target-xtensa/cpu.c
> +++ b/target-xtensa/cpu.c
> @@ -117,7 +117,8 @@ static void xtensa_cpu_initfn(Object *obj)
>  
>      cs->env_ptr = env;
>      env->config = xcc->config;
> -    cpu_exec_init(cs, &error_abort);
> +    cpu_exec_init(cs);
> +    cpu_exec_realize(cs, &error_abort);
>  
>      if (tcg_enabled() && !tcg_inited) {
>          tcg_inited = true;

Re: [Qemu-devel] [PATCH v2 02/20] target-i386: move back cpu_exec_init() to init

[Laurent Vivier]

On 14/10/2016 15:48, Eduardo Habkost wrote:
> On Fri, Oct 14, 2016 at 03:34:17PM +0200, Laurent Vivier wrote:
>> On 14/10/2016 15:33, Eduardo Habkost wrote:
>>> On Thu, Oct 13, 2016 at 06:24:44PM +0200, Laurent Vivier wrote:
>>>> We have now the cpu_exec_realize() in realize,
>>>> so the init part must be in init.
>>>>
>>>> I've removed the cannot_destroy_with_object_finalize_yet field as
>>>> unsafe references have been moved to cpu_exec_realize().
>>>> (tested with QOM command provided by commit 4c315c27 with
>>>> "athlon-x86_64-cpu")
>>>>
>>>> CC: Eduardo Habkost <ehabkost@redhat.com>
>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>
>>> Resending the question I asked in my reply to v1:
>>>
>>> Instead of creating requiring each subclass to manually call
>>> cpu_exec_init()) on instance_init, why don't we move parts of
>>> cpu_exec_init()/cpu_exec_realize() code to cpu_common_initfn()?
>>> (TYPE_CPU's instance_init)
>>>
>>> (And if there's any code that needs to be run after the
>>> subclasses instance_init functions, we can just add a
>>> instance_post_init function to TYPE_CPU).
>>>
>>
>> It's done in PATCH 20/20.
>>
>> Is that what you want?
> 
> Yes (except that I would have inlined the the cpu_exec_init()
> code inside cpu_common_init()).
> 
> I think I expected this to be done in a single step, that
> wouldn't require touching code for all architectures three times.
> Something like:

Well, I've added several steps to help to review and break nothing.

> 1) Move cpu->as, cpu->num_ases, cpu->thread_id, cpu->memory
>    initialization, and "memory" property registration from
>    cpu_exec_init() to cpu_common_init() (no architecture code
>    touched).

system_memory (for the "memory" property) is declared as static in
exec.c, so we can't move it to cpu_common_init().

Laurent

Re: [Qemu-devel] [PATCH v2 01/20] exec: split cpu_exec_init()

[Laurent Vivier]

On 14/10/2016 16:11, Igor Mammedov wrote:
> On Thu, 13 Oct 2016 18:24:43 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
>> Extract the realize part to cpu_exec_realize(), update all
>> calls to cpu_exec_init() to add cpu_exec_realize() to
>> have no functionnal change.
>>
>> Put in cpu_exec_init() what initializes the CPU,
>> in cpu_exec_realize() what adds it to the environment.
> now since it's split, why not call cpu_exec_init()
> from common cpu_common_initfn() and do
> s/cpu_exec_init/cpu_exec_realize/ on individual targets
> so that they don't have to call it from their _initfn()s.

It's done later in the series.

Laurent

Re: [Qemu-devel] [PATCH v2 01/20] exec: split cpu_exec_init()

[Laurent Vivier]

On 14/10/2016 16:13, Laurent Vivier wrote:
> 
> 
> On 14/10/2016 16:11, Igor Mammedov wrote:
>> On Thu, 13 Oct 2016 18:24:43 +0200
>> Laurent Vivier <lvivier@redhat.com> wrote:
>>
>>> Extract the realize part to cpu_exec_realize(), update all
>>> calls to cpu_exec_init() to add cpu_exec_realize() to
>>> have no functionnal change.
>>>
>>> Put in cpu_exec_init() what initializes the CPU,
>>> in cpu_exec_realize() what adds it to the environment.
>> now since it's split, why not call cpu_exec_init()
>> from common cpu_common_initfn() and do
>> s/cpu_exec_init/cpu_exec_realize/ on individual targets
>> so that they don't have to call it from their _initfn()s.
> 
> It's done later in the series.

But I'm going to simplified the series as proposed by you and Eduardo.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2 02/20] target-i386: move back cpu_exec_init() to init

[Eduardo Habkost]

On Fri, Oct 14, 2016 at 04:12:20PM +0200, Laurent Vivier wrote:
> 
> 
> On 14/10/2016 15:48, Eduardo Habkost wrote:
> > On Fri, Oct 14, 2016 at 03:34:17PM +0200, Laurent Vivier wrote:
> >> On 14/10/2016 15:33, Eduardo Habkost wrote:
> >>> On Thu, Oct 13, 2016 at 06:24:44PM +0200, Laurent Vivier wrote:
> >>>> We have now the cpu_exec_realize() in realize,
> >>>> so the init part must be in init.
> >>>>
> >>>> I've removed the cannot_destroy_with_object_finalize_yet field as
> >>>> unsafe references have been moved to cpu_exec_realize().
> >>>> (tested with QOM command provided by commit 4c315c27 with
> >>>> "athlon-x86_64-cpu")
> >>>>
> >>>> CC: Eduardo Habkost <ehabkost@redhat.com>
> >>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >>>
> >>> Resending the question I asked in my reply to v1:
> >>>
> >>> Instead of creating requiring each subclass to manually call
> >>> cpu_exec_init()) on instance_init, why don't we move parts of
> >>> cpu_exec_init()/cpu_exec_realize() code to cpu_common_initfn()?
> >>> (TYPE_CPU's instance_init)
> >>>
> >>> (And if there's any code that needs to be run after the
> >>> subclasses instance_init functions, we can just add a
> >>> instance_post_init function to TYPE_CPU).
> >>>
> >>
> >> It's done in PATCH 20/20.
> >>
> >> Is that what you want?
> > 
> > Yes (except that I would have inlined the the cpu_exec_init()
> > code inside cpu_common_init()).
> > 
> > I think I expected this to be done in a single step, that
> > wouldn't require touching code for all architectures three times.
> > Something like:
> 
> Well, I've added several steps to help to review and break nothing.

To me, it made review harder. Architecture maintainers have to
review 3 different patches that touch their code, instead of a
single one.

Anyway, I would still give my Reviewed-by line for the i386 part,
if you believe it's better to do it this way.

> 
> > 1) Move cpu->as, cpu->num_ases, cpu->thread_id, cpu->memory
> >    initialization, and "memory" property registration from
> >    cpu_exec_init() to cpu_common_init() (no architecture code
> >    touched).
> 
> system_memory (for the "memory" property) is declared as static in
> exec.c, so we can't move it to cpu_common_init().

In this case we can't inline it, that's true. But it still
doesn't require touching each architecture 3 times (we could just
create a cpu_exec_instance_init() function in exec.c and call it
from cpu_common_init()).

(Or we could move system_memory to MachineState, but that could
be done in a follow-up patch).

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 02/20] target-i386: move back cpu_exec_init() to init

[Igor Mammedov]

On Fri, 14 Oct 2016 10:48:58 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Fri, Oct 14, 2016 at 03:34:17PM +0200, Laurent Vivier wrote:
> > On 14/10/2016 15:33, Eduardo Habkost wrote:  
> > > On Thu, Oct 13, 2016 at 06:24:44PM +0200, Laurent Vivier wrote:  
> > >> We have now the cpu_exec_realize() in realize,
> > >> so the init part must be in init.
> > >>
> > >> I've removed the cannot_destroy_with_object_finalize_yet field as
> > >> unsafe references have been moved to cpu_exec_realize().
> > >> (tested with QOM command provided by commit 4c315c27 with
> > >> "athlon-x86_64-cpu")
> > >>
> > >> CC: Eduardo Habkost <ehabkost@redhat.com>
> > >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>  
> > > 
> > > Resending the question I asked in my reply to v1:
> > > 
> > > Instead of creating requiring each subclass to manually call
> > > cpu_exec_init()) on instance_init, why don't we move parts of
> > > cpu_exec_init()/cpu_exec_realize() code to cpu_common_initfn()?
> > > (TYPE_CPU's instance_init)
> > > 
> > > (And if there's any code that needs to be run after the
> > > subclasses instance_init functions, we can just add a
> > > instance_post_init function to TYPE_CPU).
> > >   
> > 
> > It's done in PATCH 20/20.
> > 
> > Is that what you want?  
> 
> Yes (except that I would have inlined the the cpu_exec_init()
> code inside cpu_common_init()).
> 
> I think I expected this to be done in a single step, that
> wouldn't require touching code for all architectures three times.
> Something like:
> 
> 1) Move cpu->as, cpu->num_ases, cpu->thread_id, cpu->memory
>    initialization, and "memory" property registration from
>    cpu_exec_init() to cpu_common_init() (no architecture code
>    touched).
> 2) (optional) Rename cpu_exec_init() to cpu_exec_realize() (only
>    trivial changes in architecture code)
I'd do all of it in 1 step
  - split cpu_exec_init on init/realize parts
    (comment in cpu_exec_init says that qom/cpu.c can't be used for
     "memory" property)
  - call cpu_exec_init() from cpu_common_init()
  - s/cpu_exec_init/cpu_exec_realize/ in target-*

Follow up patches
  2) 1 patch, could move parts of split cpu_exec_init() to cpu_common_init()
              if that makes sense.
  3) 1 patch, could move cpu_exec_realize() into per target *_realizefn()
              it would be not small patch but still trivial
  4) 1 patch, do similar (#3) thing for unrealize
Perhaps #3,4 could be done in a more generic way in qom/cpu.c
but I don't have a good idea how to do it.

Re: [Qemu-devel] [PATCH v2 02/20] target-i386: move back cpu_exec_init() to init

[Eduardo Habkost]

On Fri, Oct 14, 2016 at 04:33:07PM +0200, Igor Mammedov wrote:
> On Fri, 14 Oct 2016 10:48:58 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Fri, Oct 14, 2016 at 03:34:17PM +0200, Laurent Vivier wrote:
> > > On 14/10/2016 15:33, Eduardo Habkost wrote:  
> > > > On Thu, Oct 13, 2016 at 06:24:44PM +0200, Laurent Vivier wrote:  
> > > >> We have now the cpu_exec_realize() in realize,
> > > >> so the init part must be in init.
> > > >>
> > > >> I've removed the cannot_destroy_with_object_finalize_yet field as
> > > >> unsafe references have been moved to cpu_exec_realize().
> > > >> (tested with QOM command provided by commit 4c315c27 with
> > > >> "athlon-x86_64-cpu")
> > > >>
> > > >> CC: Eduardo Habkost <ehabkost@redhat.com>
> > > >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>  
> > > > 
> > > > Resending the question I asked in my reply to v1:
> > > > 
> > > > Instead of creating requiring each subclass to manually call
> > > > cpu_exec_init()) on instance_init, why don't we move parts of
> > > > cpu_exec_init()/cpu_exec_realize() code to cpu_common_initfn()?
> > > > (TYPE_CPU's instance_init)
> > > > 
> > > > (And if there's any code that needs to be run after the
> > > > subclasses instance_init functions, we can just add a
> > > > instance_post_init function to TYPE_CPU).
> > > >   
> > > 
> > > It's done in PATCH 20/20.
> > > 
> > > Is that what you want?  
> > 
> > Yes (except that I would have inlined the the cpu_exec_init()
> > code inside cpu_common_init()).
> > 
> > I think I expected this to be done in a single step, that
> > wouldn't require touching code for all architectures three times.
> > Something like:
> > 
> > 1) Move cpu->as, cpu->num_ases, cpu->thread_id, cpu->memory
> >    initialization, and "memory" property registration from
> >    cpu_exec_init() to cpu_common_init() (no architecture code
> >    touched).
> > 2) (optional) Rename cpu_exec_init() to cpu_exec_realize() (only
> >    trivial changes in architecture code)
> I'd do all of it in 1 step
>   - split cpu_exec_init on init/realize parts
>     (comment in cpu_exec_init says that qom/cpu.c can't be used for
>      "memory" property)
>   - call cpu_exec_init() from cpu_common_init()
>   - s/cpu_exec_init/cpu_exec_realize/ in target-*

Personally, I don't mind if it is done in 1 or 2 steps. I would
just like to avoid changing architecture code 3 times.

In addition to make review easier, it would make the decision to
merge it easier for the maintainer who's going to do it (no need
to wait for Acked-bys/Reviewed-bys from multiple architecture
maintainers).

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 03/20] target-ppc: move back cpu_exec_init() to init

[Bharata B Rao]

On Fri, Oct 14, 2016 at 03:07:51PM +1100, David Gibson wrote:
> On Thu, Oct 13, 2016 at 06:24:45PM +0200, Laurent Vivier wrote:
> > We have now the cpu_exec_realize() in realize,
> > so the init part must be in init.
> > 
> > As cpu_exec_unrealize() is called from cpu_common_finalize(),
> > remove the call from ppc_cpu_unrealizefn().
> > 
> > CC: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > CC: Alexander Graf <agraf@suse.de>
> > CC: qemu-ppc@nongnu.org
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > ---
> >  target-ppc/translate_init.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> > index 094f28a..bbca8b5 100644
> > --- a/target-ppc/translate_init.c
> > +++ b/target-ppc/translate_init.c
> > @@ -9678,7 +9678,6 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
> >      }
> >  #endif
> >  
> > -    cpu_exec_init(cs);
> >      cpu_exec_realize(cs, &local_err);
> >      if (local_err != NULL) {
> >          error_propagate(errp, local_err);
> > @@ -9911,8 +9910,6 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
> >      opc_handler_t **table, **table_2;
> >      int i, j, k;
> >  
> > -    cpu_exec_unrealize(CPU(dev));
> > -
> 
> This doesn't seem right.  As you said in 0/20, cpu_exec_unrealize() is
> called from cpu_common_finalize().  But finalize should mirror init,
> not unrealize().  So it seems that unrealize() really should belong
> here, not in finalize.

For archs like PowerPC, cpu_exec_exit() was being called twice: once
from PowerPC CPU's unrealize function and once from cpu_common_finalize().
cpu_exec_exit() had two vmstate_unregister() calls and it used to ensure
that they are not called twice, but looks like this got changed sometime
back and we are now executing these two vmstate_unregister() calls twice.

While you are here, could you please take care of this ?

Regards,
Bharata.

Re: [Qemu-devel] [PATCH v2 01/20] exec: split cpu_exec_init()

[Greg Kurz]

On Fri, 14 Oct 2016 14:13:12 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 14/10/2016 09:56, Greg Kurz wrote:
> > On Thu, 13 Oct 2016 18:24:43 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >   
> >> Extract the realize part to cpu_exec_realize(), update all
> >> calls to cpu_exec_init() to add cpu_exec_realize() to
> >> have no functionnal change.
> >>
> >> Put in cpu_exec_init() what initializes the CPU,
> >> in cpu_exec_realize() what adds it to the environment.
> >>
> >> Remove error parameter from cpu_exec_init() as it can't fail.
> >>
> >> Rename cpu_exec_exit() with cpu_exec_unrealize():
> >> cpu_exec_exit() is undoing what it has been done by cpu_exec_realize(), so
> >> call it cpu_exec_unrealize().
> >>
> >> CC: Paolo Bonzini <pbonzini@redhat.com>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---  
> > 
> > Just one question: is there a reason that prevents cpu_exec_unrealize() to be
> > declared in include/exec/exec-all.h next to cpu_exec_realize() ?  
> 
> because qom/cpu.c doesn't include exec-all.h (and we can't as exec-all.h
> is target specific and qom/cpu.c is common code).
> 

That's a good reason indeed, even if cpu_exec_realize() and cpu_exec_unrealize()
could theorically be compiled only twice: once for system, once for user.

> Laurent

Thanks !

--
Greg

Re: [Qemu-devel] [PATCH v2 03/20] target-ppc: move back cpu_exec_init() to init

[Laurent Vivier]

On 14/10/2016 16:49, Bharata B Rao wrote:
> On Fri, Oct 14, 2016 at 03:07:51PM +1100, David Gibson wrote:
>> On Thu, Oct 13, 2016 at 06:24:45PM +0200, Laurent Vivier wrote:
>>> We have now the cpu_exec_realize() in realize,
>>> so the init part must be in init.
>>>
>>> As cpu_exec_unrealize() is called from cpu_common_finalize(),
>>> remove the call from ppc_cpu_unrealizefn().
>>>
>>> CC: Bharata B Rao <bharata@linux.vnet.ibm.com>
>>> CC: Alexander Graf <agraf@suse.de>
>>> CC: qemu-ppc@nongnu.org
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>>  target-ppc/translate_init.c | 4 +---
>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>> index 094f28a..bbca8b5 100644
>>> --- a/target-ppc/translate_init.c
>>> +++ b/target-ppc/translate_init.c
>>> @@ -9678,7 +9678,6 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
>>>      }
>>>  #endif
>>>  
>>> -    cpu_exec_init(cs);
>>>      cpu_exec_realize(cs, &local_err);
>>>      if (local_err != NULL) {
>>>          error_propagate(errp, local_err);
>>> @@ -9911,8 +9910,6 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
>>>      opc_handler_t **table, **table_2;
>>>      int i, j, k;
>>>  
>>> -    cpu_exec_unrealize(CPU(dev));
>>> -
>>
>> This doesn't seem right.  As you said in 0/20, cpu_exec_unrealize() is
>> called from cpu_common_finalize().  But finalize should mirror init,
>> not unrealize().  So it seems that unrealize() really should belong
>> here, not in finalize.
> 
> For archs like PowerPC, cpu_exec_exit() was being called twice: once
> from PowerPC CPU's unrealize function and once from cpu_common_finalize().
> cpu_exec_exit() had two vmstate_unregister() calls and it used to ensure
> that they are not called twice, but looks like this got changed sometime
> back and we are now executing these two vmstate_unregister() calls twice.
> 
> While you are here, could you please take care of this ?

Yes, I'm aware of that and taking care :)

Thanks,
Lauret