Re: [PATCH 1/2] Complete cpu initialization before signaling main thread.

[Gleb Natapov <gleb@redhat.com>]

On Tue, Oct 13, 2009 at 03:23:48PM -0300, Marcelo Tosatti wrote:
> On Tue, Oct 13, 2009 at 03:19:08PM -0300, Marcelo Tosatti wrote:
> > > @@ -2003,15 +1991,25 @@ static void *ap_main_loop(void *_env)
> > >          on_vcpu(env, kvm_arch_do_ioperm, data);
> > >  #endif
> > >  
> > > -    /* signal VCPU creation */
> > > +    setup_kernel_sigmask(env);
> > > +
> > >      pthread_mutex_lock(&qemu_mutex);
> > > +    cpu_single_env = env;
> > > +
> > > +    kvm_arch_init_vcpu(env);
> > > +#ifdef TARGET_I386
> > > +    kvm_tpr_vcpu_start(env);
> > > +#endif
> > > +
> > > +    kvm_arch_load_regs(env);
> > > +
> > > +    /* signal VCPU creation */
> > >      current_env->created = 1;
> > >      pthread_cond_signal(&qemu_vcpu_cond);
> > >  
> > >      /* and wait for machine initialization */
> > >      while (!qemu_system_ready)
> > >          qemu_cond_wait(&qemu_system_cond);
> > > -    pthread_mutex_unlock(&qemu_mutex);
> > 
> > You don't set cpu_single_env after reacquiring 
> > qemu_mutex here (via qemu_cond_wait).
> > 
> 
> Also i'm curious about the failure.
This patch by itself doesn't fix the bug. Next one does. This one
rearrange code to make more sense. CPU is created only when it is
initialized and ready to run.

> 
> Why say, bsp should care about other cpu's register state while doing MP
> init?
> 
Because vcpu init will reset MP state, so if bsp will send sipi to vcpu1
before vcpu1 is initialized sipi will be lost.

> MP state is set via apic_reset, which happens before qemu_system_ready
> is set.
> 
Without my next patch MP state is set (by set I mean ioctl(mp_state)) only on vcpu_run.

--
			Gleb.