Received-SPF: pass (google.com: domain of linux-kernel+bounces-101346-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <960ce5ba-a8b1-4db7-b76c-b97869c61f12@amd.com>
Date: Wed, 13 Mar 2024 15:56:43 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 1/3] perf/x86/amd/lbr: Use freeze based on availability
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
 x86@kernel.org, peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
 mark.rutland@arm.com, alexander.shishkin@linux.intel.com, jolsa@kernel.org,
 namhyung@kernel.org, adrian.hunter@intel.com, tglx@linutronix.de,
 bp@alien8.de, eranian@google.com, irogers@google.com,
 mario.limonciello@amd.com, ravi.bangoria@amd.com, ananth.narayan@amd.com
References: <cover.1706526029.git.sandipan.das@amd.com>
 <12f378d5c9459e765c6c3a14b092e6f91da596e3.1706526029.git.sandipan.das@amd.com>
 <ZfF8sIol5xQcJM9F@gmail.com>
Content-Language: en-US
From: Sandipan Das <sandipan.das@amd.com>
In-Reply-To: <ZfF8sIol5xQcJM9F@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?bVdkYllWeW1IdGhERDQ4WUFXQWpnVVdVbmlBdDZKTndDTk42bXZyM3BEcUMy?=
 =?utf-8?B?SldIZUVMblBrWHR6TEprWE92UGNsTlowb1NxSGw2NXBYMGlWVGhxbGFJYkNG?=
 =?utf-8?B?aW1Cc3VRb0FtTlFxSWRrZ0dIclJiYXF1Z296WDZoMDhLU3Z6b2w4blNSNjd5?=
 =?utf-8?B?NTNjUUs5TjFEWHRFWG1iVHUvT1JLaUhjQzA3TDdsamVWeHlmK3h6SEJPTDlF?=
 =?utf-8?B?bGlIeWtjdmRxZ1FNLzBJdlB2Y243eThnbFgzRkpDU0tUckVySVoyZURObXU5?=
 =?utf-8?B?cFdUOEpPWm1iUWtjVWxPWFBzYXdRaFJ3NFVMd0d6K0xkRitzUUI5QjlVWkw5?=
 =?utf-8?B?UDZVRzBkQmR2MlMrVXlmZlpISHJDQUZiTklyeHRPV2g3MFcyNGZ4L2ZkN1ht?=
 =?utf-8?B?RkNDV1FVeHNGK1JVTVA1VGt3SFJrbEdyREUwREhRejVrZ1VISkxlbEVVR1J0?=
 =?utf-8?B?NTM5blo2QytJYkhPaVJUanlJUWJ3ZlpUMWtFcWYzd2crdUdBRWhna0lDcUx2?=
 =?utf-8?B?VkhjMGRCL1hwSlBqM3N2ODc0bk5QS01mRGNKdVJPK3lhd0ZLdVNEUFFwYTJU?=
 =?utf-8?B?djlSOWx1ZUdCVmxiZGxsVmxVekJCRW5hcTl3Tzd0dXBrd0x1Y0x0Yks3S0d2?=
 =?utf-8?B?YTFYY3A5UGg2VFdFL1ZIWEQrb3ZGTWFTdi9FRGxOSW9xUnJVeGhsbUxhRzNQ?=
 =?utf-8?B?L0htMnJIeTZvYklidlVIM004SWRHOGZ4MStXNHVJQzNSM3g2SWhiK3Q3QmtY?=
 =?utf-8?B?V0pCQzhOLyt5b2t5dnVOekdMeFQrdG9uNjh5dlZNR1ZnbjA1ZHZqekQ0S3lI?=
 =?utf-8?B?c296RFhnUFlteGdDZlIvOTFSNDBJdlJLVUsxRS9uQWoyNllBd2ZranhJUGJv?=
 =?utf-8?B?NVYraHpRZGpaTERtbUxtWVBTOXMxMU5nRWQ0ZTJZaTE0dnRicThQY0pDQXN3?=
 =?utf-8?B?bWZ2Y2JWckVmZ1YyWmVQQUg1cXRCR3RVdDIxK1RKZ2lUUjlvcUYrY0hMQlZB?=
 =?utf-8?B?R24yMFdhME5Hc3ViYmphbWpoeFdJRmg4akljSGl4Y2hZVVZ6UUdoNW5JUlZT?=
 =?utf-8?B?ZHBmSG1zV3gvV1hTaUI1TU5mblUvL0lXV2FyMVNQY3ZkdnlRQkhDREk3MUtW?=
 =?utf-8?B?NzB6UHlHTXVXYUo4RmppdEpBTlV2N25tTW0yL2VNUTZnQis1WWptSXpFMU5w?=
 =?utf-8?B?dVM1dTR5WjhDS3Fzd2tiaDZwZitnbmVKekJkbWVBc1ZkNTRkNUNyclBBYVps?=
 =?utf-8?B?TUMvNWwyQVhiSVE5bXJsSnQySkhqSktCQnB4M01NNEVCSE80eFdqbHJsWS9F?=
 =?utf-8?B?UHpoWGhrUDlvNFVZYkR0clNuaC90U2c1bHB3cDAybnl2b2YxQ0srNHpadUNG?=
 =?utf-8?B?dVFIVW9TeWtWWTcrOWp6YkVuSDB0RU9RM2NRRkIwYzg1dVNOZW1JNnExUkZu?=
 =?utf-8?B?QnhRUGVRMWJzN2FUZWpCUnNQWFRJYjI0UWtMdXJtVm84T3BtaE85MEF2eXcy?=
 =?utf-8?B?M29oNkZsM1NTVkFCcHpTQjVTbk05S3RaeW1Vc3ZmSVYyY2tVVHFveHFRbDE2?=
 =?utf-8?B?QkJBZk9pWVNkQzY4S3ZPN2huNGFyaDFzaFRseFlaM25TVnd6NTN0QU4ydDFH?=
 =?utf-8?B?eXd3WTdVOTA4eW9rdmZiQ29DM25KMExmQ0M1ZnNqYmtKRmJrN1FKckJlNE04?=
 =?utf-8?B?YXZNbVlPbDNMSW1CQ1UzU3dGaVplRHFESlZHNEorYVRhUXk1UXhRNE9KNzFY?=
 =?utf-8?B?NVIxMXdRd2FVb0FQc2luVjJSSGUxMDBuR2t5cFAvajhqdi9IOXlMTkxreVRG?=
 =?utf-8?B?RkhzY1l3UTNKblB0Y29MKzg5NVVmalE5clNnVW90Z0FzQ09EeHVvN2RCY2Uy?=
 =?utf-8?B?WnJpc2lKOEVBTlZiVEVROE1VQmhHN3l0NWZWaUd0aWNkY0hDU1FsT2Fpbndr?=
 =?utf-8?B?aDRWQUJuN05EMGVaNGNrYzF0K2pXN1MzRUlGcldqTlNlUTZrQ2NROHpZSk82?=
 =?utf-8?B?djBKWStoSkRDNXF6akM4L29uMUZUbHNXVjkxaXRFZWN4eTdnRlRkaStBZlB5?=
 =?utf-8?B?QkV6Zk85QnUxM2FRNW9WQnpjRklFQWwvVVZzcU9mdWlxRzhVd0xhSUtHVk5a?=
 =?utf-8?Q?pCPeDH/v1RV/qeF9LXTEbY/e3?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 60b704b5-6b27-4601-36e3-08dc43481b95
X-MS-Exchange-CrossTenant-AuthSource: PH7PR12MB5712.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Mar 2024 10:26:56.0399
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: wyIO84NFfg6c+xs/bgHRgV/xKe8QZniS3KPIVJcQPUKqhbhlp/TU6c4DW+twAhKMrjx+n7v8nk4CgzaKYQ86Kw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR12MB8477

On 3/13/2024 3:45 PM, Ingo Molnar wrote:
> 
> * Sandipan Das <sandipan.das@amd.com> wrote:
> 
>> Currently, it is assumed that LBR Freeze is supported on all processors
>> which have CPUID leaf 0x80000022[EAX] bit 1 set. This is incorrect as
> 
> That's X86_FEATURE_AMD_LBR_V2, right? Should probably be mentioned in the 
> changelog.
> 

Yes. I'll add it to the changelog.

>> the feature availability is additionally dependent on CPUID leaf 
>> 0x80000022[EAX] bit 2 being set which may not be set for all Zen 4 
>> processors. Define a new feature bit for LBR and PMC freeze and set the 
>> freeze enable bit (FLBRI) in DebugCtl (MSR 0x1d9) conditionally.
> 
> What happens on such Zen 4 CPUs that don't support LBR Freeze? Does the CPU 
> just ignore it, or something worse?
> 

In this case, LBR ignores PMC overflows and the branch records keep getting
updated continuously as execution progresses.

>> It should still be possible to use LBR without freeze for profile-guided
>> optimization of user programs by using an user-only branch filter during
>> profiling. When the user-only filter is enabled, branches are no longer
>> recorded after the transition to CPL 0 upon PMI arrival. When branch
>> entries are read in the PMI handler, the branch stack does not change.
>>
>> E.g.
>>
>>   $ perf record -j any,u -e ex_ret_brn_tkn ./workload
>>
>> Since the feature bit is visible under flags in /proc/cpuinfo, it can be 
>> used to determine the feasibility of use-cases which require LBR Freeze 
>> to be supported by the hardware such as profile-guided optimization of 
>> kernels.
> 
> Sounds good to me.
> 
>> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
>> index 4af140cf5719..e47ea31b019d 100644
>> --- a/arch/x86/include/asm/cpufeatures.h
>> +++ b/arch/x86/include/asm/cpufeatures.h
>> @@ -97,7 +97,7 @@
>>  #define X86_FEATURE_SYSENTER32		( 3*32+15) /* "" sysenter in IA32 userspace */
>>  #define X86_FEATURE_REP_GOOD		( 3*32+16) /* REP microcode works well */
>>  #define X86_FEATURE_AMD_LBR_V2		( 3*32+17) /* AMD Last Branch Record Extension Version 2 */
>> -/* FREE, was #define X86_FEATURE_LFENCE_RDTSC		( 3*32+18) "" LFENCE synchronizes RDTSC */
>> +#define X86_FEATURE_AMD_LBR_PMC_FREEZE	( 3*32+18) /* AMD LBR and PMC Freeze */
>>  #define X86_FEATURE_ACC_POWER		( 3*32+19) /* AMD Accumulated Power Mechanism */
>>  #define X86_FEATURE_NOPL		( 3*32+20) /* The NOPL (0F 1F) instructions */
>>  #define X86_FEATURE_ALWAYS		( 3*32+21) /* "" Always-present feature */
> 
> Could you please port this to the latest upstream kernel? The 3*32+18 slot 
> is now used for another purpose, and we need to define a new synthethic 
> CPUID word, word 21 if I'm counting it right.
> 
> Don't forget to increase NCAPINTS from 21 to 22, and consider the fixed 
> asserts in the x86_bug_flags[] definitions in <asm/cpufeature.h>, and the 
> asserts in <asm/disabled-features.h> and <asm/required-features.h>. This 
> new word should probably be added in a separate preparatory patch.
> 

Sure. I'll rebase and send this along with the other changes.
Thanks for the review.