linux-kernel - Re: [PATCH 13/29] KVM: implement vCPU creation for extra planes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <729bd284-f1df-d384-8db1-37b448b0c1da@amd.com>
Date: Mon, 21 Apr 2025 17:08:17 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Paolo Bonzini <pbonzini@...hat.com>, linux-kernel@...r.kernel.org,
 kvm@...r.kernel.org
Cc: roy.hopkins@...e.com, seanjc@...gle.com, ashish.kalra@....com,
 michael.roth@....com, jroedel@...e.de, nsaenz@...zon.com, anelkz@...zon.de,
 James.Bottomley@...senPartnership.com
Subject: Re: [PATCH 13/29] KVM: implement vCPU creation for extra planes

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@...hat.com>
> ---
>  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?

> +
> +	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" ?

Thanks,
Tom

> +		atomic_inc(&kvm->online_vcpus);