On Tue, Jul 28, 2026, David Woodhouse wrote:
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5c78dd1e4c69..0680332d7d45 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3435,6 +3435,169 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu
> *vcpu,
> }
> }
>
> +#ifdef CONFIG_X86_64
> +static int kvm_vcpu_ioctl_get_clock_guest(struct kvm_vcpu *v, void __user
> *argp)
> +{
> + struct pvclock_vcpu_time_info hv_clock = {};
> + struct kvm_vcpu_arch *vcpu = &v->arch;
> + struct kvm_arch *ka = &v->kvm->arch;
> + unsigned int seq;
> +
> + /*
> + * If KVM_REQ_CLOCK_UPDATE is already pending, or if the pvclock
> + * has never been generated at all, call kvm_guest_time_update().
> + */
> + if (kvm_check_request(KVM_REQ_CLOCK_UPDATE, v) || !vcpu->hw_tsc_hz) {
> + int idx = srcu_read_lock(&v->kvm->srcu);
> + int ret = kvm_guest_time_update(v);
> +
> + srcu_read_unlock(&v->kvm->srcu, idx);
guard(srcu)(&vcpu->kvm->srcu);
if (kvm_guest_time_update(v))
return -EBUSY;
> + if (ret)
> + return -EINVAL;
This should be -EBUSY, because KVM_REQ_CLOCK_UPDATE is a transient condition.
And that's why a capability is needed: if userspace goes with the "probe"
method,
it could get a temporary failure, and then a naive userspace could stop using
the ioctl entirely.
> + }
> +
> + /*
> + * Reconstruct the pvclock from the master clock state, matching
> + * exactly what kvm_guest_time_update() writes to the guest.
> + */
> + do {
> + seq = read_seqcount_begin(&ka->pvclock_sc);
> +
> + if (!ka->use_master_clock)
> + return -EINVAL;
EINVAL also feels wrong, userspace hasn't done anything wrong. Maybe -ENODATA?
Not sure what the right returnis.
> +
> + hv_clock.tsc_timestamp = kvm_read_l1_tsc(v,
> ka->master_cycle_now);
> + hv_clock.system_time = ka->master_kernel_ns +
> ka->kvmclock_offset;
> + } while (read_seqcount_retry(&ka->pvclock_sc, seq));
> +
> + hv_clock.tsc_shift = vcpu->pvclock_tsc_shift;
> + hv_clock.tsc_to_system_mul = vcpu->pvclock_tsc_mul;
> + hv_clock.flags = ka->all_vcpus_matched_tsc ? PVCLOCK_TSC_STABLE_BIT : 0;
> +
> + if (copy_to_user(argp, &hv_clock, sizeof(hv_clock)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +/*
> + * Reverse the calculation in the hv_clock definition.
> + *
> + * time_ns = ( (cycles << shift) * mul ) >> 32;
> + * (although shift can be negative, so that's bad C)
> + *
> + * So for a single second,
> + * NSEC_PER_SEC = ( ( FREQ_HZ << shift) * mul ) >> 32
> + * NSEC_PER_SEC << 32 = ( FREQ_HZ << shift ) * mul
> + * ( NSEC_PER_SEC << 32 ) / mul = FREQ_HZ << shift
> + * ( NSEC_PER_SEC << 32 ) / mul ) >> shift = FREQ_HZ
> + */
> +static u64 hvclock_to_hz(u32 mul, s8 shift)
> +{
> + u64 tm = NSEC_PER_SEC << 32;
> +
> + /* Maximise precision. Shift right until the top bit is set */
> + tm <<= 2;
> + shift += 2;
> +
> + /* While 'mul' is even, increase the shift *after* the division */
> + while (!(mul & 1)) {
> + shift++;
> + mul >>= 1;
> + }
> +
> + tm /= mul;
> +
> + if (shift >= 64)
> + return 0;
> + if (shift > 0)
> + return tm >> shift;
> + if (shift <= -64)
> + return 0;
> + return tm << -shift;
> +}
> +
> +static int kvm_vcpu_ioctl_set_clock_guest(struct kvm_vcpu *v, void __user
> *argp)
> +{
> + struct pvclock_vcpu_time_info user_hv_clock;
> + struct kvm *kvm = v->kvm;
> + struct kvm_arch *ka = &kvm->arch;
> + u64 curr_tsc_hz, user_tsc_hz;
> + u64 user_clk_ns;
> + u64 guest_tsc;
> + int rc = 0;
> +
> + if (copy_from_user(&user_hv_clock, argp, sizeof(user_hv_clock)))
> + return -EFAULT;
> +
> + if (user_hv_clock.pad0 || user_hv_clock.pad[0] || user_hv_clock.pad[1])
> + return -EINVAL;
> +
> + if (!user_hv_clock.tsc_to_system_mul)
> + return -EINVAL;
> +
> + if (user_hv_clock.tsc_shift < -31 || user_hv_clock.tsc_shift > 31)
> + return -EINVAL;
> +
> + user_tsc_hz = hvclock_to_hz(user_hv_clock.tsc_to_system_mul,
> + user_hv_clock.tsc_shift);
> +
> + kvm_hv_request_tsc_page_update(kvm);
> +
> + /*
> + * kvm_start_pvclock_update() takes tsc_write_lock and opens
> + * the pvclock seqcount; kvm_end_pvclock_update() closes both.
> + * All clock state modifications between them are atomic with
> + * respect to readers in kvm_guest_time_update().
> + */
> + kvm_start_pvclock_update(kvm);
> + pvclock_update_vm_gtod_copy(kvm);
> +
> + if (!ka->use_master_clock) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + curr_tsc_hz = (u64)get_cpu_tsc_khz() * HZ_PER_KHZ;
> + if (unlikely(curr_tsc_hz == 0)) {
> + rc = -EINVAL;
Same comments here regarding return codes.
> + goto out;
> + }
> +
> + if (kvm_caps.has_tsc_control)
> + curr_tsc_hz = kvm_scale_tsc(curr_tsc_hz,
> + v->arch.l1_tsc_scaling_ratio);
> +
> + /*
> + * Allow for a discrepancy of 1 kHz either way between the TSC
> + * frequency used to generate the user's pvclock and the current
> + * host's measured frequency, since they may not precisely match.
> + */
> + if (user_tsc_hz < curr_tsc_hz - 1000 ||
> + user_tsc_hz > curr_tsc_hz + 1000) {
I don't follow, why is KVM restricting what frequency userspace can set?
> + rc = -ERANGE;
> + goto out;
> + }
