From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:35873) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QzujZ-0007Jr-A5 for qemu-devel@nongnu.org; Sat, 03 Sep 2011 14:14:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QzujX-0005By-SD for qemu-devel@nongnu.org; Sat, 03 Sep 2011 14:14:25 -0400 Received: from fmmailgate01.web.de ([217.72.192.221]:37135) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QzujX-0005Bn-Cl for qemu-devel@nongnu.org; Sat, 03 Sep 2011 14:14:23 -0400 Message-ID: <4E626E76.5030504@web.de> Date: Sat, 03 Sep 2011 20:14:14 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <4E58FC3F.6080809@web.de> <4E5F4B81.2050905@siemens.com> <4E620CD4.7040205@web.de> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig6C21571405B2844D9AA2EF1F" Sender: jan.kiszka@web.de Subject: Re: [Qemu-devel] [PATCH v2] pc: Fix and clean up PIC-to-APIC IRQ path List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Lucas Meneghel Rodrigues , Anthony Liguori , Marcelo Tosatti , qemu-devel , Avi Kivity , Gerd Hoffmann This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig6C21571405B2844D9AA2EF1F Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 2011-09-03 13:37, Blue Swirl wrote: > On Sat, Sep 3, 2011 at 11:17 AM, Jan Kiszka wrote: >> On 2011-09-03 10:58, Blue Swirl wrote: >>> On Thu, Sep 1, 2011 at 9:08 AM, Jan Kiszka w= rote: >>>> The master PIC is connected to the LINTIN0 of the APICs. As the APIC= >>>> currently does not track the state of that line, we have to ask the = PIC >>>> to reinject its IRQ after the CPU picked up an event from the APIC. >>>> >>>> This introduces pic_get_output to read the master PIC IRQ line state= >>>> without changing it. The APIC uses this function to decide if a PIC = IRQ >>>> should be reinjected on apic_update_irq. This reflects better how th= e >>>> real hardware works. >>>> >>>> The patch fixes some failures of the kvm unit tests apic and eventin= j by >>>> allowing to enable the proper CPU IRQ deassertion when the guest mas= ks >>>> some pending IRQs at PIC level. >>>> >>>> Signed-off-by: Jan Kiszka >>>> --- >>>> >>>> Changes in v2: >>>> - Avoid adding pic_level to the APIC state, obtain PIC output via >>>> pic_get_level instead >>>> - Do not reassert PIC interrupt if APIC is not accepting it >>>> - Use apic_deliver_pic_intr for reassertion to ensure correct >>>> processing >>>> >>>> This is not as nice as the previous version /wrt the interaction of = PIC >>>> and APIC. But it avoids breaking the APIC vmstate for the sake of >>>> internal changes, also keeping it compatible with the upcoming KVM >>>> in-kernel APIC (that allows no easy pic_level state extraction). The= >>>> interconnection between PIC and APIC may look nicer in the future wi= th >>>> QOM. And in the end this just reflects the "beauty" of the x86 >>>> architecture. >>>> >>>> hw/apic.c | 4 ++++ >>>> hw/i8259.c | 15 +++++++-------- >>>> hw/pc.c | 3 --- >>>> hw/pc.h | 2 +- >>>> 4 files changed, 12 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/hw/apic.c b/hw/apic.c >>>> index d8f56c8..8289eef 100644 >>>> --- a/hw/apic.c >>>> +++ b/hw/apic.c >>>> @@ -23,6 +23,7 @@ >>>> #include "host-utils.h" >>>> #include "sysbus.h" >>>> #include "trace.h" >>>> +#include "pc.h" >>>> >>>> /* APIC Local Vector Table */ >>>> #define APIC_LVT_TIMER 0 >>>> @@ -399,6 +400,9 @@ static void apic_update_irq(APICState *s) >>>> } >>>> if (apic_irq_pending(s) > 0) { >>>> cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD); >>>> + } else if (apic_accept_pic_intr(&s->busdev.qdev) && >>>> + pic_get_output(isa_pic)) { >>> >>> This is indeed ugly. Why doesn't APIC track PIC output? >> >> For the reasons explained above. >=20 > Breaking vmstate compatibility? Don't we have all kinds of > compatibility stuff for this? As I said, we do not have access to some pic_level equivalent with the in-kernel APIC model. And I don't want to break vmstate compatibility between user and kernel space models needlessly. >=20 > I'm not opposed to the patch as is, it's still a cleanup, but I wish > this ugliness could be avoided. It's primarily a bug fix now. And the ugliness level should drop again when IRQ management will be reworked. Jan --------------enig6C21571405B2844D9AA2EF1F Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk5ibnwACgkQitSsb3rl5xSGFQCfavQn8WU3tV8uCtJh8a9sj7Pc BKcAn3EbfoosZZ2JtC8j70aDq18wsN5d =rpYe -----END PGP SIGNATURE----- --------------enig6C21571405B2844D9AA2EF1F--