From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 018C33857C54 for ; Mon, 21 Sep 2020 09:18:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 018C33857C54 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 0DA28AEAC; Mon, 21 Sep 2020 09:18:37 +0000 (UTC) Subject: Re: Enable support to Intel Key locker instructions. To: "Cui, Lili" Cc: "binutils@sourceware.org" , "H. J. Lu" References: From: Jan Beulich Message-ID: Date: Mon, 21 Sep 2020 11:18:00 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3033.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Sep 2020 09:18:03 -0000 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