Re: [Patch v9 06/24] perf/x86: Use x86_perf_regs in NMI handlers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent use of `x86_pebs_regs` and `x86_user_regs` per-CPU variables between NMI and IRQ-disabled contexts corrupts PEBS samples.
--

commit a90fb4d272f2182569ad5a2105f6b3f7c429263d
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

perf/x86: Use x86_perf_regs in NMI handlers

Support for sampling additional register state in NMI context requires an
x86-specific register container. This switches x86 NMI handlers to use
the x86_perf_regs structure.

> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 0888d3b0923e1..d83ea02e24577 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -1788,9 +1788,11 @@ void perf_put_guest_lvtpc(void)
>  EXPORT_SYMBOL_FOR_KVM(perf_put_guest_lvtpc);
>  #endif /* CONFIG_PERF_GUEST_MEDIATED_PMU */
>  
> +static DEFINE_PER_CPU(struct x86_perf_regs, x86_intr_regs);
>  static int
>  perf_event_nmi_handler(unsigned int cmd, struct pt_regs *regs)
>  {
> +	struct x86_perf_regs *x86_regs = this_cpu_ptr(&x86_intr_regs);
>  	u64 start_clock;
>  	u64 finish_clock;
>  	int ret;
> @@ -1814,7 +1816,8 @@ perf_event_nmi_handler(unsigned int cmd, struct pt_regs *regs)
>  		return NMI_DONE;
>  
>  	start_clock = sched_clock();
> -	ret = static_call(x86_pmu_handle_irq)(regs);
> +	x86_regs->regs = *regs;
> +	ret = static_call(x86_pmu_handle_irq)(&x86_regs->regs);

[Severity: High]
Is this call path safe from NMI reentrancy when accessing the per-CPU
variables introduced earlier in this series?

If intel_pmu_drain_pebs_buffer() is called from a non-NMI context (e.g.
intel_pmu_read_event() via the perf_event_read syscall) with local IRQs
disabled, NMIs can still fire.

If a hardware PMI fires during the execution of __intel_pmu_pebs_events(),
the CPU will jump here to perf_event_nmi_handler().

The NMI handler calls x86_pmu_handle_irq() -> handle_pmi_common(), which
also calls x86_pmu_drain_pebs(). Since the earlier patch in this series
"perf/x86/intel: Convert x86_perf_regs to per-cpu variables" replaced stack
allocations with shared per-CPU variables:

arch/x86/events/intel/ds.c:__intel_pmu_pebs_events() {
    ...
	struct x86_perf_regs *perf_regs = this_cpu_ptr(&x86_pebs_regs);
	struct pt_regs *regs = &perf_regs->regs;
    ...
}

And similarly for user regs:

arch/x86/events/core.c:x86_pmu_get_regs_user() {
    ...
	struct x86_perf_regs *x86_regs_user = this_cpu_ptr(&x86_user_regs);
    ...
}

Will the NMI handler overwrite the same per-CPU structures the interrupted
non-NMI context was actively using, leading to corrupted PEBS samples when
the original context resumes?

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