Re: [Patch v6 15/20] perf/x86/intel: Refresh LBR stack for non-PEBS LBR events when needed

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 11/6/2026 1:58 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] perf/x86/intel: unrefreshed or clobbered LBR stack on
>   indirect PEBS drain
> --
>
> commit f13e61130af8b47007c91f048b386359760f05ad
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> perf/x86/intel: Refresh LBR stack for non-PEBS LBR events when needed
>
> This commit tracks whether the shared LBR stack has been refreshed during PMI
> handling with a lbr_refreshed flag and removes the unconditional LBR read at
> PMI entry so the live LBR state is read on demand for non-PEBS LBR events and
> re-read if PEBS draining overwrites it.
>
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index fbde874af6182..b267c68f94676 100644
>> --- a/arch/x86/events/intel/core.c
>> +++ b/arch/x86/events/intel/core.c
> [ ... ]
>
>> @@ -4003,8 +4013,19 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
>>  		 *
>>  		 * The PEBS buffer has to be drained before handling the A-PMI
>>  		 */
>> -		if (is_pebs_counter_event_group(event))
>> +		if (is_pebs_counter_event_group(event)) {
>> +			/*
>> +			 * PEBS counter sampling is only supported on adaptive PEBS
>> +			 * and arch-PEBS, both of which snapshot the LBR stack.
>> +			 * No explicit LBR refresh is needed here.
>> +			 */
>>  			static_call(x86_pmu_drain_pebs)(regs, &data);
>> +			/*
>> +			 * Draining the PEBS buffer can overwrite the shared
>> +			 * LBR stack, so mark lbr_refreshed as false.
>> +			 */
>> +			lbr_refreshed = false;
>> +		}
>>  
>>  		last_period = event->hw.last_period;
>>  
>> @@ -4013,8 +4034,14 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
>>  
>>  		perf_sample_data_init(&data, 0, last_period);
>>  
>> -		if (has_branch_stack(event))
>> +		if (has_branch_stack(event)) {
>> +			if (!lbr_refreshed) {
>> +				intel_pmu_lbr_read(cpu_lbr_stack(cpuc),
>> +						   cpuc->lbr_counters);
>> +				lbr_refreshed = true;
>> +			}
>>  			intel_pmu_lbr_save_brstack(&data, cpuc, event);
>> +		}
>>  
>>  		x86_pmu_clear_perf_regs(regs);
>>  		x86_pmu_update_perf_regs(event, &data, regs, false);
> [Severity: Medium]
> Can perf_event_overflow() still drain the PEBS buffer and clobber
> cpu_lbr_stack(cpuc) when is_pebs_counter_event_group(event) is false?
>
> When a non-PEBS event using PERF_SAMPLE_READ | PERF_FORMAT_GROUP is grouped
> with a PERF_X86_EVENT_AUTO_RELOAD PEBS LBR sibling (without
> PERF_X86_EVENT_PEBS_CNTR set on the leader), perf_event_overflow() ->
> perf_output_sample() -> perf_output_read_group() -> intel_pmu_read_event()
> calls intel_pmu_drain_pebs_buffer() before PERF_SAMPLE_BRANCH_STACK is
> written.  Draining pending PEBS records calls intel_pmu_store_pebs_lbrs(),
> which overwrites cpu_lbr_stack(cpuc) before the current non-PEBS sample
> copies data.br_stack and leaves lbr_refreshed set to true for any subsequent
> non-PEBS LBR events in the same PMI.
>
> In addition, because the unconditional intel_pmu_lbr_read() at PMI entry is
> removed, an indirect PEBS drain on pre-adaptive PEBS
> (!x86_pmu.intel_cap.pebs_baseline) when GLOBAL_STATUS_BUFFER_OVF_BIT is clear
> will run intel_pmu_pebs_fixup_ip() against an unrefreshed
> cpu_lbr_stack(cpuc).

Sigh, Sashiko can always give surprise. :)

It's a real issue. It looks a new version has to be posted to fix this issue.


>