From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH 5/6] KVM: x86: Preserve injected TF across emulation Date: Tue, 23 Feb 2010 13:03:01 +0200 Message-ID: <20100223110301.GJ29041@redhat.com> References: <20100223100022.GD29041@redhat.com> <4B83AA39.30403@siemens.com> <20100223103103.GG29041@redhat.com> <4B83B0B8.2090308@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , Marcelo Tosatti , kvm To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:3760 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751903Ab0BWLDE (ORCPT ); Tue, 23 Feb 2010 06:03:04 -0500 Content-Disposition: inline In-Reply-To: <4B83B0B8.2090308@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Feb 23, 2010 at 11:40:56AM +0100, Jan Kiszka wrote: > Gleb Natapov wrote: > > On Tue, Feb 23, 2010 at 11:13:13AM +0100, Jan Kiszka wrote: > >> Gleb Natapov wrote: > >>> On Mon, Feb 22, 2010 at 06:51:22PM +0100, Jan Kiszka wrote: > >>>> Call directly into the vendor services for getting/setting rflags in > >>>> emulate_instruction to ensure injected TF survives the emulation. > >>>> > >>>> Signed-off-by: Jan Kiszka > >>>> --- > >>>> arch/x86/kvm/x86.c | 4 ++-- > >>>> 1 files changed, 2 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > >>>> index e2e03a4..19e8b28 100644 > >>>> --- a/arch/x86/kvm/x86.c > >>>> +++ b/arch/x86/kvm/x86.c > >>>> @@ -3468,7 +3468,7 @@ int emulate_instruction(struct kvm_vcpu *vcpu, > >>>> kvm_x86_ops->get_cs_db_l_bits(vcpu, &cs_db, &cs_l); > >>>> > >>>> vcpu->arch.emulate_ctxt.vcpu = vcpu; > >>>> - vcpu->arch.emulate_ctxt.eflags = kvm_get_rflags(vcpu); > >>>> + vcpu->arch.emulate_ctxt.eflags = kvm_x86_ops->get_rflags(vcpu); > >>> So now emulator runs with injected TF? Hmm, then may be emulator should > >>> inject DB when appropriate and caller of emulate_instruction() should > >>> emulate DB intercept if external debugging is going on? > >> That is what patch 6 aims at, both for external as well as > >> guest-internal debugging. > >> > > It does at the wrong level. It tries to do it above emulator, but only emulator > > knows what is the sate of instruction emulation and when emulation is completed. > > If at this point TF is set it inject DB trap. The code above checks if > > DB intercept is enabled when DB is injected and calls usual DB intercept > > path. > > Sorry, can't follow, your description does not match the above code for me. > > This patch just ensures that we do not loose TF across emulation and, by > itself, already improves guest debugging: it allows to step over > emulated blocks with one instruction offset. And it has no side effects > as the emulator does not evaluate TF yet (before patch 6). > My comment was not about this patch, but about patch 6. This is the job of emulator to emulate CPU and inject exceptions/traps, not the code above it. -- Gleb.