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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.)

>> @@ -2491,16 +2493,14 @@ x86_emulate(
>>  
>>      case 0x90: /* nop / xchg %%r8,%%rax */
>>          if ( !(rex_prefix & 1) )
>> -            break; /* nop */
>> +            goto no_writeback; /* nop */
> 
> Could you add an explicit /* fallthrough */ here?  The only reason it
> isn't currently a coverity defect is because of the /* nop */ comment.

Will do; I actually had considered that, but then thought the
present comment is enough to silence Coverity.

> With these two, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

I'll wait with using this until the first point above got clarified.

Jan


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