Re: [PATCH 5/6] KVM: x86: Preserve injected TF across emulation

[Gleb Natapov <gleb@redhat.com>]

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 <jan.kiszka@siemens.com>
> >>>> ---
> >>>>  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.