From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=54165 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1OLAVh-0000nT-5e
	for qemu-devel@nongnu.org; Sun, 06 Jun 2010 03:43:11 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <blauwirbel@gmail.com>) id 1OLAVf-00027J-Gc
	for qemu-devel@nongnu.org; Sun, 06 Jun 2010 03:43:08 -0400
Received: from mail-pv0-f173.google.com ([74.125.83.173]:37338)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <blauwirbel@gmail.com>) id 1OLAVf-00027C-8N
	for qemu-devel@nongnu.org; Sun, 06 Jun 2010 03:43:07 -0400
Received: by pvg12 with SMTP id 12so1166065pvg.4
	for <qemu-devel@nongnu.org>; Sun, 06 Jun 2010 00:43:06 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <4C0B4FE1.9070608@web.de>
References: <AANLkTikMjEneitQJy0ABsx_sSYxCC3R2K95hXZ-J37zs@mail.gmail.com>
	<4C0B4FE1.9070608@web.de>
From: Blue Swirl <blauwirbel@gmail.com>
Date: Sun, 6 Jun 2010 07:42:46 +0000
Message-ID: <AANLkTilpuP5o5Yuz097qLChl2k9p_-wPCVVloQGGW9HZ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: [Qemu-devel] Re: [PATCH 6/6] apic: avoid using CPUState internals
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.nongnu.org/archive/html/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 <qemu-devel@nongnu.org>

On Sun, Jun 6, 2010 at 7:36 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> Blue Swirl wrote:
>> Use only an opaque CPUState pointer and move the actual CPUState
>> contents handling to cpu.h and cpuid.c.
>>
>> Set env->halted in pc.c and add a function to get the local APIC state
>> of the current CPU for the MMIO.
>>
>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>> ---
>> =C2=A0hw/apic.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 40 +++++++++=
++++++-------------------------
>> =C2=A0hw/apic.h =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A09 ++++=
++++-
>> =C2=A0hw/pc.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 12 ++++=
+++++++-
>> =C2=A0target-i386/cpu.h =C2=A0 | =C2=A0 27 ++++++++++++++++-----------
>> =C2=A0target-i386/cpuid.c | =C2=A0 =C2=A06 ++++++
>> =C2=A05 files changed, 56 insertions(+), 38 deletions(-)
>>
>> diff --git a/hw/apic.c b/hw/apic.c
>> index 91c8d93..332c66e 100644
>> --- a/hw/apic.c
>> +++ b/hw/apic.c
>> @@ -95,7 +95,7 @@
>> =C2=A0#define MSI_ADDR_SIZE =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 0x100000
>>
>> =C2=A0struct APICState {
>> - =C2=A0 =C2=A0CPUState *cpu_env;
>> + =C2=A0 =C2=A0void *cpu_env;
>> =C2=A0 =C2=A0 =C2=A0uint32_t apicbase;
>> =C2=A0 =C2=A0 =C2=A0uint8_t id;
>> =C2=A0 =C2=A0 =C2=A0uint8_t arb_id;
>> @@ -320,7 +320,7 @@ void cpu_set_apic_base(APICState *s, uint64_t val)
>> =C2=A0 =C2=A0 =C2=A0/* if disabled, cannot be enabled again */
>> =C2=A0 =C2=A0 =C2=A0if (!(val & MSR_IA32_APICBASE_ENABLE)) {
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0s->apicbase &=3D ~MSR_IA32_APICBASE_EN=
ABLE;
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0s->cpu_env->cpuid_features &=3D ~CPUID_APIC=
;
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0cpu_clear_apic_feature(s->cpu_env);
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0s->spurious_vec &=3D ~APIC_SV_ENABLE;
>> =C2=A0 =C2=A0 =C2=A0}
>> =C2=A0}
>> @@ -508,8 +508,6 @@ void apic_init_reset(APICState *s)
>> =C2=A0 =C2=A0 =C2=A0s->initial_count_load_time =3D 0;
>> =C2=A0 =C2=A0 =C2=A0s->next_time =3D 0;
>> =C2=A0 =C2=A0 =C2=A0s->wait_for_sipi =3D 1;
>> -
>> - =C2=A0 =C2=A0s->cpu_env->halted =3D !(s->apicbase & MSR_IA32_APICBASE_=
BSP);
>
> We are now lacking 'halted' initialization after system reset. Could be
> addressed by a special reset handler in hw/pc.c, I guess.

Good catch, I forgot to do that.