From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60380) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YTYth-0006Mj-Bj for qemu-devel@nongnu.org; Thu, 05 Mar 2015 11:45:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YTYtZ-0001c7-J0 for qemu-devel@nongnu.org; Thu, 05 Mar 2015 11:45:17 -0500 Received: from cantor2.suse.de ([195.135.220.15]:41749 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YTYtZ-0001bN-9O for qemu-devel@nongnu.org; Thu, 05 Mar 2015 11:45:09 -0500 Message-ID: <54F8880A.6050908@suse.de> Date: Thu, 05 Mar 2015 17:44:58 +0100 From: =?windows-1252?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1425569930-6660-1-git-send-email-ehabkost@redhat.com> <1425569930-6660-7-git-send-email-ehabkost@redhat.com> <20150305174253.41582208@nial.brq.redhat.com> In-Reply-To: <20150305174253.41582208@nial.brq.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 6/6] target-i386: Call cpu_exec_init() on realize List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: zhugh.fnst@cn.fujitsu.com, Eduardo Habkost , qemu-devel@nongnu.org, tangchen@cn.fujitsu.com, Gu Zheng , isimatu.yasuaki@jp.fujitsu.com, anshul.makkar@profitbricks.com, chen.fan.fnst@cn.fujitsu.com, Paolo Bonzini Am 05.03.2015 um 17:42 schrieb Igor Mammedov: > On Thu, 5 Mar 2015 12:38:50 -0300 > Eduardo Habkost wrote: >=20 >> To allow new code to ask the CPU classes for CPU model information and >> allow QOM properties to be queried by qmp_device_list_properties(), we >> need to be able to safely instantiate a X86CPU object without any >> side-effects. >> >> cpu_exec_init() has lots of side-effects on global QEMU state, move it >> to realize so it will be called only if the X86CPU instance is realize= d. >> >> For reference, this is the current cpu_exec_init() code: >> >>> void cpu_exec_init(CPUArchState *env) >>> { >>> CPUState *cpu =3D ENV_GET_CPU(env); >>> CPUClass *cc =3D CPU_GET_CLASS(cpu); >>> CPUState *some_cpu; >>> int cpu_index; >>> >>> #ifndef CONFIG_USER_ONLY >>> cpu->as =3D &address_space_memory; >>> cpu->thread_id =3D qemu_get_thread_id(); >>> #endif >> >> Those fields should be used only after actually starting the VCPU and = can be >> initialized on realize. >> >>> >>> #if defined(CONFIG_USER_ONLY) >>> cpu_list_lock(); >>> #endif >>> cpu_index =3D 0; >>> CPU_FOREACH(some_cpu) { >>> cpu_index++; >>> } >>> cpu->cpu_index =3D cpu_index; >>> QTAILQ_INSERT_TAIL(&cpus, cpu, node); >>> #if defined(CONFIG_USER_ONLY) >>> cpu_list_unlock(); >>> #endif >> >> The above initializes cpu_index and add the CPU to the global CPU list= . >> This affects QEMU global state and must be done only on realize. >> >>> if (qdev_get_vmsd(DEVICE(cpu)) =3D=3D NULL) { >>> vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu); >>> } >>> #if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY) >>> register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION, >>> cpu_save, cpu_load, env); >>> assert(cc->vmsd =3D=3D NULL); >>> assert(qdev_get_vmsd(DEVICE(cpu)) =3D=3D NULL); >>> #endif >>> if (cc->vmsd !=3D NULL) { >>> vmstate_register(NULL, cpu_index, cc->vmsd, cpu); >>> } >> >> vmstate and savevm registration also affects global QEMU state and sho= uld be >> done only on realize. >> >>> } >> >> Signed-off-by: Eduardo Habkost >> --- >> target-i386/cpu.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >> index 400b1e0..8b76604 100644 >> --- a/target-i386/cpu.c >> +++ b/target-i386/cpu.c >> @@ -2758,6 +2758,8 @@ static void x86_cpu_realizefn(DeviceState *dev, = Error **errp) >> static bool ht_warned; >> static bool tcg_initialized; >> =20 >> + cpu_exec_init(env); >> + >> if (tcg_enabled() && !tcg_initialized) { >> tcg_initialized =3D 1; >> tcg_x86_init(); >> @@ -2840,7 +2842,6 @@ static void x86_cpu_initfn(Object *obj) >> CPUX86State *env =3D &cpu->env; >> =20 >> cs->env_ptr =3D env; >> - cpu_exec_init(env); > looks wrong, later in this function we do > env->cpuid_apic_id =3D x86_cpu_apic_id_from_index(cs->cpu_index); > and with this patch will always yield 0 Being tackled in Eduardo's APIC series. ;) Cheers, Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Felix Imend=F6rffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG N=FCrnberg)