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

[Qemu-devel] [PATCH 00/19] 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.

Laurent Vivier (19):
  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.c                      |  8 +++++---
 include/exec/exec-all.h     |  1 +
 target-alpha/cpu.c          | 14 +++++++-------
 target-arm/cpu.c            | 18 +++++++-----------
 target-cris/cpu.c           | 14 +++++++-------
 target-i386/cpu.c           | 12 ++++++------
 target-lm32/cpu.c           | 14 +++++++-------
 target-m68k/cpu.c           | 14 +++++++-------
 target-microblaze/cpu.c     | 13 +++++++------
 target-mips/cpu.c           | 14 +++++++-------
 target-moxie/cpu.c          | 14 +++++++-------
 target-openrisc/cpu.c       | 14 +++++++-------
 target-ppc/translate_init.c |  3 ++-
 target-s390x/cpu.c          |  9 ++-------
 target-sh4/cpu.c            | 14 +++++++-------
 target-sparc/cpu.c          | 17 +++++++++--------
 target-tilegx/cpu.c         | 14 +++++++-------
 target-tricore/cpu.c        | 14 +++++++-------
 target-unicore32/cpu.c      | 17 +++++++++--------
 target-xtensa/cpu.c         | 14 +++++++-------
 20 files changed, 125 insertions(+), 127 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 01/19] 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.

CC: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 exec.c                      | 8 +++++---
 include/exec/exec-all.h     | 1 +
 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 | 5 +++++
 target-s390x/cpu.c          | 4 ++++
 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 +
 20 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/exec.c b/exec.c
index c8389f9..95b0aee 100644
--- a/exec.c
+++ b/exec.c
@@ -614,9 +614,6 @@ void cpu_exec_exit(CPUState *cpu)
 
 void cpu_exec_init(CPUState *cpu, Error **errp)
 {
-    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
-    Error *local_err ATTRIBUTE_UNUSED = NULL;
-
     cpu->as = NULL;
     cpu->num_ases = 0;
 
@@ -637,6 +634,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..649187d 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -58,6 +58,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
                               int cflags);
 
 void cpu_exec_init(CPUState *cpu, Error **errp);
+void cpu_exec_realize(CPUState *cpu, Error **errp);
 void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
 void QEMU_NORETURN cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc);
 
diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
index 6d01d7f..19f2eb2 100644
--- a/target-alpha/cpu.c
+++ b/target-alpha/cpu.c
@@ -267,6 +267,7 @@ static void alpha_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs, &error_abort);
+    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..5fda716 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -445,6 +445,7 @@ static void arm_cpu_initfn(Object *obj)
 
     cs->env_ptr = &cpu->env;
     cpu_exec_init(cs, &error_abort);
+    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..78753ed 100644
--- a/target-cris/cpu.c
+++ b/target-cris/cpu.c
@@ -188,6 +188,7 @@ static void cris_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs, &error_abort);
+    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..125a10c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3159,6 +3159,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         }
     }
     cpu_exec_init(cs, &error_abort);
+    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..4b93454 100644
--- a/target-lm32/cpu.c
+++ b/target-lm32/cpu.c
@@ -161,6 +161,7 @@ static void lm32_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs, &error_abort);
+    cpu_exec_realize(cs, &error_abort);
 
     env->flags = 0;
 
diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c
index 116b784..307910a 100644
--- a/target-m68k/cpu.c
+++ b/target-m68k/cpu.c
@@ -177,6 +177,7 @@ static void m68k_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs, &error_abort);
+    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..9ec2c9c 100644
--- a/target-microblaze/cpu.c
+++ b/target-microblaze/cpu.c
@@ -200,6 +200,7 @@ static void mb_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs, &error_abort);
+    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..8a42213 100644
--- a/target-mips/cpu.c
+++ b/target-mips/cpu.c
@@ -139,6 +139,7 @@ static void mips_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs, &error_abort);
+    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..28beff1 100644
--- a/target-moxie/cpu.c
+++ b/target-moxie/cpu.c
@@ -76,6 +76,7 @@ static void moxie_cpu_initfn(Object *obj)
 
     cs->env_ptr = &cpu->env;
     cpu_exec_init(cs, &error_abort);
+    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..1f2a084 100644
--- a/target-openrisc/cpu.c
+++ b/target-openrisc/cpu.c
@@ -96,6 +96,7 @@ static void openrisc_cpu_initfn(Object *obj)
 
     cs->env_ptr = &cpu->env;
     cpu_exec_init(cs, &error_abort);
+    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..5e84e65 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9683,6 +9683,11 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
         error_propagate(errp, local_err);
         return;
     }
+    cpu_exec_realize(cs, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
 #if !defined(CONFIG_USER_ONLY)
     cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 35ae2ce..20a88dd 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -211,6 +211,10 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
     if (err != NULL) {
         goto out;
     }
+    cpu_exec_realize(cs, &err);
+    if (err != NULL) {
+        goto out;
+    }
     scc->next_cpu_id++;
 
 #if !defined(CONFIG_USER_ONLY)
diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
index f589532..f172516 100644
--- a/target-sh4/cpu.c
+++ b/target-sh4/cpu.c
@@ -259,6 +259,7 @@ static void superh_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs, &error_abort);
+    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..77cc920 100644
--- a/target-sparc/cpu.c
+++ b/target-sparc/cpu.c
@@ -815,6 +815,7 @@ static void sparc_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs, &error_abort);
+    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..f7ec920 100644
--- a/target-tilegx/cpu.c
+++ b/target-tilegx/cpu.c
@@ -108,6 +108,7 @@ static void tilegx_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs, &error_abort);
+    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..2fd3325 100644
--- a/target-tricore/cpu.c
+++ b/target-tricore/cpu.c
@@ -96,6 +96,7 @@ static void tricore_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs, &error_abort);
+    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..2e5466a 100644
--- a/target-unicore32/cpu.c
+++ b/target-unicore32/cpu.c
@@ -117,6 +117,7 @@ static void uc32_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     cpu_exec_init(cs, &error_abort);
+    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..fc72726 100644
--- a/target-xtensa/cpu.c
+++ b/target-xtensa/cpu.c
@@ -118,6 +118,7 @@ static void xtensa_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     env->config = xcc->config;
     cpu_exec_init(cs, &error_abort);
+    cpu_exec_realize(cs, &error_abort);
 
     if (tcg_enabled() && !tcg_inited) {
         tcg_inited = true;
-- 
2.7.4

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

[Peter Maydell]

On 5 October 2016 at 12:38, 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.
>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  exec.c                      | 8 +++++---
>  include/exec/exec-all.h     | 1 +
>  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 | 5 +++++
>  target-s390x/cpu.c          | 4 ++++
>  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 +
>  20 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index c8389f9..95b0aee 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -614,9 +614,6 @@ void cpu_exec_exit(CPUState *cpu)
>
>  void cpu_exec_init(CPUState *cpu, Error **errp)
>  {
> -    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
> -    Error *local_err ATTRIBUTE_UNUSED = NULL;
> -
>      cpu->as = NULL;
>      cpu->num_ases = 0;
>
> @@ -637,6 +634,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);

I think cpu_list_add() needs to be in init, because
this is where we set cpu->cpu_index, and (after patch 5)
target-arm assumes that cpu_index has been set by
init but before realize. So I guess we should do it
in init and then roll back in the destructor?

thanks
-- PMM

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

[Eduardo Habkost]

On Wed, Oct 05, 2016 at 01:13:18PM -0700, Peter Maydell wrote:
> On 5 October 2016 at 12:38, 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.
> >
> > CC: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > ---
> >  exec.c                      | 8 +++++---
> >  include/exec/exec-all.h     | 1 +
> >  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 | 5 +++++
> >  target-s390x/cpu.c          | 4 ++++
> >  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 +
> >  20 files changed, 31 insertions(+), 3 deletions(-)
> >
> > diff --git a/exec.c b/exec.c
> > index c8389f9..95b0aee 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -614,9 +614,6 @@ void cpu_exec_exit(CPUState *cpu)
> >
> >  void cpu_exec_init(CPUState *cpu, Error **errp)
> >  {
> > -    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
> > -    Error *local_err ATTRIBUTE_UNUSED = NULL;
> > -
> >      cpu->as = NULL;
> >      cpu->num_ases = 0;
> >
> > @@ -637,6 +634,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);
> 
> I think cpu_list_add() needs to be in init, because
> this is where we set cpu->cpu_index, and (after patch 5)
> target-arm assumes that cpu_index has been set by
> init but before realize. So I guess we should do it
> in init and then roll back in the destructor?

instance_init is supposed to have no side-effects, that's the
main reason cpu_exec_init() was moved to realize.
qmp_device_list_properties(), for example, creates new objects to
query the available properties. query-cpu-model-expansion and
related commands will probably be implemented by many
architectures by instantiating actual objects.

-- 
Eduardo

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

[Laurent Vivier]

On 05/10/2016 22:13, Peter Maydell wrote:
> On 5 October 2016 at 12:38, 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.
>>
>> CC: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>  exec.c                      | 8 +++++---
>>  include/exec/exec-all.h     | 1 +
>>  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 | 5 +++++
>>  target-s390x/cpu.c          | 4 ++++
>>  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 +
>>  20 files changed, 31 insertions(+), 3 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index c8389f9..95b0aee 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -614,9 +614,6 @@ void cpu_exec_exit(CPUState *cpu)
>>
>>  void cpu_exec_init(CPUState *cpu, Error **errp)
>>  {
>> -    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
>> -    Error *local_err ATTRIBUTE_UNUSED = NULL;
>> -
>>      cpu->as = NULL;
>>      cpu->num_ases = 0;
>>
>> @@ -637,6 +634,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);
> 
> I think cpu_list_add() needs to be in init, because
> this is where we set cpu->cpu_index, and (after patch 5)
> target-arm assumes that cpu_index has been set by
> init but before realize. So I guess we should do it
> in init and then roll back in the destructor?

I think in some case cpu_list_add() can fail (no more index available),
it's why it has to be in realize. In the case of hotplug we must not
kill the machine.

See "5a790cc cpu: Add Error argument to cpu_exec_init()"

Laurent

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

[David Gibson]

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

On Wed, Oct 05, 2016 at 01:13:18PM -0700, Peter Maydell wrote:
> On 5 October 2016 at 12:38, 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.
> >
> > CC: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > ---
> >  exec.c                      | 8 +++++---
> >  include/exec/exec-all.h     | 1 +
> >  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 | 5 +++++
> >  target-s390x/cpu.c          | 4 ++++
> >  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 +
> >  20 files changed, 31 insertions(+), 3 deletions(-)
> >
> > diff --git a/exec.c b/exec.c
> > index c8389f9..95b0aee 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -614,9 +614,6 @@ void cpu_exec_exit(CPUState *cpu)
> >
> >  void cpu_exec_init(CPUState *cpu, Error **errp)
> >  {
> > -    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
> > -    Error *local_err ATTRIBUTE_UNUSED = NULL;
> > -
> >      cpu->as = NULL;
> >      cpu->num_ases = 0;
> >
> > @@ -637,6 +634,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);
> 
> I think cpu_list_add() needs to be in init, because
> this is where we set cpu->cpu_index, and (after patch 5)
> target-arm assumes that cpu_index has been set by
> init but before realize. So I guess we should do it
> in init and then roll back in the destructor?

That will need to be fixed on the ARM side then.  The whole new
cpu_index assignment model (which allows cross-platform hotplug)
relies on it not being fixed until realize() time.

-- 
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 01/19] exec: split cpu_exec_init()

[Peter Maydell]

On 6 October 2016 at 06:02, David Gibson <david@gibson.dropbear.id.au> wrote:
> On Wed, Oct 05, 2016 at 01:13:18PM -0700, Peter Maydell wrote:
>> I think cpu_list_add() needs to be in init, because
>> this is where we set cpu->cpu_index, and (after patch 5)
>> target-arm assumes that cpu_index has been set by
>> init but before realize. So I guess we should do it
>> in init and then roll back in the destructor?
>
> That will need to be fixed on the ARM side then.  The whole new
> cpu_index assignment model (which allows cross-platform hotplug)
> relies on it not being fixed until realize() time.

OK. I guess we need to audit all the code that can get called
between init and realize, which is a bit tedious. Is there
anything else we need to look for other than use of cpu_index ?

thanks
-- PMM

[Qemu-devel] [PATCH 02/19] 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
cpu_exec_init() is not called by x86_cpu_initfn() anymore
(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 125a10c..087c78d 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, &error_abort);
-    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, &error_abort);
 
     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 03/19] 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

CC: Alexander Graf <agraf@suse.de>
CC: qemu-ppc@nongnu.org
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-ppc/translate_init.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 5e84e65..5eb2d3a 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9678,11 +9678,6 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 #endif
 
-    cpu_exec_init(cs, &local_err);
-    if (local_err != NULL) {
-        error_propagate(errp, local_err);
-        return;
-    }
     cpu_exec_realize(cs, &local_err);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
@@ -10439,6 +10434,7 @@ static void ppc_cpu_initfn(Object *obj)
     CPUPPCState *env = &cpu->env;
 
     cs->env_ptr = env;
+    cpu_exec_init(cs, &error_abort);
 
     env->msr_mask = pcc->msr_mask;
     env->mmu_model = pcc->mmu_model;
-- 
2.7.4

[Qemu-devel] [PATCH 04/19] 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 | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 20a88dd..74154a9 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -207,10 +207,6 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
         goto out;
     }
 
-    cpu_exec_init(cs, &err);
-    if (err != NULL) {
-        goto out;
-    }
     cpu_exec_realize(cs, &err);
     if (err != NULL) {
         goto out;
@@ -293,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, &error_abort);
     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);
@@ -444,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 05/19] target-arm: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by arm_cpu_initfn() anymore
(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 | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 5fda716..7920fb0 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -445,7 +445,6 @@ static void arm_cpu_initfn(Object *obj)
 
     cs->env_ptr = &cpu->env;
     cpu_exec_init(cs, &error_abort);
-    cpu_exec_realize(cs, &error_abort);
     cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
                                          g_free, g_free);
 
@@ -577,6 +576,13 @@ 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;
+
+    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)) {
@@ -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

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

[David Gibson]

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

On Wed, Oct 05, 2016 at 09:38:39PM +0200, Laurent Vivier wrote:
> I've removed the cannot_destroy_with_object_finalize_yet field as
> cpu_exec_init() is not called by arm_cpu_initfn() anymore

I think this (and most of the similar commit messages following) needs
a slight reword.  A function called 'cpu_exec_init()' is still called
from the initfn, it's just the unsafe things it used to do 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 | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 5fda716..7920fb0 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -445,7 +445,6 @@ static void arm_cpu_initfn(Object *obj)
>  
>      cs->env_ptr = &cpu->env;
>      cpu_exec_init(cs, &error_abort);
> -    cpu_exec_realize(cs, &error_abort);
>      cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
>                                           g_free, g_free);
>  
> @@ -577,6 +576,13 @@ 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;
> +
> +    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)) {
> @@ -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)

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

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

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by alpha_cpu_initfn() anymore
(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 19f2eb2..5d739c6 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, &error_abort);
-    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 07/19] target-cris: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by cris_cpu_initfn() anymore
(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 78753ed..e83d055 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, &error_abort);
-    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 08/19] target-lm32: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by lm32_cpu_initfn() anymore
(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 4b93454..b835779 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, &error_abort);
-    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 09/19] target-m68k: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by m68k_cpu_initfn() anymore
(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 307910a..0728e02 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, &error_abort);
-    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 10/19] target-microblaze: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by mb_cpu_initfn() anymore
(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 9ec2c9c..625ce71 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, &error_abort);
-    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 11/19] target-mips: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by mips_cpu_initfn() anymore
(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 8a42213..cac8ed6 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, &error_abort);
-    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 12/19] target-moxie: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by moxie_cpu_initfn() anymore
(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 28beff1..51fa87a 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, &error_abort);
-    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 13/19] target-openrisc: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by openrisc_cpu_initfn() anymore
(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 1f2a084..bd7360f 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, &error_abort);
-    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 14/19] target-sh4: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by superh_cpu_initfn() anymore
(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 f172516..c7ed4ea 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, &error_abort);
-    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 15/19] target-sparc: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by sparc_cpu_initfn() anymore
(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 77cc920..77979a0 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, &error_abort);
-    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 16/19] target-tilegx: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by tilegx_cpu_initfn() anymore
(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 f7ec920..6be69ef 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, &error_abort);
-    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

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

[Chen Gang]

On 10/6/16 03:38, Laurent Vivier wrote:
> I've removed the cannot_destroy_with_object_finalize_yet field as
> cpu_exec_init() is not called by tilegx_cpu_initfn() anymore
> (not tested with QOM command as tilegx is only available in linux-user
>  mode)
> 


For master branch, I can not find cpu_exec_realize in total source code,
I am not quite sure whether it is ok or not.

And in honest, now, tilegx-linux-user is not implemented completely, the
floating point insns are not supported by our main branch (which should
be one of main feature for tilegx) -- which I should try.

Thanks.

> 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 f7ec920..6be69ef 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, &error_abort);
> -    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 = {
> 

-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

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

[David Gibson]

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

On Thu, Oct 06, 2016 at 07:57:34AM +0800, Chen Gang wrote:
> 
> On 10/6/16 03:38, Laurent Vivier wrote:
> > I've removed the cannot_destroy_with_object_finalize_yet field as
> > cpu_exec_init() is not called by tilegx_cpu_initfn() anymore
> > (not tested with QOM command as tilegx is only available in linux-user
> >  mode)
> > 
> 
> 
> For master branch, I can not find cpu_exec_realize in total source code,
> I am not quite sure whether it is ok or not.

It's a new function introduced in the first patch of this series.  The
old cpu_exec_init() was split into cpu_exec_init() which contains
things which are safe at init time, and cpu_exec_realize() which is
supposed to be called at realize() time.

> 
> And in honest, now, tilegx-linux-user is not implemented completely, the
> floating point insns are not supported by our main branch (which should
> be one of main feature for tilegx) -- which I should try.
> 
> Thanks.
> 
> > 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 f7ec920..6be69ef 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, &error_abort);
> > -    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 = {
> > 
> 

-- 
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 16/19] target-tilegx: move cpu_exec_realize() to realize function

[Laurent Vivier]

On 06/10/2016 01:57, Chen Gang wrote:
> 
> On 10/6/16 03:38, Laurent Vivier wrote:
>> I've removed the cannot_destroy_with_object_finalize_yet field as
>> cpu_exec_init() is not called by tilegx_cpu_initfn() anymore
>> (not tested with QOM command as tilegx is only available in linux-user
>>  mode)
>>
> 
> 
> For master branch, I can not find cpu_exec_realize in total source code,
> I am not quite sure whether it is ok or not.

See PATCH 1/19  exec: split cpu_exec_init()

Thanks,
Laurent

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

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by tricore_cpu_initfn() anymore
(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 2fd3325..6a57aa9 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, &error_abort);
-    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 18/19] target-unicore32: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by uc32_cpu_initfn() anymore
(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 2e5466a..a98908a 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, &error_abort);
-    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 19/19] target-xtensa: move cpu_exec_realize() to realize function

[Laurent Vivier]

I've removed the cannot_destroy_with_object_finalize_yet field as
cpu_exec_init() is not called by xtensa_cpu_initfn() anymore
(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 fc72726..79e7fa3 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, &error_abort);
-    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

Re: [Qemu-devel] [PATCH 00/19] Split cpu_exec_init() into an init and a realize part

[Peter Maydell]

On 5 October 2016 at 12:38, Laurent Vivier <lvivier@redhat.com> wrote:
> 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.

At some point following this we should remove the Error** argument
from cpu_exec_init() : init isn't permitted to fail and indeed
cpu_exec_init() never does, so the argument is unneeded.

If realize fails do we correctly release the reference we take on
cpu->memory in cpu_exec_init()?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 00/19] Split cpu_exec_init() into an init and a realize part

[Laurent Vivier]

On 05/10/2016 22:11, Peter Maydell wrote:
> On 5 October 2016 at 12:38, Laurent Vivier <lvivier@redhat.com> wrote:
>> 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.
> 
> At some point following this we should remove the Error** argument
> from cpu_exec_init() : init isn't permitted to fail and indeed
> cpu_exec_init() never does, so the argument is unneeded.

I agree.

> If realize fails do we correctly release the reference we take on
> cpu->memory in cpu_exec_init()?

Because of OBJ_PROP_LINK_UNREF_ON_RELEASE, I think the reference is
decreased when the object is released.

Laurent

Re: [Qemu-devel] [PATCH 00/19] Split cpu_exec_init() into an init and a realize part

[Eduardo Habkost]

On Wed, Oct 05, 2016 at 09:38:34PM +0200, Laurent Vivier wrote:
> 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).

Instead of requiring each subclass to call both the init and
realize functions at the right moment, why don't we move the code
that needs to run at instance_init time to cpu_common_initfn()?
(TYPE_CPU's instance_init)

cpu_common_initfn() runs before the subclasses' instance_init
functions. But if we need anything to run _after_ subclasses'
instance_init functions, we can also make TYPE_CPU implement a
instance_post_init function.

> 
> 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.
> 
> Laurent Vivier (19):
>   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.c                      |  8 +++++---
>  include/exec/exec-all.h     |  1 +
>  target-alpha/cpu.c          | 14 +++++++-------
>  target-arm/cpu.c            | 18 +++++++-----------
>  target-cris/cpu.c           | 14 +++++++-------
>  target-i386/cpu.c           | 12 ++++++------
>  target-lm32/cpu.c           | 14 +++++++-------
>  target-m68k/cpu.c           | 14 +++++++-------
>  target-microblaze/cpu.c     | 13 +++++++------
>  target-mips/cpu.c           | 14 +++++++-------
>  target-moxie/cpu.c          | 14 +++++++-------
>  target-openrisc/cpu.c       | 14 +++++++-------
>  target-ppc/translate_init.c |  3 ++-
>  target-s390x/cpu.c          |  9 ++-------
>  target-sh4/cpu.c            | 14 +++++++-------
>  target-sparc/cpu.c          | 17 +++++++++--------
>  target-tilegx/cpu.c         | 14 +++++++-------
>  target-tricore/cpu.c        | 14 +++++++-------
>  target-unicore32/cpu.c      | 17 +++++++++--------
>  target-xtensa/cpu.c         | 14 +++++++-------
>  20 files changed, 125 insertions(+), 127 deletions(-)
> 
> -- 
> 2.7.4
> 

-- 
Eduardo