| Message ID | 20250227021855.3257188-11-seanjc@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86: Try to wrangle PV clocks vs. TSC | expand |
From: Sean Christopherson <seanjc@google.com> Sent: Wednesday, February 26, 2025 6:18 PM > > Now that Hyper-V overrides the sched_clock save/restore hooks if and only > sched_clock itself is set to the Hyper-V timer, drop the invocation of the > "old" save/restore callbacks. When the registration of the PV sched_clock > was done separate from overriding the save/restore hooks, it was possible > for Hyper-V to clobber the TSC save/restore callbacks without actually > switching to the Hyper-V timer. Terminology again. > > Enabling a PV sched_clock is a one-way street, i.e. the kernel will never > revert to using TSC for sched_clock, and so there is no need to invoke the > TSC save/restore hooks (and if there was, it belongs in common PV code). > > Signed-off-by: Sean Christopherson <seanjc@google.com> I've tested the full patch series in a Hyper-V guest on x86 with and without the Hyper-V TSC_INVARIANT option. In both cases, the sched_clock is correctly maintained across hibernation and resume from hibernation, and the "flags" in /proc/cpuinfo are as expected. I also compiled hyperv_timer.c on arm64, and that is clean with no errors. So all is good. Modulo the terminology used in the commit message, Reviewed-by: Michael Kelley <mhklinux@outlook.com> Tested-by: Michael Kelley <mhklinux@outlook.com> > --- > drivers/clocksource/hyperv_timer.c | 10 ---------- > 1 file changed, 10 deletions(-) > > diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c > index 5a52d0acf31f..4a21874e91b9 100644 > --- a/drivers/clocksource/hyperv_timer.c > +++ b/drivers/clocksource/hyperv_timer.c > @@ -524,9 +524,6 @@ static __always_inline void hv_setup_sched_clock(void > *sched_clock) > } > #elif defined CONFIG_PARAVIRT > static u64 hv_ref_counter_at_suspend; > -static void (*old_save_sched_clock_state)(void); > -static void (*old_restore_sched_clock_state)(void); > - > /* > * Hyper-V clock counter resets during hibernation. Save and restore clock > * offset during suspend/resume, while also considering the time passed > @@ -536,8 +533,6 @@ static void (*old_restore_sched_clock_state)(void); > */ > static void hv_save_sched_clock_state(void) > { > - old_save_sched_clock_state(); > - > hv_ref_counter_at_suspend = hv_read_reference_counter(); > } > > @@ -550,8 +545,6 @@ static void hv_restore_sched_clock_state(void) > * - reference counter (time) now. > */ > hv_sched_clock_offset -= (hv_ref_counter_at_suspend - hv_read_reference_counter()); > - > - old_restore_sched_clock_state(); > } > > static __always_inline void hv_setup_sched_clock(void *sched_clock) > @@ -559,10 +552,7 @@ static __always_inline void hv_setup_sched_clock(void > *sched_clock) > /* We're on x86/x64 *and* using PV ops */ > paravirt_set_sched_clock(sched_clock); > > - old_save_sched_clock_state = x86_platform.save_sched_clock_state; > x86_platform.save_sched_clock_state = hv_save_sched_clock_state; > - > - old_restore_sched_clock_state = x86_platform.restore_sched_clock_state; > x86_platform.restore_sched_clock_state = hv_restore_sched_clock_state; > } > #else /* !CONFIG_GENERIC_SCHED_CLOCK && !CONFIG_PARAVIRT */ > -- > 2.48.1.711.g2feabab25a-goog >
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c index 5a52d0acf31f..4a21874e91b9 100644 --- a/drivers/clocksource/hyperv_timer.c +++ b/drivers/clocksource/hyperv_timer.c @@ -524,9 +524,6 @@ static __always_inline void hv_setup_sched_clock(void *sched_clock) } #elif defined CONFIG_PARAVIRT static u64 hv_ref_counter_at_suspend; -static void (*old_save_sched_clock_state)(void); -static void (*old_restore_sched_clock_state)(void); - /* * Hyper-V clock counter resets during hibernation. Save and restore clock * offset during suspend/resume, while also considering the time passed @@ -536,8 +533,6 @@ static void (*old_restore_sched_clock_state)(void); */ static void hv_save_sched_clock_state(void) { - old_save_sched_clock_state(); - hv_ref_counter_at_suspend = hv_read_reference_counter(); } @@ -550,8 +545,6 @@ static void hv_restore_sched_clock_state(void) * - reference counter (time) now. */ hv_sched_clock_offset -= (hv_ref_counter_at_suspend - hv_read_reference_counter()); - - old_restore_sched_clock_state(); } static __always_inline void hv_setup_sched_clock(void *sched_clock) @@ -559,10 +552,7 @@ static __always_inline void hv_setup_sched_clock(void *sched_clock) /* We're on x86/x64 *and* using PV ops */ paravirt_set_sched_clock(sched_clock); - old_save_sched_clock_state = x86_platform.save_sched_clock_state; x86_platform.save_sched_clock_state = hv_save_sched_clock_state; - - old_restore_sched_clock_state = x86_platform.restore_sched_clock_state; x86_platform.restore_sched_clock_state = hv_restore_sched_clock_state; } #else /* !CONFIG_GENERIC_SCHED_CLOCK && !CONFIG_PARAVIRT */
Now that Hyper-V overrides the sched_clock save/restore hooks if and only sched_clock itself is set to the Hyper-V timer, drop the invocation of the "old" save/restore callbacks. When the registration of the PV sched_clock was done separate from overriding the save/restore hooks, it was possible for Hyper-V to clobber the TSC save/restore callbacks without actually switching to the Hyper-V timer. Enabling a PV sched_clock is a one-way street, i.e. the kernel will never revert to using TSC for sched_clock, and so there is no need to invoke the TSC save/restore hooks (and if there was, it belongs in common PV code). Signed-off-by: Sean Christopherson <seanjc@google.com> --- drivers/clocksource/hyperv_timer.c | 10 ---------- 1 file changed, 10 deletions(-)