Re: [PATCH] perf/arm-ccn: Set up the DT base before requesting the IRQ

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] perf/arm-ccn: pending event overflow in arm_ccn_probe() hits
  WARN_ON_ONCE()
--

commit bd2c94bb4323c8419a6bdf342c08aa582e84af55
Author: Qifeng He <he.qifeng1@zte.com.cn>

perf/arm-ccn: Set up the DT base before requesting the IRQ

This commit initializes ccn->dt.base in arm_ccn_probe() before requesting
the CCN interrupt so that a pending overflow interrupt does not dereference
a NULL dt->base.

> diff --git a/drivers/perf/arm-ccn.c b/drivers/perf/arm-ccn.c
> index 0c69165e00418..3cbdda843b591 100644
> --- a/drivers/perf/arm-ccn.c
> +++ b/drivers/perf/arm-ccn.c
> @@ -1213,8 +1213,10 @@ static int arm_ccn_pmu_init(struct arm_ccn *ccn)
>  	char *name;
>  	int err;
>  
> -	/* Initialize DT subsystem */
> -	ccn->dt.base = ccn->base + CCN_REGION_SIZE;
> +	/*
> +	 * Initialize the DT subsystem; dt.base was set up in
> +	 * arm_ccn_probe() before the IRQ was requested.
> +	 */
>  	spin_lock_init(&ccn->dt.config_lock);
>  	writel(CCN_DT_PMOVSR_CLR__MASK, ccn->dt.base + CCN_DT_PMOVSR_CLR);
>  	writel(CCN_DT_CTL__DT_EN, ccn->dt.base + CCN_DT_CTL);
> @@ -1463,6 +1465,14 @@ static int arm_ccn_probe(struct platform_device *pdev)
>  	if (IS_ERR(ccn->base))
>  		return PTR_ERR(ccn->base);
>  
> +	/*
> +	 * The CCN interrupt is level-triggered and may already be pending,
> +	 * e.g. an overflow left by a previous kernel across kexec.  The
> +	 * handler can run as soon as devm_request_irq() below installs it,
> +	 * and it dereferences dt->base, so set it up before that.
> +	 */
> +	ccn->dt.base = ccn->base + CCN_REGION_SIZE;
> +
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0)
>  		return irq;

[Severity: Medium]
Can a pending overflow on an event counter still trigger the WARN_ON_ONCE()
in arm_ccn_pmu_overflow_handler() when devm_request_irq() unmasks the
interrupt?

When devm_request_irq() runs in arm_ccn_probe(), CCN_DT_PMOVSR has not yet
been cleared (commit fa637bf0595e ("bus: arm-ccn: Fix spurious warning
message") added the CCN_DT_PMOVSR_CLR write in arm_ccn_pmu_init() to avoid
warnings on pre-existing overflows), CCN_DT_PMCR is still enabled from the
previous kernel, and dt->pmu_counters[].event are all NULL:

drivers/perf/arm-ccn.c:arm_ccn_pmu_overflow_handler() {
	...
	for (idx = 0; idx < CCN_NUM_PMU_EVENT_COUNTERS + 1; idx++) {
		struct perf_event *event = dt->pmu_counters[idx].event;
		int overflowed = pmovsr & BIT(idx);

		WARN_ON_ONCE(overflowed && !event &&
				idx != CCN_IDX_PMU_CYCLE_COUNTER);
	...
}

If the previous kernel left a pending overflow on any of the event counters
(indices 0..7) rather than only the cycle counter, or an active counter
overflows before arm_ccn_pmu_init() runs, the handler hits WARN_ON_ONCE().
Disabling CCN_DT_PMCR and clearing CCN_DT_PMOVSR_CLR before requesting the
IRQ (or requesting the IRQ after the hardware state is reset) would avoid
taking a stale interrupt with uninitialized counter state.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113163430071vxN-ZVM94Ib9cZ18HMyHa@zte.com.cn?part=1