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

[Will Deacon <will@kernel.org>]

On Mon, Jul 14, 2025 at 04:09:21PM +0100, Leo Yan wrote:
> 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.

I thought userspace typically had midr-based json files to figure this
stuff out? The supported events aren't probe-able afaict so I don't
think the driver can help.

Will