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