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