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

[Dave Jiang <dave.jiang@intel.com>]

On 7/30/26 11:53 AM, Jonathan Cameron wrote:
> 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.

I can add a bool as a en/disable state in info to guard against that. > 
>>
>> Thanks,
>> Robin.
>>
>>> +
>>>   	return IRQ_HANDLED;
>>>   }
>>>     
>>
>