[PATCH] KVM: x86: add bit to indicate correct tsc_shift

[PATCH] KVM: x86: add bit to indicate correct tsc_shift

[Marcelo Tosatti]

Before commit 78db6a5037965429c04d708281f35a6e5562d31b,
kvm_guest_time_update() would use vcpu->virtual_tsc_khz to calculate
tsc_shift value in the vcpus pvclock structure written to guest memory.

For those kernels, if vcpu->virtual_tsc_khz != tsc_khz (which can be the
case when guest state is restored via migration, or if tsc-khz option is
passed to QEMU), and TSC scaling is not enabled (which happens if the
difference between the frequency requested via KVM_SET_TSC_KHZ and the
host TSC KHZ is smaller than 250ppm), then there can be a difference
between what KVM_GET_CLOCK would return and what the guest reads as
kvmclock value.

When KVM_SET_CLOCK'ing what is read with KVM_GET_CLOCK, the
guest can observe a forward or backwards time jump.

Advertise to userspace that current kernel contains
this fix, so QEMU can workaround the problem by reading
pvclock via guest memory directly otherwise.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 6aaae18f1854..61c5876cfd87 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -2160,7 +2160,8 @@ static inline int kvm_cpu_get_apicid(int mps_cpu)
 int memslot_rmap_alloc(struct kvm_memory_slot *slot, unsigned long npages);
 
 #define KVM_CLOCK_VALID_FLAGS						\
-	(KVM_CLOCK_TSC_STABLE | KVM_CLOCK_REALTIME | KVM_CLOCK_HOST_TSC)
+	(KVM_CLOCK_TSC_STABLE | KVM_CLOCK_REALTIME | KVM_CLOCK_HOST_TSC | \
+	 KVM_CLOCK_CORRECT_TSC_SHIFT)
 
 #define KVM_X86_VALID_QUIRKS			\
 	(KVM_X86_QUIRK_LINT0_REENABLED |	\
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index da4bbd043a7b..142a53d06100 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3014,6 +3014,8 @@ static void __get_kvmclock(struct kvm *kvm, struct kvm_clock_data *data)
 		data->clock = get_kvmclock_base_ns() + ka->kvmclock_offset;
 	}
 
+	data->flags |= KVM_CLOCK_CORRECT_TSC_SHIFT;
+
 	put_cpu();
 }
 
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 55155e262646..7b94d7bd03a6 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1301,6 +1301,9 @@ struct kvm_irqfd {
 #define KVM_CLOCK_TSC_STABLE		2
 #define KVM_CLOCK_REALTIME		(1 << 2)
 #define KVM_CLOCK_HOST_TSC		(1 << 3)
+/* whether tsc_shift as seen by the guest matches guest visible TSC */
+/* This is true since commit 78db6a5037965429c04d708281f35a6e5562d31b */
+#define KVM_CLOCK_CORRECT_TSC_SHIFT	(1 << 4)
 
 struct kvm_clock_data {
 	__u64 clock;

Re: [PATCH] KVM: x86: add bit to indicate correct tsc_shift

[Paolo Bonzini]

On 1/20/23 02:06, Marcelo Tosatti wrote:
> Before commit 78db6a5037965429c04d708281f35a6e5562d31b,
> kvm_guest_time_update() would use vcpu->virtual_tsc_khz to calculate
> tsc_shift value in the vcpus pvclock structure written to guest memory.
> 
> For those kernels, if vcpu->virtual_tsc_khz != tsc_khz (which can be the
> case when guest state is restored via migration, or if tsc-khz option is
> passed to QEMU), and TSC scaling is not enabled (which happens if the
> difference between the frequency requested via KVM_SET_TSC_KHZ and the
> host TSC KHZ is smaller than 250ppm), then there can be a difference
> between what KVM_GET_CLOCK would return and what the guest reads as
> kvmclock value.

I don't think it's justifiable to further complicate the userspace API 
for a bug that's been fixed six years ago.  I'd be very surprised if any 
combination of modern upstream {QEMU,kernel} is going to do a successful 
migration from such an old {QEMU,kernel}.  RHEL/CentOS are able to do so 
because *specific pairs* have been tested, but as far as upstream is 
concerned this adds complexity that absolutely no one will use.

I replied on the QEMU patch with further suggestions.

Paolo