Re: [PATCH v5 2/2] KVM: VMX: Cap VMX preemption timer to work around Intel erratum

[Sean Christopherson <seanjc@google.com>]

On Fri, Jul 24, 2026, Jim Mattson wrote:

Sorry for the late review, I didn't actually look at the code in the previous
versions.

> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index a07faa066ef0..ebe83a641473 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -153,6 +153,7 @@ module_param(dump_invalid_vmcs, bool, 0644);
>  #ifdef CONFIG_X86_64
>  static int __read_mostly cpu_preemption_timer_multi;
>  static bool __read_mostly enable_preemption_timer = 1;
> +static u64 __ro_after_init preemption_timer_limit;
>  module_param_named(preemption_timer, enable_preemption_timer, bool, S_IRUGO);
>  #else
>  #define enable_preemption_timer false
> @@ -8306,6 +8307,26 @@ static inline int u64_shl_div_u64(u64 a, unsigned int shift,
>  	return 0;
>  }
>  
> +/*
> + * Workaround for a widespread Intel erratum (e.g. EMR158) where the
> + * VMX-preemption timer may expire earlier than expected when programmed
> + * with large values. The workaround is to cap the timer value to strictly
> + * less than 2^25 * CPUID.15H:EBX / CPUID.15H:EAX.
> + */
> +static __init u64 calc_preemption_timer_limit(void)
> +{
> +	u32 eax, ebx, ecx, edx;
> +
> +	if (cpuid_eax(0) < 0x15)
> +		return 1ULL << 32;

Tracking an exclusive limit is cumbersome because all of KVM's usage deals with
the max value, i.e. it forces all usage to effectively do "- 1".  I also think
we should go with "max_value" instead of "limit" so that there's less chance for
confusion around whether the limit is exclusive or inclusive.

And we should provide a local "const u64" for maximum architectural value.  The
other option would be to initialize the global to the max architectural value,
and then do early returns here, but I like explicitly setting the global before
checking it against use_timer_freq.

> +
> +	cpuid(0x15, &eax, &ebx, &ecx, &edx);
> +	if (!eax || !ebx)
> +		return 1ULL << 32;
> +
> +	return min_t(u64, 1ULL << 32, ((u64)ebx << 25) / eax);

While I appreciate Sashiko's paranoia about "((u64)ebx << 25) / eax)" yielding
zero, I think we should treat that as a WARNable offence.  And to avoid a false
positive due to running as a VM of a misconfigured hypervisor, I think we should
only apply the erratum workaround on bare metal, i.e. use the maximum value if
KVM detects X86_FEATURE_HYPERVISOR.

So this as fixup?  If this looks good to you, I'm happy to post v6 since I've
already got it locally and tested on CLX, ICX, and EMR.

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index ebe83a641473..dae41842a9ce 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -153,7 +153,7 @@ module_param(dump_invalid_vmcs, bool, 0644);
 #ifdef CONFIG_X86_64
 static int __read_mostly cpu_preemption_timer_multi;
 static bool __read_mostly enable_preemption_timer = 1;
-static u64 __ro_after_init preemption_timer_limit;
+static u64 __ro_after_init preemption_timer_max_value;
 module_param_named(preemption_timer, enable_preemption_timer, bool, S_IRUGO);
 #else
 #define enable_preemption_timer false
@@ -8313,18 +8313,25 @@ static inline int u64_shl_div_u64(u64 a, unsigned int shift,
  * with large values. The workaround is to cap the timer value to strictly
  * less than 2^25 * CPUID.15H:EBX / CPUID.15H:EAX.
  */
-static __init u64 calc_preemption_timer_limit(void)
+static __init u64 calc_preemption_timer_max_value(void)
 {
+       const u64 ARCHITECTURAL_MAX_VALUE = UINT_MAX;
        u32 eax, ebx, ecx, edx;
 
+       if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
+               return ARCHITECTURAL_MAX_VALUE;
+
        if (cpuid_eax(0) < 0x15)
-               return 1ULL << 32;
+               return ARCHITECTURAL_MAX_VALUE;
 
        cpuid(0x15, &eax, &ebx, &ecx, &edx);
        if (!eax || !ebx)
-               return 1ULL << 32;
+               return ARCHITECTURAL_MAX_VALUE;
 
-       return min_t(u64, 1ULL << 32, ((u64)ebx << 25) / eax);
+       if (WARN_ON_ONCE(!(((u64)ebx << 25) / eax)))
+               return ARCHITECTURAL_MAX_VALUE;
+
+       return (((u64)ebx << 25) / eax) - 1;
 }
 
 static __init void vmx_setup_preemption_timer(void)
@@ -8338,7 +8345,7 @@ static __init void vmx_setup_preemption_timer(void)
                cpu_preemption_timer_multi =
                        vmx_misc_preemption_timer_rate(vmcs_config.misc);
 
-               preemption_timer_limit = calc_preemption_timer_limit();
+               preemption_timer_max_value = calc_preemption_timer_max_value();
 
                if (tsc_khz)
                        use_timer_freq = (u64)tsc_khz * 1000;
@@ -8349,8 +8356,7 @@ static __init void vmx_setup_preemption_timer(void)
                 * value.  Don't use the timer if it might cause spurious exits
                 * at a rate faster than 0.1 Hz (of uninterrupted guest time).
                 */
-               if (!preemption_timer_limit ||
-                   use_timer_freq > (preemption_timer_limit - 1) / 10)
+               if (use_timer_freq > preemption_timer_max_value / 10)
                        enable_preemption_timer = false;
        }
 
@@ -8392,7 +8398,7 @@ int vmx_set_hv_timer(struct kvm_vcpu *vcpu, u64 guest_deadline_tsc,
         * It's possible that it fits on later vmentries, but checking
         * on every vmentry is costly so we just use an hrtimer.
         */
-       if ((delta_tsc >> cpu_preemption_timer_multi) >= preemption_timer_limit)
+       if ((delta_tsc >> cpu_preemption_timer_multi) > preemption_timer_max_value)
                return -ERANGE;
 
        vmx->hv_deadline_tsc = tscl + delta_tsc;
@@ -8426,7 +8432,7 @@ static void vmx_update_hv_timer(struct kvm_vcpu *vcpu, bool force_immediate_exit
                vmcs_write32(VMX_PREEMPTION_TIMER_VALUE, delta_tsc);
                vmx->loaded_vmcs->hv_timer_soft_disabled = false;
        } else if (!vmx->loaded_vmcs->hv_timer_soft_disabled) {
-               vmcs_write32(VMX_PREEMPTION_TIMER_VALUE, preemption_timer_limit - 1);
+               vmcs_write32(VMX_PREEMPTION_TIMER_VALUE, preemption_timer_max_value);
                vmx->loaded_vmcs->hv_timer_soft_disabled = true;
        }
 }