Re: [PATCH 4/4] perf/arm-cmn: Convert to PERF_PMU_SCOPE_DEVICE

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf/arm-cmn: missing hardware disable on probe error path
--

commit baed79d02fccb15f4bbc09054f99c378f1c3c918
Author: Haris Okanovic <harisokn@amazon.com>

perf/arm-cmn: Convert to PERF_PMU_SCOPE_DEVICE

This commit sets the ARM CMN PMU scope to PERF_PMU_SCOPE_DEVICE and removes
the driver-tracked cpu, hotplug hooks, and the cpumask attribute. It also
defines filter_allowed_cpus() and migrate_cpu() callbacks to keep preferences
for local NUMA node CPUs.

> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> index b162de3d9d169..00ffe45963244 100644
> --- a/drivers/perf/arm-cmn.c
> +++ b/drivers/perf/arm-cmn.c
[ ... ]
> @@ -2881,17 +2836,13 @@ static int arm_cmn_probe(struct platform_device *pdev)
>  	if (!name)
>  		return -ENOMEM;
>  
> -	err = cpuhp_state_add_instance(arm_cmn_hp_state, &cmn->cpuhp_node);
> +	err = perf_pmu_register(&cmn->pmu, name, -1);
>  	if (err)
>  		return err;

[Severity: Medium]
If perf_pmu_register() fails here, does this return without explicitly
disabling the hardware, leaving it in an active state?

In arm_cmn_remove(), we see the driver disables the hardware:

    writel_relaxed(0, cmn->dtc[0].base + CMN_DT_DTC_CTL);

Since arm_cmn_init_dtcs() was called earlier in arm_cmn_probe(), should there
be an error handling path to write 0 to CMN_DT_DTC_CTL if registration fails,
to avoid leaking hardware state?

>  
> -	err = perf_pmu_register(&cmn->pmu, name, -1);
> -	if (err)
> -		cpuhp_state_remove_instance_nocalls(arm_cmn_hp_state, &cmn->cpuhp_node);
> -	else
> -		arm_cmn_debugfs_init(cmn, this_id);
> +	arm_cmn_debugfs_init(cmn, this_id);
>  
> -	return err;
> +	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013185603.1347265-1-harisokn@amazon.com?part=4