Re: [PATCH v7 34/36] KVM: selftests: Add master clock offset test

[Sean Christopherson]

On Tue, Jul 28, 2026, David Woodhouse wrote:
> From: David Woodhouse <dwmw@xxxxxxxxxxxx>
> 
> Verify that KVM master clock mode remains active when vCPUs have
> different TSC offsets but the same frequency. Creates three vCPUs,
> sets one to a different TSC value, and confirms:
> 
>  - KVM_CLOCK_HOST_TSC is set (master clock active)
>  - KVM_CLOCK_TSC_STABLE IS set (master clock ⇒ stable flag)
>  - PVCLOCK_TSC_STABLE_BIT is NOT set (offsets differ)
> 
> Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
> Assisted-by: Kiro (claude-opus-4.6-1m)
> ---
>  tools/testing/selftests/kvm/Makefile.kvm      |   1 +
>  .../kvm/x86/masterclock_offset_test.c         | 180 ++++++++++++++++++
>  2 files changed, 181 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/x86/masterclock_offset_test.c
> 
> diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
> index 95539deea266..f6027a35f790 100644
> --- a/tools/testing/selftests/kvm/Makefile.kvm
> +++ b/tools/testing/selftests/kvm/Makefile.kvm
> @@ -108,6 +108,7 @@ TEST_GEN_PROGS_x86 += x86/pmu_event_filter_test
>  TEST_GEN_PROGS_x86 += x86/private_mem_conversions_test
>  TEST_GEN_PROGS_x86 += x86/private_mem_kvm_exits_test
>  TEST_GEN_PROGS_x86 += x86/pvclock_test
> +TEST_GEN_PROGS_x86 += x86/masterclock_offset_test

Keep these sorted alphabetically.

>  TEST_GEN_PROGS_x86 += x86/pvclock_migration_test
>  TEST_GEN_PROGS_x86 += x86/set_boot_cpu_id
>  TEST_GEN_PROGS_x86 += x86/set_sregs_test
> diff --git a/tools/testing/selftests/kvm/x86/masterclock_offset_test.c b/tools/testing/selftests/kvm/x86/masterclock_offset_test.c
> new file mode 100644
> index 000000000000..88e2bd2edab5
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/masterclock_offset_test.c
> @@ -0,0 +1,180 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Test that KVM master clock mode works with different TSC offsets
> + * as long as all vCPUs have the same TSC frequency.
> + */
> +#include <stdint.h>
> +#include <string.h>
> +
> +#include "test_util.h"
> +#include "kvm_util.h"
> +#include "processor.h"
> +
> +#include <asm/pvclock-abi.h>
> +
> +#define KVMCLOCK_GPA	0xc0000000ull
> +#define TSC_OFFSET	(1000000000ULL)
> +
> +static uint64_t pvclock_calc(struct pvclock_vcpu_time_info *pvti, uint64_t guest_tsc)
> +{
> +	uint64_t delta = guest_tsc - pvti->tsc_timestamp;
> +
> +	if (pvti->tsc_shift >= 0)
> +		delta <<= pvti->tsc_shift;
> +	else
> +		delta >>= -(int)pvti->tsc_shift;
> +
> +	return pvti->system_time + ((__uint128_t)delta * pvti->tsc_to_system_mul >> 32);
> +}
> +
> +static void guest_code(void)
> +{
> +	wrmsr(MSR_KVM_SYSTEM_TIME_NEW, KVMCLOCK_GPA | KVM_MSR_ENABLED);
> +	for (;;)
> +		GUEST_SYNC(0);
> +}
> +
> +int main(void)
> +{
> +	struct kvm_vcpu *vcpus[3];
> +	struct kvm_clock_data clock;
> +	struct pvclock_vcpu_time_info pvti[3];
> +	struct kvm_vm *vm;
> +	uint64_t offset0, host_tsc, clk0, clk2;
> +	int i;
> +
> +	TEST_REQUIRE(sys_clocksource_is_based_on_tsc());
> +
> +	vm = vm_create_with_vcpus(3, guest_code, vcpus);
> +
> +	TEST_REQUIRE(!__vcpu_has_device_attr(vcpus[0], KVM_VCPU_TSC_CTRL,
> +					     KVM_VCPU_TSC_OFFSET));
> +
> +	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
> +				    KVMCLOCK_GPA, 1,
> +				    vm_calc_num_guest_pages(VM_MODE_DEFAULT,
> +							   getpagesize()), 0);
> +	virt_map(vm, KVMCLOCK_GPA, KVMCLOCK_GPA,
> +		 vm_calc_num_guest_pages(VM_MODE_DEFAULT, getpagesize()));
> +
> +	/* Get vCPU 0's default offset and set vCPU 2's offset higher */
> +	vcpu_device_attr_get(vcpus[0], KVM_VCPU_TSC_CTRL,
> +			     KVM_VCPU_TSC_OFFSET, &offset0);
> +	uint64_t offset2 = offset0 + TSC_OFFSET;
> +	vcpu_device_attr_set(vcpus[2], KVM_VCPU_TSC_CTRL,
> +			     KVM_VCPU_TSC_OFFSET, &offset2);
> +
> +	/* Run each vCPU to enable kvmclock (with offset already set) */
> +	for (i = 0; i < 3; i++) {

ARRAY_SIZE(vcpus)

> +		vcpu_run(vcpus[i]);
> +		TEST_ASSERT_KVM_EXIT_REASON(vcpus[i], KVM_EXIT_IO);
> +	}
> +
> +	/* Check master clock is active */
> +	memset(&clock, 0, sizeof(clock));
> +	vm_ioctl(vm, KVM_GET_CLOCK, &clock);

Provide an API.

> +	pr_info("KVM_GET_CLOCK flags: 0x%x\n", clock.flags);
> +	TEST_ASSERT(clock.flags & KVM_CLOCK_HOST_TSC,
> +		    "Master clock should be active, flags=0x%x", clock.flags);
> +	TEST_ASSERT(clock.flags & KVM_CLOCK_TSC_STABLE,
> +		    "KVM_CLOCK_TSC_STABLE should be set, flags=0x%x", clock.flags);
> +
> +	/* Get per-vCPU pvclock in order 0, 2, 1 */
> +	int order[] = {0, 2, 1};

Please don't declare variables in the middle code.  Yes, it's now technically
possible thanks to all the guard() magic, but it's still against preferred kernel
style.

Though this itself is probably a really, really good indication that this needs
to be split into multiple testcases.  Maybe using TEST_F() and a custom fixture?