From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KTCLp-0007aV-7N for qemu-devel@nongnu.org; Wed, 13 Aug 2008 05:09:05 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KTCLo-0007aF-3p for qemu-devel@nongnu.org; Wed, 13 Aug 2008 05:09:04 -0400 Received: from [199.232.76.173] (port=41651 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KTCLn-0007Zz-ET for qemu-devel@nongnu.org; Wed, 13 Aug 2008 05:09:03 -0400 Received: from fmmailgate03.web.de ([217.72.192.234]:55981) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KTCLm-0005sx-SR for qemu-devel@nongnu.org; Wed, 13 Aug 2008 05:09:03 -0400 Message-ID: <48A2A4AB.6050601@web.de> Date: Wed, 13 Aug 2008 11:08:59 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <48A297F0.9000700@web.de> In-Reply-To: <48A297F0.9000700@web.de> Content-Type: text/plain; charset=ISO-8859-1 Sender: jan.kiszka@web.de Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] Re: Breakage with local APIC routing Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Johannes Schindelin Cc: qemu-devel@nongnu.org Jan Kiszka wrote: > Johannes Schindelin wrote: >> Hi, >> >> due to the change in revision 3371 (well, at that time, CVS was used,=20 >> which was no better than Subversion) installation of win64 is broken i= n=20 >> QEmu. The commit message reads like this: >> >> Don't route PIC interrupts through the local APIC if the local=20 >> APIC config says so. By Ari Kivity. >> >> A bit of research showed that the patch was actually originally from Q= ing=20 >> He, but he told me privately that the part that actually broke win64 (= the=20 >> removal of the call to cpu_reset_interrupt(), as opposed to moving tha= t=20 >> call into the "else" condition) was not part of his patch. >> >> Unfortunately, a lot has been done to the APIC handling in the meantim= e,=20 >> so it is not a simple matter of a revert. >> >> Being a complete idiot when it comes to APICs, I have no clue how to f= ix=20 >> the issue. >> >> However, I am quite willing to test whatever patch is thrown at me. >> >> Can somebody help? >=20 > I recalled some earlier post on this which claimed to fix the issue and > found it in the archive: >=20 > http://permalink.gmane.org/gmane.comp.emulators.qemu/25415 >=20 > However, no one replied on this, and I'm right now not sure if the fix > is OK for all cases. But it is a starting point for new discussion abou= t > what is actually borken here. This might not be the last word, but my feeling that it's closer to the actual issue than the older fix. Please give it a try. Comments welcome. -------- Ensure that PIC-delivered IRQs are properly de-asserted in case the APIC is in EXTINT or FIXED mode (with level-triggering selected) on LINT0. Should fix Win64 boot issues. This patch also cleans up a bit the interface between PIC and APIC, making apic_local_deliver private again. Signed-off-by: Jan Kiszka --- hw/apic.c | 23 ++++++++++++++++++++++- hw/pc.c | 5 +---- hw/pc.h | 4 +--- 3 files changed, 24 insertions(+), 8 deletions(-) Index: b/hw/apic.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- a/hw/apic.c +++ b/hw/apic.c @@ -166,7 +166,7 @@ static inline void reset_bit(uint32_t *t tab[i] &=3D ~mask; } =20 -void apic_local_deliver(CPUState *env, int vector) +static void apic_local_deliver(CPUState *env, int vector) { APICState *s =3D env->apic_state; uint32_t lvt =3D s->lvt[vector]; @@ -197,6 +197,27 @@ void apic_local_deliver(CPUState *env, i } } =20 +void apic_deliver_pic_intr(CPUState *env, int level) +{ + if (level) + apic_local_deliver(env, APIC_LVT_LINT0); + else { + APICState *s =3D env->apic_state; + uint32_t lvt =3D s->lvt[APIC_LVT_LINT0]; + + switch ((lvt >> 8) & 7) { + case APIC_DM_FIXED: + if (!(lvt & APIC_LVT_LEVEL_TRIGGER)) + break; + reset_bit(s->irr, lvt & 0xff); + /* fall through */ + case APIC_DM_EXTINT: + cpu_reset_interrupt(env, CPU_INTERRUPT_HARD); + break; + } + } +} + #define foreach_apic(apic, deliver_bitmask, code) \ {\ int __i, __j, __mask;\ Index: b/hw/pc.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- a/hw/pc.c +++ b/hw/pc.c @@ -118,12 +118,9 @@ static void pic_irq_request(void *opaque { CPUState *env =3D first_cpu; =20 - if (!level) - return; - while (env) { if (apic_accept_pic_intr(env)) - apic_local_deliver(env, APIC_LINT0); + apic_deliver_pic_intr(env, level); env =3D env->next_cpu; } } Index: b/hw/pc.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- a/hw/pc.h +++ b/hw/pc.h @@ -40,11 +40,9 @@ void irq_info(void); /* APIC */ typedef struct IOAPICState IOAPICState; =20 -#define APIC_LINT0 3 - int apic_init(CPUState *env); int apic_accept_pic_intr(CPUState *env); -void apic_local_deliver(CPUState *env, int vector); +void apic_deliver_pic_intr(CPUState *env, int level); int apic_get_interrupt(CPUState *env); IOAPICState *ioapic_init(void); void ioapic_set_irq(void *opaque, int vector, int level);