Re: [PATCH 15/16] x86/kvmclock: Stuff local APIC bus period when core crystal freq comes from CPUID

[Sean Christopherson <seanjc@google.com>]

On Tue, Feb 04, 2025, Dan Carpenter wrote:
> Hi Sean,
> 
> kernel test robot noticed the following build warnings:
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Sean-Christopherson/x86-tsc-Add-a-standalone-helpers-for-getting-TSC-info-from-CPUID-0x15/20250201-102521
> base:   ebbb8be421eefbe2d47b99c2e1a6dd840d7930f9
> patch link:    https://lore.kernel.org/r/20250201021718.699411-16-seanjc%40google.com
> patch subject: [PATCH 15/16] x86/kvmclock: Stuff local APIC bus period when core crystal freq comes from CPUID
> config: i386-randconfig-141-20250204 (https://download.01.org/0day-ci/archive/20250204/202502041246.3weiY0qo-lkp@intel.com/config)
> compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202502041246.3weiY0qo-lkp@intel.com/
> 
> smatch warnings:
> arch/x86/kernel/kvmclock.c:117 kvm_get_tsc_khz() error: uninitialized symbol 'crystal_khz'.
> 
> vim +/crystal_khz +117 arch/x86/kernel/kvmclock.c
> 
> aea5814308ed7c Sean Christopherson 2025-01-31  105  static unsigned long kvm_get_tsc_khz(void)
> aea5814308ed7c Sean Christopherson 2025-01-31  106  {
> aea5814308ed7c Sean Christopherson 2025-01-31  107  	unsigned int __tsc_khz, crystal_khz;
> aea5814308ed7c Sean Christopherson 2025-01-31  108  
> 32cd84ebbd5dbd Sean Christopherson 2025-01-31  109  	/*
> 32cd84ebbd5dbd Sean Christopherson 2025-01-31  110  	 * Prefer CPUID over kvmclock when possible, as CPUID also includes the
> 32cd84ebbd5dbd Sean Christopherson 2025-01-31  111  	 * core crystal frequency, i.e. the APIC timer frequency.  When the core
> 32cd84ebbd5dbd Sean Christopherson 2025-01-31  112  	 * crystal frequency is enumerated in CPUID.0x15, KVM's ABI is that the
> 32cd84ebbd5dbd Sean Christopherson 2025-01-31  113  	 * (virtual) APIC BUS runs at the same frequency.
> 32cd84ebbd5dbd Sean Christopherson 2025-01-31  114  	 */
> 32cd84ebbd5dbd Sean Christopherson 2025-01-31  115  	if (!cpuid_get_tsc_freq(&__tsc_khz, &crystal_khz)) {
> 
> The cpuid_get_tsc_freq() function has a bug so it doesn't actually
> initialize crystal_khz.  It actually uses the uninitialized value, inside
> the function but Smatch doesn't catch that.

Argh.  I caught this in testing and fixed it locally, but clobbered my branch
and accidentally reverted to the broken code when posting.  The bug is way back
in patch 1, I'll point it out there.

Thanks!

> 32cd84ebbd5dbd Sean Christopherson 2025-01-31  116  #ifdef CONFIG_X86_LOCAL_APIC
> 32cd84ebbd5dbd Sean Christopherson 2025-01-31 @117  		lapic_timer_period = crystal_khz * 1000 / HZ;
> 32cd84ebbd5dbd Sean Christopherson 2025-01-31  118  #endif
> aea5814308ed7c Sean Christopherson 2025-01-31  119  		return __tsc_khz;
> 32cd84ebbd5dbd Sean Christopherson 2025-01-31  120  	}
> aea5814308ed7c Sean Christopherson 2025-01-31  121  
> aea5814308ed7c Sean Christopherson 2025-01-31  122  	return pvclock_tsc_khz(this_cpu_pvti());
> aea5814308ed7c Sean Christopherson 2025-01-31  123  }
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>