[PATCH v8 01/17] KVM: x86: Update "last guest TSC" snapshot prior to enabling IRQs/preemption

Sean Christopherson <[email protected]>
Newsgroups org.kernel.vger.linux-kernel,org.kernel.vger.kvm
Message-ID <[email protected]>
When refreshing the last observed guest TSC during a guest time update,
write the snapshot before enabling IRQs, i.e. before enabling preemption.
If the task is migrated between updating the local tsc_timestamp, e.g. to
account for catch-up mode, and setting last_guest_tsc, kvm_arch_vcpu_load()
would set the vCPU's TSC offset using the old last_guest_tsc.

In practice, the bug is largely benign as it's not even strictly necessary
for KVM to refresh last_guest_tsc when updating guest time, as KVM's goal
is purely to prevent the guest from observing time jump backwards, i.e.
super duper strictly speaking, KVM only *needs* to update last_guest_tsc in
the VM-Exit path.

In fact, the update kvm_guest_time_update() in wasn't even added to play
nice with kvm_arch_vcpu_load(), it was added by commit 28e4639adf0c ("KVM:
x86: Fix kvmclock bug") to fix code that no longer exists.  As of commit
28e4639adf0c, kvm_guest_time_update() also consumed last_guest_tsc, to try
and prevent guest time from jumping backwards.  That code was eventually
removed by commit f25e656d31ad ("KVM: x86: fix tsc catchup issue with tsc
scaling"), but the last_guest_tsc update hung around.

Keep the update even though it's technically ok to drop the update, e.g. so
that the tsc_catchup updates aren't lost, and so that the guest won't see a
PV clock timestamp that appears to be in the future.

Signed-off-by: Sean Christopherson <[email protected]>
---
 arch/x86/kvm/x86.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d94b59140c45..d3b47e38698c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1820,6 +1820,12 @@ int kvm_guest_time_update(struct kvm_vcpu *v)
 		}
 	}
 
+	/*
+	 * Refresh L1's last "observed" TSC to match the PV clock's timestamp,
+	 * e.g. so that the guest can't see a TSC that's behind the reference.
+	 */
+	vcpu->last_guest_tsc = tsc_timestamp;
+
 	local_irq_restore(flags);
 
 	/* With all the info we got, fill in the values */
@@ -1841,7 +1847,6 @@ int kvm_guest_time_update(struct kvm_vcpu *v)
 	hv_clock.tsc_to_system_mul = vcpu->pvclock_tsc_mul;
 	hv_clock.tsc_timestamp = tsc_timestamp;
 	hv_clock.system_time = kernel_ns + v->kvm->arch.kvmclock_offset;
-	vcpu->last_guest_tsc = tsc_timestamp;
 
 	/* If the host uses TSC clocksource, then it is stable */
 	hv_clock.flags = 0;
-- 
2.55.0.571.g244d577d93-goog
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.