Re: [PATCH v4 13/30] KVM: x86: Expose several TSC helpers via x86.h for use by MSR code

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Lockless iteration of `vm_list` in `kvm_arch_enable_virtualization_cpu` during CPU hotplug.
- [High] Lockless cross-CPU modification of `vcpu->arch.tsc_offset_adjustment`.
--

commit ab7eedcd233e127b5aac182b220a36fa043a7767
Author: Sean Christopherson <seanjc@google.com>
Date:   Sat Jun 13 00:03:12 2026 +0000

KVM: x86: Expose several TSC helpers via x86.h for use by MSR code

Begrudgingly move adjust_tsc_offset_{guest,host}() to x86.h as inlines,
and expose several other TSC helpers in anticipation of moving KVM's MSR
code to a dedicated msrs.c. Unfortunately for KVM, several MSRs that KVM
emulates can affect TSC state.

Opportunistically drop a superfluous local "tsc_offset" variable, whose
existence causes checkpatch to complain about lack of a blank line.

No functional change intended.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ece8d53fb7fe6..316ec7a57f7dd 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
[ ... ]
> @@ -2448,22 +2448,6 @@ static void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 *user_value)
>  	raw_spin_unlock_irqrestore(&kvm->arch.tsc_write_lock, flags);
>  }
>  
> -static inline void adjust_tsc_offset_guest(struct kvm_vcpu *vcpu,
> -					   s64 adjustment)
> -{
> -	u64 tsc_offset = vcpu->arch.l1_tsc_offset;
> -	kvm_vcpu_write_tsc_offset(vcpu, tsc_offset + adjustment);
> -}
> -
> -static inline void adjust_tsc_offset_host(struct kvm_vcpu *vcpu, s64 adjustment)
> -{
> -	if (vcpu->arch.l1_tsc_scaling_ratio != kvm_caps.default_tsc_scaling_ratio)
> -		WARN_ON(adjustment < 0);
> -	adjustment = kvm_scale_tsc((u64) adjustment,
> -				   vcpu->arch.l1_tsc_scaling_ratio);
> -	adjust_tsc_offset_guest(vcpu, adjustment);
> -}
> -
>  #ifdef CONFIG_X86_64
>  
>  static u64 read_tsc(void)

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing these TSC
offset adjustments I noticed a race condition in how
vcpu->arch.tsc_offset_adjustment is updated during CPU hotplug.

In kvm_arch_enable_virtualization_cpu(), there is a lockless cross-CPU
modification of vcpu->arch.tsc_offset_adjustment:

arch/x86/kvm/x86.c:kvm_arch_enable_virtualization_cpu()
	if (backwards_tsc) {
		u64 delta_cyc = max_tsc - local_tsc;
		list_for_each_entry(kvm, &vm_list, vm_list) {
			kvm->arch.backwards_tsc_observed = true;
			kvm_for_each_vcpu(i, vcpu, kvm) {
				vcpu->arch.tsc_offset_adjustment += delta_cyc;

Since CPU hotplug only pauses the CPU being brought online, could the
targeted VCPU be concurrently executing kvm_arch_vcpu_load() on another CPU?

If kvm_arch_vcpu_load() reads and clears this field concurrently without
vcpu->mutex, could the adjustment be lost or applied twice and lead to
incorrect guest TSC values?

[Severity: High]
This is a pre-existing issue, but I also noticed that the iteration over
vm_list in kvm_arch_enable_virtualization_cpu() is lockless:

arch/x86/kvm/x86.c:kvm_arch_enable_virtualization_cpu()
	list_for_each_entry(kvm, &vm_list, vm_list) {
		kvm_for_each_vcpu(i, vcpu, kvm) {

If a user concurrently creates or destroys a VM (which modifies vm_list
under kvm_lock), couldn't this lockless list_for_each_entry() dereference
poisoned pointers from list_del() during destruction, leading to a kernel
panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613000329.732085-1-seanjc@google.com?part=13