diff mbox

KVM: x86: fix mov immediate emulation for 64-bit operands

Message ID 20121206235510.GA30302@amt.cnet (mailing list archive)
State New, archived
Headers show

Commit Message

Marcelo Tosatti Dec. 6, 2012, 11:55 p.m. UTC 
From: Nadav Amit <nadav.amit@gmail.com>

MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand.
The previous emulation implementation assumes the operand is no longer than 32.
Adding OpImm64 for this matter.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=881579

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

--
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

Comments

Gleb Natapov Dec. 7, 2012, 7:31 a.m. UTC | #1 
On Thu, Dec 06, 2012 at 09:55:10PM -0200, Marcelo Tosatti wrote:
> 
> From: Nadav Amit <nadav.amit@gmail.com>
> 
> MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand.
> The previous emulation implementation assumes the operand is no longer than 32.
> Adding OpImm64 for this matter.
> 
> Fixes https://bugzilla.redhat.com/show_bug.cgi?id=881579
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
Needs author's sign-off and test case.

> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 39171cb..6fec09c 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -43,7 +43,7 @@
>  #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 OpImm             12ull  /* Sign extended up to 32-bit immediate */
>  #define OpMem16           13ull  /* Memory operand (16-bit). */
>  #define OpMem32           14ull  /* Memory operand (32-bit). */
>  #define OpImmU            15ull  /* Immediate operand, zero extended */
> @@ -58,6 +58,7 @@
>  #define OpFS              24ull  /* FS */
>  #define OpGS              25ull  /* GS */
>  #define OpMem8            26ull  /* 8-bit zero extended memory operand */
> +#define OpImm64           27ull  /* Sign extended 16/32/64-bit immediate */
>  
>  #define OpBits             5  /* Width of operand field */
>  #define OpMask             ((1ull << OpBits) - 1)
> @@ -101,6 +102,7 @@
>  #define SrcMemFAddr (OpMemFAddr << SrcShift)
>  #define SrcAcc      (OpAcc << SrcShift)
>  #define SrcImmU16   (OpImmU16 << SrcShift)
> +#define SrcImm64    (OpImm64 << SrcShift)
>  #define SrcDX       (OpDX << SrcShift)
>  #define SrcMem8     (OpMem8 << SrcShift)
>  #define SrcMask     (OpMask << SrcShift)
> @@ -3786,7 +3788,7 @@ static const struct opcode opcode_table[256] = {
>  	/* 0xB0 - 0xB7 */
>  	X8(I(ByteOp | DstReg | SrcImm | Mov, em_mov)),
>  	/* 0xB8 - 0xBF */
> -	X8(I(DstReg | SrcImm | Mov, em_mov)),
> +	X8(I(DstReg | SrcImm64 | Mov, em_mov)),
>  	/* 0xC0 - 0xC7 */
>  	D2bv(DstMem | SrcImmByte | ModRM),
>  	I(ImplicitOps | Stack | SrcImmU16, em_ret_near_imm),
> @@ -3950,6 +3952,9 @@ static int decode_imm(struct x86_emulate_ctxt *ctxt, struct operand *op,
>  	case 4:
>  		op->val = insn_fetch(s32, ctxt);
>  		break;
> +	case 8:
> +		op->val = insn_fetch(s64, ctxt);
> +		break;
>  	}
>  	if (!sign_extension) {
>  		switch (op->bytes) {
> @@ -4028,6 +4033,9 @@ 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 OpImm64:
> +		rc = decode_imm(ctxt, op, ctxt->op_bytes, true);
> +		break;
>  	case OpMem8:
>  		ctxt->memop.bytes = 1;
>  		goto mem_common;

--
			Gleb.
--
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
Nadav Amit Dec. 7, 2012, 2:36 p.m. UTC | #2 
On 7/12/12 09:31 , Gleb Natapov wrote:
> On Thu, Dec 06, 2012 at 09:55:10PM -0200, Marcelo Tosatti wrote:
>>
>> From: Nadav Amit <nadav.amit@gmail.com>
>>
>> MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand.
>> The previous emulation implementation assumes the operand is no longer than 32.
>> Adding OpImm64 for this matter.
>>
>> Fixes https://bugzilla.redhat.com/show_bug.cgi?id=881579
>>
>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>
> Needs author's sign-off and test case.
I've already signed-off the patch I sent a while ago.
I was busy, put the test-case implementation low in my priority list,
and forgot about it.
I would appreciate if Marcelo implements the test-case.
Otherwise, let me know - and I'll do it next week.


> 
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index 39171cb..6fec09c 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -43,7 +43,7 @@
>>  #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 OpImm             12ull  /* Sign extended up to 32-bit immediate */
>>  #define OpMem16           13ull  /* Memory operand (16-bit). */
>>  #define OpMem32           14ull  /* Memory operand (32-bit). */
>>  #define OpImmU            15ull  /* Immediate operand, zero extended */
>> @@ -58,6 +58,7 @@
>>  #define OpFS              24ull  /* FS */
>>  #define OpGS              25ull  /* GS */
>>  #define OpMem8            26ull  /* 8-bit zero extended memory operand */
>> +#define OpImm64           27ull  /* Sign extended 16/32/64-bit immediate */
>>  
>>  #define OpBits             5  /* Width of operand field */
>>  #define OpMask             ((1ull << OpBits) - 1)
>> @@ -101,6 +102,7 @@
>>  #define SrcMemFAddr (OpMemFAddr << SrcShift)
>>  #define SrcAcc      (OpAcc << SrcShift)
>>  #define SrcImmU16   (OpImmU16 << SrcShift)
>> +#define SrcImm64    (OpImm64 << SrcShift)
>>  #define SrcDX       (OpDX << SrcShift)
>>  #define SrcMem8     (OpMem8 << SrcShift)
>>  #define SrcMask     (OpMask << SrcShift)
>> @@ -3786,7 +3788,7 @@ static const struct opcode opcode_table[256] = {
>>  	/* 0xB0 - 0xB7 */
>>  	X8(I(ByteOp | DstReg | SrcImm | Mov, em_mov)),
>>  	/* 0xB8 - 0xBF */
>> -	X8(I(DstReg | SrcImm | Mov, em_mov)),
>> +	X8(I(DstReg | SrcImm64 | Mov, em_mov)),
>>  	/* 0xC0 - 0xC7 */
>>  	D2bv(DstMem | SrcImmByte | ModRM),
>>  	I(ImplicitOps | Stack | SrcImmU16, em_ret_near_imm),
>> @@ -3950,6 +3952,9 @@ static int decode_imm(struct x86_emulate_ctxt *ctxt, struct operand *op,
>>  	case 4:
>>  		op->val = insn_fetch(s32, ctxt);
>>  		break;
>> +	case 8:
>> +		op->val = insn_fetch(s64, ctxt);
>> +		break;
>>  	}
>>  	if (!sign_extension) {
>>  		switch (op->bytes) {
>> @@ -4028,6 +4033,9 @@ 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 OpImm64:
>> +		rc = decode_imm(ctxt, op, ctxt->op_bytes, true);
>> +		break;
>>  	case OpMem8:
>>  		ctxt->memop.bytes = 1;
>>  		goto mem_common;
> 
> --
> 			Gleb.
> 

Regards,
Nadav

--
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
diff mbox

Patch

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 39171cb..6fec09c 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -43,7 +43,7 @@ 
 #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 OpImm             12ull  /* Sign extended up to 32-bit immediate */
 #define OpMem16           13ull  /* Memory operand (16-bit). */
 #define OpMem32           14ull  /* Memory operand (32-bit). */
 #define OpImmU            15ull  /* Immediate operand, zero extended */
@@ -58,6 +58,7 @@ 
 #define OpFS              24ull  /* FS */
 #define OpGS              25ull  /* GS */
 #define OpMem8            26ull  /* 8-bit zero extended memory operand */
+#define OpImm64           27ull  /* Sign extended 16/32/64-bit immediate */
 
 #define OpBits             5  /* Width of operand field */
 #define OpMask             ((1ull << OpBits) - 1)
@@ -101,6 +102,7 @@ 
 #define SrcMemFAddr (OpMemFAddr << SrcShift)
 #define SrcAcc      (OpAcc << SrcShift)
 #define SrcImmU16   (OpImmU16 << SrcShift)
+#define SrcImm64    (OpImm64 << SrcShift)
 #define SrcDX       (OpDX << SrcShift)
 #define SrcMem8     (OpMem8 << SrcShift)
 #define SrcMask     (OpMask << SrcShift)
@@ -3786,7 +3788,7 @@  static const struct opcode opcode_table[256] = {
 	/* 0xB0 - 0xB7 */
 	X8(I(ByteOp | DstReg | SrcImm | Mov, em_mov)),
 	/* 0xB8 - 0xBF */
-	X8(I(DstReg | SrcImm | Mov, em_mov)),
+	X8(I(DstReg | SrcImm64 | Mov, em_mov)),
 	/* 0xC0 - 0xC7 */
 	D2bv(DstMem | SrcImmByte | ModRM),
 	I(ImplicitOps | Stack | SrcImmU16, em_ret_near_imm),
@@ -3950,6 +3952,9 @@  static int decode_imm(struct x86_emulate_ctxt *ctxt, struct operand *op,
 	case 4:
 		op->val = insn_fetch(s32, ctxt);
 		break;
+	case 8:
+		op->val = insn_fetch(s64, ctxt);
+		break;
 	}
 	if (!sign_extension) {
 		switch (op->bytes) {
@@ -4028,6 +4033,9 @@  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 OpImm64:
+		rc = decode_imm(ctxt, op, ctxt->op_bytes, true);
+		break;
 	case OpMem8:
 		ctxt->memop.bytes = 1;
 		goto mem_common;