Re: [PATCH v3 36/41] x86/kvmclock: Get local APIC bus frequency from PV CPUID Timing Info

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] lapic_timer_period is incorrectly assigned the raw APIC frequency in kHz, which fails to account for the kernel's HZ configuration and breaks APIC timer calibration for any HZ != 1000.
--

commit e5bbef05c7ba30fb00c716accae48693e295289a
Author: Sean Christopherson <seanjc@google.com>

x86/kvmclock: Get local APIC bus frequency from PV CPUID Timing Info

This commit reads the local APIC bus frequency from the PV CPUID leaf
and uses it to directly set lapic_timer_period, bypassing the standard
calibration process.

> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
[ ... ]
> @@ -200,10 +200,19 @@ void kvmclock_cpu_action(enum kvm_guest_cpu_action action)
>   */
>  static unsigned long kvm_get_tsc_khz(void)
>  {
> +#ifdef CONFIG_X86_LOCAL_APIC
> +	u32 apic_khz = kvm_para_apic_bus_khz();
> +
>  	/*
> -	 * If KVM advertises the frequency directly in CPUID, use that
> -	 * instead of reverse-calculating it from the KVM clock data.
> +	 * Use the TSC frequency from KVM's (and other hypervisors') PV CPUID
> +	 * leaf when available, instead of reverse-calculating it from the KVM
> +	 * clock data.  As a bonus, the CPUID leaf also includes the local APIC
> +	 * bus/timer frequency.
>  	 */
> +	if (apic_khz)
> +		lapic_timer_period = apic_khz;

Does setting lapic_timer_period directly to apic_khz assume that HZ is
always configured to 1000?

The lapic_timer_period variable represents the number of APIC timer ticks
per jiffy. This is usually calculated as (frequency_in_hz / HZ), which would
be (apic_khz * 1000) / HZ.

Assigning the value of apic_khz directly means the period is exactly
frequency_in_hz / 1000. If the kernel is built with HZ set to 100 or 250,
could this result in a timer period that is too small, potentially causing
timer interrupts to fire much more frequently than intended?

> +#endif
> +
>  	return kvm_para_tsc_khz() ? : pvclock_tsc_khz(this_cpu_pvti());
>  }

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