[PATCH] hw/i2c/aspeed: fix lost interrupts on back-to-back commands

[PATCH] hw/i2c/aspeed: fix lost interrupts on back-to-back commands

[Jithu Joseph]

QEMU executes I2C commands synchronously inside the CMD register write
handler. On real hardware each command takes time on the bus, so the
ISR can clear the previous interrupt status before the next completion
arrives. In QEMU, when the guest ISR handles a TX_ACK and immediately
issues the next command by writing to CMD, that command completes
instantly — before the ISR returns to W1C-clear the first TX_ACK.
Since the bit is already set, setting it again is a no-op. The ISR
then clears it, wiping both completions at once. No interrupt fires
for the second command and the driver stalls.

This affects any multi-step I2C transaction: register reads, SMBus
word reads, and PMBus device probes all fail ("Error: Read failed"
from i2cget, -ETIMEDOUT from kernel drivers).

The issue is exposed when the guest kernel includes commit "i2c:
aspeed: Acknowledge Tx done with and without ACK irq late" [1] which
defers W1C acknowledgment of TX_ACK until after the ISR has issued
the next command. This means the old TX_ACK is still set when the
next command completes synchronously, and the subsequent W1C wipes
both completions at once.

The trace below shows `i2cget -y 15 0x50 0x00` (read EEPROM register
0x00) failing without the fix. The first START+TX sets TX_ACK. The
ISR handles it and issues a second TX to send the register address.
That TX completes synchronously while TX_ACK is still set:

  aspeed_i2c_bus_cmd cmd=0x3 start|tx| intr=0x0    # START+TX, clean
  aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK set
  aspeed_i2c_bus_read  0x10: 0x1                    # ISR reads TX_ACK
  aspeed_i2c_bus_write 0x14: 0x2                    # ISR issues TX cmd
  aspeed_i2c_bus_cmd cmd=0x400002 tx| intr=0x1      # TX runs, TX_ACK already set!
  aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # re-set is no-op
  aspeed_i2c_bus_write 0x10: 0x1                    # ISR W1C clears TX_ACK
  aspeed_i2c_bus_read  0x10: 0x0                    # LOST — both ACKs wiped

The driver sees INTR_STS=0 and never proceeds to the read phase.

Fix this by tracking interrupt bits that collide with already-pending
bits. Before calling aspeed_i2c_bus_handle_cmd(), save and clear
INTR_STS so that only freshly set bits are visible after the call.
Any overlap between the old and new bits is saved in pending_intr_sts.
When the ISR later W1C-clears the old bits, re-apply the saved
pending bits so the ISR sees them on its next loop iteration.

With the fix, the same operation completes successfully:

  aspeed_i2c_bus_cmd cmd=0x3 start|tx| intr=0x0    # START+TX, clean
  aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK set
  aspeed_i2c_bus_read  0x10: 0x1                    # ISR reads TX_ACK
  aspeed_i2c_bus_write 0x14: 0x2                    # ISR issues TX cmd
  aspeed_i2c_bus_cmd cmd=0x400002 tx| intr=0x0      # INTR_STS cleared first
  aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK freshly set
  aspeed_i2c_bus_write 0x10: 0x1                    # ISR W1C clears TX_ACK
  aspeed_i2c_bus_read  0x10: 0x1                    # RE-DELIVERED from pending
  aspeed_i2c_bus_write 0x14: 0x1b                   # ISR proceeds: START+RX
  aspeed_i2c_bus_cmd cmd=0x40001b start|tx|rx|last| # read phase completes
  i2c_recv recv(addr:0x50) data:0x00                # data received

[1] https://lore.kernel.org/all/20231211102217.2436294-3-quan@os.amperecomputing.com/

Signed-off-by: Jithu Joseph <jithu.joseph@oss.qualcomm.com>
---
Background: A Linux kernel patch series by Quan Nguyen (Ampere) reworked
the i2c-aspeed driver's interrupt handling to delay the W1C clear of
TX_ACK until after the ISR has processed the completion and issued any
follow-up command. The motivation was to correctly handle AST2600 and
AST2500 hardware where I2C slave mode requires this ordering — the
authors explicitly verified the series on both platforms. On our internal
AST2600 hardware we carry this patch as a necessary fix for I2C slave
mode to work correctly.

The series did not make it into the upstream Linux kernel because test
failures on QEMU blocked it. However, the failures are on the emulation
side: QEMU executes I2C commands synchronously inside the MMIO write
handler, causing a follow-up command to complete and re-raise an
already-pending interrupt bit before the ISR has had a chance to
W1C-clear it. On real hardware this race cannot occur since bus
transactions take measurable time.

Since the kernel patch reflects correct real-hardware behaviour, the
right fix is to adapt QEMU's interrupt delivery to handle this ordering,
rather than requiring the kernel driver to work around a QEMU-specific
timing artifact. This patch does that.

 include/hw/i2c/aspeed_i2c.h |  1 +
 hw/i2c/aspeed_i2c.c         | 46 +++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
index 53a9dba71b07..d42cb4865aa5 100644
--- a/include/hw/i2c/aspeed_i2c.h
+++ b/include/hw/i2c/aspeed_i2c.h
@@ -256,6 +256,7 @@ struct AspeedI2CBus {
     qemu_irq irq;
 
     uint32_t regs[ASPEED_I2C_NEW_NUM_REG];
+    uint32_t pending_intr_sts;
     uint8_t pool[ASPEED_I2C_BUS_POOL_SIZE];
     uint64_t dma_dram_offset;
 };
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 8022938f3478..ad6342bec0d4 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -628,6 +628,8 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
     AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
     bool handle_rx;
     bool w1t;
+    uint32_t old_intr;
+    uint32_t cmd_intr;
 
     trace_aspeed_i2c_bus_write(bus->id, offset, size, value);
 
@@ -665,6 +667,17 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
             break;
         }
         bus->regs[R_I2CM_INTR_STS] &= ~(value & 0xf007f07f);
+        /*
+         * Re-apply interrupts lost due to synchronous command completion.
+         * When a command completes instantly during an MMIO write, the new
+         * interrupt status bits collide with already-pending bits. After
+         * the ISR clears them, re-apply the saved bits so the ISR can
+         * process the new completion.
+         */
+        if (bus->pending_intr_sts) {
+            bus->regs[R_I2CM_INTR_STS] |= bus->pending_intr_sts;
+            bus->pending_intr_sts = 0;
+        }
         if (!bus->regs[R_I2CM_INTR_STS]) {
             bus->controller->intr_status &= ~(1 << bus->id);
             qemu_irq_lower(aic->bus_get_irq(bus));
@@ -708,7 +721,17 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
             bus->regs[R_I2CM_CMD] = value;
         }
 
+        old_intr = bus->regs[R_I2CM_INTR_STS];
+        bus->regs[R_I2CM_INTR_STS] = 0;
         aspeed_i2c_bus_handle_cmd(bus, value);
+        /*
+         * cmd_intr has only the bits handle_cmd freshly set.
+         * Overlap with old_intr means the same bit was re-fired
+         * and would be lost when the ISR W1C-clears the old one.
+         */
+        cmd_intr = bus->regs[R_I2CM_INTR_STS];
+        bus->regs[R_I2CM_INTR_STS] = cmd_intr | old_intr;
+        bus->pending_intr_sts |= old_intr & cmd_intr;
         aspeed_i2c_bus_raise_interrupt(bus);
         break;
     case A_I2CM_DMA_TX_ADDR:
@@ -845,6 +868,8 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
 {
     AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
     bool handle_rx;
+    uint32_t old_intr;
+    uint32_t cmd_intr;
 
     trace_aspeed_i2c_bus_write(bus->id, offset, size, value);
 
@@ -868,6 +893,17 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
         handle_rx = SHARED_ARRAY_FIELD_EX32(bus->regs, R_I2CD_INTR_STS, RX_DONE)
                     && SHARED_FIELD_EX32(value, RX_DONE);
         bus->regs[R_I2CD_INTR_STS] &= ~(value & 0x7FFF);
+        /*
+         * Re-apply interrupts lost due to synchronous command completion.
+         * When a command completes instantly during an MMIO write, the new
+         * interrupt status bits collide with already-pending bits. After
+         * the ISR clears them, re-apply the saved bits so the ISR can
+         * process the new completion.
+         */
+        if (bus->pending_intr_sts) {
+            bus->regs[R_I2CD_INTR_STS] |= bus->pending_intr_sts;
+            bus->pending_intr_sts = 0;
+        }
         if (!bus->regs[R_I2CD_INTR_STS]) {
             bus->controller->intr_status &= ~(1 << bus->id);
             qemu_irq_lower(aic->bus_get_irq(bus));
@@ -915,7 +951,17 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
         bus->regs[R_I2CD_CMD] &= ~0xFFFF;
         bus->regs[R_I2CD_CMD] |= value & 0xFFFF;
 
+        old_intr = bus->regs[R_I2CD_INTR_STS];
+        bus->regs[R_I2CD_INTR_STS] = 0;
         aspeed_i2c_bus_handle_cmd(bus, value);
+        /*
+         * cmd_intr has only the bits handle_cmd freshly set.
+         * Overlap with old_intr means the same bit was re-fired
+         * and would be lost when the ISR W1C-clears the old one.
+         */
+        cmd_intr = bus->regs[R_I2CD_INTR_STS];
+        bus->regs[R_I2CD_INTR_STS] = cmd_intr | old_intr;
+        bus->pending_intr_sts |= old_intr & cmd_intr;
         aspeed_i2c_bus_raise_interrupt(bus);
         break;
     case A_I2CD_DMA_ADDR:
-- 
2.43.0

Re: [PATCH] hw/i2c/aspeed: fix lost interrupts on back-to-back commands

[Cédric Le Goater]

Hello Jithu

On 3/11/26 03:37, Jithu Joseph wrote:
> QEMU executes I2C commands synchronously inside the CMD register write
> handler. On real hardware each command takes time on the bus, so the
> ISR can clear the previous interrupt status before the next completion
> arrives. In QEMU, when the guest ISR handles a TX_ACK and immediately
> issues the next command by writing to CMD, that command completes
> instantly — before the ISR returns to W1C-clear the first TX_ACK.
> Since the bit is already set, setting it again is a no-op. The ISR
> then clears it, wiping both completions at once. No interrupt fires
> for the second command and the driver stalls.
> 
> This affects any multi-step I2C transaction: register reads, SMBus
> word reads, and PMBus device probes all fail ("Error: Read failed"
> from i2cget, -ETIMEDOUT from kernel drivers).
> 
> The issue is exposed when the guest kernel includes commit "i2c:
> aspeed: Acknowledge Tx done with and without ACK irq late" [1] which
> defers W1C acknowledgment of TX_ACK until after the ISR has issued
> the next command. This means the old TX_ACK is still set when the
> next command completes synchronously, and the subsequent W1C wipes
> both completions at once.
> 
> The trace below shows `i2cget -y 15 0x50 0x00` (read EEPROM register
> 0x00) failing without the fix. The first START+TX sets TX_ACK. The
> ISR handles it and issues a second TX to send the register address.
> That TX completes synchronously while TX_ACK is still set:
> 
>    aspeed_i2c_bus_cmd cmd=0x3 start|tx| intr=0x0    # START+TX, clean
>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK set
>    aspeed_i2c_bus_read  0x10: 0x1                    # ISR reads TX_ACK
>    aspeed_i2c_bus_write 0x14: 0x2                    # ISR issues TX cmd
>    aspeed_i2c_bus_cmd cmd=0x400002 tx| intr=0x1      # TX runs, TX_ACK already set!
>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # re-set is no-op
>    aspeed_i2c_bus_write 0x10: 0x1                    # ISR W1C clears TX_ACK
>    aspeed_i2c_bus_read  0x10: 0x0                    # LOST — both ACKs wiped
> 
> The driver sees INTR_STS=0 and never proceeds to the read phase.
> 
> Fix this by tracking interrupt bits that collide with already-pending
> bits. Before calling aspeed_i2c_bus_handle_cmd(), save and clear
> INTR_STS so that only freshly set bits are visible after the call.
> Any overlap between the old and new bits is saved in pending_intr_sts.
> When the ISR later W1C-clears the old bits, re-apply the saved
> pending bits so the ISR sees them on its next loop iteration.
> 
> With the fix, the same operation completes successfully:
> 
>    aspeed_i2c_bus_cmd cmd=0x3 start|tx| intr=0x0    # START+TX, clean
>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK set
>    aspeed_i2c_bus_read  0x10: 0x1                    # ISR reads TX_ACK
>    aspeed_i2c_bus_write 0x14: 0x2                    # ISR issues TX cmd
>    aspeed_i2c_bus_cmd cmd=0x400002 tx| intr=0x0      # INTR_STS cleared first
>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK freshly set
>    aspeed_i2c_bus_write 0x10: 0x1                    # ISR W1C clears TX_ACK
>    aspeed_i2c_bus_read  0x10: 0x1                    # RE-DELIVERED from pending
>    aspeed_i2c_bus_write 0x14: 0x1b                   # ISR proceeds: START+RX
>    aspeed_i2c_bus_cmd cmd=0x40001b start|tx|rx|last| # read phase completes
>    i2c_recv recv(addr:0x50) data:0x00                # data received
> 
> [1] https://lore.kernel.org/all/20231211102217.2436294-3-quan@os.amperecomputing.com/
> 
> Signed-off-by: Jithu Joseph <jithu.joseph@oss.qualcomm.com>

Could you please provide a Fixes: tag for this change ?

> ---
> Background: A Linux kernel patch series by Quan Nguyen (Ampere) reworked
> the i2c-aspeed driver's interrupt handling to delay the W1C clear of
> TX_ACK until after the ISR has processed the completion and issued any
> follow-up command. The motivation was to correctly handle AST2600 and
> AST2500 hardware where I2C slave mode requires this ordering — the
> authors explicitly verified the series on both platforms. On our internal
> AST2600 hardware we carry this patch as a necessary fix for I2C slave
> mode to work correctly.
> 
> The series did not make it into the upstream Linux kernel because test
> failures on QEMU blocked it. 

Oh. Good to see that QEMU is trusted (even when wrong). Time for a fix !

> However, the failures are on the emulation
> side: QEMU executes I2C commands synchronously inside the MMIO write
> handler, causing a follow-up command to complete and re-raise an
> already-pending interrupt bit before the ISR has had a chance to
> W1C-clear it. On real hardware this race cannot occur since bus
> transactions take measurable time.
> 
> Since the kernel patch reflects correct real-hardware behaviour, the
> right fix is to adapt QEMU's interrupt delivery to handle this ordering,
> rather than requiring the kernel driver to work around a QEMU-specific
> timing artifact. This patch does that.

LGTM,

Thanks for this detailed report.

C.
  
> 
>   include/hw/i2c/aspeed_i2c.h |  1 +
>   hw/i2c/aspeed_i2c.c         | 46 +++++++++++++++++++++++++++++++++++++
>   2 files changed, 47 insertions(+)
> 
> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
> index 53a9dba71b07..d42cb4865aa5 100644
> --- a/include/hw/i2c/aspeed_i2c.h
> +++ b/include/hw/i2c/aspeed_i2c.h
> @@ -256,6 +256,7 @@ struct AspeedI2CBus {
>       qemu_irq irq;
>   
>       uint32_t regs[ASPEED_I2C_NEW_NUM_REG];
> +    uint32_t pending_intr_sts;
>       uint8_t pool[ASPEED_I2C_BUS_POOL_SIZE];
>       uint64_t dma_dram_offset;
>   };
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 8022938f3478..ad6342bec0d4 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -628,6 +628,8 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>       AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
>       bool handle_rx;
>       bool w1t;
> +    uint32_t old_intr;
> +    uint32_t cmd_intr;
>   
>       trace_aspeed_i2c_bus_write(bus->id, offset, size, value);
>   
> @@ -665,6 +667,17 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>               break;
>           }
>           bus->regs[R_I2CM_INTR_STS] &= ~(value & 0xf007f07f);
> +        /*
> +         * Re-apply interrupts lost due to synchronous command completion.
> +         * When a command completes instantly during an MMIO write, the new
> +         * interrupt status bits collide with already-pending bits. After
> +         * the ISR clears them, re-apply the saved bits so the ISR can
> +         * process the new completion.
> +         */
> +        if (bus->pending_intr_sts) {
> +            bus->regs[R_I2CM_INTR_STS] |= bus->pending_intr_sts;
> +            bus->pending_intr_sts = 0;
> +        }
>           if (!bus->regs[R_I2CM_INTR_STS]) {
>               bus->controller->intr_status &= ~(1 << bus->id);
>               qemu_irq_lower(aic->bus_get_irq(bus));
> @@ -708,7 +721,17 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>               bus->regs[R_I2CM_CMD] = value;
>           }
>   
> +        old_intr = bus->regs[R_I2CM_INTR_STS];
> +        bus->regs[R_I2CM_INTR_STS] = 0;
>           aspeed_i2c_bus_handle_cmd(bus, value);
> +        /*
> +         * cmd_intr has only the bits handle_cmd freshly set.
> +         * Overlap with old_intr means the same bit was re-fired
> +         * and would be lost when the ISR W1C-clears the old one.
> +         */
> +        cmd_intr = bus->regs[R_I2CM_INTR_STS];
> +        bus->regs[R_I2CM_INTR_STS] = cmd_intr | old_intr;
> +        bus->pending_intr_sts |= old_intr & cmd_intr;
>           aspeed_i2c_bus_raise_interrupt(bus);
>           break;
>       case A_I2CM_DMA_TX_ADDR:
> @@ -845,6 +868,8 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
>   {
>       AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
>       bool handle_rx;
> +    uint32_t old_intr;
> +    uint32_t cmd_intr;
>   
>       trace_aspeed_i2c_bus_write(bus->id, offset, size, value);
>   
> @@ -868,6 +893,17 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
>           handle_rx = SHARED_ARRAY_FIELD_EX32(bus->regs, R_I2CD_INTR_STS, RX_DONE)
>                       && SHARED_FIELD_EX32(value, RX_DONE);
>           bus->regs[R_I2CD_INTR_STS] &= ~(value & 0x7FFF);
> +        /*
> +         * Re-apply interrupts lost due to synchronous command completion.
> +         * When a command completes instantly during an MMIO write, the new
> +         * interrupt status bits collide with already-pending bits. After
> +         * the ISR clears them, re-apply the saved bits so the ISR can
> +         * process the new completion.
> +         */
> +        if (bus->pending_intr_sts) {
> +            bus->regs[R_I2CD_INTR_STS] |= bus->pending_intr_sts;
> +            bus->pending_intr_sts = 0;
> +        }
>           if (!bus->regs[R_I2CD_INTR_STS]) {
>               bus->controller->intr_status &= ~(1 << bus->id);
>               qemu_irq_lower(aic->bus_get_irq(bus));
> @@ -915,7 +951,17 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
>           bus->regs[R_I2CD_CMD] &= ~0xFFFF;
>           bus->regs[R_I2CD_CMD] |= value & 0xFFFF;
>   
> +        old_intr = bus->regs[R_I2CD_INTR_STS];
> +        bus->regs[R_I2CD_INTR_STS] = 0;
>           aspeed_i2c_bus_handle_cmd(bus, value);
> +        /*
> +         * cmd_intr has only the bits handle_cmd freshly set.
> +         * Overlap with old_intr means the same bit was re-fired
> +         * and would be lost when the ISR W1C-clears the old one.
> +         */
> +        cmd_intr = bus->regs[R_I2CD_INTR_STS];
> +        bus->regs[R_I2CD_INTR_STS] = cmd_intr | old_intr;
> +        bus->pending_intr_sts |= old_intr & cmd_intr;
>           aspeed_i2c_bus_raise_interrupt(bus);
>           break;
>       case A_I2CD_DMA_ADDR:

Re: [PATCH] hw/i2c/aspeed: fix lost interrupts on back-to-back commands

[Jithu Joseph]

On 3/11/2026 9:27 AM, Cédric Le Goater wrote:
> Hello Jithu
> 
> On 3/11/26 03:37, Jithu Joseph wrote:
>> QEMU executes I2C commands synchronously inside the CMD register write
>> handler. On real hardware each command takes time on the bus, so the
>> ISR can clear the previous interrupt status before the next completion
>> arrives. In QEMU, when the guest ISR handles a TX_ACK and immediately
>> issues the next command by writing to CMD, that command completes
>> instantly — before the ISR returns to W1C-clear the first TX_ACK.
>> Since the bit is already set, setting it again is a no-op. The ISR
>> then clears it, wiping both completions at once. No interrupt fires
>> for the second command and the driver stalls.
>>
>> This affects any multi-step I2C transaction: register reads, SMBus
>> word reads, and PMBus device probes all fail ("Error: Read failed"
>> from i2cget, -ETIMEDOUT from kernel drivers).
>>
>> The issue is exposed when the guest kernel includes commit "i2c:
>> aspeed: Acknowledge Tx done with and without ACK irq late" [1] which
>> defers W1C acknowledgment of TX_ACK until after the ISR has issued
>> the next command. This means the old TX_ACK is still set when the
>> next command completes synchronously, and the subsequent W1C wipes
>> both completions at once.
>>
>> The trace below shows `i2cget -y 15 0x50 0x00` (read EEPROM register
>> 0x00) failing without the fix. The first START+TX sets TX_ACK. The
>> ISR handles it and issues a second TX to send the register address.
>> That TX completes synchronously while TX_ACK is still set:
>>
>>    aspeed_i2c_bus_cmd cmd=0x3 start|tx| intr=0x0    # START+TX, clean
>>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK set
>>    aspeed_i2c_bus_read  0x10: 0x1                    # ISR reads TX_ACK
>>    aspeed_i2c_bus_write 0x14: 0x2                    # ISR issues TX cmd
>>    aspeed_i2c_bus_cmd cmd=0x400002 tx| intr=0x1      # TX runs, TX_ACK already set!
>>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # re-set is no-op
>>    aspeed_i2c_bus_write 0x10: 0x1                    # ISR W1C clears TX_ACK
>>    aspeed_i2c_bus_read  0x10: 0x0                    # LOST — both ACKs wiped
>>
>> The driver sees INTR_STS=0 and never proceeds to the read phase.
>>
>> Fix this by tracking interrupt bits that collide with already-pending
>> bits. Before calling aspeed_i2c_bus_handle_cmd(), save and clear
>> INTR_STS so that only freshly set bits are visible after the call.
>> Any overlap between the old and new bits is saved in pending_intr_sts.
>> When the ISR later W1C-clears the old bits, re-apply the saved
>> pending bits so the ISR sees them on its next loop iteration.
>>
>> With the fix, the same operation completes successfully:
>>
>>    aspeed_i2c_bus_cmd cmd=0x3 start|tx| intr=0x0    # START+TX, clean
>>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK set
>>    aspeed_i2c_bus_read  0x10: 0x1                    # ISR reads TX_ACK
>>    aspeed_i2c_bus_write 0x14: 0x2                    # ISR issues TX cmd
>>    aspeed_i2c_bus_cmd cmd=0x400002 tx| intr=0x0      # INTR_STS cleared first
>>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK freshly set
>>    aspeed_i2c_bus_write 0x10: 0x1                    # ISR W1C clears TX_ACK
>>    aspeed_i2c_bus_read  0x10: 0x1                    # RE-DELIVERED from pending
>>    aspeed_i2c_bus_write 0x14: 0x1b                   # ISR proceeds: START+RX
>>    aspeed_i2c_bus_cmd cmd=0x40001b start|tx|rx|last| # read phase completes
>>    i2c_recv recv(addr:0x50) data:0x00                # data received
>>
>> [1] https://lore.kernel.org/all/20231211102217.2436294-3-quan@os.amperecomputing.com/
>>
>> Signed-off-by: Jithu Joseph <jithu.joseph@oss.qualcomm.com>
> 
> Could you please provide a Fixes: tag for this change ?

The issue stems from QEMU's synchronous I2C command execution model which has been present since the initial driver introduction (1602001195dc). It is not a regression introduced by a specific commit, but rather a fundamental mismatch between QEMU's synchronous model and the temporal separation that real hardware has between command issue, command completion and ISR acknowledgment. This mismatch is exposed by the pending Linux kernel driver change [1 above] which reorders the ISR to issue the next command before W1C-clearing the current interrupt — a change needed for I2C slave mode to work correctly on real hardware.

I spent a bit of time looking at the commit history for hw/i2c/aspeed_i2c.c and considered 5540cb97f711 ("aspeed/i2c: interrupts should be cleared by software only") for fixes tag,  since it seemed to make the collision more visible by correctly implementing W1C semantics (removing an initialization bus->intr_status = 0), but the underlying race I think would have existed prior to that commit as well.

Happy to add Fixes: 1602001195dc ("i2c: add aspeed i2c controller") if you feel a Fixes tag is still appropriate, or drop it entirely given the fundamental nature of the issue. Your call.


> 
>> ---
>> Background: A Linux kernel patch series by Quan Nguyen (Ampere) reworked
>> the i2c-aspeed driver's interrupt handling to delay the W1C clear of
>> TX_ACK until after the ISR has processed the completion and issued any
>> follow-up command. The motivation was to correctly handle AST2600 and
>> AST2500 hardware where I2C slave mode requires this ordering — the
>> authors explicitly verified the series on both platforms. On our internal
>> AST2600 hardware we carry this patch as a necessary fix for I2C slave
>> mode to work correctly.
>>
>> The series did not make it into the upstream Linux kernel because test
>> failures on QEMU blocked it. 
> 
> Oh. Good to see that QEMU is trusted (even when wrong). Time for a fix !

:-)

> 
>> However, the failures are on the emulation
>> side: QEMU executes I2C commands synchronously inside the MMIO write
>> handler, causing a follow-up command to complete and re-raise an
>> already-pending interrupt bit before the ISR has had a chance to
>> W1C-clear it. On real hardware this race cannot occur since bus
>> transactions take measurable time.
>>
>> Since the kernel patch reflects correct real-hardware behaviour, the
>> right fix is to adapt QEMU's interrupt delivery to handle this ordering,
>> rather than requiring the kernel driver to work around a QEMU-specific
>> timing artifact. This patch does that.
> 
> LGTM,
> 
> Thanks for this detailed report.
> 

Thanks for considering and reading through


Jithu

Re: [PATCH] hw/i2c/aspeed: fix lost interrupts on back-to-back commands

[Cédric Le Goater]

On 3/11/26 03:37, Jithu Joseph wrote:
> QEMU executes I2C commands synchronously inside the CMD register write
> handler. On real hardware each command takes time on the bus, so the
> ISR can clear the previous interrupt status before the next completion
> arrives. In QEMU, when the guest ISR handles a TX_ACK and immediately
> issues the next command by writing to CMD, that command completes
> instantly — before the ISR returns to W1C-clear the first TX_ACK.
> Since the bit is already set, setting it again is a no-op. The ISR
> then clears it, wiping both completions at once. No interrupt fires
> for the second command and the driver stalls.
> 
> This affects any multi-step I2C transaction: register reads, SMBus
> word reads, and PMBus device probes all fail ("Error: Read failed"
> from i2cget, -ETIMEDOUT from kernel drivers).
> 
> The issue is exposed when the guest kernel includes commit "i2c:
> aspeed: Acknowledge Tx done with and without ACK irq late" [1] which
> defers W1C acknowledgment of TX_ACK until after the ISR has issued
> the next command. This means the old TX_ACK is still set when the
> next command completes synchronously, and the subsequent W1C wipes
> both completions at once.
> 
> The trace below shows `i2cget -y 15 0x50 0x00` (read EEPROM register
> 0x00) failing without the fix. The first START+TX sets TX_ACK. The
> ISR handles it and issues a second TX to send the register address.
> That TX completes synchronously while TX_ACK is still set:
> 
>    aspeed_i2c_bus_cmd cmd=0x3 start|tx| intr=0x0    # START+TX, clean
>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK set
>    aspeed_i2c_bus_read  0x10: 0x1                    # ISR reads TX_ACK
>    aspeed_i2c_bus_write 0x14: 0x2                    # ISR issues TX cmd
>    aspeed_i2c_bus_cmd cmd=0x400002 tx| intr=0x1      # TX runs, TX_ACK already set!
>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # re-set is no-op
>    aspeed_i2c_bus_write 0x10: 0x1                    # ISR W1C clears TX_ACK
>    aspeed_i2c_bus_read  0x10: 0x0                    # LOST — both ACKs wiped
> 
> The driver sees INTR_STS=0 and never proceeds to the read phase.
> 
> Fix this by tracking interrupt bits that collide with already-pending
> bits. Before calling aspeed_i2c_bus_handle_cmd(), save and clear
> INTR_STS so that only freshly set bits are visible after the call.
> Any overlap between the old and new bits is saved in pending_intr_sts.
> When the ISR later W1C-clears the old bits, re-apply the saved
> pending bits so the ISR sees them on its next loop iteration.
> 
> With the fix, the same operation completes successfully:
> 
>    aspeed_i2c_bus_cmd cmd=0x3 start|tx| intr=0x0    # START+TX, clean
>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK set
>    aspeed_i2c_bus_read  0x10: 0x1                    # ISR reads TX_ACK
>    aspeed_i2c_bus_write 0x14: 0x2                    # ISR issues TX cmd
>    aspeed_i2c_bus_cmd cmd=0x400002 tx| intr=0x0      # INTR_STS cleared first
>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK freshly set
>    aspeed_i2c_bus_write 0x10: 0x1                    # ISR W1C clears TX_ACK
>    aspeed_i2c_bus_read  0x10: 0x1                    # RE-DELIVERED from pending
>    aspeed_i2c_bus_write 0x14: 0x1b                   # ISR proceeds: START+RX
>    aspeed_i2c_bus_cmd cmd=0x40001b start|tx|rx|last| # read phase completes
>    i2c_recv recv(addr:0x50) data:0x00                # data received
> 
> [1] https://lore.kernel.org/all/20231211102217.2436294-3-quan@os.amperecomputing.com/
> 
> Signed-off-by: Jithu Joseph <jithu.joseph@oss.qualcomm.com>

Fixes: 1602001195dc ("i2c: add aspeed i2c controller")

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.



> ---
> Background: A Linux kernel patch series by Quan Nguyen (Ampere) reworked
> the i2c-aspeed driver's interrupt handling to delay the W1C clear of
> TX_ACK until after the ISR has processed the completion and issued any
> follow-up command. The motivation was to correctly handle AST2600 and
> AST2500 hardware where I2C slave mode requires this ordering — the
> authors explicitly verified the series on both platforms. On our internal
> AST2600 hardware we carry this patch as a necessary fix for I2C slave
> mode to work correctly.
> 
> The series did not make it into the upstream Linux kernel because test
> failures on QEMU blocked it. However, the failures are on the emulation
> side: QEMU executes I2C commands synchronously inside the MMIO write
> handler, causing a follow-up command to complete and re-raise an
> already-pending interrupt bit before the ISR has had a chance to
> W1C-clear it. On real hardware this race cannot occur since bus
> transactions take measurable time.
> 
> Since the kernel patch reflects correct real-hardware behaviour, the
> right fix is to adapt QEMU's interrupt delivery to handle this ordering,
> rather than requiring the kernel driver to work around a QEMU-specific
> timing artifact. This patch does that.
> 
>   include/hw/i2c/aspeed_i2c.h |  1 +
>   hw/i2c/aspeed_i2c.c         | 46 +++++++++++++++++++++++++++++++++++++
>   2 files changed, 47 insertions(+)
> 
> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
> index 53a9dba71b07..d42cb4865aa5 100644
> --- a/include/hw/i2c/aspeed_i2c.h
> +++ b/include/hw/i2c/aspeed_i2c.h
> @@ -256,6 +256,7 @@ struct AspeedI2CBus {
>       qemu_irq irq;
>   
>       uint32_t regs[ASPEED_I2C_NEW_NUM_REG];
> +    uint32_t pending_intr_sts;
>       uint8_t pool[ASPEED_I2C_BUS_POOL_SIZE];
>       uint64_t dma_dram_offset;
>   };
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 8022938f3478..ad6342bec0d4 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -628,6 +628,8 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>       AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
>       bool handle_rx;
>       bool w1t;
> +    uint32_t old_intr;
> +    uint32_t cmd_intr;
>   
>       trace_aspeed_i2c_bus_write(bus->id, offset, size, value);
>   
> @@ -665,6 +667,17 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>               break;
>           }
>           bus->regs[R_I2CM_INTR_STS] &= ~(value & 0xf007f07f);
> +        /*
> +         * Re-apply interrupts lost due to synchronous command completion.
> +         * When a command completes instantly during an MMIO write, the new
> +         * interrupt status bits collide with already-pending bits. After
> +         * the ISR clears them, re-apply the saved bits so the ISR can
> +         * process the new completion.
> +         */
> +        if (bus->pending_intr_sts) {
> +            bus->regs[R_I2CM_INTR_STS] |= bus->pending_intr_sts;
> +            bus->pending_intr_sts = 0;
> +        }
>           if (!bus->regs[R_I2CM_INTR_STS]) {
>               bus->controller->intr_status &= ~(1 << bus->id);
>               qemu_irq_lower(aic->bus_get_irq(bus));
> @@ -708,7 +721,17 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>               bus->regs[R_I2CM_CMD] = value;
>           }
>   
> +        old_intr = bus->regs[R_I2CM_INTR_STS];
> +        bus->regs[R_I2CM_INTR_STS] = 0;
>           aspeed_i2c_bus_handle_cmd(bus, value);
> +        /*
> +         * cmd_intr has only the bits handle_cmd freshly set.
> +         * Overlap with old_intr means the same bit was re-fired
> +         * and would be lost when the ISR W1C-clears the old one.
> +         */
> +        cmd_intr = bus->regs[R_I2CM_INTR_STS];
> +        bus->regs[R_I2CM_INTR_STS] = cmd_intr | old_intr;
> +        bus->pending_intr_sts |= old_intr & cmd_intr;
>           aspeed_i2c_bus_raise_interrupt(bus);
>           break;
>       case A_I2CM_DMA_TX_ADDR:
> @@ -845,6 +868,8 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
>   {
>       AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
>       bool handle_rx;
> +    uint32_t old_intr;
> +    uint32_t cmd_intr;
>   
>       trace_aspeed_i2c_bus_write(bus->id, offset, size, value);
>   
> @@ -868,6 +893,17 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
>           handle_rx = SHARED_ARRAY_FIELD_EX32(bus->regs, R_I2CD_INTR_STS, RX_DONE)
>                       && SHARED_FIELD_EX32(value, RX_DONE);
>           bus->regs[R_I2CD_INTR_STS] &= ~(value & 0x7FFF);
> +        /*
> +         * Re-apply interrupts lost due to synchronous command completion.
> +         * When a command completes instantly during an MMIO write, the new
> +         * interrupt status bits collide with already-pending bits. After
> +         * the ISR clears them, re-apply the saved bits so the ISR can
> +         * process the new completion.
> +         */
> +        if (bus->pending_intr_sts) {
> +            bus->regs[R_I2CD_INTR_STS] |= bus->pending_intr_sts;
> +            bus->pending_intr_sts = 0;
> +        }
>           if (!bus->regs[R_I2CD_INTR_STS]) {
>               bus->controller->intr_status &= ~(1 << bus->id);
>               qemu_irq_lower(aic->bus_get_irq(bus));
> @@ -915,7 +951,17 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
>           bus->regs[R_I2CD_CMD] &= ~0xFFFF;
>           bus->regs[R_I2CD_CMD] |= value & 0xFFFF;
>   
> +        old_intr = bus->regs[R_I2CD_INTR_STS];
> +        bus->regs[R_I2CD_INTR_STS] = 0;
>           aspeed_i2c_bus_handle_cmd(bus, value);
> +        /*
> +         * cmd_intr has only the bits handle_cmd freshly set.
> +         * Overlap with old_intr means the same bit was re-fired
> +         * and would be lost when the ISR W1C-clears the old one.
> +         */
> +        cmd_intr = bus->regs[R_I2CD_INTR_STS];
> +        bus->regs[R_I2CD_INTR_STS] = cmd_intr | old_intr;
> +        bus->pending_intr_sts |= old_intr & cmd_intr;
>           aspeed_i2c_bus_raise_interrupt(bus);
>           break;
>       case A_I2CD_DMA_ADDR:

Re: [PATCH] hw/i2c/aspeed: fix lost interrupts on back-to-back commands

[Cédric Le Goater]

On 3/11/26 03:37, Jithu Joseph wrote:
> QEMU executes I2C commands synchronously inside the CMD register write
> handler. On real hardware each command takes time on the bus, so the
> ISR can clear the previous interrupt status before the next completion
> arrives. In QEMU, when the guest ISR handles a TX_ACK and immediately
> issues the next command by writing to CMD, that command completes
> instantly — before the ISR returns to W1C-clear the first TX_ACK.
> Since the bit is already set, setting it again is a no-op. The ISR
> then clears it, wiping both completions at once. No interrupt fires
> for the second command and the driver stalls.
> 
> This affects any multi-step I2C transaction: register reads, SMBus
> word reads, and PMBus device probes all fail ("Error: Read failed"
> from i2cget, -ETIMEDOUT from kernel drivers).
> 
> The issue is exposed when the guest kernel includes commit "i2c:
> aspeed: Acknowledge Tx done with and without ACK irq late" [1] which
> defers W1C acknowledgment of TX_ACK until after the ISR has issued
> the next command. This means the old TX_ACK is still set when the
> next command completes synchronously, and the subsequent W1C wipes
> both completions at once.
> 
> The trace below shows `i2cget -y 15 0x50 0x00` (read EEPROM register
> 0x00) failing without the fix. The first START+TX sets TX_ACK. The
> ISR handles it and issues a second TX to send the register address.
> That TX completes synchronously while TX_ACK is still set:
> 
>    aspeed_i2c_bus_cmd cmd=0x3 start|tx| intr=0x0    # START+TX, clean
>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK set
>    aspeed_i2c_bus_read  0x10: 0x1                    # ISR reads TX_ACK
>    aspeed_i2c_bus_write 0x14: 0x2                    # ISR issues TX cmd
>    aspeed_i2c_bus_cmd cmd=0x400002 tx| intr=0x1      # TX runs, TX_ACK already set!
>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # re-set is no-op
>    aspeed_i2c_bus_write 0x10: 0x1                    # ISR W1C clears TX_ACK
>    aspeed_i2c_bus_read  0x10: 0x0                    # LOST — both ACKs wiped
> 
> The driver sees INTR_STS=0 and never proceeds to the read phase.
> 
> Fix this by tracking interrupt bits that collide with already-pending
> bits. Before calling aspeed_i2c_bus_handle_cmd(), save and clear
> INTR_STS so that only freshly set bits are visible after the call.
> Any overlap between the old and new bits is saved in pending_intr_sts.
> When the ISR later W1C-clears the old bits, re-apply the saved
> pending bits so the ISR sees them on its next loop iteration.
> 
> With the fix, the same operation completes successfully:
> 
>    aspeed_i2c_bus_cmd cmd=0x3 start|tx| intr=0x0    # START+TX, clean
>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK set
>    aspeed_i2c_bus_read  0x10: 0x1                    # ISR reads TX_ACK
>    aspeed_i2c_bus_write 0x14: 0x2                    # ISR issues TX cmd
>    aspeed_i2c_bus_cmd cmd=0x400002 tx| intr=0x0      # INTR_STS cleared first
>    aspeed_i2c_bus_raise_interrupt intr=0x1 ack|      # TX_ACK freshly set
>    aspeed_i2c_bus_write 0x10: 0x1                    # ISR W1C clears TX_ACK
>    aspeed_i2c_bus_read  0x10: 0x1                    # RE-DELIVERED from pending
>    aspeed_i2c_bus_write 0x14: 0x1b                   # ISR proceeds: START+RX
>    aspeed_i2c_bus_cmd cmd=0x40001b start|tx|rx|last| # read phase completes
>    i2c_recv recv(addr:0x50) data:0x00                # data received
> 
> [1] https://lore.kernel.org/all/20231211102217.2436294-3-quan@os.amperecomputing.com/
> 
> Signed-off-by: Jithu Joseph <jithu.joseph@oss.qualcomm.com>
> ---
> Background: A Linux kernel patch series by Quan Nguyen (Ampere) reworked
> the i2c-aspeed driver's interrupt handling to delay the W1C clear of
> TX_ACK until after the ISR has processed the completion and issued any
> follow-up command. The motivation was to correctly handle AST2600 and
> AST2500 hardware where I2C slave mode requires this ordering — the
> authors explicitly verified the series on both platforms. On our internal
> AST2600 hardware we carry this patch as a necessary fix for I2C slave
> mode to work correctly.
> 
> The series did not make it into the upstream Linux kernel because test
> failures on QEMU blocked it. However, the failures are on the emulation
> side: QEMU executes I2C commands synchronously inside the MMIO write
> handler, causing a follow-up command to complete and re-raise an
> already-pending interrupt bit before the ISR has had a chance to
> W1C-clear it. On real hardware this race cannot occur since bus
> transactions take measurable time.
> 
> Since the kernel patch reflects correct real-hardware behaviour, the
> right fix is to adapt QEMU's interrupt delivery to handle this ordering,
> rather than requiring the kernel driver to work around a QEMU-specific
> timing artifact. This patch does that.
> 
>   include/hw/i2c/aspeed_i2c.h |  1 +
>   hw/i2c/aspeed_i2c.c         | 46 +++++++++++++++++++++++++++++++++++++
>   2 files changed, 47 insertions(+)
> 
> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
> index 53a9dba71b07..d42cb4865aa5 100644
> --- a/include/hw/i2c/aspeed_i2c.h
> +++ b/include/hw/i2c/aspeed_i2c.h
> @@ -256,6 +256,7 @@ struct AspeedI2CBus {
>       qemu_irq irq;
>   
>       uint32_t regs[ASPEED_I2C_NEW_NUM_REG];
> +    uint32_t pending_intr_sts;
>       uint8_t pool[ASPEED_I2C_BUS_POOL_SIZE];
>       uint64_t dma_dram_offset;
>   };
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 8022938f3478..ad6342bec0d4 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -628,6 +628,8 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>       AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
>       bool handle_rx;
>       bool w1t;
> +    uint32_t old_intr;
> +    uint32_t cmd_intr;
>   
>       trace_aspeed_i2c_bus_write(bus->id, offset, size, value);
>   
> @@ -665,6 +667,17 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>               break;
>           }
>           bus->regs[R_I2CM_INTR_STS] &= ~(value & 0xf007f07f);
> +        /*
> +         * Re-apply interrupts lost due to synchronous command completion.
> +         * When a command completes instantly during an MMIO write, the new
> +         * interrupt status bits collide with already-pending bits. After
> +         * the ISR clears them, re-apply the saved bits so the ISR can
> +         * process the new completion.
> +         */
> +        if (bus->pending_intr_sts) {
> +            bus->regs[R_I2CM_INTR_STS] |= bus->pending_intr_sts;
> +            bus->pending_intr_sts = 0;
> +        }
>           if (!bus->regs[R_I2CM_INTR_STS]) {
>               bus->controller->intr_status &= ~(1 << bus->id);
>               qemu_irq_lower(aic->bus_get_irq(bus));
> @@ -708,7 +721,17 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
>               bus->regs[R_I2CM_CMD] = value;
>           }
>   
> +        old_intr = bus->regs[R_I2CM_INTR_STS];
> +        bus->regs[R_I2CM_INTR_STS] = 0;
>           aspeed_i2c_bus_handle_cmd(bus, value);
> +        /*
> +         * cmd_intr has only the bits handle_cmd freshly set.
> +         * Overlap with old_intr means the same bit was re-fired
> +         * and would be lost when the ISR W1C-clears the old one.
> +         */
> +        cmd_intr = bus->regs[R_I2CM_INTR_STS];
> +        bus->regs[R_I2CM_INTR_STS] = cmd_intr | old_intr;
> +        bus->pending_intr_sts |= old_intr & cmd_intr;
>           aspeed_i2c_bus_raise_interrupt(bus);
>           break;
>       case A_I2CM_DMA_TX_ADDR:
> @@ -845,6 +868,8 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
>   {
>       AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
>       bool handle_rx;
> +    uint32_t old_intr;
> +    uint32_t cmd_intr;
>   
>       trace_aspeed_i2c_bus_write(bus->id, offset, size, value);
>   
> @@ -868,6 +893,17 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
>           handle_rx = SHARED_ARRAY_FIELD_EX32(bus->regs, R_I2CD_INTR_STS, RX_DONE)
>                       && SHARED_FIELD_EX32(value, RX_DONE);
>           bus->regs[R_I2CD_INTR_STS] &= ~(value & 0x7FFF);
> +        /*
> +         * Re-apply interrupts lost due to synchronous command completion.
> +         * When a command completes instantly during an MMIO write, the new
> +         * interrupt status bits collide with already-pending bits. After
> +         * the ISR clears them, re-apply the saved bits so the ISR can
> +         * process the new completion.
> +         */
> +        if (bus->pending_intr_sts) {
> +            bus->regs[R_I2CD_INTR_STS] |= bus->pending_intr_sts;
> +            bus->pending_intr_sts = 0;
> +        }
>           if (!bus->regs[R_I2CD_INTR_STS]) {
>               bus->controller->intr_status &= ~(1 << bus->id);
>               qemu_irq_lower(aic->bus_get_irq(bus));
> @@ -915,7 +951,17 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
>           bus->regs[R_I2CD_CMD] &= ~0xFFFF;
>           bus->regs[R_I2CD_CMD] |= value & 0xFFFF;
>   
> +        old_intr = bus->regs[R_I2CD_INTR_STS];
> +        bus->regs[R_I2CD_INTR_STS] = 0;
>           aspeed_i2c_bus_handle_cmd(bus, value);
> +        /*
> +         * cmd_intr has only the bits handle_cmd freshly set.
> +         * Overlap with old_intr means the same bit was re-fired
> +         * and would be lost when the ISR W1C-clears the old one.
> +         */
> +        cmd_intr = bus->regs[R_I2CD_INTR_STS];
> +        bus->regs[R_I2CD_INTR_STS] = cmd_intr | old_intr;
> +        bus->pending_intr_sts |= old_intr & cmd_intr;
>           aspeed_i2c_bus_raise_interrupt(bus);
>           break;
>       case A_I2CD_DMA_ADDR:

Applied to aspeed-next.

Thanks,

C.