Re: [Xen-devel] [PATCH v4 10/17] x86emul: add tables for 0f38 and 0f3a extension space

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/02/17 12:54, Jan Beulich wrote:
> Convert the few existing opcodes so far supported.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v3: New.
>
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -43,6 +43,8 @@
>  #define SrcMask     (7<<3)
>  /* Generic ModRM decode. */
>  #define ModRM       (1<<6)
> +/* vSIB addressing mode (0f38 extension opcodes only), aliasing ModRM. */
> +#define vSIB        (1<<6)
>  /* Destination is only written; never read. */
>  #define Mov         (1<<7)
>  /* VEX/EVEX (SIMD only): 2nd source operand unused (must be all ones) */
> @@ -340,6 +342,28 @@ static const struct {
>      [0xff] = { ModRM }
>  };
>  
> +static const struct {
> +    uint8_t simd_size:5;
> +    uint8_t to_memory:1;

Depending on how often it is used, what about shortening to "to_mem"? 
It is no less clear.

> +    uint8_t two_op:1;
> +    uint8_t vsib:1;
> +} ext0f38_table[256] = {
> +    [0x2a] = { .simd_size = simd_packed_int, .two_op = 1 },
> +    [0xf0] = { .two_op = 1 },
> +    [0xf1] = { .to_memory = 1, .two_op = 1 },
> +    [0xf2 ... 0xf3] = {},
> +    [0xf5 ... 0xf7] = {},
> +};
> +
> +static const struct {
> +    uint8_t simd_size:5;
> +    uint8_t to_memory:1;
> +    uint8_t two_op:1;
> +    uint8_t four_op:1;
> +} ext0f3a_table[256] = {
> +    [0xf0] = {},
> +};
> +
>  static const opcode_desc_t xop_table[] = {
>      DstReg|SrcImmByte|ModRM,
>      DstReg|SrcMem|ModRM,
> @@ -2692,6 +2737,11 @@ x86_decode(
>              }
>          }
>      }
> +    else
> +    {
> +        modrm_mod = 0xff;
> +        modrm_reg = modrm_rm = modrm = 0;
> +    }
>  
>      if ( override_seg != x86_seg_none )
>          ea.mem.seg = override_seg;
> @@ -2740,6 +2790,13 @@ x86_decode(
>          break;
>  
>      case ext_0f3a:
> +        d = ext0f3a_table[b].to_memory ? DstMem | SrcReg : DstReg | SrcMem;
> +        if ( ext0f3a_table[b].two_op )
> +            d |= TwoOp;
> +        else if ( ext0f3a_table[b].four_op && !mode_64bit() && vex.opcx )
> +            imm1 &= 0x7f;

Is this sensible to do?  The behaviour of imm1 doesn't appear to be very
consistent across encodings.  As it is all passed onto hardware anyway
via stub, does it really matter?

~Andrew

> +        state->desc = d;
> +        state->simd_size = ext0f3a_table[b].simd_size;
>          if ( !vex.opcx )
>              ctxt->opcode |= MASK_INSR(vex.pfx, X86EMUL_OPC_PFX_MASK);
>          break;
>


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