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

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi Sashiko,

On Thu, Sep 03, 2026 at 04:41:26PM +0000, sashiko-bot@kernel.org wrote:
> 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?

It doesn't look to me like ID_AA64DFR0_EL1.PMSVer is user writable:

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_WRPs_MASK |
                    ID_AA64DFR0_EL1_PMUVer_MASK |
                    ID_AA64DFR0_EL1_DebugVer_MASK),

unless I'm badly misunderstanding what the masks in the ID_FILTERED() macro do.

> 
> 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?

What is "KVM_SET_ONE_REG validation"?

> 
> 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.

I don't think ID_AA64DFR0_EL1.PMSVer is user-writable.

> 
> [ ... ]
> > @@ -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?

Indeed. My understanding from how the PMU VCPU feature is handled is that even
though KVM doesn't forbid running a VCPU without a PMU when other VCPUs do have
one, this kind of setup is not supported in KVM. This is what I was trying to
implement here.

Should I tighten the check and explicitly forbid running a VCPU without a SPE
PMU?

Thanks,
Alex

> 
> 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