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

On 7/29/26 3:29 PM, Jonathan Cameron wrote:
> On Wed, 29 Jul 2026 07:55:47 -0700
> Dave Jiang <[email protected]> wrote:
> 
>> 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.
>>
>> Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
>> Reported-by: [email protected]
>> Closes: https://sashiko.dev/#/patchset/[email protected]?part=1
>> Assisted-by: Claude:claude-opus-4-8
>> Signed-off-by: Dave Jiang <[email protected]>
> 
> One minor thing.  Otherwise LGTM
> Reviewed-by: Jonathan Cameron <[email protected]>
> 
>> ---
> 
>>  drivers/perf/cxl_pmu.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
>> index 68a54d97d2a8..e77c0650a7eb 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)
> Hmm. We never want to get counter_idx and not event_idx.  Perhaps
> add this check to the one for counter_idx at the top of this scope.

Ok I'll address that.

> 
>> +			*event_idx = pmu_ev->event_idx;
>>  		return 0;
>>  	}
>>  
>