Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] Moving kvmclock_disable() for the BSP to x86_platform.save_sched_clock_state leaves kvmclock enabled across hibernation if the callback is overwritten by another clocksource (e.g., Hyper-V), leading to memory corruption. --
commit 1ada3230c82e463b5d5311eb45fc919a4e06098c Author: Sean Christopherson <[email protected]> x86/kvm: Don't disable kvmclock on BSP in syscore_suspend() This commit changes the suspend logic to avoid disabling kvmclock on the BSP during syscore_suspend(). It defers the operation to the sched_clock restore callback and introduces a new action enum to handle different offline and suspend cases. > diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c > index df95516a9d899..006e3a13500b4 100644 > --- a/arch/x86/kernel/kvmclock.c > +++ b/arch/x86/kernel/kvmclock.c [ ... ] > @@ -178,8 +178,22 @@ static void kvm_register_clock(char *txt) > pr_debug("kvm-clock: cpu %d, msr %llx, %s", smp_processor_id(), pa, > txt); > } > > +static void kvmclock_disable(void) > +{ > + if (msr_kvm_system_time) > + native_write_msr(msr_kvm_system_time, 0); > +} > + > static void kvm_save_sched_clock_state(void) > { > + /* > + * Stop host writes to kvmclock immediately prior to suspend/hibernate. > + * If the system is hibernating, then kvmclock will likely reside at a > + * different physical address when the system awakens, and host writes > + * to the old address prior to reconfiguring kvmclock would clobber > + * random memory. > + */ > + kvmclock_disable(); > } Can this lead to memory corruption if another clocksource overwrites the x86_platform.save_sched_clock_state callback? In environments where KVM emulates Hyper-V, both KVM and Hyper-V clocksources might be initialized. If Hyper-V overwrites x86_platform.save_sched_clock_state with its own callback, and previous callbacks are not chained or invoked, kvm_save_sched_clock_state() will never be called. Because kvmclock clears its clocksource.suspend hook when registering as sched_clock, the clocksource framework cannot act as a fallback. This would leave kvmclock enabled on the BSP across hibernation. As mentioned in the comment, could this result in the hypervisor writing to the old physical memory address after resume and clobbering random memory? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
