Re: [PATCH 1/1] KVM: selftests: add kvmclock drift test

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@redhat.com/

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@oracle.com

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