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

[Fuad Tabba]

Hi Lorenzo,

On Fri, 18 Sep 2026 17:10:43 +0100, Lorenzo Stoakes (ARM)
<ljs@xxxxxxxxxx> wrote:
[...]
> > Nothing in the tree loads a vCPU whose init failed, so this is latent.
> > The pending KVM_PRE_FAULT_MEMORY series for arm64 does, and the second
>
> I think this needs clarification :)
>
> The KVM_PRE_FAULT_MEMORY series doesn't do this, or rather it only exposes the
> ability for the pre-existing generic kvm_vcpu_pre_fault_memory() function to be
> invoked by setting flags such as to expose pre-faulting as available for arm64.
>
> So something like:
>
>     The upcoming series that enables KVM_PRE_FAULT_MEMORY exposes an
>     existing bug in how vCPU initialisation is performed as it allows
>     the generic kvm_vcpu_pre_fault_memory() function to be called which
>     calls vcpu_load() before the vCPU might be initialised.
>
> For that bit.

That's what "does" was standing in for. I'll say that the series
enables the generic kvm_vcpu_pre_fault_memory(), which calls
vcpu_load() whether or not the vCPU has been initialised.

>
> > such load NULL-dereferences in get_s2_mmu_nested() under mmu_lock.
>
> Also I think this is a bit unclear, esp. the prior reference to 'second',
> not clear what that refers to. I think you should bring in some of the
> context you provide under the fold for the patch.
[...]

I'll bring the two-ioctl walk up from below the fold: the first
KVM_PRE_FAULT_MEMORY's vcpu_put() clearing hw_mmu, then the second's
vcpu_load() reaching get_s2_mmu_nested() with no nested MMUs to find.

> > Fixes: 1de10b7d13a97 ("KVM: arm64: Get rid of vCPU-scoped feature bitmap")
> > Fixes: 427733579744e ("KVM: arm64: Select default PMU in KVM_ARM_VCPU_INIT handler")
>
> Hmm are 2 fixes really needed? It seems to me that commit 427733579744e
> ("KVM: arm64: Select default PMU in KVM_ARM_VCPU_INIT handler") is the
> right one because that added a step _after_ setting the bitmap that can go
> wrong.

I'd added the first one because it's where the copy into the VM-wide
bitmap moved ahead of the reset, but nothing after the copy could fail
there. I'll drop it and keep 427733579744e.

[...]
> With the comments re: commit message/fixes tag addressed this seems correct
> and a very good spot thanks! We very much need this for the pre fault
> functionality.
>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>

Thanks!

[...]
> I also think you should put the stack trace in the commit message, ideally
> decoded via scripts/decode_stacktrace.sh!

Sure, will do.

Thanks,
/fuad