From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: KVM: init bsp_vcpu before kvm_arch_vcpu_init Date: Wed, 17 Jun 2009 16:58:05 +0300 Message-ID: <20090617135805.GC19508@redhat.com> References: <20090616143316.GA2765@amt.cnet> <20090617122905.GB19508@redhat.com> <20090617130759.GA14234@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm To: Marcelo Tosatti Return-path: Received: from mx2.redhat.com ([66.187.237.31]:51006 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756469AbZFQN6E (ORCPT ); Wed, 17 Jun 2009 09:58:04 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n5HDw7WV004685 for ; Wed, 17 Jun 2009 09:58:07 -0400 Content-Disposition: inline In-Reply-To: <20090617130759.GA14234@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Jun 17, 2009 at 10:07:59AM -0300, Marcelo Tosatti wrote: > On Wed, Jun 17, 2009 at 03:29:05PM +0300, Gleb Natapov wrote: > > On Tue, Jun 16, 2009 at 11:33:16AM -0300, Marcelo Tosatti wrote: > > > > > > On x86 mp_state is initialized by kvm_arch_vcpu_init. Right > > > now kvm_vcpu_is_bsp returns false because kvm->bsp_vcpu has > > > not been initialized, so vcpu_id == 0 ends up with mp_state == > > > KVM_MP_STATE_UNINITIALIZED. > > > > > > Gleb do you see a better way to fix this? > > > > > I have two, not necessary better ways. The first one is to change > > kvm_vcpu_is_bsp() to do kvm->bsp_vcpu_id == vcpu->vcpu_id. > > Thats much better. > > > Another one is to understand why mp_state is set to runnable for bsp > > here at all. May be we can drop this use of kvm_vcpu_is_bsp() since > > mp_state will be set to RUNNABLE in kvm_arch_vcpu_ioctl_set_sregs() > > anyway? > > Testcase is kvmctl without -p, which does not do > kvm_arch_vcpu_ioctl_set_sregs. I suppose that is valid? (in practice its > no big deal since kvmctl can be updated, and qemu-kvm does set_sregs, > but..). > I am not sure why all those special cases are present, may be for backwards compatibility. How can we make vcpu runnable without initializing it first (setting cpuid/regs/sregd)? But the patch is good regardless. > KVM: use vcpu_id instead of bsp_vcpu pointer in kvm_vcpu_is_bsp > > Change kvm_vcpu_is_bsp to use vcpu_id instead of bsp_vcpu pointer, which > is only initialized at the end of kvm_vm_ioctl_create_vcpu. > > Signed-off-by: Marcelo Tosatti > > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 1b48092..026ed0a 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -569,7 +569,7 @@ static inline void kvm_irqfd_release(struct kvm *kvm) {} > #ifdef CONFIG_KVM_APIC_ARCHITECTURE > static inline bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu) > { > - return vcpu->kvm->bsp_vcpu == vcpu; > + return vcpu->kvm->bsp_vcpu_id == vcpu->vcpu_id; > } > #endif > #endif -- Gleb.