Re: [Patch v4 5/5] pef/x86/intel: Always snapshot live hardware LBR state for BPF path

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

Sashiko found several new long-standing LBR issues (not caused by the patch
series), https://lore.kernel.org/all/sashiko-outbox-179823@kernel.org/

Would post new patch series to fix the newly found issues. Thanks.

On 10/21/2026 1:07 PM, Dapeng Mi wrote:
> __intel_pmu_snapshot_branch_stack() is meant to snapshot the current
> runtime LBR state from hardware for the BPF path. However, stale cached
> PEBS LBR data can be snapshotted instead when a PEBS LBR user is active.
>
> Fix this by always snapshotting the live hardware LBR state, regardless
> of whether a PEBS LBR user is active. Reading the runtime LBR registers
> can overwrite the cached PEBS LBR stack, so save and restore the cached
> PEBS LBR state around the snapshot. This preserves the PEBS LBR data for
> later output to userspace in the normal PEBS record path.
>
> Fixes: c22ac2a3d4bd ("perf: Enable branch record for software events")
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> ---
>  arch/x86/events/intel/core.c | 43 ++++++++++++++++++++++++++++++++++--
>  arch/x86/events/intel/lbr.c  |  9 --------
>  2 files changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 5dfb9e9156e8..07d1a8a7845b 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -2815,16 +2815,50 @@ static void intel_pmu_enable_all(int added)
>  	__intel_pmu_enable_all(added, false);
>  }
>  
> +struct lbr_snapshot {
> +	struct perf_branch_stack stack;
> +	struct perf_branch_entry entries[MAX_LBR_ENTRIES];
> +	u64 counters[MAX_LBR_ENTRIES];
> +};
> +static DEFINE_PER_CPU(struct lbr_snapshot, lbr_snapshot);
> +
>  static noinline int
>  __intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries,
>  				  unsigned int cnt, unsigned long flags)
>  {
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> +	struct lbr_snapshot *snapshot = this_cpu_ptr(&lbr_snapshot);
> +
> +	/*
> +	 * This may be called from bpf_event_handler() in a PMI context. If
> +	 * a PEBS LBR user is active, the PEBS sample record has already updated
> +	 * cpuc->lbr_stack. Reading the LBR registers here would overwrite that
> +	 * cached PEBS LBR stack, so save it first and restore it afterward. This
> +	 * keeps the cached LBR data available for later output to userspace in
> +	 * the normal PEBS record path.
> +	 */
> +	if (cpuc->lbr_pebs_users) {
> +		memcpy(&snapshot->stack, &cpuc->lbr_stack,
> +		       sizeof(struct perf_branch_stack));
> +		memcpy(snapshot->entries, cpuc->lbr_entries,
> +		       sizeof(struct perf_branch_entry) * cpuc->lbr_stack.nr);
> +		memcpy(snapshot->counters, cpuc->lbr_counters,
> +		       sizeof(u64) * cpuc->lbr_stack.nr);
> +	}
>  
>  	intel_pmu_lbr_read();
>  	cnt = min_t(unsigned int, cnt, cpuc->lbr_stack.nr);
> -
>  	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
> +
> +	if (cpuc->lbr_pebs_users) {
> +		memcpy(&cpuc->lbr_stack, &snapshot->stack,
> +		       sizeof(struct perf_branch_stack));
> +		memcpy(cpuc->lbr_entries, snapshot->entries,
> +		       sizeof(struct perf_branch_entry) * cpuc->lbr_stack.nr);
> +		memcpy(cpuc->lbr_counters, snapshot->counters,
> +		       sizeof(u64) * cpuc->lbr_stack.nr);
> +	}
> +
>  	intel_pmu_enable_all(0);
>  	local_irq_restore(flags);
>  	return cnt;
> @@ -4022,7 +4056,12 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
>  
>  	loops = 0;
>  again:
> -	intel_pmu_lbr_read();
> +	/*
> +	 * No need to read the LBR stack when all LBR users are PEBS events,
> +	 * since the PEBS handler updates it.
> +	 */
> +	if (cpuc->lbr_users != cpuc->lbr_pebs_users)
> +		intel_pmu_lbr_read();
>  	intel_pmu_ack_status(status);
>  	if (++loops > 100) {
>  		static bool warned;
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index ac1759a71288..07b32255e48d 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -1030,15 +1030,6 @@ void intel_pmu_lbr_read(void)
>  		return;
>  	}
>  
> -	/*
> -	 * Don't read when all LBRs users are using adaptive PEBS.
> -	 *
> -	 * This could be smarter and actually check the event,
> -	 * but this simple approach seems to work for now.
> -	 */
> -	if (cpuc->lbr_users == cpuc->lbr_pebs_users)
> -		return;
> -
>  	x86_pmu.lbr_read(cpuc);
>  
>  	intel_pmu_lbr_filter(cpuc);