[PATCH v3 1/4] hw/intc: Save time_delta in RISC-V mtimer VMState

TANG Tiancheng posted 4 patches 2 weeks, 3 days ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>
[PATCH v3 1/4] hw/intc: Save time_delta in RISC-V mtimer VMState
Posted by TANG Tiancheng 2 weeks, 3 days ago
In QEMU's RISC-V ACLINT timer model, 'mtime' is not stored directly as a
state variable. It is computed on demand as:

    mtime = rtc_r + time_delta

where:
- 'rtc_r' is the current VM virtual time (in ticks) obtained via
  cpu_riscv_read_rtc_raw() from QEMU_CLOCK_VIRTUAL.
- 'time_delta' is an offset applied when the guest writes a new 'mtime'
  value via riscv_aclint_mtimer_write():

    time_delta = value - rtc_r

Under this design, 'rtc_r' is assumed to be monotonically increasing
during VM execution. Even if the guest writes an 'mtime' value smaller
than the current one (making 'time_delta' negative in signed arithmetic,
or underflow in unsigned arithmetic), the computed 'mtime' remains
correct because 'rtc_r_new > rtc_r_old':

    mtime_new = rtc_r_new + (value - rtc_r_old)

However, this monotonicity assumption breaks on snapshot load.

Before restoring a snapshot, QEMU resets the guest, which calls
riscv_aclint_mtimer_reset_enter() to set 'mtime' to 0 and recompute
'time_delta' as:

    time_delta = 0 - rtc_r_reset

Here, the time_delta differs from the value that was present when the
snapshot was saved. As a result, subsequent reads produce a fixed offset
from the true mtime.

This can be observed with the 'date' command inside the guest: after loading
a snapshot, the reported time appears "frozen" at the save point, and only
resumes correctly after the guest has run long enough to compensate for the
erroneous offset.

The fix is to treat 'time_delta' as part of the device's migratable
state and save/restore it via vmstate. This preserves the correct
relation between 'rtc_r' and 'mtime' across snapshot save/load, ensuring
'mtime' continues incrementing from the precise saved value after
restore.

Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>
---
 hw/intc/riscv_aclint.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
index 4623cfa029365c6cbdead4bd4a9f0d8b9e88b939..318a9c8248432a8cd4c3f3fa990739917ecf7ca1 100644
--- a/hw/intc/riscv_aclint.c
+++ b/hw/intc/riscv_aclint.c
@@ -323,9 +323,10 @@ static void riscv_aclint_mtimer_reset_enter(Object *obj, ResetType type)
 
 static const VMStateDescription vmstate_riscv_mtimer = {
     .name = "riscv_mtimer",
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .fields = (const VMStateField[]) {
+            VMSTATE_UINT64(time_delta, RISCVAclintMTimerState),
             VMSTATE_VARRAY_UINT32(timecmp, RISCVAclintMTimerState,
                                   num_harts, 0,
                                   vmstate_info_uint64, uint64_t),

-- 
2.43.0
Re: [PATCH v3 1/4] hw/intc: Save time_delta in RISC-V mtimer VMState
Posted by Alistair Francis 1 week, 6 days ago
On Thu, Sep 11, 2025 at 7:58 PM TANG Tiancheng <lyndra@linux.alibaba.com> wrote:
>
> In QEMU's RISC-V ACLINT timer model, 'mtime' is not stored directly as a
> state variable. It is computed on demand as:
>
>     mtime = rtc_r + time_delta
>
> where:
> - 'rtc_r' is the current VM virtual time (in ticks) obtained via
>   cpu_riscv_read_rtc_raw() from QEMU_CLOCK_VIRTUAL.
> - 'time_delta' is an offset applied when the guest writes a new 'mtime'
>   value via riscv_aclint_mtimer_write():
>
>     time_delta = value - rtc_r
>
> Under this design, 'rtc_r' is assumed to be monotonically increasing
> during VM execution. Even if the guest writes an 'mtime' value smaller
> than the current one (making 'time_delta' negative in signed arithmetic,
> or underflow in unsigned arithmetic), the computed 'mtime' remains
> correct because 'rtc_r_new > rtc_r_old':
>
>     mtime_new = rtc_r_new + (value - rtc_r_old)
>
> However, this monotonicity assumption breaks on snapshot load.
>
> Before restoring a snapshot, QEMU resets the guest, which calls
> riscv_aclint_mtimer_reset_enter() to set 'mtime' to 0 and recompute
> 'time_delta' as:
>
>     time_delta = 0 - rtc_r_reset
>
> Here, the time_delta differs from the value that was present when the
> snapshot was saved. As a result, subsequent reads produce a fixed offset
> from the true mtime.
>
> This can be observed with the 'date' command inside the guest: after loading
> a snapshot, the reported time appears "frozen" at the save point, and only
> resumes correctly after the guest has run long enough to compensate for the
> erroneous offset.
>
> The fix is to treat 'time_delta' as part of the device's migratable
> state and save/restore it via vmstate. This preserves the correct
> relation between 'rtc_r' and 'mtime' across snapshot save/load, ensuring
> 'mtime' continues incrementing from the precise saved value after
> restore.
>
> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/intc/riscv_aclint.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
> index 4623cfa029365c6cbdead4bd4a9f0d8b9e88b939..318a9c8248432a8cd4c3f3fa990739917ecf7ca1 100644
> --- a/hw/intc/riscv_aclint.c
> +++ b/hw/intc/riscv_aclint.c
> @@ -323,9 +323,10 @@ static void riscv_aclint_mtimer_reset_enter(Object *obj, ResetType type)
>
>  static const VMStateDescription vmstate_riscv_mtimer = {
>      .name = "riscv_mtimer",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .fields = (const VMStateField[]) {
> +            VMSTATE_UINT64(time_delta, RISCVAclintMTimerState),
>              VMSTATE_VARRAY_UINT32(timecmp, RISCVAclintMTimerState,
>                                    num_harts, 0,
>                                    vmstate_info_uint64, uint64_t),
>
> --
> 2.43.0
>
>