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

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

[hu.shengming]

From: Qifeng He <he.qifeng1@zte.com.cn>

arm_ccn_probe() requests the CCN interrupt before arm_ccn_pmu_init()
has initialised ccn->dt.base.  The interrupt is level-triggered, so a
pending overflow (for example one left behind by a previous kernel
across kexec) is delivered as soon as devm_request_irq() unmasks it.
arm_ccn_pmu_overflow_handler() then dereferences the NULL dt->base:

  NULL pointer dereference at virtual address 0x198
  pc : arm_ccn_pmu_overflow_handler+0x20
  lr : arm_ccn_irq_handler+0x13c

Set up dt->base as soon as the CCN MN block is mapped, before the
interrupt can be requested.  The rest of the DT/PMU setup stays in
arm_ccn_pmu_init(); before the PMU is registered the handler only
needs dt->base and the zero-initialised pmu_counters array, so it is
safe to take the interrupt during that window.

Fixes: a33b0daab73a ("bus: ARM CCN PMU driver")
Cc: stable@vger.kernel.org
Signed-off-by: Qifeng He <he.qifeng1@zte.com.cn>
Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
---
 drivers/perf/arm-ccn.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/arm-ccn.c b/drivers/perf/arm-ccn.c
index 0c69165e0041..3cbdda843b59 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;
-- 
2.25.1

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

[sashiko-bot]

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