diff mbox

KVM: init bsp_vcpu before kvm_arch_vcpu_init

Message ID 20090617130759.GA14234@amt.cnet (mailing list archive)
State New, archived
Headers show

Commit Message

Marcelo Tosatti June 17, 2009, 1:07 p.m. UTC
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..).

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 <mtosatti@redhat.com>


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Gleb Natapov June 17, 2009, 1:58 p.m. UTC | #1
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 <mtosatti@redhat.com>
> 
> 
> 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.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity June 29, 2009, 12:39 p.m. UTC | #2
On 06/17/2009 04:07 PM, Marcelo Tosatti wrote:
> 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.
>    

Applied, thanks.
diff mbox

Patch

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