Re: [PATCH v3 01/14] drivers/perf: arm_spe: Expose event filter

[Will Deacon <will@kernel.org>]

On Mon, Jul 07, 2025 at 02:39:23PM +0100, Leo Yan wrote:
> Expose an "event_filter" entry in the caps folder to inform user space
> about which events can be filtered.
> 
> Change the return type of arm_spe_pmu_cap_get() from u32 to u64 to
> accommodate the added event filter entry.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/perf/arm_spe_pmu.c | 36 ++++++++++++++++++++----------------
>  1 file changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
> index 3efed8839a4ec5604eba242cb620327cd2a6a87d..78d8cb59b66d7bc6319eb4ee40e6d2d32ffb8bdf 100644
> --- a/drivers/perf/arm_spe_pmu.c
> +++ b/drivers/perf/arm_spe_pmu.c
> @@ -115,6 +115,7 @@ enum arm_spe_pmu_capabilities {
>  	SPE_PMU_CAP_FEAT_MAX,
>  	SPE_PMU_CAP_CNT_SZ = SPE_PMU_CAP_FEAT_MAX,
>  	SPE_PMU_CAP_MIN_IVAL,
> +	SPE_PMU_CAP_EVENT_FILTER,
>  };
>  
>  static int arm_spe_pmu_feat_caps[SPE_PMU_CAP_FEAT_MAX] = {
> @@ -122,7 +123,21 @@ static int arm_spe_pmu_feat_caps[SPE_PMU_CAP_FEAT_MAX] = {
>  	[SPE_PMU_CAP_ERND]	= SPE_PMU_FEAT_ERND,
>  };
>  
> -static u32 arm_spe_pmu_cap_get(struct arm_spe_pmu *spe_pmu, int cap)
> +static u64 arm_spe_pmsevfr_res0(u16 pmsver)
> +{
> +	switch (pmsver) {
> +	case ID_AA64DFR0_EL1_PMSVer_IMP:
> +		return PMSEVFR_EL1_RES0_IMP;
> +	case ID_AA64DFR0_EL1_PMSVer_V1P1:
> +		return PMSEVFR_EL1_RES0_V1P1;
> +	case ID_AA64DFR0_EL1_PMSVer_V1P2:
> +	/* Return the highest version we support in default */
> +	default:
> +		return PMSEVFR_EL1_RES0_V1P2;
> +	}
> +}

Hmm. This logic was already a little shakey and so I'm not sure it's a
good idea to expose it directly to userspace. Maintaining RES0 masks for
different versions of SPE won't scale and there are already things that
we can't sensibly handle. For example, E[8]:

  | When (FEAT_SPEv1p4 is implemented or filtering on event 8 is
  | optionally supported) and event 8 is implemented:

So, stepping back, can we remove this stuff altogether? The bits are
RAZ/WI in the case that the even is not implement, but that means that:

  | Software can rely on the field reading as all 0s, and on writes being
  | ignored.

so why are we even bothering to police this?

In other words, remove arm_spe_pmsevfr_res0() and the two checks that
use it in arm_spe_pmu_event_init(). If userspace tries to filter events
that aren't implemented, then it gets to keep the pieces.

Will