From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 1/5] Implement #NMI exiting for nested SVM Date: Fri, 18 Sep 2009 15:33:34 +0200 Message-ID: <4AB38C2E.307@siemens.com> References: <1253278832-31803-1-git-send-email-agraf@suse.de> <1253278832-31803-2-git-send-email-agraf@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Alexander Graf Return-path: Received: from goliath.siemens.de ([192.35.17.28]:22539 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750911AbZIRNdd (ORCPT ); Fri, 18 Sep 2009 09:33:33 -0400 In-Reply-To: <1253278832-31803-2-git-send-email-agraf@suse.de> Sender: kvm-owner@vger.kernel.org List-ID: Alexander Graf wrote: > When injecting an NMI to the l1 guest while it was running the l2 guest, we > didn't #VMEXIT but just injected the NMI to the l2 guest. > > Let's be closer to real hardware and #VMEXIT if we're supposed to do so. > > Signed-off-by: Alexander Graf > --- > arch/x86/kvm/svm.c | 38 ++++++++++++++++++++++++++++++++------ > 1 files changed, 32 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 9a4daca..f12a669 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -1375,6 +1375,21 @@ static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr, > return nested_svm_exit_handled(svm); > } > > +static inline int nested_svm_nmi(struct vcpu_svm *svm) > +{ > + if (!is_nested(svm)) > + return 0; > + > + svm->vmcb->control.exit_code = SVM_EXIT_NMI; > + > + if (nested_svm_exit_handled(svm)) { > + nsvm_printk("VMexit -> NMI\n"); > + return 1; > + } > + > + return 0; > +} > + > static inline int nested_svm_intr(struct vcpu_svm *svm) > { > if (!is_nested(svm)) > @@ -2462,7 +2477,9 @@ static int svm_nmi_allowed(struct kvm_vcpu *vcpu) > struct vcpu_svm *svm = to_svm(vcpu); > struct vmcb *vmcb = svm->vmcb; > return !(vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) && > - !(svm->vcpu.arch.hflags & HF_NMI_MASK); > + !(svm->vcpu.arch.hflags & HF_NMI_MASK) && > + gif_set(svm) && ^^^^^^^^^^^^ I'm not claiming to be up-to-date with the SVM code around NMI injection, but this addition irritates me. Can you explain why I don't have to worry that a cleared IF could now defer NMI injections for L1 guests? > + !is_nested(svm); > } > > static int svm_interrupt_allowed(struct kvm_vcpu *vcpu) > @@ -2488,22 +2505,31 @@ static void enable_irq_window(struct kvm_vcpu *vcpu) > struct vcpu_svm *svm = to_svm(vcpu); > nsvm_printk("Trying to open IRQ window\n"); > > - nested_svm_intr(svm); > + if (nested_svm_intr(svm)) > + return; > > /* In case GIF=0 we can't rely on the CPU to tell us when > * GIF becomes 1, because that's a separate STGI/VMRUN intercept. > * The next time we get that intercept, this function will be > * called again though and we'll get the vintr intercept. */ > - if (gif_set(svm)) { > - svm_set_vintr(svm); > - svm_inject_irq(svm, 0x0); > - } > + if (!gif_set(svm)) > + return; > + > + svm_set_vintr(svm); > + svm_inject_irq(svm, 0x0); The last change is pure refactoring that should not belong into this patch, should it? > } > > static void enable_nmi_window(struct kvm_vcpu *vcpu) > { > struct vcpu_svm *svm = to_svm(vcpu); > > + if (nested_svm_nmi(svm)) > + return; > + > + /* NMI is deferred until GIF == 1. Setting GIF will cause a #VMEXIT */ > + if (!gif_set(svm)) > + return; > + The second half is an unrelated optimization? Then please file a separate patch. > if ((svm->vcpu.arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) > == HF_NMI_MASK) > return; /* IRET will cause a vm exit */ Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux