Re: [RESEND PATCH v4 03/11] perf/cxl: Fix the counter overflow delta fixup

[Jonathan Cameron <jic23@kernel.org>]

On Wed,  5 Aug 2026 08:59:03 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> The counter is masked to counter_width, so the subtraction in
> __cxl_pmu_read() throws away the bit that records the wrap. A delta of one
> whole period reads back as 0, the same as no events at all, and only the
> overflow status tells the two apart - which is why __cxl_pmu_read() takes
> an overflow argument.
> 
> The fixup keys off the delta rather than the operands:
> 
> 	delta = (new_cnt - prev_cnt) & GENMASK_ULL(counter_width - 1, 0);
> 	if (overflow && delta < GENMASK_ULL(counter_width - 1, 0))
> 		delta += (1UL << counter_width);
> 
> so it cannot tell which of these it is looking at:
> 
>   event_start         polled read          wrap
>   ctr = 0 ..........  ctr = P/2 .........  mask -> 0 (+r)
>   prev = 0            prev = P/2                 IRQ reads new = r
> 
>   prev = 0    (no read yet):  new >= prev, fell short -> add period
>   prev = P/2  (polled):       new <  prev, spans wrap -> add nothing
> 
> Both rows are the same interrupt and the old guard adds a period in both,
> so a mid-period read makes the event over-count. 'perf stat -I' hits that.
> 
> Condition the fixup on new_cnt >= prev_cnt, the one case the subtraction
> cannot express. Dropping it outright would break the first row. It stays
> exact whatever the residual r is, which matters because some events
> increment by more than 1 per cycle (CXL r4.0 8.2.7.2.1, Threshold) and can
> step past 0 as they wrap.
> 
> Use mask + 1 for the period rather than a shift. It is 0 for a 64-bit
> counter, and avoids the old 1UL << counter_width - undefined for width 64,
> and for >= 32 on 32-bit kernels.
> 
> 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>
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>