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

[sashiko-bot@kernel.org]

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).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106043257.842864-1-dapeng1.mi@linux.intel.com?part=15