Re: [PATCH v2 1/9] perf/cxl: Program the requested event group on configurable counters

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware State Pollution / Missing Bitmask Clear on PMU Counter Reuse
- [High] Invalid 64-bit Write on a 32-bit PCIe Register
- [High] Undefined Behavior Due to Out-of-Bounds Shift in Overflow Handling
--

--- Patch [1]: [PATCH v2 1/9] perf/cxl: Program the requested event group on configurable counters ---
commit 3f34f60eb49c6f10c29064fdd1d2a3cf3e95a287
Author: Dave Jiang <dave.jiang@intel.com>
Date:   Wed Jul 29 14:55:47 2026 +0000

perf/cxl: Program the requested event group on configurable counters

cxl_pmu_get_event_idx() sets *counter_idx for a configurable counter but
never sets *event_idx, so hwc->event_base stays 0 and cxl_pmu_event_start()
programs event group 0 regardless of the group the user requested.

Return the matched capability's event_idx.

Link: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 68a54d97d2a8a..e77c0650a7eb6 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -552,6 +552,8 @@ static int cxl_pmu_get_event_idx(struct perf_event *event, int *counter_idx,
>  			return -EINVAL;
>  
>  		*counter_idx = i;
> +		if (event_idx)
> +			*event_idx = pmu_ev->event_idx;

[Severity: High]
This isn't a bug introduced by this patch, but returning the correct 
event_idx here fully exposes an existing state pollution issue in 
cxl_pmu_event_start().

When a hardware counter is reused, the configuration is bitwise-ORed:

drivers/perf/cxl_pmu.c:cxl_pmu_event_start() {
...
	cfg = readq(base + CXL_PMU_COUNTER_CFG_REG(hwc->idx));
	cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_INT_ON_OVRFLW, 1);
...
	if (test_bit(hwc->idx, info->conf_counter_bm)) {
		cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK,
				  hwc->event_base);
		cfg |= FIELD_PREP(CXL_PMU_COUNTER_CFG_EVENTS_MSK,
				  cxl_pmu_config_get_mask(event));
	}
...
}

Because the previous bits are not cleared, does this code accumulate 
garbage on reused counters (e.g., ORing the new hwc->event_base with 
the old one)?


[Severity: High]
This is another pre-existing issue, but I noticed a potential hardware access
violation in cxl_pmu_event_start() when dealing with HDM filtering:

drivers/perf/cxl_pmu.c:cxl_pmu_event_start() {
...
		writeq(cfg, base + CXL_PMU_FILTER_CFG_REG(hwc->idx, 0));
...
}

Since CXL_PMU_FILTER_CFG_REG is a 32-bit register, could sending a 64-bit
write with writeq() trigger a PCIe Unsupported Request error or silently
overwrite the adjacent Filter 1 register?


[Severity: High]
This is a pre-existing issue, but there appears to be a shift out of bounds
in the overflow handling path:

drivers/perf/cxl_pmu.c:__cxl_pmu_read() {
...
	if (overflow && delta < GENMASK_ULL(info->counter_width - 1, 0))
		delta += (1UL << info->counter_width);
...
}

If the platform supports a counter_width of 64 (or >= 32 on 32-bit
architectures), does the use of 1UL rather than 1ULL invoke undefined
behavior and silently corrupt the delta calculation?

>  		return 0;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729145555.3919550-1-dave.jiang@intel.com?part=1