Re: [Patch v5 13/18] perf/x86/intel: Refresh LBR stack for non-PEBS LBR events if needed

[Peter Zijlstra]

On Thu, Oct 29, 2026 at 12:39:09PM +0800, Dapeng Mi wrote:
> PEBS and non-PEBS events currently share the same LBR stack,
> cpuc->lbr_stack. In the PMI handler, the non-PEBS path refreshes the
> LBR state before the PEBS buffer is drained.
> 
> When PEBS and non-PEBS LBR events overflow in the same PMI, draining the
> PEBS buffer can overwrite the shared LBR stack with the PEBS snapshot.
> This leaves stale LBR data from the PEBS record in place, and userspace
> may observe the wrong branch stack instead of the live LBR state at PMI
> entry.
> 
> Add a lbr_need_update flag to indicate when the LBR state must be
> refreshed. If PEBS events and non-PEBS LBR events overflow in the same
> PMI, set this flag so the non-PEBS LBR event refreshes the live LBR
> state. Since the LBR state is frozen for the duration of PMI handling, it
> only needs to be read once even when multiple non-PEBS LBR events
> overflow in the same PMI.
> 
> Fixes: c22497f5838c ("perf/x86/intel: Support adaptive PEBS v4")
> Signed-off-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
> ---
>  arch/x86/events/intel/core.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 110fff65bd50..37f304a1b48c 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -3863,8 +3863,9 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
>  {
>  	struct perf_sample_data data;
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> -	int bit;
> +	bool lbr_need_update = false;
>  	int handled = 0;
> +	int bit;
>  
>  	inc_perf_irq_stat();
>  
> @@ -3929,6 +3930,8 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
>  		if (cpuc->events[INTEL_PMC_IDX_FIXED_SLOTS] &&
>  		    is_pebs_counter_event_group(cpuc->events[INTEL_PMC_IDX_FIXED_SLOTS]))
>  			status &= ~GLOBAL_STATUS_PERF_METRICS_OVF_BIT;
> +
> +		lbr_need_update = true;
>  	}
>  
>  	/*
> @@ -3944,6 +3947,8 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
>  		if (cpuc->events[INTEL_PMC_IDX_FIXED_SLOTS] &&
>  		    is_pebs_counter_event_group(cpuc->events[INTEL_PMC_IDX_FIXED_SLOTS]))
>  			status &= ~GLOBAL_STATUS_PERF_METRICS_OVF_BIT;
> +
> +		lbr_need_update = true;
>  	}
>  
>  	/*
> @@ -4002,8 +4007,14 @@ 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)) {
>  			static_call(x86_pmu_drain_pebs)(regs, &data);
> +			/*
> +			 * Draining the PEBS buffer can overwrite the shared
> +			 * LBR stack, so mark lbr_need_update as true.
> +			 */
> +			lbr_need_update = true;
> +		}
>  
>  		last_period = event->hw.last_period;
>  
> @@ -4012,8 +4023,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_need_update) {
> +				intel_pmu_lbr_read(&cpuc->lbr_stack,
> +					cpuc->lbr_entries, cpuc->lbr_counters);
> +				lbr_need_update = false;
> +			}
>  			intel_pmu_lbr_save_brstack(&data, cpuc, event);
> +		}
>  
>  		x86_pmu_clear_perf_regs(regs);
>  		x86_pmu_update_perf_regs(event, &data, regs, false);

Or... you move intel_pmu_lbr_read() from handle_irq() into pmi_common()
after we done the PEBS things?