Gleb Natapov wrote: > If NMI is received during handling of another NMI it should be injected > immediately after IRET from previous NMI handler, but SVM intercept IRET > before instruction execution so we can't inject pending NMI at this > point and there is not way to request exit when NMI window opens. This > patch fix SVM code to open NMI window after IRET by single stepping over > IRET instruction. > > Signed-off-by: Gleb Natapov <[email protected]> > --- > arch/x86/include/asm/kvm_host.h | 2 + > arch/x86/kvm/svm.c | 62 +++++++++++++++++++++++++++++++------- > 2 files changed, 52 insertions(+), 12 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index fea0429..bcd0857 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -358,6 +358,7 @@ struct kvm_vcpu_arch { > unsigned int time_offset; > struct page *time_page; > > + bool singlestep; /* guest is single stepped by KVM */ > bool nmi_pending; > bool nmi_injected; > > @@ -772,6 +773,7 @@ enum { > #define HF_HIF_MASK (1 << 1) > #define HF_VINTR_MASK (1 << 2) > #define HF_NMI_MASK (1 << 3) > +#define HF_IRET_MASK (1 << 4) > > /* > * Hardware virtualization extension instructions may fault if a > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index d5173a2..bf10991 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -933,15 +933,16 @@ static void svm_set_segment(struct kvm_vcpu *vcpu, > > } > > -static int svm_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug > *dbg) > +static void update_db_intercept(struct kvm_vcpu *vcpu) > { > - int old_debug = vcpu->guest_debug; > struct vcpu_svm *svm = to_svm(vcpu); > > - vcpu->guest_debug = dbg->control; > - > svm->vmcb->control.intercept_exceptions &= > ~((1 << DB_VECTOR) | (1 << BP_VECTOR)); > + > + if (vcpu->arch.singlestep) > + svm->vmcb->control.intercept_exceptions |= (1 << DB_VECTOR); > + > if (vcpu->guest_debug & KVM_GUESTDBG_ENABLE) { > if (vcpu->guest_debug & > (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) > @@ -952,6 +953,16 @@ static int svm_guest_debug(struct kvm_vcpu *vcpu, struct > kvm_guest_debug *dbg) > 1 << BP_VECTOR; > } else > vcpu->guest_debug = 0; > +} > + > +static int svm_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug > *dbg) > +{ > + int old_debug = vcpu->guest_debug; > + struct vcpu_svm *svm = to_svm(vcpu); > + > + vcpu->guest_debug = dbg->control; > + > + update_db_intercept(vcpu); > > if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) > svm->vmcb->save.dr7 = dbg->arch.debugreg[7]; > @@ -1101,14 +1112,30 @@ static int pf_interception(struct vcpu_svm *svm, > struct kvm_run *kvm_run) > static int db_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run) > { > if (!(svm->vcpu.guest_debug & > - (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP))) { > + (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) && > + !svm->vcpu.arch.singlestep) { > kvm_queue_exception(&svm->vcpu, DB_VECTOR); > return 1; > } > - kvm_run->exit_reason = KVM_EXIT_DEBUG; > - kvm_run->debug.arch.pc = svm->vmcb->save.cs.base + svm->vmcb->save.rip; > - kvm_run->debug.arch.exception = DB_VECTOR; > - return 0; > + > + if (svm->vcpu.arch.singlestep) { > + svm->vcpu.arch.singlestep = false; > + if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_SINGLESTEP)) > + svm->vmcb->save.rflags &= > + ~(X86_EFLAGS_TF | X86_EFLAGS_RF); > + update_db_intercept(to_svm(svm)); > + } > + > + if (svm->vcpu.guest_debug & > + (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)){ > + kvm_run->exit_reason = KVM_EXIT_DEBUG; > + kvm_run->debug.arch.pc = > + svm->vmcb->save.cs.base + svm->vmcb->save.rip; > + kvm_run->debug.arch.exception = DB_VECTOR; > + return 0; > + } > + > + return 1; > } > > static int bp_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run) > @@ -1855,7 +1882,7 @@ static int iret_interception(struct vcpu_svm *svm, > struct kvm_run *kvm_run) > { > ++svm->vcpu.stat.nmi_window_exits; > svm->vmcb->control.intercept &= ~(1UL << INTERCEPT_IRET); > - svm->vcpu.arch.hflags &= ~HF_NMI_MASK; > + svm->vcpu.arch.hflags |= HF_IRET_MASK; > return 1; > } > > @@ -2331,8 +2358,16 @@ static void enable_nmi_window(struct kvm_vcpu *vcpu) > { > struct vcpu_svm *svm = to_svm(vcpu); > > - if (svm->vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) > - enable_irq_window(vcpu); > + if ((svm->vcpu.arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) > + == HF_NMI_MASK) > + return; /* IRET will cause a vm exit */ > + > + /* Something prevents NMI from been injected. Single step over > + possible problem (IRET or exception injection or interrupt > + shadow) */ > + vcpu->arch.singlestep = true; > + svm->vmcb->save.rflags |= (X86_EFLAGS_TF | X86_EFLAGS_RF);
Can you single-step like this out of an IRQ handler? I mean, IRET will restore the flags from the stack, and those settings should be overwritten. Or am I missing something? > + update_db_intercept(vcpu); > } > > static int svm_set_tss_addr(struct kvm *kvm, unsigned int addr) > @@ -2375,6 +2410,9 @@ static void svm_complete_interrupts(struct vcpu_svm > *svm) > int type; > u32 exitintinfo = svm->vmcb->control.exit_int_info; > > + if (svm->vcpu.arch.hflags & HF_IRET_MASK) > + svm->vcpu.arch.hflags &= ~(HF_NMI_MASK | HF_IRET_MASK); > + > svm->vcpu.arch.nmi_injected = false; > kvm_clear_exception_queue(&svm->vcpu); > kvm_clear_interrupt_queue(&svm->vcpu); Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
