Re: [Patch v5 01/18] perf/x86: Disable LBR if x86_pmu.lbr_nr exceeds MAX_LBR_ENTRIES

[Mi, Dapeng]

On 10/29/2026 5:38 PM, Peter Zijlstra wrote:
> On Thu, Oct 29, 2026 at 12:38:57PM +0800, Dapeng Mi wrote:
>> Add a sanity check for x86_pmu.lbr_nr. If the hardware reports more LBR
>> entries than MAX_LBR_ENTRIES, disable LBR to avoid out-of-bounds access
>> in the cached LBR stack handling path.
> Cute, can this actually happen?

Currently it won't happen on real hardware since the maximum LBR stack
depth what hardware supports is 32. But it's better have this sanity check
assume the LBR stack depth could be extended to 64 or larger.


>
>> Signed-off-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
>> ---
>>  arch/x86/events/amd/lbr.c    | 8 +++++++-
>>  arch/x86/events/intel/core.c | 7 +++++++
>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
>> index a55646fcb846..96c5b5e56053 100644
>> --- a/arch/x86/events/amd/lbr.c
>> +++ b/arch/x86/events/amd/lbr.c
>> @@ -433,7 +433,13 @@ __init int amd_pmu_lbr_init(void)
>>  	ebx.full = cpuid_ebx(EXT_PERFMON_DEBUG_FEATURES);
>>  	x86_pmu.lbr_nr = ebx.split.lbr_v2_stack_sz;
>>  
>> -	pr_cont("%d-deep LBR, ", x86_pmu.lbr_nr);
>> +	if (x86_pmu.lbr_nr > MAX_LBR_ENTRIES) {
>> +		pr_warn("LBR entries %d exceeds max %d, disable LBR.\n",
>> +			x86_pmu.lbr_nr, MAX_LBR_ENTRIES);
>> +		x86_pmu.lbr_nr = 0;
>> +	} else {
>> +		pr_cont("%d-deep LBR, ", x86_pmu.lbr_nr);
>> +	}
>>  
>>  	return 0;
>>  }
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index 0a34d674df59..10c694db85ea 100644
>> --- a/arch/x86/events/intel/core.c
>> +++ b/arch/x86/events/intel/core.c
>> @@ -9023,6 +9023,13 @@ __init int intel_pmu_init(void)
>>  	 */
>>  	if (x86_pmu.lbr_tos && !check_msr(x86_pmu.lbr_tos, 0x3UL))
>>  		x86_pmu.lbr_nr = 0;
>> +
>> +	if (x86_pmu.lbr_nr > MAX_LBR_ENTRIES) {
>> +		pr_warn("LBR entries %d exceeds max %d, disable LBR.\n",
>> +			x86_pmu.lbr_nr, MAX_LBR_ENTRIES);
>> +		x86_pmu.lbr_nr = 0;
>> +	}
>> +
>>  	for (i = 0; i < x86_pmu.lbr_nr; i++) {
>>  		if (!(check_msr(x86_pmu.lbr_from + i, 0xffffUL) &&
>>  		      check_msr(x86_pmu.lbr_to + i, 0xffffUL)))
> Would it make sense to have but one copy of this check in
> init_hw_perf_events(), after calling into the micro-arch code?

Yes, that's what the previous version does, but Sashiko complains it's too
late.

"

Does zeroing x86_pmu.lbr_nr here in init_hw_perf_events() happen too late,
after intel_pmu_init() and amd_pmu_init() have already configured hardware
and driver state using the unvalidated x86_pmu.lbr_nr value?

"

https://lore.kernel.org/all/sashiko-outbox-179814@xxxxxxxxxx/

It makes sense in some ways, so move the sanity check into the vendor init
in this version. 

Thanks.