Re: [PATCH v2] perf/x86/amd: Move NMI latency window to last-resort suppression

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] x86/nmi: TOCTOU race in perf_nmi_window_active() causes spurious panic
--

commit 48f851c34890a3a07c65994472936573ea009e9e
Author: Guanghui Feng <guanghuifeng@linux.alibaba.com>

perf/x86/amd: Move NMI latency window to last-resort suppression

This commit delays claiming NMIs in the perf handler when no counter
overflowed, allowing other NMI handlers to run first. It updates the NMI
dispatch path to consult the latency window twice: once to skip the
reason port read, and a second time as a last resort to suppress
bogus unknown NMI reports.

> diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
> index 3c9f60d6ca5a7..1226c18e37a61 100644
> --- a/arch/x86/kernel/nmi.c
> +++ b/arch/x86/kernel/nmi.c

[ ... ]

> @@ -407,38 +430,50 @@ static noinstr void default_do_nmi(struct pt_regs *regs)
>  	}
>  
>  	/*
> -	 * Non-CPU-specific NMI: NMI sources can be processed on any CPU.
> +	 * If the perf NMI latency window is still open, a recently handled PMC
> +	 * overflow may have generated a latent NMI that arrives too late to be
> +	 * paired with that overflow. Skip the expensive reason port read
> +	 * (inb 0x61 + global nmi_reason_lock) which is a known scalability
> +	 * bottleneck when spurious PMIs arrive at high frequency on AMD.
>  	 *
> -	 * Another CPU may be processing panic routines while holding
> -	 * nmi_reason_lock. Check if the CPU issued the IPI for crash dumping,
> -	 * and if so, call its callback directly.  If there is no CPU preparing
> -	 * crash dump, we simply loop here.
> +	 * NMI_UNKNOWN handlers (hpwdt, etc.) are still invoked below via
> +	 * unknown_nmi_error(), so hardware watchdog NMIs are not lost.
>  	 */
> -	while (!raw_spin_trylock(&nmi_reason_lock)) {
> -		run_crash_ipi_callback(regs);
> -		cpu_relax();
> -	}
> +	if (!perf_nmi_window_active()) {

[Severity: High]
Could this introduce a race condition with jiffies updates on other CPUs?

Looking at perf_nmi_window_active(), it evaluates the window state against
the globally incrementing jiffies variable:

arch/x86/events/amd/core.c:perf_nmi_window_active() {
    ...
    return !time_after(jiffies, this_cpu_read(perf_nmi_tstamp));
}

If perf_nmi_window_active() returns true here in default_do_nmi(), the
NMI reason port read is skipped and execution proceeds down to
unknown_nmi_error().

If a timer interrupt on another CPU increments jiffies between this first
check and the second check inside unknown_nmi_error():

arch/x86/kernel/nmi.c:unknown_nmi_error() {
    ...
    if (perf_nmi_window_active())
        return;
    ...
}

Would this cause the second check to evaluate to false, resulting in a
fall-through that logs a bogus "unknown NMI" error and potentially triggers
a spurious kernel panic if unknown_nmi_panic is set?

> +		/*
> +		 * Non-CPU-specific NMI: NMI sources can be processed on any CPU.
> +		 *
> +		 * Another CPU may be processing panic routines while holding
> +		 * nmi_reason_lock. Check if the CPU issued the IPI for crash
> +		 * dumping, and if so, call its callback directly.  If there is no
> +		 * CPU preparing crash dump, we simply loop here.
> +		 */

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916125016.2266596-1-guanghuifeng@linux.alibaba.com?part=1