From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: KVM: x86: do not execute halted vcpus (resend) Date: Wed, 10 Sep 2008 15:37:16 -0300 Message-ID: <20080910183716.GA6373@dmt.cnet> References: <20080908182347.GA8457@dmt.cnet> <48C68502.9010500@qumranet.com> <48C7B7BA.70405@qumranet.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm-devel To: Avi Kivity Return-path: Received: from mx1.redhat.com ([66.187.233.31]:41453 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750745AbYIJSiT (ORCPT ); Wed, 10 Sep 2008 14:38:19 -0400 Content-Disposition: inline In-Reply-To: <48C7B7BA.70405@qumranet.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Sep 10, 2008 at 03:04:10PM +0300, Avi Kivity wrote: > Avi Kivity wrote: >> Marcelo Tosatti wrote: >>> Offline or uninitialized vcpu's can be executed if requested to perform >>> userspace work. >>> Follow Avi's suggestion to handle halted vcpu's in the main loop, >>> simplifying kvm_emulate_halt(). Introduce a new vcpu->requests bit to >>> indicate events that promote state from halted to running. >>> >>> Also standardize vcpu wake sites. >>> >>> Avi, please confirm that this does not break Windows reboot (which I >>> can't >>> reproduce). >>> >> >> I couldn't reproduce it either (probably mistested earlier), so I >> applied it. Sorry about the drops. >> > > Actually it does reproduce. If you reboot twice, the second reboot will > hang during reset (with cs:ip = f000:fff0). Plain Windows XP. OK, easily reproducible by adding a sleep at the start of qemu_kvm_system_reset. The problem is that the vcpu HLT's between the KBD ioport write and the actual system reset by the IO thread. By then time there will be no event that takes it out of STATE_HALTED. The following fixes it, however older userspace will remain borked. Alternative is to special case f000:fff0 in-kernel. What is preferred? diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h index 346eedf..f3790fa 100644 --- a/libkvm/libkvm.h +++ b/libkvm/libkvm.h @@ -330,7 +330,13 @@ int kvm_set_mpstate(kvm_context_t kvm, int vcpu, */ static inline int kvm_reset_mpstate(kvm_context_t kvm, int vcpu) { - struct kvm_mp_state mp_state = {.mp_state = KVM_MP_STATE_UNINITIALIZED}; + struct kvm_mp_state mp_state; + + if (vcpu) + mp_state.mp_state = KVM_MP_STATE_UNINITIALIZED; + else + mp_state.mp_state = KVM_MP_STATE_RUNNABLE; + return kvm_set_mpstate(kvm, vcpu, &mp_state); } #endif diff --git a/qemu/qemu-kvm-x86.c b/qemu/qemu-kvm-x86.c index 5daedd1..9983e5a 100644 --- a/qemu/qemu-kvm-x86.c +++ b/qemu/qemu-kvm-x86.c @@ -673,15 +673,13 @@ int handle_tpr_access(void *opaque, int vcpu, void kvm_arch_cpu_reset(CPUState *env) { kvm_arch_load_regs(env); - if (env->cpu_index != 0) { - if (kvm_irqchip_in_kernel(kvm_context)) { + if (kvm_irqchip_in_kernel(kvm_context)) { #ifdef KVM_CAP_MP_STATE - kvm_reset_mpstate(kvm_context, env->cpu_index); + kvm_reset_mpstate(kvm_context, env->cpu_index); #endif - } else { - env->interrupt_request &= ~CPU_INTERRUPT_HARD; - env->halted = 1; - env->exception_index = EXCP_HLT; - } + } else if (env->cpu_index != 0) { + env->interrupt_request &= ~CPU_INTERRUPT_HARD; + env->halted = 1; + env->exception_index = EXCP_HLT; } }