On 07/11/2014 18:37, Radim Krčmář wrote: > 2014-11-06 11:15+0200, Nadav Amit: >> As we run out of bits in the KVM emulator instruction flags, we can merge >> together the Mmx/Sse/Avx bits. These bits are mutual exclusive (i.e., each >> instruction is either MMX, SSE, AVX, or none), so we can save one bit in the >> flags by merging them. >> >> Signed-off-by: Nadav Amit <[email protected]> >> --- > > It looks that Avx behaves a bit differently that legacy Sse, so having > it exclusive is better. > > I'd make changes, but the behavior doesn't look wrong now, so
Thanks for the review, Radim. I think we have no clear idea of what Avx would do (I have one---same as Sse but make VEX prefix mandatory, see VBROADCASTSS---but I'm not sure it's the right one either). Let's keep these patches on hold. Paolo > Reviewed-by: Radim Krčmář <[email protected]> > >> arch/x86/kvm/emulate.c | 44 ++++++++++++++++++++++++++++---------------- >> 1 file changed, 28 insertions(+), 16 deletions(-) >> >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c >> index cd2029b..f98ead7 100644 >> --- a/arch/x86/kvm/emulate.c >> +++ b/arch/x86/kvm/emulate.c >> @@ -123,7 +123,6 @@ >> #define Prefix (3<<15) /* Instruction varies with 66/f2/f3 prefix >> */ >> #define RMExt (4<<15) /* Opcode extension in ModRM r/m if mod == >> 3 */ >> #define Escape (5<<15) /* Escape to coprocessor instruction */ >> -#define Sse (1<<18) /* SSE Vector instruction */ > > (I liked that Paolo moved something to the empty spot.) > >> /* Generic ModRM decode. */ >> #define ModRM (1<<19) >> /* Destination is only written; never read. */ >> @@ -155,9 +154,11 @@ >> #define Src2GS (OpGS << Src2Shift) >> #define Src2Mask (OpMask << Src2Shift) >> #define Mmx ((u64)1 << 40) /* MMX Vector instruction */ >> -#define Aligned ((u64)1 << 41) /* Explicitly aligned (e.g. MOVDQA) */ >> -#define Unaligned ((u64)1 << 42) /* Explicitly unaligned (e.g. MOVDQU) */ >> -#define Avx ((u64)1 << 43) /* Advanced Vector Extensions */ > > #define OpExtShift 40 > >> +#define Sse ((u64)2 << 40) /* SSE Vector instruction */ >> +#define Avx ((u64)3 << 40) /* Advanced Vector Extensions */ > > (Precedents set OpExt{None,Mmx,Sse,Avx}.) > >> +#define OpExtMask ((u64)3 << 40) > > (Wouldn't Ext be enough?) > >> +#define Aligned ((u64)1 << 42) /* Explicitly aligned (e.g. MOVDQA) */ >> +#define Unaligned ((u64)1 << 43) /* Explicitly unaligned (e.g. MOVDQU) */ >> #define Fastop ((u64)1 << 44) /* Use opcode::u.fastop */ >> #define NoWrite ((u64)1 << 45) /* No writeback */ >> #define SrcWrite ((u64)1 << 46) /* Write back src operand */ >> @@ -1082,18 +1083,19 @@ static void decode_register_operand(struct >> x86_emulate_ctxt *ctxt, >> struct operand *op) >> { >> unsigned reg = ctxt->modrm_reg; >> + u64 op_ext = ctxt->d & OpExtMask; > > (We kept it inline.) > >> >> if (!(ctxt->d & ModRM)) >> reg = (ctxt->b & 7) | ((ctxt->rex_prefix & 1) << 3); >> >> - if (ctxt->d & Sse) { >> + if (op_ext == Sse) { >> op->type = OP_XMM; >> op->bytes = 16; >> op->addr.xmm = reg; >> read_sse_reg(ctxt, &op->vec_val, reg); >> return; >> } >> - if (ctxt->d & Mmx) { >> + if (op_ext == Mmx) { >> reg &= 7; >> op->type = OP_MM; >> op->bytes = 8; > [...] >> @@ -1137,14 +1140,15 @@ static int decode_modrm(struct x86_emulate_ctxt >> *ctxt, >> - if ((ctxt->d & (Sse|Mmx)) && (ops->get_cr(ctxt, 0) & >> X86_CR0_TS)) { >> + if ((op_ext == Sse || op_ext == Mmx) && > > It could be just op_ext here -- Avx doesn't differ. > >> + (ops->get_cr(ctxt, 0) & X86_CR0_TS)) { >> rc = emulate_nm(ctxt); >> goto done; >> } >> >> - if (ctxt->d & Mmx) { >> + if (op_ext == Mmx) { >> rc = flush_pending_x87_faults(ctxt); >> if (rc != X86EMUL_CONTINUE) >> goto done; >> -- >> 1.9.1 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> the body of a message to [email protected] >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
