From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45081) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cEl02-000740-K8 for qemu-devel@nongnu.org; Wed, 07 Dec 2016 17:47:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cEl01-0000FC-Nj for qemu-devel@nongnu.org; Wed, 07 Dec 2016 17:47:42 -0500 Date: Wed, 7 Dec 2016 23:46:34 +0100 From: "Edgar E. Iglesias" Message-ID: <20161207224634.GG9606@toto> References: <1481046379-32632-1-git-send-email-peter.maydell@linaro.org> <1481046379-32632-4-git-send-email-peter.maydell@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1481046379-32632-4-git-send-email-peter.maydell@linaro.org> Subject: Re: [Qemu-devel] [PATCH 3/3] hw/intc/arm_gicv3: Don't signal Pending+Active interrupts to CPU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-arm@nongnu.org, qemu-devel@nongnu.org On Tue, Dec 06, 2016 at 05:46:19PM +0000, Peter Maydell wrote: > The GICv3 requires that we only signal Pending interrupts to > the CPU. This category does not include Pending+Active interrupts, > which means we need to check whether the interrupt is Active in > the gicr_int_pending() and gicd_int_pending() functions. > > Interrupts are rarely in the Active+Pending state, but KVM > uses this as part of its handling of the virtual timer, so > this bug was causing KVM to go into an infinite loop of > taking the vtimer interrupt when the guest first triggered it. > > Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias > --- > hw/intc/arm_gicv3.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c > index 8a6c647..f0c967b 100644 > --- a/hw/intc/arm_gicv3.c > +++ b/hw/intc/arm_gicv3.c > @@ -54,6 +54,7 @@ static uint32_t gicd_int_pending(GICv3State *s, int irq) > * + the PENDING latch is set OR it is level triggered and the input is 1 > * + its ENABLE bit is set > * + the GICD enable bit for its group is set > + * + its ACTIVE bit is not set (otherwise it would be Active+Pending) > * Conveniently we can bulk-calculate this with bitwise operations. > */ > uint32_t pend, grpmask; > @@ -63,9 +64,11 @@ static uint32_t gicd_int_pending(GICv3State *s, int irq) > uint32_t group = *gic_bmp_ptr32(s->group, irq); > uint32_t grpmod = *gic_bmp_ptr32(s->grpmod, irq); > uint32_t enable = *gic_bmp_ptr32(s->enabled, irq); > + uint32_t active = *gic_bmp_ptr32(s->active, irq); > > pend = pending | (~edge_trigger & level); > pend &= enable; > + pend &= ~active; > > if (s->gicd_ctlr & GICD_CTLR_DS) { > grpmod = 0; > @@ -96,12 +99,14 @@ static uint32_t gicr_int_pending(GICv3CPUState *cs) > * + the PENDING latch is set OR it is level triggered and the input is 1 > * + its ENABLE bit is set > * + the GICD enable bit for its group is set > + * + its ACTIVE bit is not set (otherwise it would be Active+Pending) > * Conveniently we can bulk-calculate this with bitwise operations. > */ > uint32_t pend, grpmask, grpmod; > > pend = cs->gicr_ipendr0 | (~cs->edge_trigger & cs->level); > pend &= cs->gicr_ienabler0; > + pend &= ~cs->gicr_iactiver0; > > if (cs->gic->gicd_ctlr & GICD_CTLR_DS) { > grpmod = 0; > -- > 2.7.4 >