[PATCH 01/12] drivers/perf: arm_spe: Store event reserved bits in driver data

[James Clark]

On 13/06/2025 4:53 pm, Leo Yan wrote:
> Store the reserved event bits in the driver structure. This cached value
> avoids redundant calls to arm_spe_pmsevfr_res0() each time a profiling
> session is created.
 > > Signed-off-by: Leo Yan <leo.yan at arm.com>
> ---
>   drivers/perf/arm_spe_pmu.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
> index 3efed8839a4ec5604eba242cb620327cd2a6a87d..be2ed326bb794d7e5dd1d6cfa89330753ced3ca5 100644
> --- a/drivers/perf/arm_spe_pmu.c
> +++ b/drivers/perf/arm_spe_pmu.c
> @@ -77,6 +77,7 @@ struct arm_spe_pmu {
>   	u16					pmsver;
>   	u16					min_period;
>   	u16					counter_sz;
> +	u64					pmsevfr_res0;

IMO this is a premature optimization and we shouldn't store things that 
are only transformations of some other already stored value. It becomes 
another thing to worry about when it's valid to read and to potentially 
keep it up to date etc.

It's just one new one now, but eventually you end up with tons of them 
and then someone forgets to update a dependent one when the parent value 
changes and it makes a mess.

And arm_spe_pmu_event_init() isn't even a particularly hot path.

>   
>   #define SPE_PMU_FEAT_FILT_EVT			(1UL << 0)
>   #define SPE_PMU_FEAT_FILT_TYP			(1UL << 1)
> @@ -722,10 +723,10 @@ static int arm_spe_pmu_event_init(struct perf_event *event)
>   	    !cpumask_test_cpu(event->cpu, &spe_pmu->supported_cpus))
>   		return -ENOENT;
>   
> -	if (arm_spe_event_to_pmsevfr(event) & arm_spe_pmsevfr_res0(spe_pmu->pmsver))
> +	if (arm_spe_event_to_pmsevfr(event) & spe_pmu->pmsevfr_res0)
>   		return -EOPNOTSUPP;
>   
> -	if (arm_spe_event_to_pmsnevfr(event) & arm_spe_pmsevfr_res0(spe_pmu->pmsver))
> +	if (arm_spe_event_to_pmsnevfr(event) & spe_pmu->pmsevfr_res0)
>   		return -EOPNOTSUPP;
>   
>   	if (attr->exclude_idle)
> @@ -1103,6 +1104,8 @@ static void __arm_spe_pmu_dev_probe(void *info)
>   		spe_pmu->counter_sz = 16;
>   	}
>   
> +	spe_pmu->pmsevfr_res0 = arm_spe_pmsevfr_res0(spe_pmu->pmsver);
> +
>   	dev_info(dev,
>   		 "probed SPEv1.%d for CPUs %*pbl [max_record_sz %u, align %u, features 0x%llx]\n",
>   		 spe_pmu->pmsver - 1, cpumask_pr_args(&spe_pmu->supported_cpus),
>