[PATCH v2 0/9] perf/cxlpmu: Misc sashiko raised issues fixes

[PATCH v2 0/9] perf/cxlpmu: Misc sashiko raised issues fixes

[Dave Jiang]

Teed off of Davidlohr's misc cxlpmu series [1]. I had Claude pick up all
the sashiko raised issues and then continued multiple internal review
iterations to pick up a number of fixes. I'm no CXL PMU or perf expert,
but the fixes look reasonable to me AFAICT.

Please give extra scrutiny to patch 3 and ensure the math is correct.

Since v1:
- Updated 3/9 to address sashiko issue.
- No other changes as sashiko only raised issue with pre-existing.

[1]: https://lore.kernel.org/linux-cxl/20260715191454.459673-1-dave@stgolabs.net/

Dave Jiang (9):
  perf/cxl: Program the requested event group on configurable counters
  perf/cxl: Clear stale event fields before reprogramming a counter
  perf/cxl: Fix the counter overflow delta fixup
  perf/cxl: Accept an overflow interrupt on MSI message number 0
  perf/cxl: Keep the overflow interrupt pinned to the managed CPU
  perf/cxl: Unfreeze counters after handling an overflow interrupt
  perf/cxl: Validate the hardware-reported counter width
  perf/cxl: Don't use pmu.dev in IRQ and hotplug callbacks after
    unregister
  perf/cxl: Avoid cpumask_of(-1) when no CPU is assigned

 drivers/perf/cxl_pmu.c | 61 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 50 insertions(+), 11 deletions(-)


base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
-- 
2.55.0

[PATCH v2 1/9] perf/cxl: Program the requested event group on configurable counters

[Dave Jiang]

cxl_pmu_get_event_idx() sets *counter_idx for a configurable counter but
never sets *event_idx, so hwc->event_base stays 0 and cxl_pmu_event_start()
programs event group 0 regardless of the group the user requested.

Return the matched capability's event_idx.

Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/perf/cxl_pmu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 68a54d97d2a8..e77c0650a7eb 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -552,6 +552,8 @@ static int cxl_pmu_get_event_idx(struct perf_event *event, int *counter_idx,
 			return -EINVAL;
 
 		*counter_idx = i;
+		if (event_idx)
+			*event_idx = pmu_ev->event_idx;
 		return 0;
 	}
 
-- 
2.55.0

Re: [PATCH v2 1/9] perf/cxl: Program the requested event group on configurable counters

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware State Pollution / Missing Bitmask Clear on PMU Counter Reuse
- [High] Invalid 64-bit Write on a 32-bit PCIe Register
- [High] Undefined Behavior Due to Out-of-Bounds Shift in Overflow Handling
--

--- Patch [1]: [PATCH v2 1/9] perf/cxl: Program the requested event group on configurable counters ---
commit 3f34f60eb49c6f10c29064fdd1d2a3cf3e95a287
Author: Dave Jiang <dave.jiang@intel.com>
Date:   Wed Jul 29 14:55:47 2026 +0000

perf/cxl: Program the requested event group on configurable counters

cxl_pmu_get_event_idx() sets *counter_idx for a configurable counter but
never sets *event_idx, so hwc->event_base stays 0 and cxl_pmu_event_start()
programs event group 0 regardless of the group the user requested.

Return the matched capability's event_idx.

Link: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 68a54d97d2a8a..e77c0650a7eb6 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -552,6 +552,8 @@ static int cxl_pmu_get_event_idx(struct perf_event *event, int *counter_idx,
>  			return -EINVAL;
>  
>  		*counter_idx = i;
> +		if (event_idx)
> +			*event_idx = pmu_ev->event_idx;

[Severity: High]
This isn't a bug introduced by this patch, but returning the correct 
event_idx here fully exposes an existing state pollution issue in 
cxl_pmu_event_start().

When a hardware counter is reused, the configuration is bitwise-ORed:

drivers/perf/cxl_pmu.c:cxl_pmu_event_start() {
...
	cfg = readq(base + CXL_PMU_COUNTER_CFG_REG(hwc->idx));
	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_INT_ON_OVRFLW, 1);
...
	if (test_bit(hwc->idx, info->conf_counter_bm)) {
		cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK,
				  hwc->event_base);
		cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_EVENTS_MSK,
				  cxl_pmu_config_get_mask(event));
	}
...
}

Because the previous bits are not cleared, does this code accumulate 
garbage on reused counters (e.g., ORing the new hwc->event_base with 
the old one)?


[Severity: High]
This is another pre-existing issue, but I noticed a potential hardware access
violation in cxl_pmu_event_start() when dealing with HDM filtering:

drivers/perf/cxl_pmu.c:cxl_pmu_event_start() {
...
		writeq(cfg, base + CXL_PMU_FILTER_CFG_REG(hwc->idx, 0));
...
}

Since CXL_PMU_FILTER_CFG_REG is a 32-bit register, could sending a 64-bit
write with writeq() trigger a PCIe Unsupported Request error or silently
overwrite the adjacent Filter 1 register?


[Severity: High]
This is a pre-existing issue, but there appears to be a shift out of bounds
in the overflow handling path:

drivers/perf/cxl_pmu.c:__cxl_pmu_read() {
...
	if (overflow && delta < GENMASK_ULL(info->counter_width - 1, 0))
		delta += (1UL << info->counter_width);
...
}

If the platform supports a counter_width of 64 (or >= 32 on 32-bit
architectures), does the use of 1UL rather than 1ULL invoke undefined
behavior and silently corrupt the delta calculation?

>  		return 0;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729145555.3919550-1-dave.jiang@intel.com?part=1

[PATCH v2 2/9] perf/cxl: Clear stale event fields before reprogramming a counter

[Dave Jiang]

cxl_pmu_event_start() ORs the event group id, event mask, edge and invert
selections into a configurable counter's config register without clearing
those fields first, and cxl_pmu_event_stop() leaves them set. Reusing a
counter for another event therefore ORs the new selection onto the old,
counting the wrong events with stale edge/invert behaviour.

Clear EVENT_GRP_ID_IDX, EVENTS, EDGE and INVERT before applying the new
values.

Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/perf/cxl_pmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index e77c0650a7eb..3138514157cd 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -641,6 +641,7 @@ static void cxl_pmu_event_start(struct perf_event *event, int flags)
 	}
 
 	cfg = readq(base + CXL_PMU_COUNTER_CFG_REG(hwc->idx));
+	cfg &= ~(CXL_PMU_COUNTER_CFG_EDGE | CXL_PMU_COUNTER_CFG_INVERT);
 	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_INT_ON_OVRFLW, 1);
 	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_FREEZE_ON_OVRFLW, 1);
 	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_ENABLE, 1);
@@ -651,6 +652,8 @@ static void cxl_pmu_event_start(struct perf_event *event, int flags)
 
 	/* Fixed purpose counters have next two fields RO */
 	if (test_bit(hwc->idx, info->conf_counter_bm)) {
+		cfg &= ~(CXL_PMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK |
+			 CXL_PMU_COUNTER_CFG_EVENTS_MSK);
 		cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK,
 				  hwc->event_base);
 		cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_EVENTS_MSK,
-- 
2.55.0

Re: [PATCH v2 2/9] perf/cxl: Clear stale event fields before reprogramming a counter

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] A 64-bit `writeq()` is incorrectly used to write a 32-bit filter configuration register, corrupting the adjacent filter register.
- [High] The PMU event read logic incorrectly adds `2^width` to the event delta on typical overflows, causing massive overcounting and undefined behavior.
- [Medium] The PMU sysfs format attribute for `threshold` accepts a 16-bit value, but the accessor silently truncates it to 8 bits.
--

--- Patch [2]: [PATCH v2 2/9] perf/cxl: Clear stale event fields before reprogramming a counter ---
commit 4bfb25bc15d355490e2f63cedff157f6adb1461a
Author: Dave Jiang <dave.jiang@intel.com>

perf/cxl: Clear stale event fields before reprogramming a counter

This commit fixes an issue where reprogramming a performance counter leaves
stale values in the configuration register. It ensures the event group id,
event mask, edge, and invert fields are cleared before being reprogrammed.
This prevents the counter from accumulating statistics with old selections.

Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index e77c0650a7eb6..3138514157cde 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c

[ ... ]

> @@ -641,6 +641,7 @@ static void cxl_pmu_event_start(struct perf_event *event, int flags)
>  	}
>  
>  	cfg = readq(base + CXL_PMU_COUNTER_CFG_REG(hwc->idx));
> +	cfg &= ~(CXL_PMU_COUNTER_CFG_EDGE | CXL_PMU_COUNTER_CFG_INVERT);
>  	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_INT_ON_OVRFLW, 1);
>  	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_FREEZE_ON_OVRFLW, 1);
>  	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_ENABLE, 1);

[Severity: High]
This isn't a bug introduced by this patch, but does the filter configuration
overwrite the adjacent Filter 1 register?

drivers/perf/cxl_pmu.c:cxl_pmu_event_start()
	if (info->filter_hdm) {
		if (cxl_pmu_config1_hdm_filter_en(event))
			cfg = cxl_pmu_config2_get_hdm_decoder(event);
		else
			cfg = GENMASK(31, 0); /* No filtering if 0xFFFF_FFFF */
		writeq(cfg, base + CXL_PMU_FILTER_CFG_REG(hwc->idx, 0));
	}

Because CXL_PMU_FILTER_CFG_REG uses a 4-byte stride, the registers are
expected to be 32-bit wide. A 64-bit writeq() will write 8 bytes, which
on a little-endian system clobbers the adjacent Filter 1 register when
writing to Filter 0.

> @@ -651,6 +652,8 @@ static void cxl_pmu_event_start(struct perf_event *event, int flags)
>  
>  	/* Fixed purpose counters have next two fields RO */
>  	if (test_bit(hwc->idx, info->conf_counter_bm)) {
> +		cfg &= ~(CXL_PMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK |
> +			 CXL_PMU_COUNTER_CFG_EVENTS_MSK);
>  		cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK,
>  				  hwc->event_base);
>  		cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_EVENTS_MSK,

[Severity: Medium]
This is a pre-existing issue, but could the threshold accessor silently
truncate user-provided values?

drivers/perf/cxl_pmu.c:cxl_pmu_config1_get_threshold()
static u8 cxl_pmu_config1_get_threshold(struct perf_event *event)
{
	return FIELD_GET(CXL_PMU_ATTR_CONFIG1_THRESHOLD_MSK,
			 event->attr.config1);
}

The accessor returns a u8, but the PMU sysfs format attribute for threshold
accepts a 16-bit value (config1:0-15). If a user requests an event with a
threshold configuration greater than 255, the value gets silently truncated
before being programmed into the hardware.

[Severity: High]
This isn't a bug introduced by this patch, but does the event read logic
cause massive overcounting when a counter overflows?

drivers/perf/cxl_pmu.c:__cxl_pmu_read()
	delta = (new_cnt - prev_cnt) & GENMASK_ULL(info->counter_width - 1, 0);
	if (overflow && delta < GENMASK_ULL(info->counter_width - 1, 0))
		delta += (1UL << info->counter_width);

The calculation (new_cnt - prev_cnt) & mask correctly handles a single
wrap-around when new_cnt < prev_cnt due to two's complement arithmetic.
Because delta < mask is almost always true during an overflow, the subsequent
addition double counts the wrap-around.

Additionally, if info->counter_width is 64, shifting 1UL by 64 will invoke
undefined behavior.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729145555.3919550-1-dave.jiang@intel.com?part=2

[PATCH v2 3/9] perf/cxl: Fix the counter overflow delta fixup

[Dave Jiang]

Counters are configured with Freeze on Overflow and are never reloaded: on
overflow the counter wraps to 0, counts on until the CPMU freezes, and
retains that residual (CXL r4.0 8.2.7.2.3). So the masked subtraction in
__cxl_pmu_read() only spans the wrap when new_cnt < prev_cnt.

The fixup keys off the delta rather than the operands:

	delta = (new_cnt - prev_cnt) & GENMASK_ULL(counter_width - 1, 0);
	if (overflow && delta < GENMASK_ULL(counter_width - 1, 0))
		delta += (1UL << counter_width);

and is wrong both ways. After a mid-period read the subtraction already
spans the wrap, but the guard holds for all but one delta value, so a
second period is added and the event over-counts. After an earlier
overflow read - or after event_start(), which leaves prev_count at 0 -
prev_cnt is a small residual, and once the new residual is greater or
equal only the difference between the two is counted and the period is
lost, which for a residual of 0 means every period after the first.
'perf stat -I' hits the former, plain 'perf stat' the latter.

The shift is also undefined for counter_width == 64, and for >= 32 on
32-bit kernels where 1UL is 32 bits.

Condition the fixup on new_cnt >= prev_cnt, the one case the masked
subtraction cannot express, and use mask + 1 for the period: it is 0 for a
64-bit counter, the correct sum mod 2^64, and needs no shift. A
userspace read landing between the wrap and the handler clearing the
overflow status stays correct because the counter is frozen and cannot
advance between the two reads.

Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v2:
- Fix the fixup instead of deleting it. v1 dropped it on the rationale
  that the masked subtraction already handles the wrap, which only holds
  for new_cnt < prev_cnt; with freeze on overflow and no reload the
  overflow path routinely sees new_cnt >= prev_cnt, where v1 silently
  dropped a full period (sashiko-bot,
  https://lore.kernel.org/linux-cxl/20260728211411.47B2D1F000E9@smtp.kernel.org/).
- Keep the overflow argument and the __cxl_pmu_read()/cxl_pmu_read()
  split; v1 folded them together once the argument went unused.
- Use mask + 1 rather than 1ULL << counter_width so the 64-bit counter
  case needs no special casing.
---
 drivers/perf/cxl_pmu.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 3138514157cd..956da8202551 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -690,7 +690,7 @@ static void __cxl_pmu_read(struct perf_event *event, bool overflow)
 {
 	struct cxl_pmu_info *info = pmu_to_cxl_pmu_info(event->pmu);
 	struct hw_perf_event *hwc = &event->hw;
-	u64 new_cnt, prev_cnt, delta;
+	u64 new_cnt, prev_cnt, delta, mask;
 
 	do {
 		prev_cnt = local64_read(&hwc->prev_count);
@@ -698,12 +698,18 @@ static void __cxl_pmu_read(struct perf_event *event, bool overflow)
 	} while (local64_cmpxchg(&hwc->prev_count, prev_cnt, new_cnt) != prev_cnt);
 
 	/*
-	 * If we know an overflow occur then take that into account.
-	 * Note counter is not reset as that would lose events
+	 * The counter wraps to 0 and is never reloaded, so the masked unsigned
+	 * subtraction only recovers a wrap when new_cnt < prev_cnt. On the
+	 * overflow path new_cnt >= prev_cnt means the full period elapsed
+	 * (prev_cnt is 0 from event_start(), or the residual left by an earlier
+	 * overflow), so add the period back. mask + 1 is 2^counter_width, which
+	 * evaluates to 0 for a 64-bit counter - the correct sum mod 2^64, and
+	 * avoids the undefined 1 << 64.
 	 */
-	delta = (new_cnt - prev_cnt) & GENMASK_ULL(info->counter_width - 1, 0);
-	if (overflow && delta < GENMASK_ULL(info->counter_width - 1, 0))
-		delta += (1UL << info->counter_width);
+	mask = GENMASK_ULL(info->counter_width - 1, 0);
+	delta = (new_cnt - prev_cnt) & mask;
+	if (overflow && new_cnt >= prev_cnt)
+		delta += mask + 1;
 
 	local64_add(delta, &event->count);
 }
-- 
2.55.0

Re: [PATCH v2 3/9] perf/cxl: Fix the counter overflow delta fixup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A race condition between a lockless PMU read and the PMU overflow IRQ handler causes a full counter period to be erroneously added, massively double-counting the event.
--

--- Patch [3]: [PATCH v2 3/9] perf/cxl: Fix the counter overflow delta fixup ---
commit 72c8c960c955004fc0b667e90e3698f58a0d0ea6
Author: Dave Jiang <dave.jiang@intel.com>

perf/cxl: Fix the counter overflow delta fixup

This commit updates the CXL PMU counter overflow handling since the counters
freeze on overflow and are not reloaded. It modifies the delta fixup logic
to trigger when new_cnt is greater than or equal to prev_cnt, adding the
full period back to account for the wrap.

Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 3138514157cde..956da82025513 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c

[ ... ]

> @@ -698,12 +698,18 @@ static void __cxl_pmu_read(struct perf_event *event, bool overflow)
>  	} while (local64_cmpxchg(&hwc->prev_count, prev_cnt, new_cnt) != prev_cnt);
>  
>  	/*
> -	 * If we know an overflow occur then take that into account.
> -	 * Note counter is not reset as that would lose events
> +	 * The counter wraps to 0 and is never reloaded, so the masked unsigned
> +	 * subtraction only recovers a wrap when new_cnt < prev_cnt. On the
> +	 * overflow path new_cnt >= prev_cnt means the full period elapsed
> +	 * (prev_cnt is 0 from event_start(), or the residual left by an earlier
> +	 * overflow), so add the period back. mask + 1 is 2^counter_width, which
> +	 * evaluates to 0 for a 64-bit counter - the correct sum mod 2^64, and
> +	 * avoids the undefined 1 << 64.
>  	 */
> -	delta = (new_cnt - prev_cnt) & GENMASK_ULL(info->counter_width - 1, 0);
> -	if (overflow && delta < GENMASK_ULL(info->counter_width - 1, 0))
> -		delta += (1UL << info->counter_width);
> +	mask = GENMASK_ULL(info->counter_width - 1, 0);
> +	delta = (new_cnt - prev_cnt) & mask;
> +	if (overflow && new_cnt >= prev_cnt)
> +		delta += mask + 1;

[Severity: High]
This is a pre-existing issue, but can this lead to massive double-counting
if a concurrent lockless read races with the overflow handler?

If a userspace lockless PMU read, such as perf stat -I, happens after the
counter wraps and freezes, but before the delayed overflow IRQ handler
executes, the lockless reader will correctly compute the wrapped delta and
update hwc->prev_count to the frozen residual.

When the IRQ handler in __cxl_pmu_read() subsequently runs, both prev_cnt
and new_cnt will read the same frozen residual. The condition
new_cnt >= prev_cnt will evaluate to true since they are equal, causing the
handler to blindly add a full period (mask + 1) on top of the already
accounted wrap.

Could this be addressed by distinguishing between a full period elapsing and
a frozen residual being consumed by a concurrent lockless read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729145555.3919550-1-dave.jiang@intel.com?part=3

[PATCH v2 4/9] perf/cxl: Accept an overflow interrupt on MSI message number 0

[Dave Jiang]

cxl_pmu_probe() rejects the PMU with -EINVAL when info->irq <= 0, but
info->irq holds the MSI/MSI-X message number the device signals overflow
on (CXL_PMU_CAP_MSI_N_MSK, a 0-based 4-bit field), with -1 meaning "no
interrupt support". Message number 0 is valid and pci_irq_vector() takes
a 0-based index, so a compliant device signalling on the first vector is
wrongly refused and fails to probe.

Reject only the no-interrupt case (info->irq < 0), matching the sibling
CXL mailbox and event IRQ handling.

Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/perf/cxl_pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 956da8202551..d1e810601e36 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -876,7 +876,7 @@ static int cxl_pmu_probe(struct device *dev)
 		.capabilities = PERF_PMU_CAP_NO_EXCLUDE,
 	};
 
-	if (info->irq <= 0)
+	if (info->irq < 0)
 		return -EINVAL;
 
 	rc = pci_irq_vector(pdev, info->irq);
-- 
2.55.0

[PATCH v2 5/9] perf/cxl: Keep the overflow interrupt pinned to the managed CPU

[Dave Jiang]

The PMU pins its overflow interrupt to info->on_cpu in the hotplug
online/offline callbacks, but requests it with only IRQF_SHARED |
IRQF_NO_THREAD. Without IRQF_NOBALANCING, irqbalance or a userspace
smp_affinity write can move the interrupt to another CPU. cxl_pmu_irq()
then runs cxl_pmu_read() there, doing local64_cmpxchg()/local64_add() on
hwc->prev_count and event->count concurrently with the managing CPU;
local64_t is only atomic against same-CPU access, so counts get
corrupted.

Add IRQF_NOBALANCING so the pinning done in the hotplug callbacks holds,
matching other uncore-style PMU drivers.

Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/perf/cxl_pmu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index d1e810601e36..8b89db8f4d68 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -888,7 +888,8 @@ static int cxl_pmu_probe(struct device *dev)
 	if (!irq_name)
 		return -ENOMEM;
 
-	rc = devm_request_irq(dev, irq, cxl_pmu_irq, IRQF_SHARED | IRQF_NO_THREAD,
+	rc = devm_request_irq(dev, irq, cxl_pmu_irq,
+			      IRQF_SHARED | IRQF_NO_THREAD | IRQF_NOBALANCING,
 			      irq_name, info);
 	if (rc)
 		return rc;
-- 
2.55.0

Re: [PATCH v2 5/9] perf/cxl: Keep the overflow interrupt pinned to the managed CPU

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Modifying the affinity of a shared IRQ line during CPU hotplug violates local64_t locking rules.
- [High] Race condition during CPU hotplug migration violates local64_t locking rules.
--

--- Patch [5]: [PATCH v2 5/9] perf/cxl: Keep the overflow interrupt pinned to the managed CPU ---
commit d9c09f0087580638fe392e6b190d53e0657bbc87
Author: Dave Jiang <dave.jiang@intel.com>

perf/cxl: Keep the overflow interrupt pinned to the managed CPU

The PMU driver pins its overflow interrupt to the managing CPU, but requests
it with IRQF_SHARED without IRQF_NOBALANCING. This allows irqbalance to
move the interrupt, leading to concurrent cross-CPU access of local64_t
event counts. This commit adds IRQF_NOBALANCING to ensure the CPU hotplug
pinning holds.

Link: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index d1e810601e363..8b89db8f4d689 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -888,7 +888,8 @@ static int cxl_pmu_probe(struct device *dev)
>  	if (!irq_name)
>  		return -ENOMEM;
>  
> -	rc = devm_request_irq(dev, irq, cxl_pmu_irq, IRQF_SHARED | IRQF_NO_THREAD,
> +	rc = devm_request_irq(dev, irq, cxl_pmu_irq,
> +			      IRQF_SHARED | IRQF_NO_THREAD | IRQF_NOBALANCING,
>  			      irq_name, info);

[Severity: High]
This isn't a bug introduced by this patch, but there appears to be a
pre-existing race condition in the CPU hotplug migration window that violates
local64_t locking rules.

In cxl_pmu_offline_cpu(), perf_pmu_migrate_context() moves the PMU events
to the target CPU and restarts them:

drivers/perf/cxl_pmu.c:cxl_pmu_offline_cpu() {
	...
	perf_pmu_migrate_context(&info->pmu, cpu, target);
	info->on_cpu = target;
	/*
	 * CPU HP lock is held so we should be guaranteed that this CPU hasn't yet
	 * gone away.
	 */
	WARN_ON(irq_set_affinity(info->irq, cpumask_of(target)));
	return 0;
}

Since perf_pmu_migrate_context() re-enables hardware interrupts, and
irq_set_affinity() is called after this migration, an interrupt firing
during this window will still be delivered to the old offlining CPU.

Concurrently, user-space reads of the migrated event will execute on the
new target CPU.

Could this lead to cross-CPU concurrent accesses to the local64_t event
counts, corrupting the data?

[Severity: High]
This is a pre-existing issue, but modifying the affinity of a shared IRQ
line during CPU hotplug appears to also violate local64_t locking rules.

Looking at the same cxl_pmu_offline_cpu() snippet above, the driver
registers its interrupt with IRQF_SHARED. Because affinity is a per-IRQ
property, calling irq_set_affinity() moves the interrupt for all PMU
instances sharing this IRQ line.

If another PMU instance is still managed by the old CPU (e.g. its offline
callback hasn't run yet), its interrupts will now fire on the new target CPU
while user-space reads concurrently execute on the old CPU.

Is there a risk that this cross-CPU access corrupts the local64_t counts?

>  	if (rc)
>  		return rc;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729145555.3919550-1-dave.jiang@intel.com?part=5

[PATCH v2 6/9] perf/cxl: Unfreeze counters after handling an overflow interrupt

[Dave Jiang]

The counters are configured with Freeze on Overflow, so when any counter
overflows the CPMU freezes every counter in the block (CXL r4.0
§8.2.7.2.1). cxl_pmu_irq() reads the overflowed counters and clears the
overflow status, but never writes the CPMU Freeze register to unfreeze,
so all counters stay frozen until the next pmu_enable() and events in
that window are silently lost.

Unfreeze after clearing the overflow status so counting resumes.

Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/perf/cxl_pmu.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 8b89db8f4d68..52e78a6e0960 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -804,6 +804,15 @@ static irqreturn_t cxl_pmu_irq(int irq, void *data)
 
 	writeq(overflowed, base + CXL_PMU_OVERFLOW_REG);
 
+	/*
+	 * Counters are configured to freeze on overflow (Freeze on Overflow),
+	 * which freezes every counter in the CPMU. Once the overflowed counters
+	 * have been read and their status cleared, unfreeze so counting resumes;
+	 * otherwise all counters stay frozen until the next pmu_enable() and
+	 * events are silently lost.
+	 */
+	writeq(0, base + CXL_PMU_FREEZE_REG);
+
 	return IRQ_HANDLED;
 }
 
-- 
2.55.0

[PATCH v2 7/9] perf/cxl: Validate the hardware-reported counter width

[Dave Jiang]

cxl_pmu_parse_caps() takes the Counter Width straight from the CPMU
Capability register with no bounds check. The Counter Data register is
64 bits, so a device reporting width 0 (or > 64) is invalid, and the
read path's GENMASK_ULL(counter_width - 1, 0) would shift out of range
(undefined behavior, e.g. a UBSAN splat) on a value of 0.

Reject a counter width outside 1..64 at probe.

Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/perf/cxl_pmu.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 52e78a6e0960..2e817a52ff1e 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -139,6 +139,15 @@ static int cxl_pmu_parse_caps(struct device *dev, struct cxl_pmu_info *info)
 
 	info->num_counters = FIELD_GET(CXL_PMU_CAP_NUM_COUNTERS_MSK, val) + 1;
 	info->counter_width = FIELD_GET(CXL_PMU_CAP_COUNTER_WIDTH_MSK, val);
+	/*
+	 * The Counter Data register is 64 bits wide, so a Counter Width of 0 or
+	 * >64 is invalid. Reject it rather than let GENMASK_ULL(width - 1, 0) in
+	 * the read path shift out of range.
+	 */
+	if (info->counter_width == 0 || info->counter_width > 64) {
+		dev_err(dev, "Invalid counter width %d\n", info->counter_width);
+		return -ENODEV;
+	}
 	info->num_event_capabilities = FIELD_GET(CXL_PMU_CAP_NUM_EVN_CAP_REG_SUP_MSK, val) + 1;
 
 	info->filter_hdm = FIELD_GET(CXL_PMU_CAP_FILTERS_SUP_MSK, val) & CXL_PMU_FILTER_HDM;
-- 
2.55.0

Re: [PATCH v2 7/9] perf/cxl: Validate the hardware-reported counter width

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds MMIO write via `writeq()` overwriting the adjacent 32-bit filter register.
--

--- Patch [7]: [PATCH v2 7/9] perf/cxl: Validate the hardware-reported counter width ---
commit 5420efbe1c059e0ba51ccd9a185a7ae26d9866db
Author: Dave Jiang <dave.jiang@intel.com>

perf/cxl: Validate the hardware-reported counter width

cxl_pmu_parse_caps() takes the Counter Width straight from the CPMU Capability
register with no bounds check. A device reporting width 0 or > 64 is invalid,
so reject a counter width outside 1..64 at probe.

Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 52e78a6e09603..2e817a52ff1e1 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential
out-of-bounds MMIO write in cxl_pmu_event_start() in this same file:

    writeq(cfg, base + CXL_PMU_FILTER_CFG_REG(hwc->idx, 0));

Can writing 64 bits using writeq to a 32-bit MMIO register cause an
Unsupported Request (UR) PCIe error on devices that only support 32-bit
access to this region?

If the hardware does happen to accept the 64-bit write, will this silently
overwrite the adjacent 32-bit filter register with zeroes, corrupting its
state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729145555.3919550-1-dave.jiang@intel.com?part=7

[PATCH v2 8/9] perf/cxl: Don't use pmu.dev in IRQ and hotplug callbacks after unregister

[Dave Jiang]

On device removal the devm actions unwind LIFO, so cxl_pmu_perf_unregister()
runs first and perf_pmu_unregister() frees info->pmu.dev (device_del() +
put_device() -> kfree()). The overflow IRQ (freed last) and the CPU-hotplug
instance (removed next) are still live at that point, and both
cxl_pmu_irq() and cxl_pmu_offline_cpu() log via dev_dbg()/dev_err() on
info->pmu.dev, dereferencing freed memory. The shared IRQ can be entered
for a co-function on the same MSI vector, and a CPU can go offline in the
window before the hotplug instance is removed.

Log through info->pmu.parent instead, the cxl_pmu device passed to probe,
which is devm-managed and outlives every teardown action.

Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/perf/cxl_pmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 2e817a52ff1e..f42238b2b6b0 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -803,7 +803,7 @@ static irqreturn_t cxl_pmu_irq(int irq, void *data)
 		struct perf_event *event = info->hw_events[i];
 
 		if (!event) {
-			dev_dbg(info->pmu.dev,
+			dev_dbg(info->pmu.parent,
 				"overflow but on non enabled counter %d\n", i);
 			continue;
 		}
@@ -966,7 +966,7 @@ static int cxl_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
 	info->on_cpu = -1;
 	target = cpumask_any_but(cpu_online_mask, cpu);
 	if (target >= nr_cpu_ids) {
-		dev_err(info->pmu.dev, "Unable to find a suitable CPU\n");
+		dev_err(info->pmu.parent, "Unable to find a suitable CPU\n");
 		return 0;
 	}
 
-- 
2.55.0

Re: [PATCH v2 8/9] perf/cxl: Don't use pmu.dev in IRQ and hotplug callbacks after unregister

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch correctly fixes a UAF in logging statements during teardown but leaves a critical lifecycle race unaddressed. The IRQ is registered via `devm_request_irq` before `cxl_pmu_perf_unregister` is registered. On device removal, `devm` unwinds LIFO, so `perf_pmu_unregister` runs while the shared IRQ is still live. If the IRQ fires concurrently on another CPU (e.g., due to a co-function changing the shared MSI vector's affinity), `cxl_pmu_irq` can read a non-NULL `event` right before it is detached, leading to a NULL pointer dereference in `__cxl_pmu_read` when it accesses `event->pmu`.
--

--- Patch [8]: [PATCH v2 8/9] perf/cxl: Don't use pmu.dev in IRQ and hotplug callbacks after unregister ---
commit 34ab4c1ad03386965fbc73a1d534f56456842698
Author: Dave Jiang <dave.jiang@intel.com>

perf/cxl: Don't use pmu.dev in IRQ and hotplug callbacks after unregister

The patch replaces info->pmu.dev with info->pmu.parent in the IRQ and CPU
hotplug callbacks to fix a use-after-free during device removal.

Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 2e817a52ff1e1..f42238b2b6b0d 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -803,7 +803,7 @@ static irqreturn_t cxl_pmu_irq(int irq, void *data)
>  		struct perf_event *event = info->hw_events[i];
>  
>  		if (!event) {
> -			dev_dbg(info->pmu.dev,
> +			dev_dbg(info->pmu.parent,
>  				"overflow but on non enabled counter %d\n", i);
>  			continue;
>  		}

[Severity: High]
This is a pre-existing issue, but since this patch addresses teardown races,
does it leave a critical lifecycle race unaddressed here?

Because the IRQ is registered via devm before cxl_pmu_perf_unregister() is
registered, the LIFO unwinding order means perf_pmu_unregister() runs while
the shared IRQ is still live.

If a shared MSI interrupt fires concurrently with cxl_pmu_perf_unregister(),
cxl_pmu_irq() can read a valid event right before it is detached:

cxl_pmu_irq() {
        ...
        struct perf_event *event = info->hw_events[i];
        ...
        __cxl_pmu_read(event, true);
}

Meanwhile on another CPU, the unregister thread detaches the events and
synchronously sets event->pmu = NULL. When the IRQ handler calls
__cxl_pmu_read(), it dereferences event->pmu:

__cxl_pmu_read() {
        struct cxl_pmu_info *info = pmu_to_cxl_pmu_info(event->pmu);
        ...
}

Could this lead to a null pointer dereference and a kernel panic when
info->base is subsequently accessed during device unbind or module removal?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729145555.3919550-1-dave.jiang@intel.com?part=8

[PATCH v2 9/9] perf/cxl: Avoid cpumask_of(-1) when no CPU is assigned

[Dave Jiang]

cpumask_show() feeds info->on_cpu straight into cpumask_of() for the
world-readable cpumask sysfs attribute. on_cpu is -1 before the first
hotplug online callback and transiently in cxl_pmu_offline_cpu() before a
new target is chosen. cpumask_of(-1) treats the CPU number as unsigned and
does out-of-bounds pointer arithmetic in get_cpu_mask(), so a concurrent
read of the attribute dereferences a wild pointer and can fault -- a local
denial of service.

Read on_cpu once and emit an empty mask when it is negative.

Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/perf/cxl_pmu.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index f42238b2b6b0..6aad381c0376 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -501,8 +501,17 @@ static ssize_t cpumask_show(struct device *dev, struct device_attribute *attr,
 			    char *buf)
 {
 	struct cxl_pmu_info *info = dev_get_drvdata(dev);
+	int cpu = READ_ONCE(info->on_cpu);
 
-	return cpumap_print_to_pagebuf(true, buf, cpumask_of(info->on_cpu));
+	/*
+	 * on_cpu is -1 before the first online callback and transiently during
+	 * cxl_pmu_offline_cpu(). cpumask_of(-1) computes an out-of-bounds
+	 * pointer, so report an empty mask instead.
+	 */
+	if (cpu < 0)
+		return sysfs_emit(buf, "\n");
+
+	return cpumap_print_to_pagebuf(true, buf, cpumask_of(cpu));
 }
 static DEVICE_ATTR_RO(cpumask);
 
-- 
2.55.0

Re: [PATCH v2 9/9] perf/cxl: Avoid cpumask_of(-1) when no CPU is assigned

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but missing validation in cxl_pmu_event_init() leads to a severe out-of-bounds memory access via event->cpu.
--

--- Patch [9]: [PATCH v2 9/9] perf/cxl: Avoid cpumask_of(-1) when no CPU is assigned ---
commit a8e317fb50ca3d5fbb66719c1a84b3a5f9c785fc
Author: Dave Jiang <dave.jiang@intel.com>

perf/cxl: Avoid cpumask_of(-1) when no CPU is assigned

This commit fixes an out-of-bounds pointer arithmetic issue in cpumask_show()
caused by cpumask_of(-1) when info->on_cpu is transiently -1 during CPU
hotplug. It reads on_cpu once and emits an empty mask if it is negative.

Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -501,8 +501,17 @@ static ssize_t cpumask_show(struct device *dev, struct device_attribute *attr,
>  			    char *buf)
>  {
>  	struct cxl_pmu_info *info = dev_get_drvdata(dev);
> +	int cpu = READ_ONCE(info->on_cpu);
>  
> -	return cpumap_print_to_pagebuf(true, buf, cpumask_of(info->on_cpu));
> +	/*
> +	 * on_cpu is -1 before the first online callback and transiently during
> +	 * cxl_pmu_offline_cpu(). cpumask_of(-1) computes an out-of-bounds
> +	 * pointer, so report an empty mask instead.
> +	 */

[Severity: High]
This problem wasn't introduced by this patch, but since we are handling the
transient -1 state of info->on_cpu here, could the same state cause an
out-of-bounds memory access in cxl_pmu_event_init()?

If perf_event_open() is called concurrently while a CPU hotplug offline
operation (cxl_pmu_offline_cpu()) is temporarily setting info->on_cpu = -1,
the event initialization seems to blindly copy this value:

drivers/perf/cxl_pmu.c:cxl_pmu_event_init() {
    ...
	event->cpu = info->on_cpu;
    ...
}

The core perf subsystem then appears to dereference this -1 CPU index as an
array offset:

kernel/events/core.c:find_get_context() {
    ...
		cpuctx = per_cpu_ptr(&perf_cpu_context, event->cpu);
		ctx = &cpuctx->ctx;
    ...
}

Does passing -1 here cause an out-of-bounds array read that results in a
wild pointer to cpuctx? It looks like the kernel then immediately attempts
to take a spinlock on this wild pointer, which might lead to memory
corruption. Is additional validation needed in cxl_pmu_event_init() to
prevent this?

> +	if (cpu < 0)
> +		return sysfs_emit(buf, "\n");
> +
> +	return cpumap_print_to_pagebuf(true, buf, cpumask_of(cpu));
>  }
>  static DEVICE_ATTR_RO(cpumask);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729145555.3919550-1-dave.jiang@intel.com?part=9