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

[sashiko-bot@kernel.org]

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