Re: [PATCH] x86: don't write_tsc() non-zero values on CPUs updating only the lower 32 bits

[Keir Fraser <keir.xen@gmail.com>]

On 14/04/2011 09:06, "Jan Beulich" <JBeulich@novell.com> wrote:

>> 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.

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.

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.

> 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.

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().

 -- Keir