Re: [PATCH v3 29/41] x86/paravirt: Plumb a return code into __paravirt_set_sched_clock()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm_sched_clock_read is registered as the system sched_clock before its required offset (kvm_sched_clock_offset) is initialized.
--

commit b6e429e4990c00d845488d46e8dc4132f3a2fd8b
Author: Sean Christopherson <seanjc@google.com>
x86/paravirt: Plumb a return code into __paravirt_set_sched_clock()

This commit adds a return code to __paravirt_set_sched_clock() to allow the
kernel to reject attempts to use a PV sched_clock. It updates the caller
in kvmclock to check the return value and abort initialization if needed.

> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index d3bb281c0805d..9b3d1ed1a96dc 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
[ ... ]
> @@ -338,10 +338,12 @@ static int kvmclock_setup_percpu(unsigned int cpu)
>  
>  static __init void kvm_sched_clock_init(bool stable)
>  {
> +	if (__paravirt_set_sched_clock(kvm_sched_clock_read, stable,
> +				       kvm_save_sched_clock_state,
> +				       kvm_restore_sched_clock_state))
> +		return;
> +
>  	kvm_sched_clock_offset = kvm_clock_read();

Could this cause a massive backward jump in system time if an NMI or early
exception calls sched_clock() right after registration?

__paravirt_set_sched_clock() uses static_call_update() to globally publish
kvm_sched_clock_read() as the live system sched_clock. Since the
initialization of kvm_sched_clock_offset is now delayed until after this
registration completes, any early caller will execute kvm_sched_clock_read()
with a zero offset, returning a huge absolute raw clock value.

When the code resumes, it sets the offset to the current raw clock value.
Subsequent sched_clock() calls will abruptly subtract this large offset,
causing time to jump backwards and violating monotonic clock guarantees.

Would it be safer to initialize kvm_sched_clock_offset before calling
__paravirt_set_sched_clock(), since calculating the offset early has no
side effects even if the registration is subsequently rejected?

> -	__paravirt_set_sched_clock(kvm_sched_clock_read, stable,
> -				   kvm_save_sched_clock_state,
> -				   kvm_restore_sched_clock_state);
>  	kvmclock_is_sched_clock = true;
>  
>  	/*

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