Re: [PATCH 2/2] KVM: SVM: Make stepping out of NMI handlers more robust

[Jan Kiszka <jan.kiszka@siemens.com>]

Gleb Natapov wrote:
> On Tue, Feb 16, 2010 at 12:08:58PM +0200, Gleb Natapov wrote:
>>>>> Besides this, proper #DB forwarding to the guest was missing.
>>>> During NMI injection? How to reproduce?
>>> Inject, e.g., an NMI over code with TF set. A bit harder is placing a
>>> guest HW breakpoint at the spot the NMI handler returns to.
>>>
>> Will try to reproduce.
>>
> How can I make gdb to run debugged process with TF set? Is this patch
> fixes it:
> 
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 52f78dd..b85b200 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -109,6 +109,7 @@ struct vcpu_svm {
>  	struct nested_state nested;
>  
>  	bool nmi_singlestep;
> +	bool nmi_singlestep_tf;
>  };
>  
>  /* enable NPT for AMD64 and X86 with PAE */
> @@ -1221,9 +1222,14 @@ static int db_interception(struct vcpu_svm *svm)
>  
>  	if (svm->nmi_singlestep) {
>  		svm->nmi_singlestep = false;
> -		if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_SINGLESTEP))
> +		if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_SINGLESTEP)) {
>  			svm->vmcb->save.rflags &=
>  				~(X86_EFLAGS_TF | X86_EFLAGS_RF);
> +			if (svm->nmi_singlestep_tf) {
> +				svm->vmcb->save.rflags |= X86_EFLAGS_TF;
> +				kvm_queue_exception(&svm->vcpu, DB_VECTOR);
> +			}
> +		}
>  		update_db_intercept(&svm->vcpu);
>  	}
>  
> @@ -2586,6 +2592,7 @@ static void enable_nmi_window(struct kvm_vcpu *vcpu)
>  	   possible problem (IRET or exception injection or interrupt
>  	   shadow) */
>  	svm->nmi_singlestep = true;
> +	svm->nmi_singlestep_tf = (svm->vmcb->save.rflags | X86_EFLAGS_TF);
>  	svm->vmcb->save.rflags |= (X86_EFLAGS_TF | X86_EFLAGS_RF);
>  	update_db_intercept(vcpu);
>  }

That's closer. However, I've a version here that restores TF&RF only if
you did not execute an IRET but stepped over the shadow (which is still
not correct either, e.g. when stepping popf). I will break up my patch
into parts that fix the issues separately so that we can decide what to
merge.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux