Re: [RFC PATCH v6 16/35] KVM: arm64: Advertise SPE version in ID_AA64DFR0_EL1.PMSver

Suzuki K Poulose
2025-12-16T11:40:30Z

On 14/11/2025 16:06, Alexandru Elisei wrote:
> The VCPU registers are reset during the KVM_ARM_VCPU_INIT ioctl, before
> userspace can set the desired SPU. Assume that the VCPU is initialized from

> a thread that runs on one of the physical CPUs that correspond to the SPU
> that userspace will choose for the VM. Set PMSVer to that CPUs hardware
> value.

This doesn't match the code. See below.

>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> arch/arm64/include/asm/kvm_spe.h | 6 ++++++
> arch/arm64/kvm/spe.c | 10 ++++++++++
> arch/arm64/kvm/sys_regs.c | 10 +++++++++-
> 3 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/kvm_spe.h b/arch/arm64/include/asm/kvm_spe.h
> index 6ce70cf2abaf..5e6d7e609a48 100644
> --- a/arch/arm64/include/asm/kvm_spe.h
> +++ b/arch/arm64/include/asm/kvm_spe.h
> @@ -33,6 +33,8 @@ static __always_inline bool kvm_supports_spe(void)
> void kvm_spe_init_vm(struct kvm *kvm);
> int kvm_spe_vcpu_first_run_init(struct kvm_vcpu *vcpu);
>
> +u8 kvm_spe_get_pmsver_limit(void);
> +
> int kvm_spe_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
> int kvm_spe_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
> int kvm_spe_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
> @@ -53,6 +55,10 @@ static inline int kvm_spe_vcpu_first_run_init(struct kvm_vcpu *vcpu)
> {
> return 0;
> }
> +static inline u8 kvm_spe_get_pmsver_limit(void)
> +{
> + return 0;
> +}
> static inline int kvm_spe_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
> {
> return -ENXIO;
> diff --git a/arch/arm64/kvm/spe.c b/arch/arm64/kvm/spe.c
> index 6bd074e40f6c..0c4896c6a873 100644
> --- a/arch/arm64/kvm/spe.c
> +++ b/arch/arm64/kvm/spe.c
> @@ -68,6 +68,16 @@ int kvm_spe_vcpu_first_run_init(struct kvm_vcpu *vcpu)
> return 0;
> }
>
> +u8 kvm_spe_get_pmsver_limit(void)
> +{
> + unsigned int pmsver;
> +
> + pmsver = SYS_FIELD_GET(ID_AA64DFR0_EL1, PMSVer,
> + read_sanitised_ftr_reg(SYS_ID_AA64DFR0_EL1));

The read_sanitised_ftr_reg() gives you the system wide sanitised
version, not the one on the current CPU. You may need
read_sysreg_s() instead here.

> +
> + return min(pmsver, ID_AA64DFR0_EL1_PMSVer_V1P5);
> +}
> +
> static u64 max_buffer_size_to_pmbidr_el1(u64 size)
> {
> u64 msb_idx, num_bits;
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index e67eb39ddc11..ac859c39c2be 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -29,6 +29,7 @@
> #include <asm/kvm_hyp.h>
> #include <asm/kvm_mmu.h>
> #include <asm/kvm_nested.h>
> +#include <asm/kvm_spe.h>
> #include <asm/perf_event.h>
> #include <asm/sysreg.h>
>
> @@ -1652,6 +1653,9 @@ static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
> case ID_AA64DFR0_EL1_DebugVer_SHIFT:
> kvm_ftr.type = FTR_LOWER_SAFE;
> break;
> + case ID_AA64DFR0_EL1_PMSVer_SHIFT:
> + kvm_ftr.type = FTR_LOWER_SAFE;

PMSVer is already FTR_LOWER_SAFE, and we don't need to override it here
? (unlike the DebugVer or PMU Ver)

> + break;
> }
> break;
> case SYS_ID_DFR0_EL1:
> @@ -2021,8 +2025,11 @@ static u64 sanitise_id_aa64dfr0_el1(const struct kvm_vcpu *vcpu, u64 val)
> val |= SYS_FIELD_PREP(ID_AA64DFR0_EL1, PMUVer,
> kvm_arm_pmu_get_pmuver_limit());
>
> - /* Hide SPE from guests */
> val &= ~ID_AA64DFR0_EL1_PMSVer_MASK;
> + if (vcpu_has_spe(vcpu)) {
> + val |= SYS_FIELD_PREP(ID_AA64DFR0_EL1, PMSVer,
> + kvm_spe_get_pmsver_limit());
> + }

So, we ignore value that the user sets and go with what the SPE instance
that has been chosen ? Should we make it non-writable then ?

Suzuki

>
> /* Hide BRBE from guests */
> val &= ~ID_AA64DFR0_EL1_BRBE_MASK;
> @@ -3209,6 +3216,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> */
> ID_FILTERED(ID_AA64DFR0_EL1, id_aa64dfr0_el1,
> ID_AA64DFR0_EL1_DoubleLock_MASK |
> + ID_AA64DFR0_EL1_PMSVer_MASK |
> ID_AA64DFR0_EL1_WRPs_MASK |
> ID_AA64DFR0_EL1_PMUVer_MASK |
> ID_AA64DFR0_EL1_DebugVer_MASK),