On Fri, May 15, 2026, [email protected] wrote:
> 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 <[email protected]>
> 
> 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.

Ugh, TIL.  That's quite evil.  I'll fix in the next version.

Reply via email to