[BUG REPORT] KVM: x86: Keep kvmclock_offset monotonic across same-host pause/resume

["Chenzhendong (alex)" <alex.chen@huawei.com>]

Hi,

While testing storage snapshots that suspend and resume a VM on the same
host (vm_stop -> snapshot -> vm_start), we observed that the guest
CLOCK_MONOTONIC jumps *backward* by the freeze duration after every
resume.

The problem
-----------

On a same-host pause/resume cycle, QEMU saves the kvmclock value at the
freeze instant (T1) via KVM_GET_CLOCK in vm_stop, and feeds it back via
KVM_SET_CLOCK in vm_start (T2 = T1 + freeze_duration).

kvm_vm_ioctl_set_clock() recomputes the offset unconditionally:

    kvmclock_offset = data.clock - now_raw_ns

with now_raw_ns sampled at T2. Since

    data.clock  = now_raw_ns(T1) + old_kvmclock_offset

this gives

    new_offset  = old_kvmclock_offset - freeze_duration

i.e. the offset is pulled back by the freeze duration.

The guest computes CLOCK_MONOTONIC from the pvclock page as:

    CLOCK_MONOTONIC = system_time + (rdtsc - tsc_timestamp) * mult >> shift

where system_time = kernel_ns + kvmclock_offset is filled by the host at
each vCPU entry. After resume the host re-samples tsc_timestamp, so the
(rdtsc - tsc_timestamp) term starts growing normally again and never
goes backward. The only term that can shrink is system_time: because
kvmclock_offset was pulled back by freeze_duration, the new system_time
is freeze_duration smaller than the value the guest last observed before
the freeze. The guest's next clock_gettime(CLOCK_MONOTONIC) therefore
reads a value freeze_duration smaller than before the freeze -- a
backward jump, violating the invariant that CLOCK_MONOTONIC never goes
backward.

The fix
-------

Only allow kvmclock_offset to move forward (or stay the same). If the
newly computed offset would be smaller than the current one, keep the
current value, so the guest clock keeps running forward and the freeze
duration is simply skipped from the guest's perspective -- which is the
expected behavior for a same-host pause/resume. This is a pure monotonic
protection with no time threshold; the normal forward path (guest boot,
KVM_CLOCK_REALTIME, cross-host migration toward a later destination host
clock) is unaffected.

    old_off = ka->kvmclock_offset;
    new_off = data.clock - now_raw_ns;
    if (new_off >= old_off)
            ka->kvmclock_offset = new_off;

Test
----

Inside the guest, pin a vCPU and call clock_gettime(CLOCK_MONOTONIC)
twice in a row, flagging any backward step. Over 50 snapshot
suspend/resume cycles the backward-step counter stays at 0 with this
patch, while the unpatched kernel reports a backward jump on every cycle
(measured freeze duration 27-36 ms).

To the best of our knowledge this scenario is not handled upstream. Any
feedback on whether monotonic protection is the right approach (vs. e.g.
teaching QEMU to feed a post-resume clock) would be much appreciated.

Signed-off-by: <Alex Chen> <alex.chen@huawei.com>
---
 arch/x86/kvm/x86.c | 19 ++++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7a0d0e8e5c1f..000000000000 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -XXXX,XX +XXXX,XX @@ static int kvm_vm_ioctl_set_clock(struct kvm *kvm, void __user *argp)
 	struct kvm_arch *ka = &kvm->arch;
 	struct kvm_clock_data data;
 	u64 now_raw_ns;
+	s64 old_off, new_off;
 	...
 	if (ka->use_master_clock)
 		now_raw_ns = ka->master_kernel_ns;
 	else
 		now_raw_ns = get_kvmclock_base_ns();
-	ka->kvmclock_offset = data.clock - now_raw_ns;
+
+	/*
+	 * Monotonic protection: data.clock is the kvmclock value userspace
+	 * saved at vm_stop (freeze instant); now_raw_ns is sampled at resume.
+	 * The naive kvmclock_offset = data.clock - now_raw_ns pulls the
+	 * offset back to the freeze instant, which makes pvti.system_time =
+	 * kernel_ns + kvmclock_offset jump backward by the freeze duration ->
+	 * guest vDSO CLOCK_MONOTONIC goes backward.
+	 *
+	 * Only allow kvmclock_offset to move forward (or stay). If the new
+	 * offset would shrink, keep the old one so the guest clock keeps
+	 * running forward (the freeze duration is simply skipped from the
+	 * guest's perspective, which is the desired behavior for a same-host
+	 * pause/resume). Pure monotonic protection, no time threshold.
+	 */
+	old_off = ka->kvmclock_offset;
+	new_off = data.clock - now_raw_ns;
+	if (new_off >= old_off)
+		ka->kvmclock_offset = new_off;
 	kvm_end_pvclock_update(kvm);