From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38021) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SSCX8-0004js-I4 for qemu-devel@nongnu.org; Wed, 09 May 2012 15:26:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SSCX6-0001TT-Hc for qemu-devel@nongnu.org; Wed, 09 May 2012 15:26:46 -0400 Received: from cantor2.suse.de ([195.135.220.15]:46893 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SSCX6-0001TP-86 for qemu-devel@nongnu.org; Wed, 09 May 2012 15:26:44 -0400 Message-ID: <4FAAC4F1.9030902@suse.de> Date: Wed, 09 May 2012 21:26:41 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1336590089-30154-1-git-send-email-imammedo@redhat.com> In-Reply-To: <1336590089-30154-1-git-send-email-imammedo@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for 1.1-rc1] mce_init should be called after parsing cpu_model List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, ehabkost@redhat.com Am 09.05.2012 21:01, schrieb Igor Mammedov: > Signed-off-by: Igor Mammedov $subject is right, my bad. (Although I'm missing the target-i386: prefix. ;)) Re "for 1.1-rc1", fwiw this was broken in rc0 already, but then again there was no tarball of that. Does this fix some particular guest-visible behavior? Commit message doesn't mention. More comments inline: > --- > target-i386/cpu.c | 35 ++++++++++++++++++----------------- > 1 files changed, 18 insertions(+), 17 deletions(-) >=20 > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 65d9af6..5d11e7b 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -1153,6 +1153,22 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_= fprintf, const char *optarg) > } > } > =20 > +static void mce_init(X86CPU *cpu) > +{ > + CPUX86State *cenv =3D &cpu->env; > + unsigned int bank; > + > + if (((cenv->cpuid_version >> 8) & 0xf) >=3D 6 > + && (cenv->cpuid_features & (CPUID_MCE | CPUID_MCA)) =3D=3D > + (CPUID_MCE | CPUID_MCA)) { > + cenv->mcg_cap =3D MCE_CAP_DEF | MCE_BANKS_DEF; > + cenv->mcg_ctl =3D ~(uint64_t)0; > + for (bank =3D 0; bank < MCE_BANKS_DEF; bank++) { > + cenv->mce_banks[bank * 4] =3D ~(uint64_t)0; > + } > + } > +} > + > int cpu_x86_register(X86CPU *cpu, const char *cpu_model) > { > CPUX86State *env =3D &cpu->env; > @@ -1204,6 +1220,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu= _model) > error_free(error); > return -1; > } > + > + mce_init(cpu); This would be okay as a short-term rc1 fix. But long-term this doesn't fix the problem either, since we'd run into the same problem again when the CPU gets manipulated via QMP after the initialization. A more correct fix would thus be to move mce_init() back into cpu_x86_init(), but that's in a different file iirc. My preferred solution would be to introduce an x86-local x86_cpu_realize() function in cpu.c calling mce_init() - mce_init() won't need to move then - and to call that from cpu_x86_init(). I'll send a patch once I'm through fixing my cpu_xxx_init() changes. Regards, Andreas > return 0; > } > =20 > @@ -1706,22 +1724,6 @@ static void x86_cpu_reset(CPUState *s) > cpu_watchpoint_remove_all(env, BP_CPU); > } > =20 > -static void mce_init(X86CPU *cpu) > -{ > - CPUX86State *cenv =3D &cpu->env; > - unsigned int bank; > - > - if (((cenv->cpuid_version >> 8) & 0xf) >=3D 6 > - && (cenv->cpuid_features & (CPUID_MCE | CPUID_MCA)) =3D=3D > - (CPUID_MCE | CPUID_MCA)) { > - cenv->mcg_cap =3D MCE_CAP_DEF | MCE_BANKS_DEF; > - cenv->mcg_ctl =3D ~(uint64_t)0; > - for (bank =3D 0; bank < MCE_BANKS_DEF; bank++) { > - cenv->mce_banks[bank * 4] =3D ~(uint64_t)0; > - } > - } > -} > - > static void x86_cpu_initfn(Object *obj) > { > X86CPU *cpu =3D X86_CPU(obj); > @@ -1755,7 +1757,6 @@ static void x86_cpu_initfn(Object *obj) > x86_cpuid_set_tsc_freq, NULL, NULL, NULL); > =20 > env->cpuid_apic_id =3D env->cpu_index; > - mce_init(cpu); > } > =20 > static void x86_cpu_common_class_init(ObjectClass *oc, void *data) --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg