From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D6FC4C61D97 for ; Fri, 24 Nov 2023 16:03:40 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1r6Ydi-0003aL-9N; Fri, 24 Nov 2023 11:02:48 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1r6YdR-0003Zg-0W for qemu-devel@nongnu.org; Fri, 24 Nov 2023 11:02:33 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1r6YdO-0005N0-Ub for qemu-devel@nongnu.org; Fri, 24 Nov 2023 11:02:28 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1700841744; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8P0KL1fEY9kxdqndJJeI7mDazu1N0fp4Dqr+BGe7acQ=; b=hw9J8LAG4DBppvmR9TmiEWTO5uPAm7t2ius9lwaOkcqrXQrtrCA9Si9ykbY90373GGNybL jDcA0j0GWsbTad05aXJ49GAWFyGydL2yQcewKq8vqJHKk2r4yueB42jvXrOLYGhM4Ko6CX BV4TLp6iYZuutB8wPxcfBw1Z/nLXfog= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-387-sWsz5LfzPJujyFo9jYI3CQ-1; Fri, 24 Nov 2023 11:02:22 -0500 X-MC-Unique: sWsz5LfzPJujyFo9jYI3CQ-1 Received: by mail-ed1-f70.google.com with SMTP id 4fb4d7f45d1cf-543298e3cc8so1451189a12.3 for ; Fri, 24 Nov 2023 08:02:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700841741; x=1701446541; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=8P0KL1fEY9kxdqndJJeI7mDazu1N0fp4Dqr+BGe7acQ=; b=qdzv21JybZ6oYy7u4QF573/IeXZu2DSVQqA+AcsT4y7BPPWm7BULlcNkLpQpbZgy4D ANg077Yzdc/8hmF6/A3DJOUpEIl0mAtQNgNfhd0vNejmlURURrJnBzTZJw+T9ZY6Gepf T9nQmjCQc7TRwz+GAy/ovnBwZT8NXPrI6KsoV4OQh2WJShWzMxLCGcphAvDVcTTKE38f aeBcA7IyTMa2H+gGLQw4NVbm2a4vl7bkHlWDb3wDNjwVGKG+Vw+YKbg4Of+5SJa8Tglq QI4eOMRLt4KZ2E3HaRAcbZSdPdiFS8u2pWIsh8xQ/RHvJqYG9+eDf45IPe91n4VjGVS5 eFBQ== X-Gm-Message-State: AOJu0Yxp3cw5b9s0t+e+ziTpyNBrL6o2XnqUiE1iORlfX3oTDnF9P4U1 183SNYEPDWFW8urVCl/DSy5ApgJGEJpNMlodFlErdpLrp149ExsFzm8hCVxyKhs0uA2sbO7MItn mYdA1O7InPAT15GY= X-Received: by 2002:a05:6402:514a:b0:54b:a7b:8198 with SMTP id n10-20020a056402514a00b0054b0a7b8198mr832009edd.17.1700841741345; Fri, 24 Nov 2023 08:02:21 -0800 (PST) X-Google-Smtp-Source: AGHT+IFIl82ntwMaTszRoy9sw1ySPbzPDX/jdgovO9aUXbyQaYTPxywpvP9EDByOoRSAkwdPtpYgFg== X-Received: by 2002:a05:6402:514a:b0:54b:a7b:8198 with SMTP id n10-20020a056402514a00b0054b0a7b8198mr831955edd.17.1700841740604; Fri, 24 Nov 2023 08:02:20 -0800 (PST) Received: from imammedo.users.ipa.redhat.com ([185.140.112.229]) by smtp.gmail.com with ESMTPSA id v28-20020aa7cd5c000000b00548d649f638sm1929571edw.96.2023.11.24.08.02.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Nov 2023 08:02:20 -0800 (PST) Date: Fri, 24 Nov 2023 17:02:17 +0100 From: Igor Mammedov To: Salil Mehta Cc: , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , Subject: Re: [PATCH V7 1/8] accel/kvm: Extract common KVM vCPU {creation,parking} code Message-ID: <20231124170217.40165579@imammedo.users.ipa.redhat.com> In-Reply-To: <20231113201236.30668-2-salil.mehta@huawei.com> References: <20231113201236.30668-1-salil.mehta@huawei.com> <20231113201236.30668-2-salil.mehta@huawei.com> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=170.10.129.124; envelope-from=imammedo@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org On Mon, 13 Nov 2023 20:12:29 +0000 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 > Reviewed-by: Gavin Shan > Tested-by: Vishnu Pajjuri > Reviewed-by: Jonathan Cameron > Tested-by: Xianglai Li > Tested-by: Miguel Luis > Reviewed-by: Shaoqin Huang even though it will used by arm and loongarch later on with respective cpu hotplug series, it doesn't justify including refactoring without actual user. (Put yourself in shoes of someone looking at git history and not finding a single user) I'd suggest to include this patch in both (ARM & Loongson) CPU hotplug series, this way, whichever is merged 1st will get this patch in without causing conflicts. > --- > accel/kvm/kvm-all.c | 64 ++++++++++++++++++++++++++++++++---------- > accel/kvm/trace-events | 4 +++ > include/sysemu/kvm.h | 16 +++++++++++ > 3 files changed, 69 insertions(+), 15 deletions(-) > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index 72e1d1141c..bfa7816aaa 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,14 +321,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; > +} > + > static int do_kvm_destroy_vcpu(CPUState *cpu) > { > KVMState *s = kvm_state; > long mmap_size; > - struct KVMParkedVcpu *vcpu = NULL; > int ret = 0; > > - DPRINTF("kvm_destroy_vcpu\n"); > + trace_kvm_destroy_vcpu(cpu->cpu_index, kvm_arch_vcpu_id(cpu)); > > ret = kvm_arch_destroy_vcpu(cpu); > if (ret < 0) { > @@ -353,10 +393,7 @@ static int do_kvm_destroy_vcpu(CPUState *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); > + kvm_park_vcpu(cpu); > err: > return ret; > } > @@ -377,6 +414,8 @@ static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id) > if (cpu->vcpu_id == vcpu_id) { > int kvm_fd; > > + trace_kvm_get_vcpu(vcpu_id); > + > QLIST_REMOVE(cpu, node); > kvm_fd = cpu->kvm_fd; > g_free(cpu); > @@ -384,7 +423,7 @@ static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id) > } > } > > - return kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)vcpu_id); > + return -ENOENT; > } > > int kvm_init_vcpu(CPUState *cpu, Error **errp) > @@ -395,19 +434,14 @@ int kvm_init_vcpu(CPUState *cpu, Error **errp) > > trace_kvm_init_vcpu(cpu->cpu_index, kvm_arch_vcpu_id(cpu)); > > - ret = kvm_get_vcpu(s, kvm_arch_vcpu_id(cpu)); > + ret = kvm_create_vcpu(cpu); > if (ret < 0) { > - error_setg_errno(errp, -ret, "kvm_init_vcpu: kvm_get_vcpu failed (%lu)", > + error_setg_errno(errp, -ret, > + "kvm_init_vcpu: kvm_create_vcpu failed (%lu)", > kvm_arch_vcpu_id(cpu)); > goto err; > } > > - cpu->kvm_fd = ret; > - cpu->kvm_state = s; > - cpu->vcpu_dirty = true; > - cpu->dirty_pages = 0; > - cpu->throttle_us_per_full = 0; > - > mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0); > if (mmap_size < 0) { > ret = mmap_size; > diff --git a/accel/kvm/trace-events b/accel/kvm/trace-events > index 399aaeb0ec..cdd0c95c09 100644 > --- a/accel/kvm/trace-events > +++ b/accel/kvm/trace-events > @@ -9,6 +9,10 @@ kvm_device_ioctl(int fd, int type, void *arg) "dev fd %d, type 0x%x, arg %p" > kvm_failed_reg_get(uint64_t id, const char *msg) "Warning: Unable to retrieve ONEREG %" PRIu64 " from KVM: %s" > kvm_failed_reg_set(uint64_t id, const char *msg) "Warning: Unable to set ONEREG %" PRIu64 " to KVM: %s" > kvm_init_vcpu(int cpu_index, unsigned long arch_cpu_id) "index: %d id: %lu" > +kvm_create_vcpu(int cpu_index, unsigned long arch_cpu_id) "index: %d id: %lu" > +kvm_get_vcpu(unsigned long arch_cpu_id) "id: %lu" > +kvm_destroy_vcpu(int cpu_index, unsigned long arch_cpu_id) "index: %d id: %lu" > +kvm_park_vcpu(int cpu_index, unsigned long arch_cpu_id) "index: %d id: %lu" > kvm_irqchip_commit_routes(void) "" > kvm_irqchip_add_msi_route(char *name, int vector, int virq) "dev %s vector %d virq %d" > kvm_irqchip_update_msi_route(int virq) "Updating MSI route virq=%d" > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index ee9025f8e9..8137e6a44c 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -465,6 +465,22 @@ void kvm_set_sigmask_len(KVMState *s, unsigned int sigmask_len); > int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr, > hwaddr *phys_addr); > > +/** > + * kvm_create_vcpu - Gets a parked KVM vCPU or creates a KVM vCPU > + * @cpu: QOM CPUState object for which KVM vCPU has to be fetched/created. > + * > + * @returns: 0 when success, errno (<0) when failed. > + */ > +int kvm_create_vcpu(CPUState *cpu); > + > +/** > + * kvm_park_vcpu - Park QEMU KVM vCPU context > + * @cpu: QOM CPUState object for which QEMU KVM vCPU context has to be parked. > + * > + * @returns: none > + */ > +void kvm_park_vcpu(CPUState *cpu); > + > #endif /* NEED_CPU_H */ > > void kvm_cpu_synchronize_state(CPUState *cpu);