Re: [PATCH v4 16/30] KVM: x86: Restructure kvm_guest_time_update() for TSC upscaling

[Dongli Zhang <dongli.zhang@oracle.com>]

I have encountered this build error with this patch.

Perhaps it is because all usage of "flags" are removed.

$ make -j32 > /dev/null
arch/x86/kvm/x86.c: In function ‘kvm_guest_time_update’:
arch/x86/kvm/x86.c:3359:23: error: unused variable ‘flags’ [-Werror=unused-variable]
 3359 |         unsigned long flags;
      |                       ^~~~~
cc1: all warnings being treated as errors
make[4]: *** [scripts/Makefile.build:289: arch/x86/kvm/x86.o] Error 1
make[3]: *** [scripts/Makefile.build:548: arch/x86/kvm] Error 2
make[2]: *** [scripts/Makefile.build:548: arch/x86] Error 2
make[1]: *** [/home/opc/ext4/mainline-linux/Makefile:2143: .] Error 2
make: *** [Makefile:248: __sub-make] Error 2

Thank you very much!

Dongli Zhang

On 2026-05-09 3:46 PM, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> Restructure kvm_guest_time_update() so that kernel_ns/host_tsc are
> always "now" when doing TSC catchup, then swap in the master clock
> reference values afterward for the hv_clock.
> 
> This makes the TSC upscaling code considerably simpler: the catchup
> adjustment is computed as the delta between what the guest TSC *should*
> be at "now" and what it actually is, rather than mixing "now" and
> "master clock reference" timestamps.
> 
> The seqcount loop now also contains the kvm_get_time_and_clockread()
> call (matching get_kvmclock's pattern), with the same WARN for
> unexpected failure.
> 
> Based on a suggestion by Sean Christopherson.
> 
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  arch/x86/kvm/x86.c | 67 ++++++++++++++++++++++++++++++++--------------
>  1 file changed, 47 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index e281c49561fa..8e4993ef4f6b 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3363,39 +3363,51 @@ int kvm_guest_time_update(struct kvm_vcpu *v)
>  	struct kvm_arch *ka = &v->kvm->arch;
>  	s64 kernel_ns;
>  	u64 tsc_timestamp, host_tsc;
> +	u64 master_host_tsc = 0;
> +	s64 master_kernel_ns = 0;
>  	bool use_master_clock;
>  
> -	kernel_ns = 0;
> -	host_tsc = 0;
> -
>  	/*
>  	 * If the host uses TSC clock, then passthrough TSC as stable
>  	 * to the guest.
>  	 */
>  	do {
>  		seq = read_seqcount_begin(&ka->pvclock_sc);
> +
>  		use_master_clock = ka->use_master_clock;
> -		if (use_master_clock) {
> -			host_tsc = ka->master_cycle_now;
> -			kernel_ns = ka->master_kernel_ns;
> -		}
> +
> +		/*
> +		 * The TSC read and the call to get_cpu_tsc_khz() must happen
> +		 * on the same CPU.
> +		 */
> +		get_cpu();
> +
> +		tgt_tsc_hz = (u64)get_cpu_tsc_khz() * 1000;
> +
> +		if (use_master_clock &&
> +		    !kvm_get_time_and_clockread(&kernel_ns, &host_tsc) &&
> +		    WARN_ON_ONCE(!read_seqcount_retry(&ka->pvclock_sc, seq)))
> +			use_master_clock = false;
> +
> +		put_cpu();
> +
> +		if (!use_master_clock)
> +			break;
> +
> +		master_host_tsc = ka->master_cycle_now;
> +		master_kernel_ns = ka->master_kernel_ns;
>  	} while (read_seqcount_retry(&ka->pvclock_sc, seq));
>  
> -	/* Keep irq disabled to prevent changes to the clock */
> -	local_irq_save(flags);
> -	tgt_tsc_hz = (u64)get_cpu_tsc_khz() * 1000;
>  	if (unlikely(tgt_tsc_hz == 0)) {
> -		local_irq_restore(flags);
>  		kvm_make_request(KVM_REQ_CLOCK_UPDATE, v);
>  		return 1;
>  	}
> +
>  	if (!use_master_clock) {
>  		host_tsc = rdtsc();
>  		kernel_ns = get_kvmclock_base_ns();
>  	}
>  
> -	tsc_timestamp = kvm_read_l1_tsc(v, host_tsc);
> -
>  	/*
>  	 * We may have to catch up the TSC to match elapsed wall clock
>  	 * time for two reasons, even if kvmclock is used.
> @@ -3404,17 +3416,32 @@ int kvm_guest_time_update(struct kvm_vcpu *v)
>  	 *      entry to avoid unknown leaps of TSC even when running
>  	 *      again on the same CPU.  This may cause apparent elapsed
>  	 *      time to disappear, and the guest to stand still or run
> -	 *	very slowly.
> +	 *      very slowly.
>  	 */
>  	if (vcpu->tsc_catchup) {
> -		u64 tsc = compute_guest_tsc(v, kernel_ns);
> -		if (tsc > tsc_timestamp) {
> -			adjust_tsc_offset_guest(v, tsc - tsc_timestamp);
> -			tsc_timestamp = tsc;
> -		}
> +		s64 adjustment;
> +
> +		/*
> +		 * Calculate the delta between what the guest TSC *should* be
> +		 * and what it actually is according to kvm_read_l1_tsc().
> +		 */
> +		adjustment = compute_guest_tsc(v, kernel_ns) -
> +			     kvm_read_l1_tsc(v, host_tsc);
> +		if (adjustment > 0)
> +			adjust_tsc_offset_guest(v, adjustment);
>  	}
>  
> -	local_irq_restore(flags);
> +	/*
> +	 * Now that TSC upscaling is out of the way, the remaining calculations
> +	 * are all relative to the reference time that's placed in hv_clock.
> +	 * If the master clock is NOT in use, the reference time is "now".  If
> +	 * master clock is in use, the reference time comes from there.
> +	 */
> +	if (use_master_clock) {
> +		host_tsc = master_host_tsc;
> +		kernel_ns = master_kernel_ns;
> +	}
> +	tsc_timestamp = kvm_read_l1_tsc(v, host_tsc);
>  
>  	/* With all the info we got, fill in the values */
>