On Tue, Dec 15, 2009 at 02:33:27PM +0200, Gleb Natapov wrote: > 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 */ load_lapic ioctl is called from pc_new_cpu -> apic_reset, so its alright. No need for the patch above, then. -- 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
