[PATCH 0/8] Simplify byte/word opcode pair decode

[PATCH 0/8] Simplify byte/word opcode pair decode

[Avi Kivity]

Many x86 instructions come in pairs - byte and word variants where bit 0
of the opcode determines operand size.  Use this to simplify the decode
tables.

Avi Kivity (8):
  KVM: x86 emulator: support byte/word opcode pairs
  KVM: x86 emulator: simplify ALU block (opcodes 00-3F) decode flags
  KVM: x86 emulator: simplify string instruction decode flags
  KVM: x86 emulator: simplify instruction decode flags for opcodes
    80-8F
  KVM: x86 emulator: simplify instruction decode flags for opcodes
    A0-AF
  KVM: x86 emulator: simplify instruction decode flags for opcodes
    C0-DF
  KVM: x86 emulator: simplify instruction decode flags for opcodes
    E0-FF
  KVM: x86 emulator: simplify instruction decode flags for opcodes 0F
    00-FF

 arch/x86/kvm/emulate.c |   89 ++++++++++++++++++++++--------------------------
 1 files changed, 41 insertions(+), 48 deletions(-)

[PATCH 1/8] KVM: x86 emulator: support byte/word opcode pairs

[Avi Kivity]

Many x86 instructions come in byte and word variants distinguished with bit
0 of the opcode.  Add macros to aid in defining them.

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

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 3b35e13..152654e 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2315,6 +2315,9 @@ static int em_rdtsc(struct x86_emulate_ctxt *ctxt)
 #define GD(_f, _g) { .flags = ((_f) | Group | GroupDual), .u.gdual = (_g) }
 #define I(_f, _e) { .flags = (_f), .u.execute = (_e) }
 
+#define D2bv(_f)      D((_f) | ByteOp), D(_f)
+#define I2bv(_f, _e)  I((_f) | ByteOp, _e), I(_f, _e)
+
 static struct opcode group1[] = {
 	X7(D(Lock)), N
 };
@@ -2557,6 +2560,9 @@ static struct opcode twobyte_table[256] = {
 #undef GD
 #undef I
 
+#undef D2bv
+#undef I2bv
+
 static unsigned imm_size(struct decode_cache *c)
 {
 	unsigned size;
-- 
1.7.1

[PATCH 2/8] KVM: x86 emulator: simplify ALU block (opcodes 00-3F) decode flags

[Avi Kivity]

Use the new byte/word dual opcode decode.

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

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 152654e..79a5f81 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2370,42 +2370,34 @@ static struct group_dual group9 = { {
 
 static struct opcode opcode_table[256] = {
 	/* 0x00 - 0x07 */
-	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
-	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
-	D(ByteOp | DstAcc | SrcImm), D(DstAcc | SrcImm),
+	D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
+	D2bv(DstAcc | SrcImm),
 	D(ImplicitOps | Stack | No64), D(ImplicitOps | Stack | No64),
 	/* 0x08 - 0x0F */
-	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
-	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
-	D(ByteOp | DstAcc | SrcImm), D(DstAcc | SrcImm),
+	D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
+	D2bv(DstAcc | SrcImm),
 	D(ImplicitOps | Stack | No64), N,
 	/* 0x10 - 0x17 */
-	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
-	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
-	D(ByteOp | DstAcc | SrcImm), D(DstAcc | SrcImm),
+	D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
+	D2bv(DstAcc | SrcImm),
 	D(ImplicitOps | Stack | No64), D(ImplicitOps | Stack | No64),
 	/* 0x18 - 0x1F */
-	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
-	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
-	D(ByteOp | DstAcc | SrcImm), D(DstAcc | SrcImm),
+	D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
+	D2bv(DstAcc | SrcImm),
 	D(ImplicitOps | Stack | No64), D(ImplicitOps | Stack | No64),
 	/* 0x20 - 0x27 */
-	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
-	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
-	D(ByteOp | DstAcc | SrcImmByte), D(DstAcc | SrcImm), N, N,
+	D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
+	D2bv(DstAcc | SrcImmByte), N, N,
 	/* 0x28 - 0x2F */
-	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
-	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
-	D(ByteOp | DstAcc | SrcImmByte), D(DstAcc | SrcImm),
+	D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
+	D2bv(DstAcc | SrcImm),
 	N, I(ByteOp | DstAcc | No64, em_das),
 	/* 0x30 - 0x37 */
-	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
-	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
-	D(ByteOp | DstAcc | SrcImmByte), D(DstAcc | SrcImm), N, N,
+	D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
+	D2bv(DstAcc | SrcImm), N, N,
 	/* 0x38 - 0x3F */
-	D(ByteOp | DstMem | SrcReg | ModRM), D(DstMem | SrcReg | ModRM),
-	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
-	D(ByteOp | DstAcc | SrcImm), D(DstAcc | SrcImm),
+	D2bv(DstMem | SrcReg | ModRM), D2bv(DstReg | SrcMem | ModRM),
+	D2bv(DstAcc | SrcImm),
 	N, N,
 	/* 0x40 - 0x4F */
 	X16(D(DstReg)),
-- 
1.7.1

[PATCH 3/8] KVM: x86 emulator: simplify string instruction decode flags

[Avi Kivity]

Use the new byte/word dual opcode decode.

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

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 79a5f81..2a480ce 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2414,8 +2414,8 @@ static struct opcode opcode_table[256] = {
 	I(DstReg | SrcMem | ModRM | Src2Imm, em_imul_3op),
 	I(SrcImmByte | Mov | Stack, em_push),
 	I(DstReg | SrcMem | ModRM | Src2ImmByte, em_imul_3op),
-	D(DstDI | ByteOp | Mov | String), D(DstDI | Mov | String), /* insb, insw/insd */
-	D(SrcSI | ByteOp | ImplicitOps | String), D(SrcSI | ImplicitOps | String), /* outsb, outsw/outsd */
+	D2bv(DstDI | Mov | String), /* insb, insw/insd */
+	D2bv(SrcSI | ImplicitOps | String), /* outsb, outsw/outsd */
 	/* 0x70 - 0x7F */
 	X16(D(SrcImmByte)),
 	/* 0x80 - 0x87 */
@@ -2439,13 +2439,12 @@ static struct opcode opcode_table[256] = {
 	/* 0xA0 - 0xA7 */
 	D(ByteOp | DstAcc | SrcMem | Mov | MemAbs), D(DstAcc | SrcMem | Mov | MemAbs),
 	D(ByteOp | DstMem | SrcAcc | Mov | MemAbs), D(DstMem | SrcAcc | Mov | MemAbs),
-	D(ByteOp | SrcSI | DstDI | Mov | String), D(SrcSI | DstDI | Mov | String),
-	D(ByteOp | SrcSI | DstDI | String), D(SrcSI | DstDI | String),
+	D2bv(SrcSI | DstDI | Mov | String), D2bv(SrcSI | DstDI | String),
 	/* 0xA8 - 0xAF */
 	D(DstAcc | SrcImmByte | ByteOp), D(DstAcc | SrcImm),
-	D(ByteOp | SrcAcc | DstDI | Mov | String), D(SrcAcc | DstDI | Mov | String),
-	D(ByteOp | SrcSI | DstAcc | Mov | String), D(SrcSI | DstAcc | Mov | String),
-	D(ByteOp | SrcAcc | DstDI | String), D(SrcAcc | DstDI | String),
+	D2bv(SrcAcc | DstDI | Mov | String),
+	D2bv(SrcSI | DstAcc | Mov | String),
+	D2bv(SrcAcc | DstDI | String),
 	/* 0xB0 - 0xB7 */
 	X8(D(ByteOp | DstReg | SrcImm | Mov)),
 	/* 0xB8 - 0xBF */
-- 
1.7.1

[PATCH 4/8] KVM: x86 emulator: simplify instruction decode flags for opcodes 80-8F

[Avi Kivity]

Use the new byte/word dual opcode decode.

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

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 2a480ce..1c174e4 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2423,11 +2423,10 @@ static struct opcode opcode_table[256] = {
 	G(DstMem | SrcImm | ModRM | Group, group1),
 	G(ByteOp | DstMem | SrcImm | ModRM | No64 | Group, group1),
 	G(DstMem | SrcImmByte | ModRM | Group, group1),
-	D(ByteOp | DstMem | SrcReg | ModRM), D(DstMem | SrcReg | ModRM),
-	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
+	D2bv(DstMem | SrcReg | ModRM), D2bv(DstMem | SrcReg | ModRM | Lock),
 	/* 0x88 - 0x8F */
-	D(ByteOp | DstMem | SrcReg | ModRM | Mov), D(DstMem | SrcReg | ModRM | Mov),
-	D(ByteOp | DstReg | SrcMem | ModRM | Mov), D(DstReg | SrcMem | ModRM | Mov),
+	D2bv(DstMem | SrcReg | ModRM | Mov),
+	D2bv(DstReg | SrcMem | ModRM | Mov),
 	D(DstMem | SrcNone | ModRM | Mov), D(ModRM | SrcMem | NoAccess | DstReg),
 	D(ImplicitOps | SrcMem16 | ModRM), G(0, group1A),
 	/* 0x90 - 0x97 */
-- 
1.7.1

[PATCH 5/8] KVM: x86 emulator: simplify instruction decode flags for opcodes A0-AF

[Avi Kivity]

Use the new byte/word dual opcode decode.

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

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 1c174e4..6888502 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2436,11 +2436,11 @@ static struct opcode opcode_table[256] = {
 	D(SrcImmFAddr | No64), N,
 	D(ImplicitOps | Stack), D(ImplicitOps | Stack), N, N,
 	/* 0xA0 - 0xA7 */
-	D(ByteOp | DstAcc | SrcMem | Mov | MemAbs), D(DstAcc | SrcMem | Mov | MemAbs),
-	D(ByteOp | DstMem | SrcAcc | Mov | MemAbs), D(DstMem | SrcAcc | Mov | MemAbs),
+	D2bv(DstAcc | SrcMem | Mov | MemAbs),
+	D2bv(DstMem | SrcAcc | Mov | MemAbs),
 	D2bv(SrcSI | DstDI | Mov | String), D2bv(SrcSI | DstDI | String),
 	/* 0xA8 - 0xAF */
-	D(DstAcc | SrcImmByte | ByteOp), D(DstAcc | SrcImm),
+	D2bv(DstAcc | SrcImm),
 	D2bv(SrcAcc | DstDI | Mov | String),
 	D2bv(SrcSI | DstAcc | Mov | String),
 	D2bv(SrcAcc | DstDI | String),
-- 
1.7.1

[PATCH 6/8] KVM: x86 emulator: simplify instruction decode flags for opcodes C0-DF

[Avi Kivity]

Use the new byte/word dual opcode decode.

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

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 6888502..c61f73d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2449,17 +2449,16 @@ static struct opcode opcode_table[256] = {
 	/* 0xB8 - 0xBF */
 	X8(D(DstReg | SrcImm | Mov)),
 	/* 0xC0 - 0xC7 */
-	D(ByteOp | DstMem | SrcImm | ModRM), D(DstMem | SrcImmByte | ModRM),
+	D2bv(DstMem | SrcImm | ModRM),
 	I(ImplicitOps | Stack | SrcImmU16, em_ret_near_imm),
 	D(ImplicitOps | Stack),
 	N, N,
-	D(ByteOp | DstMem | SrcImm | ModRM | Mov), D(DstMem | SrcImm | ModRM | Mov),
+	D2bv(DstMem | SrcImm | ModRM | Mov),
 	/* 0xC8 - 0xCF */
 	N, N, N, D(ImplicitOps | Stack),
 	D(ImplicitOps), D(SrcImmByte), D(ImplicitOps | No64), D(ImplicitOps),
 	/* 0xD0 - 0xD7 */
-	D(ByteOp | DstMem | SrcOne | ModRM), D(DstMem | SrcOne | ModRM),
-	D(ByteOp | DstMem | ModRM), D(DstMem | ModRM),
+	D2bv(DstMem | SrcOne | ModRM), D2bv(DstMem | ModRM),
 	N, N, N, N,
 	/* 0xD8 - 0xDF */
 	N, N, N, N, N, N, N, N,
-- 
1.7.1

[PATCH 7/8] KVM: x86 emulator: simplify instruction decode flags for opcodes E0-FF

[Avi Kivity]

Use the new byte/word dual opcode decode.

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

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index c61f73d..6cb5663 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2464,13 +2464,11 @@ static struct opcode opcode_table[256] = {
 	N, N, N, N, N, N, N, N,
 	/* 0xE0 - 0xE7 */
 	X3(D(SrcImmByte)), N,
-	D(ByteOp | SrcImmUByte | DstAcc), D(SrcImmUByte | DstAcc),
-	D(ByteOp | SrcAcc | DstImmUByte), D(SrcAcc | DstImmUByte),
+	D2bv(SrcImmUByte | DstAcc), D2bv(SrcAcc | DstImmUByte),
 	/* 0xE8 - 0xEF */
 	D(SrcImm | Stack), D(SrcImm | ImplicitOps),
 	D(SrcImmFAddr | No64), D(SrcImmByte | ImplicitOps),
-	D(SrcNone | ByteOp | DstAcc), D(SrcNone | DstAcc),
-	D(ByteOp | SrcAcc | ImplicitOps), D(SrcAcc | ImplicitOps),
+	D2bv(SrcNone | DstAcc),	D2bv(SrcAcc | ImplicitOps),
 	/* 0xF0 - 0xF7 */
 	N, N, N, N,
 	D(ImplicitOps | Priv), D(ImplicitOps), G(ByteOp, group3), G(0, group3),
-- 
1.7.1

[PATCH 8/8] KVM: x86 emulator: simplify instruction decode flags for opcodes 0F 00-FF

[Avi Kivity]

Use the new byte/word dual opcode decode.

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

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 6cb5663..808934c 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2519,7 +2519,7 @@ static struct opcode twobyte_table[256] = {
 	D(DstMem | SrcReg | Src2CL | ModRM),
 	D(ModRM), I(DstReg | SrcMem | ModRM, em_imul),
 	/* 0xB0 - 0xB7 */
-	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
+	D2bv(DstMem | SrcReg | ModRM | Lock),
 	N, D(DstMem | SrcReg | ModRM | BitOp | Lock),
 	N, N, D(ByteOp | DstReg | SrcMem | ModRM | Mov),
 	    D(DstReg | SrcMem16 | ModRM | Mov),
@@ -2529,7 +2529,7 @@ static struct opcode twobyte_table[256] = {
 	D(DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
 	D(ByteOp | DstReg | SrcMem | ModRM | Mov), D(DstReg | SrcMem16 | ModRM | Mov),
 	/* 0xC0 - 0xCF */
-	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
+	D2bv(DstMem | SrcReg | ModRM | Lock),
 	N, D(DstMem | SrcReg | ModRM | Mov),
 	N, N, N, GD(0, &group9),
 	N, N, N, N, N, N, N, N,
-- 
1.7.1

Re: [PATCH 0/8] Simplify byte/word opcode pair decode

[Paolo Bonzini]

On 08/23/2010 01:06 PM, Avi Kivity wrote:
> Many x86 instructions come in pairs - byte and word variants where bit 0
> of the opcode determines operand size.  Use this to simplify the decode
> tables.

Many actually come in quadruples, bit 0 is the size and bit 1 says which 
of ModRM and REG is the destination.

example:

D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),

Paolo

Re: [PATCH 7/8] KVM: x86 emulator: simplify instruction decode flags for opcodes E0-FF

[Gleb Natapov]

On Mon, Aug 23, 2010 at 02:06:16PM +0300, Avi Kivity wrote:
> Use the new byte/word dual opcode decode.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/kvm/emulate.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index c61f73d..6cb5663 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -2464,13 +2464,11 @@ static struct opcode opcode_table[256] = {
>  	N, N, N, N, N, N, N, N,
>  	/* 0xE0 - 0xE7 */
>  	X3(D(SrcImmByte)), N,
> -	D(ByteOp | SrcImmUByte | DstAcc), D(SrcImmUByte | DstAcc),
> -	D(ByteOp | SrcAcc | DstImmUByte), D(SrcAcc | DstImmUByte),
> +	D2bv(SrcImmUByte | DstAcc), D2bv(SrcAcc | DstImmUByte),
>  	/* 0xE8 - 0xEF */
>  	D(SrcImm | Stack), D(SrcImm | ImplicitOps),
>  	D(SrcImmFAddr | No64), D(SrcImmByte | ImplicitOps),
> -	D(SrcNone | ByteOp | DstAcc), D(SrcNone | DstAcc),
> -	D(ByteOp | SrcAcc | ImplicitOps), D(SrcAcc | ImplicitOps),
> +	D2bv(SrcNone | DstAcc),	D2bv(SrcAcc | ImplicitOps),
>  	/* 0xF0 - 0xF7 */
>  	N, N, N, N,
>  	D(ImplicitOps | Priv), D(ImplicitOps), G(ByteOp, group3), G(0, group3),
Hmm. I actually have a patch that makes encoding of those to be different.

===
    Some instructions has 32 bit operand even in long mode.
    
    Decode this through decode table instead of during instruction
    emulation.

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 3b35e13..b4361fa 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -84,6 +84,7 @@
 #define Group       (1<<14)     /* Bits 3:5 of modrm byte extend opcode */
 #define GroupDual   (1<<15)     /* Alternate decoding of mod == 3 */
 /* Misc flags */
+#define Op32in64    (1<<22) /* Operand is 32b even in long mode */
 #define NoAccess    (1<<23) /* Don't access memory (lea/invlpg/verr etc) */
 #define Op3264      (1<<24) /* Operand is 64b in long mode, 32b otherwise */
 #define Undefined   (1<<25) /* No Such Instruction */
@@ -2472,13 +2473,13 @@ static struct opcode opcode_table[256] = {
 	N, N, N, N, N, N, N, N,
 	/* 0xE0 - 0xE7 */
 	X3(D(SrcImmByte)), N,
-	D(ByteOp | SrcImmUByte | DstAcc), D(SrcImmUByte | DstAcc),
-	D(ByteOp | SrcAcc | DstImmUByte), D(SrcAcc | DstImmUByte),
+	D(ByteOp | SrcImmUByte | DstAcc), D(SrcImmUByte | DstAcc | Op32in64),
+	D(ByteOp | SrcAcc | DstImmUByte), D(SrcAcc | DstImmUByte | Op32in64),
 	/* 0xE8 - 0xEF */
 	D(SrcImm | Stack), D(SrcImm | ImplicitOps),
 	D(SrcImmFAddr | No64), D(SrcImmByte | ImplicitOps),
-	D(SrcNone | ByteOp | DstAcc), D(SrcNone | DstAcc),
-	D(ByteOp | SrcAcc | ImplicitOps), D(SrcAcc | ImplicitOps),
+	D(SrcNone | ByteOp | DstAcc), D(SrcNone | DstAcc | Op32in64),
+	D(ByteOp | SrcAcc | ImplicitOps), D(SrcAcc | ImplicitOps | Op32in64),
 	/* 0xF0 - 0xF7 */
 	N, N, N, N,
 	D(ImplicitOps | Priv), D(ImplicitOps), G(ByteOp, group3), G(0, group3),
@@ -2750,6 +2751,9 @@ done_prefixes:
 			c->op_bytes = 4;
 	}
 
+	if (mode == X86EMUL_MODE_PROT64 && (c->d & Op32in64))
+		c->op_bytes = 4;
+
 	/* ModRM and SIB bytes. */
 	if (c->d & ModRM) {
 		rc = decode_modrm(ctxt, ops, &memop);
@@ -3295,7 +3299,6 @@ special_insn:
 	case 0xed: /* in (e/r)ax,dx */
 		c->src.val = c->regs[VCPU_REGS_RDX];
 	do_io_in:
-		c->dst.bytes = min(c->dst.bytes, 4u);
 		if (!emulator_io_permited(ctxt, ops, c->src.val, c->dst.bytes)) {
 			emulate_gp(ctxt, 0);
 			goto done;
@@ -3308,7 +3311,6 @@ special_insn:
 	case 0xef: /* out dx,(e/r)ax */
 		c->dst.val = c->regs[VCPU_REGS_RDX];
 	do_io_out:
-		c->src.bytes = min(c->src.bytes, 4u);
 		if (!emulator_io_permited(ctxt, ops, c->dst.val,
 					  c->src.bytes)) {
 			emulate_gp(ctxt, 0);
--
			Gleb.

Re: [PATCH 7/8] KVM: x86 emulator: simplify instruction decode flags for opcodes E0-FF

[Avi Kivity]

  On 08/23/2010 04:33 PM, Gleb Natapov wrote:
> On Mon, Aug 23, 2010 at 02:06:16PM +0300, Avi Kivity wrote:
>> Use the new byte/word dual opcode decode.
>>
>> Signed-off-by: Avi Kivity<avi@redhat.com>
>> ---
>>   arch/x86/kvm/emulate.c |    6 ++----
>>   1 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index c61f73d..6cb5663 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -2464,13 +2464,11 @@ static struct opcode opcode_table[256] = {
>>   	N, N, N, N, N, N, N, N,
>>   	/* 0xE0 - 0xE7 */
>>   	X3(D(SrcImmByte)), N,
>> -	D(ByteOp | SrcImmUByte | DstAcc), D(SrcImmUByte | DstAcc),
>> -	D(ByteOp | SrcAcc | DstImmUByte), D(SrcAcc | DstImmUByte),
>> +	D2bv(SrcImmUByte | DstAcc), D2bv(SrcAcc | DstImmUByte),
>>   	/* 0xE8 - 0xEF */
>>   	D(SrcImm | Stack), D(SrcImm | ImplicitOps),
>>   	D(SrcImmFAddr | No64), D(SrcImmByte | ImplicitOps),
>> -	D(SrcNone | ByteOp | DstAcc), D(SrcNone | DstAcc),
>> -	D(ByteOp | SrcAcc | ImplicitOps), D(SrcAcc | ImplicitOps),
>> +	D2bv(SrcNone | DstAcc),	D2bv(SrcAcc | ImplicitOps),
>>   	/* 0xF0 - 0xF7 */
>>   	N, N, N, N,
>>   	D(ImplicitOps | Priv), D(ImplicitOps), G(ByteOp, group3), G(0, group3),
> Hmm. I actually have a patch that makes encoding of those to be different.
>
> ===
>      Some instructions has 32 bit operand even in long mode.
>
>      Decode this through decode table instead of during instruction
>      emulation.
>

This can still work.

> @@ -2750,6 +2751,9 @@ done_prefixes:
>   			c->op_bytes = 4;
>   	}
>
> +	if (mode == X86EMUL_MODE_PROT64&&  (c->d&  Op32in64))
> +		c->op_bytes = 4;

Just check that it isn't a ByteOp.

btw, shouldn't that be

     if (c->op_bytes == 8 && (c->d & Op32in64))

?  to account for c->op_bytes = 2 initially.


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

Re: [PATCH 0/8] Simplify byte/word opcode pair decode

[Avi Kivity]

  On 08/23/2010 04:29 PM, Paolo Bonzini wrote:
> On 08/23/2010 01:06 PM, Avi Kivity wrote:
>> Many x86 instructions come in pairs - byte and word variants where bit 0
>> of the opcode determines operand size.  Use this to simplify the decode
>> tables.
>
> Many actually come in quadruples, bit 0 is the size and bit 1 says 
> which of ModRM and REG is the destination.
>
> example:
>
> D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
>

Right.  We can't exploit those yet.

I'd like to make operand encoding regular (same bit definitions for Src, 
Dst, and Src2) which would allow us to do this (and have a 
decode_operand() and fetch_operand() instead of the current three switches).

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

Re: [PATCH 7/8] KVM: x86 emulator: simplify instruction decode flags for opcodes E0-FF

[Gleb Natapov]

On Mon, Aug 23, 2010 at 04:46:01PM +0300, Avi Kivity wrote:
>  On 08/23/2010 04:33 PM, Gleb Natapov wrote:
> >On Mon, Aug 23, 2010 at 02:06:16PM +0300, Avi Kivity wrote:
> >>Use the new byte/word dual opcode decode.
> >>
> >>Signed-off-by: Avi Kivity<avi@redhat.com>
> >>---
> >>  arch/x86/kvm/emulate.c |    6 ++----
> >>  1 files changed, 2 insertions(+), 4 deletions(-)
> >>
> >>diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> >>index c61f73d..6cb5663 100644
> >>--- a/arch/x86/kvm/emulate.c
> >>+++ b/arch/x86/kvm/emulate.c
> >>@@ -2464,13 +2464,11 @@ static struct opcode opcode_table[256] = {
> >>  	N, N, N, N, N, N, N, N,
> >>  	/* 0xE0 - 0xE7 */
> >>  	X3(D(SrcImmByte)), N,
> >>-	D(ByteOp | SrcImmUByte | DstAcc), D(SrcImmUByte | DstAcc),
> >>-	D(ByteOp | SrcAcc | DstImmUByte), D(SrcAcc | DstImmUByte),
> >>+	D2bv(SrcImmUByte | DstAcc), D2bv(SrcAcc | DstImmUByte),
> >>  	/* 0xE8 - 0xEF */
> >>  	D(SrcImm | Stack), D(SrcImm | ImplicitOps),
> >>  	D(SrcImmFAddr | No64), D(SrcImmByte | ImplicitOps),
> >>-	D(SrcNone | ByteOp | DstAcc), D(SrcNone | DstAcc),
> >>-	D(ByteOp | SrcAcc | ImplicitOps), D(SrcAcc | ImplicitOps),
> >>+	D2bv(SrcNone | DstAcc),	D2bv(SrcAcc | ImplicitOps),
> >>  	/* 0xF0 - 0xF7 */
> >>  	N, N, N, N,
> >>  	D(ImplicitOps | Priv), D(ImplicitOps), G(ByteOp, group3), G(0, group3),
> >Hmm. I actually have a patch that makes encoding of those to be different.
> >
> >===
> >     Some instructions has 32 bit operand even in long mode.
> >
> >     Decode this through decode table instead of during instruction
> >     emulation.
> >
> 
> This can still work.
> 
> >@@ -2750,6 +2751,9 @@ done_prefixes:
> >  			c->op_bytes = 4;
> >  	}
> >
> >+	if (mode == X86EMUL_MODE_PROT64&&  (c->d&  Op32in64))
> >+		c->op_bytes = 4;
> 
> Just check that it isn't a ByteOp.
> 
Hmm. It seams that even that is not needed. We have
 c->dst.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
everywhere.

> btw, shouldn't that be
> 
>     if (c->op_bytes == 8 && (c->d & Op32in64))
> 
> ?  to account for c->op_bytes = 2 initially.
> 
> 
Initially it will be 4 in X86EMUL_MODE_PROT64, but it may be overridden
to 2, so yes we should check c->op_bytes == 8 not X86EMUL_MODE_PROT64.

--
			Gleb.

Re: [PATCH 0/8] Simplify byte/word opcode pair decode

[Paolo Bonzini]

On 08/23/2010 03:48 PM, Avi Kivity wrote:
> On 08/23/2010 04:29 PM, Paolo Bonzini wrote:
>> On 08/23/2010 01:06 PM, Avi Kivity wrote:
>>> Many x86 instructions come in pairs - byte and word variants where bit 0
>>> of the opcode determines operand size. Use this to simplify the decode
>>> tables.
>>
>> Many actually come in quadruples, bit 0 is the size and bit 1 says
>> which of ModRM and REG is the destination.
>>
>> example:
>>
>> D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
>>
>
> Right. We can't exploit those yet.

What about this?  It wouldn't work for things such as SrcAcc, but it 
would still reap most of the benefit:

#define D4modrm(x) \
   D2bv(DstMem | SrcReg | ModRM | (x)), \
   D2bv(DstReg | SrcMem | ModRM | ((x) & ~Lock))

e.g.
- ALU block -> D4modrm(Lock)
- 0x88-0x8b -> D4modrm(Mov)

Paolo

Re: [PATCH 0/8] Simplify byte/word opcode pair decode

[Avi Kivity]

  On 08/23/2010 05:04 PM, Paolo Bonzini wrote:
> On 08/23/2010 03:48 PM, Avi Kivity wrote:
>> On 08/23/2010 04:29 PM, Paolo Bonzini wrote:
>>> On 08/23/2010 01:06 PM, Avi Kivity wrote:
>>>> Many x86 instructions come in pairs - byte and word variants where 
>>>> bit 0
>>>> of the opcode determines operand size. Use this to simplify the decode
>>>> tables.
>>>
>>> Many actually come in quadruples, bit 0 is the size and bit 1 says
>>> which of ModRM and REG is the destination.
>>>
>>> example:
>>>
>>> D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
>>>
>>
>> Right. We can't exploit those yet.
>
> What about this?  It wouldn't work for things such as SrcAcc, but it 
> would still reap most of the benefit:
>
> #define D4modrm(x) \
>   D2bv(DstMem | SrcReg | ModRM | (x)), \
>   D2bv(DstReg | SrcMem | ModRM | ((x) & ~Lock))
>

It's fine except for the name. "bv" stands for the single letter flags 
Intel gave to byte and word operations so it's somewhat descriptive.  
Maybe we could call it ALU4() and pretend that mov is an ALU 
instruction.  We could also do an ALU6() for the ALU block.

A really clever macro could probably compress the entire table into a 
single line.

> e.g.
> - ALU block -> D4modrm(Lock)
> - 0x88-0x8b -> D4modrm(Mov)


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

Re: [PATCH 2/8] KVM: x86 emulator: simplify ALU block (opcodes 00-3F) decode flags

[Marcelo Tosatti]

On Mon, Aug 23, 2010 at 02:06:11PM +0300, Avi Kivity wrote:
> Use the new byte/word dual opcode decode.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/kvm/emulate.c |   40 ++++++++++++++++------------------------
>  1 files changed, 16 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 152654e..79a5f81 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -2370,42 +2370,34 @@ static struct group_dual group9 = { {
>  
>  static struct opcode opcode_table[256] = {
>  	/* 0x00 - 0x07 */
> -	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
> -	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
> -	D(ByteOp | DstAcc | SrcImm), D(DstAcc | SrcImm),
> +	D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
> +	D2bv(DstAcc | SrcImm),
>  	D(ImplicitOps | Stack | No64), D(ImplicitOps | Stack | No64),
>  	/* 0x08 - 0x0F */
> -	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
> -	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
> -	D(ByteOp | DstAcc | SrcImm), D(DstAcc | SrcImm),
> +	D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
> +	D2bv(DstAcc | SrcImm),
>  	D(ImplicitOps | Stack | No64), N,
>  	/* 0x10 - 0x17 */
> -	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
> -	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
> -	D(ByteOp | DstAcc | SrcImm), D(DstAcc | SrcImm),
> +	D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
> +	D2bv(DstAcc | SrcImm),
>  	D(ImplicitOps | Stack | No64), D(ImplicitOps | Stack | No64),
>  	/* 0x18 - 0x1F */
> -	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
> -	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
> -	D(ByteOp | DstAcc | SrcImm), D(DstAcc | SrcImm),
> +	D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
> +	D2bv(DstAcc | SrcImm),
>  	D(ImplicitOps | Stack | No64), D(ImplicitOps | Stack | No64),
>  	/* 0x20 - 0x27 */
> -	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
> -	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
> -	D(ByteOp | DstAcc | SrcImmByte), D(DstAcc | SrcImm), N, N,
> +	D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
> +	D2bv(DstAcc | SrcImmByte), N, N,

SrcImm? opcode 0x25...

Re: [PATCH 2/8] KVM: x86 emulator: simplify ALU block (opcodes 00-3F) decode flags

[Avi Kivity]

  On 08/25/2010 10:39 PM, Marcelo Tosatti wrote:
>
>>   	/* 0x20 - 0x27 */
>> -	D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
>> -	D(ByteOp | DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
>> -	D(ByteOp | DstAcc | SrcImmByte), D(DstAcc | SrcImm), N, N,
>> +	D2bv(DstMem | SrcReg | ModRM | Lock), D2bv(DstReg | SrcMem | ModRM),
>> +	D2bv(DstAcc | SrcImmByte), N, N,
> SrcImm? opcode 0x25...
>
Ouch!

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