Re: [PATCH RFC 1/1] KVM: x86: add param to update master clock periodically

[David Woodhouse]

On Mon, 2023-10-02 at 17:53 -0700, Sean Christopherson wrote:
> 
> This is *very* lightly tested, as in it compiles and doesn't explode, but that's
> about all I've tested.

I don't think it's working, if I understand what it's supposed to be
doing.

I hacked my wallclock patch *not* to use
kvm_get_walltime_and_clockread(), but instead use
kvm_get_time_and_clockread() so it should be able to compare
monotonic_raw with kvmclock time. It looks like this.

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.
	 */
#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;

		/* It all has to happen on the same CPU */
		get_cpu();

		local_tsc_khz = get_cpu_tsc_khz();

		if (local_tsc_khz &&
		    !kvm_get_time_and_clockread(&ts.tv_sec, &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,
				   &hv_clock.tsc_shift,
				   &hv_clock.tsc_to_system_mul);

		uint64_t res = __pvclock_read_cycles(&hv_clock, host_tsc);
		uint64_t d2 = ts.tv_sec + ka->kvmclock_offset;
		printk("Calculated %lld (%lld/%lld delta %lld, ns %lld o %lld)\n",
		       res,
		       ts.tv_sec, d2, d2-res,
		       ka->master_kernel_ns, ka->kvmclock_offset);
		if (0)
		return ts.tv_nsec + NSEC_PER_SEC * ts.tv_sec -
			__pvclock_read_cycles(&hv_clock, host_tsc);
	}
#endif
	return ktime_get_real_ns() - get_kvmclock_ns(kvm);
}

I also removed the kvm_make_all_cpus_request(kvm,
KVM_REQ_MASTERCLOCK_UPDATE) from kvm_xen_shared_info_init(). I don't
even know why that's there at all, let alone on *all* vCPUs. So now KVM
doesn't keep clamping the kvmclock back to monotonic_raw.

When I run xen_shinfo_test, the kvmclock still drifts from the
"monotonic_raw" I get from kvm_get_time_and_clockread(), even with your
patch.

[98513.851371] Calculated 522601895 (98513572796678/522601913 delta 18, ns 98513057857491 o -98513050194765)
[98513.851388] Calculated 522619091 (98513572813874/522619109 delta 18, ns 98513057857491 o -98513050194765)
[98513.851394] Calculated 522625423 (98513572820206/522625441 delta 18, ns 98513057857491 o -98513050194765)
[98513.851401] Calculated 522631389 (98513572826172/522631407 delta 18, ns 98513057857491 o -98513050194765)
[98513.851406] Calculated 522636947 (98513572831730/522636965 delta 18, ns 98513057857491 o -98513050194765)
[98513.851412] Calculated 522643004 (98513572837787/522643022 delta 18, ns 98513057857491 o -98513050194765)
[98513.851417] Calculated 522648344 (98513572843127/522648362 delta 18, ns 98513057857491 o -98513050194765)
[98513.851423] Calculated 522654367 (98513572849150/522654385 delta 18, ns 98513057857491 o -98513050194765)
...
[98517.386027] Calculated 4057257718 (98517107452615/4057257850 delta 132, ns 98513057857491 o -98513050194765)
[98517.386030] Calculated 4057261038 (98517107455934/4057261169 delta 131, ns 98513057857491 o -98513050194765)
[98517.386034] Calculated 4057265133 (98517107460029/4057265264 delta 131, ns 98513057857491 o -98513050194765)
[98517.386037] Calculated 4057268310 (98517107463206/4057268441 delta 131, ns 98513057857491 o -98513050194765)
[98517.386040] Calculated 4057271463 (98517107466359/4057271594 delta 131, ns 98513057857491 o -98513050194765)
[98517.386044] Calculated 4057274508 (98517107469404/4057274639 delta 131, ns 98513057857491 o -98513050194765)
[98517.386048] Calculated 4057278587 (98517107473483/4057278718 delta 131, ns 98513057857491 o -98513050194765)
[98517.386051] Calculated 4057281674 (98517107476570/4057281805 delta 131, ns 98513057857491 o -98513050194765)
[98517.386057] Calculated 4057288187 (98517107483083/4057288318 delta 131, ns 98513057857491 o -98513050194765)
[98517.386064] Calculated 4057294557 (98517107489453/4057294688 delta 131, ns 98513057857491 o -98513050194765)

Attachment: smime.p7s
Description: S/MIME cryptographic signature