----- Gleb Natapov <[email protected]> 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 <[email protected]> > > --- > > 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 linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
