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 <namit@xxxxxxxxxxxxxxxxx>
>> ---
>
> 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ář <rkrcmar@xxxxxxxxxx>
>
>> 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 majordomo@xxxxxxxxxxxxxxx
>> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
