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 17:28, "Jan Beulich" <JBeulich@novell.com> wrote:

> 
>> I also simplified the actual writability check itself. I couldn't figure out
>> what the benefit of your more complex approach would be. In fact it looked
>> like it wouldn't work if bit 32 was set already in the TSC counter, as then
>> you would write back an unmodified TSC (and in fact you would detect the
>> wrong way round, as you'd see a big delta if the write silently cleared bit
>> 32 (and bits 33-63)). And the final write of tsc+4*delta, wasn't sure what
>> that was about either! But if you can explain why your test is better I'd be
>> happy to use it as you originally wrote it.
> 
> So you were concerned about getting the TSC slightly off, and now
> you flush it to zero, without any attempt to restore the original
> value?

Haha, well it doesn't matter too much if we sync TSCs as we bring them
online anyway. But I agree it makes sense to try if we are only able to
write the lower 32 bits -- we can at least hope the write test happens while
TSC counter is a 32-bit value anyway, and at least we've had a best-effort
attempt to keep TSCs in sync.

> As to my original test being broken - I don't think that was the case:
> The first write used (u32)tsc as the input, so the two writes, if
> happening completely, would be certain to be apart by
> approximately 1<<32 (or more, depending on what was in the
> upper half originally).

Ah yes, I missed the importance of the (u32)tsc write. Fair enough, your way
is better. :-)

> The only case not handled was if the TSC
> overflowed 64 bits during the process - I considered this case
> hypothetical only.
> 
> The final write of tsc+4*delta was basically an attempt to restore
> the value that would have been there if we didn't fiddle with it.

But the write is actually tsc + 4*(s32)(tmp-tsc), and tmp has 1U<<32 ORed
into it (because it was read after your second write to the TSC. Perhaps we
should just write back the full original tsc and call that good enough?

 -- Keir


> The factor 4 was sort of invented, on the basis that the delta was
> between one write and an immediate read back, with there being
> a total of four such windows (read->write or write->read). As
> one wouldn't get it precise anyway, the number seemed fine to
> me, though just writing back the original values probably wouldn't
> have been much worse.

> Jan
>