Re: [PATCH v5 01/51] x86/apic: Provide helpers to set local APIC timer period in hz and khz
From: sashiko-bot@kernel.org
Date: 2026-07-01 19:46:21
Also in:
kvm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The `apic_set_timer_period_khz()` and `apic_set_timer_period_hz()` helper functions accept a `const char *source` argument which is never used, resulting in dead code and a silent loss of boot diagnostic logging on platforms where logging was removed. -- commit 4fb576b0d909663c9b3e6628dbed45d329c08a5d Author: Sean Christopherson [off-list ref] x86/apic: Provide helpers to set local APIC timer period in hz and khz This commit adds APIs to set the local APIC timer period, abstracting the HZ math previously open-coded by external callers. It also changes the lapic_timer_period variable to be local to apic.c.
quoted hunk ↗ jump to hunk
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c index aa1e19979aa8e..8d3d930576fd7 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -796,6 +796,16 @@ bool __init apic_needs_pit(void) return lapic_timer_period == 0; } +void apic_set_timer_period_khz(u64 period_khz, const char *source) +{ + lapic_timer_period = mul_u64_u32_div(period_khz, 1000, HZ); +} + +void apic_set_timer_period_hz(u64 period_hz, const char *source) +{ + lapic_timer_period = div_u64(period_hz, HZ); +}
[Severity: Medium] It looks like the source parameter is accepted by apic_set_timer_period_khz() and apic_set_timer_period_hz() in arch/x86/kernel/apic/apic.c but is never used. Was it intended to log the frequency using this source string?
quoted hunk ↗ jump to hunk
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c index 185d4f677ec0f..87beecec76f04 100644 --- a/arch/x86/kernel/cpu/mshyperv.c +++ b/arch/x86/kernel/cpu/mshyperv.c@@ -646,10 +646,7 @@ static void __init ms_hyperv_init_platform(void) u64 hv_lapic_frequency; rdmsrq(HV_X64_MSR_APIC_FREQUENCY, hv_lapic_frequency); - hv_lapic_frequency = div_u64(hv_lapic_frequency, HZ); - lapic_timer_period = hv_lapic_frequency; - pr_info("Hyper-V: LAPIC Timer Frequency: %#x\n", - lapic_timer_period); + apic_set_timer_period_hz(hv_lapic_frequency, "Hyper-V hypervisor"); }
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c index 34b73573b108d..36f779dd311dc 100644 --- a/arch/x86/kernel/cpu/vmware.c +++ b/arch/x86/kernel/cpu/vmware.c@@ -424,9 +424,7 @@ static void __init vmware_platform_setup(void) #ifdef CONFIG_X86_LOCAL_APIC /* Skip lapic calibration since we know the bus frequency. */ - lapic_timer_period = ecx / HZ; - pr_info("Host bus clock speed read from hypervisor : %u Hz\n", - ecx); + apic_set_timer_period_hz(ecx, "VMware hypervisor"); #endif
[Severity: Medium] These changes remove the pr_info() logging of the APIC frequency during early boot APIC initialization in ms_hyperv_init_platform() (arch/x86/kernel/cpu/mshyperv.c) and vmware_platform_setup() (arch/x86/kernel/cpu/vmware.c). Since the new apic_set_timer_period_hz() helper does not log this information, does this result in an unintended loss of boot diagnostic logging on these platforms? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260701193212.749551-1-seanjc@google.com?part=1