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/20240109141121.1619463-2-vkuznets@xxxxxxxxxx/ 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 <dongli.zhang@xxxxxxxxxx> > --- > 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
