All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Dapeng Mi" <dapeng1.mi@linux.intel.com>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [Patch v4 3/5] perf/x86/intel: Disable PMI for self-reloaded ACR events
Date: Thu, 30 Apr 2026 02:36:35 +0000	[thread overview]
Message-ID: <20260430023637.37B6AC19425@smtp.kernel.org> (raw)
In-Reply-To: <20260430002558.712334-4-dapeng1.mi@linux.intel.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Event PMIs can be permanently disabled due to a stale `hw.config1` state when an ACR group is broken up.
--

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

perf/x86/intel: Disable PMI for self-reloaded ACR events

This commit disables Performance Monitoring Interrupts (PMI) for all events
configured with Auto Counter Reload (ACR) self-reload to prevent spurious
"unknown reason" NMI warnings.

[ ... ]

> diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
> index fad87d3c8b2c..524668dcf4cc 100644
> --- a/arch/x86/events/perf_event.h
> +++ b/arch/x86/events/perf_event.h
> @@ -137,6 +137,16 @@ static inline bool is_acr_event_group(struct perf_event *event)
>  	return check_leader_group(event->group_leader, PERF_X86_EVENT_ACR);
>  }
>  
> +static inline bool is_acr_self_reload_event(struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	if (hwc->idx < 0)
> +		return false;
> +
> +	return test_bit(hwc->idx, (unsigned long *)&hwc->config1);
> +}

Can this cause PMIs to be permanently disabled if an ACR group is broken up?

If the original group leader is closed, perf_group_detach() promotes the
siblings to standalone events, making them their own leaders. These new
standalone events no longer have the PERF_X86_EVENT_ACR flag (as it was only
set on the original leader).

Because they lack the flag, intel_pmu_acr_late_setup() skips them, and
their hw.config1 field retains the stale physical counter bitmask from when
they were in the ACR group.

Since is_acr_self_reload_event() checks hw.config1 without validating if the
event is still in an ACR group via is_acr_event_group(), could a detached
event that gets assigned to a counter matching the stale bitmask incorrectly
return true here?

This would cause intel_pmu_enable_event() to incorrectly clear the
ARCH_PERFMON_EVENTSEL_INT bit, breaking PMI generation and sampling for a
now-normal event.

>  struct amd_nb {
>  	int nb_id;  /* NorthBridge id */
>  	int refcnt; /* reference count */

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

  reply	other threads:[~2026-04-30  2:36 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-30  0:25 [Patch v4 0/5] perf/x86/intel: Fix bugs of auto counter reload sampling Dapeng Mi
2026-04-30  0:25 ` [Patch v4 1/5] perf/x86/intel: Improve validation and configuration of ACR masks Dapeng Mi
2026-05-05 10:50   ` [tip: perf/core] " tip-bot2 for Dapeng Mi
2026-04-30  0:25 ` [Patch v4 2/5] perf/x86/intel: Always reprogram ACR events to prevent stale masks Dapeng Mi
2026-04-30  2:04   ` sashiko-bot
2026-04-30  3:02     ` Mi, Dapeng
2026-05-05 10:50   ` [tip: perf/core] " tip-bot2 for Dapeng Mi
2026-04-30  0:25 ` [Patch v4 3/5] perf/x86/intel: Disable PMI for self-reloaded ACR events Dapeng Mi
2026-04-30  2:36   ` sashiko-bot [this message]
2026-04-30  6:33     ` Mi, Dapeng
2026-05-05 10:49   ` [tip: perf/core] " tip-bot2 for Dapeng Mi
2026-04-30  0:25 ` [Patch v4 4/5] perf/x86/intel: Enable auto counter reload for DMR Dapeng Mi
2026-05-05 10:49   ` [tip: perf/core] " tip-bot2 for Dapeng Mi
2026-04-30  0:25 ` [Patch v4 5/5] perf/x86/intel: Consolidate MSR_IA32_PERF_CFG_C tracking Dapeng Mi
2026-05-05 10:49   ` [tip: perf/core] " tip-bot2 for Dapeng Mi

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=20260430023637.37B6AC19425@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dapeng1.mi@linux.intel.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.