[Qemu-devel] Re: [COMMIT 733318e] don't call cpu_sychronize_state from reset handlers

[Glauber Costa]

On Fri, Sep 11, 2009 at 01:15:49PM +0200, Jan Kiszka wrote:
> Anthony Liguori wrote:
> > From: Glauber Costa <address@hidden>
> > 
> > Doing this will make the vcpu ioctl be issued from the I/O thread, instead
> > of cpu thread. The correct behaviour is to call it from within the cpu 
> > thread,
> > as soon as we are ready to go.
> 
> Note that in the good old days, this used to work properly (in qemu-kvm)
> as registers write-back was routed through on_vcpu.

I believe we should avoid the use of those things, specially at initialization. 
They are
totally racy and fragile. One way to do that, is to do all the reset functions 
inside the
cpu thread.

I already have something hacked up for this, will send as soon as I finish 
testing.

> 
> > 
> > Signed-off-by: Glauber Costa <address@hidden>
> > Signed-off-by: Anthony Liguori <aliguori-r/address@hidden>
> > 
> > diff --git a/hw/apic.c b/hw/apic.c
> > index 2c414c1..9f1d25e 100644
> > --- a/hw/apic.c
> > +++ b/hw/apic.c
> > @@ -938,8 +938,6 @@ static void apic_reset(void *opaque)
> >      APICState *s = opaque;
> >      int bsp;
> >  
> > -    cpu_synchronize_state(s->cpu_env);
> > -
> >      bsp = cpu_is_bsp(s->cpu_env);
> >      s->apicbase = 0xfee00000 |
> >          (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE;
> > diff --git a/vl.c b/vl.c
> > index 8e5d9db..c6c6a6b 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -3683,10 +3683,12 @@ static void *kvm_cpu_thread_fn(void *arg)
> >      while (!qemu_system_ready)
> >          qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100);
> >  
> > +    cpu_synchronize_state(env);
> > +
> >      while (1) {
> > +        qemu_wait_io_event(env);
> >          if (cpu_can_run(env))
> >              qemu_cpu_exec(env);
> > -        qemu_wait_io_event(env);
> >      }
> >  
> >      return NULL;
> > @@ -3711,6 +3713,9 @@ static void *tcg_cpu_thread_fn(void *arg)
> >      while (!qemu_system_ready)
> >          qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100);
> >  
> > +    for (env = first_cpu; env != NULL; env = env->next_cpu) {
> > +        cpu_synchronize_state(env);
> > +    }
> >      while (1) {
> >          tcg_cpu_exec();
> >          qemu_wait_io_event(cur_cpu);
> > 
> 
> This unfortunately breaks upstream KVM (Linux fails to detect the tiemr
> IRQ properly). Is there a fix already in sight? I lost a bit overview of
> all on_vcpu refactorings and kvm-register sync "simplifications".
> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT SE 2
> Corporate Competence Center Embedded Linux