[PATCH] KVM: LAPIC: Narrow down the timer fastpath to tscdeadline timer

[PATCH] KVM: LAPIC: Narrow down the timer fastpath to tscdeadline timer

[Wanpeng Li]

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.

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;
+}
+
+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;
 }
 
-- 
2.25.1

Re: [PATCH] KVM: LAPIC: Narrow down the timer fastpath to tscdeadline timer

[Sean Christopherson]

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;
}

Re: [PATCH] KVM: LAPIC: Narrow down the timer fastpath to tscdeadline timer

[Paolo Bonzini]

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