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

[PATCH 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.

[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: Drop bogus counter overflow 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 | 59 ++++++++++++++++++++++++++++++------------
 1 file changed, 43 insertions(+), 16 deletions(-)


base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
-- 
2.55.0

[PATCH 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 1/9] perf/cxl: Program the requested event group on configurable counters

[sashiko-bot]

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

Pre-existing issues:
- [High] In cxl_pmu_event_start(), configuration bitfields are bitwise-ORed with the existing register value without being cleared first, leading to corrupted register state.
--

--- Patch [1]: [PATCH 1/9] perf/cxl: Program the requested event group on configurable counters ---
commit a96f996812668f5b85e51cf8c46d25fbd3781fce
Author: Dave Jiang <dave.jiang@intel.com>

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

cxl_pmu_get_event_idx() sets the counter index for a configurable counter but
never sets the event index, causing the event base to stay 0. This change
returns the matched capability's event index to correctly program the group.

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
> --- 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 is a pre-existing issue, but does assigning non-zero event indices here
expose a bit accumulation bug in cxl_pmu_event_start()?

When the event starts, cxl_pmu_event_start() updates the configuration
register using a bitwise-OR against the existing value:

drivers/perf/cxl_pmu.c:cxl_pmu_event_start() {
    cfg = readq(base + CXL_PMU_COUNTER_CFG_REG(hwc->idx));
    ...
    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));
    }
    ...
}

Since cxl_pmu_event_stop() only clears the ENABLE and INT_ON_OVRFLW fields,
the old group ID bits remain in the register. Because the event base is
now correctly populated by this patch instead of remaining 0, won't
consecutive events reusing the same counter cause EVENT_GRP_ID_IDX_MSK
and other fields to accumulate stale bits from previous events?

Could this lead to corrupted hardware state over time, and should the
relevant register fields be explicitly cleared before applying the new
configuration?

>  		return 0;
>  	}

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

[PATCH 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 2/9] perf/cxl: Clear stale event fields before reprogramming a counter

[sashiko-bot]

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

Pre-existing issues:
- [High] Incorrect overflow handling in __cxl_pmu_read() double-counts events and invokes undefined behavior via an out-of-bounds shift.
--

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

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

This commit clears stale event fields from the configuration register
before reprogramming a counter in the CXL PMU driver. This ensures
the counter does not count the wrong events with old edge or invert
behavior when reused.

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);

[ ... ]

> @@ -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);

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential problem
in how overflows are handled in __cxl_pmu_read() in this same file:

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);
    ...
}

Does this double-count the overflow events? Since the subtraction is already
masked by GENMASK_ULL(), the wraparound is correctly accounted for in delta.
Adding (1UL << info->counter_width) appears redundant and might corrupt the
resulting metrics.

Additionally, does this shift operation risk undefined behavior? If
info->counter_width is 64 (the maximum width allowed by the CXL 3.0
specification) on 64-bit systems, or 32 or greater on 32-bit systems,
shifting 1UL by that width would cause an out-of-bounds shift.

Is it safe to rely on this delta calculation when counter_width can be large
enough to trigger this undefined behavior?

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

[PATCH 3/9] perf/cxl: Drop bogus counter overflow fixup

[Dave Jiang]

On the overflow path __cxl_pmu_read() adds a full counter period to the
delta: `delta += (1UL << counter_width)` when `delta < mask`. The masked
unsigned subtraction already gives the correct delta across a wrap, so
this over-counts by ~one period on nearly every overflow (the guard holds
for all but one delta value), and the shift is undefined for counter
widths >= 32 (32-bit kernels) or == 64.

Drop the fixup to match every other perf driver. The overflow argument is
now unused, so fold __cxl_pmu_read() into cxl_pmu_read().

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 | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 3138514157cd..410c9162c824 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -686,7 +686,7 @@ static u64 cxl_pmu_read_counter(struct perf_event *event)
 	return readq(base + CXL_PMU_COUNTER_REG(event->hw.idx));
 }
 
-static void __cxl_pmu_read(struct perf_event *event, bool overflow)
+static void cxl_pmu_read(struct perf_event *event)
 {
 	struct cxl_pmu_info *info = pmu_to_cxl_pmu_info(event->pmu);
 	struct hw_perf_event *hwc = &event->hw;
@@ -698,21 +698,15 @@ 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 is not reset on overflow, and the unsigned subtraction
+	 * masked to the counter width already yields the correct delta across a
+	 * single wrap, so no overflow fixup is needed.
 	 */
 	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);
 
 	local64_add(delta, &event->count);
 }
 
-static void cxl_pmu_read(struct perf_event *event)
-{
-	__cxl_pmu_read(event, false);
-}
-
 static void cxl_pmu_event_stop(struct perf_event *event, int flags)
 {
 	struct cxl_pmu_info *info = pmu_to_cxl_pmu_info(event->pmu);
@@ -793,7 +787,7 @@ static irqreturn_t cxl_pmu_irq(int irq, void *data)
 			continue;
 		}
 
-		__cxl_pmu_read(event, true);
+		cxl_pmu_read(event);
 	}
 
 	writeq(overflowed, base + CXL_PMU_OVERFLOW_REG);
-- 
2.55.0

Re: [PATCH 3/9] perf/cxl: Drop bogus counter overflow fixup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Removing the overflow fixup entirely causes the CXL PMU driver to silently drop a full period of events on overflow when the new counter skid is greater than or equal to the previous skid.
--

--- Patch [3]: [PATCH 3/9] perf/cxl: Drop bogus counter overflow fixup ---
commit 9b5168bc2e13d25590b1bb314d5cd8b527ba4f3e
Author: Dave Jiang <dave.jiang@intel.com>

perf/cxl: Drop bogus counter overflow fixup

This commit removes the overflow fixup in __cxl_pmu_read() and folds it into
cxl_pmu_read(). The stated rationale is that masked unsigned subtraction
already produces the correct delta across a wrap, and that the previous
fixup caused overcounting.

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..410c9162c8246 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
[ ... ]
> @@ -698,21 +698,15 @@ 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 is not reset on overflow, and the unsigned subtraction
> +	 * masked to the counter width already yields the correct delta across a
> +	 * single wrap, so no overflow fixup is needed.
>  	 */
>  	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);

[Severity: Medium]
Does removing the overflow fixup entirely cause the driver to drop a full
period of events when new_cnt >= prev_cnt?

While standard modulo math handles wraps when the counter is reprogrammed to
a high value (ensuring new_cnt < prev_cnt), the CXL PMU does not reprogram
counters on overflow. They start at 0, wrap, and freeze at a small skid
value.

If a counter overflows, prev_cnt becomes skid1. On the next overflow,
new_cnt becomes skid2. If skid2 >= skid1 (which has a roughly 50% chance),
the masked subtraction only yields the tiny difference (skid2 - skid1),
completely missing the wrapped period.

Would it be more correct to keep an overflow fixup specifically for the
new_cnt >= prev_cnt case when an overflow occurs?

>  
>  	local64_add(delta, &event->count);
>  }

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

Re: [PATCH 3/9] perf/cxl: Drop bogus counter overflow fixup

[Dave Jiang]

On 7/28/26 2:14 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Removing the overflow fixup entirely causes the CXL PMU driver to silently drop a full period of events on overflow when the new counter skid is greater than or equal to the previous skid.

Will fix in v2.

DJ

> --
> 
> --- Patch [3]: [PATCH 3/9] perf/cxl: Drop bogus counter overflow fixup ---
> commit 9b5168bc2e13d25590b1bb314d5cd8b527ba4f3e
> Author: Dave Jiang <dave.jiang@intel.com>
> 
> perf/cxl: Drop bogus counter overflow fixup
> 
> This commit removes the overflow fixup in __cxl_pmu_read() and folds it into
> cxl_pmu_read(). The stated rationale is that masked unsigned subtraction
> already produces the correct delta across a wrap, and that the previous
> fixup caused overcounting.
> 
> 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..410c9162c8246 100644
>> --- a/drivers/perf/cxl_pmu.c
>> +++ b/drivers/perf/cxl_pmu.c
> [ ... ]
>> @@ -698,21 +698,15 @@ 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 is not reset on overflow, and the unsigned subtraction
>> +	 * masked to the counter width already yields the correct delta across a
>> +	 * single wrap, so no overflow fixup is needed.
>>  	 */
>>  	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);
> 
> [Severity: Medium]
> Does removing the overflow fixup entirely cause the driver to drop a full
> period of events when new_cnt >= prev_cnt?
> 
> While standard modulo math handles wraps when the counter is reprogrammed to
> a high value (ensuring new_cnt < prev_cnt), the CXL PMU does not reprogram
> counters on overflow. They start at 0, wrap, and freeze at a small skid
> value.
> 
> If a counter overflows, prev_cnt becomes skid1. On the next overflow,
> new_cnt becomes skid2. If skid2 >= skid1 (which has a roughly 50% chance),
> the masked subtraction only yields the tiny difference (skid2 - skid1),
> completely missing the wrapped period.
> 
> Would it be more correct to keep an overflow fixup specifically for the
> new_cnt >= prev_cnt case when an overflow occurs?
> 
>>  
>>  	local64_add(delta, &event->count);
>>  }
> 

[PATCH 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 410c9162c824..7db858727ac6 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -864,7 +864,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 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 7db858727ac6..0c53f9e5ccb1 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -876,7 +876,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 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] Race window during CPU offline allows interrupts to fire on the old CPU after events have migrated, causing cross-CPU concurrency on local64_t.
- [High] Using IRQF_SHARED with independent per-device CPU hotplug states allows PMU instances sharing an IRQ to drift onto different CPUs, breaking the local64_t same-CPU guarantee.
--

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

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

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.

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>

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 7db858727ac61..0c53f9e5ccb1d 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -876,7 +876,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 is a pre-existing issue, but does using IRQF_SHARED with independent
per-device CPU hotplug states allow PMU instances sharing an IRQ to drift onto
different CPUs?

If multiple PMU instances on a CXL device share the same MSI vector and request
it with IRQF_SHARED, each PMU registers its own independent CPU hotplug
instance.

If an administrator unbinds and rebinds one PMU instance, its
cxl_pmu_online_cpu() callback executes and sets the shared IRQ's affinity to
its newly chosen CPU.

The other PMU instances sharing the IRQ remain affine to their existing on_cpu,
but will now receive interrupts on the new CPU. When the IRQ fires, the kernel
executes cxl_pmu_irq() for all sharing PMUs on the new CPU. This causes
cxl_pmu_read() to execute on the wrong CPU concurrently with userspace reads on
the correct CPU, corrupting the local64_t counters.

While this patch adds IRQF_NOBALANCING to stop external irqbalance, does it
leave the driver vulnerable to its own internal hotplug divergence?

[Severity: High]
This isn't a bug introduced by this patch, but is there a race window during
CPU offline that allows interrupts to fire on the old CPU after events have
migrated?

Looking at 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)));

Does perf_pmu_migrate_context() schedule the events out from the old CPU and
into the new CPU before the driver updates the IRQ affinity?

If an interrupt fires during this window, it would be routed to the old CPU.
cxl_pmu_irq() would execute on the old CPU and call cxl_pmu_read().
Concurrently, an unprivileged user reading the counter forces cxl_pmu_read() to
execute on the new CPU via IPI, corrupting the local64_t counters.

>  	if (rc)
>  		return rc;

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

[PATCH 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 0c53f9e5ccb1..e13811cf0c81 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -792,6 +792,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 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 e13811cf0c81..d1fa97f77e25 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

[PATCH 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 d1fa97f77e25..f1110c5029d6 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -791,7 +791,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;
 		}
@@ -954,7 +954,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

[PATCH 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 f1110c5029d6..80145e85fe36 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 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] The patch fixes an out-of-bounds pointer dereference in `cpumask_show()` caused by `info->on_cpu == -1`, but fails to fix the exact same condition in `cxl_pmu_event_init()`. This omission allows `event->cpu` to become `-1`, leading to an out-of-bounds array read and a wild pointer dereference in the perf core.
--

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

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

This patch fixes an out-of-bounds pointer dereference in cpumask_show()
by checking if info->on_cpu is -1. When no CPU is assigned, it emits
an empty mask instead of performing invalid pointer arithmetic.

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 f1110c5029d6a..80145e85fe36b 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));
>  }

[Severity: High]
This is a pre-existing issue, but since info->on_cpu can be -1 during
cxl_pmu_offline_cpu(), does this same out-of-bounds issue happen in
cxl_pmu_event_init()?

cxl_pmu_event_init() unconditionally assigns info->on_cpu to event->cpu:

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

If a user calls perf_event_open() concurrently with cxl_pmu_offline_cpu(),
this bypasses the perf core's initial validation of the cpu argument.

The perf core will then use the -1 value:

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

Could this lead to an out-of-bounds array read and wild pointer dereference
in the perf core? Should cxl_pmu_event_init() also validate that
info->on_cpu is not -1 before assignment?

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