Gleb Natapov wrote: > On Tue, Feb 02, 2010 at 09:19:01AM +0100, Jan Kiszka wrote: >> Push mpstate reading/writing into kvm_arch_load/save_regs and, on x86, >> properly synchronize with halted in the accessor functions. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> >> --- >> hw/apic.c | 7 ---- >> qemu-kvm-ia64.c | 4 ++- >> qemu-kvm-x86.c | 88 +++++++++++++++++++++++++++--------------------- >> qemu-kvm.c | 30 ----------------- >> qemu-kvm.h | 15 -------- >> target-i386/machine.c | 6 --- >> target-ia64/machine.c | 3 ++ >> 7 files changed, 55 insertions(+), 98 deletions(-) >> >> diff --git a/hw/apic.c b/hw/apic.c >> index 3e03e10..092c61e 100644 >> --- a/hw/apic.c >> +++ b/hw/apic.c >> @@ -507,13 +507,6 @@ void apic_init_reset(CPUState *env) >> s->wait_for_sipi = 1; >> >> env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP); >> -#ifdef KVM_CAP_MP_STATE >> - if (kvm_enabled() && kvm_irqchip_in_kernel()) { >> - env->mp_state >> - = env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE; >> - kvm_load_mpstate(env); >> - } >> -#endif >> } >> >> static void apic_startup(APICState *s, int vector_num) >> diff --git a/qemu-kvm-ia64.c b/qemu-kvm-ia64.c >> index fc8110e..39bcbeb 100644 >> --- a/qemu-kvm-ia64.c >> +++ b/qemu-kvm-ia64.c >> @@ -124,7 +124,9 @@ void kvm_arch_cpu_reset(CPUState *env) >> { >> if (kvm_irqchip_in_kernel(kvm_context)) { >> #ifdef KVM_CAP_MP_STATE >> - kvm_reset_mpstate(env->kvm_cpu_state.vcpu_ctx); >> + struct kvm_mp_state mp_state = {.mp_state = KVM_MP_STATE_UNINITIALIZED >> + }; >> + kvm_set_mpstate(env, &mp_state); >> #endif >> } else { >> env->interrupt_request &= ~CPU_INTERRUPT_HARD; >> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c >> index 63cd095..6b5895f 100644 >> --- a/qemu-kvm-x86.c >> +++ b/qemu-kvm-x86.c >> @@ -754,6 +754,48 @@ static int get_msr_entry(struct kvm_msr_entry *entry, CPUState *env) >> return 0; >> } >> >> +static void kvm_arch_save_mpstate(CPUState *env) >> +{ >> +#ifdef KVM_CAP_MP_STATE >> + int r; >> + struct kvm_mp_state mp_state; >> + >> + r = kvm_get_mpstate(env, &mp_state); >> + if (r < 0) { >> + env->mp_state = -1; >> + } else { >> + env->mp_state = mp_state.mp_state; >> + if (kvm_irqchip_in_kernel()) { >> + env->halted = (env->mp_state == KVM_MP_STATE_HALTED); >> + } >> + } >> +#else >> + env->mp_state = -1; >> +#endif >> +} >> + >> +static void kvm_arch_load_mpstate(CPUState *env) >> +{ >> +#ifdef KVM_CAP_MP_STATE >> + struct kvm_mp_state mp_state; >> + >> + /* >> + * -1 indicates that the host did not support GET_MP_STATE ioctl, >> + * so don't touch it. >> + */ >> + if (env->mp_state != -1) { >> + if (kvm_irqchip_in_kernel()) { >> + env->mp_state = env->halted ? KVM_MP_STATE_UNINITIALIZED : >> + KVM_MP_STATE_RUNNABLE; > When irqchip is in kernel env->halted doesn't contain any relevant > information, so this is incorrect. Actually env->halted is updated only > to show correct cpu state during "info cpus". OK, copied from apic_init_reset, see above. So that hunk was probably at least useless, and now it's harmfull. Will drop this and only sync from mp_state -> halted. Thanks, Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html