Re: [PATCH kvm-unit-tests v2] KVM: x86: add hyperv clock test case

[Marcelo Tosatti <mtosatti@redhat.com>]

On Thu, Apr 21, 2016 at 08:21:25PM +0300, Roman Kagan wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> The test checks the relative precision of the reference TSC page
> and the time reference counter.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> [Adjust types to pass printf type checks - Roman Kagan]
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> ---
>  x86/Makefile.common |   3 +
>  x86/Makefile.x86_64 |   1 +
>  x86/hyperv.h        |   9 +++
>  x86/hyperv_clock.c  | 197 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  x86/unittests.cfg   |   5 ++
>  5 files changed, 215 insertions(+)
>  create mode 100644 x86/hyperv_clock.c
> 
> diff --git a/x86/Makefile.common b/x86/Makefile.common
> index ca80367..456e188 100644
> --- a/x86/Makefile.common
> +++ b/x86/Makefile.common
> @@ -123,6 +123,9 @@ $(TEST_DIR)/hyperv_synic.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
>  $(TEST_DIR)/hyperv_stimer.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
>                                 $(TEST_DIR)/hyperv_stimer.o
>  
> +$(TEST_DIR)/hyperv_clock.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
> +                               $(TEST_DIR)/hyperv_clock.o
> +
>  $(TEST_DIR)/setjmp.elf: $(cstart.o) $(TEST_DIR)/setjmp.o
>  
>  arch_clean:
> diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
> index 6b7ccfb..56de0f0 100644
> --- a/x86/Makefile.x86_64
> +++ b/x86/Makefile.x86_64
> @@ -14,5 +14,6 @@ tests = $(TEST_DIR)/access.flat $(TEST_DIR)/apic.flat \
>  tests += $(TEST_DIR)/svm.flat
>  tests += $(TEST_DIR)/vmx.flat
>  tests += $(TEST_DIR)/tscdeadline_latency.flat
> +tests += $(TEST_DIR)/hyperv_clock.flat
>  
>  include $(TEST_DIR)/Makefile.common
> diff --git a/x86/hyperv.h b/x86/hyperv.h
> index faf931b..974df56 100644
> --- a/x86/hyperv.h
> +++ b/x86/hyperv.h
> @@ -12,6 +12,7 @@
>  #define HV_X64_MSR_SYNTIMER_AVAILABLE           (1 << 3)
>  
>  #define HV_X64_MSR_TIME_REF_COUNT               0x40000020
> +#define HV_X64_MSR_REFERENCE_TSC                0x40000021
>  
>  /* Define synthetic interrupt controller model specific registers. */
>  #define HV_X64_MSR_SCONTROL                     0x40000080
> @@ -180,4 +181,12 @@ void synic_sint_create(int vcpu, int sint, int vec, bool auto_eoi);
>  void synic_sint_set(int vcpu, int sint);
>  void synic_sint_destroy(int vcpu, int sint);
>  
> +struct hv_reference_tsc_page {
> +        uint32_t tsc_sequence;
> +        uint32_t res1;
> +        uint64_t tsc_scale;
> +        int64_t tsc_offset;
> +};
> +
> +
>  #endif
> diff --git a/x86/hyperv_clock.c b/x86/hyperv_clock.c
> new file mode 100644
> index 0000000..a346d99
> --- /dev/null
> +++ b/x86/hyperv_clock.c
> @@ -0,0 +1,197 @@
> +#include "libcflat.h"
> +#include "smp.h"
> +#include "atomic.h"
> +#include "processor.h"
> +#include "hyperv.h"
> +#include "vm.h"
> +
> +#define MAX_CPU 4
> +#define TICKS_PER_SEC (1000000000 / 100)
> +
> +struct hv_reference_tsc_page *tsc_ref;
> +
> +/*
> + * Scale a 64-bit delta by scaling and multiplying by a 32-bit fraction,
> + * yielding a 64-bit result.
> + */
> +static inline u64 scale_delta(u64 delta, u64 mul_frac)
> +{
> +	u64 product, unused;
> +
> +	__asm__ (
> +		"mul %3"
> +		: "=d" (product), "=a" (unused) : "1" (delta), "rm" ((u64)mul_frac) );
> +
> +	return product;
> +}
> +
> +static u64 hvclock_tsc_to_ticks(struct hv_reference_tsc_page *shadow, u64 tsc)
> +{
> +	return scale_delta(tsc, shadow->tsc_scale) + shadow->tsc_offset;
> +}
> +
> +/*
> + * Reads a consistent set of time-base values from hypervisor,
> + * into a shadow data area.
> + */
> +static void hvclock_get_time_values(struct hv_reference_tsc_page *shadow,
> +				    struct hv_reference_tsc_page *page)
> +{
> +	int seq;
> +	do {
> +		seq = page->tsc_sequence;
> +		rmb();		/* fetch version before data */
> +		*shadow = *page;
> +		rmb();		/* test version after fetching data */
> +	} while (shadow->tsc_sequence != seq);
> +}
> +
> +u64 tsc_ref_read(void)
> +{
> +	struct hv_reference_tsc_page shadow;
> +
> +	hvclock_get_time_values(&shadow, tsc_ref);
> +	return hvclock_tsc_to_ticks(&shadow, rdtsc());
> +}
> +
> +atomic_t cpus_left;
> +bool ok[MAX_CPU];
> +u64 loops[MAX_CPU];
> +
> +static void tsc_ref_test(void *data)
> +{
> +	int i = smp_id();
> +	unsigned long long t = rdmsr(HV_X64_MSR_TIME_REF_COUNT);
> +	unsigned long long end = t + 3 * TICKS_PER_SEC;
> +
> +	ok[i] = true;
> +	do {
> +		u64 now = tsc_ref_read();
> +		if (now < t) {
> +			printf("warp on CPU %d!\n", smp_id());
> +			ok[i] = false;
> +			break;
> +		}
> +		t = now;
> +	} while(t < end);
> +
> +	barrier();

***


> +	if (t >= end) {
> +		long long ref = rdmsr(HV_X64_MSR_TIME_REF_COUNT);
> +		if (i == 0)
> +			printf("Time reference MSR drift: %lld\n\n", ref - end);
> +		ok[i] &= (ref - end) > -5 && (ref - end) < 5;

This is prone to fail: guest can be scheduled at "***" above and 
test will fail.

The [-5,5] comes from the standard? What the standard dictates?