From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JwbcH-0006jK-JV for qemu-devel@nongnu.org; Thu, 15 May 2008 07:27:21 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JwbcF-0006j5-Rg for qemu-devel@nongnu.org; Thu, 15 May 2008 07:27:20 -0400 Received: from [199.232.76.173] (port=44816 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JwbcF-0006j2-Lv for qemu-devel@nongnu.org; Thu, 15 May 2008 07:27:19 -0400 Received: from smtp.syd.people.net.au ([218.214.225.98]:52678) by monty-python.gnu.org with smtp (Exim 4.60) (envelope-from ) id 1JwbcE-0004t4-S1 for qemu-devel@nongnu.org; Thu, 15 May 2008 07:27:19 -0400 Received: from hendrix (hendrix [192.168.200.99]) by hendrix.mega-nerd.net (Postfix) with SMTP id 07737ACA78 for ; Thu, 15 May 2008 21:27:16 +1000 (EST) Date: Thu, 15 May 2008 21:27:15 +1000 From: Erik de Castro Lopo Subject: Re: [Qemu-devel] [Patch] Add handling of edge triggered interrupts in function pic_irq_request. Message-Id: <20080515212715.03d4ad29.mle+tools@mega-nerd.com> In-Reply-To: <20080515203718.0983979e.mle+tools@mega-nerd.com> References: <20080515193204.cff99c8a.mle+tools@mega-nerd.com> <20080515203718.0983979e.mle+tools@mega-nerd.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Erik de Castro Lopo wrote: > Sorry, this patch needs more testing. Ah, I had one crash of the guest OS, but I now realize that was due to another bug related to rebooting the guest OS. I plan on tracking down this bug later. For now, as long as I use the -no-reboot option I can install XP64 under qemu-system-x86_64. I should probably explain a little about how I came up with this fix. As had been noted on the qemu-user forum: http://qemu-forum.ipi.fi/viewtopic.php?f=9&t=4329 somewhere between version 0.9.0 and 0.9.1, support for running XP64 as a guest OS was broken. For me, this meant that installing XP64 under qemu would hang very early in the install process. Over the last two nights, I did a binary search from SVN revision 3000 to 4000 and found that XP64 stopped working at SVN 3371. Inspecting the differences between 3370 and 3371 I zeroed in on this difference: ------- Following diff for illustration purposes only ------- --- hw/pc.c (revision 3370) +++ hw/pc.c (revision 3371) @@ -100,10 +103,8 @@ static void pic_irq_request(void *opaque, int irq, int level) { CPUState *env = opaque; - if (level) + if (level && apic_accept_pic_intr(env)) cpu_interrupt(env, CPU_INTERRUPT_HARD); - else - cpu_reset_interrupt(env, CPU_INTERRUPT_HARD); } /* PC cmos mappings */ --------- Above diff for illustration purposes only --------- and I noted that patch 3371 was removing call to cpu_reset_interrupt for the case when the interrupt was not level triggered (is that right?). To me, removing this interrupt handling didn't seem right. I therefore applied the following patch to 3371 to get the xp64 install process past the place where is was hanging before. ------- Following diff for illustration purposes only ------- --- hw/pc.c 2008-05-15 01:00:12 +0000 +++ hw/pc.c 2008-05-15 01:00:53 +0000 @@ -105,6 +105,8 @@ CPUState *env = opaque; if (level && apic_accept_pic_intr(env)) cpu_interrupt(env, CPU_INTERRUPT_HARD); + else + cpu_reset_interrupt(env, CPU_INTERRUPT_HARD); } /* PC cmos mappings */ --------- Above diff for illustration purposes only --------- Unfortunately, the codebase has moved on since 3371 and this patch did not apply to current SVN head. I then did another binary search to find where my patch could no longer be applied. That was at SVN revision 4207. The diff between my patched SVN revision 4206 and SVN revision 4207 is below. ------- Following diff for illustration purposes only ------- static void pic_irq_request(void *opaque, int irq, int level) { - CPUState *env = opaque; - if (level && apic_accept_pic_intr(env)) - cpu_interrupt(env, CPU_INTERRUPT_HARD); - else - cpu_reset_interrupt(env, CPU_INTERRUPT_HARD); + CPUState *env = first_cpu; + + if (!level) + return; + + while (env) { + if (apic_accept_pic_intr(env)) + apic_local_deliver(env, APIC_LINT0); + env = env->next_cpu; + } } --------- Above diff for illustration purposes only --------- Comparing my patched 4206 with 4207 resulted in the following patch. --------- This is the patch I'd like to see applied --------- diff --git a/hw/pc.c b/hw/pc.c index c92384c..65ea5c6 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -118,8 +118,10 @@ static void pic_irq_request(void *opaque, int irq, int level) { CPUState *env = first_cpu; - if (!level) + if (!level) { + cpu_reset_interrupt(env, CPU_INTERRUPT_HARD); return; + } while (env) { if (apic_accept_pic_intr(env)) --------- This is the patch I'd like to see applied --------- Cheers, Erik -- ----------------------------------------------------------------- Erik de Castro Lopo ----------------------------------------------------------------- "If Java had true garbage collection, most programs would delete themselves upon execution." -- Robert Sewell