[PATCH v2 04/11] KVM: x86: Process "guest stopped request" once per guest time update

Sean Christopherson posted 11 patches 1 year ago
[PATCH v2 04/11] KVM: x86: Process "guest stopped request" once per guest time update
Posted by Sean Christopherson 1 year ago
Handle "guest stopped" requests once per guest time update in preparation
of restoring KVM's historical behavior of setting PVCLOCK_GUEST_STOPPED
for kvmclock and only kvmclock.  For now, simply move the code to minimize
the probability of an unintentional change in functionally.

Note, in practice, all clocks are guaranteed to see the request (or not)
even though each PV clock processes the request individual, as KVM holds
vcpu->mutex (blocks KVM_KVMCLOCK_CTRL) and it should be impossible for
KVM's suspend notifier to run while KVM is handling requests.  And because
the helper updates the reference flags, all subsequent PV clock updates
will pick up PVCLOCK_GUEST_STOPPED.

Note #2, once PVCLOCK_GUEST_STOPPED is restricted to kvmclock, the
horrific #ifdef will go away.

Cc: Paul Durrant <pdurrant@amazon.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/x86.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d8ee37dd2b57..de281c328cb1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3150,11 +3150,6 @@ static void kvm_setup_guest_pvclock(struct kvm_vcpu *v,
 	/* retain PVCLOCK_GUEST_STOPPED if set in guest copy */
 	vcpu->hv_clock.flags |= (guest_hv_clock->flags & PVCLOCK_GUEST_STOPPED);
 
-	if (vcpu->pvclock_set_guest_stopped_request) {
-		vcpu->hv_clock.flags |= PVCLOCK_GUEST_STOPPED;
-		vcpu->pvclock_set_guest_stopped_request = false;
-	}
-
 	memcpy(guest_hv_clock, &vcpu->hv_clock, sizeof(*guest_hv_clock));
 
 	if (force_tsc_unstable)
@@ -3264,6 +3259,18 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
 	if (use_master_clock)
 		vcpu->hv_clock.flags |= PVCLOCK_TSC_STABLE_BIT;
 
+	if (vcpu->pv_time.active
+#ifdef CONFIG_KVM_XEN
+	 || vcpu->xen.vcpu_info_cache.active
+	 || vcpu->xen.vcpu_time_info_cache.active
+#endif
+	    ) {
+		if (vcpu->pvclock_set_guest_stopped_request) {
+			vcpu->hv_clock.flags |= PVCLOCK_GUEST_STOPPED;
+			vcpu->pvclock_set_guest_stopped_request = false;
+		}
+	}
+
 	if (vcpu->pv_time.active)
 		kvm_setup_guest_pvclock(v, &vcpu->pv_time, 0, false);
 #ifdef CONFIG_KVM_XEN
-- 
2.48.1.362.g079036d154-goog
Re: [PATCH v2 04/11] KVM: x86: Process "guest stopped request" once per guest time update
Posted by Paul Durrant 1 year ago
On 01/02/2025 01:38, Sean Christopherson wrote:
> Handle "guest stopped" requests once per guest time update in preparation
> of restoring KVM's historical behavior of setting PVCLOCK_GUEST_STOPPED
> for kvmclock and only kvmclock.  For now, simply move the code to minimize
> the probability of an unintentional change in functionally.
> 
> Note, in practice, all clocks are guaranteed to see the request (or not)
> even though each PV clock processes the request individual, as KVM holds
> vcpu->mutex (blocks KVM_KVMCLOCK_CTRL) and it should be impossible for
> KVM's suspend notifier to run while KVM is handling requests.  And because
> the helper updates the reference flags, all subsequent PV clock updates
> will pick up PVCLOCK_GUEST_STOPPED.
> 
> Note #2, once PVCLOCK_GUEST_STOPPED is restricted to kvmclock, the
> horrific #ifdef will go away.
> 

:-)

> Cc: Paul Durrant <pdurrant@amazon.com>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/kvm/x86.c | 17 ++++++++++++-----
>   1 file changed, 12 insertions(+), 5 deletions(-)
> 

Reviewed-by: Paul Durrant <paul@xen.org>