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. 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/[email protected]?part=36
