Re: [PATCH v2 4/5] perf/arm-cmn: Plumb in new filter types

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly introduced filter types (SEL_EVICT_STATE_SEL, SEL_ENHANCED_HBT_LBT_SEL, SEL_SNP_VC_SEL) are not mapped to any hardware bitmasks in `arm_cmn_filters`, causing them to be silently ignored when programming the PMU hardware.
--

--- Patch [4]: [PATCH v2 4/5] perf/arm-cmn: Plumb in new filter types ---
commit b05e0afda744759b2e246b65abc52e94120966e6
Author: Robin Murphy <robin.murphy@arm.com>

perf/arm-cmn: Plumb in new filter types

This commit adds logic to handle events with upcoming new filter controls,
special-casing the EVICT_STATE_SEL events which require HBT_LBT_SEL as a
secondary filter.

> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> index 6536f0ec699b7..9d5833f8cff2b 100644
> --- a/drivers/perf/arm-cmn.c
> +++ b/drivers/perf/arm-cmn.c

[ ... ]

> @@ -284,6 +286,9 @@ enum cmn_filter_select {
>  	SEL_CBUSY_SNTHROTTLE_SEL,
>  	SEL_HBT_LBT_SEL,
>  	SEL_SN_HOME_SEL,
> +	SEL_SNP_VC_SEL,
> +	SEL_ENHANCED_HBT_LBT_SEL,
> +	SEL_EVICT_STATE_SEL,
>  	SEL_MAX
>  };

[Severity: Medium]
Will these newly introduced filter types be silently ignored when programming the
PMU hardware?

It looks like they are not mapped to any hardware bitmasks in the
arm_cmn_filters translation array:

drivers/perf/arm-cmn.c:
static const u64 arm_cmn_filters[][SEL_MAX] = {
	[FILT_NONE] = {},
	/* DVM etc. */

Because missing entries default to 0, arm_cmn_set_event_sel_hi() appears to
bypass them when preparing the register payload:

drivers/perf/arm-cmn.c:arm_cmn_set_event_sel_hi() {
    ...
		for (int i = SEL_OCCUP1_ID; i < SEL_MAX; i++) {
			if (filter[i])
				reg |= field_prep(filter[i], dn->filter[i].val);
		}
    ...
}

Should the arm_cmn_filters array be updated to include bitmasks for these new
selectors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785252273.git.robin.murphy@arm.com?part=4