| Message ID | 1651830457-11284-1-git-send-email-wanpengli@tencent.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: LAPIC: Narrow down the timer fastpath to tscdeadline timer | expand |
On Fri, May 06, 2022, Wanpeng Li wrote: > From: Wanpeng Li <wanpengli@tencent.com> > > The original timer fastpath is developed for tscdeadline timer, however, > the apic timer periodic/oneshot mode which is emulated by vmx preemption > timer goes to preemption timer vmexit fastpath quietly, let's leave the > complex recompute periodic timer's target expiration and restart apic > timer to the slowpath vm-exit. Narrow down the timer fastpath to tscdeadline > timer mode. Why? I get that the original intention was only to handle deadline mode, but that doesn't mean using it for other modes is inherently flawed/problematic. KVM also uses a fastpath for starting the deadline timer on MSR write, i.e. KVM eats the cost of starting the timer in the fastpath no matter what, and advance_periodic_target_expiration() isn't _that_ complex. > Signed-off-by: Wanpeng Li <wanpengli@tencent.com> > --- > arch/x86/kvm/lapic.c | 6 ++++++ > arch/x86/kvm/lapic.h | 1 + > arch/x86/kvm/vmx/vmx.c | 14 +++++++++++--- > 3 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 137c3a2f5180..3e6cb2bf56dc 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -2459,6 +2459,12 @@ static bool lapic_is_periodic(struct kvm_lapic *apic) > return apic_lvtt_period(apic); > } > > +bool lapic_is_tscdeadline(struct kvm_lapic *apic) > +{ > + return apic_lvtt_tscdeadline(apic); > +} > +EXPORT_SYMBOL_GPL(lapic_is_tscdeadline); > + > int apic_has_pending_timer(struct kvm_vcpu *vcpu) > { > struct kvm_lapic *apic = vcpu->arch.apic; > diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h > index 4e4f8a22754f..6e1b2f349237 100644 > --- a/arch/x86/kvm/lapic.h > +++ b/arch/x86/kvm/lapic.h > @@ -241,6 +241,7 @@ void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu); > bool kvm_lapic_hv_timer_in_use(struct kvm_vcpu *vcpu); > void kvm_lapic_restart_hv_timer(struct kvm_vcpu *vcpu); > bool kvm_can_use_hv_timer(struct kvm_vcpu *vcpu); > +bool lapic_is_tscdeadline(struct kvm_lapic *apic); > > static inline enum lapic_mode kvm_apic_mode(u64 apic_base) > { > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index bb09fc9a7e55..2a8f4253df35 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -5713,22 +5713,30 @@ static int handle_pml_full(struct kvm_vcpu *vcpu) > return 1; > } > > -static fastpath_t handle_fastpath_preemption_timer(struct kvm_vcpu *vcpu) > +static bool __handle_preemption_timer(struct kvm_vcpu *vcpu) > { > struct vcpu_vmx *vmx = to_vmx(vcpu); > > if (!vmx->req_immediate_exit && > !unlikely(vmx->loaded_vmcs->hv_timer_soft_disabled)) { > kvm_lapic_expired_hv_timer(vcpu); > - return EXIT_FASTPATH_REENTER_GUEST; > + return true; > } > > + return false; It's a bit odd for the non-fastpath case, but I'd prefer to return fastpath_t instead of a bool from the inner helper, e.g. static fastpath_t __handle_preemption_timer(struct kvm_vcpu *vcpu) { struct vcpu_vmx *vmx = to_vmx(vcpu); if (!vmx->req_immediate_exit && !unlikely(vmx->loaded_vmcs->hv_timer_soft_disabled)) { kvm_lapic_expired_hv_timer(vcpu); return EXIT_FASTPATH_REENTER_GUEST; } return EXIT_FASTPATH_NONE; } static fastpath_t handle_fastpath_preemption_timer(struct kvm_vcpu *vcpu) { if (lapic_is_tscdeadline(vcpu->arch.apic) return __handle_preemption_timer(vcpu)) return EXIT_FASTPATH_NONE; }
On 5/10/22 16:15, Sean Christopherson wrote: >> >> -static fastpath_t handle_fastpath_preemption_timer(struct kvm_vcpu *vcpu) >> +static bool __handle_preemption_timer(struct kvm_vcpu *vcpu) >> { >> struct vcpu_vmx *vmx = to_vmx(vcpu); >> >> if (!vmx->req_immediate_exit && >> !unlikely(vmx->loaded_vmcs->hv_timer_soft_disabled)) { >> kvm_lapic_expired_hv_timer(vcpu); >> - return EXIT_FASTPATH_REENTER_GUEST; >> + return true; >> } >> >> + return false; > It's a bit odd for the non-fastpath case, but I'd prefer to return fastpath_t > instead of a bool from the inner helper, e.g. > Yeah, enum > bool almost always (or negative errno). But I also agree that using the fast path for periodic or oneshot timers is not inherently a bad idea. Paolo
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 137c3a2f5180..3e6cb2bf56dc 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -2459,6 +2459,12 @@ static bool lapic_is_periodic(struct kvm_lapic *apic) return apic_lvtt_period(apic); } +bool lapic_is_tscdeadline(struct kvm_lapic *apic) +{ + return apic_lvtt_tscdeadline(apic); +} +EXPORT_SYMBOL_GPL(lapic_is_tscdeadline); + int apic_has_pending_timer(struct kvm_vcpu *vcpu) { struct kvm_lapic *apic = vcpu->arch.apic; diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h index 4e4f8a22754f..6e1b2f349237 100644 --- a/arch/x86/kvm/lapic.h +++ b/arch/x86/kvm/lapic.h @@ -241,6 +241,7 @@ void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu); bool kvm_lapic_hv_timer_in_use(struct kvm_vcpu *vcpu); void kvm_lapic_restart_hv_timer(struct kvm_vcpu *vcpu); bool kvm_can_use_hv_timer(struct kvm_vcpu *vcpu); +bool lapic_is_tscdeadline(struct kvm_lapic *apic); static inline enum lapic_mode kvm_apic_mode(u64 apic_base) { diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index bb09fc9a7e55..2a8f4253df35 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -5713,22 +5713,30 @@ static int handle_pml_full(struct kvm_vcpu *vcpu) return 1; } -static fastpath_t handle_fastpath_preemption_timer(struct kvm_vcpu *vcpu) +static bool __handle_preemption_timer(struct kvm_vcpu *vcpu) { struct vcpu_vmx *vmx = to_vmx(vcpu); if (!vmx->req_immediate_exit && !unlikely(vmx->loaded_vmcs->hv_timer_soft_disabled)) { kvm_lapic_expired_hv_timer(vcpu); - return EXIT_FASTPATH_REENTER_GUEST; + return true; } + return false; +} + +static fastpath_t handle_fastpath_preemption_timer(struct kvm_vcpu *vcpu) +{ + if (lapic_is_tscdeadline(vcpu->arch.apic) && __handle_preemption_timer(vcpu)) + return EXIT_FASTPATH_REENTER_GUEST; + return EXIT_FASTPATH_NONE; } static int handle_preemption_timer(struct kvm_vcpu *vcpu) { - handle_fastpath_preemption_timer(vcpu); + __handle_preemption_timer(vcpu); return 1; }