From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Luoqb-00085t-5y for qemu-devel@nongnu.org; Fri, 17 Apr 2009 10:15:17 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LuoqW-00082z-29 for qemu-devel@nongnu.org; Fri, 17 Apr 2009 10:15:16 -0400 Received: from [199.232.76.173] (port=60455 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LuoqV-00082w-VI for qemu-devel@nongnu.org; Fri, 17 Apr 2009 10:15:11 -0400 Received: from gecko.sbs.de ([194.138.37.40]:22205) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LuoqV-0008NI-Ha for qemu-devel@nongnu.org; Fri, 17 Apr 2009 10:15:11 -0400 Message-ID: <49E88EE4.6040307@siemens.com> Date: Fri, 17 Apr 2009 16:15:00 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1239945321-3903-1-git-send-email-glommer@redhat.com> <49E82825.602@web.de> <5d6222a80904170622g1977ec37s8872800a12bfafbe@mail.gmail.com> <5d6222a80904170640p7c50950fla5ebb65e0b1db138@mail.gmail.com> In-Reply-To: <5d6222a80904170640p7c50950fla5ebb65e0b1db138@mail.gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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, kvm@vger.kernel.org, Glauber Costa , qemu-devel@nongnu.org, Jan Kiszka , avi@redhat.com Glauber Costa wrote: > On Fri, Apr 17, 2009 at 10:22 AM, Glauber Costa wrote: >>> Even on sunny days, this collides with QEMU commit #7048. :) >>> >>> Does Intel specify what non-existent MSRs should return, ie. is your >>> version still correct if !s->apicbase means that there is actually no >>> APIC? And does kvm depend on the default base? If so, I would say: >>> provide a patch against upstream. >> hummm, I missed this one going in. >> But sadly, your patch also breaks cpu hotplug. Not a segfault anymore, but the >> VM will freeze instead of shutting down, if we ask too. It does not even respond >> to ^C anymore. >> >> By leaving your patch as is, and changing the apic base return to >> >> return s ? s->apicbase : (0xfee00000 | MSR_IA32_APICBASE_ENABLE); >> >> fixes the issue. >> > After reading the manual, my understanding is that only the flag must > be set. I tried, > and in fact: > > return s ? s->apicbase : MSR_IA32_APICBASE_ENABLE; > > still fixes it. > If it works for you, I believe this is a good solution, and will send > a descriptive patch. > If we ever read the apic as disabled, we will have problems enabling > it again. So for > my test case, kvm should never see a disabled apic. > > For yours, you'll still see the apic base address as zero. > > what do you think? > My use case (you can try it yourself: -M isapc) will likely already be fine if there is no segv on accidentally reading that msr. I was more concerned about the correct value according to the spec - if that case isn't simply "undefined". On the other hand, I didn't get the precise race yet, and my feeling about this patch as a fix for something else than an inexistent apic is not that good. But it's just a feeling. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux