From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH] x86: don't write_tsc() non-zero values on CPUs updating only the lower 32 bits Date: Thu, 14 Apr 2011 09:06:45 +0100 Message-ID: <4DA6C735020000780003B8C0@vpn.id2.novell.com> References: <4DA6C16F020000780003B899@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: Content-Disposition: inline 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 Cc: Dan Magenheimer , "xen-devel@lists.xensource.com" , "winston.l.wang" List-Id: xen-devel@lists.xenproject.org >>> On 14.04.11 at 09:50, Keir Fraser wrote: > On 14/04/2011 08:42, "Jan Beulich" wrote: >=20 >>>>> On 14.04.11 at 09:25, Keir Fraser wrote: >>> On 14/04/2011 08:18, "Jan Beulich" wrote: >>>=20 >>>> This means suppressing the uses in time_calibration_tsc_rendezvous(), >>>> cstate_restore_tsc(), and synchronize_tsc_slave(), and fixes a boot >>>> hang of Linux Dom0 when loading processor.ko on such systems that >>>> have support for C states above C1. >>>=20 >>> Should your new test be gated on !X86_FEATURE_TSC_RELIABLE? We already >>=20 >> Which test? The write-TSC-probe itself? >>=20 >>> *never* write the TSC when boot_cpu_has(TSC_RELIABLE) -- Dan Magenheime= r >>> made that change on the assumption that TSCs were globally synced by >>> firmware in this case, and us writing one or more TSCs could only ever = make >>> things worse. >>=20 >> That's not true - we only avoid the writing for TSC sync during boot. >> Post-boot bringup of CPUs will write the TSC no matter what, and >=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. 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. >=20 > 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. Jan