Re: [Xen-devel] [PATCH 4/4] x86emul: use DstEax also for XCHG

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/08/16 12:31, Jan Beulich wrote:
>>>> On 16.08.16 at 12:59, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 15/08/16 09:35, Jan Beulich wrote:
>>> Just like said in commit c0bc0adf24 ("x86emul: use DstEax where
>>> possible"): While it avoids just a few instructions, we should
>>> nevertheless make use of generic code as much as possible. Here we can
>>> arrange for that by simply swapping source and destination (as they're
>>> interchangeable).
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>
>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>> @@ -118,8 +118,10 @@ static uint8_t opcode_table[256] = {
>>>      DstMem|SrcReg|ModRM|Mov, DstReg|SrcNone|ModRM,
>>>      DstReg|SrcMem16|ModRM|Mov, DstMem|SrcNone|ModRM|Mov,
>>>      /* 0x90 - 0x97 */
>>> -    ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
>>> -    ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
>>> +    DstEax|SrcImplicit, DstEax|SrcImplicit,
>>> +    DstEax|SrcImplicit, DstEax|SrcImplicit,
>>> +    DstEax|SrcImplicit, DstEax|SrcImplicit,
>>> +    DstEax|SrcImplicit, DstEax|SrcImplicit,
>> Please add a comment explaining that DstEax is interchangeable with
>> SrcEax in the xchg case.  Otherwise, the decode table reads incorrectly.
> Do you mean me to do so even considering there's no SrcEax
> (yet, it'll come with the not yet posted patch finally doing the
> split off of the decode part)? (Nor can I see why the decode
> table reads incorrectly the way it is above.)

xchg is explicitly specified to have SrcEax, so people comparing the
instruction manuals to our implementation can be forgiven for thinking
that our code is wrong if it has DstEax instead.

If SrcEax is imminent then it perhaps it doesn't matter too much.

~Andrew

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