The Aspeed INTC records an interrupt source in the pending bitmap when the source is masked or another status bit is still being handled. When the guest later clears the status register, the model promotes all saved pending bits back to status unconditionally.
This is not correct for level-triggered sources. A source can deassert while another source connected to the same OR gate keeps the aggregated INTC line asserted. Promoting the stale bit later makes the guest demux a child interrupt whose device status has already been cleared. This is visible on AST2700 I2C, where the I2C buses are aggregated through INTCIO GICINT194 before reaching the GIC. A stale I2C source bit can be promoted back to the INTCIO status register, causing Linux to run the corresponding I2C ISR with an empty I2C interrupt status register. For example, the Linux aspeed-i2c debug ring shows a transfer that first receives a valid status interrupt, then receives a spurious ISR with both isr and raw status equal to zero. The zero-status ISR clears the saved command error and the transfer completes with ret=0: event=start isr=0x00000000 raw=0x00000000 cmd_err=0 msgs_idx=0 event=isr isr=0x00010011 raw=0x00010011 cmd_err=0 msgs_idx=0 event=isr isr=0x00000000 raw=0x00000000 cmd_err=1 msgs_idx=1 event=complete ret=0 cmd_err=0 msgs_idx=1 A normal command can then be reported as zero transferred messages, which is converted to -EIO by Linux i2c_smbus_xfer_emulated(). The race is more likely when multiple I2C buses are accessed concurrently. Drop pending bits that no longer correspond to an asserted and enabled source before they can be promoted back to status. Signed-off-by: Jian Zhang <[email protected]> --- hw/intc/aspeed_intc.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c index 5a36fff5204..ab80c239bc1 100644 --- a/hw/intc/aspeed_intc.c +++ b/hw/intc/aspeed_intc.c @@ -107,6 +107,27 @@ static const AspeedINTCIRQ *aspeed_intc_get_irq(AspeedINTCClass *aic, g_assert_not_reached(); } +static uint32_t aspeed_intc_orgate_levels(AspeedINTCState *s, int inpin_idx) +{ + AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s); + uint32_t levels = 0; + int i; + + for (i = 0; i < aic->num_lines && i < 32; i++) { + if (s->orgates[inpin_idx].levels[i]) { + levels |= BIT(i); + } + } + + return levels; +} + +static void aspeed_intc_drop_stale_pending(AspeedINTCState *s, int inpin_idx) +{ + s->pending[inpin_idx] &= aspeed_intc_orgate_levels(s, inpin_idx) & + s->enable[inpin_idx]; +} + /* * Update the state of an interrupt controller pin by setting * the specified output pin to the given level. @@ -231,6 +252,8 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int level) trace_aspeed_intc_set_irq(name, inpin_idx, level); enable = s->enable[inpin_idx]; + aspeed_intc_drop_stale_pending(s, inpin_idx); + if (!level) { return; } @@ -343,6 +366,7 @@ static void aspeed_intc_status_handler(AspeedINTCState *s, hwaddr offset, /* All source ISR execution are done */ if (!s->regs[reg]) { trace_aspeed_intc_all_isr_done(name, inpin_idx); + aspeed_intc_drop_stale_pending(s, inpin_idx); if (s->pending[inpin_idx]) { /* * handle pending source interrupt @@ -402,6 +426,7 @@ static void aspeed_intc_status_handler_multi_outpins(AspeedINTCState *s, /* All source ISR executions are done from a specific bit */ if (data & BIT(i)) { trace_aspeed_intc_all_isr_done_bit(name, inpin_idx, i); + aspeed_intc_drop_stale_pending(s, inpin_idx); if (s->pending[inpin_idx] & BIT(i)) { /* * Handle pending source interrupt. -- 2.20.1
