[PATCH 1/6] KVM: x86 emulator: fix 'mov sreg,rm16' instruction decoding

[PATCH 1/6] KVM: x86 emulator: fix 'mov sreg,rm16' instruction decoding

[Wei Yongjun]

Memory reads for 'mov sreg,rm16' should be 16 bits only.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.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 e8bdddc..d842a7d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -170,7 +170,7 @@ static u32 opcode_table[256] = {
 	ByteOp | DstMem | SrcReg | ModRM | Mov, DstMem | SrcReg | ModRM | Mov,
 	ByteOp | DstReg | SrcMem | ModRM | Mov, DstReg | SrcMem | ModRM | Mov,
 	DstMem | SrcReg | ModRM | Mov, ModRM | DstReg,
-	ImplicitOps | SrcMem | ModRM, Group | Group1A,
+	ImplicitOps | SrcMem16 | ModRM, Group | Group1A,
 	/* 0x90 - 0x97 */
 	DstReg, DstReg, DstReg, DstReg,	DstReg, DstReg, DstReg, DstReg,
 	/* 0x98 - 0x9F */
-- 
1.7.0.4

[PATCH 2/6] KVM: x86 emulator: fix the comment of out instruction

[Wei Yongjun]

Fix the comment of out instruction, using the same style as the
other instructions.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.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 d842a7d..ad8d7cd 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2949,8 +2949,8 @@ special_insn:
 				     &c->dst.val))
 			goto done; /* IO is needed */
 		break;
-	case 0xee: /* out al,dx */
-	case 0xef: /* out (e/r)ax,dx */
+	case 0xee: /* out dx,al */
+	case 0xef: /* out dx,(e/r)ax */
 		c->src.val = c->regs[VCPU_REGS_RDX];
 	do_io_out:
 		c->dst.bytes = min(c->dst.bytes, 4u);
-- 
1.7.0.4

[PATCH 3/6] KVM: x86 emulator: fix 'and AL,imm8' instruction decoding

[Wei Yongjun]

'and AL,imm8' should be mask as ByteOp, otherwise the dest operand
length will no correct and we may fill the full EAX when writeback.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.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 ad8d7cd..59568ad 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -123,7 +123,7 @@ static u32 opcode_table[256] = {
 	/* 0x20 - 0x27 */
 	ByteOp | DstMem | SrcReg | ModRM | Lock, DstMem | SrcReg | ModRM | Lock,
 	ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-	DstAcc | SrcImmByte, DstAcc | SrcImm, 0, 0,
+	ByteOp | DstAcc | SrcImmByte, DstAcc | SrcImm, 0, 0,
 	/* 0x28 - 0x2F */
 	ByteOp | DstMem | SrcReg | ModRM | Lock, DstMem | SrcReg | ModRM | Lock,
 	ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-- 
1.7.0.4

[PATCH 4/6] KVM: x86 emulator: fix 'mov rm,sreg' instruction decoding

[Wei Yongjun]

The source operand of 'mov rm,sreg' is segment register, not
general-purpose register, so remove SrcReg from decoding.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.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 59568ad..8337567 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -169,7 +169,7 @@ static u32 opcode_table[256] = {
 	/* 0x88 - 0x8F */
 	ByteOp | DstMem | SrcReg | ModRM | Mov, DstMem | SrcReg | ModRM | Mov,
 	ByteOp | DstReg | SrcMem | ModRM | Mov, DstReg | SrcMem | ModRM | Mov,
-	DstMem | SrcReg | ModRM | Mov, ModRM | DstReg,
+	DstMem | SrcNone | ModRM | Mov, ModRM | DstReg,
 	ImplicitOps | SrcMem16 | ModRM, Group | Group1A,
 	/* 0x90 - 0x97 */
 	DstReg, DstReg, DstReg, DstReg,	DstReg, DstReg, DstReg, DstReg,
-- 
1.7.0.4

[PATCH 5/6] KVM: x86 emulator: fix 'mov AL,moffs' instruction decoding

[Wei Yongjun]

'mov AL,moffs' do not need decode dest operand and
'mov moffs,AL' do not need decode source operand.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.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 8337567..d4526f2 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -177,8 +177,8 @@ static u32 opcode_table[256] = {
 	0, 0, SrcImmFAddr | No64, 0,
 	ImplicitOps | Stack, ImplicitOps | Stack, 0, 0,
 	/* 0xA0 - 0xA7 */
-	ByteOp | DstReg | SrcMem | Mov | MemAbs, DstReg | SrcMem | Mov | MemAbs,
-	ByteOp | DstMem | SrcReg | Mov | MemAbs, DstMem | SrcReg | Mov | MemAbs,
+	ByteOp | SrcMem | Mov | MemAbs, SrcMem | Mov | MemAbs,
+	ByteOp | DstMem | Mov | MemAbs, DstMem | Mov | MemAbs,
 	ByteOp | SrcSI | DstDI | Mov | String, SrcSI | DstDI | Mov | String,
 	ByteOp | SrcSI | DstDI | String, SrcSI | DstDI | String,
 	/* 0xA8 - 0xAF */
-- 
1.7.0.4

[PATCH 6/6] KVM: x86 emulator: fix cli/sti instruction emulation

[Wei Yongjun]

If IOPL check fail, the cli/sti emulate GP and then we should
skip writeback since the default write OP is OP_REG.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
 arch/x86/kvm/emulate.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index d4526f2..99fa1c7 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2979,17 +2979,19 @@ special_insn:
 		c->dst.type = OP_NONE;	/* Disable writeback. */
 		break;
 	case 0xfa: /* cli */
-		if (emulator_bad_iopl(ctxt, ops))
+		if (emulator_bad_iopl(ctxt, ops)) {
 			emulate_gp(ctxt, 0);
-		else {
+			goto done;
+		} else {
 			ctxt->eflags &= ~X86_EFLAGS_IF;
 			c->dst.type = OP_NONE;	/* Disable writeback. */
 		}
 		break;
 	case 0xfb: /* sti */
-		if (emulator_bad_iopl(ctxt, ops))
+		if (emulator_bad_iopl(ctxt, ops)) {
 			emulate_gp(ctxt, 0);
-		else {
+			goto done;
+		} else {
 			ctxt->interruptibility = KVM_X86_SHADOW_INT_STI;
 			ctxt->eflags |= X86_EFLAGS_IF;
 			c->dst.type = OP_NONE;	/* Disable writeback. */
-- 
1.7.0.4

Re: [PATCH 1/6] KVM: x86 emulator: fix 'mov sreg,rm16' instruction decoding

[Avi Kivity]

On 07/06/2010 11:49 AM, Wei Yongjun wrote:
> Memory reads for 'mov sreg,rm16' should be 16 bits only.
>
>   

Applied all 6, thanks.

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

Re: [PATCH 5/6] KVM: x86 emulator: fix 'mov AL,moffs' instruction decoding

[Avi Kivity]

On 07/06/2010 11:53 AM, Wei Yongjun wrote:
> 'mov AL,moffs' do not need decode dest operand and
> 'mov moffs,AL' do not need decode source operand.
>
>
> @@ -177,8 +177,8 @@ static u32 opcode_table[256] = {
>  	0, 0, SrcImmFAddr | No64, 0,
>  	ImplicitOps | Stack, ImplicitOps | Stack, 0, 0,
>  	/* 0xA0 - 0xA7 */
> -	ByteOp | DstReg | SrcMem | Mov | MemAbs, DstReg | SrcMem | Mov | MemAbs,
> -	ByteOp | DstMem | SrcReg | Mov | MemAbs, DstMem | SrcReg | Mov | MemAbs,
> +	ByteOp | SrcMem | Mov | MemAbs, SrcMem | Mov | MemAbs,
> +	ByteOp | DstMem | Mov | MemAbs, DstMem | Mov | MemAbs,
>  	ByteOp | SrcSI | DstDI | Mov | String, SrcSI | DstDI | Mov | String,
>  	ByteOp | SrcSI | DstDI | String, SrcSI | DstDI | String,
>  	/* 0xA8 - 0xAF */
>   

Autotest rejected this patch (Windows XP fails to install). I think the
reason is that without DstReg, op->type is not initialized and writeback
fails.

I dropped this patch. Suggest re-implementing with DstAcc and (new) SrcAcc.

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

Re: [PATCH 5/6] KVM: x86 emulator: fix 'mov AL,moffs' instruction decoding

[Wei Yongjun]

> On 07/06/2010 11:53 AM, Wei Yongjun wrote:
>   
>> 'mov AL,moffs' do not need decode dest operand and
>> 'mov moffs,AL' do not need decode source operand.
>>
>>
>> @@ -177,8 +177,8 @@ static u32 opcode_table[256] = {
>>  	0, 0, SrcImmFAddr | No64, 0,
>>  	ImplicitOps | Stack, ImplicitOps | Stack, 0, 0,
>>  	/* 0xA0 - 0xA7 */
>> -	ByteOp | DstReg | SrcMem | Mov | MemAbs, DstReg | SrcMem | Mov | MemAbs,
>> -	ByteOp | DstMem | SrcReg | Mov | MemAbs, DstMem | SrcReg | Mov | MemAbs,
>> +	ByteOp | SrcMem | Mov | MemAbs, SrcMem | Mov | MemAbs,
>> +	ByteOp | DstMem | Mov | MemAbs, DstMem | Mov | MemAbs,
>>  	ByteOp | SrcSI | DstDI | Mov | String, SrcSI | DstDI | Mov | String,
>>  	ByteOp | SrcSI | DstDI | String, SrcSI | DstDI | String,
>>  	/* 0xA8 - 0xAF */
>>   
>>     
> Autotest rejected this patch (Windows XP fails to install). I think the
> reason is that without DstReg, op->type is not initialized and writeback
> fails.
>
>   

> I dropped this patch. Suggest re-implementing with DstAcc and (new) SrcAcc.
>   

sorry about that, I will send a patch to do this.

[PATCH] KVM: x86 emulator: re-implementing 'mov AL,moffs' instruction decoding

[Wei Yongjun]

This patch change to use DstAcc for decoding 'mov AL, moffs'
and introduced SrcAcc for decoding 'mov moffs, AL'.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
 arch/x86/kvm/emulate.c |   30 +++++++++++++++++++++++-------
 1 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 99fa1c7..87289c2 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -70,6 +70,7 @@
 #define SrcSI       (0xa<<4)	/* Source is in the DS:RSI */
 #define SrcImmFAddr (0xb<<4)	/* Source is immediate far address */
 #define SrcMemFAddr (0xc<<4)	/* Source is far address in memory */
+#define SrcAcc      (0xd<<4)	/* Source Accumulator */
 #define SrcMask     (0xf<<4)
 /* Generic ModRM decode. */
 #define ModRM       (1<<8)
@@ -177,8 +178,8 @@ static u32 opcode_table[256] = {
 	0, 0, SrcImmFAddr | No64, 0,
 	ImplicitOps | Stack, ImplicitOps | Stack, 0, 0,
 	/* 0xA0 - 0xA7 */
-	ByteOp | SrcMem | Mov | MemAbs, SrcMem | Mov | MemAbs,
-	ByteOp | DstMem | Mov | MemAbs, DstMem | Mov | MemAbs,
+	ByteOp | DstAcc | SrcMem | Mov | MemAbs, DstAcc | SrcMem | Mov | MemAbs,
+	ByteOp | DstMem | SrcAcc | Mov | MemAbs, DstMem | SrcAcc | Mov | MemAbs,
 	ByteOp | SrcSI | DstDI | Mov | String, SrcSI | DstDI | Mov | String,
 	ByteOp | SrcSI | DstDI | String, SrcSI | DstDI | String,
 	/* 0xA8 - 0xAF */
@@ -1186,6 +1187,25 @@ done_prefixes:
 		else
 			c->src.val = insn_fetch(u8, 1, c->eip);
 		break;
+	case SrcAcc:
+		c->src.type = OP_REG;
+		c->src.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
+		c->src.ptr = &c->regs[VCPU_REGS_RAX];
+		switch (c->src.bytes) {
+			case 1:
+				c->src.val = *(u8 *)c->src.ptr;
+				break;
+			case 2:
+				c->src.val = *(u16 *)c->src.ptr;
+				break;
+			case 4:
+				c->src.val = *(u32 *)c->src.ptr;
+				break;
+			case 8:
+				c->src.val = *(u64 *)c->src.ptr;
+				break;
+		}
+		break;
 	case SrcOne:
 		c->src.bytes = 1;
 		c->src.val = 1;
@@ -2854,13 +2874,9 @@ special_insn:
 		if (rc != X86EMUL_CONTINUE)
 			goto done;
 		break;
-	case 0xa0 ... 0xa1:	/* mov */
-		c->dst.ptr = (unsigned long *)&c->regs[VCPU_REGS_RAX];
+	case 0xa0 ... 0xa3:	/* mov */
 		c->dst.val = c->src.val;
 		break;
-	case 0xa2 ... 0xa3:	/* mov */
-		c->dst.val = (unsigned long)c->regs[VCPU_REGS_RAX];
-		break;
 	case 0xa4 ... 0xa5:	/* movs */
 		goto mov;
 	case 0xa6 ... 0xa7:	/* cmps */
-- 
1.7.0.4

Re: [PATCH] KVM: x86 emulator: re-implementing 'mov AL,moffs' instruction decoding

[Mohammed Gamal]

2010/7/7 Wei Yongjun <yjwei@cn.fujitsu.com>:
> This patch change to use DstAcc for decoding 'mov AL, moffs'
> and introduced SrcAcc for decoding 'mov moffs, AL'.
>
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> ---
>  arch/x86/kvm/emulate.c |   30 +++++++++++++++++++++++-------
>  1 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 99fa1c7..87289c2 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -70,6 +70,7 @@
>  #define SrcSI       (0xa<<4)   /* Source is in the DS:RSI */
>  #define SrcImmFAddr (0xb<<4)   /* Source is immediate far address */
>  #define SrcMemFAddr (0xc<<4)   /* Source is far address in memory */
> +#define SrcAcc      (0xd<<4)   /* Source Accumulator */
>  #define SrcMask     (0xf<<4)
>  /* Generic ModRM decode. */
>  #define ModRM       (1<<8)
> @@ -177,8 +178,8 @@ static u32 opcode_table[256] = {
>        0, 0, SrcImmFAddr | No64, 0,
>        ImplicitOps | Stack, ImplicitOps | Stack, 0, 0,
>        /* 0xA0 - 0xA7 */
> -       ByteOp | SrcMem | Mov | MemAbs, SrcMem | Mov | MemAbs,
> -       ByteOp | DstMem | Mov | MemAbs, DstMem | Mov | MemAbs,
> +       ByteOp | DstAcc | SrcMem | Mov | MemAbs, DstAcc | SrcMem | Mov | MemAbs,
> +       ByteOp | DstMem | SrcAcc | Mov | MemAbs, DstMem | SrcAcc | Mov | MemAbs,
>        ByteOp | SrcSI | DstDI | Mov | String, SrcSI | DstDI | Mov | String,
>        ByteOp | SrcSI | DstDI | String, SrcSI | DstDI | String,
>        /* 0xA8 - 0xAF */
> @@ -1186,6 +1187,25 @@ done_prefixes:
>                else
>                        c->src.val = insn_fetch(u8, 1, c->eip);
>                break;
> +       case SrcAcc:
> +               c->src.type = OP_REG;
> +               c->src.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
> +               c->src.ptr = &c->regs[VCPU_REGS_RAX];
> +               switch (c->src.bytes) {
> +                       case 1:
> +                               c->src.val = *(u8 *)c->src.ptr;
> +                               break;
> +                       case 2:
> +                               c->src.val = *(u16 *)c->src.ptr;
> +                               break;
> +                       case 4:
> +                               c->src.val = *(u32 *)c->src.ptr;
> +                               break;
> +                       case 8:
> +                               c->src.val = *(u64 *)c->src.ptr;
> +                               break;
> +               }
> +               break;
>        case SrcOne:
>                c->src.bytes = 1;
>                c->src.val = 1;
> @@ -2854,13 +2874,9 @@ special_insn:
>                if (rc != X86EMUL_CONTINUE)
>                        goto done;
>                break;
> -       case 0xa0 ... 0xa1:     /* mov */
> -               c->dst.ptr = (unsigned long *)&c->regs[VCPU_REGS_RAX];
> +       case 0xa0 ... 0xa3:     /* mov */
>                c->dst.val = c->src.val;
>                break;
> -       case 0xa2 ... 0xa3:     /* mov */
> -               c->dst.val = (unsigned long)c->regs[VCPU_REGS_RAX];
> -               break;
>        case 0xa4 ... 0xa5:     /* movs */
>                goto mov;
Instead of duplicating the mov code, you could've changed the case
statement to handle 0xa0...0xa5 which will jump to the "mov" label
anyway. The decoder flags will correctly handle src and dst values.

>        case 0xa6 ... 0xa7:     /* cmps */
> --
> 1.7.0.4
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] KVM: x86 emulator: re-implementing 'mov AL,moffs' instruction decoding

[Wei Yongjun]

> 2010/7/7 Wei Yongjun <yjwei@cn.fujitsu.com>:
>   
>> This patch change to use DstAcc for decoding 'mov AL, moffs'
>> and introduced SrcAcc for decoding 'mov moffs, AL'.
>>
>> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
>> ---
>>  arch/x86/kvm/emulate.c |   30 +++++++++++++++++++++++-------
>>  1 files changed, 23 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index 99fa1c7..87289c2 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -70,6 +70,7 @@
>>  #define SrcSI       (0xa<<4)   /* Source is in the DS:RSI */
>>  #define SrcImmFAddr (0xb<<4)   /* Source is immediate far address */
>>  #define SrcMemFAddr (0xc<<4)   /* Source is far address in memory */
>> +#define SrcAcc      (0xd<<4)   /* Source Accumulator */
>>  #define SrcMask     (0xf<<4)
>>  /* Generic ModRM decode. */
>>  #define ModRM       (1<<8)
>> @@ -177,8 +178,8 @@ static u32 opcode_table[256] = {
>>        0, 0, SrcImmFAddr | No64, 0,
>>        ImplicitOps | Stack, ImplicitOps | Stack, 0, 0,
>>        /* 0xA0 - 0xA7 */
>> -       ByteOp | SrcMem | Mov | MemAbs, SrcMem | Mov | MemAbs,
>> -       ByteOp | DstMem | Mov | MemAbs, DstMem | Mov | MemAbs,
>> +       ByteOp | DstAcc | SrcMem | Mov | MemAbs, DstAcc | SrcMem | Mov | MemAbs,
>> +       ByteOp | DstMem | SrcAcc | Mov | MemAbs, DstMem | SrcAcc | Mov | MemAbs,
>>        ByteOp | SrcSI | DstDI | Mov | String, SrcSI | DstDI | Mov | String,
>>        ByteOp | SrcSI | DstDI | String, SrcSI | DstDI | String,
>>        /* 0xA8 - 0xAF */
>> @@ -1186,6 +1187,25 @@ done_prefixes:
>>                else
>>                        c->src.val = insn_fetch(u8, 1, c->eip);
>>                break;
>> +       case SrcAcc:
>> +               c->src.type = OP_REG;
>> +               c->src.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
>> +               c->src.ptr = &c->regs[VCPU_REGS_RAX];
>> +               switch (c->src.bytes) {
>> +                       case 1:
>> +                               c->src.val = *(u8 *)c->src.ptr;
>> +                               break;
>> +                       case 2:
>> +                               c->src.val = *(u16 *)c->src.ptr;
>> +                               break;
>> +                       case 4:
>> +                               c->src.val = *(u32 *)c->src.ptr;
>> +                               break;
>> +                       case 8:
>> +                               c->src.val = *(u64 *)c->src.ptr;
>> +                               break;
>> +               }
>> +               break;
>>        case SrcOne:
>>                c->src.bytes = 1;
>>                c->src.val = 1;
>> @@ -2854,13 +2874,9 @@ special_insn:
>>                if (rc != X86EMUL_CONTINUE)
>>                        goto done;
>>                break;
>> -       case 0xa0 ... 0xa1:     /* mov */
>> -               c->dst.ptr = (unsigned long *)&c->regs[VCPU_REGS_RAX];
>> +       case 0xa0 ... 0xa3:     /* mov */
>>                c->dst.val = c->src.val;
>>                break;
>> -       case 0xa2 ... 0xa3:     /* mov */
>> -               c->dst.val = (unsigned long)c->regs[VCPU_REGS_RAX];
>> -               break;
>>        case 0xa4 ... 0xa5:     /* movs */
>>                goto mov;
>>     
> Instead of duplicating the mov code, you could've changed the case
> statement to handle 0xa0...0xa5 which will jump to the "mov" label
> anyway. The decoder flags will correctly handle src and dst values.
>   

good idea, I will fix this, thanks.

>
>
>   

[PATCHv2] KVM: x86 emulator: re-implementing 'mov AL,moffs' instruction decoding

[Wei Yongjun]

This patch change to use DstAcc for decoding 'mov AL, moffs'
and introduced SrcAcc for decoding 'mov moffs, AL'.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
 arch/x86/kvm/emulate.c |   32 +++++++++++++++++++++++---------
 1 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 99fa1c7..255473f 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -70,6 +70,7 @@
 #define SrcSI       (0xa<<4)	/* Source is in the DS:RSI */
 #define SrcImmFAddr (0xb<<4)	/* Source is immediate far address */
 #define SrcMemFAddr (0xc<<4)	/* Source is far address in memory */
+#define SrcAcc      (0xd<<4)	/* Source Accumulator */
 #define SrcMask     (0xf<<4)
 /* Generic ModRM decode. */
 #define ModRM       (1<<8)
@@ -177,8 +178,8 @@ static u32 opcode_table[256] = {
 	0, 0, SrcImmFAddr | No64, 0,
 	ImplicitOps | Stack, ImplicitOps | Stack, 0, 0,
 	/* 0xA0 - 0xA7 */
-	ByteOp | SrcMem | Mov | MemAbs, SrcMem | Mov | MemAbs,
-	ByteOp | DstMem | Mov | MemAbs, DstMem | Mov | MemAbs,
+	ByteOp | DstAcc | SrcMem | Mov | MemAbs, DstAcc | SrcMem | Mov | MemAbs,
+	ByteOp | DstMem | SrcAcc | Mov | MemAbs, DstMem | SrcAcc | Mov | MemAbs,
 	ByteOp | SrcSI | DstDI | Mov | String, SrcSI | DstDI | Mov | String,
 	ByteOp | SrcSI | DstDI | String, SrcSI | DstDI | String,
 	/* 0xA8 - 0xAF */
@@ -1186,6 +1187,25 @@ done_prefixes:
 		else
 			c->src.val = insn_fetch(u8, 1, c->eip);
 		break;
+	case SrcAcc:
+		c->src.type = OP_REG;
+		c->src.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
+		c->src.ptr = &c->regs[VCPU_REGS_RAX];
+		switch (c->src.bytes) {
+			case 1:
+				c->src.val = *(u8 *)c->src.ptr;
+				break;
+			case 2:
+				c->src.val = *(u16 *)c->src.ptr;
+				break;
+			case 4:
+				c->src.val = *(u32 *)c->src.ptr;
+				break;
+			case 8:
+				c->src.val = *(u64 *)c->src.ptr;
+				break;
+		}
+		break;
 	case SrcOne:
 		c->src.bytes = 1;
 		c->src.val = 1;
@@ -2854,13 +2874,7 @@ special_insn:
 		if (rc != X86EMUL_CONTINUE)
 			goto done;
 		break;
-	case 0xa0 ... 0xa1:	/* mov */
-		c->dst.ptr = (unsigned long *)&c->regs[VCPU_REGS_RAX];
-		c->dst.val = c->src.val;
-		break;
-	case 0xa2 ... 0xa3:	/* mov */
-		c->dst.val = (unsigned long)c->regs[VCPU_REGS_RAX];
-		break;
+	case 0xa0 ... 0xa3:	/* mov */
 	case 0xa4 ... 0xa5:	/* movs */
 		goto mov;
 	case 0xa6 ... 0xa7:	/* cmps */
-- 
1.7.0.4

Re: [PATCHv2] KVM: x86 emulator: re-implementing 'mov AL,moffs' instruction decoding

[Avi Kivity]

On 07/07/2010 12:43 PM, Wei Yongjun wrote:
> This patch change to use DstAcc for decoding 'mov AL, moffs'
> and introduced SrcAcc for decoding 'mov moffs, AL'.
>
>   

Applied, thanks.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.