[PATCH 0/1] trace all instructions whose emulation failed

[PATCH 0/1] trace all instructions whose emulation failed

[Manish Regmi]

Hi,
  The following patch makes sure all code path of failed emulation
runs trace_kvm_emulate_insn_failed().
Please let me know if there is anything missing or wrong.
Thank you.

Signed-off-by: Manish Regmi <regmi.manish@gmail.com>
--------
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b6e7535..fd1e875 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3784,36 +3784,35 @@ int emulate_instruction(struct kvm_vcpu *vcpu,
 		c = &vcpu->arch.emulate_ctxt.decode;
 		if (emulation_type & EMULTYPE_TRAP_UD) {
 			if (!c->twobyte)
-				return EMULATE_FAIL;
+				goto emulate_failed;
 			switch (c->b) {
 			case 0x01: /* VMMCALL */
 				if (c->modrm_mod != 3 || c->modrm_rm != 1)
-					return EMULATE_FAIL;
+					goto emulate_failed;
 				break;
 			case 0x34: /* sysenter */
 			case 0x35: /* sysexit */
 				if (c->modrm_mod != 0 || c->modrm_rm != 0)
-					return EMULATE_FAIL;
+					goto emulate_failed;
 				break;
 			case 0x05: /* syscall */
 				if (c->modrm_mod != 0 || c->modrm_rm != 0)
-					return EMULATE_FAIL;
+					goto emulate_failed;;
 				break;
 			default:
-				return EMULATE_FAIL;
+				goto emulate_failed;
 			}

 			if (!(c->modrm_reg == 0 || c->modrm_reg == 3))
-				return EMULATE_FAIL;
+				goto emulate_failed;
 		}

 		++vcpu->stat.insn_emulation;
 		if (r)  {
 			++vcpu->stat.insn_emulation_fail;
-			trace_kvm_emulate_insn_failed(vcpu);
 			if (kvm_mmu_unprotect_page_virt(vcpu, cr2))
 				return EMULATE_DONE;
-			return EMULATE_FAIL;
+			goto emulate_failed;
 		}
 	}

@@ -3848,9 +3847,8 @@ restart:
 			goto done;
 		if (!vcpu->mmio_needed) {
 			++vcpu->stat.insn_emulation_fail;
-			trace_kvm_emulate_insn_failed(vcpu);
 			kvm_report_emulation_failure(vcpu, "mmio");
-			return EMULATE_FAIL;
+			goto emulate_failed;
 		}
 		return EMULATE_DO_MMIO;
 	}
@@ -3868,6 +3866,10 @@ done:
 		goto restart;

 	return EMULATE_DONE;
+
+emulate_failed:
+	trace_kvm_emulate_insn_failed(vcpu);
+	return EMULATE_FAIL;
 }
 EXPORT_SYMBOL_GPL(emulate_instruction);


---------------------------------------------------------------
regards
Manish Regmi

http://manish-cs.blogspot.com
http://ext2read.sf.net

Re: [PATCH 0/1] trace all instructions whose emulation failed

[Avi Kivity]

On 04/18/2010 09:33 AM, Manish Regmi wrote:
> Hi,
>    The following patch makes sure all code path of failed emulation
> runs trace_kvm_emulate_insn_failed().
> Please let me know if there is anything missing or wrong.
> Thank you.
>
> Signed-off-by: Manish Regmi<regmi.manish@gmail.com>
> --------
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index b6e7535..fd1e875 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3784,36 +3784,35 @@ int emulate_instruction(struct kvm_vcpu *vcpu,
>   		c =&vcpu->arch.emulate_ctxt.decode;
>   		if (emulation_type&  EMULTYPE_TRAP_UD) {
>   			if (!c->twobyte)
> -				return EMULATE_FAIL;
> +				goto emulate_failed;
>   			switch (c->b) {
>   			case 0x01: /* VMMCALL */
>   				if (c->modrm_mod != 3 || c->modrm_rm != 1)
> -					return EMULATE_FAIL;
> +					goto emulate_failed;
>   				break;
>   			case 0x34: /* sysenter */
>   			case 0x35: /* sysexit */
>   				if (c->modrm_mod != 0 || c->modrm_rm != 0)
> -					return EMULATE_FAIL;
> +					goto emulate_failed;
>   				break;
>   			case 0x05: /* syscall */
>   				if (c->modrm_mod != 0 || c->modrm_rm != 0)
> -					return EMULATE_FAIL;
> +					goto emulate_failed;;
>   				break;
>   			default:
> -				return EMULATE_FAIL;
> +				goto emulate_failed;
>   			}
>
>   			if (!(c->modrm_reg == 0 || c->modrm_reg == 3))
> -				return EMULATE_FAIL;
> +				goto emulate_failed;
>   		}
>
>   		++vcpu->stat.insn_emulation;
>   		if (r)  {
>   			++vcpu->stat.insn_emulation_fail;
> -			trace_kvm_emulate_insn_failed(vcpu);
>   			if (kvm_mmu_unprotect_page_virt(vcpu, cr2))
>   				return EMULATE_DONE;
> -			return EMULATE_FAIL;
> +			goto emulate_failed;
>   		}
>   	}
>
>    

It's better not to trace #UD triggered emulations, since we except these 
to fail, for example if the guest executes the UD2 instruction.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 0/1] trace all instructions whose emulation failed

[Manish Regmi]

On Mon, Apr 19, 2010 at 4:20 AM, Avi Kivity <avi@redhat.com> wrote:
>>        }
>>
>>
>
> It's better not to trace #UD triggered emulations, since we except these to
> fail, for example if the guest executes the UD2 instruction.
>

ya. that sounds more logical. Thanks for explaining.
---------------------------------------------------------------
regards
Manish Regmi

http://manish-cs.blogspot.com
http://ext2read.sf.net