linux-kernel - Re: [PATCH] x86/hyperv: Set X86_FEATURE_TSC

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: <20241120005106.GA18115@skinsburskii.>
Date: Tue, 19 Nov 2024 16:51:06 -0800
From: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>
To: Michael Kelley <mhklinux@...look.com>
Cc: "tglx@...utronix.de" <tglx@...utronix.de>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"bp@...en8.de" <bp@...en8.de>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"kys@...rosoft.com" <kys@...rosoft.com>,
	"haiyangz@...rosoft.com" <haiyangz@...rosoft.com>,
	"wei.liu@...nel.org" <wei.liu@...nel.org>,
	"decui@...rosoft.com" <decui@...rosoft.com>,
	"x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86/hyperv: Set X86_FEATURE_TSC_RELIABLE unconditionally

On Tue, Nov 12, 2024 at 07:48:06PM +0000, Michael Kelley wrote:
> From: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com> Sent: Tuesday, November 12, 2024 10:18 AM
> > 
> > Enable X86_FEATURE_TSC_RELIABLE by default as X86_FEATURE_TSC_RELIABLE is
> > independent from invariant TSC and should have never been gated by the
> > HV_ACCESS_TSC_INVARIANT privilege.
> 
> I think originally X86_FEATURE_TSC_RELIABLE was gated by the Hyper-V
> TSC Invariant feature because otherwise VM live migration may cause
> the TSC value reported by the RDTSC/RDTSCP instruction in the guest
> to abruptly change frequency and value. In such cases, the TSC isn't
> useable by the kernel or user space.
> 
> Enabling the Hyper-V TSC Invariant feature fixes that by using the
> hardware scaling available in more recent processors to automatically
> fixup the TSC value returned by RDTSC/RDTSCP in the guest.
> 
> Is there a practical problem that is fixed by always enabling
> X86_FEATURE_TSC_RELIABLE?
> 

The particular problem is that HV_ACCESS_TSC_INVARIANT is not set for the
nested root, which in turn leads to keeping tsc clocksource watchdog
thread and TSC sycn check timer around.

But the live migration concern you raised is indeed still out there.

Thank you for the input Michael, I'll think more about it.

Stanislav

> Michael
> 
> > 
> > To elaborate, the HV_ACCESS_TSC_INVARIANT privilege allows certain types of
> > guests to opt-in to invariant TSC by writing the
> > HV_X64_MSR_TSC_INVARIANT_CONTROL register. Not all guests will have this
> > privilege and the hypervisor will automatically opt-in certain types of
> > guests (e.g. EXO partitions) to invariant TSC, but this functionality is
> > unrelated to the TSC reliability.
> > 
> > Signed-off-by: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>
> > ---
> >  arch/x86/kernel/cpu/mshyperv.c |    6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> > index d18078834ded..14412afcc398 100644
> > --- a/arch/x86/kernel/cpu/mshyperv.c
> > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > @@ -515,7 +515,7 @@ static void __init ms_hyperv_init_platform(void)
> >  	machine_ops.crash_shutdown = hv_machine_crash_shutdown;
> >  #endif
> >  #endif
> > -	if (ms_hyperv.features & HV_ACCESS_TSC_INVARIANT) {
> > +	if (ms_hyperv.features & HV_ACCESS_TSC_INVARIANT)
> >  		/*
> >  		 * Writing to synthetic MSR 0x40000118 updates/changes the
> >  		 * guest visible CPUIDs. Setting bit 0 of this MSR  enables
> > @@ -526,8 +526,8 @@ static void __init ms_hyperv_init_platform(void)
> >  		 * is called.
> >  		 */
> >  		wrmsrl(HV_X64_MSR_TSC_INVARIANT_CONTROL, HV_EXPOSE_INVARIANT_TSC);
> > -		setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> > -	}
> > +
> > +	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> > 
> >  	/*
> >  	 * Generation 2 instances don't support reading the NMI status from
> > 
> > 
>