Re: [PATCH v7 31/36] KVM: x86: Add KVM_[GS]ET_CLOCK_GUEST for accurate KVM clock migration

[David Woodhouse]

On Tue, 2026-08-11 at 16:40 -0700, Sean Christopherson wrote:
> 
> Invoking kvm_guest_time_update() here is probably a deal-breaker.  Updating the
> master clock and other internal state is far from ideal, but should be ok.
> 
> However, writing guest memory is not.  Specifically, dirtying memory after the
> last KVM_RUN is a non-starter for many usecases, as is modifying state that is
> visible via other GET uAPI (though I don't think that applies here?).  E.g. see
> commits:

Ack. Calling kvm_guest_time_update() is also *entirely* pointless. We
are in masterclock mode, by definition, as this ioctl only works in
masterclock mode. So kvm_guest_time_update() isn't actually creating
any new information; it just calculates the same per-VM tsc_shift/mul.
And kvm_vcpu_ioctl_get_clock_guest() could just use those directly.
Even better, it can do so *inside* the seqcount loop.

Fixup below (not amending commits while you're co-opting the tree, as
we'd both go insane). I'll push it to the top of 
https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/kvmclock9-part2
on top of with the offset test which is already there.

>  And we'd probably want to build on my idea to report that
> KVM_RUN needs completion[*], but that'd be a good thing overall.
> 
> https://lore.kernel.org/all/20250111012450.1262638-1-seanjc@xxxxxxxxxx ;

Aha... *that* is why I found that lore thread open in my browser
yesterday; I was confused about how I'd got there. It's missing
KVM_EXIT_XEN btw. (Of which, KVM_EXIT_XEN_HYPERCALL is the only
subtype. I suspect João originally expected that there would be more).

From d2e54f439f0a5c0f2bddeaead916b23daedb2b2f Mon Sep 17 00:00:00 2001
From: David Woodhouse <dwmw@xxxxxxxxxxxx>
Date: Fri, 14 Aug 2026 07:56:58 +0100
Subject: [PATCH] fixup! KVM: x86: Add KVM_[GS]ET_CLOCK_GUEST for accurate KVM
 clock migration

Make KVM_GET_CLOCK_GUEST stateless, never writing guest memory.

Invoking kvm_guest_time_update() from the GET ioctl writes the guest's
pvclock pages, and dirtying guest memory after the final KVM_RUN breaks
migration flows which have already completed their last dirty-log pass.

There is no need for it. The only outputs previously taken from the
vCPU's shadow pvclock were tsc_shift and tsc_to_system_mul, and in
master clock mode (which this ioctl requires; it returns -ENODATA
otherwise) those are identical to the master clock's own mul/shift:
both are computed by kvm_get_time_scale() from the same guest TSC
frequency, which all vCPUs share. Use ka->master_tsc_{shift,mul}
directly, which pvclock_update_vm_gtod_copy() precomputed for exactly
this kind of vCPU-less consumer.

The only remaining per-vCPU input is kvm_read_l1_tsc(), which is a pure
read, translating the master clock snapshot into the vCPU's own TSC
domain.

This also removes the -EBUSY case and the special handling of a vCPU
which has never run: the master clock state exists from VM creation,
so the pvclock can always be constructed. And it moves the reads of
tsc_shift, tsc_to_system_mul and flags inside the seqcount loop, where
they should have been all along: previously they could tear against a
concurrent master clock update.

A KVM_REQ_CLOCK_UPDATE pending on the vCPU is now left pending rather
than being consumed: the pvclock returned here is anchored to the
current master clock snapshot and describes the same linear function
of the guest TSC as the guest-visible copy, so there is nothing to
refresh. The guest's own pages are updated, as ever, only from KVM_RUN.

Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
---
 arch/x86/kvm/x86.c | 37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8eb73e3ada66..4de98cc16751 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3460,27 +3460,22 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
 static int kvm_vcpu_ioctl_get_clock_guest(struct kvm_vcpu *v, void __user *argp)
 {
 	struct pvclock_vcpu_time_info hv_clock = {};
-	struct kvm_vcpu_arch *vcpu = &v->arch;
 	struct kvm_arch *ka = &v->kvm->arch;
 	unsigned int seq;
 
 	/*
-	 * If KVM_REQ_CLOCK_UPDATE is already pending, or if the pvclock
-	 * has never been generated at all, call kvm_guest_time_update().
-	 * Its only failure mode is transient (the TSC frequency of the
-	 * current CPU is momentarily unknown), so return -EBUSY to tell
-	 * userspace to try again.
-	 */
-	if (kvm_check_request(KVM_REQ_CLOCK_UPDATE, v) || !vcpu->hw_tsc_hz) {
-		guard(srcu)(&v->kvm->srcu);
-
-		if (kvm_guest_time_update(v))
-			return -EBUSY;
-	}
-
-	/*
-	 * Reconstruct the pvclock from the master clock state, matching
-	 * exactly what kvm_guest_time_update() writes to the guest.
+	 * Construct the pvclock purely from the master clock state. The
+	 * master mul/shift are computed for the guest TSC frequency, which
+	 * in master clock mode is the frequency of every vCPU; only the
+	 * tsc_timestamp is per-vCPU, translating the master snapshot into
+	 * this vCPU's TSC domain via its scaling ratio and offset.
+	 *
+	 * Note, this deliberately does NOT invoke kvm_guest_time_update(),
+	 * which would write the guest's pvclock pages: dirtying guest
+	 * memory after the final KVM_RUN would break post-copy migration
+	 * flows. The pvclock returned here describes the same clock as the
+	 * guest-visible copy (the same linear function of the guest TSC),
+	 * anchored at the current master clock snapshot.
 	 */
 	do {
 		seq = read_seqcount_begin(&ka->pvclock_sc);
@@ -3490,12 +3485,12 @@ static int kvm_vcpu_ioctl_get_clock_guest(struct kvm_vcpu *v, void __user *argp)
 
 		hv_clock.tsc_timestamp = kvm_read_l1_tsc(v, ka->master_cycle_now);
 		hv_clock.system_time = ka->master_kernel_ns + ka->kvmclock_offset;
+		hv_clock.tsc_shift = ka->master_tsc_shift;
+		hv_clock.tsc_to_system_mul = ka->master_tsc_mul;
+		hv_clock.flags = ka->all_vcpus_matched_tsc ?
+				 PVCLOCK_TSC_STABLE_BIT : 0;
 	} while (read_seqcount_retry(&ka->pvclock_sc, seq));
 
-	hv_clock.tsc_shift = vcpu->pvclock_tsc_shift;
-	hv_clock.tsc_to_system_mul = vcpu->pvclock_tsc_mul;
-	hv_clock.flags = ka->all_vcpus_matched_tsc ? PVCLOCK_TSC_STABLE_BIT : 0;
-
 	if (copy_to_user(argp, &hv_clock, sizeof(hv_clock)))
 		return -EFAULT;
 
-- 
2.43.0

Attachment: smime.p7s
Description: S/MIME cryptographic signature