Re: [RFC PATCH v7 14/28] KVM: arm64: Add SPE VCPU device attribute to initialize SPE

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

On Thu, Sep 03, 2026 at 05:06:09PM +0100, Alexandru Elisei wrote:
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 8ffa3e89d38c..c695aae484d3 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -449,6 +449,7 @@ enum {
>  #define KVM_ARM_VCPU_SPE_CTRL		3
>  #define   KVM_ARM_VCPU_SPE_IRQ		0
>  #define   KVM_ARM_VCPU_SPE_ID		1
> +#define   KVM_ARM_VCPU_SPE_INIT		2
>
>  /* KVM_IRQ_LINE irq field index values */
>  #define KVM_ARM_IRQ_VCPU2_SHIFT		28
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 132df44c3088..c9a026c61ce7 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1026,6 +1026,12 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
>  			return ret;
>  	}
>
> +	if (vcpu_has_spe(vcpu)) {
> +		ret = kvm_spe_vcpu_first_run_init(vcpu);
> +		if (ret)
> +			return ret;
> +	}
> +

So, staring at patch 18 made me think about ordering again, which seems to be
_super_ sensitive in KVM :)

The next patch in the series (15) makes PMSVer depend on the SPE instance having
been assigned via KVM_ARM_VCPU_SPE_ID.

But you can still KVM_RUN before KVM_ARM_VCPU_SPE_INIT, which means this will
return -EINVAL:

int kvm_spe_vcpu_first_run_init(struct kvm_vcpu *vcpu)
{
	...
	if (!kvm_vcpu_spe_initialized(vcpu))
		return -EINVAL;
	...
}

And this is quite far down the function:

int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
{
	struct kvm *kvm = vcpu->kvm;
	int ret;

	if (!kvm_vcpu_initialized(vcpu))
		return -ENOEXEC;

	if (!kvm_arm_vcpu_is_finalized(vcpu))
		return -EPERM;

	if (likely(vcpu_has_run_once(vcpu)))
		return 0;

	...

	ret = kvm_vcpu_finalize_sys_regs(vcpu);
	if (ret)
		return ret;

	...

	/*
	 * This needs to happen after any restriction has been applied
	 * to the feature set.
	 */
	kvm_calculate_traps(vcpu);

	...

	if (vcpu_has_spe(vcpu)) {
		ret = kvm_spe_vcpu_first_run_init(vcpu);
		if (ret)
			return ret;
	}

	...
}

patch 18 made me think of this as kvm_calculate_traps() does the FGU
initialisation (which derives from SPE state), and once done sets
KVM_ARCH_FLAG_FGU_INITIALIZED, meaning this state is incorrectly setup.

This is a real problem as HAS_RAN_ONCE isn't set on that failed path,
userland can finish the SPE setup, set KVM_ARCH_FLAG_FGU_INITIALIZED, and
that state is never recomputed.

Also kvm_vcpu_finalize_sys_regs() calls kvm_spe_finalize_idregs() (via
kvm_vm_finalize_sys_regs()) which writes PMSVer in the first place.

So it seems you should put this check _before_ these, e.g.:

int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
{
	struct kvm *kvm = vcpu->kvm;
	int ret;

	if (!kvm_vcpu_initialized(vcpu))
		return -ENOEXEC;

	if (!kvm_arm_vcpu_is_finalized(vcpu))
		return -EPERM;

	if (likely(vcpu_has_run_once(vcpu)))
		return 0;

	...

>>>>
	if (vcpu_has_spe(vcpu)) {
		ret = kvm_spe_vcpu_first_run_init(vcpu);
		if (ret)
			return ret;
	}
>>>

	ret = kvm_vcpu_finalize_sys_regs(vcpu);
	if (ret)
		return ret;

	...

	/*
	 * This needs to happen after any restriction has been applied
	 * to the feature set.
	 */
	kvm_calculate_traps(vcpu);

	...

}

--
Cheers, Lorenzo