Re: [PATCH v2 6/9] perf/cxl: Unfreeze counters after handling an overflow interrupt

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 30 Jul 2026 13:04:17 +0100
Robin Murphy <robin.murphy@arm.com> wrote:

> On 29/07/2026 3:55 pm, Dave Jiang wrote:
> > The counters are configured with Freeze on Overflow, so when any counter
> > overflows the CPMU freezes every counter in the block (CXL r4.0
> > §8.2.7.2.1). cxl_pmu_irq() reads the overflowed counters and clears the
> > overflow status, but never writes the CPMU Freeze register to unfreeze,
> > so all counters stay frozen until the next pmu_enable() and events in
> > that window are silently lost.
> > 
> > Unfreeze after clearing the overflow status so counting resumes.
> > 
> > Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
> > Reported-by: sashiko-bot@kernel.org
> > Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > ---
> >   drivers/perf/cxl_pmu.c | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> > index 8b89db8f4d68..52e78a6e0960 100644
> > --- a/drivers/perf/cxl_pmu.c
> > +++ b/drivers/perf/cxl_pmu.c
> > @@ -804,6 +804,15 @@ static irqreturn_t cxl_pmu_irq(int irq, void *data)
> >   
> >   	writeq(overflowed, base + CXL_PMU_OVERFLOW_REG);
> >   
> > +	/*
> > +	 * Counters are configured to freeze on overflow (Freeze on Overflow),
> > +	 * which freezes every counter in the CPMU. Once the overflowed counters
> > +	 * have been read and their status cleared, unfreeze so counting resumes;
> > +	 * otherwise all counters stay frozen until the next pmu_enable() and
> > +	 * events are silently lost.
> > +	 */
> > +	writeq(0, base + CXL_PMU_FREEZE_REG);  
> 
> Does this unconditionally unfreeze _all_ counters, including any which 
> might have have overflowed since the read of CXL_PMU_OVERFLOW_REG and 
> thus have not been handled yet? Or are we hoping that the "global 
> freeze" behaviour prevents that from being able to happen?
> 

Yes global freeze should stop world on first one overflowing - if two go together
we should see them both in the handler.

> Furthermore, what if an overflow happens to occur just as 
> cxl_pmu_disable() is intentionally freezing all the counters, such that 
> by the time we get here, unfreezing them would be the wrong thing to do?

Hmm. That may indeed be bad.

> 
> Thanks,
> Robin.
> 
> > +
> >   	return IRQ_HANDLED;
> >   }
> >     
>