| Message ID | 1414922101-17626-21-git-send-email-namit@cs.technion.ac.il (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/11/2014 10:55, Nadav Amit wrote: > Commit 3b32004a66e9 ("KVM: x86: movnti minimum op size of 32-bit is not kept") > did not fully fix the minimum operand size of MONTI emulation. Still, MOVNTI > may be mistakenly performed using 16-bit opsize. > > This patch add No16 flag to mark an instruction does not support 16-bits > operand size. So a .byte 0x66 movntiw (%esi), %eax will zero the higher two bytes of %eax before this patch, and load 4 bytes from (%esi) after? Paolo -- 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 Nov 5, 2014, at 14:18, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > On 02/11/2014 10:55, Nadav Amit wrote: >> Commit 3b32004a66e9 ("KVM: x86: movnti minimum op size of 32-bit is not kept") >> did not fully fix the minimum operand size of MONTI emulation. Still, MOVNTI >> may be mistakenly performed using 16-bit opsize. >> >> This patch add No16 flag to mark an instruction does not support 16-bits >> operand size. > > So a > > .byte 0x66 > movntiw (%esi), %eax > > will zero the higher two bytes of %eax before this patch, and load 4 > bytes from (%esi) after? > Well, actually the 0x66 prefix is an illegal prefix for this instruction, so it will cause #UD. But if the default operand size is 16 (e.g., CS.D = 0), then yes - after this patch it will load 4 bytes from (%esi), and this is the expected behaviour. Here is a small test to show the behaviour (build with -m32 ). We set CS to 16-bit segment, so default operand size is 16-bit, but 32-bits are assigned. If you replace movntil with movl, you’ll see only 16-bits are stored, as you would expect from mov. --- #include <sys/types.h> #include <asm/ldt.h> #include <stdio.h> int main() { unsigned int a = 0; unsigned int b = 0x87654321u; struct user_desc d = { .entry_number = 0, .base_addr = 0, .limit = 0xfffffu, .seg_32bit = 0, .contents = 2, .read_exec_only = 1, .limit_in_pages = 1, .seg_not_present = 0, .useable = 1, }; modify_ldt(1, &d, sizeof(d)); asm volatile ( "lcall $0x7, $1f\n\t" "jmp 2f\n\t" "1: .byte 0x67\n\t" "movntil %%ebx, (%%eax)\n\t" ".byte 0x66\n\t" "lret\n\t" "2:\n\t" : : "a"(&a), "b"(b) : "memory"); printf("result %x\n", a); return 0; } --- 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
> On Nov 5, 2014, at 14:18, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > On 02/11/2014 10:55, Nadav Amit wrote: >> Commit 3b32004a66e9 ("KVM: x86: movnti minimum op size of 32-bit is not kept") >> did not fully fix the minimum operand size of MONTI emulation. Still, MOVNTI >> may be mistakenly performed using 16-bit opsize. >> >> This patch add No16 flag to mark an instruction does not support 16-bits >> operand size. > > So a > > .byte 0x66 > movntiw (%esi), %eax > > will zero the higher two bytes of %eax before this patch, and load 4 > bytes from (%esi) after? > Well, actually the 0x66 prefix is an illegal prefix for this instruction, so it will cause #UD. But if the default operand size is 16 (e.g., CS.D = 0), then yes - after this patch it will load 4 bytes from (%esi), and this is the expected behaviour. Here is a small test to show the behaviour (build with -m32 ). We set CS to 16-bit segment, so default operand size is 16-bit, but 32-bits are assigned. If you replace movntil with movl, you’ll see only 16-bits are stored, as you would expect from mov. --- #include <sys/types.h> #include <asm/ldt.h> #include <stdio.h> int main() { unsigned int a = 0; unsigned int b = 0x87654321u; struct user_desc d = { .entry_number = 0, .base_addr = 0, .limit = 0xfffffu, .seg_32bit = 0, .contents = 2, .read_exec_only = 1, .limit_in_pages = 1, .seg_not_present = 0, .useable = 1, }; modify_ldt(1, &d, sizeof(d)); asm volatile ( "lcall $0x7, $1f\n\t" "jmp 2f\n\t" "1: .byte 0x67\n\t" "movntil %%ebx, (%%eax)\n\t" ".byte 0x66\n\t" "lret\n\t" "2:\n\t" : : "a"(&a), "b"(b) : "memory"); printf("result %x\n", a); return 0; } --- 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
On 02/11/2014 10:55, Nadav Amit wrote: > Signed-off-by: Nadav Amit <namit@cs.technion.ac.il> > --- Applied, thanks. Paolo -- 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 --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 24b0df7..84a83dc 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -167,6 +167,7 @@ #define NoBigReal ((u64)1 << 50) /* No big real mode */ #define PrivUD ((u64)1 << 51) /* #UD instead of #GP on CPL > 0 */ #define NearBranch ((u64)1 << 52) /* Near branches */ +#define No16 ((u64)1 << 53) /* No 16 bit operand */ #define DstXacc (DstAccLo | SrcAccHi | SrcWrite) @@ -4116,7 +4117,7 @@ static const struct opcode twobyte_table[256] = { D(DstReg | SrcMem8 | ModRM | Mov), D(DstReg | SrcMem16 | ModRM | Mov), /* 0xC0 - 0xC7 */ F2bv(DstMem | SrcReg | ModRM | SrcWrite | Lock, em_xadd), - N, D(DstMem | SrcReg | ModRM | Mov), + N, I(DstMem | SrcReg | ModRM | No16 | Mov, em_mov), N, N, N, GD(0, &group9), /* 0xC8 - 0xCF */ X8(I(DstReg, em_bswap)), @@ -4561,7 +4562,8 @@ done_prefixes: return EMULATION_FAILED; if (unlikely(ctxt->d & - (NotImpl|Stack|Op3264|Sse|Mmx|Intercept|CheckPerm|NearBranch))) { + (NotImpl|Stack|Op3264|Sse|Mmx|Intercept|CheckPerm|NearBranch| + No16))) { /* * These are copied unconditionally here, and checked unconditionally * in x86_emulate_insn. @@ -4586,6 +4588,9 @@ done_prefixes: ctxt->op_bytes = 4; } + if ((ctxt->d & No16) && ctxt->op_bytes == 2) + ctxt->op_bytes = 4; + if (ctxt->d & Sse) ctxt->op_bytes = 16; else if (ctxt->d & Mmx) @@ -5051,11 +5056,6 @@ twobyte_insn: ctxt->dst.val = (ctxt->src.bytes == 1) ? (s8) ctxt->src.val : (s16) ctxt->src.val; break; - case 0xc3: /* movnti */ - ctxt->dst.bytes = ctxt->op_bytes; - ctxt->dst.val = (ctxt->op_bytes == 8) ? (u64) ctxt->src.val : - (u32) ctxt->src.val; - break; default: goto cannot_emulate; }
Commit 3b32004a66e9 ("KVM: x86: movnti minimum op size of 32-bit is not kept") did not fully fix the minimum operand size of MONTI emulation. Still, MOVNTI may be mistakenly performed using 16-bit opsize. This patch add No16 flag to mark an instruction does not support 16-bits operand size. Signed-off-by: Nadav Amit <namit@cs.technion.ac.il> --- arch/x86/kvm/emulate.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)