Re: [PATCH v3 9/9] perf/cxl: Don't log through pmu.dev in the overflow interrupt handler
[email protected] Fri, 31 Jul 2026 23:46:04 +0000
| Newsgroups | org.kernel.vger.linux-cxl,org.kernel.vger.linux-perf-users |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: Pre-existing issues: - [High] Double counting and cross-event corruption of performance events d= ue to uncleared pending overflows. -- --- Patch [9]: [PATCH v3 9/9] perf/cxl: Don't log through pmu.dev in the ov= erflow interrupt handler --- commit d6b58846ef576f998be916a54f3a5a31363d2d44 Author: Dave Jiang <[email protected]> perf/cxl: Don't log through pmu.dev in the overflow interrupt handler This commit fixes a use-after-free in the CXL PMU overflow interrupt handler by changing the logging device to the parent and clears any stale overflow status before requesting the IRQ. Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolab= s.net?part=3D1 [ ... ] > --- a/drivers/perf/cxl_pmu.c > +++ b/drivers/perf/cxl_pmu.c [ ... ] > @@ -903,6 +903,15 @@ static int cxl_pmu_probe(struct device *dev) > if (!irq_name) > return -ENOMEM; > =20 > + /* > + * Clear any overflow status left set by firmware or a previous kernel > + * before the handler goes live, so it cannot mistake a stale bit for an > + * overflow on a counter no event owns yet. The register is RW1C, and > + * bits above the implemented counters are RsvdZ, so only write those. > + */ > + writeq(GENMASK_ULL(info->num_counters - 1, 0), > + info->base + CXL_PMU_OVERFLOW_REG); > + [Severity: High] This isn't a bug introduced by this patch, but since we are clearing stale= =20 overflow bits here, does cxl_pmu_event_stop() also need to clear the overfl= ow=20 bit when stopping an event? Currently, cxl_pmu_event_stop() in drivers/perf/cxl_pmu.c disables the coun= ter=20 but doesn't clear any pending overflow bit in CXL_PMU_OVERFLOW_REG. If an=20 event overflows just before being stopped (e.g., during PMU multiplexing),= =20 and a new event is started on the same counter or reset via=20 PERF_EVENT_IOC_RESET, the hardware overflow bit remains set. When IRQs are re-enabled, cxl_pmu_irq() handles the pending IRQ and calls __cxl_pmu_read() with overflow=3Dtrue. Because the counter was just reset by cxl_pmu_event_start(), both new_cnt and prev_cnt will be 0.=20 Since new_cnt (0) >=3D prev_cnt (0), would __cxl_pmu_read() falsely apply t= he=20 wraparound fixup: delta =3D (new_cnt - prev_cnt) & mask; if (overflow && new_cnt >=3D prev_cnt) delta +=3D mask + 1; Could this erroneously add a full period (e.g., 2^64 ticks) to the newly started or reset event and corrupt the counter statistics? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260731232827.4014= [email protected]?part=3D9