From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 2/2] KVM: SVM: Make stepping out of NMI handlers more robust Date: Wed, 17 Feb 2010 20:16:45 +0100 Message-ID: <4B7C409D.1090703@siemens.com> References: <8cb458e40c9d594f27975b10bb24ecb9a90d8102.1266257833.git.jan.kiszka@siemens.com> <20100216080436.GX2995@redhat.com> <4B7A6210.5060606@siemens.com> <20100216093416.GA2995@redhat.com> <4B7A692B.8050207@siemens.com> <20100216094954.GB2995@redhat.com> <4B7A6E03.2020606@siemens.com> <20100216100858.GF2995@redhat.com> <20100217134916.GA14767@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , Marcelo Tosatti , kvm To: Gleb Natapov Return-path: Received: from goliath.siemens.de ([192.35.17.28]:20010 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754090Ab0BQTRF (ORCPT ); Wed, 17 Feb 2010 14:17:05 -0500 In-Reply-To: <20100217134916.GA14767@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Gleb Natapov wrote: > On Tue, Feb 16, 2010 at 12:08:58PM +0200, Gleb Natapov wrote: >>>>> Besides this, proper #DB forwarding to the guest was missing. >>>> During NMI injection? How to reproduce? >>> Inject, e.g., an NMI over code with TF set. A bit harder is placing a >>> guest HW breakpoint at the spot the NMI handler returns to. >>> >> Will try to reproduce. >> > How can I make gdb to run debugged process with TF set? Is this patch > fixes it: > > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 52f78dd..b85b200 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -109,6 +109,7 @@ struct vcpu_svm { > struct nested_state nested; > > bool nmi_singlestep; > + bool nmi_singlestep_tf; > }; > > /* enable NPT for AMD64 and X86 with PAE */ > @@ -1221,9 +1222,14 @@ static int db_interception(struct vcpu_svm *svm) > > if (svm->nmi_singlestep) { > svm->nmi_singlestep = false; > - if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_SINGLESTEP)) > + if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_SINGLESTEP)) { > svm->vmcb->save.rflags &= > ~(X86_EFLAGS_TF | X86_EFLAGS_RF); > + if (svm->nmi_singlestep_tf) { > + svm->vmcb->save.rflags |= X86_EFLAGS_TF; > + kvm_queue_exception(&svm->vcpu, DB_VECTOR); > + } > + } > update_db_intercept(&svm->vcpu); > } > > @@ -2586,6 +2592,7 @@ static void enable_nmi_window(struct kvm_vcpu *vcpu) > possible problem (IRET or exception injection or interrupt > shadow) */ > svm->nmi_singlestep = true; > + svm->nmi_singlestep_tf = (svm->vmcb->save.rflags | X86_EFLAGS_TF); > svm->vmcb->save.rflags |= (X86_EFLAGS_TF | X86_EFLAGS_RF); > update_db_intercept(vcpu); > } That's closer. However, I've a version here that restores TF&RF only if you did not execute an IRET but stepped over the shadow (which is still not correct either, e.g. when stepping popf). I will break up my patch into parts that fix the issues separately so that we can decide what to merge. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux