There is kvmclock drift issue during the vCPU hotplug. It has been fixed by
the commit c52ffadc65e2 ("KVM: x86: Don't unnecessarily force masterclock
update on vCPU hotplug").
This is to add the test to verify if the master clock is updated when we
write 0 to MSR_IA32_TSC from the host side.
Here is the usage example on the KVM with the bugfix reverted.
$ ./kvm_clock_drift -v -p 5
kvmclock based on old pvclock_vcpu_time_info: 5012221999
version: 2
tsc_timestamp: 3277968
system_time: 11849519
tsc_to_system_mul: 2152530255
tsc_shift: 0
flags: 1
kvmclock based on new pvclock_vcpu_time_info: 5012222411
version: 4
tsc_timestamp: 9980576184
system_time: 5012222411
tsc_to_system_mul: 2152530255
tsc_shift: 0
flags: 1
==== Test Assertion Failure ====
x86_64/kvm_clock_drift.c:216: clock_old == clock_new
pid=14257 tid=14257 errno=4 - Interrupted system call
1 0x000000000040277b: main at kvm_clock_drift.c:216
2 0x00007f7766fa7e44: ?? ??:0
3 0x000000000040286d: _start at ??:?
kvmclock drift detected, old=5012221999, new=5012222411
Signed-off-by: Dongli Zhang <[email protected]>
---
tools/testing/selftests/kvm/Makefile | 1 +
.../selftests/kvm/x86_64/kvm_clock_drift.c | 223 ++++++++++++++++++
2 files changed, 224 insertions(+)
create mode 100644 tools/testing/selftests/kvm/x86_64/kvm_clock_drift.c
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 4412b42d95de..c665d0d8d348 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -84,6 +84,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/hyperv_features
TEST_GEN_PROGS_x86_64 += x86_64/hyperv_ipi
TEST_GEN_PROGS_x86_64 += x86_64/hyperv_svm_test
TEST_GEN_PROGS_x86_64 += x86_64/hyperv_tlb_flush
+TEST_GEN_PROGS_x86_64 += x86_64/kvm_clock_drift
TEST_GEN_PROGS_x86_64 += x86_64/kvm_clock_test
TEST_GEN_PROGS_x86_64 += x86_64/kvm_pv_test
TEST_GEN_PROGS_x86_64 += x86_64/monitor_mwait_test
diff --git a/tools/testing/selftests/kvm/x86_64/kvm_clock_drift.c b/tools/testing/selftests/kvm/x86_64/kvm_clock_drift.c
new file mode 100644
index 000000000000..324f0dbc5762
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/kvm_clock_drift.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * The kvmclock drift test. Emulate vCPU hotplug and online to verify if
+ * there is kvmclock drift.
+ *
+ * Adapted from steal_time.c
+ *
+ * Copyright (C) 2020, Red Hat, Inc.
+ * Copyright (C) 2024 Oracle and/or its affiliates.
+ */
+
+#include <asm/kvm_para.h>
+#include <asm/pvclock.h>
+#include <asm/pvclock-abi.h>
+#include <sys/stat.h>
+
+#include "kvm_util.h"
+#include "processor.h"
+
+#define NR_VCPUS 2
+#define NR_SLOTS 2
+#define KVMCLOCK_SIZE sizeof(struct pvclock_vcpu_time_info)
+/*
+ * KVMCLOCK_GPA is identity mapped
+ */
+#define KVMCLOCK_GPA (1 << 30)
+
+static uint64_t kvmclock_gpa = KVMCLOCK_GPA;
+
+static void guest_code(int cpu)
+{
+ struct pvclock_vcpu_time_info *kvmclock;
+
+ /*
+ * vCPU#0 is to detect the change of pvclock_vcpu_time_info
+ */
+ if (cpu == 0) {
+ GUEST_SYNC(0);
+
+ kvmclock = (struct pvclock_vcpu_time_info *) kvmclock_gpa;
+ wrmsr(MSR_KVM_SYSTEM_TIME_NEW, kvmclock_gpa | KVM_MSR_ENABLED);
+
+ /*
+ * Backup the pvclock_vcpu_time_info before vCPU#1 hotplug
+ */
+ kvmclock[1] = kvmclock[0];
+
+ GUEST_SYNC(2);
+ /*
+ * Enter the guest to update pvclock_vcpu_time_info
+ */
+ GUEST_SYNC(4);
+ }
+
+ /*
+ * vCPU#1 is to emulate the vCPU hotplug
+ */
+ if (cpu == 1) {
+ GUEST_SYNC(1);
+ /*
+ * This is after the host side MSR_IA32_TSC
+ */
+ GUEST_SYNC(3);
+ }
+}
+
+static void run_vcpu(struct kvm_vcpu *vcpu)
+{
+ struct ucall uc;
+
+ vcpu_run(vcpu);
+
+ switch (get_ucall(vcpu, &uc)) {
+ case UCALL_SYNC:
+ case UCALL_DONE:
+ break;
+ case UCALL_ABORT:
+ REPORT_GUEST_ASSERT(uc);
+ default:
+ TEST_ASSERT(false, "Unexpected exit: %s",
+ exit_reason_str(vcpu->run->exit_reason));
+ }
+}
+
+static void kvmclock_dump(struct pvclock_vcpu_time_info *kvmclock)
+{
+ pr_info(" version: %u\n", kvmclock->version);
+ pr_info(" tsc_timestamp: %lu\n", kvmclock->tsc_timestamp);
+ pr_info(" system_time: %lu\n", kvmclock->system_time);
+ pr_info(" tsc_to_system_mul: %u\n", kvmclock->tsc_to_system_mul);
+ pr_info(" tsc_shift: %d\n", kvmclock->tsc_shift);
+ pr_info(" flags: %u\n", kvmclock->flags);
+ pr_info("\n");
+}
+
+#define CLOCKSOURCE_PATH "/sys/devices/system/clocksource/clocksource0/current_clocksource"
+
+static void check_clocksource(void)
+{
+ char *clk_name;
+ struct stat st;
+ FILE *fp;
+
+ fp = fopen(CLOCKSOURCE_PATH, "r");
+ if (!fp) {
+ pr_info("failed to open clocksource file: %d; assuming TSC.\n",
+ errno);
+ return;
+ }
+
+ if (fstat(fileno(fp), &st)) {
+ pr_info("failed to stat clocksource file: %d; assuming TSC.\n",
+ errno);
+ goto out;
+ }
+
+ clk_name = malloc(st.st_size);
+ TEST_ASSERT(clk_name, "failed to allocate buffer to read file\n");
+
+ if (!fgets(clk_name, st.st_size, fp)) {
+ pr_info("failed to read clocksource file: %d; assuming TSC.\n",
+ ferror(fp));
+ goto out;
+ }
+
+ TEST_ASSERT(!strncmp(clk_name, "tsc\n", st.st_size),
+ "clocksource not supported: %s", clk_name);
+out:
+ fclose(fp);
+}
+
+int main(int argc, char *argv[])
+{
+ struct pvclock_vcpu_time_info *kvmclock;
+ struct kvm_vcpu *vcpus[NR_VCPUS];
+ uint64_t clock_old, clock_new;
+ bool verbose = false;
+ unsigned int gpages;
+ struct kvm_vm *vm;
+ int period = 2;
+ uint64_t tsc;
+ int opt;
+
+ check_clocksource();
+
+ while ((opt = getopt(argc, argv, "p:vh")) != -1) {
+ switch (opt) {
+ case 'p':
+ period = atoi_positive("The period (seconds) between vCPU hotplug",
+ optarg);
+ break;
+ case 'v':
+ verbose = true;
+ break;
+ case 'h':
+ default:
+ pr_info("usage: %s [-p period (seconds)] [-v]\n", argv[0]);
+ exit(1);
+ }
+ }
+
+ vm = vm_create_with_vcpus(NR_VCPUS, guest_code, vcpus);
+ gpages = vm_calc_num_guest_pages(VM_MODE_DEFAULT,
+ KVMCLOCK_SIZE * NR_SLOTS);
+ vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
+ KVMCLOCK_GPA, 1, gpages, 0);
+ virt_map(vm, KVMCLOCK_GPA, KVMCLOCK_GPA, gpages);
+
+ vcpu_args_set(vcpus[0], 1, 0);
+ vcpu_args_set(vcpus[1], 1, 1);
+
+ /*
+ * Run vCPU#0 and vCPU#1 to update both pvclock_vcpu_time_info and
+ * master clock
+ */
+ run_vcpu(vcpus[0]);
+ run_vcpu(vcpus[1]);
+
+ /*
+ * Run vCPU#0 to backup the current pvclock_vcpu_time_info
+ */
+ run_vcpu(vcpus[0]);
+
+ sleep(period);
+
+ /*
+ * Emulate the hotplug of vCPU#1
+ */
+ vcpu_set_msr(vcpus[1], MSR_IA32_TSC, 0);
+
+ /*
+ * Emulate the online of vCPU#1
+ */
+ run_vcpu(vcpus[1]);
+
+ /*
+ * Run vCPU#0 to backup the new pvclock_vcpu_time_info to detect
+ * if there is any change or kvmclock drift
+ */
+ run_vcpu(vcpus[0]);
+
+ kvmclock = addr_gva2hva(vm, kvmclock_gpa);
+ tsc = kvmclock[0].tsc_timestamp;
+ clock_old = __pvclock_read_cycles(&kvmclock[1], tsc);
+ clock_new = __pvclock_read_cycles(&kvmclock[0], tsc);
+
+ if (verbose) {
+ pr_info("kvmclock based on old pvclock_vcpu_time_info: %lu\n",
+ clock_old);
+ kvmclock_dump(&kvmclock[1]);
+ pr_info("kvmclock based on new pvclock_vcpu_time_info: %lu\n",
+ clock_new);
+ kvmclock_dump(&kvmclock[0]);
+ }
+
+ TEST_ASSERT(clock_old == clock_new,
+ "kvmclock drift detected, old=%lu, new=%lu",
+ clock_old, clock_new);
+
+ kvm_vm_free(vm);
+
+ return 0;
+}
base-commit: f2a3fb7234e52f72ff4a38364dbf639cf4c7d6c6
--
2.34.1
Ping :)
BTW, I see Vitaly Kuznetsov has a patch to generalize check_clocksource(),
which is also used by this patch.
[PATCH 1/5] KVM: selftests: Generalize check_clocksource() from kvm_clock_test
https://lore.kernel.org/all/[email protected]/
Thank you very much!
Dongli Zhang
On 1/6/24 00:33, Dongli Zhang wrote:
> There is kvmclock drift issue during the vCPU hotplug. It has been fixed by
> the commit c52ffadc65e2 ("KVM: x86: Don't unnecessarily force masterclock
> update on vCPU hotplug").
>
> This is to add the test to verify if the master clock is updated when we
> write 0 to MSR_IA32_TSC from the host side.
>
> Here is the usage example on the KVM with the bugfix reverted.
>
> $ ./kvm_clock_drift -v -p 5
> kvmclock based on old pvclock_vcpu_time_info: 5012221999
> version: 2
> tsc_timestamp: 3277968
> system_time: 11849519
> tsc_to_system_mul: 2152530255
> tsc_shift: 0
> flags: 1
>
> kvmclock based on new pvclock_vcpu_time_info: 5012222411
> version: 4
> tsc_timestamp: 9980576184
> system_time: 5012222411
> tsc_to_system_mul: 2152530255
> tsc_shift: 0
> flags: 1
>
> ==== Test Assertion Failure ====
> x86_64/kvm_clock_drift.c:216: clock_old == clock_new
> pid=14257 tid=14257 errno=4 - Interrupted system call
> 1 0x000000000040277b: main at kvm_clock_drift.c:216
> 2 0x00007f7766fa7e44: ?? ??:0
> 3 0x000000000040286d: _start at ??:?
> kvmclock drift detected, old=5012221999, new=5012222411
>
> Signed-off-by: Dongli Zhang <[email protected]>
> ---
> tools/testing/selftests/kvm/Makefile | 1 +
> .../selftests/kvm/x86_64/kvm_clock_drift.c | 223 ++++++++++++++++++
> 2 files changed, 224 insertions(+)
> create mode 100644 tools/testing/selftests/kvm/x86_64/kvm_clock_drift.c
>
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 4412b42d95de..c665d0d8d348 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -84,6 +84,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/hyperv_features
> TEST_GEN_PROGS_x86_64 += x86_64/hyperv_ipi
> TEST_GEN_PROGS_x86_64 += x86_64/hyperv_svm_test
> TEST_GEN_PROGS_x86_64 += x86_64/hyperv_tlb_flush
> +TEST_GEN_PROGS_x86_64 += x86_64/kvm_clock_drift
> TEST_GEN_PROGS_x86_64 += x86_64/kvm_clock_test
> TEST_GEN_PROGS_x86_64 += x86_64/kvm_pv_test
> TEST_GEN_PROGS_x86_64 += x86_64/monitor_mwait_test
> diff --git a/tools/testing/selftests/kvm/x86_64/kvm_clock_drift.c b/tools/testing/selftests/kvm/x86_64/kvm_clock_drift.c
> new file mode 100644
> index 000000000000..324f0dbc5762
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/kvm_clock_drift.c
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * The kvmclock drift test. Emulate vCPU hotplug and online to verify if
> + * there is kvmclock drift.
> + *
> + * Adapted from steal_time.c
> + *
> + * Copyright (C) 2020, Red Hat, Inc.
> + * Copyright (C) 2024 Oracle and/or its affiliates.
> + */
> +
> +#include <asm/kvm_para.h>
> +#include <asm/pvclock.h>
> +#include <asm/pvclock-abi.h>
> +#include <sys/stat.h>
> +
> +#include "kvm_util.h"
> +#include "processor.h"
> +
> +#define NR_VCPUS 2
> +#define NR_SLOTS 2
> +#define KVMCLOCK_SIZE sizeof(struct pvclock_vcpu_time_info)
> +/*
> + * KVMCLOCK_GPA is identity mapped
> + */
> +#define KVMCLOCK_GPA (1 << 30)
> +
> +static uint64_t kvmclock_gpa = KVMCLOCK_GPA;
> +
> +static void guest_code(int cpu)
> +{
> + struct pvclock_vcpu_time_info *kvmclock;
> +
> + /*
> + * vCPU#0 is to detect the change of pvclock_vcpu_time_info
> + */
> + if (cpu == 0) {
> + GUEST_SYNC(0);
> +
> + kvmclock = (struct pvclock_vcpu_time_info *) kvmclock_gpa;
> + wrmsr(MSR_KVM_SYSTEM_TIME_NEW, kvmclock_gpa | KVM_MSR_ENABLED);
> +
> + /*
> + * Backup the pvclock_vcpu_time_info before vCPU#1 hotplug
> + */
> + kvmclock[1] = kvmclock[0];
> +
> + GUEST_SYNC(2);
> + /*
> + * Enter the guest to update pvclock_vcpu_time_info
> + */
> + GUEST_SYNC(4);
> + }
> +
> + /*
> + * vCPU#1 is to emulate the vCPU hotplug
> + */
> + if (cpu == 1) {
> + GUEST_SYNC(1);
> + /*
> + * This is after the host side MSR_IA32_TSC
> + */
> + GUEST_SYNC(3);
> + }
> +}
> +
> +static void run_vcpu(struct kvm_vcpu *vcpu)
> +{
> + struct ucall uc;
> +
> + vcpu_run(vcpu);
> +
> + switch (get_ucall(vcpu, &uc)) {
> + case UCALL_SYNC:
> + case UCALL_DONE:
> + break;
> + case UCALL_ABORT:
> + REPORT_GUEST_ASSERT(uc);
> + default:
> + TEST_ASSERT(false, "Unexpected exit: %s",
> + exit_reason_str(vcpu->run->exit_reason));
> + }
> +}
> +
> +static void kvmclock_dump(struct pvclock_vcpu_time_info *kvmclock)
> +{
> + pr_info(" version: %u\n", kvmclock->version);
> + pr_info(" tsc_timestamp: %lu\n", kvmclock->tsc_timestamp);
> + pr_info(" system_time: %lu\n", kvmclock->system_time);
> + pr_info(" tsc_to_system_mul: %u\n", kvmclock->tsc_to_system_mul);
> + pr_info(" tsc_shift: %d\n", kvmclock->tsc_shift);
> + pr_info(" flags: %u\n", kvmclock->flags);
> + pr_info("\n");
> +}
> +
> +#define CLOCKSOURCE_PATH "/sys/devices/system/clocksource/clocksource0/current_clocksource"
> +
> +static void check_clocksource(void)
> +{
> + char *clk_name;
> + struct stat st;
> + FILE *fp;
> +
> + fp = fopen(CLOCKSOURCE_PATH, "r");
> + if (!fp) {
> + pr_info("failed to open clocksource file: %d; assuming TSC.\n",
> + errno);
> + return;
> + }
> +
> + if (fstat(fileno(fp), &st)) {
> + pr_info("failed to stat clocksource file: %d; assuming TSC.\n",
> + errno);
> + goto out;
> + }
> +
> + clk_name = malloc(st.st_size);
> + TEST_ASSERT(clk_name, "failed to allocate buffer to read file\n");
> +
> + if (!fgets(clk_name, st.st_size, fp)) {
> + pr_info("failed to read clocksource file: %d; assuming TSC.\n",
> + ferror(fp));
> + goto out;
> + }
> +
> + TEST_ASSERT(!strncmp(clk_name, "tsc\n", st.st_size),
> + "clocksource not supported: %s", clk_name);
> +out:
> + fclose(fp);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + struct pvclock_vcpu_time_info *kvmclock;
> + struct kvm_vcpu *vcpus[NR_VCPUS];
> + uint64_t clock_old, clock_new;
> + bool verbose = false;
> + unsigned int gpages;
> + struct kvm_vm *vm;
> + int period = 2;
> + uint64_t tsc;
> + int opt;
> +
> + check_clocksource();
> +
> + while ((opt = getopt(argc, argv, "p:vh")) != -1) {
> + switch (opt) {
> + case 'p':
> + period = atoi_positive("The period (seconds) between vCPU hotplug",
> + optarg);
> + break;
> + case 'v':
> + verbose = true;
> + break;
> + case 'h':
> + default:
> + pr_info("usage: %s [-p period (seconds)] [-v]\n", argv[0]);
> + exit(1);
> + }
> + }
> +
> + vm = vm_create_with_vcpus(NR_VCPUS, guest_code, vcpus);
> + gpages = vm_calc_num_guest_pages(VM_MODE_DEFAULT,
> + KVMCLOCK_SIZE * NR_SLOTS);
> + vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
> + KVMCLOCK_GPA, 1, gpages, 0);
> + virt_map(vm, KVMCLOCK_GPA, KVMCLOCK_GPA, gpages);
> +
> + vcpu_args_set(vcpus[0], 1, 0);
> + vcpu_args_set(vcpus[1], 1, 1);
> +
> + /*
> + * Run vCPU#0 and vCPU#1 to update both pvclock_vcpu_time_info and
> + * master clock
> + */
> + run_vcpu(vcpus[0]);
> + run_vcpu(vcpus[1]);
> +
> + /*
> + * Run vCPU#0 to backup the current pvclock_vcpu_time_info
> + */
> + run_vcpu(vcpus[0]);
> +
> + sleep(period);
> +
> + /*
> + * Emulate the hotplug of vCPU#1
> + */
> + vcpu_set_msr(vcpus[1], MSR_IA32_TSC, 0);
> +
> + /*
> + * Emulate the online of vCPU#1
> + */
> + run_vcpu(vcpus[1]);
> +
> + /*
> + * Run vCPU#0 to backup the new pvclock_vcpu_time_info to detect
> + * if there is any change or kvmclock drift
> + */
> + run_vcpu(vcpus[0]);
> +
> + kvmclock = addr_gva2hva(vm, kvmclock_gpa);
> + tsc = kvmclock[0].tsc_timestamp;
> + clock_old = __pvclock_read_cycles(&kvmclock[1], tsc);
> + clock_new = __pvclock_read_cycles(&kvmclock[0], tsc);
> +
> + if (verbose) {
> + pr_info("kvmclock based on old pvclock_vcpu_time_info: %lu\n",
> + clock_old);
> + kvmclock_dump(&kvmclock[1]);
> + pr_info("kvmclock based on new pvclock_vcpu_time_info: %lu\n",
> + clock_new);
> + kvmclock_dump(&kvmclock[0]);
> + }
> +
> + TEST_ASSERT(clock_old == clock_new,
> + "kvmclock drift detected, old=%lu, new=%lu",
> + clock_old, clock_new);
> +
> + kvm_vm_free(vm);
> +
> + return 0;
> +}
>
> base-commit: f2a3fb7234e52f72ff4a38364dbf639cf4c7d6c6
On Sat, 2024-01-06 at 00:33 -0800, Dongli Zhang wrote:
> There is kvmclock drift issue during the vCPU hotplug. It has been fixed by
> the commit c52ffadc65e2 ("KVM: x86: Don't unnecessarily force masterclock
> update on vCPU hotplug").
>
> This is to add the test to verify if the master clock is updated when we
> write 0 to MSR_IA32_TSC from the host side.
>
> Here is the usage example on the KVM with the bugfix reverted.
>
> $ ./kvm_clock_drift -v -p 5
> kvmclock based on old pvclock_vcpu_time_info: 5012221999
> version: 2
> tsc_timestamp: 3277968
> system_time: 11849519
> tsc_to_system_mul: 2152530255
> tsc_shift: 0
> flags: 1
>
> kvmclock based on new pvclock_vcpu_time_info: 5012222411
> version: 4
> tsc_timestamp: 9980576184
> system_time: 5012222411
> tsc_to_system_mul: 2152530255
> tsc_shift: 0
> flags: 1
>
> ==== Test Assertion Failure ====
> x86_64/kvm_clock_drift.c:216: clock_old == clock_new
> pid=14257 tid=14257 errno=4 - Interrupted system call
> 1 0x000000000040277b: main at kvm_clock_drift.c:216
> 2 0x00007f7766fa7e44: ?? ??:0
> 3 0x000000000040286d: _start at ??:?
> kvmclock drift detected, old=5012221999, new=5012222411
>
> Signed-off-by: Dongli Zhang <[email protected]>
We should extend this to cover live update — it should create a
*second* KVM, migrate the guest including its clock information to
that, and validate that the kvmclock information still doesn't change.
Ideally during a leap second.