[PATCH 0/7] Streamline arithmetic instruction emulation

[PATCH 0/7] Streamline arithmetic instruction emulation

[Avi Kivity]

The current arithmetic instruction emulation is fairly clumsy: after
decode, each instruction gets a switch (size), and for every size
we fetch the operands, prepare flags, emulate the instruction, then store
back the flags and operands.

This patchset simplifies things by moving everything into common code
except the instruction itself.  All the pre- and post- processing is
coded just once.  The per-instrution code looks like:

  add %bl, %al
  ret

  add %bx, %ax
  ret

  add %ebx, %eax
  ret

  add %rbx, %rax
  ret

The savings in size, for the ten instructions converted in this patchset,
are fairly large:

   text	   data	    bss	    dec	    hex	filename
  63724	      0	      0	  63724	   f8ec	arch/x86/kvm/emulate.o.before
  61268	      0	      0	  61268	   ef54	arch/x86/kvm/emulate.o.after

- around 2500 bytes.

Avi Kivity (7):
  KVM: x86 emulator: framework for streamlining arithmetic opcodes
  KVM: x86 emulator: Support for declaring single operand fastops
  KVM: x86 emulator: introduce NoWrite flag
  KVM: x86 emulator: mark CMP, CMPS, SCAS, TEST as NoWrite
  KVM: x86 emulator: convert NOT, NEG to fastop
  KVM: x86 emulator: add macros for defining 2-operand fastop emulation
  KVM: x86 emulator: convert basic ALU ops to fastop

 arch/x86/kvm/emulate.c | 215 +++++++++++++++++++++++++++----------------------
 1 file changed, 120 insertions(+), 95 deletions(-)

-- 
1.7.11.7

[PATCH 1/7] KVM: x86 emulator: framework for streamlining arithmetic opcodes

[Avi Kivity]

We emulate arithmetic opcodes by executing a "similar" (same operation,
different operands) on the cpu.  This ensures accurate emulation, esp. wrt.
eflags.  However, the prologue and epilogue around the opcode is fairly long,
consisting of a switch (for the operand size) and code to load and save the
operands.  This is repeated for every opcode.

This patch introduces an alternative way to emulate arithmetic opcodes.
Instead of the above, we have four (three on i386) functions consisting
of just the opcode and a ret; one for each operand size.  For example:

   .align 8
   em_notb:
	not %al
	ret

   .align 8
   em_notw:
	not %ax
	ret

   .align 8
   em_notl:
	not %eax
	ret

   .align 8
   em_notq:
	not %rax
	ret

The prologue and epilogue are shared across all opcodes.  Note the functions
use a special calling convention; notably eflags is an input/output parameter
and is not clobbered.  Rather than dispatching the four functions through a
jump table, the functions are declared as a constant size (8) so their address
can be calculated.

Signed-off-by: Avi Kivity <avi.kivity@gmail.com>
---
 arch/x86/kvm/emulate.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index c7547b3..cdf7b97 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -148,6 +148,7 @@
 #define Aligned     ((u64)1 << 41)  /* Explicitly aligned (e.g. MOVDQA) */
 #define Unaligned   ((u64)1 << 42)  /* Explicitly unaligned (e.g. MOVDQU) */
 #define Avx         ((u64)1 << 43)  /* Advanced Vector Extensions */
+#define Fastop      ((u64)1 << 44)  /* Use opcode::u.fastop */
 
 #define X2(x...) x, x
 #define X3(x...) X2(x), x
@@ -158,6 +159,27 @@
 #define X8(x...) X4(x), X4(x)
 #define X16(x...) X8(x), X8(x)
 
+#define NR_FASTOP (ilog2(sizeof(ulong)) + 1)
+#define FASTOP_SIZE 8
+
+/*
+ * fastop functions have a special calling convention:
+ *
+ * dst:    [rdx]:rax  (in/out)
+ * src:    rbx        (in/out)
+ * src2:   rcx        (in)
+ * flags:  rflags     (in/out)
+ *
+ * Moreover, they are all exactly FASTOP_SIZE bytes long, so functions for
+ * different operand sizes can be reached by calculation, rather than a jump
+ * table (which would be bigger than the code).
+ *
+ * fastop functions are declared as taking a never-defined fastop parameter,
+ * so they can't be called from C directly.
+ */
+
+struct fastop;
+
 struct opcode {
 	u64 flags : 56;
 	u64 intercept : 8;
@@ -166,6 +188,7 @@ struct opcode {
 		const struct opcode *group;
 		const struct group_dual *gdual;
 		const struct gprefix *gprefix;
+		void (*fastop)(struct fastop *fake);
 	} u;
 	int (*check_perm)(struct x86_emulate_ctxt *ctxt);
 };
@@ -3596,6 +3619,7 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
 #define G(_f, _g) { .flags = ((_f) | Group | ModRM), .u.group = (_g) }
 #define GD(_f, _g) { .flags = ((_f) | GroupDual | ModRM), .u.gdual = (_g) }
 #define I(_f, _e) { .flags = (_f), .u.execute = (_e) }
+#define F(_f, _e) { .flags = (_f) | Fastop, .u.fastop = (_e) }
 #define II(_f, _e, _i) \
 	{ .flags = (_f), .u.execute = (_e), .intercept = x86_intercept_##_i }
 #define IIP(_f, _e, _i, _p) \
@@ -4383,6 +4407,16 @@ static void fetch_possible_mmx_operand(struct x86_emulate_ctxt *ctxt,
 		read_mmx_reg(ctxt, &op->mm_val, op->addr.mm);
 }
 
+static int fastop(struct x86_emulate_ctxt *ctxt, void (*fop)(struct fastop *))
+{
+	ulong flags = (ctxt->eflags & EFLAGS_MASK) | X86_EFLAGS_IF;
+	fop += __ffs(ctxt->dst.bytes) * FASTOP_SIZE;
+	asm("push %[flags]; popf; call *%[fastop]; pushf; pop %[flags]\n"
+	    : "+a"(ctxt->dst.val), "+b"(ctxt->src.val), [flags]"+D"(flags)
+	: "c"(ctxt->src2.val), [fastop]"S"(fop));
+	ctxt->eflags = (ctxt->eflags & ~EFLAGS_MASK) | (flags & EFLAGS_MASK);
+	return X86EMUL_CONTINUE;
+}
 
 int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 {
@@ -4512,6 +4546,13 @@ special_insn:
 	}
 
 	if (ctxt->execute) {
+		if (ctxt->d & Fastop) {
+			void (*fop)(struct fastop *) = (void *)ctxt->execute;
+			rc = fastop(ctxt, fop);
+			if (rc != X86EMUL_CONTINUE)
+				goto done;
+			goto writeback;
+		}
 		rc = ctxt->execute(ctxt);
 		if (rc != X86EMUL_CONTINUE)
 			goto done;
-- 
1.7.11.7

Re: [PATCH 1/7] KVM: x86 emulator: framework for streamlining arithmetic opcodes

[Gleb Natapov]

On Sat, Dec 22, 2012 at 02:26:51PM +0200, Avi Kivity wrote:
> We emulate arithmetic opcodes by executing a "similar" (same operation,
> different operands) on the cpu.  This ensures accurate emulation, esp. wrt.
> eflags.  However, the prologue and epilogue around the opcode is fairly long,
> consisting of a switch (for the operand size) and code to load and save the
> operands.  This is repeated for every opcode.
> 
> This patch introduces an alternative way to emulate arithmetic opcodes.
> Instead of the above, we have four (three on i386) functions consisting
> of just the opcode and a ret; one for each operand size.  For example:
> 
>    .align 8
>    em_notb:
> 	not %al
> 	ret
> 
>    .align 8
>    em_notw:
> 	not %ax
> 	ret
> 
>    .align 8
>    em_notl:
> 	not %eax
> 	ret
> 
>    .align 8
>    em_notq:
> 	not %rax
> 	ret
> 
> The prologue and epilogue are shared across all opcodes.  Note the functions
> use a special calling convention; notably eflags is an input/output parameter
> and is not clobbered.  Rather than dispatching the four functions through a
> jump table, the functions are declared as a constant size (8) so their address
> can be calculated.
> 
> Signed-off-by: Avi Kivity <avi.kivity@gmail.com>
> ---
>  arch/x86/kvm/emulate.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index c7547b3..cdf7b97 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -148,6 +148,7 @@
>  #define Aligned     ((u64)1 << 41)  /* Explicitly aligned (e.g. MOVDQA) */
>  #define Unaligned   ((u64)1 << 42)  /* Explicitly unaligned (e.g. MOVDQU) */
>  #define Avx         ((u64)1 << 43)  /* Advanced Vector Extensions */
> +#define Fastop      ((u64)1 << 44)  /* Use opcode::u.fastop */
>  
>  #define X2(x...) x, x
>  #define X3(x...) X2(x), x
> @@ -158,6 +159,27 @@
>  #define X8(x...) X4(x), X4(x)
>  #define X16(x...) X8(x), X8(x)
>  
> +#define NR_FASTOP (ilog2(sizeof(ulong)) + 1)
> +#define FASTOP_SIZE 8
> +
> +/*
> + * fastop functions have a special calling convention:
> + *
> + * dst:    [rdx]:rax  (in/out)
May be I miss something obvious but I do not see why rdx is here.

> + * src:    rbx        (in/out)
> + * src2:   rcx        (in)
> + * flags:  rflags     (in/out)
> + *
> + * Moreover, they are all exactly FASTOP_SIZE bytes long, so functions for
> + * different operand sizes can be reached by calculation, rather than a jump
> + * table (which would be bigger than the code).
> + *
> + * fastop functions are declared as taking a never-defined fastop parameter,
> + * so they can't be called from C directly.
> + */
> +
> +struct fastop;
> +
>  struct opcode {
>  	u64 flags : 56;
>  	u64 intercept : 8;
> @@ -166,6 +188,7 @@ struct opcode {
>  		const struct opcode *group;
>  		const struct group_dual *gdual;
>  		const struct gprefix *gprefix;
> +		void (*fastop)(struct fastop *fake);
>  	} u;
>  	int (*check_perm)(struct x86_emulate_ctxt *ctxt);
>  };
> @@ -3596,6 +3619,7 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
>  #define G(_f, _g) { .flags = ((_f) | Group | ModRM), .u.group = (_g) }
>  #define GD(_f, _g) { .flags = ((_f) | GroupDual | ModRM), .u.gdual = (_g) }
>  #define I(_f, _e) { .flags = (_f), .u.execute = (_e) }
> +#define F(_f, _e) { .flags = (_f) | Fastop, .u.fastop = (_e) }
>  #define II(_f, _e, _i) \
>  	{ .flags = (_f), .u.execute = (_e), .intercept = x86_intercept_##_i }
>  #define IIP(_f, _e, _i, _p) \
> @@ -4383,6 +4407,16 @@ static void fetch_possible_mmx_operand(struct x86_emulate_ctxt *ctxt,
>  		read_mmx_reg(ctxt, &op->mm_val, op->addr.mm);
>  }
>  
> +static int fastop(struct x86_emulate_ctxt *ctxt, void (*fop)(struct fastop *))
> +{
> +	ulong flags = (ctxt->eflags & EFLAGS_MASK) | X86_EFLAGS_IF;
> +	fop += __ffs(ctxt->dst.bytes) * FASTOP_SIZE;
> +	asm("push %[flags]; popf; call *%[fastop]; pushf; pop %[flags]\n"
> +	    : "+a"(ctxt->dst.val), "+b"(ctxt->src.val), [flags]"+D"(flags)
> +	: "c"(ctxt->src2.val), [fastop]"S"(fop));
> +	ctxt->eflags = (ctxt->eflags & ~EFLAGS_MASK) | (flags & EFLAGS_MASK);
> +	return X86EMUL_CONTINUE;
> +}
>  
>  int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
>  {
> @@ -4512,6 +4546,13 @@ special_insn:
>  	}
>  
>  	if (ctxt->execute) {
> +		if (ctxt->d & Fastop) {
> +			void (*fop)(struct fastop *) = (void *)ctxt->execute;
> +			rc = fastop(ctxt, fop);
> +			if (rc != X86EMUL_CONTINUE)
> +				goto done;
> +			goto writeback;
> +		}
>  		rc = ctxt->execute(ctxt);
>  		if (rc != X86EMUL_CONTINUE)
>  			goto done;
> -- 
> 1.7.11.7

--
			Gleb.

Re: [PATCH 1/7] KVM: x86 emulator: framework for streamlining arithmetic opcodes

[Avi Kivity]

On Sat, Dec 22, 2012 at 7:00 PM, Gleb Natapov <gleb@redhat.com> wrote:
> On Sat, Dec 22, 2012 at 02:26:51PM +0200, Avi Kivity wrote:
>> +
>> +/*
>> + * fastop functions have a special calling convention:
>> + *
>> + * dst:    [rdx]:rax  (in/out)
> May be I miss something obvious but I do not see why rdx is here.

MUL, DIV, CWD, etc.

Re: [PATCH 1/7] KVM: x86 emulator: framework for streamlining arithmetic opcodes

[Gleb Natapov]

On Sat, Dec 22, 2012 at 07:42:55PM +0200, Avi Kivity wrote:
> On Sat, Dec 22, 2012 at 7:00 PM, Gleb Natapov <gleb@redhat.com> wrote:
> > On Sat, Dec 22, 2012 at 02:26:51PM +0200, Avi Kivity wrote:
> >> +
> >> +/*
> >> + * fastop functions have a special calling convention:
> >> + *
> >> + * dst:    [rdx]:rax  (in/out)
> > May be I miss something obvious but I do not see why rdx is here.
> 
> MUL, DIV, CWD, etc.
Yes, but my question was more about actual fastop() implementation
which does not seams to handle rdx at all.

--
			Gleb.

Re: [PATCH 1/7] KVM: x86 emulator: framework for streamlining arithmetic opcodes

[Avi Kivity]

On Sat, Dec 22, 2012 at 8:01 PM, Gleb Natapov <gleb@redhat.com> wrote:
> On Sat, Dec 22, 2012 at 07:42:55PM +0200, Avi Kivity wrote:
>> On Sat, Dec 22, 2012 at 7:00 PM, Gleb Natapov <gleb@redhat.com> wrote:
>> > On Sat, Dec 22, 2012 at 02:26:51PM +0200, Avi Kivity wrote:
>> >> +
>> >> +/*
>> >> + * fastop functions have a special calling convention:
>> >> + *
>> >> + * dst:    [rdx]:rax  (in/out)
>> > May be I miss something obvious but I do not see why rdx is here.
>>
>> MUL, DIV, CWD, etc.
> Yes, but my question was more about actual fastop() implementation
> which does not seams to handle rdx at all.


It will be added later.  If you like, I can remove it from the comment for now.

Re: [PATCH 1/7] KVM: x86 emulator: framework for streamlining arithmetic opcodes

[Gleb Natapov]

On Sat, Dec 22, 2012 at 08:07:30PM +0200, Avi Kivity wrote:
> On Sat, Dec 22, 2012 at 8:01 PM, Gleb Natapov <gleb@redhat.com> wrote:
> > On Sat, Dec 22, 2012 at 07:42:55PM +0200, Avi Kivity wrote:
> >> On Sat, Dec 22, 2012 at 7:00 PM, Gleb Natapov <gleb@redhat.com> wrote:
> >> > On Sat, Dec 22, 2012 at 02:26:51PM +0200, Avi Kivity wrote:
> >> >> +
> >> >> +/*
> >> >> + * fastop functions have a special calling convention:
> >> >> + *
> >> >> + * dst:    [rdx]:rax  (in/out)
> >> > May be I miss something obvious but I do not see why rdx is here.
> >>
> >> MUL, DIV, CWD, etc.
> > Yes, but my question was more about actual fastop() implementation
> > which does not seams to handle rdx at all.
> 
> 
> It will be added later.  If you like, I can remove it from the comment for now.
No, I was just truing to make sure I am not missing something :)
Wouldn't we have to have separate fastop() function to handle rax/rdx
output instructions?

--
			Gleb.

Re: [PATCH 1/7] KVM: x86 emulator: framework for streamlining arithmetic opcodes

[Avi Kivity]

On Sat, Dec 22, 2012 at 8:11 PM, Gleb Natapov <gleb@redhat.com> wrote:
> On Sat, Dec 22, 2012 at 08:07:30PM +0200, Avi Kivity wrote:
>> On Sat, Dec 22, 2012 at 8:01 PM, Gleb Natapov <gleb@redhat.com> wrote:
>> > On Sat, Dec 22, 2012 at 07:42:55PM +0200, Avi Kivity wrote:
>> >> On Sat, Dec 22, 2012 at 7:00 PM, Gleb Natapov <gleb@redhat.com> wrote:
>> >> > On Sat, Dec 22, 2012 at 02:26:51PM +0200, Avi Kivity wrote:
>> >> >> +
>> >> >> +/*
>> >> >> + * fastop functions have a special calling convention:
>> >> >> + *
>> >> >> + * dst:    [rdx]:rax  (in/out)
>> >> > May be I miss something obvious but I do not see why rdx is here.
>> >>
>> >> MUL, DIV, CWD, etc.
>> > Yes, but my question was more about actual fastop() implementation
>> > which does not seams to handle rdx at all.
>>
>>
>> It will be added later.  If you like, I can remove it from the comment for now.
> No, I was just truing to make sure I am not missing something :)
> Wouldn't we have to have separate fastop() function to handle rax/rdx
> output instructions?

No.  As long as fastop() sets up and writes back rdx correctly (and
also #DE for DIV, IDIV), a single implementation works for everything.
 I don't want to add even more layers of indirect dispatch.

Re: [PATCH 1/7] KVM: x86 emulator: framework for streamlining arithmetic opcodes

[Gleb Natapov]

On Sat, Dec 22, 2012 at 08:18:14PM +0200, Avi Kivity wrote:
> On Sat, Dec 22, 2012 at 8:11 PM, Gleb Natapov <gleb@redhat.com> wrote:
> > On Sat, Dec 22, 2012 at 08:07:30PM +0200, Avi Kivity wrote:
> >> On Sat, Dec 22, 2012 at 8:01 PM, Gleb Natapov <gleb@redhat.com> wrote:
> >> > On Sat, Dec 22, 2012 at 07:42:55PM +0200, Avi Kivity wrote:
> >> >> On Sat, Dec 22, 2012 at 7:00 PM, Gleb Natapov <gleb@redhat.com> wrote:
> >> >> > On Sat, Dec 22, 2012 at 02:26:51PM +0200, Avi Kivity wrote:
> >> >> >> +
> >> >> >> +/*
> >> >> >> + * fastop functions have a special calling convention:
> >> >> >> + *
> >> >> >> + * dst:    [rdx]:rax  (in/out)
> >> >> > May be I miss something obvious but I do not see why rdx is here.
> >> >>
> >> >> MUL, DIV, CWD, etc.
> >> > Yes, but my question was more about actual fastop() implementation
> >> > which does not seams to handle rdx at all.
> >>
> >>
> >> It will be added later.  If you like, I can remove it from the comment for now.
> > No, I was just truing to make sure I am not missing something :)
> > Wouldn't we have to have separate fastop() function to handle rax/rdx
> > output instructions?
> 
> No.  As long as fastop() sets up and writes back rdx correctly (and
> also #DE for DIV, IDIV), a single implementation works for everything.
>  I don't want to add even more layers of indirect dispatch.

OK, so fastop() will always writeback RDX and handle exceptions, or you
want to mark instructions that needs this special handling with yet
another flag?

--
			Gleb.

Re: [PATCH 1/7] KVM: x86 emulator: framework for streamlining arithmetic opcodes

[Avi Kivity]

On Sat, Dec 22, 2012 at 8:44 PM, Gleb Natapov <gleb@redhat.com> wrote:
> On Sat, Dec 22, 2012 at 08:18:14PM +0200, Avi Kivity wrote:
>> > No, I was just truing to make sure I am not missing something :)
>> > Wouldn't we have to have separate fastop() function to handle rax/rdx
>> > output instructions?
>>
>> No.  As long as fastop() sets up and writes back rdx correctly (and
>> also #DE for DIV, IDIV), a single implementation works for everything.
>>  I don't want to add even more layers of indirect dispatch.
>
> OK, so fastop() will always writeback RDX and handle exceptions, or you
> want to mark instructions that needs this special handling with yet
> another flag?

Hopefully completely unconditional, but we'll see.

The problem is that if we pass rdx and another output parameter for
exceptions, we run out of registers on i386.  We can probably hack our
way though.

[PATCH 2/7] KVM: x86 emulator: Support for declaring single operand fastops

[Avi Kivity]

Signed-off-by: Avi Kivity <avi.kivity@gmail.com>
---
 arch/x86/kvm/emulate.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index cdf7b97..9859df6 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -24,6 +24,7 @@
 #include "kvm_cache_regs.h"
 #include <linux/module.h>
 #include <asm/kvm_emulate.h>
+#include <linux/stringify.h>
 
 #include "x86.h"
 #include "tss.h"
@@ -432,6 +433,30 @@ static void invalidate_registers(struct x86_emulate_ctxt *ctxt)
 		}							\
 	} while (0)
 
+#define FOP_ALIGN ".align " __stringify(FASTOP_SIZE) " \n\t"
+#define FOP_RET   "ret \n\t"
+
+#define FOP_START(op) \
+	extern void em_##op(struct fastop *fake); \
+	asm(".pushsection .text, \"ax\" \n\t" \
+	    ".global em_" #op " \n\t" \
+            FOP_ALIGN \
+	    "em_" #op ": \n\t"
+
+#define FOP_END \
+	    ".popsection")
+
+#define FOP1E(op,  dst) \
+	FOP_ALIGN #op " %" #dst " \n\t" FOP_RET
+
+#define FASTOP1(op) \
+	FOP_START(op) \
+	FOP1E(op##b, al) \
+	FOP1E(op##w, ax) \
+	FOP1E(op##l, eax) \
+	ON64(FOP1E(op##q, rax))	\
+	FOP_END
+
 #define __emulate_1op_rax_rdx(ctxt, _op, _suffix, _ex)			\
 	do {								\
 		unsigned long _tmp;					\
-- 
1.7.11.7

[PATCH 3/7] KVM: x86 emulator: introduce NoWrite flag

[Avi Kivity]

Instead of disabling writeback via OP_NONE, just specify NoWrite.

Signed-off-by: Avi Kivity <avi.kivity@gmail.com>
---
 arch/x86/kvm/emulate.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 9859df6..9dfbd07 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -150,6 +150,7 @@
 #define Unaligned   ((u64)1 << 42)  /* Explicitly unaligned (e.g. MOVDQU) */
 #define Avx         ((u64)1 << 43)  /* Advanced Vector Extensions */
 #define Fastop      ((u64)1 << 44)  /* Use opcode::u.fastop */
+#define NoWrite     ((u64)1 << 45)  /* No writeback */
 
 #define X2(x...) x, x
 #define X3(x...) X2(x), x
@@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt)
 {
 	int rc;
 
+	if (ctxt->d & NoWrite)
+		return X86EMUL_CONTINUE;
+
 	switch (ctxt->dst.type) {
 	case OP_REG:
 		write_register_operand(&ctxt->dst);
-- 
1.7.11.7

Re: [PATCH 3/7] KVM: x86 emulator: introduce NoWrite flag

[Gleb Natapov]

On Sat, Dec 22, 2012 at 02:26:53PM +0200, Avi Kivity wrote:
> Instead of disabling writeback via OP_NONE, just specify NoWrite.
> 
> Signed-off-by: Avi Kivity <avi.kivity@gmail.com>
> ---
>  arch/x86/kvm/emulate.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 9859df6..9dfbd07 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -150,6 +150,7 @@
>  #define Unaligned   ((u64)1 << 42)  /* Explicitly unaligned (e.g. MOVDQU) */
>  #define Avx         ((u64)1 << 43)  /* Advanced Vector Extensions */
>  #define Fastop      ((u64)1 << 44)  /* Use opcode::u.fastop */
> +#define NoWrite     ((u64)1 << 45)  /* No writeback */
>  
>  #define X2(x...) x, x
>  #define X3(x...) X2(x), x
> @@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt)
>  {
>  	int rc;
>  
> +	if (ctxt->d & NoWrite)
> +		return X86EMUL_CONTINUE;
> +
Why not make it Dst flag and set dst.type to OP_NONE during decoding
instead of this special case?

>  	switch (ctxt->dst.type) {
>  	case OP_REG:
>  		write_register_operand(&ctxt->dst);
> -- 
> 1.7.11.7

--
			Gleb.

Re: [PATCH 3/7] KVM: x86 emulator: introduce NoWrite flag

[Avi Kivity]

On Sat, Dec 22, 2012 at 5:11 PM, Gleb Natapov <gleb@redhat.com> wrote:
>>  #define X2(x...) x, x
>>  #define X3(x...) X2(x), x
>> @@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt)
>>  {
>>       int rc;
>>
>> +     if (ctxt->d & NoWrite)
>> +             return X86EMUL_CONTINUE;
>> +
> Why not make it Dst flag and set dst.type to OP_NONE during decoding
> instead of this special case?

My thinking was to eventually convert everything to NoWrite, then it's
not a special case.  But your approach works too, I can change it if
you like.  We'll have an if () either way though.

Re: [PATCH 3/7] KVM: x86 emulator: introduce NoWrite flag

[Gleb Natapov]

On Sat, Dec 22, 2012 at 05:28:00PM +0200, Avi Kivity wrote:
> On Sat, Dec 22, 2012 at 5:11 PM, Gleb Natapov <gleb@redhat.com> wrote:
> >>  #define X2(x...) x, x
> >>  #define X3(x...) X2(x), x
> >> @@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt)
> >>  {
> >>       int rc;
> >>
> >> +     if (ctxt->d & NoWrite)
> >> +             return X86EMUL_CONTINUE;
> >> +
> > Why not make it Dst flag and set dst.type to OP_NONE during decoding
> > instead of this special case?
> 
> My thinking was to eventually convert everything to NoWrite, then it's
> not a special case.  But your approach works too, I can change it if
> you like.  We'll have an if () either way though.
But meanwhile we will have two different ways to disable writeback. When
all instructions will be marked with NoWrite we can switch it easily if
we want. Also some instruction can disable writeback conditionally. cmov is one
(and may be only) of them.

--
			Gleb.

Re: [PATCH 3/7] KVM: x86 emulator: introduce NoWrite flag

[Avi Kivity]

On Sat, Dec 22, 2012 at 5:39 PM, Gleb Natapov <gleb@redhat.com> wrote:
> On Sat, Dec 22, 2012 at 05:28:00PM +0200, Avi Kivity wrote:
>> On Sat, Dec 22, 2012 at 5:11 PM, Gleb Natapov <gleb@redhat.com> wrote:
>> >>  #define X2(x...) x, x
>> >>  #define X3(x...) X2(x), x
>> >> @@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt)
>> >>  {
>> >>       int rc;
>> >>
>> >> +     if (ctxt->d & NoWrite)
>> >> +             return X86EMUL_CONTINUE;
>> >> +
>> > Why not make it Dst flag and set dst.type to OP_NONE during decoding
>> > instead of this special case?
>>
>> My thinking was to eventually convert everything to NoWrite, then it's
>> not a special case.  But your approach works too, I can change it if
>> you like.  We'll have an if () either way though.
> But meanwhile we will have two different ways to disable writeback. When
> all instructions will be marked with NoWrite we can switch it easily if
> we want. Also some instruction can disable writeback conditionally. cmov is one
> (and may be only) of them.

Makes sense, will update for the next iteration.

Re: [PATCH 3/7] KVM: x86 emulator: introduce NoWrite flag

[Avi Kivity]

On Sat, Dec 22, 2012 at 5:45 PM, Avi Kivity <avi.kivity@gmail.com> wrote:
> On Sat, Dec 22, 2012 at 5:39 PM, Gleb Natapov <gleb@redhat.com> wrote:
>> On Sat, Dec 22, 2012 at 05:28:00PM +0200, Avi Kivity wrote:
>>> On Sat, Dec 22, 2012 at 5:11 PM, Gleb Natapov <gleb@redhat.com> wrote:
>>> >>  #define X2(x...) x, x
>>> >>  #define X3(x...) X2(x), x
>>> >> @@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt)
>>> >>  {
>>> >>       int rc;
>>> >>
>>> >> +     if (ctxt->d & NoWrite)
>>> >> +             return X86EMUL_CONTINUE;
>>> >> +
>>> > Why not make it Dst flag and set dst.type to OP_NONE during decoding
>>> > instead of this special case?
>>>
>>> My thinking was to eventually convert everything to NoWrite, then it's
>>> not a special case.  But your approach works too, I can change it if
>>> you like.  We'll have an if () either way though.
>> But meanwhile we will have two different ways to disable writeback. When
>> all instructions will be marked with NoWrite we can switch it easily if
>> we want. Also some instruction can disable writeback conditionally. cmov is one
>> (and may be only) of them.
>
> Makes sense, will update for the next iteration.

Actually that doesn't work. x86_emulate_insn() needs dst.type in order
to fetch the operand.

Re: [PATCH 3/7] KVM: x86 emulator: introduce NoWrite flag

[Gleb Natapov]

On Sat, Dec 22, 2012 at 05:51:08PM +0200, Avi Kivity wrote:
> On Sat, Dec 22, 2012 at 5:45 PM, Avi Kivity <avi.kivity@gmail.com> wrote:
> > On Sat, Dec 22, 2012 at 5:39 PM, Gleb Natapov <gleb@redhat.com> wrote:
> >> On Sat, Dec 22, 2012 at 05:28:00PM +0200, Avi Kivity wrote:
> >>> On Sat, Dec 22, 2012 at 5:11 PM, Gleb Natapov <gleb@redhat.com> wrote:
> >>> >>  #define X2(x...) x, x
> >>> >>  #define X3(x...) X2(x), x
> >>> >> @@ -1584,6 +1585,9 @@ static int writeback(struct x86_emulate_ctxt *ctxt)
> >>> >>  {
> >>> >>       int rc;
> >>> >>
> >>> >> +     if (ctxt->d & NoWrite)
> >>> >> +             return X86EMUL_CONTINUE;
> >>> >> +
> >>> > Why not make it Dst flag and set dst.type to OP_NONE during decoding
> >>> > instead of this special case?
> >>>
> >>> My thinking was to eventually convert everything to NoWrite, then it's
> >>> not a special case.  But your approach works too, I can change it if
> >>> you like.  We'll have an if () either way though.
> >> But meanwhile we will have two different ways to disable writeback. When
> >> all instructions will be marked with NoWrite we can switch it easily if
> >> we want. Also some instruction can disable writeback conditionally. cmov is one
> >> (and may be only) of them.
> >
> > Makes sense, will update for the next iteration.
> 
> Actually that doesn't work. x86_emulate_insn() needs dst.type in order
> to fetch the operand.
True :(. Actually if we will move from OP_NONE to NoWrite we can drop
saved_dst_type hack from x86_emulate_insn.

--
			Gleb.

[PATCH 4/7] KVM: x86 emulator: mark CMP, CMPS, SCAS, TEST as NoWrite

[Avi Kivity]

Signed-off-by: Avi Kivity <avi.kivity@gmail.com>
---
 arch/x86/kvm/emulate.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 9dfbd07..58cccb3 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3020,16 +3020,12 @@ static int em_xor(struct x86_emulate_ctxt *ctxt)
 static int em_cmp(struct x86_emulate_ctxt *ctxt)
 {
 	emulate_2op_SrcV(ctxt, "cmp");
-	/* Disable writeback. */
-	ctxt->dst.type = OP_NONE;
 	return X86EMUL_CONTINUE;
 }
 
 static int em_test(struct x86_emulate_ctxt *ctxt)
 {
 	emulate_2op_SrcV(ctxt, "test");
-	/* Disable writeback. */
-	ctxt->dst.type = OP_NONE;
 	return X86EMUL_CONTINUE;
 }
 
@@ -3697,7 +3693,7 @@ static const struct opcode group1[] = {
 	I(Lock | PageTable, em_and),
 	I(Lock, em_sub),
 	I(Lock, em_xor),
-	I(0, em_cmp),
+	I(NoWrite, em_cmp),
 };
 
 static const struct opcode group1A[] = {
@@ -3705,8 +3701,8 @@ static const struct opcode group1A[] = {
 };
 
 static const struct opcode group3[] = {
-	I(DstMem | SrcImm, em_test),
-	I(DstMem | SrcImm, em_test),
+	I(DstMem | SrcImm | NoWrite, em_test),
+	I(DstMem | SrcImm | NoWrite, em_test),
 	I(DstMem | SrcNone | Lock, em_not),
 	I(DstMem | SrcNone | Lock, em_neg),
 	I(SrcMem, em_mul_ex),
@@ -3807,7 +3803,7 @@ static const struct opcode opcode_table[256] = {
 	/* 0x30 - 0x37 */
 	I6ALU(Lock, em_xor), N, N,
 	/* 0x38 - 0x3F */
-	I6ALU(0, em_cmp), N, N,
+	I6ALU(NoWrite, em_cmp), N, N,
 	/* 0x40 - 0x4F */
 	X16(D(DstReg)),
 	/* 0x50 - 0x57 */
@@ -3833,7 +3829,7 @@ static const struct opcode opcode_table[256] = {
 	G(DstMem | SrcImm, group1),
 	G(ByteOp | DstMem | SrcImm | No64, group1),
 	G(DstMem | SrcImmByte, group1),
-	I2bv(DstMem | SrcReg | ModRM, em_test),
+	I2bv(DstMem | SrcReg | ModRM | NoWrite, em_test),
 	I2bv(DstMem | SrcReg | ModRM | Lock | PageTable, em_xchg),
 	/* 0x88 - 0x8F */
 	I2bv(DstMem | SrcReg | ModRM | Mov | PageTable, em_mov),
@@ -3853,12 +3849,12 @@ static const struct opcode opcode_table[256] = {
 	I2bv(DstAcc | SrcMem | Mov | MemAbs, em_mov),
 	I2bv(DstMem | SrcAcc | Mov | MemAbs | PageTable, em_mov),
 	I2bv(SrcSI | DstDI | Mov | String, em_mov),
-	I2bv(SrcSI | DstDI | String, em_cmp),
+	I2bv(SrcSI | DstDI | String | NoWrite, em_cmp),
 	/* 0xA8 - 0xAF */
-	I2bv(DstAcc | SrcImm, em_test),
+	I2bv(DstAcc | SrcImm | NoWrite, em_test),
 	I2bv(SrcAcc | DstDI | Mov | String, em_mov),
 	I2bv(SrcSI | DstAcc | Mov | String, em_mov),
-	I2bv(SrcAcc | DstDI | String, em_cmp),
+	I2bv(SrcAcc | DstDI | String | NoWrite, em_cmp),
 	/* 0xB0 - 0xB7 */
 	X8(I(ByteOp | DstReg | SrcImm | Mov, em_mov)),
 	/* 0xB8 - 0xBF */
-- 
1.7.11.7

[PATCH 5/7] KVM: x86 emulator: convert NOT, NEG to fastop

[Avi Kivity]

Signed-off-by: Avi Kivity <avi.kivity@gmail.com>
---
 arch/x86/kvm/emulate.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 58cccb3..88579fa 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2001,17 +2001,8 @@ static int em_grp2(struct x86_emulate_ctxt *ctxt)
 	return X86EMUL_CONTINUE;
 }
 
-static int em_not(struct x86_emulate_ctxt *ctxt)
-{
-	ctxt->dst.val = ~ctxt->dst.val;
-	return X86EMUL_CONTINUE;
-}
-
-static int em_neg(struct x86_emulate_ctxt *ctxt)
-{
-	emulate_1op(ctxt, "neg");
-	return X86EMUL_CONTINUE;
-}
+FASTOP1(not);
+FASTOP1(neg);
 
 static int em_mul_ex(struct x86_emulate_ctxt *ctxt)
 {
@@ -3703,8 +3694,8 @@ static const struct opcode group1A[] = {
 static const struct opcode group3[] = {
 	I(DstMem | SrcImm | NoWrite, em_test),
 	I(DstMem | SrcImm | NoWrite, em_test),
-	I(DstMem | SrcNone | Lock, em_not),
-	I(DstMem | SrcNone | Lock, em_neg),
+	F(DstMem | SrcNone | Lock, em_not),
+	F(DstMem | SrcNone | Lock, em_neg),
 	I(SrcMem, em_mul_ex),
 	I(SrcMem, em_imul_ex),
 	I(SrcMem, em_div_ex),
-- 
1.7.11.7

[PATCH 6/7] KVM: x86 emulator: add macros for defining 2-operand fastop emulation

[Avi Kivity]

Signed-off-by: Avi Kivity <avi.kivity@gmail.com>
---
 arch/x86/kvm/emulate.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 88579fa..8f650d7 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -458,6 +458,17 @@ static void invalidate_registers(struct x86_emulate_ctxt *ctxt)
 	ON64(FOP1E(op##q, rax))	\
 	FOP_END
 
+#define FOP2E(op,  dst, src)	   \
+	FOP_ALIGN #op " %" #dst ", %" #src " \n\t" FOP_RET
+
+#define FASTOP2(op) \
+	FOP_START(op) \
+	FOP2E(op##b, al, bl) \
+	FOP2E(op##w, ax, bx) \
+	FOP2E(op##l, eax, ebx) \
+	ON64(FOP2E(op##q, rax, rbx)) \
+	FOP_END
+
 #define __emulate_1op_rax_rdx(ctxt, _op, _suffix, _ex)			\
 	do {								\
 		unsigned long _tmp;					\
@@ -3646,6 +3657,7 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
 #define D2bv(_f)      D((_f) | ByteOp), D(_f)
 #define D2bvIP(_f, _i, _p) DIP((_f) | ByteOp, _i, _p), DIP(_f, _i, _p)
 #define I2bv(_f, _e)  I((_f) | ByteOp, _e), I(_f, _e)
+#define F2bv(_f, _e)  F((_f) | ByteOp, _e), F(_f, _e)
 #define I2bvIP(_f, _e, _i, _p) \
 	IIP((_f) | ByteOp, _e, _i, _p), IIP(_f, _e, _i, _p)
 
-- 
1.7.11.7

[PATCH 7/7] KVM: x86 emulator: convert basic ALU ops to fastop

[Avi Kivity]

Opcodes:
	TEST
	CMP
	ADD
	ADC
	SUB
	SBB
	XOR
	OR
	AND

Signed-off-by: Avi Kivity <avi.kivity@gmail.com>
---
 arch/x86/kvm/emulate.c | 112 +++++++++++++++----------------------------------
 1 file changed, 34 insertions(+), 78 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 8f650d7..cfae6ca 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2977,59 +2977,15 @@ static int em_ret_near_imm(struct x86_emulate_ctxt *ctxt)
 	return X86EMUL_CONTINUE;
 }
 
-static int em_add(struct x86_emulate_ctxt *ctxt)
-{
-	emulate_2op_SrcV(ctxt, "add");
-	return X86EMUL_CONTINUE;
-}
-
-static int em_or(struct x86_emulate_ctxt *ctxt)
-{
-	emulate_2op_SrcV(ctxt, "or");
-	return X86EMUL_CONTINUE;
-}
-
-static int em_adc(struct x86_emulate_ctxt *ctxt)
-{
-	emulate_2op_SrcV(ctxt, "adc");
-	return X86EMUL_CONTINUE;
-}
-
-static int em_sbb(struct x86_emulate_ctxt *ctxt)
-{
-	emulate_2op_SrcV(ctxt, "sbb");
-	return X86EMUL_CONTINUE;
-}
-
-static int em_and(struct x86_emulate_ctxt *ctxt)
-{
-	emulate_2op_SrcV(ctxt, "and");
-	return X86EMUL_CONTINUE;
-}
-
-static int em_sub(struct x86_emulate_ctxt *ctxt)
-{
-	emulate_2op_SrcV(ctxt, "sub");
-	return X86EMUL_CONTINUE;
-}
-
-static int em_xor(struct x86_emulate_ctxt *ctxt)
-{
-	emulate_2op_SrcV(ctxt, "xor");
-	return X86EMUL_CONTINUE;
-}
-
-static int em_cmp(struct x86_emulate_ctxt *ctxt)
-{
-	emulate_2op_SrcV(ctxt, "cmp");
-	return X86EMUL_CONTINUE;
-}
-
-static int em_test(struct x86_emulate_ctxt *ctxt)
-{
-	emulate_2op_SrcV(ctxt, "test");
-	return X86EMUL_CONTINUE;
-}
+FASTOP2(add);
+FASTOP2(or);
+FASTOP2(adc);
+FASTOP2(sbb);
+FASTOP2(and);
+FASTOP2(sub);
+FASTOP2(xor);
+FASTOP2(cmp);
+FASTOP2(test);
 
 static int em_xchg(struct x86_emulate_ctxt *ctxt)
 {
@@ -3661,9 +3617,9 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
 #define I2bvIP(_f, _e, _i, _p) \
 	IIP((_f) | ByteOp, _e, _i, _p), IIP(_f, _e, _i, _p)
 
-#define I6ALU(_f, _e) I2bv((_f) | DstMem | SrcReg | ModRM, _e),		\
-		I2bv(((_f) | DstReg | SrcMem | ModRM) & ~Lock, _e),	\
-		I2bv(((_f) & ~Lock) | DstAcc | SrcImm, _e)
+#define F6ALU(_f, _e) F2bv((_f) | DstMem | SrcReg | ModRM, _e),		\
+		F2bv(((_f) | DstReg | SrcMem | ModRM) & ~Lock, _e),	\
+		F2bv(((_f) & ~Lock) | DstAcc | SrcImm, _e)
 
 static const struct opcode group7_rm1[] = {
 	DI(SrcNone | Priv, monitor),
@@ -3689,14 +3645,14 @@ static const struct opcode group7_rm7[] = {
 };
 
 static const struct opcode group1[] = {
-	I(Lock, em_add),
-	I(Lock | PageTable, em_or),
-	I(Lock, em_adc),
-	I(Lock, em_sbb),
-	I(Lock | PageTable, em_and),
-	I(Lock, em_sub),
-	I(Lock, em_xor),
-	I(NoWrite, em_cmp),
+	F(Lock, em_add),
+	F(Lock | PageTable, em_or),
+	F(Lock, em_adc),
+	F(Lock, em_sbb),
+	F(Lock | PageTable, em_and),
+	F(Lock, em_sub),
+	F(Lock, em_xor),
+	F(NoWrite, em_cmp),
 };
 
 static const struct opcode group1A[] = {
@@ -3704,8 +3660,8 @@ static const struct opcode group1A[] = {
 };
 
 static const struct opcode group3[] = {
-	I(DstMem | SrcImm | NoWrite, em_test),
-	I(DstMem | SrcImm | NoWrite, em_test),
+	F(DstMem | SrcImm | NoWrite, em_test),
+	F(DstMem | SrcImm | NoWrite, em_test),
 	F(DstMem | SrcNone | Lock, em_not),
 	F(DstMem | SrcNone | Lock, em_neg),
 	I(SrcMem, em_mul_ex),
@@ -3784,29 +3740,29 @@ static const struct gprefix pfx_vmovntpx = {
 
 static const struct opcode opcode_table[256] = {
 	/* 0x00 - 0x07 */
-	I6ALU(Lock, em_add),
+	F6ALU(Lock, em_add),
 	I(ImplicitOps | Stack | No64 | Src2ES, em_push_sreg),
 	I(ImplicitOps | Stack | No64 | Src2ES, em_pop_sreg),
 	/* 0x08 - 0x0F */
-	I6ALU(Lock | PageTable, em_or),
+	F6ALU(Lock | PageTable, em_or),
 	I(ImplicitOps | Stack | No64 | Src2CS, em_push_sreg),
 	N,
 	/* 0x10 - 0x17 */
-	I6ALU(Lock, em_adc),
+	F6ALU(Lock, em_adc),
 	I(ImplicitOps | Stack | No64 | Src2SS, em_push_sreg),
 	I(ImplicitOps | Stack | No64 | Src2SS, em_pop_sreg),
 	/* 0x18 - 0x1F */
-	I6ALU(Lock, em_sbb),
+	F6ALU(Lock, em_sbb),
 	I(ImplicitOps | Stack | No64 | Src2DS, em_push_sreg),
 	I(ImplicitOps | Stack | No64 | Src2DS, em_pop_sreg),
 	/* 0x20 - 0x27 */
-	I6ALU(Lock | PageTable, em_and), N, N,
+	F6ALU(Lock | PageTable, em_and), N, N,
 	/* 0x28 - 0x2F */
-	I6ALU(Lock, em_sub), N, I(ByteOp | DstAcc | No64, em_das),
+	F6ALU(Lock, em_sub), N, I(ByteOp | DstAcc | No64, em_das),
 	/* 0x30 - 0x37 */
-	I6ALU(Lock, em_xor), N, N,
+	F6ALU(Lock, em_xor), N, N,
 	/* 0x38 - 0x3F */
-	I6ALU(NoWrite, em_cmp), N, N,
+	F6ALU(NoWrite, em_cmp), N, N,
 	/* 0x40 - 0x4F */
 	X16(D(DstReg)),
 	/* 0x50 - 0x57 */
@@ -3832,7 +3788,7 @@ static const struct opcode opcode_table[256] = {
 	G(DstMem | SrcImm, group1),
 	G(ByteOp | DstMem | SrcImm | No64, group1),
 	G(DstMem | SrcImmByte, group1),
-	I2bv(DstMem | SrcReg | ModRM | NoWrite, em_test),
+	F2bv(DstMem | SrcReg | ModRM | NoWrite, em_test),
 	I2bv(DstMem | SrcReg | ModRM | Lock | PageTable, em_xchg),
 	/* 0x88 - 0x8F */
 	I2bv(DstMem | SrcReg | ModRM | Mov | PageTable, em_mov),
@@ -3852,12 +3808,12 @@ static const struct opcode opcode_table[256] = {
 	I2bv(DstAcc | SrcMem | Mov | MemAbs, em_mov),
 	I2bv(DstMem | SrcAcc | Mov | MemAbs | PageTable, em_mov),
 	I2bv(SrcSI | DstDI | Mov | String, em_mov),
-	I2bv(SrcSI | DstDI | String | NoWrite, em_cmp),
+	F2bv(SrcSI | DstDI | String | NoWrite, em_cmp),
 	/* 0xA8 - 0xAF */
-	I2bv(DstAcc | SrcImm | NoWrite, em_test),
+	F2bv(DstAcc | SrcImm | NoWrite, em_test),
 	I2bv(SrcAcc | DstDI | Mov | String, em_mov),
 	I2bv(SrcSI | DstAcc | Mov | String, em_mov),
-	I2bv(SrcAcc | DstDI | String | NoWrite, em_cmp),
+	F2bv(SrcAcc | DstDI | String | NoWrite, em_cmp),
 	/* 0xB0 - 0xB7 */
 	X8(I(ByteOp | DstReg | SrcImm | Mov, em_mov)),
 	/* 0xB8 - 0xBF */
-- 
1.7.11.7

Re: [PATCH 0/7] Streamline arithmetic instruction emulation

[Gleb Natapov]

On Sat, Dec 22, 2012 at 02:26:50PM +0200, Avi Kivity wrote:
> The current arithmetic instruction emulation is fairly clumsy: after
> decode, each instruction gets a switch (size), and for every size
> we fetch the operands, prepare flags, emulate the instruction, then store
> back the flags and operands.
> 
> This patchset simplifies things by moving everything into common code
> except the instruction itself.  All the pre- and post- processing is
> coded just once.  The per-instrution code looks like:
> 
>   add %bl, %al
>   ret
> 
>   add %bx, %ax
>   ret
> 
>   add %ebx, %eax
>   ret
> 
>   add %rbx, %rax
>   ret
> 
> The savings in size, for the ten instructions converted in this patchset,
> are fairly large:
> 
>    text	   data	    bss	    dec	    hex	filename
>   63724	      0	      0	  63724	   f8ec	arch/x86/kvm/emulate.o.before
>   61268	      0	      0	  61268	   ef54	arch/x86/kvm/emulate.o.after
> 
> - around 2500 bytes.
> 
> Avi Kivity (7):
>   KVM: x86 emulator: framework for streamlining arithmetic opcodes
>   KVM: x86 emulator: Support for declaring single operand fastops
>   KVM: x86 emulator: introduce NoWrite flag
>   KVM: x86 emulator: mark CMP, CMPS, SCAS, TEST as NoWrite
>   KVM: x86 emulator: convert NOT, NEG to fastop
>   KVM: x86 emulator: add macros for defining 2-operand fastop emulation
>   KVM: x86 emulator: convert basic ALU ops to fastop
> 
>  arch/x86/kvm/emulate.c | 215 +++++++++++++++++++++++++++----------------------
>  1 file changed, 120 insertions(+), 95 deletions(-)
> 
Looks good to me.

--
			Gleb.

Re: [PATCH 0/7] Streamline arithmetic instruction emulation

[Marcelo Tosatti]

On Thu, Jan 03, 2013 at 09:39:14PM +0200, Avi Kivity wrote:
> > > Avi Kivity (7):
> > >   KVM: x86 emulator: framework for streamlining arithmetic opcodes
> > >   KVM: x86 emulator: Support for declaring single operand fastops
> > >   KVM: x86 emulator: introduce NoWrite flag
> > >   KVM: x86 emulator: mark CMP, CMPS, SCAS, TEST as NoWrite
> > >   KVM: x86 emulator: convert NOT, NEG to fastop
> > >   KVM: x86 emulator: add macros for defining 2-operand fastop emulation
> > >   KVM: x86 emulator: convert basic ALU ops to fastop
> > >
> > >  arch/x86/kvm/emulate.c | 215
> +++++++++++++++++++++++++++----------------------
> > >  1 file changed, 120 insertions(+), 95 deletions(-)
> > >
> > Looks good to me.
> >
> Ping?

Please regenerate against 'queue' branch.