From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1Luo1X-00051q-Uu
	for qemu-devel@nongnu.org; Fri, 17 Apr 2009 09:22:32 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1Luo1S-00050U-1S
	for qemu-devel@nongnu.org; Fri, 17 Apr 2009 09:22:30 -0400
Received: from [199.232.76.173] (port=45824 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Luo1R-00050O-Od
	for qemu-devel@nongnu.org; Fri, 17 Apr 2009 09:22:25 -0400
Received: from mail-qy0-f111.google.com ([209.85.221.111]:55189)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <glommer@gmail.com>) id 1Luo1Q-0007Eh-Tz
	for qemu-devel@nongnu.org; Fri, 17 Apr 2009 09:22:25 -0400
Received: by qyk9 with SMTP id 9so1908676qyk.4
	for <qemu-devel@nongnu.org>; Fri, 17 Apr 2009 06:22:20 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <49E82825.602@web.de>
References: <1239945321-3903-1-git-send-email-glommer@redhat.com>
	<49E82825.602@web.de>
Date: Fri, 17 Apr 2009 10:22:14 -0300
Message-ID: <5d6222a80904170622g1977ec37s8872800a12bfafbe@mail.gmail.com>
From: Glauber Costa <glommer@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
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: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Jan Kiszka <jan.kiszka@web.de>
Cc: qemu-devel@nongnu.org, Glauber Costa <glommer@redhat.com>, avi@redhat.com, kvm@vger.kernel.org, aliguori@us.ibm.com

> 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 re=
spond
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.

I'm not sure about what the manual says (will check now), but I
believe if we ever try to
read from apic, we should return a meaningful value. Can you verify if
this also works for your
test case?

>
>>
>> =C2=A0void cpu_set_apic_tpr(CPUX86State *env, uint8_t val)
>> @@ -314,7 +319,10 @@ void cpu_set_apic_tpr(CPUX86State *env, uint8_t val=
)
>> =C2=A0uint8_t cpu_get_apic_tpr(CPUX86State *env)
>> =C2=A0{
>> =C2=A0 =C2=A0 =C2=A0APICState *s =3D env->apic_state;
>> - =C2=A0 =C2=A0return s->tpr >> 4;
>> + =C2=A0 =C2=A0if (s)
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0return s->tpr >> 4;
>> + =C2=A0 =C2=A0else
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
>> =C2=A0}
>>
>> =C2=A0/* return -1 if no bit is set */
>
> This is already upstream.

Yeah, and the rest of your patch is totally ok for me.


--=20
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."