Re: KVM: init bsp_vcpu before kvm_arch_vcpu_init

Gleb Natapov
2009-06-17T12:29:05Z

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.