Re: [PATCH 0/5] KVM: Serialize vCPU creation and revert vcpu_ids tracking

Re: [PATCH 0/5] KVM: Serialize vCPU creation and revert vcpu_ids tracking

[Christian Borntraeger]

Am 14.09.26 um 20:12 schrieb Sean Christopherson:
> Serialize vCPU creation by holding kvm->lock for the entirety of
> kvm_vm_ioctl_create_vcpu(), and then revert the now-redundant tracking adding
> by commit 97d65b544f48 ("KVM: Check for duplicate vcpu_id as early as
> possible").  I botched the math when justifying the vcpu_ids tracking; it's not
> an extra 256 bytes, it's an extra 2048 bytes.  Roughly doubling the size of
> "struct kvm" tripped x86's KVM_SANITY_CHECK_VM_STRUCT_SIZE, and obviously isn't
> something we want to do in general.
> 
> The TL;DR of why it's a-ok to serialize vCPU creation is that no VMM actually
> does parallel vCPU creation.  As with so many things, KVM's current behavior is
> the result of decades-old cruft, not intentional, deliberate design.
> 
> Patch 1 is a tangentially related bug fix; I included it here because holding
> kvm->lock for all of vCPU creation allows WARNing if KVM attempts to lock all
> vCPUs if vCPU creation is in-progress (the caller is must hold kvm->lock).
> 
> Sean Christopherson (5):
>    KVM: Reject attempts to lock all vCPUs if vCPU creation is in-progress
>    KVM: Protect all of kvm_vm_ioctl_create_vcpu() with kvm->lock


Interesting, that would allow to simplify several aspects in s390 kvm code as well. We might
also be able to move most things from postcreate into create.

Before that, the series as is needs some s390 fixups.
s390 takes the kvm->lock in
- kvm_s390_vcpu_setup() locks kvm->lock around kvm_s390_pv_create_cpu() at arch/s390/kvm/s390/s390.c:3760 (added with 29b40f105ec8)
- kvm_arch_vcpu_postcreate() locks kvm->lock around the epoch copy at arch/s390/kvm/s390/s390.c:3582. That dates back to the TOD attribute commit 72f250206f0f.
as far as I can tell.