Re: [PATCH 1/5] KVM: x86 emulator: Use single stage decoding for Group 1 instructions

[Gleb Natapov <gleb@redhat.com>]

On Mon, Mar 14, 2011 at 12:17:27AM +0900, Takuya Yoshikawa wrote:
> From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
> 
> ADD, OR, ADC, SBB, AND, SUB, XOR, CMP are converted using a new macro
> I6ALU(_f, _e).
> 
> CMPS, SCAS will be converted later.
> 
> Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
> ---
>  arch/x86/kvm/emulate.c |  151 ++++++++++++++++++++++++++++--------------------
>  1 files changed, 88 insertions(+), 63 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 14c5ad5..bd9572a 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -2325,6 +2325,70 @@ static int em_mov(struct x86_emulate_ctxt *ctxt)
>  	return X86EMUL_CONTINUE;
>  }
>  
> +static int em_add(struct x86_emulate_ctxt *ctxt)
> +{
> +	struct decode_cache *c = &ctxt->decode;
> +
> +	emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
> +	return X86EMUL_CONTINUE;
> +}
> +
> +static int em_or(struct x86_emulate_ctxt *ctxt)
> +{
> +	struct decode_cache *c = &ctxt->decode;
> +
> +	emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
> +	return X86EMUL_CONTINUE;
> +}
> +
> +static int em_adc(struct x86_emulate_ctxt *ctxt)
> +{
> +	struct decode_cache *c = &ctxt->decode;
> +
> +	emulate_2op_SrcV("adc", c->src, c->dst, ctxt->eflags);
> +	return X86EMUL_CONTINUE;
> +}
> +
> +static int em_sbb(struct x86_emulate_ctxt *ctxt)
> +{
> +	struct decode_cache *c = &ctxt->decode;
> +
> +	emulate_2op_SrcV("sbb", c->src, c->dst, ctxt->eflags);
> +	return X86EMUL_CONTINUE;
> +}
> +
> +static int em_and(struct x86_emulate_ctxt *ctxt)
> +{
> +	struct decode_cache *c = &ctxt->decode;
> +
> +	emulate_2op_SrcV("and", c->src, c->dst, ctxt->eflags);
> +	return X86EMUL_CONTINUE;
> +}
> +
> +static int em_sub(struct x86_emulate_ctxt *ctxt)
> +{
> +	struct decode_cache *c = &ctxt->decode;
> +
> +	emulate_2op_SrcV("sub", c->src, c->dst, ctxt->eflags);
> +	return X86EMUL_CONTINUE;
> +}
> +
> +static int em_xor(struct x86_emulate_ctxt *ctxt)
> +{
> +	struct decode_cache *c = &ctxt->decode;
> +
> +	emulate_2op_SrcV("xor", c->src, c->dst, ctxt->eflags);
> +	return X86EMUL_CONTINUE;
> +}
> +
> +static int em_cmp(struct x86_emulate_ctxt *ctxt)
> +{
> +	struct decode_cache *c = &ctxt->decode;
> +
> +	emulate_2op_SrcV("cmp", c->src, c->dst, ctxt->eflags);
> +	return X86EMUL_CONTINUE;
> +}
> +
>  #define D(_y) { .flags = (_y) }
>  #define N    D(0)
>  #define G(_f, _g) { .flags = ((_f) | Group), .u.group = (_g) }
> @@ -2337,10 +2401,20 @@ static int em_mov(struct x86_emulate_ctxt *ctxt)
>  #define D6ALU(_f) D2bv((_f) | DstMem | SrcReg | ModRM),			\
>  		D2bv(((_f) | DstReg | SrcMem | ModRM) & ~Lock),		\
>  		D2bv(((_f) & ~Lock) | DstAcc | SrcImm)
> +#define I6ALU(_f, _e) I2bv((_f) | DstMem | SrcReg | ModRM, _e),		\
> +		I2bv(((_f) | DstReg | SrcMem | ModRM) & ~Lock, _e),	\
> +		I2bv(((_f) & ~Lock) | DstAcc | SrcImm, _e)
>  
>  
>  static struct opcode group1[] = {
> -	X7(D(Lock)), N
> +	I(Lock, em_add),
> +	I(Lock, em_or),
> +	I(Lock, em_adc),
> +	I(Lock, em_sbb),
> +	I(Lock, em_and),
> +	I(Lock, em_sub),
> +	I(Lock, em_xor),
> +	I(0, em_cmp)
>  };
>  
>  static struct opcode group1A[] = {
> @@ -2396,25 +2470,25 @@ static struct opcode group11[] = {
>  
>  static struct opcode opcode_table[256] = {
>  	/* 0x00 - 0x07 */
> -	D6ALU(Lock),
> +	I6ALU(Lock, em_add),
>  	D(ImplicitOps | Stack | No64), D(ImplicitOps | Stack | No64),
>  	/* 0x08 - 0x0F */
> -	D6ALU(Lock),
> +	I6ALU(Lock, em_or),
>  	D(ImplicitOps | Stack | No64), N,
>  	/* 0x10 - 0x17 */
> -	D6ALU(Lock),
> +	I6ALU(Lock, em_adc),
>  	D(ImplicitOps | Stack | No64), D(ImplicitOps | Stack | No64),
>  	/* 0x18 - 0x1F */
> -	D6ALU(Lock),
> +	I6ALU(Lock, em_sbb),
>  	D(ImplicitOps | Stack | No64), D(ImplicitOps | Stack | No64),
>  	/* 0x20 - 0x27 */
> -	D6ALU(Lock), N, N,
> +	I6ALU(Lock, em_and), N, N,
>  	/* 0x28 - 0x2F */
> -	D6ALU(Lock), N, I(ByteOp | DstAcc | No64, em_das),
> +	I6ALU(Lock, em_sub), N, I(ByteOp | DstAcc | No64, em_das),
>  	/* 0x30 - 0x37 */
> -	D6ALU(Lock), N, N,
> +	I6ALU(Lock, em_xor), N, N,
>  	/* 0x38 - 0x3F */
> -	D6ALU(0), N, N,
> +	I6ALU(0, em_cmp), N, N,
>  	/* 0x40 - 0x4F */
>  	X16(D(DstReg)),
>  	/* 0x50 - 0x57 */
> @@ -2568,6 +2642,7 @@ static struct opcode twobyte_table[256] = {
>  #undef D2bv
>  #undef I2bv
>  #undef D6ALU
> +#undef I6ALU
>  
>  static unsigned imm_size(struct decode_cache *c)
>  {
> @@ -3034,59 +3109,27 @@ special_insn:
>  		goto twobyte_insn;
>  
>  	switch (c->b) {
> -	case 0x00 ... 0x05:
> -	      add:		/* add */
> -		emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
> -		break;
>  	case 0x06:		/* push es */
>  		emulate_push_sreg(ctxt, ops, VCPU_SREG_ES);
>  		break;
>  	case 0x07:		/* pop es */
>  		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
>  		break;
> -	case 0x08 ... 0x0d:
> -	      or:		/* or */
> -		emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
> -		break;
>  	case 0x0e:		/* push cs */
>  		emulate_push_sreg(ctxt, ops, VCPU_SREG_CS);
>  		break;
> -	case 0x10 ... 0x15:
> -	      adc:		/* adc */
> -		emulate_2op_SrcV("adc", c->src, c->dst, ctxt->eflags);
> -		break;
>  	case 0x16:		/* push ss */
>  		emulate_push_sreg(ctxt, ops, VCPU_SREG_SS);
>  		break;
>  	case 0x17:		/* pop ss */
>  		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_SS);
>  		break;
> -	case 0x18 ... 0x1d:
> -	      sbb:		/* sbb */
> -		emulate_2op_SrcV("sbb", c->src, c->dst, ctxt->eflags);
> -		break;
>  	case 0x1e:		/* push ds */
>  		emulate_push_sreg(ctxt, ops, VCPU_SREG_DS);
>  		break;
>  	case 0x1f:		/* pop ds */
>  		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_DS);
>  		break;
> -	case 0x20 ... 0x25:
> -	      and:		/* and */
> -		emulate_2op_SrcV("and", c->src, c->dst, ctxt->eflags);
> -		break;
> -	case 0x28 ... 0x2d:
> -	      sub:		/* sub */
> -		emulate_2op_SrcV("sub", c->src, c->dst, ctxt->eflags);
> -		break;
> -	case 0x30 ... 0x35:
> -	      xor:		/* xor */
> -		emulate_2op_SrcV("xor", c->src, c->dst, ctxt->eflags);
> -		break;
> -	case 0x38 ... 0x3d:
> -	      cmp:		/* cmp */
> -		emulate_2op_SrcV("cmp", c->src, c->dst, ctxt->eflags);
> -		break;
>  	case 0x40 ... 0x47: /* inc r16/r32 */
>  		emulate_1op("inc", c->dst, ctxt->eflags);
>  		break;
> @@ -3121,26 +3164,6 @@ special_insn:
>  		if (test_cc(c->b, ctxt->eflags))
>  			jmp_rel(c, c->src.val);
>  		break;
> -	case 0x80 ... 0x83:	/* Grp1 */
> -		switch (c->modrm_reg) {
> -		case 0:
> -			goto add;
> -		case 1:
> -			goto or;
> -		case 2:
> -			goto adc;
> -		case 3:
> -			goto sbb;
> -		case 4:
> -			goto and;
> -		case 5:
> -			goto sub;
> -		case 6:
> -			goto xor;
> -		case 7:
> -			goto cmp;
> -		}
> -		break;
>  	case 0x84 ... 0x85:
>  	test:
>  		emulate_2op_SrcV("test", c->src, c->dst, ctxt->eflags);
> @@ -3212,11 +3235,13 @@ special_insn:
>  		break;
>  	case 0xa6 ... 0xa7:	/* cmps */
>  		c->dst.type = OP_NONE; /* Disable writeback. */
> -		goto cmp;
> +		emulate_2op_SrcV("cmp", c->src, c->dst, ctxt->eflags);
Why not call em_cmp() here?
> +		break;
>  	case 0xa8 ... 0xa9:	/* test ax, imm */
>  		goto test;
>  	case 0xae ... 0xaf:	/* scas */
> -		goto cmp;
> +		emulate_2op_SrcV("cmp", c->src, c->dst, ctxt->eflags);
And here?

> +		break;
>  	case 0xc0 ... 0xc1:
>  		emulate_grp2(ctxt);
>  		break;
> -- 
> 1.7.1

--
			Gleb.