| Message ID | 20130409234602.GI5077@pd.tnic (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 10, 2013 at 01:46:02AM +0200, Borislav Petkov wrote: > +static int em_movbe(struct x86_emulate_ctxt *ctxt) > +{ > + char *valptr = ctxt->src.valptr; > + > + switch (ctxt->op_bytes) { > + case 2: > + *(u16 *)valptr = swab16(*(u16 *)valptr); > + break; > + case 4: > + *(u32 *)valptr = swab32(*(u32 *)valptr); Note to self: this destroys the src operand but it shouldn't. Fix it tomorrow.
On 04/09/2013 05:03 PM, Borislav Petkov wrote: > > Note to self: this destroys the src operand but it shouldn't. Fix it > tomorrow. > I thought movbe was already in qemu just not on by default...? -hpa -- 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
On Tue, Apr 09, 2013 at 05:04:06PM -0700, H. Peter Anvin wrote: > On 04/09/2013 05:03 PM, Borislav Petkov wrote: > > > > Note to self: this destroys the src operand but it shouldn't. Fix it > > tomorrow. > > > > I thought movbe was already in qemu just not on by default...? Yep, this went upstream just last month. However and AFAICT, this still doesn't help the issue when we run qemu -enable-kvm and the host doesn't have MOVBE. With my simplistic thinking, I would expect that kvm would jump to qemu on #UD and let it emulate the unsupported instruction and go back. However, as Andre explained it to me, qemu emulation and kvm are completely unrelated and it is probably very expensive to copy emulation states to and fro just for a simple instruction. Thus, this simpler approach to do the emulation straight in kvm as it is done already for a bunch of other instructions.
diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h index 15f960c06ff7..ae01c765cd77 100644 --- a/arch/x86/include/asm/kvm_emulate.h +++ b/arch/x86/include/asm/kvm_emulate.h @@ -281,6 +281,7 @@ struct x86_emulate_ctxt { /* decode cache */ u8 twobyte; + u8 thirdbyte; u8 b; u8 intercept; u8 lock_prefix; diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index a335cc6cde72..0ccff339359d 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -152,6 +152,7 @@ #define Avx ((u64)1 << 43) /* Advanced Vector Extensions */ #define Fastop ((u64)1 << 44) /* Use opcode::u.fastop */ #define NoWrite ((u64)1 << 45) /* No writeback */ +#define ThreeByte ((u64)1 << 46) /* Three byte opcodes 0F 38 and 0F 3A */ #define X2(x...) x, x #define X3(x...) X2(x), x @@ -3107,6 +3108,34 @@ static int em_mov(struct x86_emulate_ctxt *ctxt) return X86EMUL_CONTINUE; } +static int em_movbe(struct x86_emulate_ctxt *ctxt) +{ + char *valptr = ctxt->src.valptr; + + switch (ctxt->op_bytes) { + case 2: + *(u16 *)valptr = swab16(*(u16 *)valptr); + break; + case 4: + *(u32 *)valptr = swab32(*(u32 *)valptr); + + /* + * clear upper dword for 32-bit operand size in 64-bit mode. + */ + if (ctxt->mode == X86EMUL_MODE_PROT64) + *((u32 *)valptr + 1) = 0x0; + break; + case 8: + *(u64 *)valptr = swab64(*(u64 *)valptr); + break; + default: + return X86EMUL_PROPAGATE_FAULT; + } + + memcpy(ctxt->dst.valptr, ctxt->src.valptr, ctxt->op_bytes); + return X86EMUL_CONTINUE; +} + static int em_cr_write(struct x86_emulate_ctxt *ctxt) { if (ctxt->ops->set_cr(ctxt, ctxt->modrm_reg, ctxt->src.val)) @@ -3974,7 +4003,8 @@ static const struct opcode twobyte_table[256] = { I(ImplicitOps | VendorSpecific, em_sysenter), I(ImplicitOps | Priv | VendorSpecific, em_sysexit), N, N, - N, N, N, N, N, N, N, N, + I(ModRM | Mov | ThreeByte | VendorSpecific, em_movbe), + N, N, N, N, N, N, N, /* 0x40 - 0x4F */ X16(D(DstReg | SrcMem | ModRM | Mov)), /* 0x50 - 0x5F */ @@ -4323,6 +4353,15 @@ done_prefixes: } ctxt->d = opcode.flags; + if (ctxt->d & ThreeByte) { + ctxt->thirdbyte = insn_fetch(u8, ctxt); + + if (ctxt->thirdbyte == 0xf0) + ctxt->d |= DstReg | SrcMem; + else + ctxt->d |= DstMem | SrcReg; + } + if (ctxt->d & ModRM) ctxt->modrm = insn_fetch(u8, ctxt);