[PATCH 00/14] Emulator decode generalization

[PATCH 00/14] Emulator decode generalization

[Avi Kivity]

- merge dst/src/src2 decode
- generalize %seg embedded in opcode decode

plus a fix.

Avi Kivity (14):
  KVM: x86 emulator: fix Src2CL decode
  KVM: x86 emulator: convert group 3 instructions to direct decode
  KVM: x86 emulator: move memop, memopp into emulation context
  KVM: x86 emulator: split dst decode to a generic decode_operand()
  KVM: x86 emulator: expand decode flags to 64 bits
  KVM: x86 emulator: switch src2 to generic decode_operand()
  KVM: x86 emulator: free up some flag bits near src, dst
  KVM: x86 emulator: switch OpImmUByte decode to decode_imm()
  KVM: x86 emulator: qualify OpReg inhibit_byte_regs hack
  KVM: x86 emulator: switch src decode to decode_operand()
  KVM: x86 emulator: simplify OpMem64 decode
  KVM: x86 emulator: streamline decode of segment registers
  KVM: x86 emulator: switch lds/les/lss/lfs/lgs to direct decode
  KVM: x86 emulator: convert push %sreg/pop %sreg to direct decode

 arch/x86/include/asm/kvm_emulate.h |    4 +-
 arch/x86/kvm/emulate.c             |  563 ++++++++++++++++++------------------
 2 files changed, 286 insertions(+), 281 deletions(-)

-- 
1.7.6.1

[PATCH 01/14] KVM: x86 emulator: fix Src2CL decode

[Avi Kivity]

Src2CL decode (used for double width shifts) erronously decodes only bit 3
of %rcx, instead of bits 7:0.

Fix by decoding %cl in its entirety.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index c37f67e..af06539 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3567,7 +3567,7 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
 		break;
 	case Src2CL:
 		ctxt->src2.bytes = 1;
-		ctxt->src2.val = ctxt->regs[VCPU_REGS_RCX] & 0x8;
+		ctxt->src2.val = ctxt->regs[VCPU_REGS_RCX] & 0xff;
 		break;
 	case Src2ImmByte:
 		rc = decode_imm(ctxt, &ctxt->src2, 1, true);
-- 
1.7.6.1

[PATCH 02/14] KVM: x86 emulator: convert group 3 instructions to direct decode

[Avi Kivity]

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |   82 ++++++++++++++++++++++++++++--------------------
 1 files changed, 48 insertions(+), 34 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index af06539..ed819bd 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1663,37 +1663,49 @@ static int em_grp2(struct x86_emulate_ctxt *ctxt)
 	return X86EMUL_CONTINUE;
 }
 
-static int em_grp3(struct x86_emulate_ctxt *ctxt)
+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;
+}
+
+static int em_mul_ex(struct x86_emulate_ctxt *ctxt)
+{
+	u8 ex = 0;
+
+	emulate_1op_rax_rdx(ctxt, "mul", ex);
+	return X86EMUL_CONTINUE;
+}
+
+static int em_imul_ex(struct x86_emulate_ctxt *ctxt)
+{
+	u8 ex = 0;
+
+	emulate_1op_rax_rdx(ctxt, "imul", ex);
+	return X86EMUL_CONTINUE;
+}
+
+static int em_div_ex(struct x86_emulate_ctxt *ctxt)
 {
 	u8 de = 0;
 
-	switch (ctxt->modrm_reg) {
-	case 0 ... 1:	/* test */
-		emulate_2op_SrcV(ctxt, "test");
-		/* Disable writeback. */
-		ctxt->dst.type = OP_NONE;
-		break;
-	case 2:	/* not */
-		ctxt->dst.val = ~ctxt->dst.val;
-		break;
-	case 3:	/* neg */
-		emulate_1op(ctxt, "neg");
-		break;
-	case 4: /* mul */
-		emulate_1op_rax_rdx(ctxt, "mul", de);
-		break;
-	case 5: /* imul */
-		emulate_1op_rax_rdx(ctxt, "imul", de);
-		break;
-	case 6: /* div */
-		emulate_1op_rax_rdx(ctxt, "div", de);
-		break;
-	case 7: /* idiv */
-		emulate_1op_rax_rdx(ctxt, "idiv", de);
-		break;
-	default:
-		return X86EMUL_UNHANDLEABLE;
-	}
+	emulate_1op_rax_rdx(ctxt, "div", de);
+	if (de)
+		return emulate_de(ctxt);
+	return X86EMUL_CONTINUE;
+}
+
+static int em_idiv_ex(struct x86_emulate_ctxt *ctxt)
+{
+	u8 de = 0;
+
+	emulate_1op_rax_rdx(ctxt, "idiv", de);
 	if (de)
 		return emulate_de(ctxt);
 	return X86EMUL_CONTINUE;
@@ -2989,9 +3001,14 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
 };
 
 static struct opcode group3[] = {
-	D(DstMem | SrcImm | ModRM), D(DstMem | SrcImm | ModRM),
-	D(DstMem | SrcNone | ModRM | Lock), D(DstMem | SrcNone | ModRM | Lock),
-	X4(D(SrcMem | ModRM)),
+	I(DstMem | SrcImm | ModRM, em_test),
+	I(DstMem | SrcImm | ModRM, em_test),
+	I(DstMem | SrcNone | ModRM | Lock, em_not),
+	I(DstMem | SrcNone | ModRM | Lock, em_neg),
+	I(SrcMem | ModRM, em_mul_ex),
+	I(SrcMem | ModRM, em_imul_ex),
+	I(SrcMem | ModRM, em_div_ex),
+	I(SrcMem | ModRM, em_idiv_ex),
 };
 
 static struct opcode group4[] = {
@@ -3917,9 +3934,6 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 		/* complement carry flag from eflags reg */
 		ctxt->eflags ^= EFLG_CF;
 		break;
-	case 0xf6 ... 0xf7:	/* Grp3 */
-		rc = em_grp3(ctxt);
-		break;
 	case 0xf8: /* clc */
 		ctxt->eflags &= ~EFLG_CF;
 		break;
-- 
1.7.6.1

[PATCH 03/14] KVM: x86 emulator: move memop, memopp into emulation context

[Avi Kivity]

Simplifies further generalization of decode.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/include/asm/kvm_emulate.h |    2 ++
 arch/x86/kvm/emulate.c             |   34 +++++++++++++++++-----------------
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index 6040d11..56bac3e 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -275,6 +275,8 @@ struct x86_emulate_ctxt {
 	unsigned long _eip;
 	/* Fields above regs are cleared together. */
 	unsigned long regs[NR_VCPU_REGS];
+	struct operand memop;
+	struct operand *memopp;
 	struct fetch_cache fetch;
 	struct read_cache io_read;
 	struct read_cache mem_read;
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index ed819bd..58172fb 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3323,8 +3323,9 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
 	int def_op_bytes, def_ad_bytes, goffset, simd_prefix;
 	bool op_prefix = false;
 	struct opcode opcode;
-	struct operand memop = { .type = OP_NONE }, *memopp = NULL;
 
+	ctxt->memop.type = OP_NONE;
+	ctxt->memopp = NULL;
 	ctxt->_eip = ctxt->eip;
 	ctxt->fetch.start = ctxt->_eip;
 	ctxt->fetch.end = ctxt->fetch.start + insn_len;
@@ -3482,21 +3483,21 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
 
 	/* ModRM and SIB bytes. */
 	if (ctxt->d & ModRM) {
-		rc = decode_modrm(ctxt, &memop);
+		rc = decode_modrm(ctxt, &ctxt->memop);
 		if (!ctxt->has_seg_override)
 			set_seg_override(ctxt, ctxt->modrm_seg);
 	} else if (ctxt->d & MemAbs)
-		rc = decode_abs(ctxt, &memop);
+		rc = decode_abs(ctxt, &ctxt->memop);
 	if (rc != X86EMUL_CONTINUE)
 		goto done;
 
 	if (!ctxt->has_seg_override)
 		set_seg_override(ctxt, VCPU_SREG_DS);
 
-	memop.addr.mem.seg = seg_override(ctxt);
+	ctxt->memop.addr.mem.seg = seg_override(ctxt);
 
-	if (memop.type == OP_MEM && ctxt->ad_bytes != 8)
-		memop.addr.mem.ea = (u32)memop.addr.mem.ea;
+	if (ctxt->memop.type == OP_MEM && ctxt->ad_bytes != 8)
+		ctxt->memop.addr.mem.ea = (u32)ctxt->memop.addr.mem.ea;
 
 	/*
 	 * Decode and fetch the source operand: register, memory
@@ -3509,17 +3510,16 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
 		decode_register_operand(ctxt, &ctxt->src, 0);
 		break;
 	case SrcMem16:
-		memop.bytes = 2;
+		ctxt->memop.bytes = 2;
 		goto srcmem_common;
 	case SrcMem32:
-		memop.bytes = 4;
+		ctxt->memop.bytes = 4;
 		goto srcmem_common;
 	case SrcMem:
-		memop.bytes = (ctxt->d & ByteOp) ? 1 :
-							   ctxt->op_bytes;
+		ctxt->memop.bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
 	srcmem_common:
-		ctxt->src = memop;
-		memopp = &ctxt->src;
+		ctxt->src = ctxt->memop;
+		ctxt->memopp = &ctxt->src;
 		break;
 	case SrcImmU16:
 		rc = decode_imm(ctxt, &ctxt->src, 2, false);
@@ -3561,7 +3561,7 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
 		insn_fetch_arr(ctxt->src.valptr, ctxt->src.bytes, ctxt);
 		break;
 	case SrcMemFAddr:
-		memop.bytes = ctxt->op_bytes + 2;
+		ctxt->memop.bytes = ctxt->op_bytes + 2;
 		goto srcmem_common;
 		break;
 	case SrcDX:
@@ -3615,8 +3615,8 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
 		break;
 	case DstMem:
 	case DstMem64:
-		ctxt->dst = memop;
-		memopp = &ctxt->dst;
+		ctxt->dst = ctxt->memop;
+		ctxt->memopp = &ctxt->dst;
 		if ((ctxt->d & DstMask) == DstMem64)
 			ctxt->dst.bytes = 8;
 		else
@@ -3654,8 +3654,8 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
 	}
 
 done:
-	if (memopp && memopp->type == OP_MEM && ctxt->rip_relative)
-		memopp->addr.mem.ea += ctxt->_eip;
+	if (ctxt->memopp && ctxt->memopp->type == OP_MEM && ctxt->rip_relative)
+		ctxt->memopp->addr.mem.ea += ctxt->_eip;
 
 	return (rc != X86EMUL_CONTINUE) ? EMULATION_FAILED : EMULATION_OK;
 }
-- 
1.7.6.1

Re: [PATCH 03/14] KVM: x86 emulator: move memop, memopp into emulation context

[Takuya Yoshikawa]

On Tue, 13 Sep 2011 10:45:40 +0300
Avi Kivity <avi@redhat.com> wrote:

> Simplifies further generalization of decode.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/include/asm/kvm_emulate.h |    2 ++
>  arch/x86/kvm/emulate.c             |   34 +++++++++++++++++-----------------
>  2 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
> index 6040d11..56bac3e 100644
> --- a/arch/x86/include/asm/kvm_emulate.h
> +++ b/arch/x86/include/asm/kvm_emulate.h
> @@ -275,6 +275,8 @@ struct x86_emulate_ctxt {
>  	unsigned long _eip;
>  	/* Fields above regs are cleared together. */
>  	unsigned long regs[NR_VCPU_REGS];
> +	struct operand memop;
> +	struct operand *memopp;
>  	struct fetch_cache fetch;
>  	struct read_cache io_read;
>  	struct read_cache mem_read;

Once the emulator context gets stablized, some comments will be nice
to know which ones are supposed to be accessed from outside of the
emulator, and which ones are only for the emulator internal usage.

Practically, knowing each member's lifetime, decode stage only or
emulation stage only or throughout the emulation, will make it easy
to avoid extra ctxt/regs initialization and ... maybe more.

	Takuya

Re: [PATCH 03/14] KVM: x86 emulator: move memop, memopp into emulation context

[Avi Kivity]

On 09/14/2011 01:41 PM, Takuya Yoshikawa wrote:
> On Tue, 13 Sep 2011 10:45:40 +0300
> Avi Kivity<avi@redhat.com>  wrote:
>
> >  Simplifies further generalization of decode.
> >
> >  Signed-off-by: Avi Kivity<avi@redhat.com>
> >  ---
> >   arch/x86/include/asm/kvm_emulate.h |    2 ++
> >   arch/x86/kvm/emulate.c             |   34 +++++++++++++++++-----------------
> >   2 files changed, 19 insertions(+), 17 deletions(-)
> >
> >  diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
> >  index 6040d11..56bac3e 100644
> >  --- a/arch/x86/include/asm/kvm_emulate.h
> >  +++ b/arch/x86/include/asm/kvm_emulate.h
> >  @@ -275,6 +275,8 @@ struct x86_emulate_ctxt {
> >   	unsigned long _eip;
> >   	/* Fields above regs are cleared together. */
> >   	unsigned long regs[NR_VCPU_REGS];
> >  +	struct operand memop;
> >  +	struct operand *memopp;
> >   	struct fetch_cache fetch;
> >   	struct read_cache io_read;
> >   	struct read_cache mem_read;
>
> Once the emulator context gets stablized, some comments will be nice
> to know which ones are supposed to be accessed from outside of the
> emulator, and which ones are only for the emulator internal usage.
>
> Practically, knowing each member's lifetime, decode stage only or
> emulation stage only or throughout the emulation, will make it easy
> to avoid extra ctxt/regs initialization and ... maybe more.
>

Nothing should be accessed from outside the emulator, except via 
accessors.  We should move initialization to the emulator as well (or 
just initialize from x86_decode_insn() - any reason not to?)

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

Re: [PATCH 03/14] KVM: x86 emulator: move memop, memopp into emulation context

[Takuya Yoshikawa]

On Wed, 14 Sep 2011 14:37:21 +0300
Avi Kivity <avi@redhat.com> wrote:

> > Once the emulator context gets stablized, some comments will be nice
> > to know which ones are supposed to be accessed from outside of the
> > emulator, and which ones are only for the emulator internal usage.
> >
> > Practically, knowing each member's lifetime, decode stage only or
> > emulation stage only or throughout the emulation, will make it easy
> > to avoid extra ctxt/regs initialization and ... maybe more.
> >
> 
> Nothing should be accessed from outside the emulator, except via 
> accessors.  We should move initialization to the emulator as well (or 
> just initialize from x86_decode_insn() - any reason not to?)

Not big reason but kvm_inject_realmode_interrupt() and kvm_task_switch()
call emulate_int_real() and emulator_task_switch() respectively without
doing generic decoding.

So at least, we need some special initialization for them if we move
init_emulate_ctxt() into x86_decode_insn().

	Takuya

Re: [PATCH 03/14] KVM: x86 emulator: move memop, memopp into emulation context

[Avi Kivity]

On 09/14/2011 06:21 PM, Takuya Yoshikawa wrote:
> >
> >  Nothing should be accessed from outside the emulator, except via
> >  accessors.  We should move initialization to the emulator as well (or
> >  just initialize from x86_decode_insn() - any reason not to?)
>
> Not big reason but kvm_inject_realmode_interrupt() and kvm_task_switch()
> call emulate_int_real() and emulator_task_switch() respectively without
> doing generic decoding.
>
> So at least, we need some special initialization for them if we move
> init_emulate_ctxt() into x86_decode_insn().
>

Best if x86_decode_insn(), emulate_int_real(), and 
emulator_task_switch() all call an internal initialization function.  
This way the external caller doesn't have to worry about the details.

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

[PATCH 04/14] KVM: x86 emulator: split dst decode to a generic decode_operand()

[Avi Kivity]

Instead of decoding each operand using its own code, use a generic
function.  Start with the destination operand.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |  146 ++++++++++++++++++++++++++++-------------------
 1 files changed, 87 insertions(+), 59 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 58172fb..6a6aed9 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -29,6 +29,22 @@
 #include "tss.h"
 
 /*
+ * Operand types
+ */
+#define OpNone             0
+#define OpImplicit         1  /* No generic decode */
+#define OpReg              2  /* Register */
+#define OpMem              3  /* Memory */
+#define OpAcc              4  /* Accumulator: AL/AX/EAX/RAX */
+#define OpDI               5  /* ES:DI/EDI/RDI */
+#define OpMem64            6  /* Memory, 64-bit */
+#define OpImmUByte         7  /* Zero-extended 8-bit immediate */
+#define OpDX               8  /* DX register */
+
+#define OpBits             4  /* Width of operand field */
+#define OpMask             ((1 << OpBits) - 1)
+
+/*
  * Opcode effective-address decode tables.
  * Note that we only emulate instructions that have at least one memory
  * operand (excluding implicit stack references). We assume that stack
@@ -40,15 +56,16 @@
 /* Operand sizes: 8-bit operands or specified/overridden size. */
 #define ByteOp      (1<<0)	/* 8-bit operands. */
 /* Destination operand type. */
-#define ImplicitOps (1<<1)	/* Implicit in opcode. No generic decode. */
-#define DstReg      (2<<1)	/* Register operand. */
-#define DstMem      (3<<1)	/* Memory operand. */
-#define DstAcc      (4<<1)	/* Destination Accumulator */
-#define DstDI       (5<<1)	/* Destination is in ES:(E)DI */
-#define DstMem64    (6<<1)	/* 64bit memory operand */
-#define DstImmUByte (7<<1)	/* 8-bit unsigned immediate operand */
-#define DstDX       (8<<1)	/* Destination is in DX register */
-#define DstMask     (0xf<<1)
+#define DstShift    1
+#define ImplicitOps (OpImplicit << DstShift)
+#define DstReg      (OpReg << DstShift)
+#define DstMem      (OpMem << DstShift)
+#define DstAcc      (OpAcc << DstShift)
+#define DstDI       (OpDI << DstShift)
+#define DstMem64    (OpMem64 << DstShift)
+#define DstImmUByte (OpImmUByte << DstShift)
+#define DstDX       (OpDX << DstShift)
+#define DstMask     (OpMask << DstShift)
 /* Source operand type. */
 #define SrcNone     (0<<5)	/* No source operand. */
 #define SrcReg      (1<<5)	/* Register operand. */
@@ -3316,6 +3333,66 @@ static int decode_imm(struct x86_emulate_ctxt *ctxt, struct operand *op,
 	return rc;
 }
 
+static int decode_operand(struct x86_emulate_ctxt *ctxt, struct operand *op,
+			  unsigned d)
+{
+	int rc = X86EMUL_CONTINUE;
+
+	switch (d) {
+	case OpReg:
+		decode_register_operand(ctxt, op,
+			 ctxt->twobyte && (ctxt->b == 0xb6 || ctxt->b == 0xb7));
+		break;
+	case OpImmUByte:
+		op->type = OP_IMM;
+		op->addr.mem.ea = ctxt->_eip;
+		op->bytes = 1;
+		op->val = insn_fetch(u8, ctxt);
+		break;
+	case OpMem:
+	case OpMem64:
+		*op = ctxt->memop;
+		ctxt->memopp = op;
+		if (d == OpMem64)
+			op->bytes = 8;
+		else
+			op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
+		if (ctxt->d & BitOp)
+			fetch_bit_operand(ctxt);
+		op->orig_val = op->val;
+		break;
+	case OpAcc:
+		op->type = OP_REG;
+		op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
+		op->addr.reg = &ctxt->regs[VCPU_REGS_RAX];
+		fetch_register_operand(op);
+		op->orig_val = op->val;
+		break;
+	case OpDI:
+		op->type = OP_MEM;
+		op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
+		op->addr.mem.ea =
+			register_address(ctxt, ctxt->regs[VCPU_REGS_RDI]);
+		op->addr.mem.seg = VCPU_SREG_ES;
+		op->val = 0;
+		break;
+	case OpDX:
+		op->type = OP_REG;
+		op->bytes = 2;
+		op->addr.reg = &ctxt->regs[VCPU_REGS_RDX];
+		fetch_register_operand(op);
+		break;
+	case OpImplicit:
+		/* Special instructions do their own operand decoding. */
+	default:
+		op->type = OP_NONE; /* Disable writeback. */
+		break;
+	}
+
+done:
+	return rc;
+}
+
 int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
 {
 	int rc = X86EMUL_CONTINUE;
@@ -3602,56 +3679,7 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
 		goto done;
 
 	/* Decode and fetch the destination operand: register or memory. */
-	switch (ctxt->d & DstMask) {
-	case DstReg:
-		decode_register_operand(ctxt, &ctxt->dst,
-			 ctxt->twobyte && (ctxt->b == 0xb6 || ctxt->b == 0xb7));
-		break;
-	case DstImmUByte:
-		ctxt->dst.type = OP_IMM;
-		ctxt->dst.addr.mem.ea = ctxt->_eip;
-		ctxt->dst.bytes = 1;
-		ctxt->dst.val = insn_fetch(u8, ctxt);
-		break;
-	case DstMem:
-	case DstMem64:
-		ctxt->dst = ctxt->memop;
-		ctxt->memopp = &ctxt->dst;
-		if ((ctxt->d & DstMask) == DstMem64)
-			ctxt->dst.bytes = 8;
-		else
-			ctxt->dst.bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
-		if (ctxt->d & BitOp)
-			fetch_bit_operand(ctxt);
-		ctxt->dst.orig_val = ctxt->dst.val;
-		break;
-	case DstAcc:
-		ctxt->dst.type = OP_REG;
-		ctxt->dst.bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
-		ctxt->dst.addr.reg = &ctxt->regs[VCPU_REGS_RAX];
-		fetch_register_operand(&ctxt->dst);
-		ctxt->dst.orig_val = ctxt->dst.val;
-		break;
-	case DstDI:
-		ctxt->dst.type = OP_MEM;
-		ctxt->dst.bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
-		ctxt->dst.addr.mem.ea =
-			register_address(ctxt, ctxt->regs[VCPU_REGS_RDI]);
-		ctxt->dst.addr.mem.seg = VCPU_SREG_ES;
-		ctxt->dst.val = 0;
-		break;
-	case DstDX:
-		ctxt->dst.type = OP_REG;
-		ctxt->dst.bytes = 2;
-		ctxt->dst.addr.reg = &ctxt->regs[VCPU_REGS_RDX];
-		fetch_register_operand(&ctxt->dst);
-		break;
-	case ImplicitOps:
-		/* Special instructions do their own operand decoding. */
-	default:
-		ctxt->dst.type = OP_NONE; /* Disable writeback. */
-		break;
-	}
+	rc = decode_operand(ctxt, &ctxt->dst, (ctxt->d >> DstShift) & OpMask);
 
 done:
 	if (ctxt->memopp && ctxt->memopp->type == OP_MEM && ctxt->rip_relative)
-- 
1.7.6.1

[PATCH 05/14] KVM: x86 emulator: expand decode flags to 64 bits

[Avi Kivity]

Unifiying the operands means not taking advantage of the fact that some
operand types can only go into certain operands (for example, DI can only
be used by the destination), so we need more bits to hold the operand type.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/include/asm/kvm_emulate.h |    2 +-
 arch/x86/kvm/emulate.c             |   38 ++++++++++++++++++------------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index 56bac3e..a026507 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -262,7 +262,7 @@ struct x86_emulate_ctxt {
 	struct operand dst;
 	bool has_seg_override;
 	u8 seg_override;
-	unsigned int d;
+	u64 d;
 	int (*execute)(struct x86_emulate_ctxt *ctxt);
 	int (*check_perm)(struct x86_emulate_ctxt *ctxt);
 	/* modrm */
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 6a6aed9..8c65ff2 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -31,18 +31,18 @@
 /*
  * Operand types
  */
-#define OpNone             0
-#define OpImplicit         1  /* No generic decode */
-#define OpReg              2  /* Register */
-#define OpMem              3  /* Memory */
-#define OpAcc              4  /* Accumulator: AL/AX/EAX/RAX */
-#define OpDI               5  /* ES:DI/EDI/RDI */
-#define OpMem64            6  /* Memory, 64-bit */
-#define OpImmUByte         7  /* Zero-extended 8-bit immediate */
-#define OpDX               8  /* DX register */
+#define OpNone             0ull
+#define OpImplicit         1ull  /* No generic decode */
+#define OpReg              2ull  /* Register */
+#define OpMem              3ull  /* Memory */
+#define OpAcc              4ull  /* Accumulator: AL/AX/EAX/RAX */
+#define OpDI               5ull  /* ES:DI/EDI/RDI */
+#define OpMem64            6ull  /* Memory, 64-bit */
+#define OpImmUByte         7ull  /* Zero-extended 8-bit immediate */
+#define OpDX               8ull  /* DX register */
 
 #define OpBits             4  /* Width of operand field */
-#define OpMask             ((1 << OpBits) - 1)
+#define OpMask             ((1ull << OpBits) - 1)
 
 /*
  * Opcode effective-address decode tables.
@@ -108,12 +108,12 @@
 #define Priv        (1<<27) /* instruction generates #GP if current CPL != 0 */
 #define No64	    (1<<28)
 /* Source 2 operand type */
-#define Src2None    (0<<29)
-#define Src2CL      (1<<29)
-#define Src2ImmByte (2<<29)
-#define Src2One     (3<<29)
-#define Src2Imm     (4<<29)
-#define Src2Mask    (7<<29)
+#define Src2None    (0u<<29)
+#define Src2CL      (1u<<29)
+#define Src2ImmByte (2u<<29)
+#define Src2One     (3u<<29)
+#define Src2Imm     (4u<<29)
+#define Src2Mask    (7u<<29)
 
 #define X2(x...) x, x
 #define X3(x...) X2(x), x
@@ -125,8 +125,8 @@
 #define X16(x...) X8(x), X8(x)
 
 struct opcode {
-	u32 flags;
-	u8 intercept;
+	u64 flags : 56;
+	u64 intercept : 8;
 	union {
 		int (*execute)(struct x86_emulate_ctxt *ctxt);
 		struct opcode *group;
@@ -3530,7 +3530,7 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
 			return EMULATION_FAILED;
 		}
 
-		ctxt->d &= ~GroupMask;
+		ctxt->d &= ~(u64)GroupMask;
 		ctxt->d |= opcode.flags;
 	}
 
-- 
1.7.6.1

[PATCH 06/14] KVM: x86 emulator: switch src2 to generic decode_operand()

[Avi Kivity]

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |   51 ++++++++++++++++++++++++-----------------------
 1 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 8c65ff2..88d32fc 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -40,6 +40,10 @@
 #define OpMem64            6ull  /* Memory, 64-bit */
 #define OpImmUByte         7ull  /* Zero-extended 8-bit immediate */
 #define OpDX               8ull  /* DX register */
+#define OpCL               9ull  /* CL register (for shifts) */
+#define OpImmByte         10ull  /* 8-bit sign extended immediate */
+#define OpOne             11ull  /* Implied 1 */
+#define OpImm             12ull  /* Sign extended immediate */
 
 #define OpBits             4  /* Width of operand field */
 #define OpMask             ((1ull << OpBits) - 1)
@@ -108,12 +112,13 @@
 #define Priv        (1<<27) /* instruction generates #GP if current CPL != 0 */
 #define No64	    (1<<28)
 /* Source 2 operand type */
-#define Src2None    (0u<<29)
-#define Src2CL      (1u<<29)
-#define Src2ImmByte (2u<<29)
-#define Src2One     (3u<<29)
-#define Src2Imm     (4u<<29)
-#define Src2Mask    (7u<<29)
+#define Src2Shift   (29)
+#define Src2None    (OpNone << Src2Shift)
+#define Src2CL      (OpCL << Src2Shift)
+#define Src2ImmByte (OpImmByte << Src2Shift)
+#define Src2One     (OpOne << Src2Shift)
+#define Src2Imm     (OpImm << Src2Shift)
+#define Src2Mask    (OpMask << Src2Shift)
 
 #define X2(x...) x, x
 #define X3(x...) X2(x), x
@@ -3382,6 +3387,20 @@ static int decode_operand(struct x86_emulate_ctxt *ctxt, struct operand *op,
 		op->addr.reg = &ctxt->regs[VCPU_REGS_RDX];
 		fetch_register_operand(op);
 		break;
+	case OpCL:
+		op->bytes = 1;
+		op->val = ctxt->regs[VCPU_REGS_RCX] & 0xff;
+		break;
+	case OpImmByte:
+		rc = decode_imm(ctxt, op, 1, true);
+		break;
+	case OpOne:
+		op->bytes = 1;
+		op->val = 1;
+		break;
+	case OpImm:
+		rc = decode_imm(ctxt, op, imm_size(ctxt), true);
+		break;
 	case OpImplicit:
 		/* Special instructions do their own operand decoding. */
 	default:
@@ -3656,25 +3675,7 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
 	 * Decode and fetch the second source operand: register, memory
 	 * or immediate.
 	 */
-	switch (ctxt->d & Src2Mask) {
-	case Src2None:
-		break;
-	case Src2CL:
-		ctxt->src2.bytes = 1;
-		ctxt->src2.val = ctxt->regs[VCPU_REGS_RCX] & 0xff;
-		break;
-	case Src2ImmByte:
-		rc = decode_imm(ctxt, &ctxt->src2, 1, true);
-		break;
-	case Src2One:
-		ctxt->src2.bytes = 1;
-		ctxt->src2.val = 1;
-		break;
-	case Src2Imm:
-		rc = decode_imm(ctxt, &ctxt->src2, imm_size(ctxt), true);
-		break;
-	}
-
+	rc = decode_operand(ctxt, &ctxt->src2, (ctxt->d >> Src2Shift) & OpMask);
 	if (rc != X86EMUL_CONTINUE)
 		goto done;
 
-- 
1.7.6.1

[PATCH 07/14] KVM: x86 emulator: free up some flag bits near src, dst

[Avi Kivity]

Op fields are going to grow by a bit, we need two free bits.

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

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 88d32fc..00e0904 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -88,10 +88,6 @@
 #define SrcImmU16   (0xe<<5)    /* Immediate operand, unsigned, 16 bits */
 #define SrcDX       (0xf<<5)	/* Source is in DX register */
 #define SrcMask     (0xf<<5)
-/* Generic ModRM decode. */
-#define ModRM       (1<<9)
-/* Destination is only written; never read. */
-#define Mov         (1<<10)
 #define BitOp       (1<<11)
 #define MemAbs      (1<<12)      /* Memory operand is absolute displacement */
 #define String      (1<<13)     /* String instruction (rep capable) */
@@ -102,6 +98,10 @@
 #define Prefix      (3<<15)     /* Instruction varies with 66/f2/f3 prefix */
 #define RMExt       (4<<15)     /* Opcode extension in ModRM r/m if mod == 3 */
 #define Sse         (1<<18)     /* SSE Vector instruction */
+/* Generic ModRM decode. */
+#define ModRM       (1<<19)
+/* Destination is only written; never read. */
+#define Mov         (1<<20)
 /* Misc flags */
 #define Prot        (1<<21) /* instruction generates #UD if not in prot-mode */
 #define VendorSpecific (1<<22) /* Vendor specific instruction */
-- 
1.7.6.1

[PATCH 08/14] KVM: x86 emulator: switch OpImmUByte decode to decode_imm()

[Avi Kivity]

Similar to SrcImmUByte.

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

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 00e0904..a0d6ceb 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3349,10 +3349,7 @@ static int decode_operand(struct x86_emulate_ctxt *ctxt, struct operand *op,
 			 ctxt->twobyte && (ctxt->b == 0xb6 || ctxt->b == 0xb7));
 		break;
 	case OpImmUByte:
-		op->type = OP_IMM;
-		op->addr.mem.ea = ctxt->_eip;
-		op->bytes = 1;
-		op->val = insn_fetch(u8, ctxt);
+		rc = decode_imm(ctxt, op, 1, false);
 		break;
 	case OpMem:
 	case OpMem64:
-- 
1.7.6.1

[PATCH 09/14] KVM: x86 emulator: qualify OpReg inhibit_byte_regs hack

[Avi Kivity]

OpReg decoding has a hack that inhibits byte registers for movsx and movzx
instructions.  It should be replaced by something better, but meanwhile,
qualify that the hack is only active for the destination operand.

Note these instructions only use OpReg for the destination, but better to
be explicit about it.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index a0d6ceb..17a8910 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3346,6 +3346,7 @@ static int decode_operand(struct x86_emulate_ctxt *ctxt, struct operand *op,
 	switch (d) {
 	case OpReg:
 		decode_register_operand(ctxt, op,
+			 op == &ctxt->dst &&
 			 ctxt->twobyte && (ctxt->b == 0xb6 || ctxt->b == 0xb7));
 		break;
 	case OpImmUByte:
-- 
1.7.6.1

[PATCH 10/14] KVM: x86 emulator: switch src decode to decode_operand()

[Avi Kivity]

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |  156 +++++++++++++++++++----------------------------
 1 files changed, 63 insertions(+), 93 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 17a8910..e46809b 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -44,8 +44,15 @@
 #define OpImmByte         10ull  /* 8-bit sign extended immediate */
 #define OpOne             11ull  /* Implied 1 */
 #define OpImm             12ull  /* Sign extended immediate */
-
-#define OpBits             4  /* Width of operand field */
+#define OpMem16           13ull  /* Memory operand (16-bit). */
+#define OpMem32           14ull  /* Memory operand (32-bit). */
+#define OpImmU            15ull  /* Immediate operand, zero extended */
+#define OpSI              16ull  /* SI/ESI/RSI */
+#define OpImmFAddr        17ull  /* Immediate far address */
+#define OpMemFAddr        18ull  /* Far address in memory */
+#define OpImmU16          19ull  /* Immediate operand, 16 bits, zero extended */
+
+#define OpBits             5  /* Width of operand field */
 #define OpMask             ((1ull << OpBits) - 1)
 
 /*
@@ -71,23 +78,24 @@
 #define DstDX       (OpDX << DstShift)
 #define DstMask     (OpMask << DstShift)
 /* Source operand type. */
-#define SrcNone     (0<<5)	/* No source operand. */
-#define SrcReg      (1<<5)	/* Register operand. */
-#define SrcMem      (2<<5)	/* Memory operand. */
-#define SrcMem16    (3<<5)	/* Memory operand (16-bit). */
-#define SrcMem32    (4<<5)	/* Memory operand (32-bit). */
-#define SrcImm      (5<<5)	/* Immediate operand. */
-#define SrcImmByte  (6<<5)	/* 8-bit sign-extended immediate operand. */
-#define SrcOne      (7<<5)	/* Implied '1' */
-#define SrcImmUByte (8<<5)      /* 8-bit unsigned immediate operand. */
-#define SrcImmU     (9<<5)      /* Immediate operand, unsigned */
-#define SrcSI       (0xa<<5)	/* Source is in the DS:RSI */
-#define SrcImmFAddr (0xb<<5)	/* Source is immediate far address */
-#define SrcMemFAddr (0xc<<5)	/* Source is far address in memory */
-#define SrcAcc      (0xd<<5)	/* Source Accumulator */
-#define SrcImmU16   (0xe<<5)    /* Immediate operand, unsigned, 16 bits */
-#define SrcDX       (0xf<<5)	/* Source is in DX register */
-#define SrcMask     (0xf<<5)
+#define SrcShift    6
+#define SrcNone     (OpNone << SrcShift)
+#define SrcReg      (OpReg << SrcShift)
+#define SrcMem      (OpMem << SrcShift)
+#define SrcMem16    (OpMem16 << SrcShift)
+#define SrcMem32    (OpMem32 << SrcShift)
+#define SrcImm      (OpImm << SrcShift)
+#define SrcImmByte  (OpImmByte << SrcShift)
+#define SrcOne      (OpOne << SrcShift)
+#define SrcImmUByte (OpImmUByte << SrcShift)
+#define SrcImmU     (OpImmU << SrcShift)
+#define SrcSI       (OpSI << SrcShift)
+#define SrcImmFAddr (OpImmFAddr << SrcShift)
+#define SrcMemFAddr (OpMemFAddr << SrcShift)
+#define SrcAcc      (OpAcc << SrcShift)
+#define SrcImmU16   (OpImmU16 << SrcShift)
+#define SrcDX       (OpDX << SrcShift)
+#define SrcMask     (OpMask << SrcShift)
 #define BitOp       (1<<11)
 #define MemAbs      (1<<12)      /* Memory operand is absolute displacement */
 #define String      (1<<13)     /* String instruction (rep capable) */
@@ -3354,13 +3362,14 @@ static int decode_operand(struct x86_emulate_ctxt *ctxt, struct operand *op,
 		break;
 	case OpMem:
 	case OpMem64:
-		*op = ctxt->memop;
-		ctxt->memopp = op;
 		if (d == OpMem64)
-			op->bytes = 8;
+			ctxt->memop.bytes = 8;
 		else
-			op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
-		if (ctxt->d & BitOp)
+			ctxt->memop.bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
+	mem_common:
+		*op = ctxt->memop;
+		ctxt->memopp = op;
+		if ((ctxt->d & BitOp) && op == &ctxt->dst)
 			fetch_bit_operand(ctxt);
 		op->orig_val = op->val;
 		break;
@@ -3399,6 +3408,35 @@ static int decode_operand(struct x86_emulate_ctxt *ctxt, struct operand *op,
 	case OpImm:
 		rc = decode_imm(ctxt, op, imm_size(ctxt), true);
 		break;
+	case OpMem16:
+		ctxt->memop.bytes = 2;
+		goto mem_common;
+	case OpMem32:
+		ctxt->memop.bytes = 4;
+		goto mem_common;
+	case OpImmU16:
+		rc = decode_imm(ctxt, op, 2, false);
+		break;
+	case OpImmU:
+		rc = decode_imm(ctxt, op, imm_size(ctxt), false);
+		break;
+	case OpSI:
+		op->type = OP_MEM;
+		op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
+		op->addr.mem.ea =
+			register_address(ctxt, ctxt->regs[VCPU_REGS_RSI]);
+		op->addr.mem.seg = seg_override(ctxt);
+		op->val = 0;
+		break;
+	case OpImmFAddr:
+		op->type = OP_IMM;
+		op->addr.mem.ea = ctxt->_eip;
+		op->bytes = ctxt->op_bytes + 2;
+		insn_fetch_arr(op->valptr, op->bytes, ctxt);
+		break;
+	case OpMemFAddr:
+		ctxt->memop.bytes = ctxt->op_bytes + 2;
+		goto mem_common;
 	case OpImplicit:
 		/* Special instructions do their own operand decoding. */
 	default:
@@ -3597,75 +3635,7 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
 	 * Decode and fetch the source operand: register, memory
 	 * or immediate.
 	 */
-	switch (ctxt->d & SrcMask) {
-	case SrcNone:
-		break;
-	case SrcReg:
-		decode_register_operand(ctxt, &ctxt->src, 0);
-		break;
-	case SrcMem16:
-		ctxt->memop.bytes = 2;
-		goto srcmem_common;
-	case SrcMem32:
-		ctxt->memop.bytes = 4;
-		goto srcmem_common;
-	case SrcMem:
-		ctxt->memop.bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
-	srcmem_common:
-		ctxt->src = ctxt->memop;
-		ctxt->memopp = &ctxt->src;
-		break;
-	case SrcImmU16:
-		rc = decode_imm(ctxt, &ctxt->src, 2, false);
-		break;
-	case SrcImm:
-		rc = decode_imm(ctxt, &ctxt->src, imm_size(ctxt), true);
-		break;
-	case SrcImmU:
-		rc = decode_imm(ctxt, &ctxt->src, imm_size(ctxt), false);
-		break;
-	case SrcImmByte:
-		rc = decode_imm(ctxt, &ctxt->src, 1, true);
-		break;
-	case SrcImmUByte:
-		rc = decode_imm(ctxt, &ctxt->src, 1, false);
-		break;
-	case SrcAcc:
-		ctxt->src.type = OP_REG;
-		ctxt->src.bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
-		ctxt->src.addr.reg = &ctxt->regs[VCPU_REGS_RAX];
-		fetch_register_operand(&ctxt->src);
-		break;
-	case SrcOne:
-		ctxt->src.bytes = 1;
-		ctxt->src.val = 1;
-		break;
-	case SrcSI:
-		ctxt->src.type = OP_MEM;
-		ctxt->src.bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
-		ctxt->src.addr.mem.ea =
-			register_address(ctxt, ctxt->regs[VCPU_REGS_RSI]);
-		ctxt->src.addr.mem.seg = seg_override(ctxt);
-		ctxt->src.val = 0;
-		break;
-	case SrcImmFAddr:
-		ctxt->src.type = OP_IMM;
-		ctxt->src.addr.mem.ea = ctxt->_eip;
-		ctxt->src.bytes = ctxt->op_bytes + 2;
-		insn_fetch_arr(ctxt->src.valptr, ctxt->src.bytes, ctxt);
-		break;
-	case SrcMemFAddr:
-		ctxt->memop.bytes = ctxt->op_bytes + 2;
-		goto srcmem_common;
-		break;
-	case SrcDX:
-		ctxt->src.type = OP_REG;
-		ctxt->src.bytes = 2;
-		ctxt->src.addr.reg = &ctxt->regs[VCPU_REGS_RDX];
-		fetch_register_operand(&ctxt->src);
-		break;
-	}
-
+	rc = decode_operand(ctxt, &ctxt->src, (ctxt->d >> SrcShift) & OpMask);
 	if (rc != X86EMUL_CONTINUE)
 		goto done;
 
-- 
1.7.6.1

[PATCH 11/14] KVM: x86 emulator: simplify OpMem64 decode

[Avi Kivity]

Use the same technique as the other OpMem variants, and goto mem_common.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index e46809b..1c95935 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3361,11 +3361,7 @@ static int decode_operand(struct x86_emulate_ctxt *ctxt, struct operand *op,
 		rc = decode_imm(ctxt, op, 1, false);
 		break;
 	case OpMem:
-	case OpMem64:
-		if (d == OpMem64)
-			ctxt->memop.bytes = 8;
-		else
-			ctxt->memop.bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
+		ctxt->memop.bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
 	mem_common:
 		*op = ctxt->memop;
 		ctxt->memopp = op;
@@ -3373,6 +3369,9 @@ static int decode_operand(struct x86_emulate_ctxt *ctxt, struct operand *op,
 			fetch_bit_operand(ctxt);
 		op->orig_val = op->val;
 		break;
+	case OpMem64:
+		ctxt->memop.bytes = 8;
+		goto mem_common;
 	case OpAcc:
 		op->type = OP_REG;
 		op->bytes = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes;
-- 
1.7.6.1

[PATCH 12/14] KVM: x86 emulator: streamline decode of segment registers

[Avi Kivity]

The opcodes

  push %seg
  pop %seg
  l%seg, %mem, %reg  (e.g. lds/les/lss/lfs/lgs)

all have an segment register encoded in the instruction.  To allow reuse,
decode the segment number into src2 during the decode stage instead of the
execution stage.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |   99 +++++++++++++++++++++++++++--------------------
 1 files changed, 57 insertions(+), 42 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 1c95935..ab48611 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -51,6 +51,12 @@
 #define OpImmFAddr        17ull  /* Immediate far address */
 #define OpMemFAddr        18ull  /* Far address in memory */
 #define OpImmU16          19ull  /* Immediate operand, 16 bits, zero extended */
+#define OpES              20ull  /* ES */
+#define OpCS              21ull  /* CS */
+#define OpSS              22ull  /* SS */
+#define OpDS              23ull  /* DS */
+#define OpFS              24ull  /* FS */
+#define OpGS              25ull  /* GS */
 
 #define OpBits             5  /* Width of operand field */
 #define OpMask             ((1ull << OpBits) - 1)
@@ -126,6 +132,12 @@
 #define Src2ImmByte (OpImmByte << Src2Shift)
 #define Src2One     (OpOne << Src2Shift)
 #define Src2Imm     (OpImm << Src2Shift)
+#define Src2ES      (OpES << Src2Shift)
+#define Src2CS      (OpCS << Src2Shift)
+#define Src2SS      (OpSS << Src2Shift)
+#define Src2DS      (OpDS << Src2Shift)
+#define Src2FS      (OpFS << Src2Shift)
+#define Src2GS      (OpGS << Src2Shift)
 #define Src2Mask    (OpMask << Src2Shift)
 
 #define X2(x...) x, x
@@ -3101,16 +3113,19 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
 static struct opcode opcode_table[256] = {
 	/* 0x00 - 0x07 */
 	I6ALU(Lock, em_add),
-	D(ImplicitOps | Stack | No64), D(ImplicitOps | Stack | No64),
+	D(ImplicitOps | Stack | No64 | Src2ES),
+	D(ImplicitOps | Stack | No64 | Src2ES),
 	/* 0x08 - 0x0F */
 	I6ALU(Lock, em_or),
-	D(ImplicitOps | Stack | No64), N,
+	D(ImplicitOps | Stack | No64 | Src2CS), N,
 	/* 0x10 - 0x17 */
 	I6ALU(Lock, em_adc),
-	D(ImplicitOps | Stack | No64), D(ImplicitOps | Stack | No64),
+	D(ImplicitOps | Stack | No64 | Src2SS),
+	D(ImplicitOps | Stack | No64 | Src2SS),
 	/* 0x18 - 0x1F */
 	I6ALU(Lock, em_sbb),
-	D(ImplicitOps | Stack | No64), D(ImplicitOps | Stack | No64),
+	D(ImplicitOps | Stack | No64 | Src2DS),
+	D(ImplicitOps | Stack | No64 | Src2DS),
 	/* 0x20 - 0x27 */
 	I6ALU(Lock, em_and), N, N,
 	/* 0x28 - 0x2F */
@@ -3178,7 +3193,8 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
 	D2bv(DstMem | SrcImmByte | ModRM),
 	I(ImplicitOps | Stack | SrcImmU16, em_ret_near_imm),
 	I(ImplicitOps | Stack, em_ret),
-	D(DstReg | SrcMemFAddr | ModRM | No64), D(DstReg | SrcMemFAddr | ModRM | No64),
+	D(DstReg | SrcMemFAddr | ModRM | No64 | Src2ES),
+	D(DstReg | SrcMemFAddr | ModRM | No64 | Src2DS),
 	G(ByteOp, group11), G(0, group11),
 	/* 0xC8 - 0xCF */
 	N, N, N, I(ImplicitOps | Stack, em_ret_far),
@@ -3253,20 +3269,22 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
 	/* 0x90 - 0x9F */
 	X16(D(ByteOp | DstMem | SrcNone | ModRM| Mov)),
 	/* 0xA0 - 0xA7 */
-	D(ImplicitOps | Stack), D(ImplicitOps | Stack),
+	D(Stack | Src2FS), D(Stack | Src2FS),
 	DI(ImplicitOps, cpuid), D(DstMem | SrcReg | ModRM | BitOp),
 	D(DstMem | SrcReg | Src2ImmByte | ModRM),
 	D(DstMem | SrcReg | Src2CL | ModRM), N, N,
 	/* 0xA8 - 0xAF */
-	D(ImplicitOps | Stack), D(ImplicitOps | Stack),
+	D(Stack | Src2GS), D(Stack | Src2GS),
 	DI(ImplicitOps, rsm), D(DstMem | SrcReg | ModRM | BitOp | Lock),
 	D(DstMem | SrcReg | Src2ImmByte | ModRM),
 	D(DstMem | SrcReg | Src2CL | ModRM),
 	D(ModRM), I(DstReg | SrcMem | ModRM, em_imul),
 	/* 0xB0 - 0xB7 */
 	D2bv(DstMem | SrcReg | ModRM | Lock),
-	D(DstReg | SrcMemFAddr | ModRM), D(DstMem | SrcReg | ModRM | BitOp | Lock),
-	D(DstReg | SrcMemFAddr | ModRM), D(DstReg | SrcMemFAddr | ModRM),
+	D(DstReg | SrcMemFAddr | ModRM | Src2SS),
+	D(DstMem | SrcReg | ModRM | BitOp | Lock),
+	D(DstReg | SrcMemFAddr | ModRM | Src2FS),
+	D(DstReg | SrcMemFAddr | ModRM | Src2GS),
 	D(ByteOp | DstReg | SrcMem | ModRM | Mov), D(DstReg | SrcMem16 | ModRM | Mov),
 	/* 0xB8 - 0xBF */
 	N, N,
@@ -3436,6 +3454,24 @@ static int decode_operand(struct x86_emulate_ctxt *ctxt, struct operand *op,
 	case OpMemFAddr:
 		ctxt->memop.bytes = ctxt->op_bytes + 2;
 		goto mem_common;
+	case OpES:
+		op->val = VCPU_SREG_ES;
+		break;
+	case OpCS:
+		op->val = VCPU_SREG_CS;
+		break;
+	case OpSS:
+		op->val = VCPU_SREG_SS;
+		break;
+	case OpDS:
+		op->val = VCPU_SREG_DS;
+		break;
+	case OpFS:
+		op->val = VCPU_SREG_FS;
+		break;
+	case OpGS:
+		op->val = VCPU_SREG_GS;
+		break;
 	case OpImplicit:
 		/* Special instructions do their own operand decoding. */
 	default:
@@ -3803,26 +3839,15 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 
 	switch (ctxt->b) {
 	case 0x06:		/* push es */
-		rc = emulate_push_sreg(ctxt, VCPU_SREG_ES);
-		break;
-	case 0x07:		/* pop es */
-		rc = emulate_pop_sreg(ctxt, VCPU_SREG_ES);
-		break;
 	case 0x0e:		/* push cs */
-		rc = emulate_push_sreg(ctxt, VCPU_SREG_CS);
-		break;
 	case 0x16:		/* push ss */
-		rc = emulate_push_sreg(ctxt, VCPU_SREG_SS);
-		break;
-	case 0x17:		/* pop ss */
-		rc = emulate_pop_sreg(ctxt, VCPU_SREG_SS);
-		break;
 	case 0x1e:		/* push ds */
-		rc = emulate_push_sreg(ctxt, VCPU_SREG_DS);
+		rc = emulate_push_sreg(ctxt, ctxt->src2.val);
 		break;
+	case 0x07:		/* pop es */
+	case 0x17:		/* pop ss */
 	case 0x1f:		/* pop ds */
-		rc = emulate_pop_sreg(ctxt, VCPU_SREG_DS);
-		break;
+		rc = emulate_pop_sreg(ctxt, ctxt->src2.val);
 	case 0x40 ... 0x47: /* inc r16/r32 */
 		emulate_1op(ctxt, "inc");
 		break;
@@ -3869,10 +3894,8 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 		rc = em_grp2(ctxt);
 		break;
 	case 0xc4:		/* les */
-		rc = emulate_load_segment(ctxt, VCPU_SREG_ES);
-		break;
 	case 0xc5:		/* lds */
-		rc = emulate_load_segment(ctxt, VCPU_SREG_DS);
+		rc = emulate_load_segment(ctxt, ctxt->src2.val);
 		break;
 	case 0xcc:		/* int3 */
 		rc = emulate_int(ctxt, 3);
@@ -4078,10 +4101,12 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 		ctxt->dst.val = test_cc(ctxt->b, ctxt->eflags);
 		break;
 	case 0xa0:	  /* push fs */
-		rc = emulate_push_sreg(ctxt, VCPU_SREG_FS);
+	case 0xa8:	  /* push gs */
+		rc = emulate_push_sreg(ctxt, ctxt->src2.val);
 		break;
 	case 0xa1:	 /* pop fs */
-		rc = emulate_pop_sreg(ctxt, VCPU_SREG_FS);
+	case 0xa9:	 /* pop gs */
+		rc = emulate_pop_sreg(ctxt, ctxt->src2.val);
 		break;
 	case 0xa3:
 	      bt:		/* bt */
@@ -4094,12 +4119,6 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 	case 0xa5: /* shld cl, r, r/m */
 		emulate_2op_cl(ctxt, "shld");
 		break;
-	case 0xa8:	/* push gs */
-		rc = emulate_push_sreg(ctxt, VCPU_SREG_GS);
-		break;
-	case 0xa9:	/* pop gs */
-		rc = emulate_pop_sreg(ctxt, VCPU_SREG_GS);
-		break;
 	case 0xab:
 	      bts:		/* bts */
 		emulate_2op_SrcV_nobyte(ctxt, "bts");
@@ -4128,18 +4147,14 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 		}
 		break;
 	case 0xb2:		/* lss */
-		rc = emulate_load_segment(ctxt, VCPU_SREG_SS);
+	case 0xb4:		/* lfs */
+	case 0xb5:		/* lgs */
+		rc = emulate_load_segment(ctxt, ctxt->src2.val);
 		break;
 	case 0xb3:
 	      btr:		/* btr */
 		emulate_2op_SrcV_nobyte(ctxt, "btr");
 		break;
-	case 0xb4:		/* lfs */
-		rc = emulate_load_segment(ctxt, VCPU_SREG_FS);
-		break;
-	case 0xb5:		/* lgs */
-		rc = emulate_load_segment(ctxt, VCPU_SREG_GS);
-		break;
 	case 0xb6 ... 0xb7:	/* movzx */
 		ctxt->dst.bytes = ctxt->op_bytes;
 		ctxt->dst.val = (ctxt->d & ByteOp) ? (u8) ctxt->src.val
-- 
1.7.6.1

[PATCH 13/14] KVM: x86 emulator: switch lds/les/lss/lfs/lgs to direct decode

[Avi Kivity]

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |   22 +++++++---------------
 1 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index ab48611..bd3e488 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1828,8 +1828,9 @@ static int em_ret_far(struct x86_emulate_ctxt *ctxt)
 	return rc;
 }
 
-static int emulate_load_segment(struct x86_emulate_ctxt *ctxt, int seg)
+static int em_lseg(struct x86_emulate_ctxt *ctxt)
 {
+	int seg = ctxt->src2.val;
 	unsigned short sel;
 	int rc;
 
@@ -3193,8 +3194,8 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
 	D2bv(DstMem | SrcImmByte | ModRM),
 	I(ImplicitOps | Stack | SrcImmU16, em_ret_near_imm),
 	I(ImplicitOps | Stack, em_ret),
-	D(DstReg | SrcMemFAddr | ModRM | No64 | Src2ES),
-	D(DstReg | SrcMemFAddr | ModRM | No64 | Src2DS),
+	I(DstReg | SrcMemFAddr | ModRM | No64 | Src2ES, em_lseg),
+	I(DstReg | SrcMemFAddr | ModRM | No64 | Src2DS, em_lseg),
 	G(ByteOp, group11), G(0, group11),
 	/* 0xC8 - 0xCF */
 	N, N, N, I(ImplicitOps | Stack, em_ret_far),
@@ -3281,10 +3282,10 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
 	D(ModRM), I(DstReg | SrcMem | ModRM, em_imul),
 	/* 0xB0 - 0xB7 */
 	D2bv(DstMem | SrcReg | ModRM | Lock),
-	D(DstReg | SrcMemFAddr | ModRM | Src2SS),
+	I(DstReg | SrcMemFAddr | ModRM | Src2SS, em_lseg),
 	D(DstMem | SrcReg | ModRM | BitOp | Lock),
-	D(DstReg | SrcMemFAddr | ModRM | Src2FS),
-	D(DstReg | SrcMemFAddr | ModRM | Src2GS),
+	I(DstReg | SrcMemFAddr | ModRM | Src2FS, em_lseg),
+	I(DstReg | SrcMemFAddr | ModRM | Src2GS, em_lseg),
 	D(ByteOp | DstReg | SrcMem | ModRM | Mov), D(DstReg | SrcMem16 | ModRM | Mov),
 	/* 0xB8 - 0xBF */
 	N, N,
@@ -3893,10 +3894,6 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 	case 0xc0 ... 0xc1:
 		rc = em_grp2(ctxt);
 		break;
-	case 0xc4:		/* les */
-	case 0xc5:		/* lds */
-		rc = emulate_load_segment(ctxt, ctxt->src2.val);
-		break;
 	case 0xcc:		/* int3 */
 		rc = emulate_int(ctxt, 3);
 		break;
@@ -4146,11 +4143,6 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 			ctxt->dst.addr.reg = (unsigned long *)&ctxt->regs[VCPU_REGS_RAX];
 		}
 		break;
-	case 0xb2:		/* lss */
-	case 0xb4:		/* lfs */
-	case 0xb5:		/* lgs */
-		rc = emulate_load_segment(ctxt, ctxt->src2.val);
-		break;
 	case 0xb3:
 	      btr:		/* btr */
 		emulate_2op_SrcV_nobyte(ctxt, "btr");
-- 
1.7.6.1

[PATCH 14/14] KVM: x86 emulator: convert push %sreg/pop %sreg to direct decode

[Avi Kivity]

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |   44 +++++++++++++++-----------------------------
 1 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index bd3e488..f1e3be1 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1458,15 +1458,18 @@ static int em_popf(struct x86_emulate_ctxt *ctxt)
 	return emulate_popf(ctxt, &ctxt->dst.val, ctxt->op_bytes);
 }
 
-static int emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
+static int em_push_sreg(struct x86_emulate_ctxt *ctxt)
 {
+	int seg = ctxt->src2.val;
+
 	ctxt->src.val = get_segment_selector(ctxt, seg);
 
 	return em_push(ctxt);
 }
 
-static int emulate_pop_sreg(struct x86_emulate_ctxt *ctxt, int seg)
+static int em_pop_sreg(struct x86_emulate_ctxt *ctxt)
 {
+	int seg = ctxt->src2.val;
 	unsigned long selector;
 	int rc;
 
@@ -3114,19 +3117,20 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
 static struct opcode opcode_table[256] = {
 	/* 0x00 - 0x07 */
 	I6ALU(Lock, em_add),
-	D(ImplicitOps | Stack | No64 | Src2ES),
-	D(ImplicitOps | Stack | No64 | Src2ES),
+	I(ImplicitOps | Stack | No64 | Src2ES, em_push_sreg),
+	I(ImplicitOps | Stack | No64 | Src2ES, em_pop_sreg),
 	/* 0x08 - 0x0F */
 	I6ALU(Lock, em_or),
-	D(ImplicitOps | Stack | No64 | Src2CS), N,
+	I(ImplicitOps | Stack | No64 | Src2CS, em_push_sreg),
+	N,
 	/* 0x10 - 0x17 */
 	I6ALU(Lock, em_adc),
-	D(ImplicitOps | Stack | No64 | Src2SS),
-	D(ImplicitOps | Stack | No64 | Src2SS),
+	I(ImplicitOps | Stack | No64 | Src2SS, em_push_sreg),
+	I(ImplicitOps | Stack | No64 | Src2SS, em_pop_sreg),
 	/* 0x18 - 0x1F */
 	I6ALU(Lock, em_sbb),
-	D(ImplicitOps | Stack | No64 | Src2DS),
-	D(ImplicitOps | Stack | No64 | Src2DS),
+	I(ImplicitOps | Stack | No64 | Src2DS, em_push_sreg),
+	I(ImplicitOps | Stack | No64 | Src2DS, em_pop_sreg),
 	/* 0x20 - 0x27 */
 	I6ALU(Lock, em_and), N, N,
 	/* 0x28 - 0x2F */
@@ -3270,12 +3274,12 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
 	/* 0x90 - 0x9F */
 	X16(D(ByteOp | DstMem | SrcNone | ModRM| Mov)),
 	/* 0xA0 - 0xA7 */
-	D(Stack | Src2FS), D(Stack | Src2FS),
+	I(Stack | Src2FS, em_push_sreg), I(Stack | Src2FS, em_pop_sreg),
 	DI(ImplicitOps, cpuid), D(DstMem | SrcReg | ModRM | BitOp),
 	D(DstMem | SrcReg | Src2ImmByte | ModRM),
 	D(DstMem | SrcReg | Src2CL | ModRM), N, N,
 	/* 0xA8 - 0xAF */
-	D(Stack | Src2GS), D(Stack | Src2GS),
+	I(Stack | Src2GS, em_push_sreg), I(Stack | Src2GS, em_pop_sreg),
 	DI(ImplicitOps, rsm), D(DstMem | SrcReg | ModRM | BitOp | Lock),
 	D(DstMem | SrcReg | Src2ImmByte | ModRM),
 	D(DstMem | SrcReg | Src2CL | ModRM),
@@ -3839,16 +3843,6 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 		goto twobyte_insn;
 
 	switch (ctxt->b) {
-	case 0x06:		/* push es */
-	case 0x0e:		/* push cs */
-	case 0x16:		/* push ss */
-	case 0x1e:		/* push ds */
-		rc = emulate_push_sreg(ctxt, ctxt->src2.val);
-		break;
-	case 0x07:		/* pop es */
-	case 0x17:		/* pop ss */
-	case 0x1f:		/* pop ds */
-		rc = emulate_pop_sreg(ctxt, ctxt->src2.val);
 	case 0x40 ... 0x47: /* inc r16/r32 */
 		emulate_1op(ctxt, "inc");
 		break;
@@ -4097,14 +4091,6 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 	case 0x90 ... 0x9f:     /* setcc r/m8 */
 		ctxt->dst.val = test_cc(ctxt->b, ctxt->eflags);
 		break;
-	case 0xa0:	  /* push fs */
-	case 0xa8:	  /* push gs */
-		rc = emulate_push_sreg(ctxt, ctxt->src2.val);
-		break;
-	case 0xa1:	 /* pop fs */
-	case 0xa9:	 /* pop gs */
-		rc = emulate_pop_sreg(ctxt, ctxt->src2.val);
-		break;
 	case 0xa3:
 	      bt:		/* bt */
 		ctxt->dst.type = OP_NONE;
-- 
1.7.6.1

Re: [PATCH 00/14] Emulator decode generalization

[Marcelo Tosatti]

On Tue, Sep 13, 2011 at 10:45:37AM +0300, Avi Kivity wrote:
> - merge dst/src/src2 decode
> - generalize %seg embedded in opcode decode
> 
> plus a fix.
> 
> Avi Kivity (14):
>   KVM: x86 emulator: fix Src2CL decode
>   KVM: x86 emulator: convert group 3 instructions to direct decode
>   KVM: x86 emulator: move memop, memopp into emulation context
>   KVM: x86 emulator: split dst decode to a generic decode_operand()
>   KVM: x86 emulator: expand decode flags to 64 bits
>   KVM: x86 emulator: switch src2 to generic decode_operand()
>   KVM: x86 emulator: free up some flag bits near src, dst
>   KVM: x86 emulator: switch OpImmUByte decode to decode_imm()
>   KVM: x86 emulator: qualify OpReg inhibit_byte_regs hack
>   KVM: x86 emulator: switch src decode to decode_operand()
>   KVM: x86 emulator: simplify OpMem64 decode
>   KVM: x86 emulator: streamline decode of segment registers
>   KVM: x86 emulator: switch lds/les/lss/lfs/lgs to direct decode
>   KVM: x86 emulator: convert push %sreg/pop %sreg to direct decode
> 
>  arch/x86/include/asm/kvm_emulate.h |    4 +-
>  arch/x86/kvm/emulate.c             |  563 ++++++++++++++++++------------------
>  2 files changed, 286 insertions(+), 281 deletions(-)

Applied, thanks.