[PATCH AUTOSEL 6.19-6.1] perf/arm-cmn: Support CMN-600AE

[PATCH AUTOSEL 6.19-6.1] perf/arm-cmn: Support CMN-600AE

[Sasha Levin]

From: Robin Murphy <robin.murphy@arm.com>

[ Upstream commit 12a94953c37e834c3eabb839ce057094946fe67a ]

The functional safety features of CMN-600AE have little to no impact on
the PMU relative to the base CMN-600 design, so for simplicity we can
reasonably just treat it as the same thing. The only obvious difference
is that the revision numbers aren't aligned, so we may hide some aliases
for events which do actually exist, but those can still be specified via
the underlying "type,eventid" format so it's not too big a deal.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Tested-by: Michal Simek <michal.simek@amd.com>
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Let me now do the complete analysis summary.

---

## Complete Analysis

### 1. COMMIT MESSAGE ANALYSIS

**Subject:** "perf/arm-cmn: Support CMN-600AE"

The commit adds support for the CMN-600AE part in the ARM CMN perf
driver. The message explicitly states that CMN-600AE is a "functional
safety" (AE = Automotive Enhanced) variant of CMN-600, and from the PMU
perspective, it is effectively the same as CMN-600. The author (Robin
Murphy, the arm-cmn driver maintainer) acknowledges that revision number
differences may hide some event aliases, but these can still be used via
raw `type,eventid` format.

The commit has both `Reviewed-by` (Ilkka Koskinen from Ampere Computing)
and `Tested-by` (Michal Simek from AMD/Xilinx), indicating this was
tested on real hardware.

### 2. CODE CHANGE ANALYSIS

The change is extremely small - exactly **4 lines added, 0 lines
removed** (net +4):

**Change 1:** Adds `PART_CMN600AE = 0x438` to the `enum cmn_part` (line
~213 in the diff). This is a simple hardware ID constant.

**Change 2:** In `arm_cmn_discover()`, after reading the part number
from the hardware's peripheral ID registers, adds:
```c
/* 600AE is close enough that it's not really worth more complexity */
if (part == PART_CMN600AE)
    part = PART_CMN600;
```

This maps the CMN-600AE part number to CMN-600 before it's stored in
`cmn->part`. This is a clean aliasing approach - the rest of the driver
sees it as CMN-600.

### 3. CLASSIFICATION - This is a Hardware Device ID Addition

This commit falls squarely into the **"New Device IDs"** exception
category for stable backporting. It is analogous to adding a new PCI ID
or USB ID to an existing driver. Specifically:

- The driver already exists in all stable trees (since v5.10).
- The CMN-600 support is fully mature.
- The only change is recognizing a new hardware part number (0x438) and
  mapping it to the existing CMN-600 code path.

### 4. WHAT HAPPENS WITHOUT THE PATCH

On a system with CMN-600AE hardware:

1. **Firmware/DT matching:** The firmware would describe the device as
   CMN-600 compatible (since there's no separate "arm,cmn-600ae" DT
   binding or ACPI ID). So `cmn->part` is initially set to `PART_CMN600`
   (0x434) at probe time (line 2557).

2. **Hardware discovery:** In `arm_cmn_discover()`, the hardware
   peripheral ID register reports 0x438, NOT 0x434. This triggers the
   **firmware binding mismatch warning**: `"Firmware binding mismatch:
   expected part number 0x%x, found 0x%x"`.

3. **Part number overwrite:** `cmn->part` is set to the hardware-
   reported 0x438 (line 2273), which is an unknown value.

4. **`arm_cmn_model()` returns 0:** With an unrecognized part number,
   the model lookup returns 0 (default case in the switch). This
   triggers the **"Unknown part number: 0x%x"** warning.

5. **PMU is effectively broken:**
   - **All events are hidden:** The visibility check `eattr->model &
     arm_cmn_model(cmn)` (line 710) evaluates to `X & 0 = 0` for every
     event, so ALL event attributes return mode 0 (hidden from sysfs).
   - **Filter selection fails:** `arm_cmn_filter_sel()` (line 1758)
     checks `e->model & model`, which is always 0, so filter selection
     always returns `SEL_NONE`.
   - **CMN-600-specific paths are skipped:** All checks like `cmn->part
     == PART_CMN600` (lines 444, 728, 739, 1400, 1893, 2353, etc.) fail
     because 0x438 != 0x434.

In summary, **without this patch, the PMU driver fails to function on
CMN-600AE hardware** - it loads but provides zero usable performance
monitoring events, and produces misleading warnings.

### 5. SCOPE AND RISK ASSESSMENT

- **Lines changed:** 4 lines added, 0 removed. Extremely minimal.
- **Files touched:** 1 file (`drivers/perf/arm-cmn.c`).
- **Risk:** Extremely low. The change only affects hardware with part
  number 0x438. All other hardware paths are completely unaffected. The
  mapping `CMN600AE → CMN600` is architecturally correct per the
  hardware designer (Robin Murphy works at Arm).
- **Subsystem:** ARM perf PMU driver - isolated, doesn't affect other
  subsystems.

### 6. DEPENDENCIES

The patch has **no dependencies on other commits**. It's completely
self-contained:
- The `enum cmn_part` exists in all stable trees from v5.10 onward.
- The `arm_cmn_discover()` function and the part-number detection code
  is unchanged across stable trees.
- The patch will need trivial context adaptation for v6.6 (missing
  `PART_CMN_S3` member in the enum), but the actual insertion point is
  clean.

### 7. USER IMPACT

CMN-600AE is used in **functional safety / automotive** applications
(the "AE" stands for Automotive Enhanced). The Tested-by from Michal
Simek at AMD/Xilinx suggests it's used in Xilinx/AMD Versal platforms,
and the review from Ampere Computing's Ilkka Koskinen suggests it's also
relevant in server/cloud contexts. These are exactly the kinds of
systems that use stable kernels.

Without this patch, users with CMN-600AE silicon cannot use the `perf`
PMU monitoring at all, which is important for performance analysis and
debugging on these platforms.

### 8. COMPARISON TO SIMILAR ACCEPTED STABLE PATCHES

This is directly comparable to:
- USB device ID additions to quirks tables
- PCI ID additions to existing drivers
- ACPI ID additions
- DRM i915/amdgpu device ID additions

All of these are routinely backported to stable.

### 9. CONCERNS

The commit message describes this as "support" for new hardware, which
nominally sounds like "new feature." However, the actual implementation
is a **device ID alias** - mapping a new part number to already-
supported functionality. This is not adding new driver code, new event
types, or new functionality. It is enabling existing, proven code paths
for a hardware variant.

The only minor concern is the comment about revision numbers: "the
revision numbers aren't aligned, so we may hide some aliases for events
which do actually exist." This means some named perf events might not
show up in sysfs because the revision check for CMN-600 revisions won't
match CMN-600AE revision numbers. However, these events still work via
raw `type,eventid` format, so this is a cosmetic limitation, not a
functional one.

### CONCLUSION

This commit is a textbook example of a **hardware device ID addition**
to an existing driver. It is:
- **Small:** 4 lines, single file
- **Self-contained:** No dependencies on other commits
- **Low risk:** Only affects systems with specific hardware (part number
  0x438)
- **High value:** Without it, the PMU driver is completely non-
  functional on CMN-600AE hardware
- **Well-tested:** Has both Reviewed-by and Tested-by on real hardware
- **Applies cleanly:** The relevant code context is stable across kernel
  versions

The fix is small, surgical, and meets all stable kernel criteria as a
device ID addition.

**YES**

 drivers/perf/arm-cmn.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
index 23245352a3fc0..651edd73bfcb1 100644
--- a/drivers/perf/arm-cmn.c
+++ b/drivers/perf/arm-cmn.c
@@ -210,6 +210,7 @@ enum cmn_model {
 enum cmn_part {
 	PART_CMN600 = 0x434,
 	PART_CMN650 = 0x436,
+	PART_CMN600AE = 0x438,
 	PART_CMN700 = 0x43c,
 	PART_CI700 = 0x43a,
 	PART_CMN_S3 = 0x43e,
@@ -2266,6 +2267,9 @@ static int arm_cmn_discover(struct arm_cmn *cmn, unsigned int rgn_offset)
 	reg = readq_relaxed(cfg_region + CMN_CFGM_PERIPH_ID_01);
 	part = FIELD_GET(CMN_CFGM_PID0_PART_0, reg);
 	part |= FIELD_GET(CMN_CFGM_PID1_PART_1, reg) << 8;
+	/* 600AE is close enough that it's not really worth more complexity */
+	if (part == PART_CMN600AE)
+		part = PART_CMN600;
 	if (cmn->part && cmn->part != part)
 		dev_warn(cmn->dev,
 			 "Firmware binding mismatch: expected part number 0x%x, found 0x%x\n",
-- 
2.51.0

[PATCH AUTOSEL 6.19-6.6] perf/cxlpmu: Replace IRQF_ONESHOT with IRQF_NO_THREAD

[Sasha Levin]

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

[ Upstream commit ab26d9c85554c4ff1d95ca8341522880ed9219d6 ]

Passing IRQF_ONESHOT ensures that the interrupt source is masked until
the secondary (threaded) handler is done. If only a primary handler is
used then the flag makes no sense because the interrupt can not fire
(again) while its handler is running.
The flag also disallows force-threading of the primary handler and the
irq-core will warn about this.

The intention here was probably not allowing forced-threading.

Replace IRQF_ONESHOT with IRQF_NO_THREAD.

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have gathered all the information I need for a thorough analysis.
Let me compile my findings.

---

## Complete Analysis: `perf/cxlpmu: Replace IRQF_ONESHOT with
IRQF_NO_THREAD`

### 1. COMMIT MESSAGE ANALYSIS

The commit, authored by Sebastian Andrzej Siewior
(bigeasy@linutronix.de) — one of the PREEMPT_RT core maintainers —
replaces `IRQF_ONESHOT` with `IRQF_NO_THREAD` in the CXL PMU driver's
interrupt registration. The commit message explains:

- `IRQF_ONESHOT` ensures the interrupt source is masked until the
  threaded (secondary) handler finishes
- This driver only has a **primary** handler (`cxl_pmu_irq`) and no
  threaded handler
- Therefore `IRQF_ONESHOT` makes no sense here — the interrupt can't
  fire while its hardirq handler is running anyway
- The flag also **disables force-threading** of the primary handler
- The "irq-core will warn about this" (via lockdep assertions)
- The **intended** semantics were to prevent forced-threading, so
  `IRQF_NO_THREAD` is the correct replacement

### 2. CODE CHANGE ANALYSIS

The change is a **single flag swap** on one line:

```880:881:drivers/perf/cxl_pmu.c
        rc = devm_request_irq(dev, irq, cxl_pmu_irq, IRQF_SHARED |
IRQF_ONESHOT,
                              irq_name, info);
```

becomes:

```880:881:drivers/perf/cxl_pmu.c
        rc = devm_request_irq(dev, irq, cxl_pmu_irq, IRQF_SHARED |
IRQF_NO_THREAD,
                              irq_name, info);
```

#### What `IRQF_ONESHOT` does here (incorrectly):
Looking at `irq_setup_forced_threading()` in `kernel/irq/manage.c`:

```1291:1296:kernel/irq/manage.c
static int irq_setup_forced_threading(struct irqaction *new)
{
        if (!force_irqthreads())
                return 0;
        if (new->flags & (IRQF_NO_THREAD | IRQF_PERCPU | IRQF_ONESHOT))
                return 0;
```

Both `IRQF_NO_THREAD` and `IRQF_ONESHOT` cause
`irq_setup_forced_threading()` to bail out early, preventing the
interrupt from being force-threaded. However, `IRQF_ONESHOT` has an
**additional side effect**: it tells the IRQ core to mask the interrupt
line until a threaded handler completes. Since there is no threaded
handler here, this masking behavior is semantically wrong.

#### Why `IRQF_NO_THREAD` is the correct flag:
The `cxl_pmu_irq` handler is a PMU overflow interrupt handler that:
1. Reads the overflow register via `readq()`
2. Processes each overflowed counter via `__cxl_pmu_read()`
3. Clears the overflow status via `writeq()`

This handler interacts with perf core internals. As the arm-ccn PMU fix
(commit `0811ef7e2f54`) established, **PMU interrupt handlers must not
be force-threaded** because the perf core relies on strict CPU affinity
and interrupt disabling for mutual exclusion. Force-threading a PMU
interrupt handler would break these synchronization guarantees.

#### The actual bugs:

**Bug 1 — PREEMPT_RT / `threadirqs` warning:** When the kernel is booted
with `threadirqs` command-line parameter or PREEMPT_RT is enabled, the
IRQ core's lockdep infrastructure marks handlers as `hardirq_threaded`
if they can be force-threaded. In `kernel/irq/handle.c`:

```199:201:kernel/irq/handle.c
                if (irq_settings_can_thread(desc) &&
                    !(action->flags & (IRQF_NO_THREAD | IRQF_PERCPU |
IRQF_ONESHOT)))
                        lockdep_hardirq_threaded();
```

Because `IRQF_ONESHOT` is already set, this particular path won't
trigger the threaded annotation, but the **masking semantics** of
ONESHOT are incorrect for a primary-only handler. In the `IRQF_ONESHOT`
path, the IRQ core does:

```1726:1727:kernel/irq/manage.c
                if (new->flags & IRQF_ONESHOT)
                        desc->istate |= IRQS_ONESHOT;
```

This causes the interrupt line to be masked during the handler and
unmask logic depends on `desc->threads_oneshot` — but there's no thread
to clear this mask, so it depends on the
`cond_unmask_irq`/`cond_unmask_eoi_irq` fallback path.

**Bug 2 — IRQF_SHARED conflict potential:** The interrupt uses
`IRQF_SHARED`. When sharing interrupts, all handlers on the same line
must agree on `IRQF_ONESHOT`. If another driver on the shared line
doesn't use `IRQF_ONESHOT`, `request_threaded_irq()` will fail with
`-EINVAL` at the mismatch check:

```1606:1607:kernel/irq/manage.c
                else if ((old->flags ^ new->flags) & IRQF_ONESHOT)
                        goto mismatch;
```

This is a real failure mode for shared interrupts — CXL PMU's incorrect
use of `IRQF_ONESHOT` could prevent other handlers from sharing the same
IRQ line.

### 3. CLASSIFICATION

This is a **bug fix** — it corrects incorrect IRQ flag usage that:
1. Applies semantically wrong masking behavior (ONESHOT without a
   thread)
2. Can trigger warnings/assertions under PREEMPT_RT or `threadirqs`
3. Could cause shared IRQ registration failures
4. Prevents force-threading in the wrong way (the intention is correct,
   but the mechanism is wrong)

### 4. SCOPE AND RISK ASSESSMENT

- **Lines changed:** 1 line (single flag change)
- **Files touched:** 1 (`drivers/perf/cxl_pmu.c`)
- **Complexity:** Minimal — straightforward flag replacement
- **Risk:** Extremely low. `IRQF_NO_THREAD` is the semantically correct
  flag for the intended purpose. The interrupt handler is a hardirq-only
  handler that performs only register I/O and atomic operations — it
  should never be threaded.
- **Subsystem:** CXL PMU performance monitoring driver (PCI-based)

### 5. USER IMPACT

- **PREEMPT_RT users:** On PREEMPT_RT kernels, `IRQF_ONESHOT` without a
  thread handler causes the interrupt line to be masked with incorrect
  oneshot semantics. This could lead to lockdep warnings or subtle
  interrupt masking issues.
- **Shared IRQ users:** CXL devices sharing PCI MSI/MSI-X vectors with
  other devices could experience IRQ registration failures due to
  ONESHOT flag mismatch.
- **General users:** The CXL PMU driver is used for performance
  monitoring on CXL 3.0 memory devices — an increasingly important
  hardware platform for servers and data centers.

### 6. STABILITY INDICATORS

- **Author:** Sebastian Andrzej Siewior — PREEMPT_RT core maintainer,
  expert on IRQ threading
- **Reviewer:** Jonathan Cameron — CXL maintainer
- **Signed-off-by:** Will Deacon — ARM architecture and perf subsystem
  maintainer
- **Pattern:** This is the same fix Sebastian has applied to multiple
  drivers (i2c-exynos5, i2c-hix5hd2) with proven track record
- **Precedent:** The arm-ccn PMU used exactly this fix
  (`IRQF_NO_THREAD`) for the same class of issue

### 7. DEPENDENCY CHECK

- No dependencies on other commits
- The driver exists in stable kernels 6.6.y and later (introduced in
  v6.5-rc1)
- The change is self-contained — just a flag change in
  `devm_request_irq()`

### 8. CONCLUSION

This is a small, surgical, well-understood bug fix that corrects
incorrect IRQ flag usage in the CXL PMU driver. The `IRQF_ONESHOT` flag
was semantically wrong for a primary-only interrupt handler and could
cause:
- Incorrect interrupt line masking behavior
- Shared IRQ registration failures
- Issues on PREEMPT_RT kernels

The replacement with `IRQF_NO_THREAD` correctly expresses the intended
behavior (prevent force-threading of this PMU interrupt handler) without
the incorrect side effects. The fix is trivial, low-risk, authored by a
world-class IRQ threading expert, and reviewed by CXL and ARM
maintainers.

**YES**

 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 d094030220bf2..68a54d97d2a8a 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -877,7 +877,7 @@ 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_ONESHOT,
+	rc = devm_request_irq(dev, irq, cxl_pmu_irq, IRQF_SHARED | IRQF_NO_THREAD,
 			      irq_name, info);
 	if (rc)
 		return rc;
-- 
2.51.0