From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01on2048.outbound.protection.outlook.com [40.107.15.48]) by sourceware.org (Postfix) with ESMTPS id 6BE393858D33 for ; Mon, 16 Oct 2023 12:11:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6BE393858D33 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 6BE393858D33 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=40.107.15.48 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1697458285; cv=pass; b=wbhn03uV6CbZXgMl3CR88EFAVs+yE55NgF1ho0EZT8Y2bNaE8q8ePkIgcyiPegvTGrNtq0hAGQ3SgpKapgb3wLxu5ipZeJncoq3BaVIAIx+6vtayAJWMi8FJF8KP++RKsQ50OoZQrRRfLrtyYFQ+Sn8dC2MCAr7mH1y0UDSbEac= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1697458285; c=relaxed/simple; bh=dkvs7+VnTOUMxSEdveH04wDzW1L64dxkqRtsLCM3rNA=; h=DKIM-Signature:Message-ID:Date:Subject:To:From:MIME-Version; b=ZoS45uY2j8vP0+5cMj2Ne+eS5UXsjAEDAa7wJ5aK8/0TMjk8QFEusiiEd753IU5gRaqi/pb7FbmY6alvdEaa20a8+z/prKGT8czf0E0PXfHz8G+MlSPbkX2dcwDp0+U8Ok140Q/Re6PeFtbhpQ9lom+MEhtMpkDlHbc/6uMWSEU= ARC-Authentication-Results: i=2; server2.sourceware.org ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UKtelSdn4kDIcu94QBrFzlDPg4QVGn8z22yqfW7ljPwROY+8nwmcX2b0GecV9tEXJQYdjknNkcLxTbYMAaAfCBOIrnSJIGCAd41ZCW8L2hnBbSPHs40HW96eiH3ByFrONMrsFkV+ms1H0T+0ZF82CHcB0VNSoVGGkrXv3vshdcrX+HeP9wthscQhVWgfQuMHTwBaCHeg9g6+HgQ+DlFkSZEq9O6UbwlCQS/ODEGKz6FLJMDCf9vMZ6w3fKpyK5nfqlep9+xRo4umUxDQ7BFeML6PB1z14qO0B1KQVZ4REZDKoLaLEncDxkGmeg2UnHWfcqalohP/2rOtBkVg4Des7Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=/Yto+1f7Wd8kTAZxi65qPoo5lcBRLyxiFXIVShT5rzQ=; b=NS1JILcxDv/9yMqAo+y9v4SjVjt1xMFXpiEj564Eq+mCBTLF8bk17KdKcwzJp5Ohjjtnp4kZQtiTezLWJmf9w3JlMuNNtMwh1V/ER7H6R44Gq2Ey0H9M1LoLy/wu9vm4K4T516xFMXDN2qHbsg4h1WNVLBE/tvopBPSzrwofQ3tS7lhDU4oLWGD+DAr3xKIQODegYkSoW/k+6EMgKB4/DNNRs9Ua3mDMmo9myVC3Z2VyHCjfvkGf2p6et82EUpdgfP5gYqL5lrJFRGATAfqHVKFDfB8nQ2VGOYatQTpTkG9iunDibMqviH+PI+2NZ3mlAinfFE14Kvy2frfGxSkAzQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=/Yto+1f7Wd8kTAZxi65qPoo5lcBRLyxiFXIVShT5rzQ=; b=kWmCjqAz3WlRXNgq+JZD3h7bcsEtEgCcsofPrAgHDqBBDb0ZvebPa/xV5K/sq5uCVZ+ahv4s/Ft1vWCELVAuYfelOYD6OovlK6XRXuFvP+S1sKhWN7/DaqTuvQstm/q7LkDkVzFuETbpYGcYzb7ZKD8234j4ac+9JNFtQfxbcJ205rK3A8zqzleIf9ToWmGaWq1jyT1wFddUfExqed0KtI9W4NQZMusI31tCgOFMYdgQpL2mIytnGXmiCPcGE+uf0nTlx2r1Vjed5YEx+qF4x5wj3KI2bubC8NZ2o8xR3yVYAXQXKrG83md9hpYwidKZjwY6edVIXmQNXtRgwmrOXA== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com; Received: from DU2PR04MB8790.eurprd04.prod.outlook.com (2603:10a6:10:2e1::23) by DB9PR04MB8411.eurprd04.prod.outlook.com (2603:10a6:10:24c::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6886.35; Mon, 16 Oct 2023 12:11:20 +0000 Received: from DU2PR04MB8790.eurprd04.prod.outlook.com ([fe80::d9c0:d907:4d2d:15b3]) by DU2PR04MB8790.eurprd04.prod.outlook.com ([fe80::d9c0:d907:4d2d:15b3%6]) with mapi id 15.20.6886.034; Mon, 16 Oct 2023 12:11:20 +0000 Message-ID: Date: Mon, 16 Oct 2023 14:11:17 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH] Support Intel USER_MSR Content-Language: en-US To: "Hu, Lin1" Cc: hongjiu.lu@intel.com, binutils@sourceware.org References: <20231010072401.1383177-1-lin1.hu@intel.com> From: Jan Beulich In-Reply-To: <20231010072401.1383177-1-lin1.hu@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: FR3P281CA0148.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:95::7) To DU2PR04MB8790.eurprd04.prod.outlook.com (2603:10a6:10:2e1::23) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DU2PR04MB8790:EE_|DB9PR04MB8411:EE_ X-MS-Office365-Filtering-Correlation-Id: 47b1b4f0-af2f-4b90-b4ab-08dbce410179 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: XNAy6HwnaVH1H5z614gmXPxUZd27dFRmyJT317zNHXTWJonqEN49QRYuUE1uOKojtjzg1vV8Pn0KskndMYxWx8xetS2L1FHubFDqyY4ITYRx2kVvhqKASwxVgJ6A6pSrxEvpKsKIpuU4miuC72oEGA+WCoJiQCIZ98opK4xNyvZtHYlIvzoQy83gEGJCuvMvLBwD/zKopeRqfjpYVsClcBVDIaNHNHNzCDmVG4kuHupsOpubqiL364U/HcUpfDNFSkBxXjgv4FX702YGxCFjyK6bkfsFhbN6kR9WN9tsAkDd3jXjGxLLOvS0tT2MUzSobcz0soG4dzptc8MSNHRX00R6n3+aZLyQMCcFo8mFEkRj4NYV0zm+Ql+SuOiFyJn/nyDQe7Vvc6D3xYclMFh+fkSSCo6O7Tup57Ev/FoJQCLf2MfHJBbSF4CMQ55a59TDWQgKlRxGoxRXcMUL+xFDzNE85JkprtmJHBlCehSWJGJLZriqWIvirm46/BCGUYzsaEijD4UVNyH8wdBCXzOhKx2+OncXhw7DqpqteR89ZwHtfqbsu4bHOuVXS6jmvdddIwSZSaDHq9PlIlW5z3RgdSVyG5mZt6OCUNgsxnKyELzKTyrPxnVjJbsBIgaxQtf/Hw0kT9JRLyTJyKNwaoY6BA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DU2PR04MB8790.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(346002)(136003)(366004)(396003)(376002)(39860400002)(230922051799003)(451199024)(64100799003)(1800799009)(186009)(66899024)(31686004)(6486002)(36756003)(38100700002)(83380400001)(6666004)(53546011)(6512007)(26005)(6506007)(66946007)(66556008)(66476007)(6916009)(316002)(478600001)(2616005)(2906002)(41300700001)(31696002)(86362001)(5660300002)(8936002)(8676002)(4326008)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?L21ST2YyYzVrSmE0d1dqWGtkb0RkMisvazR1S05NRnJWazJOcnNxK21XTjB1?= =?utf-8?B?azd6a2Jid3ZTaU16T0s4Mm1MRWpRSkdpcFluaWgzd1VIU1hvbEtDeVdjcVBx?= =?utf-8?B?Ky80UzRubFFtajFWU0NSZ1ZZN2hrT2ZyV3pqMmhsSUFkaEl1ZmtsU25USWVT?= =?utf-8?B?bjdodit6VGRPa2haYUl3N0N3ZGRRSVZySHJXOVRLWGUxYzlRb01qbnJzR1dr?= =?utf-8?B?dElTWXJmZEdRYnpzMjVGaDhLZ1YxK0Rpd0IrNlQ3dDhrNWVWMXZrUE4yRnIv?= =?utf-8?B?NTA0eXlIdEJtOGRTNkZEK0MxZ0k4dkYvWDVKc0lrMDZ5NzljbWpCM3NxNVBX?= =?utf-8?B?QzZiMkp5OTRPRjdRTHBReWNqRmpCSGNiWDFXeWoxMHhuOW0xcnJQK3piWk5T?= =?utf-8?B?U1NnbnVZUjFoZkJVcFg2Q082eDZjazhTS05BLzc4a21FR2V1YUF1TDNwT3g3?= =?utf-8?B?U25DNHczaUd1OFJIS1N5SGVMZHRzTlIxZGphK2loNlZsWk1LK2ZGelFsc282?= =?utf-8?B?L3gwZHBKRzRXLzVndHArcmpkVHo3OVU4eGJXTmdWR2xUbWVBd1h6aVgwMTht?= =?utf-8?B?aUlVa1ovRHpQSXRaMHNiVmNJN2xVZGZ0dW9jUHZ4dnYxdXNMZzF6dXc2Sk5h?= =?utf-8?B?TGl3NmIwZkFGQ2Qwc0UzaElsdVlkN3NKRGVBUkhFcWNHOHhtWU9JbXZkM0pu?= =?utf-8?B?bnZsMXI5MXJScU43azBRbXFZaDRpb1JYdmhac1c4T2l3UDZXMUhCWFZnaFJ0?= =?utf-8?B?cGV4bTV4RWFPMWl2akFzMGpSd1YweTRqK2dMMjdKUU4xL3V1Nk5TWExQcFlI?= =?utf-8?B?aFZqNi85ZFpnZWs1cDVrMkNLcG4zd0w1d2Z4UVBxMlN3TFV5UUxCZW1YSzNx?= =?utf-8?B?U05hb1M1RURsWHR4dG5TdUJST2FnM2Q2ZmswaUhoSVBERE1ZRDBWemR5OGg5?= =?utf-8?B?YWNqa2xDa0t2OVptNUtNY2c3REVEbzlEK2RmNmU0WFZ6TmRjK252VU53dE9z?= =?utf-8?B?aFhGOVNkNjcxZ0RnOWVrOGxjWkNTOGdpZXJaa0dwWTBLZXkwTUFkUFFoKzdh?= =?utf-8?B?aVdFcmZoMFRob1ZPK0tGWnRibWhadkVrS050bVl1aTZqcEE0UmZ3RnhCcjFl?= =?utf-8?B?a0lCNWNQRjRVaWVRRjNMZGpvRFBudGo5MVhBZUlrRmh0RnFRbU5xQlVzZS94?= =?utf-8?B?b2E4SGNCZTF0blo0RC83eHZ4VUxkaUdkZENobkNJcnVQU1MxdUpLdXNONmEv?= =?utf-8?B?QUlPb0xNVmdoNE1QTmE2akU0OVBTRTRBeHMxRjREOXV3Z2tuQ29Qck53dGdF?= =?utf-8?B?YXpHVjhhaEZ5N2RCamdacWhVR21NckhGaFozRUhRTXl4NDEwdHpnTkEzQkNX?= =?utf-8?B?REJnZi96ZjZUcC8weVp0UzY1Rkc4cFp2MUMzODJLUVovbXBDb1Vkbm5PU2Vw?= =?utf-8?B?YVlNYVkwVmJUckN5eUtSZG9vV0lmcEgzU2ZYcDkwRDBDVS9YeUpUaGF6bXBa?= =?utf-8?B?Mkt3aEJoL1JHS0NVQjVsREs3UHJ2K01rUVI1YkJ4Mk9uOFBGbGdtcGxuanpq?= =?utf-8?B?ZmRsVTZ1eGdyR09kSWEvRFQ5ZW5rQm5KcnBjMThBZTBERzZXWERsY1pvTDdS?= =?utf-8?B?TkREdlZTL0d3bEJBMXllenNiV1RFd1ZIWUxZejhvc2JvZHc5azBDL2hUWTNC?= =?utf-8?B?czFjSndvbWNTWEV0SjN1aTJiRThQdDY3NzFpa2VPVjU2STh4ckFGS3Erb0hF?= =?utf-8?B?bzlTbDZEeUk5ZlU1TWg1eFZ6bzJGTnpRdGlMNG9mS05NZnJydmRWMWdvWlB0?= =?utf-8?B?MEkza1ZReVZ2VC95VjRnTkxkSEVpMTJmdjlhZFdoVTVLV2tDUVQxaEFsM1A4?= =?utf-8?B?WTIyd09ENDZ1OElTZnpMbWV0OUZjSCsxODVpajJ0MWF5NkhNYzllaisyUlBR?= =?utf-8?B?VTUyL0VSMmhMalgzcEpJbFhSYVZvTTgwVXdaYUVpMWw2QmVKbWVXdzUzcHdq?= =?utf-8?B?RExBd0JiekVjbGxJOHpPUE5aUjJJWjZlQnA3bTlJamRRSUl4My9iWXR6a1lP?= =?utf-8?B?TFN2d0YvZERqMU5sV2VxYlZqT1A0NEprMzFpK284bWM4MzZZdEUyc1NnK3Ba?= =?utf-8?Q?Q6qbHxSXkrFyNNmdm+vux5RYv?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: 47b1b4f0-af2f-4b90-b4ab-08dbce410179 X-MS-Exchange-CrossTenant-AuthSource: DU2PR04MB8790.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Oct 2023 12:11:20.7411 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: N7vbOMYUjemLvHam4v41kcoFjkPgln5WW8MCyymaCeWb9FDd9nh2Y2kiq1epIpTGBfRGJ4EH95qdBn7o4SKL8w== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR04MB8411 X-Spam-Status: No, score=-3027.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,LOTS_OF_MONEY,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 10.10.2023 09:24, Hu, Lin1 wrote: > @@ -3863,6 +3864,8 @@ build_vex_prefix (const insn_template *t) > case SPACE_0F: > case SPACE_0F38: > case SPACE_0F3A: > + case SPACE_EVEXMAP5: > + case SPACE_VEXMAP7: > i.vex.bytes[0] = 0xc4; > break; I can see the need for the latter line you add, but why the former? (If it is needed for some reason, this is a strong hint at the description being overly brief.) > @@ -8752,6 +8755,18 @@ build_modrm_byte (void) > source = v; > v = tmp; > } > + if (i.tm.opcode_modifier.operandconstraint == SWAP_SOURCE_DEST) > + { > + if (dest == (unsigned int) ~0) > + source = source ^ 1; > + else > + { > + unsigned int tmp = source; > + > + source = dest; > + dest = tmp; > + } > + } Why is this needed? There's only a single register operand in both affected insn forms (see comment below on the 2-register form). Furthermore I think it would be easier if you "canonicalized" the early immediate to be the 1st operand, such that for all other purposes immediates remain first. As a cosmetic nit: Please have a blank line ahead of the if() block (if it needs to stay). > --- /dev/null > +++ b/gas/testsuite/gas/i386/user_msr-inval.s > @@ -0,0 +1,7 @@ > +# Check Illegal 32bit USER_MSR instructions > + > + .allow_index_reg > + .text > +_start: > + urdmsr %r12, %r14 #USER_MSR > + uwrmsr %r12, %r14 #USER_MSR As per comments on earlier series: What use are the comments here? (Applicable also again below.) > --- /dev/null > +++ b/gas/testsuite/gas/i386/x86-64-user_msr.s > @@ -0,0 +1,15 @@ > +# Check 64bit USER_MSR instructions > + > + .allow_index_reg > + .text > +_start: > + urdmsr %r14, %r12 #USER_MSR > + urdmsr $51515151, %r12 #USER_MSR > + uwrmsr %r12, %r14 #USER_MSR > + uwrmsr %r12, $51515151 #USER_MSR > + > +.intel_syntax noprefix Nit: Please indent directives. > + urdmsr r12, r14 #USER_MSR > + urdmsr r12, 51515151 #USER_MSR > + uwrmsr r14, r12 #USER_MSR > + uwrmsr 51515151, r12 #USER_MSR I think varying registers slightly more (such that each two-register form has one low-8 and one high-8 operand, totaling to two forms each to prove that the REX.[RB] bits are also correctly dealt with) would be better. Btw, what's the interaction here with APX? The legacy forms are going to use REX2, but the VEX forms would need EVEX variants then. > @@ -618,6 +620,8 @@ enum > w_mode, > /* double word operand */ > d_mode, > + /* double word operand 0 */ > + d_0_mode, Why is this needed? IOW why does d_mode not do? Or alternatively why isn't this a name indicating that it's an unsigned 32-bit value (as opposed to other 32-bit immediates in 64-bit mode)? > @@ -845,6 +849,7 @@ enum > REG_VEX_0FAE, > REG_VEX_0F3849_X86_64_L_0_W_0_M_1_P_0, > REG_VEX_0F38F3_L_0, > + REG_VEX_MAP7_F8_L_0_W_0_M_1, > > REG_XOP_09_01_L_0, > REG_XOP_09_02_L_0, > @@ -893,8 +898,10 @@ enum > MOD_0FC7_REG_6, > MOD_0FC7_REG_7, > MOD_0F38DC_PREFIX_1, > + MOD_0F38F8, > > MOD_VEX_0F3849_X86_64_L_0_W_0, > + MOD_VEX_MAP7_F8_L_0_W_0, > }; As before - no new mod_table[] entries please which don't have both branches populated. > @@ -6791,6 +6839,297 @@ static const struct dis386 vex_table[][256] = { > { Bad_Opcode }, > { Bad_Opcode }, > }, > + /* VEX_MAP7 */ > + { > + /* 00 */ > + { Bad_Opcode }, I wonder whether adding a full new table (rather than some special case code) is really a god use of space. Of course if you know that more of it will be populated in the not too distant future ... > @@ -11248,6 +11609,20 @@ get32s (instr_info *ins, bfd_vma *res) > return true; > } > > +/* The function is used to get imm32, when imm32 is operand 0, and ins only has 2 operands. */ > +static bool > +get32_operand0 (instr_info *ins, bfd_vma *res) > +{ > + > + if (!fetch_code (ins->info, ins->codep + 5)) > + return false; > + *res = *(ins->codep++ + 1) & (bfd_vma) 0xff; > + *res |= (*(ins->codep++ + 1) & (bfd_vma) 0xff) << 8; > + *res |= (*(ins->codep++ + 1) & (bfd_vma) 0xff) << 16; > + *res |= (*(ins->codep++ + 1) & (bfd_vma) 0xff) << 24; > + return true; > +} Instead of this (which assumes ModRM.mod == 3) I think you want to arrange for dealing with ModRM first. We already have OP_Skip_MODRM() for such needs, which you could use in a first "hidden" operand. > @@ -3346,3 +3349,12 @@ erets, 0xf20f01ca, FRED|x64, NoSuf, {} > eretu, 0xf30f01ca, FRED|x64, NoSuf, {} > > // FRED instructions end. > + > +// USER_MSR instructions. > + > +urdmsr, 0xf20f38f8, USER_MSR|x64, Modrm|IgnoreSize|SwapSourceDest|NoSuf, { Reg64, Reg64 } Iirc RegMem is the attribute to use here, not any new one. > +urdmsr, 0xf2f8/0, USER_MSR|x64, Modrm|Vex128|VexMap7|VexW0|IgnoreSize|NoSuf, { Imm32S, Reg64 } This and ... > +uwrmsr, 0xf30f38f8, USER_MSR|x64, Modrm|IgnoreSize|NoSuf, { Reg64, Reg64 } > +uwrmsr, 0xf3f8/0, USER_MSR|x64, Modrm|Vex128|VexMap7|VexW0|IgnoreSize|SwapSourceDest|NoSuf, { Reg64, Imm32S } ... this needs to use Imm32, not Imm32S. I understand this is going to cause complications elsewhere, but we can't afford getting this wrong. Also in all forms I think you don't mean IgnoreSize, but NoRex64. Jan