On Mon May 20, 2024 at 7:06 AM AEST, 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.
This is a nice cleanup and helps with ppc hotplug as well.
Has there been any architecture code posted yet?
Just a few minor thing:
>
> 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>
> Reviewed-by: Vishnu Pajjuri <vis...@os.amperecomputing.com>
> ---
> accel/kvm/kvm-all.c | 64 ++++++++++++++++++++++++++++++++----------
> accel/kvm/kvm-cpus.h | 14 +++++++++
> accel/kvm/trace-events | 5 +++-
> 3 files changed, 67 insertions(+), 16 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index c0be9f5eed..9cd7d69bde 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -128,6 +128,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)
> {
> @@ -340,14 +341,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);
> +}
Could you move kvm_get_vcpu up here so it's next to kvm_park_vcpu, and
then you don't need to forward declare it. Call it kvm_unpark_vcpu() for
symmetry with park.
Thanks,
Nick
