Enable support to Intel Key locker instructions.

Jan Beulich jbeulich@suse.com
Mon Sep 21 09:18:00 GMT 2020


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.

> @@ -2890,6 +2904,13 @@ static const struct dis386 reg_table[][8] = {
>      { "nopQ",		{ Ev }, 0 },
>      { RM_TABLE (RM_0F1E_P_1_MOD_3_REG_7) },
>    },
> +  /* REG_0F38D8_PREFIX_1 */
> +  {
> +    { "aesencwide128kl",	{ M }, 0 },
> +    { "aesdecwide128kl",	{ M }, 0 },
> +    { "aesencwide256kl",	{ M }, 0 },
> +    { "aesdecwide256kl",	{ M }, 0 },
> +  },

As you properly omit PREFIX_OPCODE here, ...

> @@ -3543,6 +3564,40 @@ static const struct dis386 prefix_table[][4] = {
>      { "maskmovdqu", { XM, XS }, PREFIX_OPCODE },
>    },
>  
> +  /* PREFIX_0F38D8 */
> +  {
> +    { Bad_Opcode },
> +    { REG_TABLE (REG_0F38D8_PREFIX_1) },
> +  },
> +
> +  /* PREFIX_0F38DC */
> +  {
> +    { Bad_Opcode },
> +    { MOD_TABLE (MOD_0F38DC_PREFIX_1) },
> +    { "aesenc", { XM, EXx }, PREFIX_OPCODE },

... I wonder why you add it here (and several more times below).
It's generally pointless to have this attribute when in
prefix_table[] or already having gone through it. There are many
pointless uses left, yes, but they're slowly being eliminated.

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

> --- a/opcodes/i386-opc.h
> +++ b/opcodes/i386-opc.h
> @@ -259,6 +259,10 @@ enum
>    CpuSEV_ES,
>    /* TSXLDTRK instruction required */
>    CpuTSXLDTRK,
> +  /* KL instruction support required */
> +  CpuKL,
> +  /* WIDEKL instruction support required */
> +  CpuWIDEKL,

May I suggest CpuWideKL? I see no need for the excess capitalization.

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -4117,3 +4117,19 @@ tilerelease, 0, 0x49c0, None, 2, CpuAMX_TILE|Cpu64, Vex128|VexOpcode=1|VexW0|No_
>  tilezero, 1, 0xf249, None, 1, CpuAMX_TILE|Cpu64, Modrm|Vex128|VexOpcode=1|VexW0|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegTMM }
>  
>  // AMX instructions end.
> +
> +// KEYLOCKER instructions.
> +
> +loadiwkey, 2, 0xf30f38dc, None, 3, CpuKL, Load|Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64, { RegXMM, RegXMM }
> +encodekey128, 2, 0xf30f38fa, None, 3, CpuKL, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64, { Reg32, Reg32 }
> +encodekey256, 2, 0xf30f38fb, None, 3, CpuKL, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64, { Reg32, Reg32 }

These two and ...

> +aesenc128kl, 2, 0xf30f38dc, None, 3, CpuKL, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Unspecified|BaseIndex, RegXMM }
> +aesdec128kl, 2, 0xf30f38dd, None, 3, CpuKL, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Unspecified|BaseIndex, RegXMM }
> +aesenc256kl, 2, 0xf30f38de, None, 3, CpuKL, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Unspecified|BaseIndex, RegXMM }
> +aesdec256kl, 2, 0xf30f38df, None, 3, CpuKL, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Unspecified|BaseIndex, RegXMM }
> +aesencwide128kl, 1, 0xf30f38d8, 0x0, 3, CpuWIDEKL, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Unspecified|BaseIndex }
> +aesdecwide128kl, 1, 0xf30f38d8, 0x1, 3, CpuWIDEKL, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Unspecified|BaseIndex }
> +aesencwide256kl, 1, 0xf30f38d8, 0x2, 3, CpuWIDEKL, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { 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.

I'm also not seeing why you add IgnoreSize in a number of
cases. Could you remind me what would break without it? We're
trying to eliminate stray uses, after all, so new ones would
better not appear.

Jan


More information about the Binutils mailing list