Linux Perf Users
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Guanghui Feng" <guanghuifeng@linux.alibaba.com>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH] perf/x86/amd: Move NMI latency window to last-resort suppression
Date: Wed, 16 Sep 2026 07:10:06 +0000	[thread overview]
Message-ID: <20260916071006.9DC891F0089B@smtp.kernel.org> (raw)
In-Reply-To: <20260916065723.1976131-1-guanghuifeng@linux.alibaba.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] x86/nmi: silent unknown NMI suppression on non-AMD 32-bit CPUs
--

commit 6ca0dec4b1ba3bd7646b0d6ea1bfb276ba5b9732
Author: Guanghui Feng <guanghuifeng@linux.alibaba.com>
Subject: perf/x86/amd: Move NMI latency window to last-resort suppression

This patch defers the NMI latency window check to the very end of the NMI
dispatch path in unknown_nmi_error(). It stops claiming NMIs from within
the perf handler when no counter overflowed, and instead introduces a new
perf_nmi_window_active() check.

> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> index a787409f5a625..e9fca9a038c2e 100644
> --- a/arch/x86/events/amd/core.c
> +++ b/arch/x86/events/amd/core.c

[ ... ]

> +/**
> + * perf_nmi_window_active - Check if the PMC NMI latency window is still open
> + *
> + * Returns true if the current time is within the NMI latency mitigation window
> + * that was opened by the last PMC counter overflow handled on this CPU. An
> + * open window means that a recently processed overflow may have generated an
> + * NMI which arrived too late to be paired with that overflow, and which
> + * therefore cannot be identified by any NMI handler.
> + *
> + * This is the last resort of the x86 NMI dispatch path: it is only called
> + * after all NMI_LOCAL handlers, the NMI reason port and all NMI_UNKNOWN
> + * handlers failed to claim the NMI. It must never be used to claim an NMI on
> + * behalf of perf, otherwise unrelated NMI sources would be dropped.
> + *
> + * Context: NMI context. Must not sleep and must not take locks.
> + */
> +bool perf_nmi_window_active(void)
> +{
> +	return !time_after(jiffies, this_cpu_read(perf_nmi_tstamp));
>  }

[Severity: High]
Does this strong symbol unconditionally override the weak fallback in nmi.c
for all x86 vendors when CONFIG_CPU_SUP_AMD=y is set?

On non-AMD CPUs (like Intel), the AMD PMU driver never initializes,
leaving the per-CPU perf_nmi_tstamp permanently at 0.

On 32-bit x86 systems, jiffies starts at INITIAL_JIFFIES (5 minutes before
32-bit wrap). Does the check !time_after(jiffies, 0) incorrectly evaluate
to true for the first 5 minutes of uptime, and for 50% of the entire
32-bit wrap cycle (24.8 days out of every 49.7 days)?

> +EXPORT_SYMBOL_GPL(perf_nmi_window_active);
>  
>  static int amd_pmu_handle_irq(struct pt_regs *regs)
>  {

[ ... ]

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

[ ... ]

> @@ -340,6 +350,19 @@ unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
>  
>  	__this_cpu_add(nmi_stats.unknown, 1);
>  
> +	/*
> +	 * No handler was able to identify this NMI, so its source is unknown.
> +	 * The one exception is a latent PMC overflow NMI: the overflow NMI can
> +	 * arrive long after the counter was already processed by an earlier
> +	 * NMI, which leaves nothing here that could identify it. If the perf
> +	 * NMI latency window is still open, this NMI is very likely that late
> +	 * arrival, so keep quiet about it instead of reporting a bogus unknown
> +	 * NMI (and instead of panicking on unknown_nmi_panic). It is still
> +	 * accounted in nmi_stats.unknown and thus stays visible in debugfs.
> +	 */
> +	if (perf_nmi_window_active())
> +		return;
> +

[Severity: High]
Because of the jiffies initialization on 32-bit systems mentioned above,
could this incorrectly drop unknown NMIs on non-AMD platforms, or on AMD
platforms before the first PMC counter overflow initializes the timestamp?

If the latency window is falsely considered active, could this cause
hardware faults causing unknown NMIs (e.g., SERR/IOCHK) to be silently
suppressed without warning?

>  	pr_emerg_ratelimited("Uhhuh. NMI received for unknown reason %02x on CPU %d.\n",
>  			     reason, smp_processor_id());
>

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

  reply	other threads:[~2026-09-16  7:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-16  6:57 [PATCH] perf/x86/amd: Move NMI latency window to last-resort suppression Guanghui Feng
2026-09-16  7:10 ` sashiko-bot [this message]
2026-09-16  9:32 ` Peter Zijlstra
2026-09-16 12:50   ` [PATCH v2] " Guanghui Feng
2026-09-16 12:59     ` sashiko-bot
2026-09-16 13:04     ` Peter Zijlstra
2026-09-16 13:18   ` [PATCH] " guanghuifeng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260916071006.9DC891F0089B@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=guanghuifeng@linux.alibaba.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox