Re: [PATCH v5 05/34] KVM: selftests: Add KVM/PV clock selftest to prove timer correction

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Flawed API/Feature Check for KVM_GET_CLOCK_GUEST support.
- [Medium] Missing Capability Check for KVM_VCPU_TSC_OFFSET device attribute.
--

commit 26a6d06b6a0ac05e89b1ed831e29fd555de24ff8
Author: Jack Allister <jalliste@amazon.com>

KVM: selftests: Add KVM/PV clock selftest to prove timer correction

This commit adds a KVM/PV clock selftest to verify timer correction during
simulated live update or migration. It verifies that using KVM_GET_CLOCK_GUEST
and KVM_SET_CLOCK_GUEST maintains the relationship between a guest's TSC and
KVM/PV clock.

> diff --git a/tools/testing/selftests/kvm/x86/pvclock_test.c b/tools/testing/selftests/kvm/x86/pvclock_test.c
> new file mode 100644
> index 0000000000000..aecd62fc8a93c
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/pvclock_test.c

[ ... ]

> +int main(int argc, char *argv[])
> +{

[ ... ]

> +	/* 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 != -1 || errno != ENOTTY);

[Severity: Medium]
Does this condition correctly detect unsupported ioctls on x86?

Looking at the KVM implementation, kvm_arch_vcpu_ioctl() on x86 returns
-EINVAL for unsupported ioctls rather than -ENOTTY. On an older kernel where
the ioctl is not supported, wouldn't errno be EINVAL, making this condition
true and causing the test to proceed and fail later instead of safely
skipping?

> +	}

[ ... ]

> +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,
> +	};
> +	vcpu_ioctl(vcpu, KVM_SET_DEVICE_ATTR, &attr);

[Severity: Medium]
Will this unconditionally fail on older kernels that lack support for the
KVM_VCPU_TSC_OFFSET device attribute?

While the test requires KVM_CAP_TSC_CONTROL earlier in main(), the
KVM_VCPU_TSC_OFFSET attribute is a separate, newer feature. Should this
include a capability check using __vcpu_has_device_attr() before invoking
vcpu_ioctl() to ensure the test safely skips on older kernels?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608145455.89187-1-dwmw2@infradead.org?part=5