[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] hw/intc/arm_gicv3: Don't signal Pending+Act
From: |
Edgar E. Iglesias |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] hw/intc/arm_gicv3: Don't signal Pending+Active interrupts to CPU |
Date: |
Wed, 7 Dec 2016 23:46:34 +0100 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
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 <address@hidden>
Reviewed-by: Edgar E. Iglesias <address@hidden>
> ---
> 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
>