On Mon, Apr 21, 2025, Tom Lendacky wrote: > On 4/1/25 11:10, Paolo Bonzini wrote: > > For userspace to have fun with planes it is probably useful to let them > > create vCPUs on the non-zero planes as well. Since such vCPUs are backed > > by the same struct kvm_vcpu, these are regular vCPU file descriptors except > > that they only allow a small subset of ioctls (mostly get/set) and they > > share some of the backing resources, notably vcpu->run. > > > > TODO: prefault might be useful on non-default planes as well? > > > > Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx> > > --- > > Documentation/virt/kvm/locking.rst | 3 + > > include/linux/kvm_host.h | 4 +- > > include/uapi/linux/kvm.h | 1 + > > virt/kvm/kvm_main.c | 167 +++++++++++++++++++++++------ > > 4 files changed, 142 insertions(+), 33 deletions(-) > > > > > @@ -4200,8 +4223,13 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id) > > * release semantics, which ensures the write is visible to kvm_get_vcpu(). > > */ > > vcpu->plane = -1; > > - vcpu->vcpu_idx = atomic_read(&kvm->online_vcpus); > > - r = xa_insert(&kvm->planes[0]->vcpu_array, vcpu->vcpu_idx, vcpu, GFP_KERNEL_ACCOUNT); > > + if (plane->plane) > > + vcpu->vcpu_idx = atomic_read(&kvm->online_vcpus); > > + else > > + vcpu->vcpu_idx = plane0_vcpu->vcpu_idx; > > Don't you want the atomic_read() for the plane0 vCPU and use the plane0 > vcpu->idx value for non-zero plane vCPUs? +1, this looks backwards to me as well. > > > + > > + r = xa_insert(&plane->vcpu_array, vcpu->vcpu_idx, > > + vcpu, GFP_KERNEL_ACCOUNT); > > WARN_ON_ONCE(r == -EBUSY); > > if (r) > > goto unlock_vcpu_destroy; > > @@ -4220,13 +4248,14 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id) > > if (r < 0) > > goto kvm_put_xa_erase; > > > > - atomic_inc(&kvm->online_vcpus); > > + if (!plane0_vcpu) > > It looks like plane0_vcpu will always have value, either from input or > assigned in an else path earlier in the code. Should this be > "!plane->plane" ? Even if plane0_vcpu were viable, my vote is for !plane->plane, because that makes it much more obvious that only plane0 bumps the count.