Re: Enable support to Intel Key locker instructions.

["H.J. Lu" <hjl.tools@gmail.com>]

On Tue, Sep 22, 2020 at 9:08 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 22.09.2020 10:53, Cui, Lili wrote:
> >> On 21.09.2020 05:25, Cui, Lili wrote:
> >>> --- a/opcodes/i386-dis.c
> >>> +++ b/opcodes/i386-dis.c
> >>> @@ -691,6 +691,7 @@ enum
> >>>    REG_0F18,
> >>>    REG_0F1C_P_0_MOD_0,
> >>>    REG_0F1E_P_1_MOD_3,
> >>> +  REG_0F38D8_PREFIX_1,
> >>>    REG_0F71,
> >>>    REG_0F72,
> >>>    REG_0F73,
> >>
> >> This addition wants to go further down. While not immediately visible here, ...
> >>
> >>> @@ -797,12 +798,18 @@ enum
> >>>    MOD_VEX_0F385E_X86_64_P_1_W_0,
> >>>    MOD_VEX_0F385E_X86_64_P_2_W_0,
> >>>    MOD_VEX_0F385E_X86_64_P_3_W_0,
> >>> +  MOD_0F38DC_PREFIX_1,
> >>> +  MOD_0F38DD_PREFIX_1,
> >>> +  MOD_0F38DE_PREFIX_1,
> >>> +  MOD_0F38DF_PREFIX_1,
> >>>    MOD_0F38F5,
> >>>    MOD_0F38F6_PREFIX_0,
> >>>    MOD_0F38F8_PREFIX_1,
> >>>    MOD_0F38F8_PREFIX_2,
> >>>    MOD_0F38F8_PREFIX_3,
> >>>    MOD_0F38F9,
> >>> +  MOD_0F38FA_PREFIX_1,
> >>> +  MOD_0F38FB_PREFIX_1,
> >>>    MOD_62_32BIT,
> >>>    MOD_C4_32BIT,
> >>>    MOD_C5_32BIT,
> >>
> >> ... in this table you'll notice that MOD_0F38* all go together, and _later_
> >> there's a MOD_VEX_0F38* group. I notice that recent additions (of yours?) also
> >> already violate this sorting model - please may I ask for this to corrected as well?
> >> The more outliers we have there, the more difficult will it be to maintain this
> >> code.
> >
> > Thank you reviewing my patch. I put MOD_VEX_0F38* together.
>
> Imo this should be a separate change, not merged into here.

Yes, please make a separate patch.

> >>> @@ -8236,6 +8319,16 @@ static const struct dis386 mod_table[][2] = {
> >>>      /* MOD_0F38F9 */
> >>>      { "movdiri",   { Edq, Gdq }, PREFIX_OPCODE },
> >>>    },
> >>> +  {
> >>> +    /* MOD_0F38FA_PREFIX_1 */
> >>> +    { Bad_Opcode },
> >>> +    { "encodekey128", { Gd, Ed }, PREFIX_OPCODE },  },  {
> >>> +    /* MOD_0F38FB_PREFIX_1 */
> >>> +    { Bad_Opcode },
> >>> +    { "encodekey256", { Gd, Ed }, PREFIX_OPCODE },  },
> >>
> >> The use of Gd and Ed will, afaict, lead to REX.W decoding as 64-bit register
> >> operands, which according to doc and gas implementation looks wrong.
> >
> > I didn't find the code we use REX.W to determine the size of register with Gd and Ed.
> >  Could you help elaborate on it? It seems that Gd and Ed are correct.
>
> My mistake - I mixed up Gd/Ed with Gv/Ev, sorry.
>
> >>> +Unspecified|BaseIndex } aesdecwide256kl, 1, 0xf30f38d8, 0x3, 3,
> >>> +CpuWIDEKL,
> >>> +Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf,
> >> {
> >>> +Unspecified|BaseIndex }
> >>
> >> ... these four need special treatment in output_insn()'s setting of
> >> GNU_PROPERTY_X86_FEATURE_2_XMM, due to the lack of explicit RegXMM
> >> operands.
> >>
> > Added it.
>
> If I was making a change like this, I'm pretty sure H.J. would ask me
> to also add test cases for it.
>

Yes, please add a new testcase.  See:

commit a7e12755d57879884c523cae1cf009efc9da933c
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Wed Feb 19 04:54:45 2020 -0800

    x86: Mark cvtpi2ps and cvtpi2pd as MMX

for an example.

Thanks.

-- 
H.J.