From: Greg Kurz <groug@kaod.org>
To: Laurent Vivier <lvivier@redhat.com>
Cc: qemu-devel@nongnu.org,
Matthew Rosato <mjrosato@linux.vnet.ibm.com>,
Peter Maydell <peter.maydell@linaro.org>,
Eduardo Habkost <ehabkost@redhat.com>,
Markus Armbruster <armbru@redhat.com>,
Bharata B Rao <bharata@linux.vnet.ibm.com>,
Paolo Bonzini <pbonzini@redhat.com>,
David Gibson <david@gibson.dropbear.id.au>
Subject: Re: [Qemu-devel] [PATCH v2 01/20] exec: split cpu_exec_init()
Date: Fri, 14 Oct 2016 09:56:16 +0200 [thread overview]
Message-ID: <20161014095616.661fdf1c@bahia> (raw)
In-Reply-To: <1476375902-11715-2-git-send-email-lvivier@redhat.com>
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;
next prev parent reply other threads:[~2016-10-14 7:56 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-13 16:24 [Qemu-devel] [PATCH v2 00/20] Split cpu_exec_init() into an init and a realize part Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 01/20] exec: split cpu_exec_init() Laurent Vivier
2016-10-14 4:05 ` David Gibson
2016-10-14 7:56 ` Greg Kurz [this message]
2016-10-14 12:13 ` Laurent Vivier
2016-10-14 15:04 ` Greg Kurz
2016-10-14 13:55 ` Igor Mammedov
2016-10-14 13:57 ` Laurent Vivier
2016-10-14 14:11 ` Igor Mammedov
2016-10-14 14:13 ` Laurent Vivier
2016-10-14 14:26 ` Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 02/20] target-i386: move back cpu_exec_init() to init Laurent Vivier
2016-10-14 13:33 ` Eduardo Habkost
2016-10-14 13:34 ` Laurent Vivier
2016-10-14 13:48 ` Eduardo Habkost
2016-10-14 14:12 ` Laurent Vivier
2016-10-14 14:31 ` Eduardo Habkost
2016-10-14 14:33 ` Igor Mammedov
2016-10-14 14:43 ` Eduardo Habkost
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 03/20] target-ppc: " Laurent Vivier
2016-10-14 4:07 ` David Gibson
2016-10-14 7:12 ` Laurent Vivier
2016-10-14 14:49 ` Bharata B Rao
2016-10-14 17:26 ` Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 04/20] target-s390: " Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 05/20] target-arm: move cpu_exec_realize() to realize function Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 06/20] target-alpha: " Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 07/20] target-cris: " Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 08/20] target-lm32: " Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 09/20] target-m68k: " Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 10/20] target-microblaze: " Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 11/20] target-mips: " Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 12/20] target-moxie: " Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 13/20] target-openrisc: " Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 14/20] target-sh4: " Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 15/20] target-sparc: " Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 16/20] target-tilegx: " Laurent Vivier
2016-10-13 16:24 ` [Qemu-devel] [PATCH v2 17/20] target-tricore: " Laurent Vivier
2016-10-13 16:25 ` [Qemu-devel] [PATCH v2 18/20] target-unicore32: " Laurent Vivier
2016-10-13 16:25 ` [Qemu-devel] [PATCH v2 19/20] target-xtensa: " Laurent Vivier
2016-10-13 16:25 ` [Qemu-devel] [PATCH v2 20/20] exec: move cpu_exec_init() to cpu_common_initfn() Laurent Vivier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161014095616.661fdf1c@bahia \
--to=groug@kaod.org \
--cc=armbru@redhat.com \
--cc=bharata@linux.vnet.ibm.com \
--cc=david@gibson.dropbear.id.au \
--cc=ehabkost@redhat.com \
--cc=lvivier@redhat.com \
--cc=mjrosato@linux.vnet.ibm.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).