On Sun, Oct 01, 2023, David Woodhouse wrote:
> +uint64_t kvm_get_wall_clock_epoch(struct kvm *kvm)
> +{
> + /*
> + * The guest calculates current wall clock time by adding
> + * system time (updated by kvm_guest_time_update below) to the
> + * wall clock specified here. We do the reverse here.
I would much rather this be a function comment that first explains what "epoch"
means in this context. "epoch" is a perfect fit, but I suspect it won't be all
that intuitive for many readers (definitely wasn't for me).
> + */
> +#ifdef CONFIG_X86_64
> + struct pvclock_vcpu_time_info hv_clock;
> + struct kvm_arch *ka = &kvm->arch;
> + unsigned long seq, local_tsc_khz = 0;
> + struct timespec64 ts;
> + uint64_t host_tsc;
> +
> + do {
> + seq = read_seqcount_begin(&ka->pvclock_sc);
> +
> + if (!ka->use_master_clock)
> + break;
This needs to zero local_tsc_khz, no? E.g. read everything on loop 1, but the
pvclock_sc changes because use_master_clock is disabled, and so loop 2 will bail
and the code below will consume garbage TSC/masterclock information from loop 1.
> + /* It all has to happen on the same CPU */
Define "it all", e.g. explain exactly why the cutoff for reenabling preemption is
after reading master_kernel_ns and not before, or not after kvm_get_time_scale().
> + get_cpu();
> +
> + local_tsc_khz = get_cpu_tsc_khz();
> +
> + if (local_tsc_khz &&
> + !kvm_get_walltime_and_clockread(&ts, &host_tsc))
> + local_tsc_khz = 0; /* Fall back to old method */
> +
> + hv_clock.tsc_timestamp = ka->master_cycle_now;
> + hv_clock.system_time = ka->master_kernel_ns + ka->kvmclock_offset;
> +
> + put_cpu();
> + } while (read_seqcount_retry(&ka->pvclock_sc, seq));
> +
> + /*
> + * If the conditions were right, and obtaining the wallclock+TSC was
> + * successful, calculate the KVM clock at the corresponding time and
> + * subtract one from the other to get the epoch in nanoseconds.
> + */
> + if (local_tsc_khz) {
> + kvm_get_time_scale(NSEC_PER_SEC, local_tsc_khz * 1000LL,
s/1000LL/NSEC_PER_USEC?
> + &hv_clock.tsc_shift,
> + &hv_clock.tsc_to_system_mul);
> + return ts.tv_nsec + NSEC_PER_SEC * ts.tv_sec -
> + __pvclock_read_cycles(&hv_clock, host_tsc);
> + }
> +#endif
