1. Patchew
  2. Xen
  3. [PATCH v6 00/36] Cleaning up the KVM clock mess
  4. View patch

[PATCH v6 15/36] KVM: x86: Fix compute_guest_tsc() to handle negative time deltas

David Woodhouse posted 36 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v6 15/36] KVM: x86: Fix compute_guest_tsc() to handle negative time deltas
Posted by David Woodhouse 1 month, 1 week ago 
From: David Woodhouse <dwmw@amazon.co.uk>

The compute_guest_tsc() function computes the guest TSC at a given
kernel_ns timestamp. When the master clock reference point
(master_kernel_ns) is earlier than vcpu->arch.this_tsc_nsec, the delta
is negative. Since pvclock_scale_delta() takes a u64, the negative
value wraps to a huge positive number, producing a wildly wrong result.

Handle negative deltas explicitly by negating the delta, scaling it,
and subtracting from this_tsc_write.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 arch/x86/kvm/x86.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 376b8dc2ade9..55fb19fb7a88 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2588,11 +2588,21 @@ static int kvm_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz)
 
 static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns)
 {
-	u64 tsc = pvclock_scale_delta(kernel_ns-vcpu->arch.this_tsc_nsec,
-				      vcpu->arch.virtual_tsc_mult,
-				      vcpu->arch.virtual_tsc_shift);
-	tsc += vcpu->arch.this_tsc_write;
-	return tsc;
+	s64 delta_ns = kernel_ns - vcpu->arch.this_tsc_nsec;
+	u64 tsc;
+
+	/* Handle negative deltas gracefully (master clock ref may be earlier) */
+	if (delta_ns < 0) {
+		tsc = pvclock_scale_delta(-delta_ns,
+					  vcpu->arch.virtual_tsc_mult,
+					  vcpu->arch.virtual_tsc_shift);
+		return vcpu->arch.this_tsc_write - tsc;
+	}
+
+	tsc = pvclock_scale_delta(delta_ns,
+				  vcpu->arch.virtual_tsc_mult,
+				  vcpu->arch.virtual_tsc_shift);
+	return vcpu->arch.this_tsc_write + tsc;
 }
 
 #ifdef CONFIG_X86_64
-- 
2.54.0
Re: [PATCH v6 15/36] KVM: x86: Fix compute_guest_tsc() to handle negative time deltas
Posted by Sean Christopherson 2 weeks, 3 days ago 
On Fri, Jul 03, 2026, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> The compute_guest_tsc() function computes the guest TSC at a given
> kernel_ns timestamp. When the master clock reference point
> (master_kernel_ns) is earlier than vcpu->arch.this_tsc_nsec, the delta
> is negative. Since pvclock_scale_delta() takes a u64, the negative
> value wraps to a huge positive number, producing a wildly wrong result.
> 
> Handle negative deltas explicitly by negating the delta, scaling it,
> and subtracting from this_tsc_write.

Does this need 

  Cc: stable@vger.kernel.org

or is this a "technically a bug fix, but can't happen in practice" sort of thing?

> 
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  arch/x86/kvm/x86.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 376b8dc2ade9..55fb19fb7a88 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2588,11 +2588,21 @@ static int kvm_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz)
>  
>  static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns)
>  {
> -	u64 tsc = pvclock_scale_delta(kernel_ns-vcpu->arch.this_tsc_nsec,
> -				      vcpu->arch.virtual_tsc_mult,
> -				      vcpu->arch.virtual_tsc_shift);
> -	tsc += vcpu->arch.this_tsc_write;
> -	return tsc;
> +	s64 delta_ns = kernel_ns - vcpu->arch.this_tsc_nsec;
> +	u64 tsc;
> +
> +	/* Handle negative deltas gracefully (master clock ref may be earlier) */
> +	if (delta_ns < 0) {
> +		tsc = pvclock_scale_delta(-delta_ns,
> +					  vcpu->arch.virtual_tsc_mult,
> +					  vcpu->arch.virtual_tsc_shift);
> +		return vcpu->arch.this_tsc_write - tsc;
> +	}
> +
> +	tsc = pvclock_scale_delta(delta_ns,
> +				  vcpu->arch.virtual_tsc_mult,
> +				  vcpu->arch.virtual_tsc_shift);
> +	return vcpu->arch.this_tsc_write + tsc;

To cut down on the duplicate code, and IMO to make it easier to identify the
differences, how about this?

	s64 delta_ns = kernel_ns - vcpu->arch.this_tsc_nsec;
	u64 tsc;
	
	/* Handle negative deltas gracefully (master clock ref may be earlier) */
	tsc = pvclock_scale_delta(abs(-delta_ns),
				  vcpu->arch.virtual_tsc_mult,
				  vcpu->arch.virtual_tsc_shift);

	return vcpu->arch.this_tsc_write + (delta_ns >= 0 ? tsc : -tsc);
Re: [PATCH v6 15/36] KVM: x86: Fix compute_guest_tsc() to handle negative time deltas
Posted by David Woodhouse 2 weeks, 2 days ago 
On Fri, 2026-07-24 at 14:27 -0700, Sean Christopherson wrote:
> 
> To cut down on the duplicate code, and IMO to make it easier to identify the
> differences, how about this?
> 
> 	s64 delta_ns = kernel_ns - vcpu->arch.this_tsc_nsec;
> 	u64 tsc;
> 	
> 	/* Handle negative deltas gracefully (master clock ref may be earlier) */
> 	tsc = pvclock_scale_delta(abs(-delta_ns),
> 				  vcpu->arch.virtual_tsc_mult,
> 				  vcpu->arch.virtual_tsc_shift);
> 
> 	return vcpu->arch.this_tsc_write + (delta_ns >= 0 ? tsc : -tsc);

Oh, I missed that part. Done, rebased to kvm-x86/next and pushed to my
kvmclock7 branch while I test it and await further feedback.
Re: [PATCH v6 15/36] KVM: x86: Fix compute_guest_tsc() to handle negative time deltas
Posted by David Woodhouse 2 weeks, 3 days ago 
On Fri, 2026-07-24 at 14:27 -0700, Sean Christopherson wrote:
> On Fri, Jul 03, 2026, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@amazon.co.uk>
> > 
> > The compute_guest_tsc() function computes the guest TSC at a given
> > kernel_ns timestamp. When the master clock reference point
> > (master_kernel_ns) is earlier than vcpu->arch.this_tsc_nsec, the delta
> > is negative. Since pvclock_scale_delta() takes a u64, the negative
> > value wraps to a huge positive number, producing a wildly wrong result.
> > 
> > Handle negative deltas explicitly by negating the delta, scaling it,
> > and subtracting from this_tsc_write.
> 
> Does this need 
> 
>   Cc: stable@vger.kernel.org
> 
> or is this a "technically a bug fix, but can't happen in practice" sort of thing?

Honestly, there's so much hosed in kvmclock that we could debate a
cc:stable for half of this series. I'm not doing *any* of this to
actually add new features. For this one I'm not sure it's reachable in
practice; I think I did it mostly to shut Sashiko up.

I think even in the KVM_SET_CLOCK_GUEST it can't trigger, although I
have fantasies about changing the way KVM_SET_CLOCK_GUEST works to fix
that final ±1ns imprecision (which requires separate work on the
timekeeping core), that *might* trigger the negative delta here...
Re: [PATCH v6 15/36] KVM: x86: Fix compute_guest_tsc() to handle negative time deltas
Posted by Sean Christopherson 2 weeks ago 
On Sat, Jul 25, 2026, David Woodhouse wrote:
> On Fri, 2026-07-24 at 14:27 -0700, Sean Christopherson wrote:
> > On Fri, Jul 03, 2026, David Woodhouse wrote:
> > > From: David Woodhouse <dwmw@amazon.co.uk>
> > > 
> > > The compute_guest_tsc() function computes the guest TSC at a given
> > > kernel_ns timestamp. When the master clock reference point
> > > (master_kernel_ns) is earlier than vcpu->arch.this_tsc_nsec, the delta
> > > is negative. Since pvclock_scale_delta() takes a u64, the negative
> > > value wraps to a huge positive number, producing a wildly wrong result.
> > > 
> > > Handle negative deltas explicitly by negating the delta, scaling it,
> > > and subtracting from this_tsc_write.
> > 
> > Does this need 
> > 
> >   Cc: stable@vger.kernel.org
> > 
> > or is this a "technically a bug fix, but can't happen in practice" sort of thing?
> 
> Honestly, there's so much hosed in kvmclock that we could debate a
> cc:stable for half of this series. I'm not doing *any* of this to
> actually add new features. For this one I'm not sure it's reachable in
> practice; I think I did it mostly to shut Sashiko up.

Heh, in that case, throw a blurb in the changelog stating as much.  Knowing that
a bug is likely unreachable in practice is helpful, e.g. in the super unlikely
case that this change breaks someone.

> I think even in the KVM_SET_CLOCK_GUEST it can't trigger, although I
> have fantasies about changing the way KVM_SET_CLOCK_GUEST works to fix
> that final ±1ns imprecision (which requires separate work on the
> timekeeping core), that *might* trigger the negative delta here...

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.