Hi Philippe,
> From: Philippe Mathieu-Daudé <phi...@linaro.org>
> Sent: Friday, May 3, 2024 10:40 AM
> Subject: Re: [PATCH V8 1/8] accel/kvm: Extract common KVM vCPU
> {creation,parking} code
>
> Hi Salil,
>
> On 12/3/24 02:59, Salil Mehta wrote:
> > KVM vCPU creation is done once during the vCPU realization when Qemu
> > vCPU thread is spawned. This is common to all the architectures as of now.
> >
> > Hot-unplug of vCPU results in destruction of the vCPU object in QOM
> > but the corresponding KVM vCPU object in the Host KVM is not destroyed
> > as KVM doesn't support vCPU removal. Therefore, its representative KVM
> > vCPU object/context in Qemu is parked.
> >
> > Refactor architecture common logic so that some APIs could be reused
> > by vCPU Hotplug code of some architectures likes ARM, Loongson etc.
> > Update new/old APIs with trace events instead of DPRINTF. No functional
> change is intended here.
> >
> > Signed-off-by: Salil Mehta <salil.me...@huawei.com>
> > Reviewed-by: Gavin Shan <gs...@redhat.com>
> > Tested-by: Vishnu Pajjuri <vis...@os.amperecomputing.com>
> > Reviewed-by: Jonathan Cameron <jonathan.came...@huawei.com>
> > Tested-by: Xianglai Li <lixiang...@loongson.cn>
> > Tested-by: Miguel Luis <miguel.l...@oracle.com>
> > Reviewed-by: Shaoqin Huang <shahu...@redhat.com>
> > ---
> > accel/kvm/kvm-all.c | 64 ++++++++++++++++++++++++++++++++------
> ----
> > accel/kvm/trace-events | 5 +++-
> > include/sysemu/kvm.h | 16 +++++++++++
> > 3 files changed, 69 insertions(+), 16 deletions(-)
> >
> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index
> > a8cecd040e..3bc3207bda 100644
> > --- a/accel/kvm/kvm-all.c
> > +++ b/accel/kvm/kvm-all.c
> > @@ -126,6 +126,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)
> > {
> > @@ -314,14 +315,53 @@ err:
> > return ret;
> > }
> >
> > +void kvm_park_vcpu(CPUState *cpu)
> > +{
> > + struct KVMParkedVcpu *vcpu;
> > +
> > + trace_kvm_park_vcpu(cpu->cpu_index, kvm_arch_vcpu_id(cpu));
> > +
> > + vcpu = g_malloc0(sizeof(*vcpu));
> > + vcpu->vcpu_id = kvm_arch_vcpu_id(cpu);
> > + 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 = kvm_arch_vcpu_id(cpu);
> > + KVMState *s = kvm_state;
> > + int kvm_fd;
> > +
> > + trace_kvm_create_vcpu(cpu->cpu_index, kvm_arch_vcpu_id(cpu));
> > +
> > + /* check if the KVM vCPU already exist but is parked */
> > + kvm_fd = kvm_get_vcpu(s, vcpu_id);
> > + if (kvm_fd < 0) {
> > + /* vCPU not parked: create a new KVM vCPU */
> > + kvm_fd = kvm_vm_ioctl(s, KVM_CREATE_VCPU, vcpu_id);
> > + if (kvm_fd < 0) {
> > + error_report("KVM_CREATE_VCPU IOCTL failed for vCPU %lu",
> vcpu_id);
> > + return kvm_fd;
> > + }
> > + }
> > +
> > + cpu->kvm_fd = kvm_fd;
> > + cpu->kvm_state = s;
> > + cpu->vcpu_dirty = true;
> > + cpu->dirty_pages = 0;
> > + cpu->throttle_us_per_full = 0;
> > +
> > + return 0;
> > +}
>
> This seems generic enough to be implemented for all accelerators.
>
> See AccelOpsClass in include/sysemu/accel-ops.h.
>
> That said, can be done later on top.
Let me understand correctly. Are you suggesting to implement above even for
HVF, TCG, QTEST etc?
Thanks
Salil.
