Message ID | 20240911114826.3558302-1-jan.kloetzke@kernkonzept.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] hw/intc/arm_gic: fix spurious level triggered interrupts | expand |
On Wed, 11 Sept 2024 at 12:49, Jan Klötzke <jan.kloetzke@kernkonzept.com> wrote: > > 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(). 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). Now, writing GICD_ISENABLERn incorrectly set the > pending flag 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. > > Apparently, the 11MPCore behaves differently. A level triggered > interrupt that is disabled does not get pending. Thus we have to retain > the get-pending-on-enable logic just for this model. For GICv2 and > later, the pending status is fully handled by gic_test_pending() and > does not need any special treatment when enabling the level interrupt. > > Signed-off-by: Jan Klötzke <jan.kloetzke@kernkonzept.com> > --- > v2: > * Keep existing logic for 11MPCore > > hw/intc/arm_gic.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c > index 806832439b..4333d52ee2 100644 > --- a/hw/intc/arm_gic.c > +++ b/hw/intc/arm_gic.c > @@ -1264,8 +1264,9 @@ static void gic_dist_writeb(void *opaque, hwaddr offset, > } > 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) > + is as pending on 11MPCore. */ > + 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); Applied to target-arm.next, thanks. (I added some extra text to the commit message to give a bit more of the historical backstory about what's going on with the 11MPCore GIC model.) -- PMM
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 806832439b..4333d52ee2 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -1264,8 +1264,9 @@ static void gic_dist_writeb(void *opaque, hwaddr offset, } 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) + is as pending on 11MPCore. */ + 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);
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(). 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). Now, writing GICD_ISENABLERn incorrectly set the pending flag 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. Apparently, the 11MPCore behaves differently. A level triggered interrupt that is disabled does not get pending. Thus we have to retain the get-pending-on-enable logic just for this model. For GICv2 and later, the pending status is fully handled by gic_test_pending() and does not need any special treatment when enabling the level interrupt. Signed-off-by: Jan Klötzke <jan.kloetzke@kernkonzept.com> --- v2: * Keep existing logic for 11MPCore hw/intc/arm_gic.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)