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: <20241211180035.GA20385@skinsburskii.>
Date: Wed, 11 Dec 2024 10:00:35 -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 26, 2024 at 06:11:10AM +0000, Michael Kelley wrote:
> From: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com> Sent: Monday, November 25, 2024 2:25 PM
> > 
> > On Fri, Nov 22, 2024 at 06:33:12PM +0000, Michael Kelley wrote:
> > > From: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com> Sent: Tuesday,
> > November 19, 2024 4:51 PM
> > > >
> > > > 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.
> > >
> > > I have trouble keeping all the different TSC "features" conceptually
> > > separate. :-( The TSC frequency not changing (and the value not
> > > abruptly jumping?) should already be represented by
> > > X86_FEATURE_TSC_CONSTANT.  In the kernel, X86_FEATURE_TSC_RELIABLE
> > > effectively only controls whether the TSC clocksource watchdog is
> > > enabled, and in spite of the live migration foibles, I don't see a need
> > > for that watchdog in a Hyper-V VM. So maybe it's OK to always set
> > > X86_FEATURE_TSC_RELIABLE in a Hyper-V VM, as you have
> > > proposed.
> > >
> > > The "tsc_reliable" flag is also exposed to user space as part of the
> > > /proc/cpuinfo "flags" output, so theoretically some user space
> > > program could change behavior based on that flag. But that seems
> > > a bit far-fetched. I know there are user space programs that check
> > > the CPUID INVARIANT_TSC flag to know whether they can use
> > > the raw RDTSC instruction output to do start/stop timing. The
> > > Hyper-V TSC Invariant feature makes that work correctly, even
> > > across live migrations.
> > >
> > 
> > It sounds to me that if X86_FEATURE_TSC_CONSTANT is available
> > on Hyper-V, then we can set X86_FEATURE_TSC_RELIABLE.
> > Is it what you are saying?
> > 
> 
> No. Sorry I wasn't clear. X86_FEATURE_TSC_CONSTANT will
> be set only when the Hyper-V TSC Invariant feature is enabled, so
> tying X86_FEATURE_TSC_RELIABLE to that is what happens now.
> 
> What I'm suggesting is to take your patch "as is". In other words,
> always enable X86_FEATURE_TSC_RELIABLE. From what I can tell,
> TSC_RELIABLE is only used to disable the TSC watchdog. Since I
> can't see a use for the TSC watchdog in a VM, always setting
> TSC_RELIABLE probably makes sense. TSC_RELIABLE doesn't
> say anything about whether the TSC frequency might change, such
> as across a VM live migration. TSC_CONSTANT is what tells you that
> the frequency won't change.
> 
> My caveat is that I don't know the history of TSC_RELIABLE. I
> don't see any documentation on the details of what it is supposed
> to convey, especially in a VM. Maybe someone on the "To:" list
> who knows for sure can confirm what I'm thinking.
> 
> Michael

We had a long ionternal discussion with hypervisor folks and it looks
like we will propose a more robust solution to go forward.
The hypervisor will provide an additional CPUID bit, which guarantees
TSC reliability (including across live migration).

I'll prepare an updated version.

Thanks,
Stanislav

> 
> 
> 
>