On Tue, Dec 15, 2009 at 10:24:15AM -0200, Marcelo Tosatti wrote:
> On Tue, Dec 15, 2009 at 01:20:38PM +0200, Gleb Natapov wrote:
> > On Mon, Dec 14, 2009 at 06:36:37PM -0200, Marcelo Tosatti wrote:
> > >
> > > So that the vcpu state is initialized, from vcpu thread context, after
> > > machine initialization is settled.
> > >
> > > This allows to revert apic_init's apic_reset call. apic_reset now
> > > happens through system_reset, similarly to qemu upstream.
> > >
> > This patch essentially revers commit 898c51c3. This commit fixes two
> > races. First race is like this:
> >
> > vcpu0 vcpu1
> >
> > starts running
> > loads lapic state into kernel
> > sends event to vcpu1
> > starts running
> > loads lapic state into kernel
> > overwrites event from vcpu0
> >
> > At the time 898c51c3 was committed the race was easily reproducible
> > by starting VM with 16 cpus + seabios. Sometimes some vcpus lost INIT/SIPI
> > events. Now I am not able to reproduce it even with this patch applied,
> > so something else changed, but it doesn't make the race non existent or
> > acceptable.
>
> Note qemu_kvm_load_lapic depends on env->created set (kvm_vcpu_inited),
Uhh. What about doing this:
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 44e8b75..fa6db8e 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1920,12 +1920,12 @@ static void *ap_main_loop(void *_env)
pthread_mutex_lock(&qemu_mutex);
cpu_single_env = env;
+ current_env->created = 1;
kvm_arch_init_vcpu(env);
kvm_arch_load_regs(env);
/* signal VCPU creation */
- current_env->created = 1;
pthread_cond_signal(&qemu_vcpu_cond);
/* and wait for machine initialization */
> so having init_vcpu+load_regs before signalling vcpu creation did not
> fix this one (but yeah, thanks for the reminder on the races).
>
> > The second race is during machine start after migration. The race is
> > between event loop and vcpu:
> >
> > event loop vcpu
> >
> > starts running
> > gets RTC timer event
> > sends interrupt to vcpu
> > starts running
> > loads lapic state into kernel
> > overwrites interrupt from RTC
> >
> >
> > In short vcpu state that can be influenced by sources outside vcpu thread
> > itself should be uploaded into the kernel before signaling qemu_system_ready
> > condition.
> >
> > --
> > Gleb.
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
