On Fri, May 06, 2022, Wanpeng Li wrote:
> From: Wanpeng Li <wanpengli@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> ---
> 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;
}
