From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LuoWW-0006HT-9K for qemu-devel@nongnu.org; Fri, 17 Apr 2009 09:54:32 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LuoWR-0006EN-A4 for qemu-devel@nongnu.org; Fri, 17 Apr 2009 09:54:31 -0400 Received: from [199.232.76.173] (port=43691 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LuoWR-0006EI-65 for qemu-devel@nongnu.org; Fri, 17 Apr 2009 09:54:27 -0400 Received: from mx2.redhat.com ([66.187.237.31]:51446) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LuoWQ-0003g3-Jg for qemu-devel@nongnu.org; Fri, 17 Apr 2009 09:54:26 -0400 Date: Fri, 17 Apr 2009 10:53:59 -0300 From: Marcelo Tosatti Message-ID: <20090417135359.GA23731@amt.cnet> References: <1239945321-3903-1-git-send-email-glommer@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1239945321-3903-1-git-send-email-glommer@redhat.com> Subject: [Qemu-devel] Re: [PATCH] return default values for apic probe functions. Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Glauber Costa Cc: aliguori@us.ibm.com, avi@redhat.com, kvm@vger.kernel.org, qemu-devel@nongnu.org Hi Glauber, On Fri, Apr 17, 2009 at 01:15:21AM -0400, Glauber Costa wrote: > As KVM cpus runs on threads, it is possible that > we call kvm_load_registers() from a cpu thread, while the > apic has not yet fully initialized. kvm_load_registers() is called > from ap_main_loop. > > This is not a problem when we're starting the whole machine together, > but is a problem for hotplug, since we don't have the protection > of the locks that protect machine initialization. Currently, some executions > of cpu hotplug on rainy sundays fail with a segfault. /* and wait for machine initialization */ while (!qemu_system_ready) qemu_cond_wait(&qemu_system_cond); pthread_mutex_unlock(&qemu_mutex); Shouldnt this cover the cpu hotplug case too? Perhaps have: /* wait for machine initialization */ while (!qemu_system_ready) qemu_cond_wait(&qemu_system_cond); /* wait for vcpu initialization */ while (!env->initialized) qemu_cond_wait(&qemu_system_cond); pthread_mutex_unlock(&qemu_mutex); And then set env->initialized when the cpu is good to go. Because there could be other dependencies other than APIC initialization, for eg in pc_new_cpu if (cpu != 0) env->halted = 1; > Moving apic initialization to before kvm_init_vpcu proved fruitful, > as there are some dependencies involved. (kvm irqchip would fail to > initialize). > > This patch provides default values to be used for tpr and apic_base, > that will be returned when the apic is not yet properly initialized. > It is aimed at kvm, where the problem exists, but it could equally be > used for qemu too, if there is agreement. > > Signed-off-by: Glauber Costa > --- > qemu/hw/apic.c | 12 ++++++++++-- > 1 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/qemu/hw/apic.c b/qemu/hw/apic.c > index b926508..06fb9b5 100644 > --- a/qemu/hw/apic.c > +++ b/qemu/hw/apic.c > @@ -301,7 +301,12 @@ uint64_t cpu_get_apic_base(CPUState *env) > #ifdef DEBUG_APIC > printf("cpu_get_apic_base: %016" PRIx64 "\n", (uint64_t)s->apicbase); > #endif > - return s->apicbase; > + if (s) { > + return s->apicbase; > + } > + else { > + return 0xfee00000 | MSR_IA32_APICBASE_ENABLE; > + } > } > > void cpu_set_apic_tpr(CPUX86State *env, uint8_t val) > @@ -314,7 +319,10 @@ void cpu_set_apic_tpr(CPUX86State *env, uint8_t val) > uint8_t cpu_get_apic_tpr(CPUX86State *env) > { > APICState *s = env->apic_state; > - return s->tpr >> 4; > + if (s) > + return s->tpr >> 4; > + else > + return 0; > } > > /* return -1 if no bit is set */ > -- > 1.5.6.6 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html