Re: [Xen-devel] [PATCH v3 08/25] x86emul: add tables for XOP 08 and 09 extension spaces

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/02/18 15:15, Jan Beulich wrote:
>>>> On 02.02.18 at 12:43, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 07/12/17 14:04, Jan Beulich wrote:
>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>> @@ -458,6 +458,20 @@ static const opcode_desc_t xop_table[] =
>>>      DstReg|SrcImm|ModRM,
>>>  };
>>>  
>>> +static const struct {
>>> +    uint8_t simd_size:5;
>>> +    uint8_t two_op:1;
>>> +    uint8_t four_op:1;
>>> +} ext8f08_table[256] = {
>>> +};
>>> +
>>> +static const struct {
>>> +    uint8_t simd_size:5;
>>> +    uint8_t two_op:1;
>>> +} ext8f09_table[256] = {
>>> +    [0x01 ... 0x02] = { .two_op = 1 },
>>> +};
>>> +
>> What about 8f0a ?  We've got emulation for bextr already, and might want
>> to consider #GRP4 seeing as we expose LWP to guests.
> I'd prefer to convert that to a table at the time we need it.
>
>>> @@ -2726,7 +2740,7 @@ x86_decode(
>>>              }
>>>              break;
>>>  
>>> -        case vex_0f38:
>>> +        case ext_0f38:
>>>              d = ext0f38_table[b].to_mem ? DstMem | SrcReg
>>>                                          : DstReg | SrcMem;
>>>              if ( ext0f38_table[b].two_op )
>>> @@ -2736,7 +2750,14 @@ x86_decode(
>>>              state->simd_size = ext0f38_table[b].simd_size;
>>>              break;
>>>  
>>> -        case vex_0f3a:
>>> +        case ext_8f09:
>>> +            if ( ext8f09_table[b].two_op )
>>> +                d |= TwoOp;
>>> +            state->simd_size = ext8f09_table[b].simd_size;
>>> +            break;
>>> +
>>> +        case ext_0f3a:
>>> +        case ext_8f08:
>>>              /*
>>>               * Cannot update d here yet, as the immediate operand still
>>>               * needs fetching.
>>> @@ -2928,6 +2949,15 @@ x86_decode(
>>>          break;
>>>  
>>>      case ext_8f08:
>>> +        d = DstReg | SrcMem;
>>> +        if ( ext8f08_table[b].two_op )
>>> +            d |= TwoOp;
>>> +        else if ( ext8f08_table[b].four_op && !mode_64bit() )
>>> +            imm1 &= 0x7f;
>>> +        state->desc = d;
>>> +        state->simd_size = ext8f08_table[b].simd_size;
>>> +        break;
>> I presume that these don't actually impact our currently implemented XOP
>> instructions?
> No - as the description says, the patch converts the existing ones,
> it doesn't break them (and the test harness continuing to work
> confirms this).

Ok, in which case Reviewed-by: Andrew Cooper
<andrew.cooper3@xxxxxxxxxx>, conditional on the tables gaining names
(which you said you've already done IIRC).

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel