Re: [PATCH v2] KVM: arm64: Restore the VM's feature bitmap when kvm_setup_vcpu() fails

[Oliver Upton <oupton@kernel.org>]

Hi Fuad,

On Mon, Sep 21, 2026 at 07:37:18AM +0100, Fuad Tabba wrote:
> __kvm_vcpu_set_target() copies the requested features into the VM-wide
> bitmap before kvm_setup_vcpu() runs and doesn't undo it when setup
> fails, so a rejected KVM_ARM_VCPU_INIT leaves the VM recording features
> that were never set up. With HAS_EL2 | HAS_EL2_E2H0 on a host without
> FEAT_NV1, kvm_vcpu_init_nested() returns -EINVAL before it allocates any
> nested stage-2 MMU, and vcpu_has_nv() is then true with
> nested_mmus_size == 0; its -ENOMEM paths do the same on a VM's first
> INIT.
> 
> Nothing in the tree loads a vCPU whose init failed, so this is latent.
> The upcoming series that enables KVM_PRE_FAULT_MEMORY for arm64 exposes
> it: the generic kvm_vcpu_pre_fault_memory() calls vcpu_load() whether
> or not the vCPU has been initialised.

That's a bug, not a feature. We should require an initialized vcpu for
the ioctl.

> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index eaf583b771931..b25725f91c925 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1685,6 +1685,7 @@ static int kvm_setup_vcpu(struct kvm_vcpu *vcpu)
>  static int __kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>  				 const struct kvm_vcpu_init *init)
>  {
> +	DECLARE_BITMAP(old_features, KVM_VCPU_MAX_FEATURES);
>  	unsigned long features = init->features[0];
>  	struct kvm *kvm = vcpu->kvm;
>  	int ret = -EINVAL;
> @@ -1695,11 +1696,15 @@ static int __kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>  	    kvm_vcpu_init_changed(vcpu, init))
>  		goto out_unlock;
>  
> +	/* Setup reads the VM-wide bitmap, so undo the copy if setup fails. */
> +	bitmap_copy(old_features, kvm->arch.vcpu_features, KVM_VCPU_MAX_FEATURES);
>  	bitmap_copy(kvm->arch.vcpu_features, &features, KVM_VCPU_MAX_FEATURES);
>  
>  	ret = kvm_setup_vcpu(vcpu);
> -	if (ret)
> +	if (ret) {
> +		bitmap_copy(kvm->arch.vcpu_features, old_features, KVM_VCPU_MAX_FEATURES);
>  		goto out_unlock;
> +	}

The bitmap copy is a bit confusing because there's only two possible
situations:

 - KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED is unset and the bitmap was
   previously zero

 - KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED is set and @init->features is
   identical to vcpu_features (see kvm_vcpu_init_changed())

While there's nothing wrong with your diff, I'd prefer if the above
detail was represented directly.

	ret = kvm_setup_vcpu(vcpu);
	if (ret) {
		/*
		 * Clear the bitmap if setup fails on the first vCPU to be
		 * initialized.
		 */
		if (!test_bit(KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED, &kvm->arch.flags))
			bitmap_zero(kvm->arch.vcpu_features, KVM_VCPU_MAX_FEATURES);

		goto out_unlock;
	}

Thanks,
Oliver