From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gleb Natapov <gleb@redhat.com>
Subject: Re: KVM: init bsp_vcpu before kvm_arch_vcpu_init
Date: Wed, 17 Jun 2009 15:29:05 +0300
Message-ID: <20090617122905.GB19508@redhat.com>
References: <20090616143316.GA2765@amt.cnet>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm <kvm@vger.kernel.org>
To: Marcelo Tosatti <mtosatti@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:36968 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S933365AbZFQM3F (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 17 Jun 2009 08:29:05 -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 n5HCT86P002975
	for <kvm@vger.kernel.org>; Wed, 17 Jun 2009 08:29:08 -0400
Content-Disposition: inline
In-Reply-To: <20090616143316.GA2765@amt.cnet>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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. 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?

> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 8939ffa..7225064 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -773,6 +773,13 @@ int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id)
>  	struct page *page;
>  	int r;
>  
> +	mutex_lock(&kvm->lock);
> +#ifdef CONFIG_KVM_APIC_ARCHITECTURE
> +	if (kvm->bsp_vcpu_id == id)
> +		kvm->bsp_vcpu = vcpu;
> +#endif
> +	mutex_unlock(&kvm->lock);
> +
>  	mutex_init(&vcpu->mutex);
>  	vcpu->cpu = -1;
>  	vcpu->kvm = kvm;
> @@ -1760,14 +1767,12 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
>  	smp_wmb();
>  	atomic_inc(&kvm->online_vcpus);
>  
> -#ifdef CONFIG_KVM_APIC_ARCHITECTURE
> -	if (kvm->bsp_vcpu_id == id)
> -		kvm->bsp_vcpu = vcpu;
> -#endif
>  	mutex_unlock(&kvm->lock);
>  	return r;
>  
>  vcpu_destroy:
> +	if (kvm->bsp_vcpu_id == id)
> +		kvm->bsp_vcpu = NULL;
>  	mutex_unlock(&kvm->lock);
>  	kvm_arch_vcpu_destroy(vcpu);
>  	return r;

--
			Gleb.