Re: [Xen-devel] [PATCH 5/5] x86/emulate: add support for {, v}movd {, x}mm, r/m32 and {, v}movq {, x}mm, r/m64

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 30/09/16 13:11, Jan Beulich wrote:
>>>> On 30.09.16 at 13:59, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 08/09/16 14:46, Jan Beulich wrote:
>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>> @@ -223,7 +223,7 @@ static const opcode_desc_t twobyte_table
>>>      /* 0x70 - 0x7F */
>>>      SrcImmByte|ModRM, SrcImmByte|ModRM, SrcImmByte|ModRM, SrcImmByte|ModRM,
>>>      ModRM, ModRM, ModRM, ImplicitOps,
>>> -    ModRM, ModRM, 0, 0, ModRM, ModRM, ModRM, ImplicitOps|ModRM,
>>> +    ModRM, ModRM, 0, 0, ModRM, ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM,
>>>      /* 0x80 - 0x87 */
>>>      DstImplicit|SrcImm, DstImplicit|SrcImm,
>>>      DstImplicit|SrcImm, DstImplicit|SrcImm,
>>> @@ -2291,6 +2291,10 @@ x86_decode(
>>>          return X86EMUL_UNHANDLEABLE;
>>>      }
>>>  
>>> +    if ( op_bytes == 2 &&
>>> +         (ctxt->opcode & X86EMUL_OPC_PFX_MASK) == X86EMUL_OPC_66(0, 0) )
>>> +        op_bytes = 4;
>> What is this change for?  I presume it is to undo the effect of the
>> operand size override prefix when we have decided that the prefix
>> actually had an alternate meaning?
> Yes.
>
>> If so, can we have a comment to this effect?
> +    /*
> +     * Undo the operand-size override effect of prefix 66 when it was
> +     * determined to have another meaning.
> +     */
>
>> Everything else looks ok.
> Can I take this as R-b then with the comment added?

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

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