[PATCH v2 0/5] perf/amd/ibs: Assorted fixes

[PATCH v2 0/5] perf/amd/ibs: Assorted fixes

[Ravi Bangoria]

Address several issues in the AMD IBS perf code: interrupt accounting
for discarded samples, a Zen5-specific quirk, PhyAddrVal handling,
NMI-safety with perf_allow_kernel(), and a race between event add and
NMI.

Patches are prepared on tip/perf/core (7db06e329af3)

v1: https://lore.kernel.org/r/20260116033450.965-1-ravi.bangoria@amd.com
v1->v2:
  - Split fixes series from future enhancements
  - Add interrupt throttling for zero-rip erratum #1197
  - Avoid calling perf_allow_kernel() in NMI context (new patch)

Ravi Bangoria (5):
  perf/amd/ibs: Account interrupt for discarded samples
  perf/amd/ibs: Limit ldlat->l3missonly dependency to Zen5
  perf/amd/ibs: Preserve PhyAddrVal bit when clearing PhyAddr MSR
  perf/amd/ibs: Avoid calling perf_allow_kernel() from the IBS NMI
    handler
  perf/amd/ibs: Avoid race between event add and NMI

 arch/x86/events/amd/ibs.c          | 28 ++++++++++++++++++++++------
 arch/x86/events/perf_event_flags.h |  1 +
 2 files changed, 23 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH v2 1/5] perf/amd/ibs: Account interrupt for discarded samples

[Ravi Bangoria]

Add interrupt throttling accounting for below cases:

  o IBS Op PMU: A software filter (in addition to the hardware filter)
    drops samples whose load latency is below the user-specified
    threshold.

  o IBS Fetch PMU: Samples discarded due to the zero-RIP erratum (#1197).

Although these samples are discarded, the NMI cost is still incurred, so
they should be counted for interrupt throttling.

Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 arch/x86/events/amd/ibs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index aca89f23d2e0..705ef43325be 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -1293,8 +1293,10 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
 		 * within [128, 2048] range.
 		 */
 		if (!op_data3.ld_op || !op_data3.dc_miss ||
-		    op_data3.dc_miss_lat <= (event->attr.config1 & 0xFFF))
+		    op_data3.dc_miss_lat <= (event->attr.config1 & 0xFFF)) {
+			throttle = perf_event_account_interrupt(event);
 			goto out;
+		}
 	}
 
 	/*
@@ -1326,8 +1328,10 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
 		regs.flags &= ~PERF_EFLAGS_EXACT;
 	} else {
 		/* Workaround for erratum #1197 */
-		if (perf_ibs->fetch_ignore_if_zero_rip && !(ibs_data.regs[1]))
+		if (perf_ibs->fetch_ignore_if_zero_rip && !(ibs_data.regs[1])) {
+			throttle = perf_event_account_interrupt(event);
 			goto out;
+		}
 
 		set_linear_ip(&regs, ibs_data.regs[1]);
 		regs.flags |= PERF_EFLAGS_EXACT;
-- 
2.43.0

[PATCH v2 2/5] perf/amd/ibs: Limit ldlat->l3missonly dependency to Zen5

[Ravi Bangoria]

The ldlat dependency on l3missonly is specific to Zen 5; newer generations
are not affected. This quirk is documented as an erratum in the following
Revision Guide.

  Erratum: 1606 IBS (Instruction Based Sampling) OP Load Latency Filtering
           May Capture Unwanted Samples When L3Miss Filtering is Disabled

  Revision Guide for AMD Family 1Ah Models 00h-0Fh Processors,
  Pub. 58251 Rev. 1.30 July 2025
  https://bugzilla.kernel.org/attachment.cgi?id=309193

Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 arch/x86/events/amd/ibs.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index 705ef43325be..e0b64cb13bf9 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -356,7 +356,10 @@ static int perf_ibs_init(struct perf_event *event)
 		ldlat >>= 7;
 
 		config |= (ldlat - 1) << 59;
-		config |= IBS_OP_L3MISSONLY | IBS_OP_LDLAT_EN;
+
+		config |= IBS_OP_LDLAT_EN;
+		if (cpu_feature_enabled(X86_FEATURE_ZEN5))
+			config |= IBS_OP_L3MISSONLY;
 	}
 
 	/*
-- 
2.43.0

[PATCH v2 3/5] perf/amd/ibs: Preserve PhyAddrVal bit when clearing PhyAddr MSR

[Ravi Bangoria]

Commit 50a53b60e141 ("perf/amd/ibs: Prevent leaking sensitive data to
userspace") zeroed the physical address and also cleared the PhyAddrVal
flag before copying the value into a perf sample to avoid exposing
physical addresses to unprivileged users.

Clearing PhyAddrVal, however, has an unintended side-effect: several
other IBS fields are considered valid only when this bit is set. As a
result, those otherwise correct fields are discarded, reducing IBS
functionality.

Continue to zero the physical address, but keep the PhyAddrVal bit
intact so the related fields remain usable while still preventing any
address leak.

Fixes: 50a53b60e141 ("perf/amd/ibs: Prevent leaking sensitive data to userspace")
Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 arch/x86/events/amd/ibs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index e0b64cb13bf9..05b7c9f2ec33 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -1217,12 +1217,10 @@ static void perf_ibs_phyaddr_clear(struct perf_ibs *perf_ibs,
 				   struct perf_ibs_data *ibs_data)
 {
 	if (perf_ibs == &perf_ibs_op) {
-		ibs_data->regs[ibs_op_msr_idx(MSR_AMD64_IBSOPDATA3)] &= ~(1ULL << 18);
 		ibs_data->regs[ibs_op_msr_idx(MSR_AMD64_IBSDCPHYSAD)] = 0;
 		return;
 	}
 
-	ibs_data->regs[ibs_fetch_msr_idx(MSR_AMD64_IBSFETCHCTL)] &= ~(1ULL << 52);
 	ibs_data->regs[ibs_fetch_msr_idx(MSR_AMD64_IBSFETCHPHYSAD)] = 0;
 }
 
-- 
2.43.0

[PATCH v2 4/5] perf/amd/ibs: Avoid calling perf_allow_kernel() from the IBS NMI handler

[Ravi Bangoria]

Calling perf_allow_kernel() from the NMI context is unsafe and could be
fatal. Capture the permission at event-initialization time by storing it
in event->hw.flags, and have the NMI handler rely on that cached flag
instead of making the call directly.

Reported-by: Sadasivan Shaiju <sadasivan.shaiju2@amd.com>
Fixes: 50a53b60e141d ("perf/amd/ibs: Prevent leaking sensitive data to userspace")
Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 arch/x86/events/amd/ibs.c          | 5 ++++-
 arch/x86/events/perf_event_flags.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index 05b7c9f2ec33..004226b52ac7 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -313,6 +313,9 @@ static int perf_ibs_init(struct perf_event *event)
 	if (ret)
 		return ret;
 
+	if (perf_allow_kernel())
+		hwc->flags |= PERF_X86_EVENT_UNPRIVILEGED;
+
 	if (hwc->sample_period) {
 		if (config & perf_ibs->cnt_mask)
 			/* raw max_cnt may not be set */
@@ -1349,7 +1352,7 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
 	 * unprivileged users.
 	 */
 	if ((event->attr.sample_type & PERF_SAMPLE_RAW) &&
-	    perf_allow_kernel()) {
+	    (hwc->flags & PERF_X86_EVENT_UNPRIVILEGED)) {
 		perf_ibs_phyaddr_clear(perf_ibs, &ibs_data);
 	}
 
diff --git a/arch/x86/events/perf_event_flags.h b/arch/x86/events/perf_event_flags.h
index 70078334e4a3..47f84ee8f540 100644
--- a/arch/x86/events/perf_event_flags.h
+++ b/arch/x86/events/perf_event_flags.h
@@ -23,3 +23,4 @@ PERF_ARCH(PEBS_LAT_HYBRID,	0x0020000) /* ld and st lat for hybrid */
 PERF_ARCH(NEEDS_BRANCH_STACK,	0x0040000) /* require branch stack setup */
 PERF_ARCH(BRANCH_COUNTERS,	0x0080000) /* logs the counters in the extra space of each branch */
 PERF_ARCH(ACR,			0x0100000) /* Auto counter reload */
+PERF_ARCH(UNPRIVILEGED,		0x0200000) /* Unprivileged event (wrt perf_allow_kernel()) */
-- 
2.43.0

[PATCH v2 5/5] perf/amd/ibs: Avoid race between event add and NMI

[Ravi Bangoria]

Consider the following race:

  --------
  o OP_CTL contains stale value: OP_CTL[Val]=1, OP_CTL[En]=0
  o A new IBS OP event is being added
  o [P]: Process context, [N]: NMI context

  [P] perf_ibs_add(event) {
  [P]     if (test_and_set_bit(IBS_ENABLED, pcpu->state))
  [P]         return;
  [P]     /* pcpu->state = IBS_ENABLED */
  [P]
  [P]     pcpu->event = event;
  [P]
  [P]     perf_ibs_start(event) {
  [P]         set_bit(IBS_STARTED, pcpu->state);
  [P]         /* pcpu->state = IBS_ENABLED | IBS_STARTED */
  [P]         clear_bit(IBS_STOPPING, pcpu->state);
  [P]         /* pcpu->state = IBS_ENABLED | IBS_STARTED */

  [N] --> NMI due to genuine FETCH event. perf_ibs_handle_irq()
  [N]     called for OP PMU as well.
  [N]
  [N] perf_ibs_handle_irq(perf_ibs) {
  [N]     event = pcpu->event; /* See line 6 */
  [N]
  [N]     if (!test_bit(IBS_STARTED, pcpu->state)) /* false */
  [N]         return 0;
  [N]
  [N]     if (WARN_ON_ONCE(!event)) /* false */
  [N]         goto fail;
  [N]
  [N]     if (!(*buf++ & perf_ibs->valid_mask)) /* false due to stale
  [N]                                            * IBS_OP_CTL value */
  [N]         goto fail;
  [N]
  [N]         ...
  [N]
  [N]     perf_ibs_enable_event() // *Accidentally* enable the event.
  [N] }
  [N]
  [N] /*
  [N]  * Repeated NMIs may follow due to accidentally enabled IBS OP
  [N]  * event if the sample period is very low. It could also lead
  [N]  * to pcpu->state corruption if the event gets throttled due
  [N]  * to too frequent NMIs.
  [N]  */

  [P]         perf_ibs_enable_event();
  [P]     }
  [P] }
  --------

We cannot safely clear IBS_{FETCH|OP}_CTL while disabling the event,
because the register might be read again later. So, clear the register
in the enable path - before we update pcpu->state and enable the event.
This guarantees that any NMI that lands in the gap finds Val=0 and
bails out cleanly.

Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 arch/x86/events/amd/ibs.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index 004226b52ac7..32e6456cb5e5 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -493,6 +493,14 @@ 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);
+
 	/*
 	 * Set STARTED before enabling the hardware, such that a subsequent NMI
 	 * must observe it.
-- 
2.43.0

Re: [PATCH v2 0/5] perf/amd/ibs: Assorted fixes

[Namhyung Kim]

Hello Ravi,

On Mon, Feb 16, 2026 at 04:22:11AM +0000, Ravi Bangoria wrote:
> Address several issues in the AMD IBS perf code: interrupt accounting
> for discarded samples, a Zen5-specific quirk, PhyAddrVal handling,
> NMI-safety with perf_allow_kernel(), and a race between event add and
> NMI.
> 
> Patches are prepared on tip/perf/core (7db06e329af3)
> 
> v1: https://lore.kernel.org/r/20260116033450.965-1-ravi.bangoria@amd.com
> v1->v2:
>   - Split fixes series from future enhancements
>   - Add interrupt throttling for zero-rip erratum #1197
>   - Avoid calling perf_allow_kernel() in NMI context (new patch)
> 
> Ravi Bangoria (5):
>   perf/amd/ibs: Account interrupt for discarded samples
>   perf/amd/ibs: Limit ldlat->l3missonly dependency to Zen5
>   perf/amd/ibs: Preserve PhyAddrVal bit when clearing PhyAddr MSR
>   perf/amd/ibs: Avoid calling perf_allow_kernel() from the IBS NMI
>     handler
>   perf/amd/ibs: Avoid race between event add and NMI

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> 
>  arch/x86/events/amd/ibs.c          | 28 ++++++++++++++++++++++------
>  arch/x86/events/perf_event_flags.h |  1 +
>  2 files changed, 23 insertions(+), 6 deletions(-)
> 
> -- 
> 2.43.0
> 

Re: [PATCH v2 1/5] perf/amd/ibs: Account interrupt for discarded samples

[Michael Petlan]

Hi Ravi,

On Mon, 16 Feb 2026, Ravi Bangoria wrote:
> Add interrupt throttling accounting for below cases:
> 
>   o IBS Op PMU: A software filter (in addition to the hardware filter)
>     drops samples whose load latency is below the user-specified
>     threshold.
> 
>   o IBS Fetch PMU: Samples discarded due to the zero-RIP erratum (#1197).
> 
> Although these samples are discarded, the NMI cost is still incurred, so
> they should be counted for interrupt throttling.
> 

Maybe worth tagging

Fixes: 26db2e0c51fe83e1dd852c1321407835b481806e ("perf/x86/amd/ibs: Work around erratum #1197")
Fixes: d20610c19b4a22bc69085b7eb7a02741d51de30e ("perf/amd/ibs: Add support for OP Load Latency Filtering")

?

Cheers,
Michael

> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
> ---
>  arch/x86/events/amd/ibs.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
> index aca89f23d2e0..705ef43325be 100644
> --- a/arch/x86/events/amd/ibs.c
> +++ b/arch/x86/events/amd/ibs.c
> @@ -1293,8 +1293,10 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
>  		 * within [128, 2048] range.
>  		 */
>  		if (!op_data3.ld_op || !op_data3.dc_miss ||
> -		    op_data3.dc_miss_lat <= (event->attr.config1 & 0xFFF))
> +		    op_data3.dc_miss_lat <= (event->attr.config1 & 0xFFF)) {
> +			throttle = perf_event_account_interrupt(event);
>  			goto out;
> +		}
>  	}
>  
>  	/*
> @@ -1326,8 +1328,10 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
>  		regs.flags &= ~PERF_EFLAGS_EXACT;
>  	} else {
>  		/* Workaround for erratum #1197 */
> -		if (perf_ibs->fetch_ignore_if_zero_rip && !(ibs_data.regs[1]))
> +		if (perf_ibs->fetch_ignore_if_zero_rip && !(ibs_data.regs[1])) {
> +			throttle = perf_event_account_interrupt(event);
>  			goto out;
> +		}
>  
>  		set_linear_ip(&regs, ibs_data.regs[1]);
>  		regs.flags |= PERF_EFLAGS_EXACT;
> -- 
> 2.43.0
> 
> 
> 

Re: [PATCH v2 1/5] perf/amd/ibs: Account interrupt for discarded samples

[Ravi Bangoria]

Hi Michael,

>> Add interrupt throttling accounting for below cases:
>>
>>   o IBS Op PMU: A software filter (in addition to the hardware filter)
>>     drops samples whose load latency is below the user-specified
>>     threshold.
>>
>>   o IBS Fetch PMU: Samples discarded due to the zero-RIP erratum (#1197).
>>
>> Although these samples are discarded, the NMI cost is still incurred, so
>> they should be counted for interrupt throttling.
>>
> 
> Maybe worth tagging
> 
> Fixes: 26db2e0c51fe83e1dd852c1321407835b481806e ("perf/x86/amd/ibs: Work around erratum #1197")
> Fixes: d20610c19b4a22bc69085b7eb7a02741d51de30e ("perf/amd/ibs: Add support for OP Load Latency Filtering")

Yes, makes sense.

Ingo / Peter, Please let me know if you want me to respin.

Thanks,
Ravi

Re: [PATCH v2 1/5] perf/amd/ibs: Account interrupt for discarded samples

[Peter Zijlstra]

On Tue, Feb 24, 2026 at 01:41:54PM +0530, Ravi Bangoria wrote:
> Hi Michael,
> 
> >> Add interrupt throttling accounting for below cases:
> >>
> >>   o IBS Op PMU: A software filter (in addition to the hardware filter)
> >>     drops samples whose load latency is below the user-specified
> >>     threshold.
> >>
> >>   o IBS Fetch PMU: Samples discarded due to the zero-RIP erratum (#1197).
> >>
> >> Although these samples are discarded, the NMI cost is still incurred, so
> >> they should be counted for interrupt throttling.
> >>
> > 
> > Maybe worth tagging
> > 
> > Fixes: 26db2e0c51fe83e1dd852c1321407835b481806e ("perf/x86/amd/ibs: Work around erratum #1197")
> > Fixes: d20610c19b4a22bc69085b7eb7a02741d51de30e ("perf/amd/ibs: Add support for OP Load Latency Filtering")
> 
> Yes, makes sense.
> 
> Ingo / Peter, Please let me know if you want me to respin.

I've added them. Thanks!

Re: [PATCH v2 4/5] perf/amd/ibs: Avoid calling perf_allow_kernel() from the IBS NMI handler

[Michael Petlan]

On Mon, 16 Feb 2026, Ravi Bangoria wrote:
> Calling perf_allow_kernel() from the NMI context is unsafe and could be
> fatal. Capture the permission at event-initialization time by storing it
> in event->hw.flags, and have the NMI handler rely on that cached flag
> instead of making the call directly.
>

Hi Ravi.

I have tested this fix on a machine (below), which without this fix can
be easily crashed by recording ibs_op/l3missonly=1/ event.

With this fix, it survived 4900+ iterations without any crash.

CPU details
===========
BIOS CPU family:      107
CPU family:            26
Model:                  2
Stepping:               1

Thanks for fixing.

Cheers,
Michael

> Reported-by: Sadasivan Shaiju <sadasivan.shaiju2@amd.com>
> Fixes: 50a53b60e141d ("perf/amd/ibs: Prevent leaking sensitive data to userspace")
> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>

Tested-by: Michael Petlan <mpetlan@redhat.com>

> ---
>  arch/x86/events/amd/ibs.c          | 5 ++++-
>  arch/x86/events/perf_event_flags.h | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
> index 05b7c9f2ec33..004226b52ac7 100644
> --- a/arch/x86/events/amd/ibs.c
> +++ b/arch/x86/events/amd/ibs.c
> @@ -313,6 +313,9 @@ static int perf_ibs_init(struct perf_event *event)
>  	if (ret)
>  		return ret;
>  
> +	if (perf_allow_kernel())
> +		hwc->flags |= PERF_X86_EVENT_UNPRIVILEGED;
> +
>  	if (hwc->sample_period) {
>  		if (config & perf_ibs->cnt_mask)
>  			/* raw max_cnt may not be set */
> @@ -1349,7 +1352,7 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
>  	 * unprivileged users.
>  	 */
>  	if ((event->attr.sample_type & PERF_SAMPLE_RAW) &&
> -	    perf_allow_kernel()) {
> +	    (hwc->flags & PERF_X86_EVENT_UNPRIVILEGED)) {
>  		perf_ibs_phyaddr_clear(perf_ibs, &ibs_data);
>  	}
>  
> diff --git a/arch/x86/events/perf_event_flags.h b/arch/x86/events/perf_event_flags.h
> index 70078334e4a3..47f84ee8f540 100644
> --- a/arch/x86/events/perf_event_flags.h
> +++ b/arch/x86/events/perf_event_flags.h
> @@ -23,3 +23,4 @@ PERF_ARCH(PEBS_LAT_HYBRID,	0x0020000) /* ld and st lat for hybrid */
>  PERF_ARCH(NEEDS_BRANCH_STACK,	0x0040000) /* require branch stack setup */
>  PERF_ARCH(BRANCH_COUNTERS,	0x0080000) /* logs the counters in the extra space of each branch */
>  PERF_ARCH(ACR,			0x0100000) /* Auto counter reload */
> +PERF_ARCH(UNPRIVILEGED,		0x0200000) /* Unprivileged event (wrt perf_allow_kernel()) */
> -- 
> 2.43.0
> 
> 
> 

Re: [PATCH v2 4/5] perf/amd/ibs: Avoid calling perf_allow_kernel() from the IBS NMI handler

[Michael Petlan]

Hello Ravi.

By evaluating the privileges at the event-init moment and using cached
value later, couldn't it lead to the cached value being obsolete at
some point? E.g. a setuid program later dropping the privileges but
still being able to read physical addresses? I don't think it can be
a solid attack-surface, but still may be worth of a thought... ?

Michael

On Mon, 16 Feb 2026, Ravi Bangoria wrote:
> Calling perf_allow_kernel() from the NMI context is unsafe and could be
> fatal. Capture the permission at event-initialization time by storing it
> in event->hw.flags, and have the NMI handler rely on that cached flag
> instead of making the call directly.
> 
> Reported-by: Sadasivan Shaiju <sadasivan.shaiju2@amd.com>
> Fixes: 50a53b60e141d ("perf/amd/ibs: Prevent leaking sensitive data to userspace")
> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
> ---
>  arch/x86/events/amd/ibs.c          | 5 ++++-
>  arch/x86/events/perf_event_flags.h | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
> index 05b7c9f2ec33..004226b52ac7 100644
> --- a/arch/x86/events/amd/ibs.c
> +++ b/arch/x86/events/amd/ibs.c
> @@ -313,6 +313,9 @@ static int perf_ibs_init(struct perf_event *event)
>  	if (ret)
>  		return ret;
>  
> +	if (perf_allow_kernel())
> +		hwc->flags |= PERF_X86_EVENT_UNPRIVILEGED;
> +
>  	if (hwc->sample_period) {
>  		if (config & perf_ibs->cnt_mask)
>  			/* raw max_cnt may not be set */
> @@ -1349,7 +1352,7 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
>  	 * unprivileged users.
>  	 */
>  	if ((event->attr.sample_type & PERF_SAMPLE_RAW) &&
> -	    perf_allow_kernel()) {
> +	    (hwc->flags & PERF_X86_EVENT_UNPRIVILEGED)) {
>  		perf_ibs_phyaddr_clear(perf_ibs, &ibs_data);
>  	}
>  
> diff --git a/arch/x86/events/perf_event_flags.h b/arch/x86/events/perf_event_flags.h
> index 70078334e4a3..47f84ee8f540 100644
> --- a/arch/x86/events/perf_event_flags.h
> +++ b/arch/x86/events/perf_event_flags.h
> @@ -23,3 +23,4 @@ PERF_ARCH(PEBS_LAT_HYBRID,	0x0020000) /* ld and st lat for hybrid */
>  PERF_ARCH(NEEDS_BRANCH_STACK,	0x0040000) /* require branch stack setup */
>  PERF_ARCH(BRANCH_COUNTERS,	0x0080000) /* logs the counters in the extra space of each branch */
>  PERF_ARCH(ACR,			0x0100000) /* Auto counter reload */
> +PERF_ARCH(UNPRIVILEGED,		0x0200000) /* Unprivileged event (wrt perf_allow_kernel()) */
> -- 
> 2.43.0
> 
> 
> 

Re: [PATCH v2 4/5] perf/amd/ibs: Avoid calling perf_allow_kernel() from the IBS NMI handler

[Ravi Bangoria]

Hi Michael,

> By evaluating the privileges at the event-init moment and using cached
> value later, couldn't it lead to the cached value being obsolete at
> some point? E.g. a setuid program later dropping the privileges but
> still being able to read physical addresses?

Yes, but wouldn't the same concern apply to all other call sites of
perf_allow_kernel() ?

Thanks,
Ravi

Re: [PATCH v2 4/5] perf/amd/ibs: Avoid calling perf_allow_kernel() from the IBS NMI handler

[Michael Petlan]

On Mon, 9 Mar 2026, Ravi Bangoria wrote:
> Hi Michael,
> 
> > By evaluating the privileges at the event-init moment and using cached
> > value later, couldn't it lead to the cached value being obsolete at
> > some point? E.g. a setuid program later dropping the privileges but
> > still being able to read physical addresses?
> 
> Yes, but wouldn't the same concern apply to all other call sites of
> perf_allow_kernel() ?

Well, I don't say this case is worse than the others, just raising a
more generic question whether this design is fairly acceptable or would
deserve e.g. privilege adjusting at the time when they are dropped or
something... Nothing more, just a note... :)

Michael
> 
> Thanks,
> Ravi
> 
> 

Re: [PATCH v2 4/5] perf/amd/ibs: Avoid calling perf_allow_kernel() from the IBS NMI handler

[Ravi Bangoria]

Hi Michael,

>>> By evaluating the privileges at the event-init moment and using cached
>>> value later, couldn't it lead to the cached value being obsolete at
>>> some point? E.g. a setuid program later dropping the privileges but
>>> still being able to read physical addresses?
>>
>> Yes, but wouldn't the same concern apply to all other call sites of
>> perf_allow_kernel() ?
> 
> Well, I don't say this case is worse than the others, just raising a
> more generic question whether this design is fairly acceptable or would
> deserve e.g. privilege adjusting at the time when they are dropped or
> something... Nothing more, just a note... :)

I understand. However, performing the privilege check at the beginning
and then continuing to allow accessing the privileged resource even
after privileges have been dropped is a common design pattern, isn't it?
For example, I assume something like this is allowed:

  fd = open("/root/only/file");
  setresuid(); /* drop the privilege */
  read(fd);

Thanks,
Ravi