| Message ID | 20250118005552.2626804-10-seanjc@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: x86: pvclock fixes and cleanups | expand |
Sean Christopherson <seanjc@google.com> writes: > When updating paravirtual clocks, setup the Hyper-V TSC page before > Xen PV clocks. This will allow dropping xen_pvclock_tsc_unstable in favor > of simply clearing PVCLOCK_TSC_STABLE_BIT in the reference flags. > > Cc: Vitaly Kuznetsov <vkuznets@redhat.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/x86.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 9eabd70891dd..c68e7f7ba69d 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3280,6 +3280,8 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) > hv_clock.flags &= ~PVCLOCK_GUEST_STOPPED; > } > > + kvm_hv_setup_tsc_page(v->kvm, &hv_clock); > + > #ifdef CONFIG_KVM_XEN > if (vcpu->xen.vcpu_info_cache.active) > kvm_setup_guest_pvclock(&hv_clock, v, &vcpu->xen.vcpu_info_cache, > @@ -3289,7 +3291,6 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) > kvm_setup_guest_pvclock(&hv_clock, v, &vcpu->xen.vcpu_time_info_cache, 0, > xen_pvclock_tsc_unstable); > #endif > - kvm_hv_setup_tsc_page(v->kvm, &hv_clock); > return 0; > } "No functional change detected". Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> (What I'm wondering is if (from mostly theoretical PoV) it's OK to pass *some* of the PV clocks as stable and some as unstable to the same guest, i.e. if it would make sense to disable Hyper-V TSC page when KVM_XEN_HVM_CONFIG_PVCLOCK_TSC_UNSTABLE too. I don't know if anyone combines Xen and Hyper-V emulation capabilities for the same guest on KVM though.)
On Mon, Jan 20, 2025, Vitaly Kuznetsov wrote: > Sean Christopherson <seanjc@google.com> writes: > > > When updating paravirtual clocks, setup the Hyper-V TSC page before > > Xen PV clocks. This will allow dropping xen_pvclock_tsc_unstable in favor > > of simply clearing PVCLOCK_TSC_STABLE_BIT in the reference flags. > > > > Cc: Vitaly Kuznetsov <vkuznets@redhat.com> > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > --- > > arch/x86/kvm/x86.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 9eabd70891dd..c68e7f7ba69d 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -3280,6 +3280,8 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) > > hv_clock.flags &= ~PVCLOCK_GUEST_STOPPED; > > } > > > > + kvm_hv_setup_tsc_page(v->kvm, &hv_clock); > > + > > #ifdef CONFIG_KVM_XEN > > if (vcpu->xen.vcpu_info_cache.active) > > kvm_setup_guest_pvclock(&hv_clock, v, &vcpu->xen.vcpu_info_cache, > > @@ -3289,7 +3291,6 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) > > kvm_setup_guest_pvclock(&hv_clock, v, &vcpu->xen.vcpu_time_info_cache, 0, > > xen_pvclock_tsc_unstable); > > #endif > > - kvm_hv_setup_tsc_page(v->kvm, &hv_clock); > > return 0; > > } > > "No functional change detected". > > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> > > (What I'm wondering is if (from mostly theoretical PoV) it's OK to pass > *some* of the PV clocks as stable and some as unstable to the same > guest, i.e. if it would make sense to disable Hyper-V TSC page when > KVM_XEN_HVM_CONFIG_PVCLOCK_TSC_UNSTABLE too. I think it's ok to keep the Hyper-V TSC page in this case. It's not that the Xen PV clock is truly unstable, it's that some guests get tripped up by the STABLE flag. A guest that can't handle the STABLE flag has bigger problems than the existence of a completely unrelated clock that is implied to be stable. > I don't know if anyone combines Xen and Hyper-V emulation capabilities for > the same guest on KVM though.) That someone would have to be quite "brave" :-D
On 21/01/2025 15:44, Sean Christopherson wrote: [snip] > > I think it's ok to keep the Hyper-V TSC page in this case. It's not that the Xen > PV clock is truly unstable, it's that some guests get tripped up by the STABLE > flag. A guest that can't handle the STABLE flag has bigger problems than the > existence of a completely unrelated clock that is implied to be stable. > Agreed. >> I don't know if anyone combines Xen and Hyper-V emulation capabilities for >> the same guest on KVM though.) > > That someone would have to be quite "brave" :-D Maybe :-) Reviewed-by: Paul Durrant <paul@xen.org>
On Tue, 2025-01-21 at 15:59 +0000, Paul Durrant wrote: > On 21/01/2025 15:44, Sean Christopherson wrote: > [snip] > > > > I think it's ok to keep the Hyper-V TSC page in this case. It's not that the Xen > > PV clock is truly unstable, it's that some guests get tripped up by the STABLE > > flag. A guest that can't handle the STABLE flag has bigger problems than the > > existence of a completely unrelated clock that is implied to be stable. > > > > Agreed. > > > > I don't know if anyone combines Xen and Hyper-V emulation capabilities for > > > the same guest on KVM though.) > > > > That someone would have to be quite "brave" :-D > > Maybe :-) Xen itself does offer some Hyper-V enlightenments, and we might reasonably expect KVM-based hypervisors to offer the same. We explicitly do account for the KVM CPUID leaves moving up to let the Hyper-V ones exist. I don't recall if Xen's Hyper-V support includes the TSC page though.
On 21/01/2025 17:16, David Woodhouse wrote: > On Tue, 2025-01-21 at 15:59 +0000, Paul Durrant wrote: >> On 21/01/2025 15:44, Sean Christopherson wrote: >> [snip] >>> >>> I think it's ok to keep the Hyper-V TSC page in this case. It's not that the Xen >>> PV clock is truly unstable, it's that some guests get tripped up by the STABLE >>> flag. A guest that can't handle the STABLE flag has bigger problems than the >>> existence of a completely unrelated clock that is implied to be stable. >>> >> >> Agreed. >> >>>> I don't know if anyone combines Xen and Hyper-V emulation capabilities for >>>> the same guest on KVM though.) >>> >>> That someone would have to be quite "brave" :-D >> >> Maybe :-) > > Xen itself does offer some Hyper-V enlightenments, and we might > reasonably expect KVM-based hypervisors to offer the same. We > explicitly do account for the KVM CPUID leaves moving up to let the > Hyper-V ones exist. > > I don't recall if Xen's Hyper-V support includes the TSC page though. It does :-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 9eabd70891dd..c68e7f7ba69d 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3280,6 +3280,8 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) hv_clock.flags &= ~PVCLOCK_GUEST_STOPPED; } + kvm_hv_setup_tsc_page(v->kvm, &hv_clock); + #ifdef CONFIG_KVM_XEN if (vcpu->xen.vcpu_info_cache.active) kvm_setup_guest_pvclock(&hv_clock, v, &vcpu->xen.vcpu_info_cache, @@ -3289,7 +3291,6 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) kvm_setup_guest_pvclock(&hv_clock, v, &vcpu->xen.vcpu_time_info_cache, 0, xen_pvclock_tsc_unstable); #endif - kvm_hv_setup_tsc_page(v->kvm, &hv_clock); return 0; }
When updating paravirtual clocks, setup the Hyper-V TSC page before Xen PV clocks. This will allow dropping xen_pvclock_tsc_unstable in favor of simply clearing PVCLOCK_TSC_STABLE_BIT in the reference flags. Cc: Vitaly Kuznetsov <vkuznets@redhat.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/x86.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)