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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The driver improperly ignores `-EPROBE_DEFER` errors when requesting the IRQ for devices with 64-bit counters, breaking the deferred probe mechanism and leading to non-deterministic boot behavior.
--

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

perf/arm_cspmu: Make IRQ more optional

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.

> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index 1015d86957724..3e32342a609e4 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -1257,8 +1257,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");
> +	}

[Severity: Medium]
Will this improperly ignore -EPROBE_DEFER if the interrupt controller is not yet
probed?

If arm_cspmu_request_irq() returns -EPROBE_DEFER from platform_get_irq_optional():

drivers/perf/arm_cspmu/arm_cspmu.c:arm_cspmu_request_irq() {
    ...
	irq = platform_get_irq_optional(pdev, 0);
	if (irq < 0)
		return irq == -ENXIO ? 0 : irq;
    ...
}

and counter_size(cspmu) >= 64, it looks like this code will mask the error and
continue without an IRQ, rather than deferring the probe. This could lead to
non-deterministic boot behavior depending on probe timing.

Could we explicitly check for and propagate -EPROBE_DEFER here?

>  
>  	ret = arm_cspmu_get_cpus(cspmu);
>  	if (ret)

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