From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Magenheimer Subject: RE: [PATCH] x86: don't write_tsc() non-zero values on CPUs updating only the lower 32 bits Date: Thu, 14 Apr 2011 15:41:20 -0700 (PDT) Message-ID: <7a5514b0-b3d6-478a-9434-b80758a7445d@default> References: <4DA6C735020000780003B8C0@vpn.id2.novell.com C9CC7A6A.16558%keir.xen@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser , Jan Beulich Cc: xen-devel@lists.xensource.com, "winston.l.wang" List-Id: xen-devel@lists.xenproject.org > -----Original Message----- > From: Keir Fraser [mailto:keir.xen@gmail.com] > Sent: Thursday, April 14, 2011 3:18 AM > To: Jan Beulich > Cc: winston.l.wang; xen-devel@lists.xensource.com; Dan Magenheimer > Subject: Re: [Xen-devel] [PATCH] x86: don't write_tsc() non-zero values > on CPUs updating only the lower 32 bits >=20 > On 14/04/2011 09:06, "Jan Beulich" wrote: >=20 > >> For physically-added CPUs only. Kind of unavoidable, that one: we > can only > >> try to do our best in that case. And let's face it, that probably > affects > >> exactly zero production users of Xen/x86 right now. > > > > That latter part I agree to. > > > > But what are you afraid of? Probing the TSC write shouldn't do any > > harm. >=20 > You will end up with a BSP TSC value different than what it would have > been > if you had not probed. Since you write back a (slightly) stale TSC > value. > Which would increase cross-CPU TSC skew if the platform has done a good > sync > job at power-on. >=20 > Now, do I personally think this much matters? Not really, since I > believe > that direct guest TSC access is a huge non-issue. But it is something > that > Dan disagreed on, he did a bunch of work on time management, and the > code > as-is does try quite hard to avoid writing TSC if at all possible. I > don't > think it's a good idea to change this without feedback from Dan, at > least. My feedback is don't break what is fixed ;-) The CPU vendors are trying REALLY hard to make TSC a very fast synchronized-across-all-CPUs clocksource and Linux now does use it that way if TSC_RELIABLE is set. While it's not perfect, it's close enough as long as your recent vintage machine doesn't have hot-plug CPUs or buggy firmware. =20 > > Additionally, did you read the comment immediately preceding > > the probing code? AMD doesn't guarantee the TSC to be writable at > > all. > > > >>> cstate_restore_tsc() also has no such gating afaics. > >> > >> It is gated on NONSTOP_TSC which is implied by TSC_RELIABLE. > > > > Ah, yes. But (I think) not architecturally, only by virtue of how > > code is currently structured. If that changes, we'd be back at a > > latent (and quite non-obvious) bug. >=20 > Yeah, if we want to continue to try avoiding write_tsc() on > TSC_RELIABLE > then we should assert !TSC_RELIABLE on the write_tsc() path in > cstate_tsc_restore(). Agreed. In fact, maybe it should be asserted in write_tsc? Dan