[PATCH v2 0/4] perf/arm_cspmu: Miscellaneous improvements

[PATCH v2 0/4] perf/arm_cspmu: Miscellaneous improvements

[Robin Murphy]

Hi all,

These are a few quality-of-life improvements for CoreSight PMU that
I've accumulated based on things which have come up in practical usage.
There's no real dependency between any of the individual patches, it
just felt right to send them as a series.

v1: https://lore.kernel.org/linux-perf-users/cover.1783605641.git.robin.murphy@arm.com/
v2: Fix valid nits from Sashiko review, add Ilkka's R-b.

Thanks,
Robin.


Robin Murphy (4):
  ACPI/APMT: Use stable device ID
  perf/arm_cspmu: Improve APMT-based PMU naming
  perf/arm_cspmu: Improve sub-module error reporting
  perf/arm_cspmu: Make IRQ more optional

 drivers/acpi/arm64/apmt.c          |  6 ++--
 drivers/perf/arm_cspmu/arm_cspmu.c | 48 ++++++++++++++++++------------
 2 files changed, 33 insertions(+), 21 deletions(-)

-- 
2.54.0.dirty

[PATCH v2 1/4] ACPI/APMT: Use stable device ID

[Robin Murphy]

The APMT node format includes a unique identifier, so we can use this as
the platform device ID to give userspace stable and identifiable device
names, rather than auto numbering dependent on how the table is parsed.

Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Hanjun Guo <guohanjun@huawei.com>
Cc: Sudeep Holla <sudeep.holla@kernel.org>
Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>

---
v2: Handle and warn about unexpected signed overflow
---
 drivers/acpi/arm64/apmt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/arm64/apmt.c b/drivers/acpi/arm64/apmt.c
index bb010f6164e5..91fcdd289e63 100644
--- a/drivers/acpi/arm64/apmt.c
+++ b/drivers/acpi/arm64/apmt.c
@@ -76,10 +76,12 @@ static int __init apmt_add_platform_device(struct acpi_apmt_node *node,
 					   struct fwnode_handle *fwnode)
 {
 	struct platform_device *pdev;
-	int ret, count;
+	int ret, count, uid = node->id & INT_MAX;
 	struct resource res[DEV_MAX_RESOURCE_COUNT];
 
-	pdev = platform_device_alloc(DEV_NAME, PLATFORM_DEVID_AUTO);
+	if (uid != node->id)
+		pr_warn("Unexpectedly large UID 0x%x, truncated to 0x%x\n", node->id, uid);
+	pdev = platform_device_alloc(DEV_NAME, uid);
 	if (!pdev)
 		return -ENOMEM;
 
-- 
2.54.0.dirty

Re: [PATCH v2 1/4] ACPI/APMT: Use stable device ID

[sashiko-bot]

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

New issues:
- [High] Platform device ID collision causing APMT parsing abort and subsequent UAF.

Pre-existing issues:
- [High] Missing rollback on failure leaves previously registered platform devices holding dangling pointers to an unmapped ACPI table.
- [High] Missing bounds check on `apmt_node->length` allows out-of-bounds memory read.
- [Medium] GSI IRQ mapping is leaked on platform device registration error paths.
--

--- Patch [1]: [PATCH v2 1/4] ACPI/APMT: Use stable device ID ---
commit 895c0eb864a4218fbb99620f4fa9efb2dc4e091c
Author: Robin Murphy <robin.murphy@arm.com>

ACPI/APMT: Use stable device ID

The APMT node format includes a unique identifier, so we can use this as
the platform device ID to give userspace stable and identifiable device
names.

> diff --git a/drivers/acpi/arm64/apmt.c b/drivers/acpi/arm64/apmt.c
> index bb010f6164e52..91fcdd289e637 100644
> --- a/drivers/acpi/arm64/apmt.c
> +++ b/drivers/acpi/arm64/apmt.c
> @@ -76,10 +76,12 @@ static int __init apmt_add_platform_device(struct acpi_apmt_node *node,
>  					   struct fwnode_handle *fwnode)
>  {
>  	struct platform_device *pdev;
> -	int ret, count;
> +	int ret, count, uid = node->id & INT_MAX;
>  	struct resource res[DEV_MAX_RESOURCE_COUNT];
>  
> -	pdev = platform_device_alloc(DEV_NAME, PLATFORM_DEVID_AUTO);
> +	if (uid != node->id)
> +		pr_warn("Unexpectedly large UID 0x%x, truncated to 0x%x\n", node->id, uid);
> +	pdev = platform_device_alloc(DEV_NAME, uid);

[Severity: High]
Does this newly introduced truncation create a risk of ID collisions if two
unique 32-bit APMT node IDs differ only in the sign bit?

If a collision occurs, platform_device_add() will fail with -EEXIST. This
halts the initialization of all remaining PMU nodes and triggers another issue.

[Severity: High]
This is a pre-existing issue, but when apmt_init_platform_devices() fails
(which is now more likely due to potential ID collisions), the caller unmaps
the ACPI table without a cleanup loop:

drivers/acpi/arm64/apmt.c:acpi_apmt_init() {
    ...
	ret = apmt_init_platform_devices();
	if (ret) {
		pr_err("Failed to initialize APMT platform devices, ret: %d\n", ret);
		acpi_put_table(apmt_table);
	}
}

Will this leave any previously successfully registered platform devices holding
dangling pointers in their platform_data referencing the now unmapped table
memory, leading to a use-after-free when a driver later binds?

[Severity: Medium]
This is a pre-existing issue, but if a device addition fails, does the
cleanup path leak the GSI IRQ mapping?

drivers/acpi/arm64/apmt.c:apmt_init_resources() {
    ...
		irq = acpi_register_gsi(NULL, node->ovflw_irq, trigger,
						ACPI_ACTIVE_HIGH);
    ...
}

When apmt_add_platform_device() takes the error path:

drivers/acpi/arm64/apmt.c:apmt_add_platform_device() {
    ...
dev_put:
	platform_device_put(pdev);

	return ret;
}

Should there be an acpi_unregister_gsi() call to reverse the mapping created
during apmt_init_resources()?

[Severity: High]
This is a pre-existing issue, but does the table parsing loop in
apmt_init_platform_devices() need a bounds check on apmt_node->length?

drivers/acpi/arm64/apmt.c:apmt_init_platform_devices() {
    ...
	while (offset < end) {
		apmt_node = ACPI_ADD_PTR(struct acpi_apmt_node, apmt,
				 offset);
        ...
		offset += apmt_node->length;
	}
    ...
}

If a malformed ACPI table provides a maliciously small or truncated length,
could this lead to ACPI_ADD_PTR yielding a misaligned pointer and reading
memory outside the mapped ACPI bounds?

>  	if (!pdev)
>  		return -ENOMEM;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784213435.git.robin.murphy@arm.com?part=1

Re: [PATCH v2 1/4] ACPI/APMT: Use stable device ID

[Robin Murphy]

On 16/07/2026 4:17 pm, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> 
> New issues:
> - [High] Platform device ID collision causing APMT parsing abort and subsequent UAF.
> 
> Pre-existing issues:
> - [High] Missing rollback on failure leaves previously registered platform devices holding dangling pointers to an unmapped ACPI table.
> - [High] Missing bounds check on `apmt_node->length` allows out-of-bounds memory read.
> - [Medium] GSI IRQ mapping is leaked on platform device registration error paths.
> --
> 
> --- Patch [1]: [PATCH v2 1/4] ACPI/APMT: Use stable device ID ---
> commit 895c0eb864a4218fbb99620f4fa9efb2dc4e091c
> Author: Robin Murphy <robin.murphy@arm.com>
> 
> ACPI/APMT: Use stable device ID
> 
> The APMT node format includes a unique identifier, so we can use this as
> the platform device ID to give userspace stable and identifiable device
> names.
> 
>> diff --git a/drivers/acpi/arm64/apmt.c b/drivers/acpi/arm64/apmt.c
>> index bb010f6164e52..91fcdd289e637 100644
>> --- a/drivers/acpi/arm64/apmt.c
>> +++ b/drivers/acpi/arm64/apmt.c
>> @@ -76,10 +76,12 @@ static int __init apmt_add_platform_device(struct acpi_apmt_node *node,
>>   					   struct fwnode_handle *fwnode)
>>   {
>>   	struct platform_device *pdev;
>> -	int ret, count;
>> +	int ret, count, uid = node->id & INT_MAX;
>>   	struct resource res[DEV_MAX_RESOURCE_COUNT];
>>   
>> -	pdev = platform_device_alloc(DEV_NAME, PLATFORM_DEVID_AUTO);
>> +	if (uid != node->id)
>> +		pr_warn("Unexpectedly large UID 0x%x, truncated to 0x%x\n", node->id, uid);
>> +	pdev = platform_device_alloc(DEV_NAME, uid);
> 
> [Severity: High]
> Does this newly introduced truncation create a risk of ID collisions if two
> unique 32-bit APMT node IDs differ only in the sign bit?

In theory, yes. Hopefully the warning here and subsequent sysfs 
explosion will alert the firmware developers that they've done something 
very silly when they first try to boot to test their newly-written APMT.

Robin.

[PATCH v2 2/4] perf/arm_cspmu: Improve APMT-based PMU naming

[Robin Murphy]

On ACPI systems, it has not actually been possible for userspace to
reliably tell which PMU corresponds to which APMT entry for types
other than "ACPI device" - the evidence trail only leads from the
arbitrarily-numbered PMU device to its arbitrarily-numbered parent
platform device that has no distinguishing features either.

While we've now improved the platform device creation to associate the
actual APMT unique ID, we may as well also tweak the PMU devices to
substitute the arbitrary number with a different arbitrary number that
might be more directly meaningful based on the APMT definitions.

We don't have an equivalent for Devicetree, but in that case the
platform devices are at least identifiable via their sysfs-visible
of_node.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>

---
v2: Actually use the instance-based ID where different from node->id
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 35 +++++++++++++++++-------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 80fb314d5135..8c2dda17d73e 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -250,38 +250,43 @@ static const char *arm_cspmu_get_name(const struct arm_cspmu *cspmu)
 	struct device *dev;
 	struct acpi_apmt_node *apmt_node;
 	u8 pmu_type;
-	char *name;
 	char acpi_hid_string[ACPI_ID_LEN] = { 0 };
-	static atomic_t pmu_idx[ACPI_APMT_NODE_TYPE_COUNT] = { 0 };
+	static atomic_t pmu_idx;
+	u32 id;
 
 	dev = cspmu->dev;
 	apmt_node = arm_cspmu_apmt_node(dev);
 	if (!apmt_node)
 		return devm_kasprintf(dev, GFP_KERNEL, PMUNAME "_%u",
-				      atomic_fetch_inc(&pmu_idx[0]));
+				      atomic_fetch_inc(&pmu_idx));
 
 	pmu_type = apmt_node->type;
-
-	if (pmu_type >= ACPI_APMT_NODE_TYPE_COUNT) {
+	switch (pmu_type) {
+	default:
 		dev_err(dev, "unsupported PMU type-%u\n", pmu_type);
 		return NULL;
-	}
-
-	if (pmu_type == ACPI_APMT_NODE_TYPE_ACPI) {
+	case ACPI_APMT_NODE_TYPE_ACPI:
 		memcpy(acpi_hid_string,
 			&apmt_node->inst_primary,
 			sizeof(apmt_node->inst_primary));
-		name = devm_kasprintf(dev, GFP_KERNEL, "%s_%s_%s_%u", PMUNAME,
+		return devm_kasprintf(dev, GFP_KERNEL, "%s_%s_%s_%u", PMUNAME,
 				      arm_cspmu_type_str[pmu_type],
 				      acpi_hid_string,
 				      apmt_node->inst_secondary);
-	} else {
-		name = devm_kasprintf(dev, GFP_KERNEL, "%s_%s_%d", PMUNAME,
-				      arm_cspmu_type_str[pmu_type],
-				      atomic_fetch_inc(&pmu_idx[pmu_type]));
-	}
+	case ACPI_APMT_NODE_TYPE_MC:
+		id = apmt_node->id;
+		break;
+	case ACPI_APMT_NODE_TYPE_SMMU:
+	case ACPI_APMT_NODE_TYPE_PCIE_ROOT:
+		id = apmt_node->inst_primary;
+		break;
+	case ACPI_APMT_NODE_TYPE_CACHE:
+		id = apmt_node->inst_secondary;
+		break;
+	};
 
-	return name;
+	return devm_kasprintf(dev, GFP_KERNEL, "%s_%s_%u", PMUNAME,
+			      arm_cspmu_type_str[pmu_type], id);
 }
 
 static ssize_t arm_cspmu_cpumask_show(struct device *dev,
-- 
2.54.0.dirty

Re: [PATCH v2 2/4] perf/arm_cspmu: Improve APMT-based PMU naming

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Naming PMUs based solely on their target component's instance ID can cause sysfs name collisions if a single hardware component has multiple independent PMUs.
--

--- Patch [2]: [PATCH v2 2/4] perf/arm_cspmu: Improve APMT-based PMU naming ---
commit c464ddb014cc8172e2e952ddcc892f142c6f5839
Author: Robin Murphy <robin.murphy@arm.com>

perf/arm_cspmu: Improve APMT-based PMU naming

This commit tweaks the PMU devices to substitute the arbitrary number with a
more meaningful one based on the APMT definitions. On ACPI systems, it associates
the actual APMT unique ID rather than relying on arbitrarily-numbered platform
devices.

> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index e6292021f653e..e3aaf587e6e54 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c

[ ... ]

> @@ -250,38 +250,43 @@ static const char *arm_cspmu_get_name(const struct arm_cspmu *cspmu)
>  	struct device *dev;
>  	struct acpi_apmt_node *apmt_node;
>  	u8 pmu_type;
> -	char *name;
>  	char acpi_hid_string[ACPI_ID_LEN] = { 0 };
> -	static atomic_t pmu_idx[ACPI_APMT_NODE_TYPE_COUNT] = { 0 };
> +	static atomic_t pmu_idx;
> +	u32 id;

[ ... ]

> +	case ACPI_APMT_NODE_TYPE_MC:
> +		id = apmt_node->id;
> +		break;
> +	case ACPI_APMT_NODE_TYPE_SMMU:
> +	case ACPI_APMT_NODE_TYPE_PCIE_ROOT:
> +		id = apmt_node->inst_primary;
> +		break;

[Severity: Medium]
Will this lead to a sysfs name collision if a single hardware component has
multiple independent PMUs?

If multiple PMUs target the same component (like separate PMUs for an SMMU TCU
and TBUs), they will get identical names since apmt_node->inst_secondary is
ignored here. This could cause perf_pmu_register() to fail with -EEXIST during
device probe, preventing all but the first PMU on the hardware component from
being registered.

> +	case ACPI_APMT_NODE_TYPE_CACHE:
> +		id = apmt_node->inst_secondary;
> +		break;
> +	};
>  
> -	return name;
> +	return devm_kasprintf(dev, GFP_KERNEL, "%s_%s_%u", PMUNAME,
> +			      arm_cspmu_type_str[pmu_type], id);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784213435.git.robin.murphy@arm.com?part=2

Re: [PATCH v2 2/4] perf/arm_cspmu: Improve APMT-based PMU naming

[Ilkka Koskinen]

On Thu, 16 Jul 2026, Robin Murphy wrote:
> On ACPI systems, it has not actually been possible for userspace to
> reliably tell which PMU corresponds to which APMT entry for types
> other than "ACPI device" - the evidence trail only leads from the
> arbitrarily-numbered PMU device to its arbitrarily-numbered parent
> platform device that has no distinguishing features either.
>
> While we've now improved the platform device creation to associate the
> actual APMT unique ID, we may as well also tweak the PMU devices to
> substitute the arbitrary number with a different arbitrary number that
> might be more directly meaningful based on the APMT definitions.
>
> We don't have an equivalent for Devicetree, but in that case the
> platform devices are at least identifiable via their sysfs-visible
> of_node.
>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>
> ---
> v2: Actually use the instance-based ID where different from node->id


Looks good to me now,

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>

Cheers, Ilkka

> ---
> drivers/perf/arm_cspmu/arm_cspmu.c | 35 +++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index 80fb314d5135..8c2dda17d73e 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -250,38 +250,43 @@ static const char *arm_cspmu_get_name(const struct arm_cspmu *cspmu)
> 	struct device *dev;
> 	struct acpi_apmt_node *apmt_node;
> 	u8 pmu_type;
> -	char *name;
> 	char acpi_hid_string[ACPI_ID_LEN] = { 0 };
> -	static atomic_t pmu_idx[ACPI_APMT_NODE_TYPE_COUNT] = { 0 };
> +	static atomic_t pmu_idx;
> +	u32 id;
>
> 	dev = cspmu->dev;
> 	apmt_node = arm_cspmu_apmt_node(dev);
> 	if (!apmt_node)
> 		return devm_kasprintf(dev, GFP_KERNEL, PMUNAME "_%u",
> -				      atomic_fetch_inc(&pmu_idx[0]));
> +				      atomic_fetch_inc(&pmu_idx));
>
> 	pmu_type = apmt_node->type;
> -
> -	if (pmu_type >= ACPI_APMT_NODE_TYPE_COUNT) {
> +	switch (pmu_type) {
> +	default:
> 		dev_err(dev, "unsupported PMU type-%u\n", pmu_type);
> 		return NULL;
> -	}
> -
> -	if (pmu_type == ACPI_APMT_NODE_TYPE_ACPI) {
> +	case ACPI_APMT_NODE_TYPE_ACPI:
> 		memcpy(acpi_hid_string,
> 			&apmt_node->inst_primary,
> 			sizeof(apmt_node->inst_primary));
> -		name = devm_kasprintf(dev, GFP_KERNEL, "%s_%s_%s_%u", PMUNAME,
> +		return devm_kasprintf(dev, GFP_KERNEL, "%s_%s_%s_%u", PMUNAME,
> 				      arm_cspmu_type_str[pmu_type],
> 				      acpi_hid_string,
> 				      apmt_node->inst_secondary);
> -	} else {
> -		name = devm_kasprintf(dev, GFP_KERNEL, "%s_%s_%d", PMUNAME,
> -				      arm_cspmu_type_str[pmu_type],
> -				      atomic_fetch_inc(&pmu_idx[pmu_type]));
> -	}
> +	case ACPI_APMT_NODE_TYPE_MC:
> +		id = apmt_node->id;
> +		break;
> +	case ACPI_APMT_NODE_TYPE_SMMU:
> +	case ACPI_APMT_NODE_TYPE_PCIE_ROOT:
> +		id = apmt_node->inst_primary;
> +		break;
> +	case ACPI_APMT_NODE_TYPE_CACHE:
> +		id = apmt_node->inst_secondary;
> +		break;
> +	};
>
> -	return name;
> +	return devm_kasprintf(dev, GFP_KERNEL, "%s_%s_%u", PMUNAME,
> +			      arm_cspmu_type_str[pmu_type], id);
> }
>
> static ssize_t arm_cspmu_cpumask_show(struct device *dev,
> -- 
> 2.54.0.dirty
>
>

[PATCH v2 3/4] perf/arm_cspmu: Improve sub-module error reporting

[Robin Murphy]

When waiting for a sub-module to register, we return a bare
-EPROBE_DEFER that ends up showing the end user:

  platform arm-cs-arch-pmu.1: deferred probe pending (no reason)

wherein it's not necessarily clear that they might need to take some
action to ensure the appropriate module is available to load. Let's use
dev_err_probe() here so we can show exactly what we're waiting for.

Similarly, in the case where something's gone horribly wrong with an
already-registered module, we can use dev_WARN() to standardise the
device/driver attribution rather than just open-coding "arm_cspmu".

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>

---
v2: No change
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 8c2dda17d73e..caad9a36651d 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -437,13 +437,15 @@ static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
 				if (ret)
 					module_put(match->module);
 			} else {
-				WARN(1, "arm_cspmu failed to get module: %s\n",
+				dev_WARN(cspmu->dev, "Failed to get module: %s\n",
 					match->module_name);
 				ret = -EINVAL;
 			}
 		} else {
 			request_module_nowait(match->module_name);
-			ret = -EPROBE_DEFER;
+			ret = dev_err_probe(cspmu->dev, -EPROBE_DEFER,
+					    "Waiting for module %s to load\n",
+					    match->module_name);
 		}
 
 		mutex_unlock(&arm_cspmu_lock);
-- 
2.54.0.dirty

[PATCH v2 4/4] perf/arm_cspmu: Make IRQ more optional

[Robin Murphy]

If we have 64-bit counters, we can reasonably assume we'll never have to
handle an overflow before the end of the universe (since we're a system
PMU with no sampling). Thus even if firmware does specify an IRQ, we can
still continue in the event of being unable to request it. This can help
systems where IRQs cannot be claimed exclusively, or are broken in other
ways.

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>

---
v2: No change
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index caad9a36651d..ad82de166dd1 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -1256,8 +1256,11 @@ static int arm_cspmu_device_probe(struct platform_device *pdev)
 		return ret;
 
 	ret = arm_cspmu_request_irq(cspmu);
-	if (ret)
-		return ret;
+	if (ret) {
+		if (counter_size(cspmu) < 64)
+			return ret;
+		dev_info(cspmu->dev, "Continuing without IRQ\n");
+	}
 
 	ret = arm_cspmu_get_cpus(cspmu);
 	if (ret)
-- 
2.54.0.dirty

Re: [PATCH v2 4/4] perf/arm_cspmu: Make IRQ more optional

[sashiko-bot]

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

New issues:
- [High] Unconditional swallowing of `arm_cspmu_request_irq()` error codes (such as `-EPROBE_DEFER` and `-ENOMEM`) for PMUs with >= 64-bit counters.

Pre-existing issues:
- [High] Incorrect ordering between CPU hotplug state management and PMU registration causes Use-After-Free during teardown.
- [High] Missing cleanup of CPU hotplug state if `platform_driver_register()` fails in module init.
- [High] Failed `driver_attach()` in `arm_cspmu_impl_register()` leaves dangling pointers in the `impl_match` array.
- [High] Stack-allocated bitmap cast to `u32 *` leaves memory uninitialized on Big Endian systems, causing reads of uninitialized stack memory.
--

--- Patch [4]: [PATCH v2 4/4] perf/arm_cspmu: Make IRQ more optional ---
commit e7afdbca83ffd562889abdf4a15a47f1af9fc395
Author: Robin Murphy <robin.murphy@arm.com>

perf/arm_cspmu: Make IRQ more optional

This commit allows the driver to continue probing even if it fails to
request an IRQ, provided the PMU has 64-bit counters. It assumes that
64-bit counters will not overflow in a realistic timeframe.

> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index 385d3e34694f2..007038aa7e9a6 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -1256,8 +1256,11 @@ static int arm_cspmu_device_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	ret = arm_cspmu_request_irq(cspmu);
> -	if (ret)
> -		return ret;
> +	if (ret) {
> +		if (counter_size(cspmu) < 64)
> +			return ret;

[Severity: High]
Does this unintentionally swallow error codes like -EPROBE_DEFER or
-ENOMEM for 64-bit PMUs?

If the interrupt controller is not yet ready, arm_cspmu_request_irq() will
return -EPROBE_DEFER via platform_get_irq_optional(). By completely ignoring
the error here, does this result in completing the probe without an IRQ when
we actually just needed to defer it?

Similarly, memory allocation failures (-ENOMEM) from devm_request_irq()
would also be silently ignored.

> +		dev_info(cspmu->dev, "Continuing without IRQ\n");
> +	}
>  
>  	ret = arm_cspmu_get_cpus(cspmu);

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but is there a use-after-free
window during teardown if the PMU is unregistered before the hotplug state
is removed?

In arm_cspmu_device_remove():

    perf_pmu_unregister(&cspmu->pmu);
    cpuhp_state_remove_instance(arm_cspmu_cpuhp_state, &cspmu->cpuhp_node);

Calling perf_pmu_unregister() frees the PMU context. When
cpuhp_state_remove_instance() is called next, it synchronously triggers
arm_cspmu_cpu_teardown() on all online CPUs.

The teardown callback then invokes perf_pmu_migrate_context() using the
already freed PMU context.

A similar sequence appears in the error path of arm_cspmu_register_pmu() if
perf_pmu_register() fails.

[Severity: High]
This isn't a bug introduced by this patch, but does arm_cspmu_init() leak
the CPU hotplug callbacks if platform_driver_register() fails?

In arm_cspmu_init():

    arm_cspmu_cpuhp_state = ret;
    return platform_driver_register(&arm_cspmu_driver);

If platform_driver_register() fails, the module memory is freed. However,
cpuhp_remove_multi_state() is missing from this error path, which leaves
the kernel holding pointers to the freed module callbacks.

Can this cause a kernel panic on subsequent CPU hotplug events?

[Severity: High]
This isn't a bug introduced by this patch, but does arm_cspmu_impl_register()
leave dangling pointers in the static impl_match array on failure?

In arm_cspmu_impl_register():

    if (!match->impl_init_ops) {
        match->module = impl_match->module;
        match->impl_init_ops = impl_match->impl_init_ops;
    }
    ...
    if (!ret)
        ret = driver_attach(&arm_cspmu_driver.driver);

If driver_attach() fails, the function returns an error without clearing
the module and impl_init_ops fields. If the backend module is unloaded,
these pointers become invalid.

Would a subsequent probe of a matching device call the dangling
impl_init_ops pointer?

[Severity: High]
This isn't a bug introduced by this patch, but can the uninitialized stack
bitmap cause logic failures on Big Endian systems?

In arm_cspmu_handle_irq(), pmovs is declared on the stack:

    DECLARE_BITMAP(pmovs, ARM_CSPMU_MAX_HW_CNTRS);

And in arm_cspmu_get_reset_overflow(), it is written to as a u32 array:

    for (i = 0; i < cspmu->num_set_clr_reg; ++i) {
        pmovs[i] = readl(cspmu->base1 + pmovclr_offset);

Writing a u32 sequentially into an unsigned long array on a 64-bit Big
Endian system populates the most-significant 32 bits, leaving the
least-significant 32 bits uninitialized.

Since test_bit() checks bits based on unsigned long layout, could it read
from the uninitialized stack memory, causing spurious or missed hardware IRQs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784213435.git.robin.murphy@arm.com?part=4

Re: [PATCH v2 4/4] perf/arm_cspmu: Make IRQ more optional

[Robin Murphy]

On 16/07/2026 4:11 pm, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> 
> New issues:
> - [High] Unconditional swallowing of `arm_cspmu_request_irq()` error codes (such as `-EPROBE_DEFER` and `-ENOMEM`) for PMUs with >= 64-bit counters.
> 
> Pre-existing issues:
> - [High] Incorrect ordering between CPU hotplug state management and PMU registration causes Use-After-Free during teardown.
> - [High] Missing cleanup of CPU hotplug state if `platform_driver_register()` fails in module init.
> - [High] Failed `driver_attach()` in `arm_cspmu_impl_register()` leaves dangling pointers in the `impl_match` array.
> - [High] Stack-allocated bitmap cast to `u32 *` leaves memory uninitialized on Big Endian systems, causing reads of uninitialized stack memory.
> --
> 
> --- Patch [4]: [PATCH v2 4/4] perf/arm_cspmu: Make IRQ more optional ---
> commit e7afdbca83ffd562889abdf4a15a47f1af9fc395
> Author: Robin Murphy <robin.murphy@arm.com>
> 
> perf/arm_cspmu: Make IRQ more optional
> 
> This commit allows the driver to continue probing even if it fails to
> request an IRQ, provided the PMU has 64-bit counters. It assumes that
> 64-bit counters will not overflow in a realistic timeframe.
> 
>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
>> index 385d3e34694f2..007038aa7e9a6 100644
>> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
>> @@ -1256,8 +1256,11 @@ static int arm_cspmu_device_probe(struct platform_device *pdev)
>>   		return ret;
>>   
>>   	ret = arm_cspmu_request_irq(cspmu);
>> -	if (ret)
>> -		return ret;
>> +	if (ret) {
>> +		if (counter_size(cspmu) < 64)
>> +			return ret;
> 
> [Severity: High]
> Does this unintentionally swallow error codes like -EPROBE_DEFER or
> -ENOMEM for 64-bit PMUs?

No. It does that *intentionally* - this is literally the entire point of 
the patch. If there is an IRQ and we can claim it for the sake of 
correctness then fine, but since in practice we'll never use it, there 
is no point worrying about any error, including a missing interrupt 
controller driver that might otherwise defer forever.

Robin.