Re: [PATCH v3 2/3] x86/hyperv: move TSC reading method to asm/mshyperv.h

From: Thomas Gleixner
Date: Sat Mar 04 2017 - 04:07:47 EST


On Fri, 3 Mar 2017, Stephen Hemminger wrote:
> static inline u64 hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg)
> {
> u64 scale, offset, cur_tsc;
> u32 start;
>
> /*
> * The protocol for reading Hyper-V TSC page is specified in Hypervisor
> * Top-Level Functional Specification ver. 3.0 and above. To get the
> * reference time we must do the following:
> * - READ ReferenceTscSequence
> * A special '0' value indicates the time source is unreliable and we
> * need to use something else. The currently published specification
> * versions (up to 4.0b) contain a mistake and wrongly claim '-1'
> * instead of '0' as the special value, see commit c35b82ef0294.
> * - ReferenceTime =
> * ((RDTSC() * ReferenceTscScale) >> 64) + ReferenceTscOffset
> * - READ ReferenceTscSequence again. In case its value has changed
> * since our first reading we need to discard ReferenceTime and repeat
> * the whole sequence as the hypervisor was updating the page in
> * between.
> */
> do {
> start = READ_ONCE(tsc_pg->tsc_sequence);
> smp_rmb();
>
> if (unlikely(!start))
> return U64_MAX;
>
> scale = tsc_pg->tsc_scale;
> offset = tsc_pg->tsc_offset;
>
> /*
> * Make sure we read sequence after we read all other values
> * from TSC page.
> */
> smp_rmb();
> } while (unlikely(READ_ONCE(tsc_pg->tsc_sequence != start)));
>
> cur_tsc = rdtsc_ordered();

That's wrong. You need to read the TSC value together with the scale and
offset. That's needs to be "atomic". You can only do the mult/shift
outside.

> return mul_u64_u64_shr(cur_tsc, scale, 64) + offset;
> }

Thanks,

tglx