Re: [PATCH v5 12/51] x86/acrn: Register TSC/CPU frequency callbacks iff frequency is actually in CPUID

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] ACRN hypervisor integration statically hardcodes its CPUID leaf to 0x40000010 without applying the dynamic base offset, breaking nested setups and potentially causing severe timekeeping corruption.
--

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

x86/acrn: Register TSC/CPU frequency callbacks iff frequency is actually in CPUID

This commit modifies ACRN initialization to only register the TSC and CPU
frequency override callbacks if the hypervisor actually provides a non-zero
frequency in its CPUID leaf. It achieves this by reading the frequency
during platform initialization and caching the result.

> diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
> index ad8f2da8003b3..dc71a6fdd4613 100644
> --- a/arch/x86/kernel/cpu/acrn.c
> +++ b/arch/x86/kernel/cpu/acrn.c
[ ... ]
> @@ -26,13 +28,19 @@ static u32 __init acrn_detect(void)
>  
>  static unsigned int __init acrn_get_tsc_khz(void)
>  {
> -	return cpuid_eax(ACRN_CPUID_TIMING_INFO);
> +	return acrn_tsc_khz_cpuid;
>  }
>  
>  static void __init acrn_init_platform(void)
>  {
>  	/* Install system interrupt handler for ACRN hypervisor callback */
>  	sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_acrn_hv_callback);
> +
> +	acrn_tsc_khz_cpuid = cpuid_eax(ACRN_CPUID_TIMING_INFO);

[Severity: High]
This is a pre-existing issue, but does hardcoding ACRN_CPUID_TIMING_INFO to
0x40000010 without dynamically applying the base offset break nested setups?

If Linux is booted as an L2 guest with an L0 hypervisor like KVM occupying
the base 0x40000000 leaf, and L1 ACRN resides at 0x40000100, acrn_detect()
correctly finds ACRN using cpuid_base_hypervisor() at the 0x40000100 base.

However, acrn_init_platform() hardcodes the fetch:

arch/x86/include/asm/acrn.h:
#define ACRN_CPUID_TIMING_INFO        0x40000010

Will this bypass the dynamic base detected earlier and read the timing info
from the wrong hypervisor's CPUID space (e.g., KVM's L0 frequency)?

If nested under a hypervisor that returns garbage for 0x40000010, could
acrn_tsc_khz_cpuid be populated with a non-zero bogus value and corrupt
guest timekeeping?

> +	if (acrn_tsc_khz_cpuid) {
> +		x86_init.hyper.get_tsc_khz = acrn_get_tsc_khz;
> +		x86_init.hyper.get_cpu_khz = acrn_get_tsc_khz;
> +	}
>  }
>  
>  static bool acrn_x2apic_available(void)

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