| Message ID | 1599620119-12971-1-git-send-email-wanpengli@tencent.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] KVM: SVM: Get rid of handle_fastpath_set_msr_irqoff() | expand |
Wanpeng Li <kernellwp@gmail.com> writes: > From: Wanpeng Li <wanpengli@tencent.com> > > Moving svm_complete_interrupts() into svm_vcpu_run() which can align VMX > and SVM with respect to completing interrupts. > > Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com> > Cc: Paul K. <kronenpj@kronenpj.dyndns.org> > Signed-off-by: Wanpeng Li <wanpengli@tencent.com> > --- > arch/x86/kvm/svm/svm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index c61bc3b..74bcf0a 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -2938,8 +2938,6 @@ static int handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath) > if (npt_enabled) > vcpu->arch.cr3 = svm->vmcb->save.cr3; > > - svm_complete_interrupts(svm); > - > if (is_guest_mode(vcpu)) { > int vmexit; > > @@ -3530,6 +3528,8 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu) > SVM_EXIT_EXCP_BASE + MC_VECTOR)) > svm_handle_mce(svm); > > + svm_complete_interrupts(svm); > + > vmcb_mark_all_clean(svm->vmcb); > return exit_fastpath; > } This seems to be the right thing to do, however, the amount of code between kvm_x86_ops.run() and kvm_x86_ops.handle_exit() is non-trivial, hope it won't blow up in testing... Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> One more thing: VMX version does vmx_complete_interrupts(vmx); if (is_guest_mode(vcpu)) return EXIT_FASTPATH_NONE; and on SVM we analyze is_guest_mode() inside svm_exit_handlers_fastpath() - should we also change that for conformity?
On Wed, 9 Sep 2020 at 16:36, Vitaly Kuznetsov <vkuznets@redhat.com> wrote: > > Wanpeng Li <kernellwp@gmail.com> writes: > > > From: Wanpeng Li <wanpengli@tencent.com> > > > > Moving svm_complete_interrupts() into svm_vcpu_run() which can align VMX > > and SVM with respect to completing interrupts. > > > > Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com> > > Cc: Paul K. <kronenpj@kronenpj.dyndns.org> > > Signed-off-by: Wanpeng Li <wanpengli@tencent.com> > > --- > > arch/x86/kvm/svm/svm.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > > index c61bc3b..74bcf0a 100644 > > --- a/arch/x86/kvm/svm/svm.c > > +++ b/arch/x86/kvm/svm/svm.c > > @@ -2938,8 +2938,6 @@ static int handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath) > > if (npt_enabled) > > vcpu->arch.cr3 = svm->vmcb->save.cr3; > > > > - svm_complete_interrupts(svm); > > - > > if (is_guest_mode(vcpu)) { > > int vmexit; > > > > @@ -3530,6 +3528,8 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu) > > SVM_EXIT_EXCP_BASE + MC_VECTOR)) > > svm_handle_mce(svm); > > > > + svm_complete_interrupts(svm); > > + > > vmcb_mark_all_clean(svm->vmcb); > > return exit_fastpath; > > } > > This seems to be the right thing to do, however, the amount of code > between kvm_x86_ops.run() and kvm_x86_ops.handle_exit() is non-trivial, > hope it won't blow up in testing... > > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> > > One more thing: > > VMX version does > > vmx_complete_interrupts(vmx); > if (is_guest_mode(vcpu)) > return EXIT_FASTPATH_NONE; > > and on SVM we analyze is_guest_mode() inside > svm_exit_handlers_fastpath() - should we also change that for > conformity? Agreed, will do in v2. Wanpeng
On 09/09/20 10:47, Wanpeng Li wrote: >> One more thing: >> >> VMX version does >> >> vmx_complete_interrupts(vmx); >> if (is_guest_mode(vcpu)) >> return EXIT_FASTPATH_NONE; >> >> and on SVM we analyze is_guest_mode() inside >> svm_exit_handlers_fastpath() - should we also change that for >> conformity? > > Agreed, will do in v2. Please just send an incremental patch. Thanks! Paolo
On Sat, 12 Sep 2020 at 14:20, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 09/09/20 10:47, Wanpeng Li wrote: > >> One more thing: > >> > >> VMX version does > >> > >> vmx_complete_interrupts(vmx); > >> if (is_guest_mode(vcpu)) > >> return EXIT_FASTPATH_NONE; > >> > >> and on SVM we analyze is_guest_mode() inside > >> svm_exit_handlers_fastpath() - should we also change that for > >> conformity? > > > > Agreed, will do in v2. > > Please just send an incremental patch. Thanks! Just sent out a patch to do it. :) Wanpeng
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index c61bc3b..74bcf0a 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -2938,8 +2938,6 @@ static int handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath) if (npt_enabled) vcpu->arch.cr3 = svm->vmcb->save.cr3; - svm_complete_interrupts(svm); - if (is_guest_mode(vcpu)) { int vmexit; @@ -3530,6 +3528,8 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu) SVM_EXIT_EXCP_BASE + MC_VECTOR)) svm_handle_mce(svm); + svm_complete_interrupts(svm); + vmcb_mark_all_clean(svm->vmcb); return exit_fastpath; }