[PATCH 1/3] KVM: x86 emulator: Rename variable that shadows another local variable.

[PATCH 1/3] KVM: x86 emulator: Rename variable that shadows another local variable.

[Gleb Natapov]

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 arch/x86/kvm/emulate.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 017ae0c..f9f8353 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3393,7 +3393,7 @@ writeback:
 				&c->dst);
 
 	if (c->rep_prefix && (c->d & String)) {
-		struct read_cache *rc = &ctxt->decode.io_read;
+		struct read_cache *r = &ctxt->decode.io_read;
 		register_address_increment(c, &c->regs[VCPU_REGS_RCX], -1);
 		/* The second termination condition only applies for REPE
 		 * and REPNE. Test if the repeat string operation prefix is
@@ -3413,8 +3413,8 @@ writeback:
 		 * Re-enter guest when pio read ahead buffer is empty or,
 		 * if it is not used, after each 1024 iteration.
 		 */
-		else if ((rc->end == 0 && !(c->regs[VCPU_REGS_RCX] & 0x3ff)) ||
-			 (rc->end != 0 && rc->end == rc->pos)) {
+		else if ((r->end == 0 && !(c->regs[VCPU_REGS_RCX] & 0x3ff)) ||
+			 (r->end != 0 && r->end == r->pos)) {
 			ctxt->restart = false;
 			c->eip = ctxt->eip;
 		}
-- 
1.7.1

[PATCH 2/3] KVM: x86 emulator: move string instruction completion check into separate function

[Gleb Natapov]

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 arch/x86/kvm/emulate.c |   42 +++++++++++++++++++++++++++++-------------
 1 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index f9f8353..d34d706 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2921,6 +2921,32 @@ done:
 	return (rc == X86EMUL_UNHANDLEABLE) ? -1 : 0;
 }
 
+static bool string_inst_completed(struct x86_emulate_ctxt *ctxt)
+{
+	struct decode_cache *c = &ctxt->decode;
+
+	/* All REP prefixes have the same first termination condition */
+	if (address_mask(c, c->regs[VCPU_REGS_RCX]) == 0)
+		return true;
+
+	/* The second termination condition only applies for REPE
+	 * and REPNE. Test if the repeat string operation prefix is
+	 * REPE/REPZ or REPNE/REPNZ and if it's the case it tests the
+	 * corresponding termination condition according to:
+	 * 	- if REPE/REPZ and ZF = 0 then done
+	 * 	- if REPNE/REPNZ and ZF = 1 then done
+	 */
+	if (((c->b == 0xa6) || (c->b == 0xa7) ||
+	     (c->b == 0xae) || (c->b == 0xaf))
+	    && (((c->rep_prefix == REPE_PREFIX) &&
+		 ((ctxt->eflags & EFLG_ZF) == 0))
+		|| ((c->rep_prefix == REPNE_PREFIX) &&
+		    ((ctxt->eflags & EFLG_ZF) == EFLG_ZF))))
+		return true;
+
+	return false;
+}
+
 int
 x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 {
@@ -3395,19 +3421,9 @@ writeback:
 	if (c->rep_prefix && (c->d & String)) {
 		struct read_cache *r = &ctxt->decode.io_read;
 		register_address_increment(c, &c->regs[VCPU_REGS_RCX], -1);
-		/* The second termination condition only applies for REPE
-		 * and REPNE. Test if the repeat string operation prefix is
-		 * REPE/REPZ or REPNE/REPNZ and if it's the case it tests the
-		 * corresponding termination condition according to:
-		 * 	- if REPE/REPZ and ZF = 0 then done
-		 * 	- if REPNE/REPNZ and ZF = 1 then done
-		 */
-		if (((c->b == 0xa6) || (c->b == 0xa7) ||
-		     (c->b == 0xae) || (c->b == 0xaf))
-		    && (((c->rep_prefix == REPE_PREFIX) &&
-			 ((ctxt->eflags & EFLG_ZF) == 0))
-			|| ((c->rep_prefix == REPNE_PREFIX) &&
-			    ((ctxt->eflags & EFLG_ZF) == EFLG_ZF))))
+
+
+		if (string_inst_completed(ctxt))
 			ctxt->restart = false;
 		/*
 		 * Re-enter guest when pio read ahead buffer is empty or,
-- 
1.7.1

Re: [PATCH 2/3] KVM: x86 emulator: move string instruction completion check into separate function

[Avi Kivity]

  On 08/24/2010 02:30 PM, Gleb Natapov wrote:
> Signed-off-by: Gleb Natapov<gleb@redhat.com>
> ---
>   arch/x86/kvm/emulate.c |   42 +++++++++++++++++++++++++++++-------------
>   1 files changed, 29 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index f9f8353..d34d706 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -2921,6 +2921,32 @@ done:
>   	return (rc == X86EMUL_UNHANDLEABLE) ? -1 : 0;
>   }
>
> +static bool string_inst_completed(struct x86_emulate_ctxt *ctxt)

s/inst/insn/.

> +{
> +	struct decode_cache *c =&ctxt->decode;
> +
> +	/* All REP prefixes have the same first termination condition */
> +	if (address_mask(c, c->regs[VCPU_REGS_RCX]) == 0)
> +		return true;

This is checked during the beginning of the instruction, not after 
completion.  Why is it here?  it will just be duplicated.

> +
> +	/* The second termination condition only applies for REPE
> +	 * and REPNE. Test if the repeat string operation prefix is
> +	 * REPE/REPZ or REPNE/REPNZ and if it's the case it tests the
> +	 * corresponding termination condition according to:
> +	 * 	- if REPE/REPZ and ZF = 0 then done
> +	 * 	- if REPNE/REPNZ and ZF = 1 then done
> +	 */
> +	if (((c->b == 0xa6) || (c->b == 0xa7) ||
> +	     (c->b == 0xae) || (c->b == 0xaf))
> +	&&  (((c->rep_prefix == REPE_PREFIX)&&
> +		 ((ctxt->eflags&  EFLG_ZF) == 0))
> +		|| ((c->rep_prefix == REPNE_PREFIX)&&
> +		    ((ctxt->eflags&  EFLG_ZF) == EFLG_ZF))))
> +		return true;
> +
> +	return false;
> +}
> +

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

Re: [PATCH 2/3] KVM: x86 emulator: move string instruction completion check into separate function

[Gleb Natapov]

On Tue, Aug 24, 2010 at 04:11:20PM +0300, Avi Kivity wrote:
>  On 08/24/2010 02:30 PM, Gleb Natapov wrote:
> >Signed-off-by: Gleb Natapov<gleb@redhat.com>
> >---
> >  arch/x86/kvm/emulate.c |   42 +++++++++++++++++++++++++++++-------------
> >  1 files changed, 29 insertions(+), 13 deletions(-)
> >
> >diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> >index f9f8353..d34d706 100644
> >--- a/arch/x86/kvm/emulate.c
> >+++ b/arch/x86/kvm/emulate.c
> >@@ -2921,6 +2921,32 @@ done:
> >  	return (rc == X86EMUL_UNHANDLEABLE) ? -1 : 0;
> >  }
> >
> >+static bool string_inst_completed(struct x86_emulate_ctxt *ctxt)
> 
> s/inst/insn/.
> 
> >+{
> >+	struct decode_cache *c =&ctxt->decode;
> >+
> >+	/* All REP prefixes have the same first termination condition */
> >+	if (address_mask(c, c->regs[VCPU_REGS_RCX]) == 0)
> >+		return true;
> 
> This is checked during the beginning of the instruction, not after
> completion.  Why is it here?  it will just be duplicated.
> 
SDM describes REP instruction algorithm this way:

WHILE CountReg ≠ 0
  DO
       Service pending interrupts (if any);
       Execute associated string instruction;
       CountReg ← (CountReg – 1);
       IF CountReg = 0
            THEN exit WHILE loop; FI;
       IF (Repeat prefix is REPZ or REPE) and (ZF = 0)
       or (Repeat prefix is REPNZ or REPNE) and (ZF = 1)
            THEN exit WHILE loop; FI;
  OD;

So CountReg is checked at the beginning and after each iteration.
Practically it will save us one return to a guest and exit back
to emulator at the end of rep instruction (not a big deal).

> >+
> >+	/* The second termination condition only applies for REPE
> >+	 * and REPNE. Test if the repeat string operation prefix is
> >+	 * REPE/REPZ or REPNE/REPNZ and if it's the case it tests the
> >+	 * corresponding termination condition according to:
> >+	 * 	- if REPE/REPZ and ZF = 0 then done
> >+	 * 	- if REPNE/REPNZ and ZF = 1 then done
> >+	 */
> >+	if (((c->b == 0xa6) || (c->b == 0xa7) ||
> >+	     (c->b == 0xae) || (c->b == 0xaf))
> >+	&&  (((c->rep_prefix == REPE_PREFIX)&&
> >+		 ((ctxt->eflags&  EFLG_ZF) == 0))
> >+		|| ((c->rep_prefix == REPNE_PREFIX)&&
> >+		    ((ctxt->eflags&  EFLG_ZF) == EFLG_ZF))))
> >+		return true;
> >+
> >+	return false;
> >+}
> >+
> 
> -- 
> error compiling committee.c: too many arguments to function

--
			Gleb.

Re: [PATCH 2/3] KVM: x86 emulator: move string instruction completion check into separate function

[Avi Kivity]

  On 08/24/2010 04:20 PM, Gleb Natapov wrote:
>>
>>> +{
>>> +	struct decode_cache *c =&ctxt->decode;
>>> +
>>> +	/* All REP prefixes have the same first termination condition */
>>> +	if (address_mask(c, c->regs[VCPU_REGS_RCX]) == 0)
>>> +		return true;
>> This is checked during the beginning of the instruction, not after
>> completion.  Why is it here?  it will just be duplicated.
>>
> SDM describes REP instruction algorithm this way:
>
> WHILE CountReg ≠ 0
>    DO
>         Service pending interrupts (if any);
>         Execute associated string instruction;
>         CountReg ← (CountReg – 1);
>         IF CountReg = 0
>              THEN exit WHILE loop; FI;
>         IF (Repeat prefix is REPZ or REPE) and (ZF = 0)
>         or (Repeat prefix is REPNZ or REPNE) and (ZF = 1)
>              THEN exit WHILE loop; FI;
>    OD;
>
> So CountReg is checked at the beginning and after each iteration.

The second check is meaningless (and ZF checks should be qualified with 
the actual instruction).

> Practically it will save us one return to a guest and exit back
> to emulator at the end of rep instruction (not a big deal).

Not even that - if we reenter to the beginning of the rep instruction 
the cpu will skip over it without exiting (unless in big real mode with 
eigs=1).

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

[PATCH 3/3] KVM: x86 emulator: get rid of "restart" in emulation context.

[Gleb Natapov]

x86_emulate_insn() will return 1 if instruction can be restarted
without re-entering a guest.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 arch/x86/include/asm/kvm_emulate.h |    1 -
 arch/x86/kvm/emulate.c             |   47 ++++++++++++++++-------------------
 arch/x86/kvm/x86.c                 |   16 ++++++------
 3 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index f22e5da..5e120a7 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -220,7 +220,6 @@ struct x86_emulate_ctxt {
 	/* interruptibility state, as a result of execution of STI or MOV SS */
 	int interruptibility;
 
-	bool restart; /* restart string instruction after writeback */
 	bool perm_ok; /* do not check permissions if true */
 
 	int exception; /* exception that happens during emulation or -1 */
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index d34d706..d98fc3b 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -438,7 +438,6 @@ static void emulate_exception(struct x86_emulate_ctxt *ctxt, int vec,
 	ctxt->exception = vec;
 	ctxt->error_code = error;
 	ctxt->error_code_valid = valid;
-	ctxt->restart = false;
 }
 
 static void emulate_gp(struct x86_emulate_ctxt *ctxt, int err)
@@ -2618,9 +2617,6 @@ x86_decode_insn(struct x86_emulate_ctxt *ctxt)
 	struct opcode opcode, *g_mod012, *g_mod3;
 	struct operand memop = { .type = OP_NONE };
 
-	/* we cannot decode insn before we complete previous rep insn */
-	WARN_ON(ctxt->restart);
-
 	c->eip = ctxt->eip;
 	c->fetch.start = c->fetch.end = c->eip;
 	ctxt->cs_base = seg_base(ctxt, ops, VCPU_SREG_CS);
@@ -2977,10 +2973,11 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 	}
 
 	if (c->rep_prefix && (c->d & String)) {
-		ctxt->restart = true;
-		/* All REP prefixes have the same first termination condition */
+		/*
+		 * counter == 0 termination condition is checked before
+		 * instruction execution.
+		 */
 		if (address_mask(c, c->regs[VCPU_REGS_RCX]) == 0) {
-			ctxt->restart = false;
 			ctxt->eip = c->eip;
 			goto done;
 		}
@@ -3422,26 +3419,26 @@ writeback:
 		struct read_cache *r = &ctxt->decode.io_read;
 		register_address_increment(c, &c->regs[VCPU_REGS_RCX], -1);
 
-
-		if (string_inst_completed(ctxt))
-			ctxt->restart = false;
-		/*
-		 * Re-enter guest when pio read ahead buffer is empty or,
-		 * if it is not used, after each 1024 iteration.
-		 */
-		else if ((r->end == 0 && !(c->regs[VCPU_REGS_RCX] & 0x3ff)) ||
-			 (r->end != 0 && r->end == r->pos)) {
-			ctxt->restart = false;
-			c->eip = ctxt->eip;
+		if (!string_inst_completed(ctxt)) {
+			/*
+			 * Re-enter guest when pio read ahead buffer is empty
+			 * or, if it is not used, after each 1024 iteration.
+			 */
+			if ((r->end != 0 || c->regs[VCPU_REGS_RCX] & 0x3ff) &&
+			    (r->end == 0 || r->end != r->pos)) {
+				/*
+				 * Reset read cache. Usually happens before
+				 * decode, but since instruction is restarted
+				 * we have to do it here.
+				 */
+				ctxt->decode.mem_read.end = 0;
+				return 1; /* restart instruction */
+			}
+			goto done; /* skip rip writeback */
 		}
 	}
-	/*
-	 * reset read cache here in case string instruction is restared
-	 * without decoding
-	 */
-	ctxt->decode.mem_read.end = 0;
-	if (!ctxt->restart)
-		ctxt->eip = c->eip;
+
+	ctxt->eip = c->eip;
 
 done:
 	return (rc == X86EMUL_UNHANDLEABLE) ? -1 : 0;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 4e179c5..131f2c8 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4066,18 +4066,17 @@ int emulate_instruction(struct kvm_vcpu *vcpu,
 restart:
 	r = x86_emulate_insn(&vcpu->arch.emulate_ctxt);
 
-	if (r) { /* emulation failed */
+	if (r < 0) { /* emulation failed */
 		if (reexecute_instruction(vcpu, cr2))
 			return EMULATE_DONE;
 
 		return handle_emulation_failure(vcpu);
 	}
 
-	r = EMULATE_DONE;
-
-	if (vcpu->arch.emulate_ctxt.exception >= 0)
+	if (vcpu->arch.emulate_ctxt.exception >= 0) {
 		inject_emulated_exception(vcpu);
-	else if (vcpu->arch.pio.count) {
+		r = EMULATE_DONE;
+	} else if (vcpu->arch.pio.count) {
 		if (!vcpu->arch.pio.in)
 			vcpu->arch.pio.count = 0;
 		r = EMULATE_DO_MMIO;
@@ -4085,8 +4084,10 @@ restart:
 		if (vcpu->mmio_is_write)
 			vcpu->mmio_needed = 0;
 		r = EMULATE_DO_MMIO;
-	} else if (vcpu->arch.emulate_ctxt.restart)
+	} else if (r > 0)
 		goto restart;
+	else
+		r = EMULATE_DONE;
 
 	toggle_interruptibility(vcpu, vcpu->arch.emulate_ctxt.interruptibility);
 	kvm_x86_ops->set_rflags(vcpu, vcpu->arch.emulate_ctxt.eflags);
@@ -4909,8 +4910,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	if (!irqchip_in_kernel(vcpu->kvm))
 		kvm_set_cr8(vcpu, kvm_run->cr8);
 
-	if (vcpu->arch.pio.count || vcpu->mmio_needed ||
-	    vcpu->arch.emulate_ctxt.restart) {
+	if (vcpu->arch.pio.count || vcpu->mmio_needed) {
 		if (vcpu->mmio_needed) {
 			memcpy(vcpu->mmio_data, kvm_run->mmio.data, 8);
 			vcpu->mmio_read_completed = 1;
-- 
1.7.1

Re: [PATCH 3/3] KVM: x86 emulator: get rid of "restart" in emulation context.

[Avi Kivity]

  On 08/24/2010 02:30 PM, Gleb Natapov wrote:
> x86_emulate_insn() will return 1 if instruction can be restarted
> without re-entering a guest.
>

So now we have an undocumented -1/0/1 return code?

Better to have an enum for this.

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

Re: [PATCH 3/3] KVM: x86 emulator: get rid of "restart" in emulation context.

[Gleb Natapov]

On Tue, Aug 24, 2010 at 04:13:38PM +0300, Avi Kivity wrote:
>  On 08/24/2010 02:30 PM, Gleb Natapov wrote:
> >x86_emulate_insn() will return 1 if instruction can be restarted
> >without re-entering a guest.
> >
> 
> So now we have an undocumented -1/0/1 return code?
> 
> Better to have an enum for this.
> 
We already have two. First is X86EMUL_ (not enum but close) for
more or less internal emulator use. Second is EMULATE_* for users of
emulate_instruction() now you want one more enum for communication
between emulate_instruction() and x86_emulate_insn(). Lost in enums.
emulate_instruction() and x86_emulate_insn() are tightly coupled right
now should we define formal interface between them? May be comment will
be enough?

--
			Gleb.

Re: [PATCH 3/3] KVM: x86 emulator: get rid of "restart" in emulation context.

[Avi Kivity]

  On 08/24/2010 04:37 PM, Gleb Natapov wrote:
> On Tue, Aug 24, 2010 at 04:13:38PM +0300, Avi Kivity wrote:
>>   On 08/24/2010 02:30 PM, Gleb Natapov wrote:
>>> x86_emulate_insn() will return 1 if instruction can be restarted
>>> without re-entering a guest.
>>>
>> So now we have an undocumented -1/0/1 return code?
>>
>> Better to have an enum for this.
>>
> We already have two. First is X86EMUL_ (not enum but close) for
> more or less internal emulator use. Second is EMULATE_* for users of
> emulate_instruction() now you want one more enum for communication
> between emulate_instruction() and x86_emulate_insn(). Lost in enums.
> emulate_instruction() and x86_emulate_insn() are tightly coupled right
> now should we define formal interface between them? May be comment will
> be enough?

Can we reuse one or the other?  Perhaps with extensions?

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

Re: [PATCH 3/3] KVM: x86 emulator: get rid of "restart" in emulation context.

[Gleb Natapov]

On Tue, Aug 24, 2010 at 04:41:10PM +0300, Avi Kivity wrote:
>  On 08/24/2010 04:37 PM, Gleb Natapov wrote:
> >On Tue, Aug 24, 2010 at 04:13:38PM +0300, Avi Kivity wrote:
> >>  On 08/24/2010 02:30 PM, Gleb Natapov wrote:
> >>>x86_emulate_insn() will return 1 if instruction can be restarted
> >>>without re-entering a guest.
> >>>
> >>So now we have an undocumented -1/0/1 return code?
> >>
> >>Better to have an enum for this.
> >>
> >We already have two. First is X86EMUL_ (not enum but close) for
> >more or less internal emulator use. Second is EMULATE_* for users of
> >emulate_instruction() now you want one more enum for communication
> >between emulate_instruction() and x86_emulate_insn(). Lost in enums.
> >emulate_instruction() and x86_emulate_insn() are tightly coupled right
> >now should we define formal interface between them? May be comment will
> >be enough?
> 
> Can we reuse one or the other?  Perhaps with extensions?
> 
We can, of course. But for me it looks as arbitrary as -1/0/1 since not
all enum values have meanings to the caller.

--
			Gleb.

Re: [PATCH 3/3] KVM: x86 emulator: get rid of "restart" in emulation context.

[Avi Kivity]

  On 08/24/2010 04:52 PM, Gleb Natapov wrote:
>
> We can, of course. But for me it looks as arbitrary as -1/0/1 since not
> all enum values have meanings to the caller.

Yeah.  -1/0/1's problem is that between reading the callee code and 
caller code, I manage to forget what the values mean.

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

Re: [PATCH 3/3] KVM: x86 emulator: get rid of "restart" in emulation context.

[Gleb Natapov]

On Tue, Aug 24, 2010 at 05:01:10PM +0300, Avi Kivity wrote:
>  On 08/24/2010 04:52 PM, Gleb Natapov wrote:
> >
> >We can, of course. But for me it looks as arbitrary as -1/0/1 since not
> >all enum values have meanings to the caller.
> 
> Yeah.  -1/0/1's problem is that between reading the callee code and
> caller code, I manage to forget what the values mean.
> 
Luckily we have only one caller of x86_emulate_insn(), so documenting
return values right where function is called should help. :)

--
			Gleb.

Re: [PATCH 3/3] KVM: x86 emulator: get rid of "restart" in emulation context.

[Avi Kivity]

  On 08/24/2010 05:06 PM, Gleb Natapov wrote:
> On Tue, Aug 24, 2010 at 05:01:10PM +0300, Avi Kivity wrote:
>>   On 08/24/2010 04:52 PM, Gleb Natapov wrote:
>>> We can, of course. But for me it looks as arbitrary as -1/0/1 since not
>>> all enum values have meanings to the caller.
>> Yeah.  -1/0/1's problem is that between reading the callee code and
>> caller code, I manage to forget what the values mean.
>>
> Luckily we have only one caller of x86_emulate_insn(), so documenting
> return values right where function is called should help. :)

Please use #define instead of /* */.

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

Re: [PATCH 3/3] KVM: x86 emulator: get rid of "restart" in emulation context.

[Gleb Natapov]

On Tue, Aug 24, 2010 at 05:28:12PM +0300, Avi Kivity wrote:
>  On 08/24/2010 05:06 PM, Gleb Natapov wrote:
> >On Tue, Aug 24, 2010 at 05:01:10PM +0300, Avi Kivity wrote:
> >>  On 08/24/2010 04:52 PM, Gleb Natapov wrote:
> >>>We can, of course. But for me it looks as arbitrary as -1/0/1 since not
> >>>all enum values have meanings to the caller.
> >>Yeah.  -1/0/1's problem is that between reading the callee code and
> >>caller code, I manage to forget what the values mean.
> >>
> >Luckily we have only one caller of x86_emulate_insn(), so documenting
> >return values right where function is called should help. :)
> 
> Please use #define instead of /* */.
> 
Sigh.

--
			Gleb.