From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:55134) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QxZW3-0000AZ-B2 for qemu-devel@nongnu.org; Sun, 28 Aug 2011 03:10:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QxZW2-0004By-3p for qemu-devel@nongnu.org; Sun, 28 Aug 2011 03:10:47 -0400 Received: from mail-qw0-f45.google.com ([209.85.216.45]:45301) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QxZW1-0004Bu-Tg for qemu-devel@nongnu.org; Sun, 28 Aug 2011 03:10:46 -0400 Received: by qwj8 with SMTP id 8so3219764qwj.4 for ; Sun, 28 Aug 2011 00:10:45 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4E58FC3F.6080809@web.de> References: <4E58FC3F.6080809@web.de> From: Blue Swirl Date: Sun, 28 Aug 2011 07:10:25 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] pc: Clean up PIC-to-APIC IRQ path List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Lucas Meneghel Rodrigues , Anthony Liguori , Marcelo Tosatti , qemu-devel , Gerd Hoffmann , Avi Kivity On Sat, Aug 27, 2011 at 2:16 PM, Jan Kiszka wrote: > From: Jan Kiszka > > 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 re-inject its IRQ after the CPU picked up an event from the APIC. > > Adds the proper state tracking so that we can already re-assert the CPU > IRQ at APIC level if there is a pending PIC IRQ. This allows to remove > all the old workarounds. > > The patch also fixes some failures of the kvm unit tests apic and > eventinj by enabling a proper CPU IRQ deassert when the guest masks some > pending IRQs at PIC level. > > Signed-off-by: Jan Kiszka > --- > > It turned out that this patch from a larger cleanup series has no > dependencies and can be applied directly to master to fix the observed > bug. > > =C2=A0hw/apic.c =C2=A0| =C2=A0 =C2=A04 +++- > =C2=A0hw/i8259.c | =C2=A0 10 ++-------- > =C2=A0hw/pc.c =C2=A0 =C2=A0| =C2=A0 =C2=A03 --- > =C2=A0hw/pc.h =C2=A0 =C2=A0| =C2=A0 =C2=A01 - > =C2=A04 files changed, 5 insertions(+), 13 deletions(-) > > diff --git a/hw/apic.c b/hw/apic.c > index d8f56c8..22ad635 100644 > --- a/hw/apic.c > +++ b/hw/apic.c > @@ -104,6 +104,7 @@ struct APICState { > =C2=A0 =C2=A0 QEMUTimer *timer; > =C2=A0 =C2=A0 int sipi_vector; > =C2=A0 =C2=A0 int wait_for_sipi; > + =C2=A0 =C2=A0int pic_level; > =C2=A0}; > > =C2=A0static APICState *local_apics[MAX_APICS + 1]; > @@ -186,6 +187,7 @@ void apic_deliver_pic_intr(DeviceState *d, int level) > =C2=A0{ > =C2=A0 =C2=A0 APICState *s =3D DO_UPCAST(APICState, busdev.qdev, d); > > + =C2=A0 =C2=A0s->pic_level =3D level; > =C2=A0 =C2=A0 if (level) { > =C2=A0 =C2=A0 =C2=A0 =C2=A0 apic_local_deliver(s, APIC_LVT_LINT0); > =C2=A0 =C2=A0 } else { > @@ -397,7 +399,7 @@ static void apic_update_irq(APICState *s) > =C2=A0 =C2=A0 if (!(s->spurious_vec & APIC_SV_ENABLE)) { > =C2=A0 =C2=A0 =C2=A0 =C2=A0 return; > =C2=A0 =C2=A0 } > - =C2=A0 =C2=A0if (apic_irq_pending(s) > 0) { > + =C2=A0 =C2=A0if (apic_irq_pending(s) > 0 || s->pic_level) { > =C2=A0 =C2=A0 =C2=A0 =C2=A0 cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD)= ; > =C2=A0 =C2=A0 } > =C2=A0} > diff --git a/hw/i8259.c b/hw/i8259.c > index c0b96ab..cc6f76b 100644 > --- a/hw/i8259.c > +++ b/hw/i8259.c > @@ -144,8 +144,7 @@ static int pic_get_irq(PicState *s) > > =C2=A0/* raise irq to CPU if necessary. must be called every time the act= ive > =C2=A0 =C2=A0irq may change */ > -/* XXX: should not export it, but it is needed for an APIC kludge */ > -void pic_update_irq(PicState2 *s) > +static void pic_update_irq(PicState2 *s) > =C2=A0{ > =C2=A0 =C2=A0 int irq2, irq; > > @@ -172,14 +171,9 @@ void pic_update_irq(PicState2 *s) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 printf("pic: cpu_interrupt\n"); > =C2=A0#endif > =C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_irq_raise(s->parent_irq); > - =C2=A0 =C2=A0} > - > -/* all targets should do this rather than acking the IRQ in the cpu */ > -#if defined(TARGET_MIPS) || defined(TARGET_PPC) || defined(TARGET_ALPHA) > - =C2=A0 =C2=A0else { > + =C2=A0 =C2=A0} else { Nice cleanup, this was pretty ugly. Isn't it possible to compile the device in hwlib now? That should save about 11 compiles for the full build since it is used by many targets. > =C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_irq_lower(s->parent_irq); > =C2=A0 =C2=A0 } > -#endif > =C2=A0} > > =C2=A0#ifdef DEBUG_IRQ_LATENCY > diff --git a/hw/pc.c b/hw/pc.c > index 263fb1a..b7b5d6f 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -156,9 +156,6 @@ int cpu_get_pic_interrupt(CPUState *env) > > =C2=A0 =C2=A0 intno =3D apic_get_interrupt(env->apic_state); > =C2=A0 =C2=A0 if (intno >=3D 0) { > - =C2=A0 =C2=A0 =C2=A0 =C2=A0/* set irq request if a PIC irq is still pen= ding */ > - =C2=A0 =C2=A0 =C2=A0 =C2=A0/* XXX: improve that */ > - =C2=A0 =C2=A0 =C2=A0 =C2=A0pic_update_irq(isa_pic); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 return intno; > =C2=A0 =C2=A0 } > =C2=A0 =C2=A0 /* read the irq from the PIC */ > diff --git a/hw/pc.h b/hw/pc.h > index dae736e..f5ff4c0 100644 > --- a/hw/pc.h > +++ b/hw/pc.h > @@ -65,7 +65,6 @@ void pic_set_irq(int irq, int level); > =C2=A0void pic_set_irq_new(void *opaque, int irq, int level); > =C2=A0qemu_irq *i8259_init(qemu_irq parent_irq); > =C2=A0int pic_read_irq(PicState2 *s); > -void pic_update_irq(PicState2 *s); > =C2=A0uint32_t pic_intack_read(PicState2 *s); > =C2=A0void pic_info(Monitor *mon); > =C2=A0void irq_info(Monitor *mon); > >