linux-kernel - RE: [PATCH v2 10/38] clocksource: hyper-v: Don't save/restore TSC offset when using HV sched

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <SN6PR02MB415743496C3A9937D10E4F4FD4C82@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Tue, 4 Mar 2025 04:17:54 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Sean Christopherson <seanjc@...gle.com>, Thomas Gleixner
	<tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov
	<bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>, "x86@...nel.org"
	<x86@...nel.org>, "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Paolo Bonzini <pbonzini@...hat.com>, Juergen Gross <jgross@...e.com>, "K. Y.
 Srinivasan" <kys@...rosoft.com>, Haiyang Zhang <haiyangz@...rosoft.com>, Wei
 Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>, Ajay Kaher
	<ajay.kaher@...adcom.com>, Jan Kiszka <jan.kiszka@...mens.com>, Andy
 Lutomirski <luto@...nel.org>, Peter Zijlstra <peterz@...radead.org>, Daniel
 Lezcano <daniel.lezcano@...aro.org>, John Stultz <jstultz@...gle.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "virtualization@...ts.linux.dev"
	<virtualization@...ts.linux.dev>, "linux-hyperv@...r.kernel.org"
	<linux-hyperv@...r.kernel.org>, "xen-devel@...ts.xenproject.org"
	<xen-devel@...ts.xenproject.org>, Tom Lendacky <thomas.lendacky@....com>,
	Nikunj A Dadhania <nikunj@....com>
Subject: RE: [PATCH v2 10/38] clocksource: hyper-v: Don't save/restore TSC
 offset when using HV sched_clock

From: Sean Christopherson <seanjc@...gle.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@...gle.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@...look.com>
Tested-by: Michael Kelley <mhklinux@...look.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
>