[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for 1.1-rc1] mce_init should be called after par
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH for 1.1-rc1] mce_init should be called after parsing cpu_model |
Date: |
Wed, 09 May 2012 21:26:41 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120421 Thunderbird/12.0 |
Am 09.05.2012 21:01, schrieb Igor Mammedov:
> Signed-off-by: Igor Mammedov <address@hidden>
$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(-)
>
> 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)
> }
> }
>
> +static void mce_init(X86CPU *cpu)
> +{
> + CPUX86State *cenv = &cpu->env;
> + unsigned int bank;
> +
> + if (((cenv->cpuid_version >> 8) & 0xf) >= 6
> + && (cenv->cpuid_features & (CPUID_MCE | CPUID_MCA)) ==
> + (CPUID_MCE | CPUID_MCA)) {
> + cenv->mcg_cap = MCE_CAP_DEF | MCE_BANKS_DEF;
> + cenv->mcg_ctl = ~(uint64_t)0;
> + for (bank = 0; bank < MCE_BANKS_DEF; bank++) {
> + cenv->mce_banks[bank * 4] = ~(uint64_t)0;
> + }
> + }
> +}
> +
> int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
> {
> CPUX86State *env = &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;
> }
>
> @@ -1706,22 +1724,6 @@ static void x86_cpu_reset(CPUState *s)
> cpu_watchpoint_remove_all(env, BP_CPU);
> }
>
> -static void mce_init(X86CPU *cpu)
> -{
> - CPUX86State *cenv = &cpu->env;
> - unsigned int bank;
> -
> - if (((cenv->cpuid_version >> 8) & 0xf) >= 6
> - && (cenv->cpuid_features & (CPUID_MCE | CPUID_MCA)) ==
> - (CPUID_MCE | CPUID_MCA)) {
> - cenv->mcg_cap = MCE_CAP_DEF | MCE_BANKS_DEF;
> - cenv->mcg_ctl = ~(uint64_t)0;
> - for (bank = 0; bank < MCE_BANKS_DEF; bank++) {
> - cenv->mce_banks[bank * 4] = ~(uint64_t)0;
> - }
> - }
> -}
> -
> static void x86_cpu_initfn(Object *obj)
> {
> X86CPU *cpu = X86_CPU(obj);
> @@ -1755,7 +1757,6 @@ static void x86_cpu_initfn(Object *obj)
> x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
>
> env->cpuid_apic_id = env->cpu_index;
> - mce_init(cpu);
> }
>
> static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg