Re: [Xen-devel] [PATCH 01/10] x86/emul: Correct the decoding of vlddqu

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/03/17 12:24, Jan Beulich wrote:
>>>> On 27.03.17 at 11:56, <andrew.cooper3@xxxxxxxxxx> wrote:
>> @@ -2332,9 +2333,9 @@ x86_decode_twobyte(
>>          if ( vex.pfx == vex_f3 ) /* movq xmm/m64,xmm */
>>          {
>>      case X86EMUL_OPC_VEX_F3(0, 0x7e): /* vmovq xmm/m64,xmm */
>> -            state->desc = DstImplicit | SrcMem | Mov;
>> +            state->desc = DstImplicit | SrcMem | TwoOp;
> Why? This is a move after all.
>
>> @@ -2374,11 +2375,25 @@ x86_decode_twobyte(
>>      case X86EMUL_OPC_VEX_66(0, 0xc4): /* vpinsrw */
>>          state->desc = DstReg | SrcMem16;
>>          break;
>> +
>> +    case 0xf0:
>> +        ctxt->opcode |= MASK_INSR(vex.pfx, X86EMUL_OPC_PFX_MASK);
>> +        if ( vex.pfx == vex_f2 ) /* lddqu mem,xmm */
>> +        {
>> +        /* fall through */
>> +    case X86EMUL_OPC_VEX_F2(0, 0xf0): /* vlddqu mem,{x,y}mm */
>> +            state->desc = DstImplicit | SrcMem | TwoOp;
> I'd prefer it to be Mov here too, as the insn is a move even if its
> name doesn't say so.

The fact that TwoOp and Mov are the same constant is confusing (and why
I didn't particularly like its introduction in the first place),
especially in this context where TwoOp is the more important semantic
piece of information.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel