[PATCH 0/3] perf/amd/ibs: Assorted fixes

[PATCH 0/3] perf/amd/ibs: Assorted fixes

[Ravi Bangoria]

#1 Set IBS MSRs in syscore->resume() callback
#2 Fix kernel address leaking through REGS_USER
#3 Fix races by reducing SW states when IBS_CAPS_DIS is present

Ravi Bangoria (3):
  perf/amd/ibs: Reset CPU0 IBS state on syscore resume
  perf/amd/ibs: Fix kernel address leak via REGS_USER
  perf/amd/ibs: Simplify event state handling with IBS_CAPS_DIS

 arch/x86/events/amd/ibs.c | 156 +++++++++++++++++++++++++++++++-------
 1 file changed, 129 insertions(+), 27 deletions(-)

-- 
2.55.0

[PATCH 1/3] perf/amd/ibs: Reset CPU0 IBS state on syscore resume

[Ravi Bangoria]

During a suspend/resume cycle, all CPUs except CPU0 go through hotplug
callbacks, which sets up IBS state. CPU0 bypasses this path, leaving its
IBS state stale after resume. Fix this by explicitly setting CPU0's IBS
state in the syscore ->resume() callback.

Fixes: efa5700ec0da ("perf/amd/ibs: Support IBS_{FETCH|OP}_CTL2[Dis] to eliminate RMW race")
Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 arch/x86/events/amd/ibs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index 3531f9c23b8c..bfb51f8e184f 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -1948,7 +1948,7 @@ static int perf_ibs_suspend(void *data)
 static void perf_ibs_resume(void *data)
 {
 	ibs_eilvt_setup();
-	setup_APIC_ibs();
+	x86_pmu_amd_ibs_starting_cpu(smp_processor_id());
 }
 
 static const struct syscore_ops perf_ibs_syscore_ops = {
-- 
2.55.0

[PATCH 2/3] perf/amd/ibs: Fix kernel address leak via REGS_USER

[Ravi Bangoria]

IBS hardware captures a sample and raises an NMI. Since the NMI can be
delivered with a delay, a privilege change may occur between sample
capture and NMI delivery. Because IBS only provides the sample RIP, the
kernel records GPRs from the NMI pt_regs, which may reflect kernel mode
if a privilege switch occurred in the interim. This can leak kernel
addresses to unprivileged users through PERF_SAMPLE_REGS_USER.

Fix this by discarding samples where the NMI arrives in kernel mode and
the user has set exclude_kernel.

Reported-by: Jayapal Ashwin Nair <janair@amd.com>
Fixes: d29e744c7167 ("perf/x86: Relax privilege filter restriction on AMD IBS")
Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 arch/x86/events/amd/ibs.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index bfb51f8e184f..3cb46c775eaa 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -1373,12 +1373,15 @@ static bool perf_ibs_is_kernel_br_target(struct perf_event *event,
 }
 
 static bool perf_ibs_discard_sample(struct perf_ibs *perf_ibs, struct perf_event *event,
-				    struct pt_regs *regs, struct perf_ibs_data *ibs_data,
-				    int br_target_idx)
+				    struct pt_regs *regs, struct pt_regs *iregs,
+				    struct perf_ibs_data *ibs_data, int br_target_idx)
 {
 	if (perf_exclude_event(event, regs))
 		return true;
 
+	if (perf_exclude_event(event, iregs))
+		return true;
+
 	if (perf_ibs != &perf_ibs_op || !event->attr.exclude_kernel)
 		return false;
 
@@ -1529,7 +1532,7 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
 
 	if (((ibs_caps & IBS_CAPS_BIT63_FILTER) ||
 	     (event->attr.config2 & IBS_SW_FILTER_MASK)) &&
-	    perf_ibs_discard_sample(perf_ibs, event, &regs, &ibs_data, br_target_idx)) {
+	    perf_ibs_discard_sample(perf_ibs, event, &regs, iregs, &ibs_data, br_target_idx)) {
 		throttle = perf_event_account_interrupt(event);
 		goto out;
 	}
-- 
2.55.0

[PATCH 3/3] perf/amd/ibs: Simplify event state handling with IBS_CAPS_DIS

[Ravi Bangoria]

The IBS_{FETCH|OP}_CTL2[Dis] bit allows IBS sampling to be stopped without
a RMW of IBS_{FETCH,OP}_CTL. Systems providing this bit therefore do not
require the IBS_STARTED and IBS_STOPPED states used to coordinate control
MSR updates with late NMIs.

Add a separate stop path for these systems. Preserve Val while disabling
the event so that a delayed NMI can be identified and consumed, and wait
briefly for such an NMI before restarting the event.

Leave the existing handling unchanged on systems without IBS_CAPS_DIS.

Reported-by: Jim Mattson <jmattson@google.com>
Closes: https://lore.kernel.org/r/20261007234815.4026028-1-jmattson@google.com
Fixes: efa5700ec0da ("perf/amd/ibs: Support IBS_{FETCH|OP}_CTL2[Dis] to eliminate RMW race")
Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 arch/x86/events/amd/ibs.c | 145 ++++++++++++++++++++++++++++++++------
 1 file changed, 122 insertions(+), 23 deletions(-)

diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index 3cb46c775eaa..f49f30f53399 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -28,6 +28,7 @@ static u32 ibs_caps;
 #include <linux/hardirq.h>
 
 #include <asm/nmi.h>
+#include <asm/delay.h>
 #include <asm/amd/ibs.h>
 
 /* attr.config2 */
@@ -73,7 +74,12 @@ static u32 ibs_caps;
  *
  * XXX: we could probably be using !atomic bitops for all this.
  */
-
+/*
+ * NOTE: IBS_CAPS_DIS eliminates the RMW race on the IBS_{FETCH|OP}_CTL, making
+ * IBS_STARTED and IBS_STOPPED states unnecessary. We use only IBS_ENABLED and
+ * IBS_STOPPING states when IBS_CAPS_DIS is present. Semantics of IBS_STOPPING
+ * changes a bit as well.
+ */
 enum ibs_states {
 	IBS_ENABLED	= 0,
 	IBS_STARTED	= 1,
@@ -557,7 +563,9 @@ static void perf_ibs_start(struct perf_event *event, int flags)
 	struct hw_perf_event *hwc = &event->hw;
 	struct perf_ibs *perf_ibs = container_of(event->pmu, struct perf_ibs, pmu);
 	struct cpu_perf_ibs *pcpu = this_cpu_ptr(perf_ibs->pcpu);
+	unsigned int delay = 50;
 	u64 period, config = 0;
+	u64 ctl;
 
 	if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
 		return;
@@ -575,23 +583,87 @@ static void perf_ibs_start(struct perf_event *event, int flags)
 	}
 	config |= period >> 4;
 
-	/*
-	 * Reset the IBS_{FETCH|OP}_CTL MSR before updating pcpu->state.
-	 * Doing so prevents a race condition in which an NMI due to other
-	 * source might accidentally activate the event before we enable
-	 * it ourselves.
-	 */
-	perf_ibs_disable_event(perf_ibs, hwc, 0);
+	if (ibs_caps & IBS_CAPS_DIS) {
+		/*
+		 *      IBS HW                     SW
+		 *
+		 *  o Capture sample       o ->stop()
+		 *  o Set VAL bit            CTL2[Dis] = 1
+		 *  o Raise NMI --.        o ...
+		 *                 \       o ->start()
+		 *                  \        wrmsr(IBS_{FETCH|OP}_CTL, new value);
+		 *                   \       (wrmsr clears the VAL bit)
+		 *                    `--> o Receives delayed NMI
+		 *                           (Unknown NMI since VAL bit is cleared)
+		 *
+		 * Handle this with induced delay.
+		 */
+		rdmsrq(hwc->config_base, ctl);
+		while ((ctl & perf_ibs->valid_mask) && delay--) {
+			udelay(1);
+			rdmsrq(hwc->config_base, ctl);
+		}
+	} else {
+		/*
+		 * Reset the IBS_{FETCH|OP}_CTL MSR before updating pcpu->state.
+		 * Doing so prevents a race condition in which an NMI due to
+		 * other source might accidentally activate the event before we
+		 * enable it ourselves.
+		 */
+		perf_ibs_disable_event(perf_ibs, hwc, 0);
+		/*
+		 * Set STARTED before enabling the hardware, such that a
+		 * subsequent NMI must observe it.
+		 */
+		set_bit(IBS_STARTED,    pcpu->state);
+		clear_bit(IBS_STOPPING, pcpu->state);
+	}
+	perf_ibs_enable_event(perf_ibs, hwc, config);
+
+	perf_event_update_userpage(event);
+}
+
+/* ->stop() for platforms with additional CTL2[Dis] bit. */
+static void perf_ibs_stop_dis(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct perf_ibs *perf_ibs = container_of(event->pmu, struct perf_ibs, pmu);
+	struct cpu_perf_ibs *pcpu = this_cpu_ptr(perf_ibs->pcpu);
+	u64 config;
+	u64 ctl2;
+
+	WARN_ON_ONCE(test_bit(IBS_STOPPING, pcpu->state));
+
+	set_bit(IBS_STOPPING, pcpu->state);
+
+	rdmsrq(hwc->extra_reg.reg, ctl2);
+	if (ctl2 & perf_ibs->disable_mask)
+		goto reset_state;
+
+	wrmsrq(hwc->extra_reg.reg, perf_ibs->disable_mask);
+
+	WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
+	hwc->state |= PERF_HES_STOPPED;
+
+	if (hwc->state & PERF_HES_UPTODATE)
+		goto reset_state;
+
+	rdmsrq(hwc->config_base, config);
+
+	config &= ~perf_ibs->valid_mask;
 
 	/*
-	 * Set STARTED before enabling the hardware, such that a subsequent NMI
-	 * must observe it.
+	 * Clear the Val bit otherwise ->start() will unnecessarily
+	 * wait for 50us.
 	 */
-	set_bit(IBS_STARTED,    pcpu->state);
-	clear_bit(IBS_STOPPING, pcpu->state);
-	perf_ibs_enable_event(perf_ibs, hwc, config);
+	if (in_nmi())
+		wrmsrq(hwc->config_base, config);
 
-	perf_event_update_userpage(event);
+	perf_ibs_event_update(perf_ibs, event, &config);
+	hwc->state |= PERF_HES_UPTODATE;
+
+reset_state:
+	clear_bit(IBS_STOPPING, pcpu->state);
 }
 
 static void perf_ibs_stop(struct perf_event *event, int flags)
@@ -602,6 +674,9 @@ static void perf_ibs_stop(struct perf_event *event, int flags)
 	u64 config;
 	int stopping;
 
+	if (ibs_caps & IBS_CAPS_DIS)
+		return perf_ibs_stop_dis(event, flags);
+
 	if (test_and_set_bit(IBS_STOPPING, pcpu->state))
 		return;
 
@@ -1419,19 +1494,43 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
 	unsigned int msr;
 	u64 *buf, *config, period, new_config = 0;
 	int br_target_idx = -1;
+	u64 ctl2, ctl;
 
-	if (!test_bit(IBS_STARTED, pcpu->state)) {
-fail:
+	if (ibs_caps & IBS_CAPS_DIS) {
 		/*
-		 * Catch spurious interrupts after stopping IBS: After
-		 * disabling IBS there could be still incoming NMIs
-		 * with samples that even have the valid bit cleared.
-		 * Mark all this NMIs as handled.
+		 * ->stop() {
+		 *     set_bit(IBS_STOPPING);
+		 *                               <-- NMI (race 1)
+		 *     CTL2[Dis]=1;
+		 *     ...                       <-- NMI (race 2)
+		 * }
 		 */
-		if (test_and_clear_bit(IBS_STOPPED, pcpu->state))
-			return 1;
+		rdmsrq(perf_ibs->msr2, ctl2);
+		if (test_bit(IBS_STOPPING, pcpu->state) /* race 1 */ ||
+		    (ctl2 & perf_ibs->disable_mask) /* race 2 */) {
+			rdmsrq(perf_ibs->msr, ctl);
+			if (ctl & perf_ibs->valid_mask) {
+				ctl &= ~(perf_ibs->enable_mask |
+					 perf_ibs->valid_mask);
+				wrmsrq(perf_ibs->msr, ctl);
+				return 1;
+			}
+			return 0;
+		}
+	} else {
+		if (!test_bit(IBS_STARTED, pcpu->state)) {
+fail:
+			/*
+			 * Catch spurious interrupts after stopping IBS: After
+			 * disabling IBS there could be still incoming NMIs
+			 * with samples that even have the valid bit cleared.
+			 * Mark all this NMIs as handled.
+			 */
+			if (test_and_clear_bit(IBS_STOPPED, pcpu->state))
+				return 1;
 
-		return 0;
+			return 0;
+		}
 	}
 
 	if (WARN_ON_ONCE(!event))
-- 
2.55.0

Re: [PATCH 1/3] perf/amd/ibs: Reset CPU0 IBS state on syscore resume

[Jim Mattson]

On Wed, Oct 14, 2026 at 11:04 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>
> During a suspend/resume cycle, all CPUs except CPU0 go through hotplug
> callbacks, which sets up IBS state. CPU0 bypasses this path, leaving its
> IBS state stale after resume. Fix this by explicitly setting CPU0's IBS
> state in the syscore ->resume() callback.
>
> Fixes: efa5700ec0da ("perf/amd/ibs: Support IBS_{FETCH|OP}_CTL2[Dis] to eliminate RMW race")
> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

Re: [PATCH 2/3] perf/amd/ibs: Fix kernel address leak via REGS_USER

[Jim Mattson]

On Wed, Oct 14, 2026 at 11:05 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>
> IBS hardware captures a sample and raises an NMI. Since the NMI can be
> delivered with a delay, a privilege change may occur between sample
> capture and NMI delivery. Because IBS only provides the sample RIP, the
> kernel records GPRs from the NMI pt_regs, which may reflect kernel mode
> if a privilege switch occurred in the interim. This can leak kernel
> addresses to unprivileged users through PERF_SAMPLE_REGS_USER.
>
> Fix this by discarding samples where the NMI arrives in kernel mode and
> the user has set exclude_kernel.

This will systematically under-sample instructions just before a
transition to the kernel. For this special case, perhaps the GPRs
could be obtained from task_pt_regs(current) instead of the NMI
pt_regs?

> Reported-by: Jayapal Ashwin Nair <janair@amd.com>
> Fixes: d29e744c7167 ("perf/x86: Relax privilege filter restriction on AMD IBS")
> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
> ---
>  arch/x86/events/amd/ibs.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
> index bfb51f8e184f..3cb46c775eaa 100644
> --- a/arch/x86/events/amd/ibs.c
> +++ b/arch/x86/events/amd/ibs.c
> @@ -1373,12 +1373,15 @@ static bool perf_ibs_is_kernel_br_target(struct perf_event *event,
>  }
>
>  static bool perf_ibs_discard_sample(struct perf_ibs *perf_ibs, struct perf_event *event,
> -                                   struct pt_regs *regs, struct perf_ibs_data *ibs_data,
> -                                   int br_target_idx)
> +                                   struct pt_regs *regs, struct pt_regs *iregs,
> +                                   struct perf_ibs_data *ibs_data, int br_target_idx)
>  {
>         if (perf_exclude_event(event, regs))
>                 return true;
>
> +       if (perf_exclude_event(event, iregs))
> +               return true;

This excludes events with an NMI skid past a privilege change in both
directions. Do you want to omit a kernel sample taken just before
returning to userspace if the NMI arrives in userspace and
exclude_user is set?

Re: [PATCH 3/3] perf/amd/ibs: Simplify event state handling with IBS_CAPS_DIS

[Jim Mattson]

On Wed, Oct 14, 2026 at 11:05 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>
> The IBS_{FETCH|OP}_CTL2[Dis] bit allows IBS sampling to be stopped without
> a RMW of IBS_{FETCH,OP}_CTL. Systems providing this bit therefore do not
> require the IBS_STARTED and IBS_STOPPED states used to coordinate control
> MSR updates with late NMIs.

CTL2[Dis] doesn't make late NMIs go away. Hence, this patch needs a
50us poll. With the two-line fix in my original patch, the existing
scheme handles late NMIs indefinitely: IBS_STOPPED remains set until a
late NMI consumes it, even if the event has been restarted in the
meantime. This patch trades that for a 50us window, after which a late
NMI is unknown (which *is* fatal with unknown_nmi_panic). Moreover,
since we still have to handle hardware without IBS_CAPS_DIS, we now
have two state machines instead of one.

> Add a separate stop path for these systems. Preserve Val while disabling
> the event so that a delayed NMI can be identified and consumed, and wait
> briefly for such an NMI before restarting the event.
>
> Leave the existing handling unchanged on systems without IBS_CAPS_DIS.
>
> Reported-by: Jim Mattson <jmattson@google.com>
> Closes: https://lore.kernel.org/r/20261007234815.4026028-1-jmattson@google.com
> Fixes: efa5700ec0da ("perf/amd/ibs: Support IBS_{FETCH|OP}_CTL2[Dis] to eliminate RMW race")
> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
> ---
>  arch/x86/events/amd/ibs.c | 145 ++++++++++++++++++++++++++++++++------
>  1 file changed, 122 insertions(+), 23 deletions(-)

 [ ... ]

> +/* ->stop() for platforms with additional CTL2[Dis] bit. */
> +static void perf_ibs_stop_dis(struct perf_event *event, int flags)
> +{
> +       struct hw_perf_event *hwc = &event->hw;
> +       struct perf_ibs *perf_ibs = container_of(event->pmu, struct perf_ibs, pmu);
> +       struct cpu_perf_ibs *pcpu = this_cpu_ptr(perf_ibs->pcpu);
> +       u64 config;
> +       u64 ctl2;
> +
> +       WARN_ON_ONCE(test_bit(IBS_STOPPING, pcpu->state));
> +
> +       set_bit(IBS_STOPPING, pcpu->state);

This function needs to be reentrant. If an IBS event is a sibling in a
group led by a core PMU event, and the leader is throttled,
perf_event_throttle_group() will stop the IBS sibling from the core
PMU's NMI handler:

    amd_pmu_v2_handle_irq() ->
      perf_event_overflow() ->
        __perf_event_overflow() ->
          __perf_event_account_interrupt() ->
            perf_event_throttle_group() ->
              perf_event_throttle() ->
                perf_ibs_stop() ->
                  perf_ibs_stop_dis()

[ .. ]

> +       if (in_nmi())
> +               wrmsrq(hwc->config_base, config);

As we saw above, in_nmi() doesn't mean that we're in
perf_ibs_handle_irq() for this sample. If IBS Val=1 and the IBS NMI is
still pending, this clears Val. When the IBS NMI arrives,
perf_ibs_handle_irq() finds CTL2[Dis]=1 and Val=0, and therefore
returns 0, leading to an unknown NMI.