| Message ID | 1476375902-11715-2-git-send-email-lvivier@redhat.com |
|---|---|
| State | New |
| Headers | show |
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;
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;
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
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.
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
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;
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
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
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
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;
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(-)