Re: [PATCH v4 1/4] perf/arm_cspmu: Add Arm system PMU

[Robin Murphy <robin.murphy@arm.com>]

On 2026-11-06 6:17 pm, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] perf/arm_cspmu: arm_cspmu_init_impl_ops() defers probe when
>    ARM_SYSTEM_PMU=n
> --
> 
> commit ae4570e49b496427e88dd55c46f9506b33a8b68e
> Author: Robin Murphy <robin.murphy@arm.com>
> 
> perf/arm_cspmu: Add Arm system PMU
> 
> This commit adds the arm_syspmu backend module to support an additional
> 64-bit event filter register (PMEVFILT3R) on Arm system PMUs with product
> ID 0x9c3.
> 
>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
>> index 912f03ce3ef5f..9f298ea1a9745 100644
>> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
>> @@ -358,6 +358,13 @@ static struct arm_cspmu_impl_match impl_match[] = {
>>   		.module		= NULL,
>>   		.impl_init_ops	= NULL,
>>   	},
>> +	{
>> +		.module_name	= "arm_syspmu",
>> +		.pmiidr_val	= ARM_CSPMU_IMPL_ID_ARM,
>> +		.pmiidr_mask	= PMIIDR_IMPLEMENTER,
>> +		.module		= NULL,
>> +		.impl_init_ops	= NULL,
>> +	},
>>   	{
>>   		.module_name	= "ampere_cspmu",
>>   		.pmiidr_val	= ARM_CSPMU_IMPL_ID_AMPERE,
> 
> [Severity: Medium]
> Does adding an unconditional impl_match[] entry for ARM_CSPMU_IMPL_ID_ARM
> cause arm_cspmu_init_impl_ops() to defer probing indefinitely when
> CONFIG_ARM_SYSTEM_PMU is disabled?
> 
> Previously, CoreSight PMUs with Arm's implementer ID did not match any
> entry in impl_match[] and probed using the generic ops.  With this entry
> present, arm_cspmu_init_impl_ops() requires the arm_syspmu module to
> register match->impl_init_ops:
> 
> drivers/perf/arm_cspmu/arm_cspmu.c:arm_cspmu_init_impl_ops() {
>      ...
> 		if (match->impl_init_ops) {
>      ...
> 		} else {
> 			request_module_nowait(match->module_name);
> 			ret = dev_err_probe(cspmu->dev, -EPROBE_DEFER,
> 					    "Waiting for module %s to load\n",
> 					    match->module_name);
> 		}
>      ...
> }
> 
> Because CONFIG_ARM_SYSTEM_PMU has no default in Kconfig, existing configs
> with CONFIG_ARM_CORESIGHT_PMU_ARCH_SYSTEM_PMU enabled will have
> CONFIG_ARM_SYSTEM_PMU=n, leaving every Arm-implemented CoreSight PMU
> (including non-0x9c3 devices where arm_syspmu_init_ops() is a no-op)
> permanently stuck in -EPROBE_DEFER.

Sure, let me know what other Arm Ltd. PMU implementations those are, and 
I'll get back to you with how much I care...

Plus there's nothing here that's any different from the existing Ampere 
and NVIDIA cases. Perhaps we could #ifdef the impl_match entries if 
anyone really thought it mattered, but that would be a general matter 
for the arm_cspmu framework itself, not this patch.

Thanks,
Robin.