Commit 77750f78 authored by Peter Zijlstra's avatar Peter Zijlstra
Browse files

x86/vdso: Fix gettimeofday masking 



Because of how the virtual clocks use U64_MAX as an exception value
instead of a valid time, the clocks can no longer be assumed to wrap
cleanly. This is then compounded by arch_vdso_cycles_ok() rejecting
everything with the MSB/Sign-bit set.

Therefore, the effective mask becomes S64_MAX, and the comment with
vdso_calc_delta() that states the mask is U64_MAX and isn't optimized
out is just plain silly.

Now, the code has a negative filter -- to deal with TSC wobbles:

	if (cycles > last)

which is just plain wrong, because it should've been written as:

	if ((s64)(cycles - last) > 0)

to take wrapping into account, but per all the above, we don't
actually wrap on u64 anymore.

Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: default avatarThomas Gleixner <tglx@linutronix.de>
Tested-by: default avatarThomas Gleixner <tglx@linutronix.de>
Tested-by: Michael Kelley <mikelley@microsoft.com>  # Hyper-V
Link: https://lore.kernel.org/r/20230519102715.704767397@infradead.org
parent fc4a0db4

arch/x86/include/asm/vdso/gettimeofday.h

+28 −11
Original line number Diff line number Diff line
@@ -231,14 +231,17 @@ static u64 vread_pvclock(void) 
		ret = __pvclock_read_cycles(pvti, rdtsc_ordered());

	} while (pvclock_read_retry(pvti, version));



	return ret;

	return ret & S64_MAX;

}

#endif



#ifdef CONFIG_HYPERV_TIMER

static u64 vread_hvclock(void)

{

	return hv_read_tsc_page(&hvclock_page);

	u64 ret = hv_read_tsc_page(&hvclock_page);

	if (likely(ret != U64_MAX))

		ret &= S64_MAX;

	return ret;

}

#endif
@@ -246,7 +249,7 @@ static inline u64 __arch_get_hw_counter(s32 clock_mode, 
					const struct vdso_data *vd)

{

	if (likely(clock_mode == VDSO_CLOCKMODE_TSC))

		return (u64)rdtsc_ordered();

		return (u64)rdtsc_ordered() & S64_MAX;

	/*

	 * For any memory-mapped vclock type, we need to make sure that gcc

	 * doesn't cleverly hoist a load before the mode check.  Otherwise we
@@ -284,6 +287,9 @@ static inline bool arch_vdso_clocksource_ok(const struct vdso_data *vd) 
 * which can be invalidated asynchronously and indicate invalidation by

 * returning U64_MAX, which can be effectively tested by checking for a

 * negative value after casting it to s64.

 *

 * This effectively forces a S64_MAX mask on the calculations, unlike the

 * U64_MAX mask normally used by x86 clocksources.

 */

static inline bool arch_vdso_cycles_ok(u64 cycles)

{
@@ -303,18 +309,29 @@ static inline bool arch_vdso_cycles_ok(u64 cycles) 
 * @last. If not then use @last, which is the base time of the current

 * conversion period.

 *

 * This variant also removes the masking of the subtraction because the

 * clocksource mask of all VDSO capable clocksources on x86 is U64_MAX

 * which would result in a pointless operation. The compiler cannot

 * optimize it away as the mask comes from the vdso data and is not compile

 * time constant.

 * This variant also uses a custom mask because while the clocksource mask of

 * all the VDSO capable clocksources on x86 is U64_MAX, the above code uses

 * U64_MASK as an exception value, additionally arch_vdso_cycles_ok() above

 * declares everything with the MSB/Sign-bit set as invalid. Therefore the

 * effective mask is S64_MAX.

 */

static __always_inline

u64 vdso_calc_delta(u64 cycles, u64 last, u64 mask, u32 mult)

{

	if (cycles > last)

		return (cycles - last) * mult;

	/*

	 * Due to the MSB/Sign-bit being used as invald marker (see

	 * arch_vdso_cycles_valid() above), the effective mask is S64_MAX.

	 */

	u64 delta = (cycles - last) & S64_MAX;



	/*

	 * Due to the above mentioned TSC wobbles, filter out negative motion.

	 * Per the above masking, the effective sign bit is now bit 62.

	 */

	if (unlikely(delta & (1ULL << 62)))

		return 0;



	return delta * mult;

}

#define vdso_calc_delta vdso_calc_delta