diff mbox series

[v2,5/5] KVM: VMX: Handle preemption timer fastpath

Message ID 1587632507-18997-6-git-send-email-wanpengli@tencent.com (mailing list archive)
State New, archived
Headers show
Series KVM: VMX: Tscdeadline timer emulation fastpath | expand

Commit Message

Wanpeng Li April 23, 2020, 9:01 a.m. UTC
From: Wanpeng Li <wanpengli@tencent.com>

This patch implements handle preemption timer fastpath, after timer fire 
due to VMX-preemption timer counts down to zero, handle it as soon as 
possible and vmentry immediately without checking various kvm stuff when 
possible.

Testing on SKX Server.

cyclictest in guest(w/o mwait exposed, adaptive advance lapic timer is default -1):

5632.75ns -> 4559.25ns, 19%

kvm-unit-test/vmexit.flat:

w/o APICv, w/o advance timer:
tscdeadline_immed: 4780.75 -> 3851    19.4%
tscdeadline:       7474    -> 6528.5  12.7%

w/o APICv, w/ adaptive advance timer default -1:
tscdeadline_immed: 4845.75 -> 3930.5  18.9%
tscdeadline:       6048    -> 5871.75    3%

w/ APICv, w/o avanced timer:
tscdeadline_immed: 2919    -> 2467.75 15.5%
tscdeadline:       5661.75 -> 5188.25  8.4%

w/ APICv, w/ adaptive advance timer default -1:
tscdeadline_immed: 3018.5  -> 2561    15.2%
tscdeadline:       4663.75 -> 4537     2.7%

Tested-by: Haiwei Li <lihaiwei@tencent.com>
Cc: Haiwei Li <lihaiwei@tencent.com>
Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
---
 arch/x86/kvm/lapic.c   | 19 +++++++++++++++++++
 arch/x86/kvm/lapic.h   |  1 +
 arch/x86/kvm/vmx/vmx.c | 22 ++++++++++++++++++++++
 3 files changed, 42 insertions(+)

Comments

Paolo Bonzini April 23, 2020, 9:40 a.m. UTC | #1
On 23/04/20 11:01, Wanpeng Li wrote:
> +bool kvm_lapic_expired_hv_timer_fast(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_lapic *apic = vcpu->arch.apic;
> +	struct kvm_timer *ktimer = &apic->lapic_timer;
> +
> +	if (!apic_lvtt_tscdeadline(apic) ||
> +		!ktimer->hv_timer_in_use ||
> +		atomic_read(&ktimer->pending))
> +		return 0;
> +
> +	WARN_ON(swait_active(&vcpu->wq));
> +	cancel_hv_timer(apic);
> +
> +	ktimer->expired_tscdeadline = ktimer->tscdeadline;
> +	kvm_inject_apic_timer_irqs_fast(vcpu);
> +
> +	return 1;
> +}
> +EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer_fast);

Please re-evaluate if this is needed (or which parts are needed) after
cleaning up patch 4.  Anyway again---this is already better, I don't
like the duplicated code but at least I can understand what's going on.

Paolo
Wanpeng Li April 23, 2020, 9:56 a.m. UTC | #2
On Thu, 23 Apr 2020 at 17:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 23/04/20 11:01, Wanpeng Li wrote:
> > +bool kvm_lapic_expired_hv_timer_fast(struct kvm_vcpu *vcpu)
> > +{
> > +     struct kvm_lapic *apic = vcpu->arch.apic;
> > +     struct kvm_timer *ktimer = &apic->lapic_timer;
> > +
> > +     if (!apic_lvtt_tscdeadline(apic) ||
> > +             !ktimer->hv_timer_in_use ||
> > +             atomic_read(&ktimer->pending))
> > +             return 0;
> > +
> > +     WARN_ON(swait_active(&vcpu->wq));
> > +     cancel_hv_timer(apic);
> > +
> > +     ktimer->expired_tscdeadline = ktimer->tscdeadline;
> > +     kvm_inject_apic_timer_irqs_fast(vcpu);
> > +
> > +     return 1;
> > +}
> > +EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer_fast);
>
> Please re-evaluate if this is needed (or which parts are needed) after
> cleaning up patch 4.  Anyway again---this is already better, I don't
> like the duplicated code but at least I can understand what's going on.

Except the apic_lvtt_tscdeadline(apic) check, others are duplicated,
what do you think about apic_lvtt_tscdeadline(apic) check?

    Wanpeng
Paolo Bonzini April 23, 2020, 10:29 a.m. UTC | #3
On 23/04/20 11:56, Wanpeng Li wrote:
>> Please re-evaluate if this is needed (or which parts are needed) after
>> cleaning up patch 4.  Anyway again---this is already better, I don't
>> like the duplicated code but at least I can understand what's going on.
> Except the apic_lvtt_tscdeadline(apic) check, others are duplicated,
> what do you think about apic_lvtt_tscdeadline(apic) check?

We have to take a look again after you clean up patch 4.  My hope is to
reuse the slowpath code as much as possible, by introducing some
optimizations here and there.

Paolo
Wanpeng Li April 24, 2020, 6:38 a.m. UTC | #4
On Thu, 23 Apr 2020 at 18:29, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 23/04/20 11:56, Wanpeng Li wrote:
> >> Please re-evaluate if this is needed (or which parts are needed) after
> >> cleaning up patch 4.  Anyway again---this is already better, I don't
> >> like the duplicated code but at least I can understand what's going on.
> > Except the apic_lvtt_tscdeadline(apic) check, others are duplicated,
> > what do you think about apic_lvtt_tscdeadline(apic) check?
>
> We have to take a look again after you clean up patch 4.  My hope is to
> reuse the slowpath code as much as possible, by introducing some
> optimizations here and there.

I found we are not need to move the if (vcpu->arch.apicv_active) from
__apic_accept_irq() to a separate function if I understand you
correctly. Please see patch v3 3/5. In addition, I observe
kvm-unit-tests #UD etc if check need_cancel_enter_guest() after the
generic fastpath handler, I didn't dig too much, just move it before
the generic fastpath handler for safe in patch v3 2/5.

    Wanpeng
kernel test robot April 26, 2020, 7:38 a.m. UTC | #5
Hi Wanpeng,

I love your patch! Yet something to improve:

[auto build test ERROR on kvm/linux-next]
[also build test ERROR on next-20200424]
[cannot apply to tip/auto-latest linus/master linux/master v5.7-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Wanpeng-Li/KVM-VMX-Tscdeadline-timer-emulation-fastpath/20200426-132300
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: i386-randconfig-d001-20200426 (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> arch/x86/kvm/vmx/vmx.c:6572:13: error: 'vmx_cancel_hv_timer' declared 'static' but never defined [-Werror=unused-function]
    static void vmx_cancel_hv_timer(struct kvm_vcpu *vcpu);
                ^~~~~~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors

vim +6572 arch/x86/kvm/vmx/vmx.c

  6571	
> 6572	static void vmx_cancel_hv_timer(struct kvm_vcpu *vcpu);
  6573	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index d652bd9..2741931 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1899,6 +1899,25 @@  void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu)
 EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer);
 
 static void kvm_inject_apic_timer_irqs_fast(struct kvm_vcpu *vcpu);
+bool kvm_lapic_expired_hv_timer_fast(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->arch.apic;
+	struct kvm_timer *ktimer = &apic->lapic_timer;
+
+	if (!apic_lvtt_tscdeadline(apic) ||
+		!ktimer->hv_timer_in_use ||
+		atomic_read(&ktimer->pending))
+		return 0;
+
+	WARN_ON(swait_active(&vcpu->wq));
+	cancel_hv_timer(apic);
+
+	ktimer->expired_tscdeadline = ktimer->tscdeadline;
+	kvm_inject_apic_timer_irqs_fast(vcpu);
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer_fast);
 
 void kvm_lapic_switch_to_hv_timer(struct kvm_vcpu *vcpu)
 {
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 5ef1364..1b5abd8 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -252,6 +252,7 @@  bool kvm_lapic_hv_timer_in_use(struct kvm_vcpu *vcpu);
 void kvm_lapic_restart_hv_timer(struct kvm_vcpu *vcpu);
 bool kvm_can_post_timer_interrupt(struct kvm_vcpu *vcpu);
 int kvm_set_lapic_tscdeadline_msr_fast(struct kvm_vcpu *vcpu, u64 data);
+bool kvm_lapic_expired_hv_timer_fast(struct kvm_vcpu *vcpu);
 
 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 2613e58..527d1c1 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6569,12 +6569,34 @@  void vmx_update_host_rsp(struct vcpu_vmx *vmx, unsigned long host_rsp)
 	}
 }
 
+static void vmx_cancel_hv_timer(struct kvm_vcpu *vcpu);
+
+static enum exit_fastpath_completion handle_fastpath_preemption_timer(struct kvm_vcpu *vcpu)
+{
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
+
+	if (kvm_need_cancel_enter_guest(vcpu) ||
+		kvm_event_needs_reinjection(vcpu))
+		return EXIT_FASTPATH_NONE;
+
+	if (!vmx->req_immediate_exit &&
+		!unlikely(vmx->loaded_vmcs->hv_timer_soft_disabled) &&
+		kvm_lapic_expired_hv_timer_fast(vcpu)) {
+		trace_kvm_exit(EXIT_REASON_PREEMPTION_TIMER, vcpu, KVM_ISA_VMX);
+		return EXIT_FASTPATH_CONT_RUN;
+	}
+
+	return EXIT_FASTPATH_NONE;
+}
+
 static enum exit_fastpath_completion vmx_exit_handlers_fastpath(struct kvm_vcpu *vcpu)
 {
 	if (!is_guest_mode(vcpu)) {
 		switch (to_vmx(vcpu)->exit_reason) {
 		case EXIT_REASON_MSR_WRITE:
 			return handle_fastpath_set_msr_irqoff(vcpu);
+		case EXIT_REASON_PREEMPTION_TIMER:
+			return handle_fastpath_preemption_timer(vcpu);
 		default:
 			return EXIT_FASTPATH_NONE;
 		}