* [PATCH] KVM: fix mov immediate emulation for 64-bit operands [not found] <1325967346-12539-1-git-send-email-namit@cs.technion.ac.il> @ 2012-01-07 20:21 ` Nadav Amit 2012-01-07 20:25 ` H. Peter Anvin 0 siblings, 1 reply; 8+ messages in thread From: Nadav Amit @ 2012-01-07 20:21 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, kvm, linux-kernel MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand. The previous emulation implementation assumes the operand is no longer than 32. Signed-off-by: Nadav Amit <nadav.amit@gmail.com> --- arch/x86/kvm/emulate.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 05a562b..65d1d31 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -3502,7 +3502,8 @@ static unsigned imm_size(struct x86_emulate_ctxt *ctxt) unsigned size; size = (ctxt->d & ByteOp) ? 1 : ctxt->op_bytes; - if (size == 8) + /* Immediates are usually no longer than 4 bytes */ + if (size == 8 && ((ctxt->b & 0xF8) != 0xB8 || ctxt->twobyte)) size = 4; return size; } @@ -3526,6 +3527,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) { -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: fix mov immediate emulation for 64-bit operands 2012-01-07 20:21 ` [PATCH] KVM: fix mov immediate emulation for 64-bit operands Nadav Amit @ 2012-01-07 20:25 ` H. Peter Anvin 2012-01-08 0:30 ` Nadav Amit 0 siblings, 1 reply; 8+ messages in thread From: H. Peter Anvin @ 2012-01-07 20:25 UTC (permalink / raw) To: Nadav Amit Cc: Avi Kivity, Marcelo Tosatti, Thomas Gleixner, Ingo Molnar, x86, kvm, linux-kernel On 01/07/2012 12:21 PM, Nadav Amit wrote: > MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand. > The previous emulation implementation assumes the operand is no longer than 32. > > Signed-off-by: Nadav Amit <nadav.amit@gmail.com> There are exactly two such instructions: MOV immediate (B8-BF) and MOV moff (A0-A3); you may want to check the latter too. -hpa ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: fix mov immediate emulation for 64-bit operands 2012-01-07 20:25 ` H. Peter Anvin @ 2012-01-08 0:30 ` Nadav Amit 2012-01-08 1:26 ` Takuya Yoshikawa 0 siblings, 1 reply; 8+ messages in thread From: Nadav Amit @ 2012-01-08 0:30 UTC (permalink / raw) To: H. Peter Anvin Cc: Avi Kivity, Marcelo Tosatti, Thomas Gleixner, Ingo Molnar, x86, kvm, linux-kernel On Jan 7, 2012, at 10:25 PM, H. Peter Anvin wrote: > On 01/07/2012 12:21 PM, Nadav Amit wrote: >> MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand. >> The previous emulation implementation assumes the operand is no longer than 32. >> >> Signed-off-by: Nadav Amit <nadav.amit@gmail.com> > > There are exactly two such instructions: MOV immediate (B8-BF) and MOV > moff (A0-A3); you may want to check the latter too. > > -hpa > These instructions (A0-A3) seem to be already covered by the decode_abs function. Regards, Nadav ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: fix mov immediate emulation for 64-bit operands 2012-01-08 0:30 ` Nadav Amit @ 2012-01-08 1:26 ` Takuya Yoshikawa 2012-01-08 8:47 ` Nadav Amit 0 siblings, 1 reply; 8+ messages in thread From: Takuya Yoshikawa @ 2012-01-08 1:26 UTC (permalink / raw) To: Nadav Amit Cc: H. Peter Anvin, Avi Kivity, Marcelo Tosatti, Thomas Gleixner, Ingo Molnar, x86, kvm, linux-kernel Hi, Nadav Amit <nadav.amit@gmail.com> wrote: > On Jan 7, 2012, at 10:25 PM, H. Peter Anvin wrote: > > > On 01/07/2012 12:21 PM, Nadav Amit wrote: > >> MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand. > >> The previous emulation implementation assumes the operand is no longer than 32. > >> > >> Signed-off-by: Nadav Amit <nadav.amit@gmail.com> > > > > There are exactly two such instructions: MOV immediate (B8-BF) and MOV > > moff (A0-A3); you may want to check the latter too. > > > > -hpa > > > > These instructions (A0-A3) seem to be already covered by the decode_abs function. Like these how about introducing a new flag and change the following entries in the decode table to indicate possible 64bit immediate: /* 0xB8 - 0xBF */ X8(I(DstReg | SrcImm | Mov, em_mov)), Checking the opcode byte at the operand decoding stage, like below, does not look nice: (IMO so better ask Avi) + if (size == 8 && ((ctxt->b & 0xF8) != 0xB8 || ctxt->twobyte)) size = 4; I am now cleaning up x86_decode_insn() to make each decoding stage clearer. Takuya ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: fix mov immediate emulation for 64-bit operands 2012-01-08 1:26 ` Takuya Yoshikawa @ 2012-01-08 8:47 ` Nadav Amit 2012-01-08 14:05 ` Avi Kivity 0 siblings, 1 reply; 8+ messages in thread From: Nadav Amit @ 2012-01-08 8:47 UTC (permalink / raw) To: Takuya Yoshikawa Cc: H. Peter Anvin, Avi Kivity, Marcelo Tosatti, Thomas Gleixner, Ingo Molnar, x86, kvm, linux-kernel On Sun, Jan 8, 2012 at 3:26 AM, Takuya Yoshikawa <takuya.yoshikawa@gmail.com> wrote: > Hi, > > Nadav Amit <nadav.amit@gmail.com> wrote: >> On Jan 7, 2012, at 10:25 PM, H. Peter Anvin wrote: >> >> > On 01/07/2012 12:21 PM, Nadav Amit wrote: >> >> MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand. >> >> The previous emulation implementation assumes the operand is no longer than 32. >> >> >> >> Signed-off-by: Nadav Amit <nadav.amit@gmail.com> >> > >> > There are exactly two such instructions: MOV immediate (B8-BF) and MOV >> > moff (A0-A3); you may want to check the latter too. >> > >> > -hpa >> > >> >> These instructions (A0-A3) seem to be already covered by the decode_abs function. > > Like these how about introducing a new flag and change the following entries in the > decode table to indicate possible 64bit immediate: > > /* 0xB8 - 0xBF */ > X8(I(DstReg | SrcImm | Mov, em_mov)), > > Checking the opcode byte at the operand decoding stage, like below, does not look nice: > (IMO so better ask Avi) > > + if (size == 8 && ((ctxt->b & 0xF8) != 0xB8 || ctxt->twobyte)) > size = 4; > I agree. I remembered these flags are expensive (from the time flags were set in u32). I guess I can add OpImm64. Another less preferable alternative is to add a misc. flag or reuse another flag. Avi, please acknowledge adding OpImm64. Regards, Nadav ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: fix mov immediate emulation for 64-bit operands 2012-01-08 8:47 ` Nadav Amit @ 2012-01-08 14:05 ` Avi Kivity 2012-01-08 14:08 ` Avi Kivity 0 siblings, 1 reply; 8+ messages in thread From: Avi Kivity @ 2012-01-08 14:05 UTC (permalink / raw) To: Nadav Amit Cc: Takuya Yoshikawa, H. Peter Anvin, Marcelo Tosatti, Thomas Gleixner, Ingo Molnar, x86, kvm, linux-kernel On 01/08/2012 10:47 AM, Nadav Amit wrote: > On Sun, Jan 8, 2012 at 3:26 AM, Takuya Yoshikawa > <takuya.yoshikawa@gmail.com> wrote: > > Hi, > > > > Nadav Amit <nadav.amit@gmail.com> wrote: > >> On Jan 7, 2012, at 10:25 PM, H. Peter Anvin wrote: > >> > >> > On 01/07/2012 12:21 PM, Nadav Amit wrote: > >> >> MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand. > >> >> The previous emulation implementation assumes the operand is no longer than 32. > >> >> > >> >> Signed-off-by: Nadav Amit <nadav.amit@gmail.com> > >> > > >> > There are exactly two such instructions: MOV immediate (B8-BF) and MOV > >> > moff (A0-A3); you may want to check the latter too. > >> > > >> > -hpa > >> > > >> > >> These instructions (A0-A3) seem to be already covered by the decode_abs function. > > > > Like these how about introducing a new flag and change the following entries in the > > decode table to indicate possible 64bit immediate: > > > > /* 0xB8 - 0xBF */ > > X8(I(DstReg | SrcImm | Mov, em_mov)), > > > > Checking the opcode byte at the operand decoding stage, like below, does not look nice: > > (IMO so better ask Avi) > > > > + if (size == 8 && ((ctxt->b & 0xF8) != 0xB8 || ctxt->twobyte)) > > size = 4; > > > > I agree. I remembered these flags are expensive (from the time flags > were set in u32). > I guess I can add OpImm64. > Another less preferable alternative is to add a misc. flag or reuse > another flag. > > Avi, please acknowledge adding OpImm64. Yes, OpImm64 is the cleanest IMO. Note it doesn't even cost us a bit. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: fix mov immediate emulation for 64-bit operands 2012-01-08 14:05 ` Avi Kivity @ 2012-01-08 14:08 ` Avi Kivity 2012-01-08 14:44 ` Nadav Amit 0 siblings, 1 reply; 8+ messages in thread From: Avi Kivity @ 2012-01-08 14:08 UTC (permalink / raw) To: Nadav Amit Cc: Takuya Yoshikawa, H. Peter Anvin, Marcelo Tosatti, Thomas Gleixner, Ingo Molnar, x86, kvm, linux-kernel On 01/08/2012 04:05 PM, Avi Kivity wrote: > > > > Avi, please acknowledge adding OpImm64. > > Yes, OpImm64 is the cleanest IMO. Note it doesn't even cost us a bit. Note, usually I'd ask for a unit test to accompany the fix, but the current framework only supports testing instructions that have a memroy operand. So you're off the hook for now, unless you'd like to extend the framework. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: fix mov immediate emulation for 64-bit operands 2012-01-08 14:08 ` Avi Kivity @ 2012-01-08 14:44 ` Nadav Amit 0 siblings, 0 replies; 8+ messages in thread From: Nadav Amit @ 2012-01-08 14:44 UTC (permalink / raw) To: Avi Kivity Cc: Takuya Yoshikawa, H. Peter Anvin, Marcelo Tosatti, Thomas Gleixner, Ingo Molnar, x86, kvm, linux-kernel On Sun, Jan 8, 2012 at 4:08 PM, Avi Kivity <avi@redhat.com> wrote: > On 01/08/2012 04:05 PM, Avi Kivity wrote: >> > >> > Avi, please acknowledge adding OpImm64. >> >> Yes, OpImm64 is the cleanest IMO. Note it doesn't even cost us a bit. Very well - I'll submit a revised patch later. > > Note, usually I'd ask for a unit test to accompany the fix, but the > current framework only supports testing instructions that have a memroy > operand. So you're off the hook for now, unless you'd like to extend > the framework. Unfortunately, I don't. I am doing some odd things with KVM that cause me to encounter emulator bugs. I am likely to submit another patch or two as the emulation code has additional "issues" handling EPT violations (especially on non-memory operands). Regards, Nadav ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-01-08 14:44 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <1325967346-12539-1-git-send-email-namit@cs.technion.ac.il> 2012-01-07 20:21 ` [PATCH] KVM: fix mov immediate emulation for 64-bit operands Nadav Amit 2012-01-07 20:25 ` H. Peter Anvin 2012-01-08 0:30 ` Nadav Amit 2012-01-08 1:26 ` Takuya Yoshikawa 2012-01-08 8:47 ` Nadav Amit 2012-01-08 14:05 ` Avi Kivity 2012-01-08 14:08 ` Avi Kivity 2012-01-08 14:44 ` Nadav Amit
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.