Skip to content

Commit

Permalink
hw/intc/arm_gic: fix spurious level triggered interrupts
Browse files Browse the repository at this point in the history
On GICv2 and later, level triggered interrupts are pending when either
the interrupt line is asserted or the interrupt was made pending by a
GICD_ISPENDRn write. Making a level triggered interrupt pending by
software persists until either the interrupt is acknowledged or cleared
by writing GICD_ICPENDRn. As long as the interrupt line is asserted,
the interrupt is pending in any case.

This logic is transparently implemented in gic_test_pending() for
GICv1 and GICv2.  The function combines the "pending" irq_state flag
(used for edge triggered interrupts and software requests) and the
line status (tracked in the "level" field).  However, we also
incorrectly set the pending flag on a guest write to GICD_ISENABLERn
if the line of a level triggered interrupt was asserted.  This keeps
the interrupt pending even if the line is de-asserted after some
time.

This incorrect logic is a leftover of the initial 11MPCore GIC
implementation.  That handles things slightly differently to the
architected GICv1 and GICv2.  The 11MPCore TRM does not give a lot of
detail on the corner cases of its GIC's behaviour, and historically
we have not wanted to investigate exactly what it does in reality, so
QEMU's GIC model takes the approach of "retain our existing behaviour
for 11MPCore, and implement the architectural standard for later GIC
revisions".

On that basis, commit 8d99999 in 2013 is where we added the
"level-triggered interrupt with the line asserted" handling to
gic_test_pending(), and we deliberately kept the old behaviour of
gic_test_pending() for REV_11MPCORE.  That commit should have added
the "only if 11MPCore" condition to the setting of the pending bit on
writes to GICD_ISENABLERn, but forgot it.

Add the missing "if REV_11MPCORE" condition, so that our behaviour
on GICv1 and GICv2 matches the GIC architecture requirements.

Cc: [email protected]
Fixes: 8d99999 ("arm_gic: Fix GIC pending behavior")
Signed-off-by: Jan Klötzke <[email protected]>
Message-id: [email protected]
Reviewed-by: Peter Maydell <[email protected]>
[PMM: expanded comment a little and converted to coding-style form;
 expanded commit message with the historical backstory]
Signed-off-by: Peter Maydell <[email protected]>
(cherry picked from commit 110684c)
Signed-off-by: Michael Tokarev <[email protected]>
  • Loading branch information
jkloetzke authored and Michael Tokarev committed Sep 25, 2024
1 parent df9aa3d commit bec9a96
Showing 1 changed file with 8 additions and 3 deletions.
11 changes: 8 additions & 3 deletions hw/intc/arm_gic.c
Original file line number Diff line number Diff line change
Expand Up @@ -1263,9 +1263,14 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
trace_gic_enable_irq(irq + i);
}
GIC_DIST_SET_ENABLED(irq + i, cm);
/* If a raised level triggered IRQ enabled then mark
is as pending. */
if (GIC_DIST_TEST_LEVEL(irq + i, mask)
/*
* If a raised level triggered IRQ enabled then mark
* it as pending on 11MPCore. For other GIC revisions we
* handle the "level triggered and line asserted" check
* at the other end in gic_test_pending().
*/
if (s->revision == REV_11MPCORE
&& GIC_DIST_TEST_LEVEL(irq + i, mask)
&& !GIC_DIST_TEST_EDGE_TRIGGER(irq + i)) {
DPRINTF("Set %d pending mask %x\n", irq + i, mask);
GIC_DIST_SET_PENDING(irq + i, mask);
Expand Down

0 comments on commit bec9a96

Please sign in to comment.