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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 27.03.17 at 14:10, <andrew.cooper3@citrix.com> wrote:
> On 27/03/17 12:24, Jan Beulich wrote:
>>>>> On 27.03.17 at 11:56, <andrew.cooper3@citrix.com> 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.

Well, okay, if we consider that there aren't any real RMW SIMD
insns (albeit for now we emulate a few as such), I can buy this
argument, so

Acked-by: Jan Beulich <jbeulich@suse.com>

Jan


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