----- Gleb Natapov <gleb@xxxxxxxxxx> wrote:
> On Tue, Jul 30, 2013 at 05:11:36PM +0200, Paolo Bonzini wrote:
> > This lets debugging work better during emulation of invalid
> > guest state.
> >
> > This time the check is done after emulation, but before writeback
> > of the flags; we need to check the flags *before* execution of the
> > instruction, we cannot check singlestep_rip because the CS base may
> > have already been modified.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > ---
> > arch/x86/kvm/x86.c | 39 ++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 1368cf5..9805cfd 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -4971,6 +4971,41 @@ static int kvm_vcpu_check_hw_bp(unsigned long addr, u32 type, u32 dr7,
> > return dr6;
> > }
> >
> > +static void kvm_vcpu_check_singlestep(struct kvm_vcpu *vcpu, int *r)
> > +{
> > + struct kvm_run *kvm_run = vcpu->run;
> > +
> > + /*
> > + * Use the "raw" value to see if TF was passed to the processor.
> > + * Note that the new value of the flags has not been saved yet.
> > + *
> > + * This is correct even for TF set by the guest, because "the
> > + * processor will not generate this exception after the instruction
> > + * that sets the TF flag".
> > + */
> > + unsigned long rflags = kvm_x86_ops->get_rflags(vcpu);
> > +
> > + if (unlikely(rflags & X86_EFLAGS_TF)) {
> > + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
> > + kvm_run->debug.arch.dr6 = DR6_BS | DR6_FIXED_1;
> > + kvm_run->debug.arch.pc = vcpu->arch.singlestep_rip;
> > + kvm_run->debug.arch.exception = DB_VECTOR;
> > + kvm_run->exit_reason = KVM_EXIT_DEBUG;
> > + *r = EMULATE_USER_EXIT;
> > + } else {
> > + vcpu->arch.emulate_ctxt.eflags &= ~X86_EFLAGS_TF;
> > + /*
> > + * "Certain debug exceptions may clear bit 0-3. The
> > + * remaining contents of the DR6 register are never
> > + * cleared by the processor".
> > + */
> > + vcpu->arch.dr6 &= ~15;
> > + vcpu->arch.dr6 |= DR6_BS;
> > + kvm_queue_exception(vcpu, DB_VECTOR);
> > + }
> > + }
> > +}
> > +
> > static bool kvm_vcpu_check_breakpoint(struct kvm_vcpu *vcpu, int *r)
> > {
> > struct kvm_run *kvm_run = vcpu->run;
> > @@ -5117,10 +5152,12 @@ restart:
> >
> > if (writeback) {
> > toggle_interruptibility(vcpu, ctxt->interruptibility);
> > - kvm_set_rflags(vcpu, ctxt->eflags);
> > kvm_make_request(KVM_REQ_EVENT, vcpu);
> > vcpu->arch.emulate_regs_need_sync_to_vcpu = false;
> > kvm_rip_write(vcpu, ctxt->eip);
> > + if (r == EMULATE_DONE)
> Single step will not work for mmio write and pio out, we never return
> into emulator for those instructions.
Ok to apply the patch as is and work it out later (I suppose I need to
check for NULL complete_userspace_io, and if so set my function)? It
is already a huge improvement in usability.
Paolo
> > + kvm_vcpu_check_singlestep(vcpu, &r);
> > + kvm_set_rflags(vcpu, ctxt->eflags);
> > } else
> > vcpu->arch.emulate_regs_need_sync_to_vcpu = true;
> >
> > --
> > 1.8.1.4
>
> --
> Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
