Re: [PATCH 3/3] KVM: x86: handle singlestep during emulation

[Gleb Natapov <gleb@redhat.com>]

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@redhat.com>
> ---
>  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.

> +			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.