Re: KVM: init bsp_vcpu before kvm_arch_vcpu_init

[Gleb Natapov <gleb@redhat.com>]

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.