Re: [PATCH v3 11/41] x86/kvm: Don't disable kvmclock on BSP in syscore_suspend()

[sashiko-bot@kernel.org]

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 <seanjc@google.com>

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/20260515191942.1892718-1-seanjc@google.com?part=11