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

[Leo Yan <leo.yan@arm.com>]

Hi Will,

On Mon, Jul 14, 2025 at 02:07:32PM +0100, Will Deacon wrote:

[...]

> > +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?

It's fine with me to remove the validation for the event filter.

However, I have the following question in comment below.

> 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.

Then the question is: what information should be exposed to userspace
so that tools can decide which events are valid?

I would suggest to expose a new entry, "caps/version", to indicate the
SPE version number. Tools can use this to apply the appropriate event
validation. Please let me know if this works for you.

Thanks,
Leo