From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH 1/2] Complete cpu initialization before signaling main thread. Date: Tue, 13 Oct 2009 20:34:21 +0200 Message-ID: <20091013183421.GC25891@redhat.com> References: <1255436240-994-1-git-send-email-gleb@redhat.com> <20091013181908.GA14313@amt.cnet> <20091013182348.GB14313@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org To: Marcelo Tosatti Return-path: Received: from mx1.redhat.com ([209.132.183.28]:17700 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760891AbZJMSet (ORCPT ); Tue, 13 Oct 2009 14:34:49 -0400 Received: from int-mx03.intmail.prod.int.phx2.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n9DIYONX032036 for ; Tue, 13 Oct 2009 14:34:24 -0400 Content-Disposition: inline In-Reply-To: <20091013182348.GB14313@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: 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.