[PATCH kvm-unit-tests] kvmclock_test: test the KVM_HC_CLOCK_PAIRING hypercall

[PATCH kvm-unit-tests] kvmclock_test: test the KVM_HC_CLOCK_PAIRING hypercall

[Paolo Bonzini]

The seconds/nanoseconds pair returned from the hypercall is pretty
much the same value that is written into struct pvclock_wall_clock,
except better because it does not suffer from the y2038 problem.

Test that this is the case, in preparation for using
KVM_HC_CLOCK_PAIRING in Linux instead of MSR_KVM_WALL_CLOCK_NEW.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 x86/kvmclock.c      | 16 ++++++++++++++++
 x86/kvmclock.h      | 12 ++++++++++++
 x86/kvmclock_test.c | 29 ++++++++++++++++++++---------
 3 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/x86/kvmclock.c b/x86/kvmclock.c
index bad0784..76569ad 100644
--- a/x86/kvmclock.c
+++ b/x86/kvmclock.c
@@ -282,6 +282,22 @@ void kvm_get_wallclock(struct timespec *ts)
         pvclock_read_wallclock(&wall_clock, vcpu_time, ts);
 }
 
+uint64_t kvm_hc_get_wallclock(struct timespec *ts)
+{
+	u32 ret;
+	struct kvm_clock_pairing clk = {0};
+
+        asm volatile("vmcall"
+                     : "=a"(ret)
+                     : "a"(KVM_HC_CLOCK_PAIRING), "b"(&clk),
+		       "c"(KVM_CLOCK_PAIRING_WALLCLOCK) : "memory");
+	if (ret != 0)
+		return -ret;
+	ts->tv_sec = clk.sec;
+	ts->tv_nsec = clk.nsec;
+	return clk.tsc;
+}
+
 void pvclock_set_flags(unsigned char flags)
 {
         valid_flags = flags;
diff --git a/x86/kvmclock.h b/x86/kvmclock.h
index dff6802..f74bf8b 100644
--- a/x86/kvmclock.h
+++ b/x86/kvmclock.h
@@ -9,6 +9,9 @@
 #define PVCLOCK_TSC_STABLE_BIT (1 << 0)
 #define PVCLOCK_RAW_CYCLE_BIT (1 << 7) /* Get raw cycle */
 
+#define KVM_HC_CLOCK_PAIRING        9
+#define KVM_CLOCK_PAIRING_WALLCLOCK 0
+
 # define NSEC_PER_SEC			1000000000ULL
 
 typedef u64 cycle_t;
@@ -35,9 +38,18 @@ struct timespec {
         long   tv_nsec;
 };
 
+struct kvm_clock_pairing {
+        s64 sec;
+        s64 nsec;
+        u64 tsc;
+        u32 flags;
+        u32 pad[9];
+};
+
 void pvclock_set_flags(unsigned char flags);
 cycle_t kvm_clock_read();
 void kvm_get_wallclock(struct timespec *ts);
+uint64_t kvm_hc_get_wallclock(struct timespec *ts);
 void kvm_clock_init(void *data);
 void kvm_clock_clear(void *data);
 
diff --git a/x86/kvmclock_test.c b/x86/kvmclock_test.c
index 48a7cdb..985bba1 100644
--- a/x86/kvmclock_test.c
+++ b/x86/kvmclock_test.c
@@ -5,11 +5,12 @@
 #include "kvmclock.h"
 
 #define DEFAULT_TEST_LOOPS 100000000L
-#define DEFAULT_THRESHOLD  5L
+
+#define WALLCLOCK_THRESHOLD  5		/* seconds */
+#define KVMCLOCK_THRESHOLD  100000000	/* nanoseconds */
 
 long loops = DEFAULT_TEST_LOOPS;
 long sec = 0;
-long threshold = DEFAULT_THRESHOLD;
 
 struct test_info {
         struct spinlock lock;
@@ -25,7 +26,8 @@ struct test_info ti[4];
 static void wallclock_test(void *data)
 {
         int *p_err = data;
-        long ksec, offset;
+        uint64_t ksec, knsec;
+        long offset;
         struct timespec ts;
 
         kvm_get_wallclock(&ts);
@@ -33,12 +35,23 @@ static void wallclock_test(void *data)
 
         offset = ksec - sec;
         printf("Raw nanoseconds value from kvmclock: %" PRIu64 " (cpu %d)\n", kvm_clock_read(), smp_id());
-        printf("Seconds get from kvmclock: %ld (cpu %d, offset: %ld)\n", ksec, smp_id(), offset);
+        printf("Seconds get from kvmclock: %" PRIu64 " (cpu %d, offset: %ld)\n", ksec, smp_id(), offset);
 
-        if (offset > threshold || offset < -threshold) {
-                printf("offset too large!\n");
+        if (offset > WALLCLOCK_THRESHOLD || offset < -WALLCLOCK_THRESHOLD) {
+                printf("offset too large (threshold: %d)!\n", WALLCLOCK_THRESHOLD);
                 (*p_err)++;
         }
+
+	knsec = ksec * 1000000000ULL + ts.tv_nsec;
+	if (kvm_hc_get_wallclock(&ts) >= 0) {
+		offset = knsec - (ts.tv_sec * 1000000000ULL + ts.tv_nsec);
+		printf("Nanoseconds from hypercall: %" PRIu64 " (cpu %d, offset: %ld)\n", ksec, smp_id(), offset);
+		if (offset > KVMCLOCK_THRESHOLD || offset < -KVMCLOCK_THRESHOLD) {
+			printf("offset too large (threshold: %d)!\n", KVMCLOCK_THRESHOLD);
+			(*p_err)++;
+		}
+	}
+
 }
 
 static void kvm_clock_test(void *data)
@@ -112,8 +125,6 @@ int main(int ac, char **av)
                 loops = atol(av[1]);
         if (ac > 2)
                 sec = atol(av[2]);
-        if (ac > 3)
-                threshold = atol(av[3]);
 
         smp_init();
 
@@ -124,7 +135,7 @@ int main(int ac, char **av)
         on_cpus(kvm_clock_init, NULL);
 
         if (ac > 2) {
-                printf("Wallclock test, threshold %ld\n", threshold);
+                printf("Wallclock test\n");
                 printf("Seconds get from host:     %ld\n", sec);
                 for (i = 0; i < ncpus; ++i)
                         on_cpu(i, wallclock_test, &nerr);
-- 
2.14.3

Re: [PATCH kvm-unit-tests] kvmclock_test: test the KVM_HC_CLOCK_PAIRING hypercall

[Paolo Bonzini]

On 16/11/2017 23:54, Marcelo Tosatti wrote:
> On Mon, Nov 06, 2017 at 05:56:19PM +0100, Paolo Bonzini wrote:
>> The seconds/nanoseconds pair returned from the hypercall is pretty
>> much the same value that is written into struct pvclock_wall_clock,
>> except better because it does not suffer from the y2038 problem.
>>
>> Test that this is the case, in preparation for using
>> KVM_HC_CLOCK_PAIRING in Linux instead of MSR_KVM_WALL_CLOCK_NEW.
> 
> Hey Paolo,
> 
> Well pvclock_wall_clock has:
> 
> struct pvclock_wall_clock {
>         u32   version;
>         u32   sec;
>         u32   nsec;
>         u32   sec_hi;
> } __attribute__((__packed__));

Note that sec_hi is only used on ARM, and
Documentation/virtual/kvm/msr.txt says that KVM only writes 12 bytes.

> While the year 2038 bug has an overflow when using a _signed_ integer 
> for keeping seconds from Jan 1 1970.
> 
> (Thats a 2^31 overflow).

Yeah, that's correct.  It's only pvclock_read_wallclock that has a year
2038 bug; that could be fixed as well to use struct timespec64 and only
have a year 2106 bug, but it seemed simpler to make it correct once and
for all.

Paolo

> Did you just infer a year 2038 bug from the "32-bit sec" or there is
> actually a signed integer being used? 
> 
> Btw, should perform testing with date=2038 and Linux guests... Or are
> you doing that already?
> 
> Thanks
>