Re: [RFC PATCH v7 05/28] perf: arm_spe_pmu: Add PMBIDR_EL1 and PMSIDR_EL1 to struct arm_spe_pmu

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

I think probably some kind of commit message would be good even for a
trivial change :)

On Thu, Sep 03, 2026 at 05:06:00PM +0100, Alexandru Elisei wrote:
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  drivers/perf/arm_spe_pmu.c       | 3 +++
>  include/linux/perf/arm_spe_pmu.h | 2 ++
>  2 files changed, 5 insertions(+)
>
> diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
> index 79c571a649d5..42ba0924bd02 100644
> --- a/drivers/perf/arm_spe_pmu.c
> +++ b/drivers/perf/arm_spe_pmu.c
> @@ -1095,6 +1095,7 @@ static void __arm_spe_pmu_dev_probe(void *info)
>  			"profiling buffer owned by higher exception level\n");
>  		return;
>  	}
> +	spe_pmu->pmbidr_el1 = reg;

This doesn't seem to be used anywhere in the series?

>
>  	/* Minimum alignment. If it's out-of-range, then fail the probe */
>  	fld = FIELD_GET(PMBIDR_EL1_ALIGN, reg);
> @@ -1107,6 +1108,8 @@ static void __arm_spe_pmu_dev_probe(void *info)
>
>  	/* It's now safe to read PMSIDR and figure out what we've got */
>  	reg = read_sysreg_s(SYS_PMSIDR_EL1);
> +	spe_pmu->pmsidr_el1 = reg;
> +
>  	if (FIELD_GET(PMSIDR_EL1_FE, reg))
>  		spe_pmu->features |= SPE_PMU_FEAT_FILT_EVT;
>
> diff --git a/include/linux/perf/arm_spe_pmu.h b/include/linux/perf/arm_spe_pmu.h
> index af86fd184050..97b14e236fcd 100644
> --- a/include/linux/perf/arm_spe_pmu.h
> +++ b/include/linux/perf/arm_spe_pmu.h
> @@ -21,6 +21,8 @@ struct arm_spe_pmu {
>  	cpumask_t				supported_cpus;
>  	struct hlist_node			hotplug_node;
>
> +	u64					pmbidr_el1;

Equally, this doesn't seem to be used anywhere in the series, maybe drop?

> +	u64					pmsidr_el1;

This field is used in kvm_spe_has_feat_spe_fds() and
kvm_spe_has_feat_spe_fne():

bool kvm_spe_has_feat_spe_fds(struct kvm *kvm)
{
	...
	return kvm_has_feat(kvm, ID_AA64DFR0_EL1, PMSVer, V1P4) &&
	       FIELD_GET(PMSIDR_EL1_FDS, spe_pmu->pmsidr_el1);
}

bool kvm_spe_has_feat_spe_fne(struct kvm *kvm)
{
	...
	return kvm_has_feat(kvm, ID_AA64DFR0_EL1, PMSVer, V1P2) &&
	       FIELD_GET(PMSIDR_EL1_FnE, spe_pmu->pmsidr_el1);
}

So checking for FDS and FnE, but these are available as feature flags as
__arm_spe_pmu_dev_probe() gets these:

static void __arm_spe_pmu_dev_probe(void *info)
{
	...
	if (FIELD_GET(PMSIDR_EL1_FnE, reg))
		spe_pmu->features |= SPE_PMU_FEAT_INV_FILT_EVT;
	...
	if (FIELD_GET(PMSIDR_EL1_FDS, reg))
		spe_pmu->features |= SPE_PMU_FEAT_FDS;
	...
}

And then kvm_spe_has_feat_spe_fds() and kvm_spe_has_feat_spe_fne() can be:

bool kvm_spe_has_feat_spe_fds(struct kvm *kvm)
{
	...
	return kvm_has_feat(kvm, ID_AA64DFR0_EL1, PMSVer, V1P4) &&
	       (spe_pmu->features & SPE_PMU_FEAT_FDS);
}

bool kvm_spe_has_feat_spe_fne(struct kvm *kvm)
{
	...
	return kvm_has_feat(kvm, ID_AA64DFR0_EL1, PMSVer, V1P2) &&
	       (spe_pmu->features & SPE_PMU_FEAT_INV_FILT_EVT);
}

Right?

So it seems you could drop this patch from the series in general? Is there
another reason to get these fields?

Also - there seems to be a divergence in the naming there between FnE and
INV_FILT_EVT, is that intentional?

--
Cheers, Lorenzo