From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50572) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VZ0i9-0008K6-Tf for qemu-devel@nongnu.org; Wed, 23 Oct 2013 11:51:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VZ0i4-000386-2d for qemu-devel@nongnu.org; Wed, 23 Oct 2013 11:51:05 -0400 Received: from mail-wg0-f44.google.com ([74.125.82.44]:54658) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VZ0i3-000380-SM for qemu-devel@nongnu.org; Wed, 23 Oct 2013 11:50:59 -0400 Received: by mail-wg0-f44.google.com with SMTP id n12so1007288wgh.11 for ; Wed, 23 Oct 2013 08:50:59 -0700 (PDT) Date: Wed, 23 Oct 2013 16:50:57 +0100 From: Christoffer Dall Message-ID: <20131023155057.GC60827@lvm> References: <1380229386-24166-1-git-send-email-christoffer.dall@linaro.org> <1380229386-24166-4-git-send-email-christoffer.dall@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [RFC PATCH v2 3/6] hw: arm_gic: Keep track of SGI sources List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Patch Tracking , QEMU Developers , "kvmarm@lists.cs.columbia.edu" On Mon, Oct 14, 2013 at 05:33:38PM +0100, Peter Maydell wrote: > On 14 October 2013 16:36, Peter Maydell wrote: [...] > > Tangentially, I notice that we don't correctly handle > the PENDING bit for level triggered interrupts, since > we do: > > /* Clear pending flags for both level and edge triggered interrupts. > Level triggered IRQs will be reasserted once they become inactive. */ > gic_clear_pending(s, new_irq, GIC_TEST_MODEL(new_irq) ? ALL_CPU_MASK : cm, > GIC_SGI_SRC(new_irq, cpu)); > > in gic_acknowledge_irq(). This is wrong, because section > 3.2.4 is clear for a level triggered interrupt that if the > interrupt signal remains asserted (which it usually will be) > then we go from Pending to Active+Pending (whereas our > current implementation goes from Pending to Active and > then resets Pending later in gic_complete_irq()). > Yes, I will send this patch to address this as part of the revised series: diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index bf1eb02..fce66c6 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -175,10 +180,15 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu) return 1023; } s->last_active[new_irq][cpu] = s->running_irq[cpu]; - /* Clear pending flags for both level and edge triggered interrupts. - Level triggered IRQs will be reasserted once they become inactive. */ - gic_clear_pending(s, new_irq, GIC_TEST_MODEL(new_irq) ? ALL_CPU_MASK : cm, - GIC_SGI_SRC(new_irq, cpu)); + /* Clear pending flags for edge-triggered and non-asserted level-triggered + * interrupts. + */ + cm = GIC_TEST_MODEL(new_irq) ? ALL_CPU_MASK : cm; + if (GIC_TEST_TRIGGER(new_irq) || !GIC_TEST_LEVEL(new_irq, cm)) { + gic_clear_pending(s, new_irq, cm, GIC_SGI_SRC(new_irq, cpu)); + } + gic_set_running_irq(s, cpu, new_irq); DPRINTF("ACK %d\n", new_irq); return new_irq; -- Christoffer