Re: [RFC PATCH v7 15/28] KVM: arm64: Use PMSVer from the assigned SPE instance

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] kvm_spe_finalize_idregs() unconditionally overwrites the user-provided ID_AA64DFR0_EL1.PMSVer with the host's physical version on the first KVM_RUN, breaking live migration.
- [High] sanitise_id_aa64dfr0_el1() evaluates PMSVer dynamically based on the assigned SPE PMU, returning 0 if called before the PMU is assigned, which breaks KVM_SET_ONE_REG validation.
- [High] kvm_spe_finalize_idregs() is skipped if a vCPU without SPE runs first in a heterogeneous VM, permanently leaving PMSVer as 0 and hiding SPE from the guest.
--

commit 45e0320e2eaeb926a33b978420aa5a133c7a42c6
Author: Alexandru Elisei <alexandru.elisei@arm.com>

KVM: arm64: Use PMSVer from the assigned SPE instance

This commit updates KVM to initialize the VM's ID_AA64DFR0_EL1.PMSVer using
the SPE PMU instance assigned to the VM rather than the physical CPU executing
the vCPU init ioctl. This is intended to handle heterogeneous systems where
SPE instances may have different versions or be entirely absent on some CPUs.

> diff --git a/arch/arm64/kvm/spe.c b/arch/arm64/kvm/spe.c
> index fa87d224c8af8..4426d614a9e0b 100644
> --- a/arch/arm64/kvm/spe.c
> +++ b/arch/arm64/kvm/spe.c
[ ... ]
> @@ -87,6 +87,30 @@ bool kvm_spe_pmu_set(struct kvm *kvm)
>  	return !!kvm->arch.kvm_spe.spe_pmu;
>  }
>  
> +void kvm_spe_finalize_idregs(struct kvm *kvm)
> +{
> +	struct arm_spe_pmu *spe_pmu = kvm->arch.kvm_spe.spe_pmu;
> +	u64 reg;
> +
> +	if (!spe_pmu)
> +		return;
> +
> +	reg = kvm_read_vm_id_reg(kvm, SYS_ID_AA64DFR0_EL1);
> +	reg &= ~ID_AA64DFR0_EL1_PMSVer_MASK;
> +	reg |= SYS_FIELD_PREP(ID_AA64DFR0_EL1, PMSVer, spe_pmu->pmsver);
> +	kvm_set_vm_id_reg(kvm, SYS_ID_AA64DFR0_EL1, reg);
> +}

[Severity: High]
Does this unconditionally overwrite the user-provided ID_AA64DFR0_EL1.PMSVer
value on the first KVM_RUN?

If a VMM restores ID_AA64DFR0_EL1 during live migration (for example, when
migrating from an older host to a newer one), kvm_spe_finalize_idregs() will
silently clobber the userspace-configured value and force it back to the host
hardware's version. Could this violate the KVM ABI and break live migration
sequences?

[ ... ]
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 44aae52c473d7..a6bb6884b965a 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2220,8 +2220,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(vcpu->kvm));

[Severity: High]
Can this dynamic evaluation break KVM_SET_ONE_REG validation during VM setup?

VMMs like QEMU typically restore VM ID registers via KVM_SET_ONE_REG
immediately after KVM_ARM_VCPU_INIT, before configuring device attributes.
Because sanitise_id_aa64dfr0_el1() evaluates PMSVer based on the assigned SPE
PMU, kvm_spe_get_pmsver() returns 0 until the SPE PMU is assigned.

This strict ordering dependency will cause KVM_SET_ONE_REG to reject valid,
non-zero userspace values with -EINVAL, which breaks standard live migration
sequences.

[ ... ]
> @@ -5928,6 +5931,9 @@ int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu)
>  		kvm_vgic_finalize_idregs(kvm);
>  	}
>  
> +	if (vcpu_has_spe(vcpu))
> +		kvm_spe_finalize_idregs(kvm);
> +
>  	return 0;
>  }

[Severity: High]
Can this leave PMSVer permanently set to 0 in a heterogeneous VM if a vCPU
without SPE runs first?

kvm_finalize_sys_regs() executes on the first KVM_RUN for a given VM. It uses
a per-vCPU feature check (vcpu_has_spe(vcpu)) to guard the VM-wide ID register
initialization in kvm_spe_finalize_idregs(kvm).

If the first scheduled vCPU happens to lack SPE, vcpu_has_spe(vcpu) evaluates
to false and kvm_spe_finalize_idregs(kvm) is skipped. The VM is then marked
with KVM_ARCH_FLAG_HAS_RAN_ONCE, meaning SPE initialization is skipped forever.
This would permanently hide SPE from all vCPUs in the VM.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903160623.315525-1-alexandru.elisei@arm.com?part=15