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

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

[Fuad Tabba]

__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. After the rejected INIT, the first
call's vcpu_load() finds hw_mmu still set to the canonical MMU and
leaves it alone, but its vcpu_put() takes the vcpu_has_nv() branch into
kvm_vcpu_put_hw_mmu(), which clears hw_mmu. The second call's
vcpu_load() finds hw_mmu NULL and takes the nested branch into
get_s2_mmu_nested(), whose search over nested_mmus_size == 0 leaves
s2_mmu NULL for the BUG_ON(atomic_read(&s2_mmu->refcnt)), under
mmu_lock. On kvmarm/next with the series applied:

  Unable to handle kernel NULL pointer dereference at virtual address 0000000000000074
  Call trace:
   kvm_vcpu_load_hw_mmu (arch/arm64/kvm/nested.c:891) (P)
   kvm_arch_vcpu_load (arch/arm64/kvm/arm.c:662)
   kvm_vcpu_pre_fault_memory (virt/kvm/kvm_main.c:170 virt/kvm/kvm_main.c:4349)
   kvm_vcpu_ioctl (virt/kvm/kvm_main.c:4639)

Setup reads the VM-wide bitmap, so the copy can't be deferred; restore
the previous value instead when kvm_setup_vcpu() fails.

Fixes: 427733579744e ("KVM: arm64: Select default PMU in KVM_ARM_VCPU_INIT handler")
Link: https://lore.kernel.org/r/20260825-kvm-arm-prefault-v1-0-befe8947702e@kernel.org/
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
v2:
 - Commit message: say what the prefault series enables, bring the
   two-ioctl walk and the trace up from below the fold, the trace
   decoded, and drop the Fixes: on 1de10b7d13a97, which had nothing
   fallible after the copy (Lorenzo).
 - Fold Lorenzo's Reviewed-by.

Reproduced on kvmarm/next plus the series under QEMU (-cpu max with an
Apple M2 MIDR, which has_nv1() denies; kvm-arm.mode=nested):
KVM_ARM_VCPU_INIT with HAS_EL2 | HAS_EL2_E2H0 returns -EINVAL, then
KVM_PRE_FAULT_MEMORY twice. With the fix both calls return -ENOENT and
the host is unaffected. Applies unchanged to v7.3-rc3.

v1: https://lore.kernel.org/r/20260918120553.163139-1-fuad.tabba@linux.dev/

 arch/arm64/kvm/arm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

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;
+	}
 
 	/* Now we know what it is, we can reset it. */
 	kvm_reset_vcpu(vcpu);

base-commit: 089e4f3c4862ba3f29dff2361caa8084879194fd
-- 
2.39.5

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

[Oliver Upton]

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

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

[Fuad Tabba]

Hi Oliver,

On Mon, 21 Sep 2026 00:09:17 -0700, Oliver Upton <oupton@kernel.org> wrote:
[...]
> > 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.

That one is for Lorenzo then :) I'll reword the message so the stale
bitmap comes first and the pre-fault crash is just how it showed up.

[...]
> 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;
>         }

I'd gone for the restore so the failure path didn't depend on the
flag, but those two states are all there is and your version makes
that explicit. I'll use it in v3.

Thanks,
/fuad

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

[Lorenzo Stoakes (ARM)]

On Mon, Sep 21, 2026 at 08:24:56AM +0100, Fuad Tabba wrote:
> Hi Oliver,
>
> On Mon, 21 Sep 2026 00:09:17 -0700, Oliver Upton <oupton@kernel.org> wrote:
> [...]
> > > 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.
>
> That one is for Lorenzo then :) I'll reword the message so the stale
> bitmap comes first and the pre-fault crash is just how it showed up.

Let me dig into this.

--
Cheers, Lorenzo

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

[Lorenzo Stoakes (ARM)]

On Mon, Sep 21, 2026 at 08:24:56AM +0100, Fuad Tabba wrote:
> Hi Oliver,
>
> On Mon, 21 Sep 2026 00:09:17 -0700, Oliver Upton <oupton@kernel.org> wrote:
> [...]
> > > 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.
>
> That one is for Lorenzo then :) I'll reword the message so the stale
> bitmap comes first and the pre-fault crash is just how it showed up.

In general this is the behaviour of _generic_ KVM code
(kvm_vcpu_pre_fault_memory()), which itself does not require an initialised
vCPU, and no other arch seems to requires that either in the arch-specific
pre-fault code.

The actual pre-faulting implementation intentionally only targets the
canonical MMU to fault in S2 page tables, which doesn't need initialisation
to have taken place AFAICT.

However I could add:

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

To kvm_arch_vcpu_pre_fault_memory(), which would make this a requirement,
though that would bring us out of line with other archs.

OTOH, I don't see why anybody would pre-fault BEFORE init, feels like
something that should be done later on in VMM bring up right?

So I doubt doing this would really break any sane user.

Let me know if you want that change in the series and I can respin it.

--
Cheers, Lorenzo

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

[Oliver Upton]

Hey Lorenzo,

On Mon, Sep 21, 2026 at 02:34:12PM +0100, Lorenzo Stoakes (ARM) wrote:
> On Mon, Sep 21, 2026 at 08:24:56AM +0100, Fuad Tabba wrote:
> > Hi Oliver,
> >
> > On Mon, 21 Sep 2026 00:09:17 -0700, Oliver Upton <oupton@kernel.org> wrote:
> > [...]
> > > > 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.
> >
> > That one is for Lorenzo then :) I'll reword the message so the stale
> > bitmap comes first and the pre-fault crash is just how it showed up.
> 
> In general this is the behaviour of _generic_ KVM code
> (kvm_vcpu_pre_fault_memory()), which itself does not require an initialised
> vCPU, and no other arch seems to requires that either in the arch-specific
> pre-fault code.
> 
> The actual pre-faulting implementation intentionally only targets the
> canonical MMU to fault in S2 page tables, which doesn't need initialisation
> to have taken place AFAICT.
> 
> However I could add:
> 
> 	if (!kvm_vcpu_initialized(vcpu))
> 		return -ENOEXEC;
> 
> To kvm_arch_vcpu_pre_fault_memory(), which would make this a requirement,
> though that would bring us out of line with other archs.

Actually, we need this enforcement to happen before the arch-generic
implementation calls vcpu_load(). So that would be some amount of
kvm_arch_* boilerplate added to the generic code to enforce this
requirement like we do on the other vCPU ioctls.

I'm not too worried about aligning this with other architectures; the
idea of a half-baked vCPU getting loaded is worrying.

> OTOH, I don't see why anybody would pre-fault BEFORE init, feels like
> something that should be done later on in VMM bring up right?
> 
> So I doubt doing this would really break any sane user.
> 
> Let me know if you want that change in the series and I can respin it.

Please do, thank you for addressing this.

Best,
Oliver

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

[Lorenzo Stoakes (ARM)]

On Mon, Sep 21, 2026 at 11:39:33AM -0700, Oliver Upton wrote:
> Hey Lorenzo,
>
> On Mon, Sep 21, 2026 at 02:34:12PM +0100, Lorenzo Stoakes (ARM) wrote:
> > On Mon, Sep 21, 2026 at 08:24:56AM +0100, Fuad Tabba wrote:
> > > Hi Oliver,
> > >
> > > On Mon, 21 Sep 2026 00:09:17 -0700, Oliver Upton <oupton@kernel.org> wrote:
> > > [...]
> > > > > 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.
> > >
> > > That one is for Lorenzo then :) I'll reword the message so the stale
> > > bitmap comes first and the pre-fault crash is just how it showed up.
> >
> > In general this is the behaviour of _generic_ KVM code
> > (kvm_vcpu_pre_fault_memory()), which itself does not require an initialised
> > vCPU, and no other arch seems to requires that either in the arch-specific
> > pre-fault code.
> >
> > The actual pre-faulting implementation intentionally only targets the
> > canonical MMU to fault in S2 page tables, which doesn't need initialisation
> > to have taken place AFAICT.
> >
> > However I could add:
> >
> > 	if (!kvm_vcpu_initialized(vcpu))
> > 		return -ENOEXEC;
> >
> > To kvm_arch_vcpu_pre_fault_memory(), which would make this a requirement,
> > though that would bring us out of line with other archs.
>
> Actually, we need this enforcement to happen before the arch-generic
> implementation calls vcpu_load(). So that would be some amount of
> kvm_arch_* boilerplate added to the generic code to enforce this
> requirement like we do on the other vCPU ioctls.

Ack, will do!

>
> I'm not too worried about aligning this with other architectures; the
> idea of a half-baked vCPU getting loaded is worrying.

Yeah and I don't think anybody in their right mind is relying on being able
to pre-fault before init anyway.

>
> > OTOH, I don't see why anybody would pre-fault BEFORE init, feels like
> > something that should be done later on in VMM bring up right?
> >
> > So I doubt doing this would really break any sane user.
> >
> > Let me know if you want that change in the series and I can respin it.
>
> Please do, thank you for addressing this.

No worries, will put a commit or 2 at the start of the series to action
this and respin.

>
> Best,
> Oliver

--
Cheers, Lorenzo