[PATCH v4 00/11] perf/cxlpmu: Misc sashiko raised issues fixes

[PATCH v4 00/11] 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.

Since v3:
- 4/11 and 5/11 swapped, so the one-line MSI vector 0 fix comes before the
  info->msi_vec rename and backports on its own (Robin).
- 6/11: new patch, add the PMUs after configuring events.
- 7/11: no code change; the commit message no longer overstates what
  dropping IRQF_SHARED costs a device that shares a vector.
- 8/11: quote the spec sentence that makes the freeze sticky, and correct
  the claim about what the migration window leaves enabled (Richard,
  sashiko-bot).
- 10/11: split the probe-time overflow clear out, so this is back to the
  single hunk Jonathan acked.
- 11/11: new patch. Clear a counter's stale overflow status when starting
  an event, which is where the reuse case bites; the probe clear moved
  here from 10/11 (Richard).

Since v2:
- 1/11: check event_idx alongside counter_idx (Jonathan).
- 2/11: use FIELD_MODIFY() rather than mask-then-OR (Jonathan).
- 3/11: rewrite the rationale, no code change (Jonathan).
- 5/11: new patch, split the MSI vector out of info->irq (Robin, Jonathan).
- 7/11: drop IRQF_SHARED as well as adding IRQF_NOBALANCING, and retitle
  (Jonathan, Robin).
- 8/11: don't unfreeze if the PMU has been disabled meanwhile (Robin,
  Jonathan).
- 10/11: retitle, and drop the cxl_pmu_offline_cpu() hunk since that
  dev_err() cannot be reached (Robin).
- Dropped the old 9/9 that guarded cpumask_show() against cpumask_of(-1).
  The init case was a false positive and neither reviewer liked the shape
  of it (Jonathan, Robin).

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

Three follow-ups this series does not attempt:

cxl_pmu_read() relies on local64_t being same-CPU, which is the only reason
the interrupt has to be pinned at all. Moving prev_count and event->count
off local64_t would make the affinity question moot, and Robin may have a
view on whether that is worth doing.

cxl_pmu_offline_cpu() still leaves on_cpu at -1 across a sleeping
perf_pmu_migrate_context(), and still has an unreachable no-target branch.
Both want removing, possibly on top of the generic PMU hotplug rework
Jonathan pointed at [2].

Firmware can leave a counter enabled with Global Freeze on Overflow but not
Interrupt on Overflow, which stalls the whole CPMU with no interrupt to say
so. Quiescing the block at probe needs two writes per counter to stay
within the spec's rule about changing an enabled counter, so it belongs in
its own patch.

[1]: https://lore.kernel.org/linux-cxl/20260715191454.459673-1-dave@stgolabs.net/
[2]: https://lore.kernel.org/linux-arm-kernel/cover.1784911757.git.robin.murphy@arm.com/

Dave Jiang (11):
  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: Split the MSI vector out of info->irq
  cxl/pci: Add the PMUs after configuring events
  perf/cxl: Don't share the overflow interrupt, and keep it pinned
  perf/cxl: Unfreeze counters after handling an overflow interrupt
  perf/cxl: Validate the hardware-reported counter width
  perf/cxl: Don't log through pmu.dev in the overflow interrupt handler
  perf/cxl: Clear stale overflow status before using a counter

 drivers/cxl/pci.c      |  11 +++--
 drivers/perf/cxl_pmu.c | 107 ++++++++++++++++++++++++++++++++---------
 2 files changed, 90 insertions(+), 28 deletions(-)


base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
-- 
2.54.0

[PATCH v4 01/11] 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 *event_idx. hwc->event_base stays 0, so cxl_pmu_event_start()
programs event group 0 no matter which group the user asked for.

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
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/perf/cxl_pmu.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 68a54d97d2a8..ff01b658e1b4 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -513,7 +513,10 @@ static const struct attribute_group *cxl_pmu_attr_groups[] = {
 	NULL
 };
 
-/* If counter_idx == NULL, don't try to allocate a counter. */
+/*
+ * If counter_idx == NULL, don't try to allocate a counter. Callers that
+ * allocate pass both counter_idx and event_idx.
+ */
 static int cxl_pmu_get_event_idx(struct perf_event *event, int *counter_idx,
 				 int *event_idx)
 {
@@ -541,7 +544,7 @@ static int cxl_pmu_get_event_idx(struct perf_event *event, int *counter_idx,
 
 	pmu_ev = cxl_pmu_find_config_counter_ev_cap(info, vid, gid, mask);
 	if (!IS_ERR(pmu_ev)) {
-		if (!counter_idx)
+		if (!counter_idx || !event_idx)
 			return 0;
 
 		bitmap_andnot(configurable_and_free, info->conf_counter_bm,
@@ -552,6 +555,7 @@ static int cxl_pmu_get_event_idx(struct perf_event *event, int *counter_idx,
 			return -EINVAL;
 
 		*counter_idx = i;
+		*event_idx = pmu_ev->event_idx;
 		return 0;
 	}
 
-- 
2.54.0

[PATCH v4 02/11] 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
them first, and cxl_pmu_event_stop() leaves them set. Reuse the counter for
another event and the new selection lands on top of the old one, so the
counter counts the wrong events with stale edge and invert behaviour.

Use FIELD_MODIFY() so each field is replaced rather than accumulated.

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, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index ff01b658e1b4..b16e2e4090a3 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -646,17 +646,17 @@ static void cxl_pmu_event_start(struct perf_event *event, int flags)
 	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);
-	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_EDGE,
-			  cxl_pmu_config1_get_edge(event) ? 1 : 0);
-	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_INVERT,
-			  cxl_pmu_config1_get_invert(event) ? 1 : 0);
+	FIELD_MODIFY(CXL_PMU_COUNTER_CFG_EDGE, &cfg,
+		     cxl_pmu_config1_get_edge(event) ? 1 : 0);
+	FIELD_MODIFY(CXL_PMU_COUNTER_CFG_INVERT, &cfg,
+		     cxl_pmu_config1_get_invert(event) ? 1 : 0);
 
 	/* Fixed purpose counters have next two fields RO */
 	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));
+		FIELD_MODIFY(CXL_PMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK, &cfg,
+			     hwc->event_base);
+		FIELD_MODIFY(CXL_PMU_COUNTER_CFG_EVENTS_MSK, &cfg,
+			     cxl_pmu_config_get_mask(event));
 	}
 	cfg &= ~CXL_PMU_COUNTER_CFG_THRESHOLD_MSK;
 	/*
-- 
2.54.0

[PATCH v4 03/11] perf/cxl: Fix the counter overflow delta fixup

[Dave Jiang]

The counter is masked to counter_width, so the subtraction in
__cxl_pmu_read() throws away the bit that records the wrap. A delta of one
whole period reads back as 0, the same as no events at all, and only the
overflow status tells the two apart - which is why __cxl_pmu_read() takes
an overflow argument.

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

so it cannot tell which of these it is looking at:

  event_start         polled read          wrap
  ctr = 0 ..........  ctr = P/2 .........  mask -> 0 (+r)
  prev = 0            prev = P/2                 IRQ reads new = r

  prev = 0    (no read yet):  new >= prev, fell short -> add period
  prev = P/2  (polled):       new <  prev, spans wrap -> add nothing

Both rows are the same interrupt and the old guard adds a period in both,
so a mid-period read makes the event over-count. 'perf stat -I' hits that.

Condition the fixup on new_cnt >= prev_cnt, the one case the subtraction
cannot express. Dropping it outright would break the first row. It stays
exact whatever the residual r is, which matters because some events
increment by more than 1 per cycle (CXL r4.0 8.2.7.2.1, Threshold) and can
step past 0 as they wrap.

Use mask + 1 for the period rather than a shift. It is 0 for a 64-bit
counter, and avoids the old 1UL << counter_width - undefined for width 64,
and for >= 32 on 32-bit kernels.

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, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index b16e2e4090a3..40741e529d9b 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -689,7 +689,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);
@@ -697,12 +697,16 @@ 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 mask discards the bit that says the counter wrapped, so a delta of
+	 * one whole period reads back as 0 - the same as no events at all. Only
+	 * the overflow status separates them, and new_cnt >= prev_cnt is that
+	 * case, so add the period back. mask + 1 is 2^counter_width, which comes
+	 * out as 0 for a 64-bit counter and avoids an 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.54.0

[PATCH v4 04/11] perf/cxl: Accept an overflow interrupt on MSI message number 0

[Dave Jiang]

cxl_pmu_probe() rejects the PMU when info->irq <= 0, but at that point the
field still holds the MSI/MSI-X message number the device signals overflow
on, not a Linux virq. That number is 0-based, and -1 means 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 fails to probe.

Reject only the no-interrupt case, matching how the CXL mailbox and event
interrupts handle it.

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
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Acked-by: Richard Cheng <icheng@nvidia.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- Move ahead of the info->msi_vec rename so this one-liner stands alone for
  backporting (Robin).
---
 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 40741e529d9b..2bebbbe67a0a 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -873,7 +873,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.54.0

[PATCH v4 05/11] perf/cxl: Split the MSI vector out of info->irq

[Dave Jiang]

info->irq means two different things depending on how far probe has got.
cxl_pmu_parse_caps() sets it to the MSI/MSI-X message number from the CPMU
Capability register. cxl_pmu_probe() passes that to pci_irq_vector() and
then overwrites it with the Linux virq, which is what the hotplug callbacks
hand to irq_set_affinity().

Both are ints called irq, so nothing catches a mix-up, and the valid ranges
differ. 0 is a fine message number but never a valid virq.

Give the message number its own field. No functional change.

Assisted-by: Claude:claude-opus-4-8
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- Move after the one-line fix so that fix has no dependency on this rename
  (Robin).
---
 drivers/perf/cxl_pmu.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 2bebbbe67a0a..481d32d0c1b0 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -106,6 +106,7 @@ struct cxl_pmu_info {
 	int on_cpu;
 	struct hlist_node node;
 	bool filter_hdm;
+	int msi_vec;
 	int irq;
 };
 
@@ -143,9 +144,9 @@ static int cxl_pmu_parse_caps(struct device *dev, struct cxl_pmu_info *info)
 
 	info->filter_hdm = FIELD_GET(CXL_PMU_CAP_FILTERS_SUP_MSK, val) & CXL_PMU_FILTER_HDM;
 	if (FIELD_GET(CXL_PMU_CAP_INT, val))
-		info->irq = FIELD_GET(CXL_PMU_CAP_MSI_N_MSK, val);
+		info->msi_vec = FIELD_GET(CXL_PMU_CAP_MSI_N_MSK, val);
 	else
-		info->irq = -1;
+		info->msi_vec = -1;
 
 	/* First handle fixed function counters; note if configurable counters found */
 	for (i = 0; i < info->num_counters; i++) {
@@ -873,10 +874,10 @@ static int cxl_pmu_probe(struct device *dev)
 		.capabilities = PERF_PMU_CAP_NO_EXCLUDE,
 	};
 
-	if (info->irq < 0)
+	if (info->msi_vec < 0)
 		return -EINVAL;
 
-	rc = pci_irq_vector(pdev, info->irq);
+	rc = pci_irq_vector(pdev, info->msi_vec);
 	if (rc < 0)
 		return rc;
 	irq = rc;
-- 
2.54.0

[PATCH v4 06/11] cxl/pci: Add the PMUs after configuring events

[Dave Jiang]

cxl_pci_probe() adds the CPMU instances before configuring the event
interrupts. Both take an MSI/MSI-X vector the device names in its own
capability registers, and the spec lets a device point several capabilities
at one message number, so the two can collide.

They do not fail the same way. A failed devm_cxl_pmu_add() only breaks out
of the loop, while a failed cxl_event_config() aborts probe and the memory
device never appears. Going PMUs-first therefore lets a peripheral facility
take the vector and deny it to one that matters.

Configure events first, so a collision costs the PMU rather than the
memdev. Return 0 explicitly afterwards, since rc still holds any PMU error
that cxl_event_config() used to overwrite on the way past.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- New patch, ahead of the PMU dropping IRQF_SHARED. Without this that
  change can turn a shared vector into a probe failure for the whole
  memdev rather than a missing PMU (sashiko-bot).
---
 drivers/cxl/pci.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 267c679b0b3c..975d32716762 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -894,6 +894,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		dev_dbg(&pdev->dev, "No CXL FWCTL setup\n");
 
+	rc = cxl_event_config(host_bridge, mds, irq_avail);
+	if (rc)
+		return rc;
+
 	pmu_count = cxl_count_regblock(pdev, CXL_REGLOC_RBI_PMU);
 	if (pmu_count < 0)
 		return pmu_count;
@@ -920,13 +924,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		}
 	}
 
-	rc = cxl_event_config(host_bridge, mds, irq_avail);
-	if (rc)
-		return rc;
-
 	pci_save_state(pdev);
 
-	return rc;
+	/* A missing PMU is not fatal, the memdev is still usable */
+	return 0;
 }
 
 static const struct pci_device_id cxl_mem_pci_tbl[] = {
-- 
2.54.0

[PATCH v4 07/11] perf/cxl: Don't share the overflow interrupt, and keep it pinned

[Dave Jiang]

The PMU pins its overflow interrupt to info->on_cpu in the hotplug
callbacks, but requests it with only IRQF_SHARED | IRQF_NO_THREAD. Without
IRQF_NOBALANCING, irqbalance or a userspace smp_affinity write can move it
elsewhere, and cxl_pmu_irq() then runs local64_cmpxchg() and local64_add()
on hwc->prev_count and event->count at the same time as the managing CPU.
local64_t is only atomic against same-CPU access, so the counts corrupt.

The flag alone does not fix that while the line is shared. __setup_irq()
only acts on it for the first action on a line, and a co-owner has no
reason to want our affinity - it would keep taking the interrupt wherever
its own points, running our handler on the wrong CPU.

Drop IRQF_SHARED and add IRQF_NOBALANCING. The spec only recommends a
distinct Interrupt Message Number per CPMU instance (CXL r4.0 8.2.7.1.1),
so a device may put several on one vector, or share with the mailbox or
event log. Such a device now loses a PMU rather than silently miscounting -
and only the PMU, since the preceding patch adds the CPMUs after the event
configuration.

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
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/perf/cxl_pmu.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 481d32d0c1b0..448e1da3d59f 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -783,7 +783,7 @@ static irqreturn_t cxl_pmu_irq(int irq, void *data)
 
 	overflowed = readq(base + CXL_PMU_OVERFLOW_REG);
 
-	/* Interrupt may be shared, so maybe it isn't ours */
+	/* Nothing overflowed, so the device did not raise this */
 	if (!overflowed)
 		return IRQ_NONE;
 
@@ -886,7 +886,14 @@ 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,
+	/*
+	 * The handler must run on info->on_cpu, so the interrupt cannot be
+	 * shared - IRQF_NOBALANCING is only honoured for the first action on a
+	 * line, and a co-owner would keep taking the interrupt wherever its own
+	 * affinity points.
+	 */
+	rc = devm_request_irq(dev, irq, cxl_pmu_irq,
+			      IRQF_NO_THREAD | IRQF_NOBALANCING,
 			      irq_name, info);
 	if (rc)
 		return rc;
-- 
2.54.0

[PATCH v4 08/11] perf/cxl: Unfreeze counters after handling an overflow interrupt

[Dave Jiang]

The counters run with Freeze on Overflow set, so one overflow freezes every
counter in the block (CXL r4.0 8.2.7.2.1), and a frozen counter "remains
frozen until explicitly unfrozen by software" (8.2.7.1.3, Table 8-183).
cxl_pmu_irq() reads the overflowed counters and clears the overflow status
but never unfreezes, so everything stays frozen until the next pmu_enable()
and events in that window are lost.

Unfreeze after clearing the status, unless the PMU has been disabled in the
meantime - cxl_pmu_disable() freezes the block deliberately. Track the
enabled state and leave it frozen in that case.

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
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/perf/cxl_pmu.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 448e1da3d59f..a081fcba6991 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -108,6 +108,8 @@ struct cxl_pmu_info {
 	bool filter_hdm;
 	int msi_vec;
 	int irq;
+	/* Set between pmu_enable() and pmu_disable(), read by the IRQ handler */
+	bool enabled;
 };
 
 #define pmu_to_cxl_pmu_info(_pmu) container_of(_pmu, struct cxl_pmu_info, pmu)
@@ -596,6 +598,7 @@ static void cxl_pmu_enable(struct pmu *pmu)
 	void __iomem *base = info->base;
 
 	/* Can assume frozen at this stage */
+	WRITE_ONCE(info->enabled, true);
 	writeq(0, base + CXL_PMU_FREEZE_REG);
 }
 
@@ -604,6 +607,7 @@ static void cxl_pmu_disable(struct pmu *pmu)
 	struct cxl_pmu_info *info = pmu_to_cxl_pmu_info(pmu);
 	void __iomem *base = info->base;
 
+	WRITE_ONCE(info->enabled, false);
 	/*
 	 * Whilst bits above number of counters are RsvdZ
 	 * they are unlikely to be repurposed given
@@ -802,6 +806,21 @@ static irqreturn_t cxl_pmu_irq(int irq, void *data)
 
 	writeq(overflowed, base + CXL_PMU_OVERFLOW_REG);
 
+	/*
+	 * An overflow freezes every counter in the CPMU, so unfreeze once the
+	 * overflowed ones have been read and their status cleared. Otherwise
+	 * they stay frozen until the next pmu_enable() and events are lost.
+	 *
+	 * Not while the PMU is disabled, so as not to undo an intentional freeze.
+	 * The check is advisory, not exclusive: pmu_disable() normally runs on
+	 * info->on_cpu with interrupts off, where the pinned handler cannot
+	 * preempt it. In the one window where it does not - the migration in
+	 * cxl_pmu_offline_cpu() - the counters are legitimately running again,
+	 * so unfreezing is correct there anyway.
+	 */
+	if (READ_ONCE(info->enabled))
+		writeq(0, base + CXL_PMU_FREEZE_REG);
+
 	return IRQ_HANDLED;
 }
 
-- 
2.54.0

[PATCH v4 09/11] perf/cxl: Validate the hardware-reported counter width

[Dave Jiang]

cxl_pmu_parse_caps() takes the Counter Width straight from the CPMU
Capability register without a bounds check. The Counter Data register is 64
bits wide, so a device reporting 0 or more than 64 is reporting nonsense,
and GENMASK_ULL(width - 1, 0) in the read path then shifts out of range.
That is undefined behaviour, and a UBSAN splat where it is enabled.

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
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
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 a081fcba6991..84ebc7da181d 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -142,6 +142,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.54.0

[PATCH v4 10/11] perf/cxl: Don't log through pmu.dev in the overflow interrupt handler

[Dave Jiang]

perf_pmu_unregister() frees pmu->dev without clearing the pointer, and
cxl_pmu_probe() orders its devm actions so that teardown runs it before
free_irq(). Nothing in between masks the interrupt, and
cxl_pmu_event_stop() leaves the overflow status bit set, so an overflow
taken just before teardown is still delivered and still reaches the
dev_dbg() in the !event branch - by which point info->pmu.dev is freed.

Log through info->pmu.parent instead, 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
Acked-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- Split the probe-time overflow clear out, so this is back to the single
  hunk Jonathan acked in v2.
---
 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 84ebc7da181d..45edef53a983 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -805,7 +805,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;
 		}
-- 
2.54.0

[PATCH v4 11/11] perf/cxl: Clear stale overflow status before using a counter

[Dave Jiang]

cxl_pmu_event_stop() leaves the counter's bit set in the overflow status
register and only cxl_pmu_irq() ever clears it, so a counter handed from
one event to the next starts with the previous owner's overflow pending.
The perf core stops and starts back to back with interrupts off, so an
interrupt latched over the handover lands on the new event. prev_count is 0
by then, so the read path adds a full period: 2^counter_width phantom
counts.

Clear the bit in cxl_pmu_event_start(), before the config write arms
Interrupt on Overflow. Clear the whole register at probe too, for counters
no event ever owns - the driver never touched it, so firmware or a previous
kernel can leave a bit set there.

Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
Reported-by: Richard Cheng <icheng@nvidia.com>
Closes: https://lore.kernel.org/linux-cxl/anAVpO--i5heVfz9@MWDK4CY14F/
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- New patch. The event_start() clear is Richard's; the probe clear was a
  second hunk on the pmu.dev patch in v3.
- Clear before the config write rather than after it, so the question of
  whether arming the interrupt over a set status bit can raise an MSI does
  not arise (sashiko-bot).
---
 drivers/perf/cxl_pmu.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 45edef53a983..3ec8933b04e2 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -683,6 +683,15 @@ static void cxl_pmu_event_start(struct perf_event *event, int flags)
 	 */
 	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_THRESHOLD_MSK,
 			  cxl_pmu_config1_get_threshold(event));
+
+	/*
+	 * Drop any overflow the previous owner of this counter left pending;
+	 * cxl_pmu_event_stop() does not. Otherwise an interrupt latched over
+	 * the handover gets charged to this event. Do it before arming Interrupt
+	 * on Overflow below, and note RW1C leaves the other counters alone.
+	 */
+	writeq(BIT_ULL(hwc->idx), base + CXL_PMU_OVERFLOW_REG);
+
 	writeq(cfg, base + CXL_PMU_COUNTER_CFG_REG(hwc->idx));
 
 	local64_set(&hwc->prev_count, 0);
@@ -914,6 +923,14 @@ static int cxl_pmu_probe(struct device *dev)
 	if (!irq_name)
 		return -ENOMEM;
 
+	/*
+	 * Same for counters no event owns yet: clear whatever firmware or a
+	 * previous kernel left set before the handler goes live. Bits above the
+	 * implemented counters are reserved, so only write those.
+	 */
+	writeq(GENMASK_ULL(info->num_counters - 1, 0),
+	       info->base + CXL_PMU_OVERFLOW_REG);
+
 	/*
 	 * The handler must run on info->on_cpu, so the interrupt cannot be
 	 * shared - IRQF_NOBALANCING is only honoured for the first action on a
-- 
2.54.0

Re: [PATCH v4 04/11] perf/cxl: Accept an overflow interrupt on MSI message number 0

[sashiko-bot]

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

Pre-existing issues:
- [High] Modifying CPU affinity on a shared interrupt line alters the affinity for all devices sharing the line.
--

--- Patch [4]: [PATCH v4 04/11] perf/cxl: Accept an overflow interrupt on MSI message number 0 ---
commit 54fa1d2700a4dd3fe1865a2f2f6744ccbafc3070
Author: Dave Jiang <dave.jiang@intel.com>
Subject: perf/cxl: Accept an overflow interrupt on MSI message number 0

cxl_pmu_probe() rejects the PMU when info->irq <= 0, but at that point the
field still holds the MSI/MSI-X message number the device signals overflow
on, not a Linux virq. That number is 0-based, and -1 means 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 fails to probe.

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 40741e529d9b2..2bebbbe67a0a2 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -873,7 +873,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);

[Severity: High]
This is a pre-existing issue, but I noticed the interrupt is requested as
shared in cxl_pmu_probe() and then its affinity is forcibly modified during
hotplug operations.

The interrupt is requested as a shared interrupt:

	rc = devm_request_irq(dev, irq, cxl_pmu_irq, IRQF_SHARED | IRQF_NO_THREAD,
			      irq_name, info);

But the CPU affinity is later altered directly in cxl_pmu_online_cpu():

	WARN_ON(irq_set_affinity(info->irq, cpumask_of(cpu)));

And similarly in cxl_pmu_offline_cpu():

	WARN_ON(irq_set_affinity(info->irq, cpumask_of(target)));

Does this cause issues for other devices sharing the interrupt line?

Modifying CPU affinity on a shared interrupt alters the affinity for all devices
sharing the line, which could break CPU affinity expectations for other
endpoints and potentially cause performance degradation or incorrect behavior.

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

Re: [PATCH v4 03/11] 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] Unvalidated `counter_width` from the CXL PMU capability register causes undefined behavior in `GENMASK_ULL`.
--

--- Patch [3]: [PATCH v4 03/11] perf/cxl: Fix the counter overflow delta fixup ---
commit 6424601c16fd758ca929ead4bf2082b98255eee8
Author: Dave Jiang <dave.jiang@intel.com>

perf/cxl: Fix the counter overflow delta fixup

This commit fixes the counter overflow delta fixup by conditioning the fixup
on new_cnt >= prev_cnt instead of relying on the overflow status alone. It
also uses mask + 1 for the period to avoid undefined behavior with a 64-bit
counter width shift.

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

> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c

[ ... ]

> @@ -697,12 +697,16 @@ 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 mask discards the bit that says the counter wrapped, so a delta of
> +	 * one whole period reads back as 0 - the same as no events at all. Only
> +	 * the overflow status separates them, and new_cnt >= prev_cnt is that
> +	 * case, so add the period back. mask + 1 is 2^counter_width, which comes
> +	 * out as 0 for a 64-bit counter and avoids an 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);

[Severity: High]
This isn't a bug introduced by this patch, but could this macro expansion
result in undefined behavior if the hardware provides an invalid counter
width?

The counter_width is extracted directly from a hardware capability register
during initialization:

drivers/perf/cxl_pmu.c:cxl_pmu_parse_caps() {
    ...
    info->counter_width = FIELD_GET(CXL_PMU_CAP_COUNTER_WIDTH_MSK, val);
    ...
}

If a faulty or malicious CXL device provides a counter_width of 0 or greater
than 64, the GENMASK_ULL() macro here in __cxl_pmu_read() will evaluate with
a negative or out-of-bounds shift amount. Since GENMASK_ULL(h, l) produces
undefined behavior when h < l or h >= 64, this could trigger a kernel splat
under UBSAN and potentially lead to a panic.

Should we validate the capability register bounds when parsing it to protect
against untrusted MMIO input?

> +	delta = (new_cnt - prev_cnt) & mask;
> +	if (overflow && new_cnt >= prev_cnt)
> +		delta += mask + 1;
>  
>  	local64_add(delta, &event->count);
>  }

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

Re: [PATCH v4 05/11] perf/cxl: Split the MSI vector out of info->irq

[sashiko-bot]

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

Pre-existing issues:
- [High] The CXL PMU interrupt is requested with IRQF_SHARED, but its CPU affinity is dynamically modified during CPU hotplug, which is unsafe for shared interrupts.
--

--- Patch [5]: [PATCH v4 05/11] perf/cxl: Split the MSI vector out of info->irq ---
commit 7ced4bedee1c856d5114f92c2eb91db914c1a974
Author: Dave Jiang <dave.jiang@intel.com>

perf/cxl: Split the MSI vector out of info->irq

This commit separates the MSI/MSI-X message number from the Linux virq in
info->irq. It introduces info->msi_vec for the message number while retaining
info->irq for the Linux virq to prevent mix-ups between the two distinct
values.

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 2bebbbe67a0a2..481d32d0c1b02 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c

[ ... ]

> @@ -873,10 +874,10 @@ static int cxl_pmu_probe(struct device *dev)
>  		.capabilities = PERF_PMU_CAP_NO_EXCLUDE,
>  	};
>  
> -	if (info->irq < 0)
> +	if (info->msi_vec < 0)
>  		return -EINVAL;
>  
> -	rc = pci_irq_vector(pdev, info->irq);
> +	rc = pci_irq_vector(pdev, info->msi_vec);
>  	if (rc < 0)
>  		return rc;
>  	irq = rc;

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the IRQ
handling here, I noticed a potential problem with how the PMU interrupt is
managed in drivers/perf/cxl_pmu.c.

In cxl_pmu_probe(), the interrupt is requested as shared and without
IRQF_NOBALANCING:

rc = devm_request_irq(dev, irq, cxl_pmu_irq, IRQF_SHARED | IRQF_NO_THREAD,
                      irq_name, info);

However, the CPU hotplug callbacks dynamically modify the CPU affinity of
this shared interrupt:

cxl_pmu_online_cpu() {
    ...
    WARN_ON(irq_set_affinity(info->irq, cpumask_of(cpu)));
}

cxl_pmu_offline_cpu() {
    ...
    WARN_ON(irq_set_affinity(info->irq, cpumask_of(target)));
}

Is it safe to change the affinity of a shared interrupt like this?

Modifying the affinity alters it for all co-owners of the IRQ.

Additionally, if irqbalance changes the affinity (since IRQF_NOBALANCING is
missing), could the PMU interrupt handler be forced to execute on the wrong
CPU and potentially corrupt state when locklessly accessing per-CPU data?

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

Re: [PATCH v4 02/11] perf/cxl: Clear stale event fields before reprogramming a counter

[sashiko-bot]

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

Pre-existing issues:
- [High] Writing to the 32-bit filter configuration register with `writeq` (64-bit) overwrites the adjacent filter register, causing silent event drops on hardware with multiple filters.
- [High] The overflow calculation in `__cxl_pmu_read()` double-counts events by erroneously adding `2^width` to a correctly computed delta.
--

--- Patch [2]: [PATCH v4 02/11] perf/cxl: Clear stale event fields before reprogramming a counter ---
commit 54f01a971c5fe0d0d8f7face1960b3232e30fafb
Author: Dave Jiang <dave.jiang@intel.com>

perf/cxl: Clear stale event fields before reprogramming a counter
    
This commit uses FIELD_MODIFY() to clear stale event fields before
reprogramming a counter. This ensures new selections don't land on top of
old ones when reusing counters.

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

[ ... ]

> @@ -638,30 +638,30 @@ static void cxl_pmu_event_start(struct perf_event *event, int flags)
>  	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));

[Severity: High]
This isn't a bug introduced by this patch, but could this writeq()
unintentionally overwrite the adjacent filter register?

Since CXL_PMU_FILTER_CFG_REG is a 32-bit register and writeq() performs a
64-bit write, the variable cfg (which has 0 in its upper 32 bits) will
inadvertently zero out the adjacent 32 bits corresponding to Filter 1.

Because writing 0 enables a filter and forces an exact match of 0, any
hardware implementing Filter 1 will logically AND it with Filter 0,
effectively dropping all non-zero events.

>  	}
>  
>  	cfg = readq(base + CXL_PMU_COUNTER_CFG_REG(hwc->idx));
>  	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);
> -	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_EDGE,
> -			  cxl_pmu_config1_get_edge(event) ? 1 : 0);
> -	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_INVERT,
> -			  cxl_pmu_config1_get_invert(event) ? 1 : 0);
> +	FIELD_MODIFY(CXL_PMU_COUNTER_CFG_EDGE, &cfg,
> +		     cxl_pmu_config1_get_edge(event) ? 1 : 0);
> +	FIELD_MODIFY(CXL_PMU_COUNTER_CFG_INVERT, &cfg,
> +		     cxl_pmu_config1_get_invert(event) ? 1 : 0);

[ ... ]

> @@ -700,10 +700,10 @@ static void __cxl_pmu_read(struct perf_event *event, bool overflow)
>  	/*
>  	 * If we know an overflow occur then take that into account.
>  	 * Note counter is not reset as that would lose events
>  	 */
>  	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: High]
This is a pre-existing issue, but is it possible this addition double-counts
events?

The 2's complement arithmetic delta = (new_cnt - prev_cnt) & GENMASK_ULL(...)
already naturally calculates the delta across a single wrap-around. For
example, with a 32-bit counter, 0x10 - 0xFFFFFFF0 masked strictly yields 0x20.

If this condition evaluates to true, it injects an additional 2^width into
the delta, reporting billions of extraneous events per normal overflow.

Additionally, if info->counter_width is 64, does 1UL << 64 result in
undefined behavior in C?

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

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

Re: [PATCH v4 10/11] perf/cxl: Don't log through pmu.dev in the overflow interrupt handler

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-After-Free in `cxl_pmu_offline_cpu()` due to devres teardown order.
--

--- Patch [10]: [PATCH v4 10/11] perf/cxl: Don't log through pmu.dev in the overflow interrupt handler ---
commit 9f5d830c7ee5748e40bbe955adff5fe5abd56b9f
Author: Dave Jiang <dave.jiang@intel.com>

perf/cxl: Don't log through pmu.dev in the overflow interrupt handler

This commit changes the overflow interrupt handler to log through
info->pmu.parent instead of info->pmu.dev. This prevents a use-after-free
if an overflow occurs just before teardown, as perf_pmu_unregister() frees
pmu->dev without clearing the pointer.

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 84ebc7da181d5..45edef53a983d 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -805,7 +805,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,

[Severity: High]
This is a pre-existing issue, but since we are fixing teardown races with
freed pmu structures, does a similar use-after-free exist in
cxl_pmu_offline_cpu() during concurrent cpu hotplug events?

In cxl_pmu_probe(), the devm actions are ordered such that the pmu is
unregistered before the cpu hotplug instance is removed:

drivers/perf/cxl_pmu.c:cxl_pmu_probe() {
	...
	rc = cpuhp_state_add_instance(cxl_pmu_cpuhp_state_num, &info->node);
	if (rc)
		return rc;

	rc = devm_add_action_or_reset(dev, cxl_pmu_cpuhp_remove, info);
	if (rc)
		return rc;

	rc = perf_pmu_register(&info->pmu, info->pmu.name, -1);
	if (rc)
		return rc;

	rc = devm_add_action_or_reset(dev, cxl_pmu_perf_unregister, info);
	...
}

Because devm executes teardown in a reverse order, cxl_pmu_perf_unregister()
runs first. This calls perf_pmu_unregister(), which frees info->pmu.dev
and info->pmu.cpu_pmu_context.

However, the cpu hotplug instance remains active until cxl_pmu_cpuhp_remove()
runs. If a cpu offline event occurs in this window, cxl_pmu_offline_cpu()
could be invoked:

drivers/perf/cxl_pmu.c:cxl_pmu_offline_cpu() {
	...
		dev_err(info->pmu.dev, "Unable to find a suitable CPU\n");
		return 0;
	}

	perf_pmu_migrate_context(&info->pmu, cpu, target);
	...
}

Could this lead to a use-after-free when dereferencing the freed
info->pmu.dev in the error log, or info->pmu.cpu_pmu_context inside
perf_pmu_migrate_context()?

>  				"overflow but on non enabled counter %d\n", i);
>  			continue;
>  		}

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