hw/i2c/aspeed_i2c: reset and migrate pending_intr_sts

[PATCH] hw/i2c/aspeed_i2c: reset and migrate pending_intr_sts

Posted by GuoHan Zhao 1 week, 1 day ago

Commit 7e82393ed058 ("hw/i2c/aspeed: fix lost interrupts on
back-to-back commands") introduced pending_intr_sts to preserve
interrupt bits that collide with already pending status bits.

That deferred interrupt state is consumed later when the guest clears
INTR_STS, but it is not reset in aspeed_i2c_bus_reset() and it is not
part of the bus migration state. A reset can therefore leave stale
deferred bits behind, and migration can silently drop them.

Clear pending_intr_sts on reset and include it in VMState while keeping
compatibility with older migration streams.

Fixes: 7e82393ed058 ("hw/i2c/aspeed: fix lost interrupts on back-to-back commands")
Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
---
 hw/i2c/aspeed_i2c.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 5d18f8d49ea4..90f83a03fd4f 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -1134,10 +1134,11 @@ static const MemoryRegionOps aspeed_i2c_bus_pool_ops = {
 
 static const VMStateDescription aspeed_i2c_bus_vmstate = {
     .name = TYPE_ASPEED_I2C,
-    .version_id = 6,
+    .version_id = 7,
     .minimum_version_id = 6,
     .fields = (const VMStateField[]) {
         VMSTATE_UINT32_ARRAY(regs, AspeedI2CBus, ASPEED_I2C_NEW_NUM_REG),
+        VMSTATE_UINT32_V(pending_intr_sts, AspeedI2CBus, 7),
         VMSTATE_UINT8_ARRAY(pool, AspeedI2CBus, ASPEED_I2C_BUS_POOL_SIZE),
         VMSTATE_UINT64(dma_dram_offset, AspeedI2CBus),
         VMSTATE_END_OF_LIST()
@@ -1510,6 +1511,7 @@ static void aspeed_i2c_bus_reset(DeviceState *dev)
     AspeedI2CBus *s = ASPEED_I2C_BUS(dev);
 
     memset(s->regs, 0, sizeof(s->regs));
+    s->pending_intr_sts = 0;
     i2c_end_transfer(s->bus);
 }
 
-- 
2.43.0

Re: [PATCH] hw/i2c/aspeed_i2c: reset and migrate pending_intr_sts

Posted by Cédric Le Goater 3 days, 18 hours ago

On 3/25/26 09:54, GuoHan Zhao wrote:
> Commit 7e82393ed058 ("hw/i2c/aspeed: fix lost interrupts on
> back-to-back commands") introduced pending_intr_sts to preserve
> interrupt bits that collide with already pending status bits.
> 
> That deferred interrupt state is consumed later when the guest clears
> INTR_STS, but it is not reset in aspeed_i2c_bus_reset() and it is not
> part of the bus migration state. A reset can therefore leave stale
> deferred bits behind, and migration can silently drop them.
> 
> Clear pending_intr_sts on reset and include it in VMState while keeping
> compatibility with older migration streams.
> 
> Fixes: 7e82393ed058 ("hw/i2c/aspeed: fix lost interrupts on back-to-back commands")
> Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
> ---
>   hw/i2c/aspeed_i2c.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 5d18f8d49ea4..90f83a03fd4f 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -1134,10 +1134,11 @@ static const MemoryRegionOps aspeed_i2c_bus_pool_ops = {
>   
>   static const VMStateDescription aspeed_i2c_bus_vmstate = {
>       .name = TYPE_ASPEED_I2C,
> -    .version_id = 6,
> +    .version_id = 7,
>       .minimum_version_id = 6,
>       .fields = (const VMStateField[]) {
>           VMSTATE_UINT32_ARRAY(regs, AspeedI2CBus, ASPEED_I2C_NEW_NUM_REG),
> +        VMSTATE_UINT32_V(pending_intr_sts, AspeedI2CBus, 7),
>           VMSTATE_UINT8_ARRAY(pool, AspeedI2CBus, ASPEED_I2C_BUS_POOL_SIZE),
>           VMSTATE_UINT64(dma_dram_offset, AspeedI2CBus),
>           VMSTATE_END_OF_LIST()
> @@ -1510,6 +1511,7 @@ static void aspeed_i2c_bus_reset(DeviceState *dev)
>       AspeedI2CBus *s = ASPEED_I2C_BUS(dev);
>   
>       memset(s->regs, 0, sizeof(s->regs));
> +    s->pending_intr_sts = 0;
>       i2c_end_transfer(s->bus);
>   }
>   

Applied to aspeed-next.

Thanks,

C.

Re: [PATCH] hw/i2c/aspeed_i2c: reset and migrate pending_intr_sts

Posted by Jithu Joseph 1 week ago

On 3/25/2026 1:54 AM, GuoHan Zhao wrote:
> Commit 7e82393ed058 ("hw/i2c/aspeed: fix lost interrupts on
> back-to-back commands") introduced pending_intr_sts to preserve
> interrupt bits that collide with already pending status bits.
> 
> That deferred interrupt state is consumed later when the guest clears
> INTR_STS, but it is not reset in aspeed_i2c_bus_reset() and it is not
> part of the bus migration state. A reset can therefore leave stale
> deferred bits behind, and migration can silently drop them.

Thanks for identifying and fixing the aforementioned omissions

> 
> Clear pending_intr_sts on reset and include it in VMState while keeping
> compatibility with older migration streams.
> 
> Fixes: 7e82393ed058 ("hw/i2c/aspeed: fix lost interrupts on back-to-back commands")
> Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>


Reviewed-by: Jithu Joseph <jithu.joseph@oss.qualcomm.com>





Thanks
Jithu

Re: [PATCH] hw/i2c/aspeed_i2c: reset and migrate pending_intr_sts

Posted by Cédric Le Goater 1 week, 1 day ago

+Jithu

On 3/25/26 09:54, GuoHan Zhao wrote:
> Commit 7e82393ed058 ("hw/i2c/aspeed: fix lost interrupts on
> back-to-back commands") introduced pending_intr_sts to preserve
> interrupt bits that collide with already pending status bits.
> 
> That deferred interrupt state is consumed later when the guest clears
> INTR_STS, but it is not reset in aspeed_i2c_bus_reset() and it is not
> part of the bus migration state. A reset can therefore leave stale
> deferred bits behind, and migration can silently drop them.
> 
> Clear pending_intr_sts on reset and include it in VMState while keeping
> compatibility with older migration streams.
> 
> Fixes: 7e82393ed058 ("hw/i2c/aspeed: fix lost interrupts on back-to-back commands")
> Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>


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

Thanks,

C.


> ---
>   hw/i2c/aspeed_i2c.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 5d18f8d49ea4..90f83a03fd4f 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -1134,10 +1134,11 @@ static const MemoryRegionOps aspeed_i2c_bus_pool_ops = {
>   
>   static const VMStateDescription aspeed_i2c_bus_vmstate = {
>       .name = TYPE_ASPEED_I2C,
> -    .version_id = 6,
> +    .version_id = 7,
>       .minimum_version_id = 6,
>       .fields = (const VMStateField[]) {
>           VMSTATE_UINT32_ARRAY(regs, AspeedI2CBus, ASPEED_I2C_NEW_NUM_REG),
> +        VMSTATE_UINT32_V(pending_intr_sts, AspeedI2CBus, 7),
>           VMSTATE_UINT8_ARRAY(pool, AspeedI2CBus, ASPEED_I2C_BUS_POOL_SIZE),
>           VMSTATE_UINT64(dma_dram_offset, AspeedI2CBus),
>           VMSTATE_END_OF_LIST()
> @@ -1510,6 +1511,7 @@ static void aspeed_i2c_bus_reset(DeviceState *dev)
>       AspeedI2CBus *s = ASPEED_I2C_BUS(dev);
>   
>       memset(s->regs, 0, sizeof(s->regs));
> +    s->pending_intr_sts = 0;
>       i2c_end_transfer(s->bus);
>   }
>

Re: [PATCH] hw/i2c/aspeed_i2c: reset and migrate pending_intr_sts

Posted by Jithu Joseph 1 week ago

On 3/25/2026 3:54 AM, Cédric Le Goater wrote:
> +Jithu
> 

Thanks for looping me in Cédric. Will be mindful of these next-time


Thanks
Jithu

Re: [PATCH] hw/i2c/aspeed_i2c: reset and migrate pending_intr_sts

Posted by Cédric Le Goater 6 days, 16 hours ago

On 3/26/26 23:57, Jithu Joseph wrote:
> On 3/25/2026 3:54 AM, Cédric Le Goater wrote:
>> +Jithu
>>
> 
> Thanks for looping me in Cédric. Will be mindful of these next-time

Migration support of the Aspeed machines used to work. Maintaining
compatibility with older QEMU versions is best effort since it is
not a critical feature. But, who knows, may be, we will virtualise
the BMCs one day.

Thanks,

C.