Hi Gavin, > From: Gavin Shan <[email protected]> > Sent: Wednesday, September 27, 2023 7:52 AM > To: Salil Mehta <[email protected]>; [email protected]; qemu- > [email protected] > Cc: [email protected]; [email protected]; Jonathan Cameron > <[email protected]>; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > zhukeqian <[email protected]>; wangxiongfeng (C) > <[email protected]>; wangyanan (Y) <[email protected]>; > [email protected]; [email protected]; [email protected] > Subject: Re: [PATCH RFC V2 05/37] accel/kvm: Extract common KVM vCPU > {creation,parking} code > > Hi Salil, > > On 9/26/23 20:04, Salil Mehta wrote: > > KVM vCPU creation is done once during the initialization of the VM when Qemu > > threads are spawned. This is common to all the architectures. If the > > architecture > > supports vCPU hot-{un}plug then this KVM vCPU creation could be deferred to > > later point as well. Some architectures might in any case create KVM vCPUs > > for > > the yet-to-be plugged vCPUs (i.e. QoM Object & thread does not exists) > > during VM > > init time and park them. > > > > Hot-unplug of vCPU results in destruction of the vCPU objects in QOM but > > the KVM vCPU objects in the Host KVM are not destroyed and their > > representative > > KVM vCPU objects in Qemu are parked. > > > > Signed-off-by: Salil Mehta <[email protected]> > > --- > > accel/kvm/kvm-all.c | 61 ++++++++++++++++++++++++++++++++++---------- > > include/sysemu/kvm.h | 2 ++ > > 2 files changed, 49 insertions(+), 14 deletions(-) > > > > The most important point seems missed in the commit log: The KVM vCPU objects, > including those hotpluggable objects, need to be in place before in-host GICv3 > is initialized. So we need expose kvm_create_vcpu() to make those KVM vCPU > objects in place, even for those non-present vCPUs.
This is a patch common to all architectures. The point you are making is specific to the ARM architecture. This patch is now part of the common patch-set. Here, https://lore.kernel.org/qemu-devel/[email protected]/ > > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > > index 7b3da8dc3a..86e9c9ea60 100644 > > --- a/accel/kvm/kvm-all.c > > +++ b/accel/kvm/kvm-all.c > > @@ -137,6 +137,7 @@ static QemuMutex kml_slots_lock; > > #define kvm_slots_unlock() qemu_mutex_unlock(&kml_slots_lock) > > > > static void kvm_slot_init_dirty_bitmap(KVMSlot *mem); > > +static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id); > > > > static inline void kvm_resample_fd_remove(int gsi) > > { > > @@ -320,11 +321,51 @@ err: > > return ret; > > } > > > > +void kvm_park_vcpu(CPUState *cpu) > > +{ > > + unsigned long vcpu_id = cpu->cpu_index; > > + struct KVMParkedVcpu *vcpu; > > + > > + vcpu = g_malloc0(sizeof(*vcpu)); > > + vcpu->vcpu_id = vcpu_id; > > vcpu->vcpu_id = cpu->cpu_index; > > @vcpu_id can be dropped. Yes, agreed. Thanks Salil. > > > + vcpu->kvm_fd = cpu->kvm_fd; > > + QLIST_INSERT_HEAD(&kvm_state->kvm_parked_vcpus, vcpu, node); > > +} > > + > > +int kvm_create_vcpu(CPUState *cpu) > > +{ > > + unsigned long vcpu_id = cpu->cpu_index; > > + KVMState *s = kvm_state; > > + int ret; > > + > > + DPRINTF("kvm_create_vcpu\n"); > > + > > + /* check if the KVM vCPU already exist but is parked */ > > + ret = kvm_get_vcpu(s, kvm_arch_vcpu_id(cpu)); > > + if (ret > 0) { > > + goto found; > > + } > > + > > + /* create a new KVM vcpu */ > > + ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)vcpu_id); > > + if (ret < 0) { > > + return ret; > > + } > > + > > +found: > > + cpu->vcpu_dirty = true; > > + cpu->kvm_fd = ret; > > + cpu->kvm_state = s; > > + cpu->dirty_pages = 0; > > + cpu->throttle_us_per_full = 0; > > + > > + return 0; > > +} > > + > > The found tag can be dropped. @cpu can be initialized if vCPU fd is found > and then bail early. Yes, This patch has been refactored and found has been dropped. https://lore.kernel.org/qemu-devel/[email protected]/ Thanks Salil.
