[RFC PATCH] KVM: x86: (Failing) test case for TSC scaling and offset sync

[RFC PATCH] KVM: x86: (Failing) test case for TSC scaling and offset sync

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 6696 bytes --]

Here's a fun test case. Create a VM, then spawn a bunch of threads each
of which creates its own vCPU and then sets the TSC frequency and
value. The guest vCPUs then run a stupidly naïve loop checking for TSC
monotonicity.

It dies horribly. Even if you use its "nice" mode where it actually
serializes everything and the startup effectively isn't threaded at
all.

I can fix the serialized and "first CPU is scaled before the others are
created" modes, by making new vCPUs get created with the same frequency
of the last TSC sync. That is, in kvm_arch_vcpu_create() I make it do:
kvm_set_tsc_khz(READ_ONCE(vcpu->kvm->arch.last_tsc_khz) ? : max_tsc_khz)
instead of just using max_tsc_khz unconditionally.

The free-for-all mode of all threads just running freely and creating
their vCPU, then setting its frequency and its TSC value, remains
hosed. We end up with kvm->arch.last_tsc_khz *alternating* between the
default startup frequency and the user-configured one. I wonder if the
answer here might be for new vCPUs not to get kvm->arch.last_tsc_khz
from the last sync, but for them to inherit the last *explicitly* set
TSC frequency, which we'd have to store separately?

There are still potential race conditions between the initial frequency
being set in kvm_arch_vcpu_create(), and the first TSC sync which
happens later in kvm_arch_vcpu_postcreate(). It's possible that another
vCPU has explicitly set its frequency in between those two being called
for a newly-created vCPU, and the new one would still not sync
correctly because of the apparent mismatch.

A KVM-wide setting for the default frequency, instead of inferring it
from an explicit setting on a vCPU, might address that... ?

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/x86_64/tsc_scaling_sync.c   | 143 ++++++++++++++++++
 2 files changed, 144 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86_64/tsc_scaling_sync.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 06c3a4602bcc..33b57f8c6251 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -65,6 +65,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/state_test
 TEST_GEN_PROGS_x86_64 += x86_64/vmx_preemption_timer_test
 TEST_GEN_PROGS_x86_64 += x86_64/svm_vmcall_test
 TEST_GEN_PROGS_x86_64 += x86_64/svm_int_ctl_test
+TEST_GEN_PROGS_x86_64 += x86_64/tsc_scaling_sync
 TEST_GEN_PROGS_x86_64 += x86_64/sync_regs_test
 TEST_GEN_PROGS_x86_64 += x86_64/userspace_io_test
 TEST_GEN_PROGS_x86_64 += x86_64/userspace_msr_exit_test
diff --git a/tools/testing/selftests/kvm/x86_64/tsc_scaling_sync.c b/tools/testing/selftests/kvm/x86_64/tsc_scaling_sync.c
new file mode 100644
index 000000000000..f4a68b709a2c
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/tsc_scaling_sync.c
@@ -0,0 +1,143 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * tsc_scaling_sync.c
+ *
+ * Copyright © 2022 Amazon.com, Inc. or its affiliates.
+ *
+ * TSC scaling / sync test
+ */
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+
+#include <stdint.h>
+#include <time.h>
+#include <sched.h>
+#include <signal.h>
+#include <pthread.h>
+
+#define NR_TEST_VCPUS 20
+
+static struct kvm_vm *vm;
+pthread_spinlock_t create_lock;
+
+#define TEST_TSC_KHZ    2987654UL
+#define TEST_TSC_OFFSET 200000000
+
+uint64_t tsc_sync;
+static void guest_code(void)
+{
+	uint64_t start_tsc, local_tsc, tmp;
+
+	GUEST_SYNC(0);
+
+	start_tsc = rdtsc();
+	do {
+		tmp = READ_ONCE(tsc_sync);
+		local_tsc = rdtsc();
+		WRITE_ONCE(tsc_sync, local_tsc);
+		if (unlikely(local_tsc < tmp))
+			GUEST_SYNC_ARGS(1, local_tsc, tmp, 0, 0);
+		//GUEST_ASSERT(local_tsc <= tmp);
+	} while (local_tsc - start_tsc < 5000 * TEST_TSC_KHZ);
+
+	GUEST_DONE();
+}
+
+
+enum {
+      ALL_SERIALIZED,
+      FIRST_SERIALIZED,
+      NONE_SERIALIZED
+} serialize_mode = FIRST_SERIALIZED;
+
+
+static void *run_vcpu(void *_cpu_nr)
+{
+	unsigned long cpu = (unsigned long)_cpu_nr;
+	unsigned long failures = 0;
+	static bool first_cpu_scaled;
+	bool this_cpu_scaled = false;
+
+	pthread_spin_lock(&create_lock);
+
+	vm_vcpu_add_default(vm, cpu, guest_code);
+
+	if (serialize_mode == ALL_SERIALIZED ||
+	    (serialize_mode == FIRST_SERIALIZED && !first_cpu_scaled)) {
+		vcpu_ioctl(vm, cpu, KVM_SET_TSC_KHZ, (void *) TEST_TSC_KHZ);
+		vcpu_set_msr(vm, cpu, MSR_IA32_TSC, TEST_TSC_OFFSET);
+		this_cpu_scaled = first_cpu_scaled = true;
+	}
+
+	pthread_spin_unlock(&create_lock);
+
+	if (!this_cpu_scaled) {
+		vcpu_ioctl(vm, cpu, KVM_SET_TSC_KHZ, (void *) TEST_TSC_KHZ);
+		vcpu_set_msr(vm, cpu, MSR_IA32_TSC, TEST_TSC_OFFSET);
+	}
+
+	for (;;) {
+		volatile struct kvm_run *run = vcpu_state(vm, cpu);
+                struct ucall uc;
+
+                vcpu_run(vm, cpu);
+                TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
+                            "Got exit_reason other than KVM_EXIT_IO: %u (%s)\n",
+                            run->exit_reason,
+                            exit_reason_str(run->exit_reason));
+
+                switch (get_ucall(vm, cpu, &uc)) {
+                case UCALL_DONE:
+			goto out;
+
+                case UCALL_SYNC:
+			switch(uc.args[1]) {
+			case 0:
+				//printf("set TSC\n");
+				//	vcpu_set_msr(vm, cpu, MSR_IA32_TSC, 0);
+				break;
+
+			case 1:
+				printf("Guest %ld sync %lx %lx %ld\n", cpu, uc.args[2], uc.args[3], uc.args[2] - uc.args[3]);
+				failures++;
+				break;
+			}
+			break;
+
+                default:
+                        TEST_FAIL("Unknown ucall %lu", uc.cmd);
+		}
+	}
+ out:
+	return (void *)failures;
+}
+
+int main(int argc, char *argv[])
+{
+        if (!kvm_check_cap(KVM_CAP_TSC_CONTROL)) {
+		print_skip("KVM_CAP_TSC_CONTROL not available");
+		exit(KSFT_SKIP);
+	}
+
+	vm = vm_create_default_with_vcpus(0, DEFAULT_STACK_PGS * NR_TEST_VCPUS, 0, guest_code, NULL);
+
+	pthread_spin_init(&create_lock, PTHREAD_PROCESS_PRIVATE);
+	pthread_t cpu_threads[NR_TEST_VCPUS];
+	unsigned long cpu;
+	for (cpu = 0; cpu < NR_TEST_VCPUS; cpu++)
+		pthread_create(&cpu_threads[cpu], NULL, run_vcpu, (void *)cpu);
+
+	unsigned long failures = 0;
+	for (cpu = 0; cpu < NR_TEST_VCPUS; cpu++) {
+		void *this_cpu_failures;
+		pthread_join(cpu_threads[cpu], &this_cpu_failures);
+		failures += (unsigned long)this_cpu_failures;
+	}
+
+	TEST_ASSERT(!failures, "TSC sync failed");
+	pthread_spin_destroy(&create_lock);
+	kvm_vm_free(vm);
+	return 0;
+}



[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]