> +int main(int argc, char *argv[])
> +{
> + static const struct option long_opts[] = {
> + { "sleep", required_argument, NULL, 's' },
> + { "help", no_argument, NULL, 'h' },
> + { NULL, 0, NULL, 0 },
> + };
> + unsigned int sleep_sec = 2;
> + struct kvm_vcpu *vcpu;
> + struct kvm_vm *vm;
> + uint64_t host_khz;
> + uint64_t freq;
> + int opt;
> +
> + while ((opt = getopt_long(argc, argv, "s:h", long_opts, NULL)) != -1) {
> + switch (opt) {
> + case 's':
> + sleep_sec = atoi(optarg);
> + break;
> + case 'h':
> + default:
> + usage(argv[0]);
> + return opt == 'h' ? 0 : 1;
> + }
> + }
> +
> + TEST_REQUIRE(sys_clocksource_is_based_on_tsc());
> + TEST_REQUIRE(kvm_has_cap(KVM_CAP_TSC_CONTROL));
> +
> + vm = vm_create_with_one_vcpu(&vcpu, guest_code);
> + configure_pvclock(vm);
> +
> + /* Check KVM_GET_CLOCK_GUEST is supported */
> + {
> + struct pvclock_vcpu_time_info tmp;
> + int ret = __vcpu_ioctl(vcpu, KVM_GET_CLOCK_GUEST, &tmp);
> + TEST_REQUIRE(ret == 0);
It will likely be a moot point since we should have a CAP, but don't do
TEST_REQUIRE()
on a local variable like this, it completely defeates the purpose of the macro
shenanigans. Becuase this:
1..0 # SKIP - Requirement not met: ret == 0
is useless information, whereas this:
1..0 # SKIP - Requirement not met: !__vcpu_ioctl(vcpu, KVM_GET_CLOCK_GUEST,
&tmp)
gives the user a starting point without having to go search through the test
code.
> + }
...
> +static volatile uint32_t vcpu_counter;
> +static void guest_code_stable_bit(void)
> +{
> + uint32_t idx = __atomic_fetch_add(&vcpu_counter, 1, __ATOMIC_SEQ_CST);
> + uint64_t gpa = KVMCLOCK_GPA + idx * sizeof(struct
> pvclock_vcpu_time_info);
This series needs to be updated to catch up to upstream. Selftests now use
u32, u64, etc. And at least one patch missed an obvious opportunity for
guard().
> + wrmsr(MSR_KVM_SYSTEM_TIME_NEW, gpa | KVM_MSR_ENABLED);
> + GUEST_SYNC(0);
> + GUEST_SYNC(0);
> + GUEST_SYNC(0);
> +}
> +
> +static void set_tsc_offset(struct kvm_vcpu *vcpu, uint64_t offset)
> +{
> + struct kvm_device_attr attr = {
> + .group = KVM_VCPU_TSC_CTRL,
> + .attr = KVM_VCPU_TSC_OFFSET,
> + .addr = (__u64)(uintptr_t)&offset,
> + };
> +
> + TEST_REQUIRE(__vcpu_has_device_attr(vcpu, KVM_VCPU_TSC_CTRL,
> + KVM_VCPU_TSC_OFFSET) == 0);
> + vcpu_ioctl(vcpu, KVM_SET_DEVICE_ATTR, &attr);
This quite clearly belongs in library code.
> +}
> +
> +static void run_vcpu_once(struct kvm_vcpu *vcpu)
> +{
> + struct ucall uc;
> +
> + vcpu_run(vcpu);
> + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
> + switch (get_ucall(vcpu, &uc)) {
> + case UCALL_ABORT:
> + REPORT_GUEST_ASSERT(uc);
Gah, we really need to have vcpu_run() handle guest asserts.
> + break;
> + case UCALL_SYNC:
> + break;
> + default:
> + TEST_FAIL("Unexpected ucall");
> + }
> +}
> +
> +static void test_tsc_stable_bit(void)
> +{
> + struct pvclock_vcpu_time_info pvti;
> + struct kvm_vcpu *vcpus[2];
> + struct kvm_vm *vm;
> + int ret;
> +
> + pr_info("Testing PVCLOCK_TSC_STABLE_BIT with matched/unmatched TSCs\n");
> +
> + vm = vm_create_with_vcpus(2, guest_code_stable_bit, vcpus);
> + configure_pvclock(vm);
> +
> + /*
> + * Case 1: All TSCs matched (same frequency and offset).
> + * Master clock should be active, PVCLOCK_TSC_STABLE_BIT set.
> + */
> + run_vcpu_once(vcpus[0]);
> +
> + ret = __vcpu_ioctl(vcpus[0], KVM_GET_CLOCK_GUEST, &pvti);
> + TEST_ASSERT(!ret, "GET_CLOCK_GUEST should succeed with matched TSCs");
> + TEST_ASSERT(pvti.flags & PVCLOCK_TSC_STABLE_BIT,
> + "PVCLOCK_TSC_STABLE_BIT should be set with matched TSCs");
> +
> + /*
> + * Case 2: Different TSC offset, same frequency.
> + * Master clock should still be active (frequency matches), but
> + * PVCLOCK_TSC_STABLE_BIT should be cleared (offsets differ).
> + */
> + set_tsc_offset(vcpus[1], 12345678);
> + run_vcpu_once(vcpus[1]);
> + run_vcpu_once(vcpus[0]);
> +
> + ret = __vcpu_ioctl(vcpus[0], KVM_GET_CLOCK_GUEST, &pvti);
> + if (ret) {
> + /* Master clock disabled by offset mismatch — old kernel */
> + pr_info(" Skipping offset tests (master clock requires matched
> offsets)\n");
> + goto out_stable;
> + }
> + TEST_ASSERT(!(pvti.flags & PVCLOCK_TSC_STABLE_BIT),
> + "PVCLOCK_TSC_STABLE_BIT should be clear with
> offset-mismatched TSCs");
> +
> + /*
> + * Case 3: Different TSC frequency.
> + * Master clock should be disabled entirely.
> + */
> + vcpu_ioctl(vcpus[1], KVM_SET_TSC_KHZ,
> + (void *)(unsigned long)(__vcpu_ioctl(vcpus[1],
> KVM_GET_TSC_KHZ, NULL) / 2));
> + /* Write TSC to trigger kvm_synchronize_tsc / kvm_track_tsc_matching */
> + vcpu_set_msr(vcpus[1], MSR_IA32_TSC, 0);
> + run_vcpu_once(vcpus[1]);
> +
> + ret = __vcpu_ioctl(vcpus[0], KVM_GET_CLOCK_GUEST, &pvti);
> + TEST_ASSERT(ret && errno == EINVAL,
> + "GET_CLOCK_GUEST should fail with frequency-mismatched
> TSCs, got %d (errno %d)",
> + ret, errno);
> +
> +out_stable:
> + kvm_vm_free(vm);
> +}
> +
> +static void test_clock_guest_with_offsets(void)
> +{
> + struct pvclock_vcpu_time_info pvti0, pvti1, pvti1_after;
> + struct kvm_vcpu *vcpus[2];
> + struct kvm_vm *vm;
> + int64_t delta;
> + int ret;
> +
> + pr_info("Testing KVM_[GS]ET_CLOCK_GUEST with different TSC offsets\n");
> +
> + vm = vm_create_with_vcpus(2, guest_code_stable_bit, vcpus);
> + configure_pvclock(vm);
> +
> + /* Set different TSC offsets on the two vCPUs */
> + set_tsc_offset(vcpus[0], 0);
> + set_tsc_offset(vcpus[1], 1000000000ull);
> +
> + /* Run both to establish kvmclock */
> + run_vcpu_once(vcpus[0]);
> + run_vcpu_once(vcpus[1]);
> +
> + /* GET_CLOCK_GUEST on both — should succeed (master clock active) */
> + ret = __vcpu_ioctl(vcpus[0], KVM_GET_CLOCK_GUEST, &pvti0);
> + if (ret) {
> + pr_info(" Skipping (master clock requires matched offsets on
> this kernel)\n");
> + kvm_vm_free(vm);
> + return;
> + }
> + ret = __vcpu_ioctl(vcpus[1], KVM_GET_CLOCK_GUEST, &pvti1);
> + TEST_ASSERT(!ret, "GET_CLOCK_GUEST on vcpu1 failed");
> +
> + /* The tsc_timestamps should differ (different offsets) */
> + TEST_ASSERT(pvti0.tsc_timestamp != pvti1.tsc_timestamp,
> + "tsc_timestamps should differ with different offsets");
> +
> + /* Sleep to let time elapse, then restore vcpu0's clock */
> + sleep(1);
> + vcpu_ioctl(vcpus[0], KVM_SET_CLOCK_GUEST, &pvti0);
> +
> + /* Run vcpu0 to process the clock update */
> + run_vcpu_once(vcpus[0]);
> +
> + /* GET_CLOCK_GUEST on vcpu1 — should reflect the correction */
> + ret = __vcpu_ioctl(vcpus[1], KVM_GET_CLOCK_GUEST, &pvti1_after);
> + TEST_ASSERT(!ret, "GET_CLOCK_GUEST on vcpu1 after SET failed");
Please add proper APIs instead of copy+pasting the same code everywhere. E.g.
this should really be something like
vcpu_get_clock_guest(vcpus[1], ...);
where vcpu_ioctl() asserts success.
