From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Christopherson Date: Wed, 2 Nov 2022 23:19:08 +0000 Subject: [PATCH 41/44] KVM: Use a per-CPU variable to track which CPUs have enabled virtualization In-Reply-To: <20221102231911.3107438-1-seanjc@google.com> References: <20221102231911.3107438-1-seanjc@google.com> Message-ID: <20221102231911.3107438-42-seanjc@google.com> List-Id: To: kvm-riscv@lists.infradead.org MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Use a per-CPU variable instead of a shared bitmap to track which CPUs have successfully enabled virtualization hardware. Using a per-CPU bool avoids the need for an additional allocation, and arguably yields easier to read code. Using a bitmap would be advantageous if KVM used it to avoid generating IPIs to CPUs that failed to enable hardware, but that's an extreme edge case and not worth optimizing, and the low level helpers would still want to keep their individual checks as attempting to enable virtualization hardware when it's already enabled can be problematic, e.g. Intel's VMXON will fault. Opportunistically change the order in hardware_enable_nolock() to set the flag if and only if hardware enabling is successful, instead of speculatively setting the flag and then clearing it on failure. Add a comment explaining that the check in hardware_disable_nolock() isn't simply paranoia. Waaay back when, commit 1b6c016818a5 ("KVM: Keep track of which cpus have virtualization enabled"), added the logic as a guards against CPU hotplug racing with hardware enable/disable. Now that KVM has eliminated the race by taking cpu_hotplug_lock for read (via cpus_read_lock()) when enabling or disabling hardware, at first glance it appears that the check is now superfluous, i.e. it's tempting to remove the per-CPU flag entirely... Signed-off-by: Sean Christopherson --- virt/kvm/kvm_main.c | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 4a42b78bfb0e..31949a89fe25 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -102,7 +102,7 @@ EXPORT_SYMBOL_GPL(halt_poll_ns_shrink); DEFINE_MUTEX(kvm_lock); LIST_HEAD(vm_list); -static cpumask_var_t cpus_hardware_enabled; +static DEFINE_PER_CPU(bool, hardware_enabled); static int kvm_usage_count; static atomic_t hardware_enable_failed; @@ -5008,21 +5008,17 @@ static struct miscdevice kvm_dev = { static void hardware_enable_nolock(void *junk) { - int cpu = raw_smp_processor_id(); - int r; - - if (cpumask_test_cpu(cpu, cpus_hardware_enabled)) + if (__this_cpu_read(hardware_enabled)) return; - cpumask_set_cpu(cpu, cpus_hardware_enabled); - - r = kvm_arch_hardware_enable(); - - if (r) { - cpumask_clear_cpu(cpu, cpus_hardware_enabled); + if (kvm_arch_hardware_enable()) { atomic_inc(&hardware_enable_failed); - pr_info("kvm: enabling virtualization on CPU%d failed\n", cpu); + pr_info("kvm: enabling virtualization on CPU%d failed\n", + raw_smp_processor_id()); + return; } + + __this_cpu_write(hardware_enabled, true); } static int kvm_online_cpu(unsigned int cpu) @@ -5054,12 +5050,16 @@ static int kvm_online_cpu(unsigned int cpu) static void hardware_disable_nolock(void *junk) { - int cpu = raw_smp_processor_id(); - - if (!cpumask_test_cpu(cpu, cpus_hardware_enabled)) + /* + * Note, hardware_disable_all_nolock() tells all online CPUs to disable + * hardware, not just CPUs that successfully enabled hardware! + */ + if (!__this_cpu_read(hardware_enabled)) return; - cpumask_clear_cpu(cpu, cpus_hardware_enabled); + kvm_arch_hardware_disable(); + + __this_cpu_write(hardware_enabled, false); } static int kvm_offline_cpu(unsigned int cpu) @@ -5861,13 +5861,11 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) int r; int cpu; - if (!zalloc_cpumask_var(&cpus_hardware_enabled, GFP_KERNEL)) - return -ENOMEM; - r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_ONLINE, "kvm/cpu:online", kvm_online_cpu, kvm_offline_cpu); if (r) - goto out_free_2; + return r; + register_reboot_notifier(&kvm_reboot_notifier); /* A kmem cache lets us meet the alignment requirements of fx_save. */ @@ -5940,8 +5938,6 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) out_free_3: unregister_reboot_notifier(&kvm_reboot_notifier); cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE); -out_free_2: - free_cpumask_var(cpus_hardware_enabled); return r; } EXPORT_SYMBOL_GPL(kvm_init); @@ -5967,7 +5963,6 @@ void kvm_exit(void) unregister_reboot_notifier(&kvm_reboot_notifier); cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE); kvm_irqfd_exit(); - free_cpumask_var(cpus_hardware_enabled); } EXPORT_SYMBOL_GPL(kvm_exit); -- 2.38.1.431.g37b22c650d-goog 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 mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by smtp.lore.kernel.org (Postfix) with ESMTP id ECE8AC43217 for ; Wed, 2 Nov 2022 23:20:34 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 8A9064B889; Wed, 2 Nov 2022 19:20:34 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@google.com Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id MhvKNL3y0U0i; Wed, 2 Nov 2022 19:20:33 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 3ECCF4B89A; Wed, 2 Nov 2022 19:20:31 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 2A7C14B875 for ; Wed, 2 Nov 2022 19:20:29 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Y7mLGPU7SRDQ for ; Wed, 2 Nov 2022 19:20:27 -0400 (EDT) Received: from mail-yb1-f201.google.com (mail-yb1-f201.google.com [209.85.219.201]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id D6F774B85F for ; Wed, 2 Nov 2022 19:20:24 -0400 (EDT) Received: by mail-yb1-f201.google.com with SMTP id o2-20020a5b0502000000b006cade5e7c07so373014ybp.10 for ; Wed, 02 Nov 2022 16:20:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:from:to:cc:subject:date:message-id:reply-to; bh=OIMXqLasKb2O7+CMjQgfYLOt6T5DmXDJujGgVXhHqFI=; b=a1O/5J/wwBHL5DdZlnXyBx3ad/wWPMoFwu33ObN+0h9wckbMWe8lEdCsf3vv9dBpgd Og1tkQtFVeN8Yaq0826BqmvMwQ236D77oay2sYYYSosRusWw8ffouoxC4D4VSugKPkv6 eFSXHeIkk4rxzIyC2jBSOKzzG944O7R7WfzIzWLb9f4+zjzqLEX7pMuEWnereIkmHCla z9Wi0z30aLvdj67Gj18fnOk19FrwVJlssI8qaBiE05oy40Xc5I9xoCB6VMFe6e9n0+Qe iXYDA9B6ROLH1CUwVEa9sH8tNUrl6u56cDEtM0Lb6ytwc6hrZDZzuAPDLoXNIWglaaUo bPHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=OIMXqLasKb2O7+CMjQgfYLOt6T5DmXDJujGgVXhHqFI=; b=PsZVevlWmnc2nbZQrMdoCh38wFL1gvMwOTi0A8MqGfwRGvtcv9V+kfebNagjfn435M COT1hERSOJRPH7NskLi7Tta6kVd9NKqos58KETVR11Whritroi48qgGRGelJiJCAKYbN Jge3Tiii+oG2VZBu5xI/kEh4+s3qzaHMzdn8nq7b4iosnDcOwsbtlY835tIJ8BnO3nif t2grc5T0YrSP6RuWkwpFv7nwTpVeURWdbcCz3mnFgzEP3dW0X4gbRQpNNDcapJDb66Nq q9WqTQpEUe9nqIN/w5n5UWzmBPbeN9fzlR5AW91FuyaFtE079U3yUF49lI24iGbzV5z+ qkvw== X-Gm-Message-State: ACrzQf1MgcfxjRxCGrGkTrm2nkLOf48V9N0QYuKUt2f/3X/mqt9GOt+z ei7c7MyCN6ICtllHPHpjL7qxtFyCF+g= X-Google-Smtp-Source: AMsMyM6yCCCYKkQ3PoeOADSfJNFYsKEgxfAWyhUa6qA376xUH3n8/ExaQ+otuspws7tagrIY8b2ugf0DdfA= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a81:1713:0:b0:36b:3a6b:4b49 with SMTP id 19-20020a811713000000b0036b3a6b4b49mr26413129ywx.500.1667431224514; Wed, 02 Nov 2022 16:20:24 -0700 (PDT) Date: Wed, 2 Nov 2022 23:19:08 +0000 In-Reply-To: <20221102231911.3107438-1-seanjc@google.com> Mime-Version: 1.0 References: <20221102231911.3107438-1-seanjc@google.com> X-Mailer: git-send-email 2.38.1.431.g37b22c650d-goog Message-ID: <20221102231911.3107438-42-seanjc@google.com> Subject: [PATCH 41/44] KVM: Use a per-CPU variable to track which CPUs have enabled virtualization From: Sean Christopherson To: Paolo Bonzini , Marc Zyngier , Huacai Chen , Aleksandar Markovic , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , Matthew Rosato , Eric Farman , Sean Christopherson , Vitaly Kuznetsov Cc: kvm@vger.kernel.org, David Hildenbrand , Atish Patra , linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-s390@vger.kernel.org, Michael Ellerman , Chao Gao , Yuan Yao , kvmarm@lists.linux.dev, Thomas Gleixner , linux-arm-kernel@lists.infradead.org, Isaku Yamahata , Fabiano Rosas , linux-mips@vger.kernel.org, kvm-riscv@lists.infradead.org, linuxppc-dev@lists.ozlabs.org X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list Reply-To: Sean Christopherson List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu Use a per-CPU variable instead of a shared bitmap to track which CPUs have successfully enabled virtualization hardware. Using a per-CPU bool avoids the need for an additional allocation, and arguably yields easier to read code. Using a bitmap would be advantageous if KVM used it to avoid generating IPIs to CPUs that failed to enable hardware, but that's an extreme edge case and not worth optimizing, and the low level helpers would still want to keep their individual checks as attempting to enable virtualization hardware when it's already enabled can be problematic, e.g. Intel's VMXON will fault. Opportunistically change the order in hardware_enable_nolock() to set the flag if and only if hardware enabling is successful, instead of speculatively setting the flag and then clearing it on failure. Add a comment explaining that the check in hardware_disable_nolock() isn't simply paranoia. Waaay back when, commit 1b6c016818a5 ("KVM: Keep track of which cpus have virtualization enabled"), added the logic as a guards against CPU hotplug racing with hardware enable/disable. Now that KVM has eliminated the race by taking cpu_hotplug_lock for read (via cpus_read_lock()) when enabling or disabling hardware, at first glance it appears that the check is now superfluous, i.e. it's tempting to remove the per-CPU flag entirely... Signed-off-by: Sean Christopherson --- virt/kvm/kvm_main.c | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 4a42b78bfb0e..31949a89fe25 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -102,7 +102,7 @@ EXPORT_SYMBOL_GPL(halt_poll_ns_shrink); DEFINE_MUTEX(kvm_lock); LIST_HEAD(vm_list); -static cpumask_var_t cpus_hardware_enabled; +static DEFINE_PER_CPU(bool, hardware_enabled); static int kvm_usage_count; static atomic_t hardware_enable_failed; @@ -5008,21 +5008,17 @@ static struct miscdevice kvm_dev = { static void hardware_enable_nolock(void *junk) { - int cpu = raw_smp_processor_id(); - int r; - - if (cpumask_test_cpu(cpu, cpus_hardware_enabled)) + if (__this_cpu_read(hardware_enabled)) return; - cpumask_set_cpu(cpu, cpus_hardware_enabled); - - r = kvm_arch_hardware_enable(); - - if (r) { - cpumask_clear_cpu(cpu, cpus_hardware_enabled); + if (kvm_arch_hardware_enable()) { atomic_inc(&hardware_enable_failed); - pr_info("kvm: enabling virtualization on CPU%d failed\n", cpu); + pr_info("kvm: enabling virtualization on CPU%d failed\n", + raw_smp_processor_id()); + return; } + + __this_cpu_write(hardware_enabled, true); } static int kvm_online_cpu(unsigned int cpu) @@ -5054,12 +5050,16 @@ static int kvm_online_cpu(unsigned int cpu) static void hardware_disable_nolock(void *junk) { - int cpu = raw_smp_processor_id(); - - if (!cpumask_test_cpu(cpu, cpus_hardware_enabled)) + /* + * Note, hardware_disable_all_nolock() tells all online CPUs to disable + * hardware, not just CPUs that successfully enabled hardware! + */ + if (!__this_cpu_read(hardware_enabled)) return; - cpumask_clear_cpu(cpu, cpus_hardware_enabled); + kvm_arch_hardware_disable(); + + __this_cpu_write(hardware_enabled, false); } static int kvm_offline_cpu(unsigned int cpu) @@ -5861,13 +5861,11 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) int r; int cpu; - if (!zalloc_cpumask_var(&cpus_hardware_enabled, GFP_KERNEL)) - return -ENOMEM; - r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_ONLINE, "kvm/cpu:online", kvm_online_cpu, kvm_offline_cpu); if (r) - goto out_free_2; + return r; + register_reboot_notifier(&kvm_reboot_notifier); /* A kmem cache lets us meet the alignment requirements of fx_save. */ @@ -5940,8 +5938,6 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) out_free_3: unregister_reboot_notifier(&kvm_reboot_notifier); cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE); -out_free_2: - free_cpumask_var(cpus_hardware_enabled); return r; } EXPORT_SYMBOL_GPL(kvm_init); @@ -5967,7 +5963,6 @@ void kvm_exit(void) unregister_reboot_notifier(&kvm_reboot_notifier); cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE); kvm_irqfd_exit(); - free_cpumask_var(cpus_hardware_enabled); } EXPORT_SYMBOL_GPL(kvm_exit); -- 2.38.1.431.g37b22c650d-goog _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yb1-f201.google.com (mail-yb1-f201.google.com [209.85.219.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C3D3C24F21 for ; Wed, 2 Nov 2022 23:20:24 +0000 (UTC) Received: by mail-yb1-f201.google.com with SMTP id t9-20020a5b03c9000000b006cff5077dc9so395773ybp.3 for ; Wed, 02 Nov 2022 16:20:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:from:to:cc:subject:date:message-id:reply-to; bh=OIMXqLasKb2O7+CMjQgfYLOt6T5DmXDJujGgVXhHqFI=; b=a1O/5J/wwBHL5DdZlnXyBx3ad/wWPMoFwu33ObN+0h9wckbMWe8lEdCsf3vv9dBpgd Og1tkQtFVeN8Yaq0826BqmvMwQ236D77oay2sYYYSosRusWw8ffouoxC4D4VSugKPkv6 eFSXHeIkk4rxzIyC2jBSOKzzG944O7R7WfzIzWLb9f4+zjzqLEX7pMuEWnereIkmHCla z9Wi0z30aLvdj67Gj18fnOk19FrwVJlssI8qaBiE05oy40Xc5I9xoCB6VMFe6e9n0+Qe iXYDA9B6ROLH1CUwVEa9sH8tNUrl6u56cDEtM0Lb6ytwc6hrZDZzuAPDLoXNIWglaaUo bPHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=OIMXqLasKb2O7+CMjQgfYLOt6T5DmXDJujGgVXhHqFI=; b=rfAVPQPTJ5n1He0u4XVcZMbgzTsvctQxo6Fd/zYt1TiTKmn89FAbHhv6lqrMVvu/VO 2s88P5I3Z8WXglS/t/50poCQbq2Vuw9MQex1pY9+WyRvTFV+oN7ED+IyuMMVC+ZXVRy3 TGIruJqXRGebOWs8HptZGb8TKSpsktKTkEU5TZ273tHfxYFH3RpPo7z4pXK9FWSyyAxN W0+tKTWUaUalTaixny1SuLKZZvBhQcQG6oVqiMjSZWJpDmlHtoQKTk+FAKbW+IkHbrzq TmZ34pPPrYtquTgMb1NbyKB5pQBEzud+H/hk4uqzVGUlvAXDVZbD1WYO7fTlkRlSZo4+ +AtA== X-Gm-Message-State: ACrzQf2GC+Ju+uzzSf/CkqpEDORiJB/bipDW93udB9iFZLp6EP2lShAX LyuXtcrd2ae1Eec2W+3e2weTlksOeAA= X-Google-Smtp-Source: AMsMyM6yCCCYKkQ3PoeOADSfJNFYsKEgxfAWyhUa6qA376xUH3n8/ExaQ+otuspws7tagrIY8b2ugf0DdfA= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a81:1713:0:b0:36b:3a6b:4b49 with SMTP id 19-20020a811713000000b0036b3a6b4b49mr26413129ywx.500.1667431224514; Wed, 02 Nov 2022 16:20:24 -0700 (PDT) Reply-To: Sean Christopherson Date: Wed, 2 Nov 2022 23:19:08 +0000 In-Reply-To: <20221102231911.3107438-1-seanjc@google.com> Precedence: bulk X-Mailing-List: kvmarm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20221102231911.3107438-1-seanjc@google.com> X-Mailer: git-send-email 2.38.1.431.g37b22c650d-goog Message-ID: <20221102231911.3107438-42-seanjc@google.com> Subject: [PATCH 41/44] KVM: Use a per-CPU variable to track which CPUs have enabled virtualization From: Sean Christopherson To: Paolo Bonzini , Marc Zyngier , Huacai Chen , Aleksandar Markovic , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , Matthew Rosato , Eric Farman , Sean Christopherson , Vitaly Kuznetsov Cc: James Morse , Alexandru Elisei , Suzuki K Poulose , Oliver Upton , Atish Patra , David Hildenbrand , kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, kvmarm@lists.cs.columbia.edu, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, Isaku Yamahata , Fabiano Rosas , Michael Ellerman , Chao Gao , Thomas Gleixner , Yuan Yao Content-Type: text/plain; charset="UTF-8" Message-ID: <20221102231908.MT41Gjc1RzL2V5rnrDAaydWMTDPK3Ol-WQZHaLg3Mys@z> Use a per-CPU variable instead of a shared bitmap to track which CPUs have successfully enabled virtualization hardware. Using a per-CPU bool avoids the need for an additional allocation, and arguably yields easier to read code. Using a bitmap would be advantageous if KVM used it to avoid generating IPIs to CPUs that failed to enable hardware, but that's an extreme edge case and not worth optimizing, and the low level helpers would still want to keep their individual checks as attempting to enable virtualization hardware when it's already enabled can be problematic, e.g. Intel's VMXON will fault. Opportunistically change the order in hardware_enable_nolock() to set the flag if and only if hardware enabling is successful, instead of speculatively setting the flag and then clearing it on failure. Add a comment explaining that the check in hardware_disable_nolock() isn't simply paranoia. Waaay back when, commit 1b6c016818a5 ("KVM: Keep track of which cpus have virtualization enabled"), added the logic as a guards against CPU hotplug racing with hardware enable/disable. Now that KVM has eliminated the race by taking cpu_hotplug_lock for read (via cpus_read_lock()) when enabling or disabling hardware, at first glance it appears that the check is now superfluous, i.e. it's tempting to remove the per-CPU flag entirely... Signed-off-by: Sean Christopherson --- virt/kvm/kvm_main.c | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 4a42b78bfb0e..31949a89fe25 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -102,7 +102,7 @@ EXPORT_SYMBOL_GPL(halt_poll_ns_shrink); DEFINE_MUTEX(kvm_lock); LIST_HEAD(vm_list); -static cpumask_var_t cpus_hardware_enabled; +static DEFINE_PER_CPU(bool, hardware_enabled); static int kvm_usage_count; static atomic_t hardware_enable_failed; @@ -5008,21 +5008,17 @@ static struct miscdevice kvm_dev = { static void hardware_enable_nolock(void *junk) { - int cpu = raw_smp_processor_id(); - int r; - - if (cpumask_test_cpu(cpu, cpus_hardware_enabled)) + if (__this_cpu_read(hardware_enabled)) return; - cpumask_set_cpu(cpu, cpus_hardware_enabled); - - r = kvm_arch_hardware_enable(); - - if (r) { - cpumask_clear_cpu(cpu, cpus_hardware_enabled); + if (kvm_arch_hardware_enable()) { atomic_inc(&hardware_enable_failed); - pr_info("kvm: enabling virtualization on CPU%d failed\n", cpu); + pr_info("kvm: enabling virtualization on CPU%d failed\n", + raw_smp_processor_id()); + return; } + + __this_cpu_write(hardware_enabled, true); } static int kvm_online_cpu(unsigned int cpu) @@ -5054,12 +5050,16 @@ static int kvm_online_cpu(unsigned int cpu) static void hardware_disable_nolock(void *junk) { - int cpu = raw_smp_processor_id(); - - if (!cpumask_test_cpu(cpu, cpus_hardware_enabled)) + /* + * Note, hardware_disable_all_nolock() tells all online CPUs to disable + * hardware, not just CPUs that successfully enabled hardware! + */ + if (!__this_cpu_read(hardware_enabled)) return; - cpumask_clear_cpu(cpu, cpus_hardware_enabled); + kvm_arch_hardware_disable(); + + __this_cpu_write(hardware_enabled, false); } static int kvm_offline_cpu(unsigned int cpu) @@ -5861,13 +5861,11 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) int r; int cpu; - if (!zalloc_cpumask_var(&cpus_hardware_enabled, GFP_KERNEL)) - return -ENOMEM; - r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_ONLINE, "kvm/cpu:online", kvm_online_cpu, kvm_offline_cpu); if (r) - goto out_free_2; + return r; + register_reboot_notifier(&kvm_reboot_notifier); /* A kmem cache lets us meet the alignment requirements of fx_save. */ @@ -5940,8 +5938,6 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) out_free_3: unregister_reboot_notifier(&kvm_reboot_notifier); cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE); -out_free_2: - free_cpumask_var(cpus_hardware_enabled); return r; } EXPORT_SYMBOL_GPL(kvm_init); @@ -5967,7 +5963,6 @@ void kvm_exit(void) unregister_reboot_notifier(&kvm_reboot_notifier); cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE); kvm_irqfd_exit(); - free_cpumask_var(cpus_hardware_enabled); } EXPORT_SYMBOL_GPL(kvm_exit); -- 2.38.1.431.g37b22c650d-goog 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 5F19CC433FE for ; Wed, 2 Nov 2022 23:38:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:Reply-To:List-Subscribe:List-Help: List-Post:List-Archive:List-Unsubscribe:List-Id:Cc:To:From:Subject:Message-ID :References:Mime-Version:In-Reply-To:Date:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=pPXQ/n5F5CDFW87d/ygA3lKo4VOg0gJpgK6ZYOl7leE=; b=LqngSr7gDjFGLR EhZt4LQiONF2dEjyFJfcPuulw05QCjzhnjhZusbnuUjrE259isdH28LlesFwweQi1pAQGFia/cZJh NGzItdPe+ozPVYuqgLdBXFTEbctTGOinXpfCEzlSxIrwAgJ22pK8wuHQSmKJDZgTYwcjoLJKHzhUa tUVucU47CMe8FJJf3lzAVRZnOMsE695C1SGHNAaV3rceWnQxys8KhevvZz2wAhox2HXFlOYACvgWx xrqpMyK2svLFXziXiwi12niv0/YnA4Abpy5IG/K0kkKkXePzNT/1AfMLLgVgynGL6ze3tyZbXL4v+ ZHtkTLL5BRzy2VWzuZ3A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oqNJ8-00F3KQ-2S; Wed, 02 Nov 2022 23:38:06 +0000 Received: from mail-yb1-xb4a.google.com ([2607:f8b0:4864:20::b4a]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oqN9V-00EyXi-9e for linux-riscv@lists.infradead.org; Wed, 02 Nov 2022 23:28:10 +0000 Received: by mail-yb1-xb4a.google.com with SMTP id k7-20020a256f07000000b006cbcc030bc8so348887ybc.18 for ; Wed, 02 Nov 2022 16:28:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:from:to:cc:subject:date:message-id:reply-to; bh=OIMXqLasKb2O7+CMjQgfYLOt6T5DmXDJujGgVXhHqFI=; b=a1O/5J/wwBHL5DdZlnXyBx3ad/wWPMoFwu33ObN+0h9wckbMWe8lEdCsf3vv9dBpgd Og1tkQtFVeN8Yaq0826BqmvMwQ236D77oay2sYYYSosRusWw8ffouoxC4D4VSugKPkv6 eFSXHeIkk4rxzIyC2jBSOKzzG944O7R7WfzIzWLb9f4+zjzqLEX7pMuEWnereIkmHCla z9Wi0z30aLvdj67Gj18fnOk19FrwVJlssI8qaBiE05oy40Xc5I9xoCB6VMFe6e9n0+Qe iXYDA9B6ROLH1CUwVEa9sH8tNUrl6u56cDEtM0Lb6ytwc6hrZDZzuAPDLoXNIWglaaUo bPHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=OIMXqLasKb2O7+CMjQgfYLOt6T5DmXDJujGgVXhHqFI=; b=1GJKJPiSEv+vJ2dlBj2ApTs47T1fA4E6/BLyS0MSSjmX18Aa7U5hRy27B4Ynki1ooP Dtv9Z53YNsMmhscEHMzz6WVCxto+KLQ9NePwpSzyF4Kgo5JLW6FyUjBhlYf1063T4ZOG /BCfoBB0UTE6x/pW/w49eDgxJCkABvrUSiWhgh0bNOBqfzp9oNbyz4kz8oXtoRQ9V9oB IREpIom7VTch5CT+N4PXOr4XwgGyyItGvC4iCCAxRexo7McIY6/EYOYLc/vD8kkpe8YC TEmaSihH1xdYXvxt/a4dD32mIy4iyxXtHI57bfxBC3vLFnK0PY15s7+5asgbJZsN7TZy T1ew== X-Gm-Message-State: ACrzQf0cotNVB9QPbh2da6/5A5oU3RO2wdNqVKmeSgs27FOJj2RBp3rV 5iap/uGn6UF3GiDbVUC9jNXCvBBVXfg= X-Google-Smtp-Source: AMsMyM6yCCCYKkQ3PoeOADSfJNFYsKEgxfAWyhUa6qA376xUH3n8/ExaQ+otuspws7tagrIY8b2ugf0DdfA= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a81:1713:0:b0:36b:3a6b:4b49 with SMTP id 19-20020a811713000000b0036b3a6b4b49mr26413129ywx.500.1667431224514; Wed, 02 Nov 2022 16:20:24 -0700 (PDT) Date: Wed, 2 Nov 2022 23:19:08 +0000 In-Reply-To: <20221102231911.3107438-1-seanjc@google.com> Mime-Version: 1.0 References: <20221102231911.3107438-1-seanjc@google.com> X-Mailer: git-send-email 2.38.1.431.g37b22c650d-goog Message-ID: <20221102231911.3107438-42-seanjc@google.com> Subject: [PATCH 41/44] KVM: Use a per-CPU variable to track which CPUs have enabled virtualization From: Sean Christopherson To: Paolo Bonzini , Marc Zyngier , Huacai Chen , Aleksandar Markovic , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , Matthew Rosato , Eric Farman , Sean Christopherson , Vitaly Kuznetsov Cc: James Morse , Alexandru Elisei , Suzuki K Poulose , Oliver Upton , Atish Patra , David Hildenbrand , kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, kvmarm@lists.cs.columbia.edu, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, Isaku Yamahata , Fabiano Rosas , Michael Ellerman , Chao Gao , Thomas Gleixner , Yuan Yao X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221102_162809_351308_F6D32721 X-CRM114-Status: GOOD ( 17.22 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Sean Christopherson Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org Use a per-CPU variable instead of a shared bitmap to track which CPUs have successfully enabled virtualization hardware. Using a per-CPU bool avoids the need for an additional allocation, and arguably yields easier to read code. Using a bitmap would be advantageous if KVM used it to avoid generating IPIs to CPUs that failed to enable hardware, but that's an extreme edge case and not worth optimizing, and the low level helpers would still want to keep their individual checks as attempting to enable virtualization hardware when it's already enabled can be problematic, e.g. Intel's VMXON will fault. Opportunistically change the order in hardware_enable_nolock() to set the flag if and only if hardware enabling is successful, instead of speculatively setting the flag and then clearing it on failure. Add a comment explaining that the check in hardware_disable_nolock() isn't simply paranoia. Waaay back when, commit 1b6c016818a5 ("KVM: Keep track of which cpus have virtualization enabled"), added the logic as a guards against CPU hotplug racing with hardware enable/disable. Now that KVM has eliminated the race by taking cpu_hotplug_lock for read (via cpus_read_lock()) when enabling or disabling hardware, at first glance it appears that the check is now superfluous, i.e. it's tempting to remove the per-CPU flag entirely... Signed-off-by: Sean Christopherson --- virt/kvm/kvm_main.c | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 4a42b78bfb0e..31949a89fe25 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -102,7 +102,7 @@ EXPORT_SYMBOL_GPL(halt_poll_ns_shrink); DEFINE_MUTEX(kvm_lock); LIST_HEAD(vm_list); -static cpumask_var_t cpus_hardware_enabled; +static DEFINE_PER_CPU(bool, hardware_enabled); static int kvm_usage_count; static atomic_t hardware_enable_failed; @@ -5008,21 +5008,17 @@ static struct miscdevice kvm_dev = { static void hardware_enable_nolock(void *junk) { - int cpu = raw_smp_processor_id(); - int r; - - if (cpumask_test_cpu(cpu, cpus_hardware_enabled)) + if (__this_cpu_read(hardware_enabled)) return; - cpumask_set_cpu(cpu, cpus_hardware_enabled); - - r = kvm_arch_hardware_enable(); - - if (r) { - cpumask_clear_cpu(cpu, cpus_hardware_enabled); + if (kvm_arch_hardware_enable()) { atomic_inc(&hardware_enable_failed); - pr_info("kvm: enabling virtualization on CPU%d failed\n", cpu); + pr_info("kvm: enabling virtualization on CPU%d failed\n", + raw_smp_processor_id()); + return; } + + __this_cpu_write(hardware_enabled, true); } static int kvm_online_cpu(unsigned int cpu) @@ -5054,12 +5050,16 @@ static int kvm_online_cpu(unsigned int cpu) static void hardware_disable_nolock(void *junk) { - int cpu = raw_smp_processor_id(); - - if (!cpumask_test_cpu(cpu, cpus_hardware_enabled)) + /* + * Note, hardware_disable_all_nolock() tells all online CPUs to disable + * hardware, not just CPUs that successfully enabled hardware! + */ + if (!__this_cpu_read(hardware_enabled)) return; - cpumask_clear_cpu(cpu, cpus_hardware_enabled); + kvm_arch_hardware_disable(); + + __this_cpu_write(hardware_enabled, false); } static int kvm_offline_cpu(unsigned int cpu) @@ -5861,13 +5861,11 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) int r; int cpu; - if (!zalloc_cpumask_var(&cpus_hardware_enabled, GFP_KERNEL)) - return -ENOMEM; - r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_ONLINE, "kvm/cpu:online", kvm_online_cpu, kvm_offline_cpu); if (r) - goto out_free_2; + return r; + register_reboot_notifier(&kvm_reboot_notifier); /* A kmem cache lets us meet the alignment requirements of fx_save. */ @@ -5940,8 +5938,6 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) out_free_3: unregister_reboot_notifier(&kvm_reboot_notifier); cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE); -out_free_2: - free_cpumask_var(cpus_hardware_enabled); return r; } EXPORT_SYMBOL_GPL(kvm_init); @@ -5967,7 +5963,6 @@ void kvm_exit(void) unregister_reboot_notifier(&kvm_reboot_notifier); cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE); kvm_irqfd_exit(); - free_cpumask_var(cpus_hardware_enabled); } EXPORT_SYMBOL_GPL(kvm_exit); -- 2.38.1.431.g37b22c650d-goog _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv 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.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (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 81A6BC4332F for ; Wed, 2 Nov 2022 23:57:36 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4N2kMz17jWz3g8g for ; Thu, 3 Nov 2022 10:57:35 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20210112 header.b=a1O/5J/w; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=flex--seanjc.bounces.google.com (client-ip=2607:f8b0:4864:20::b49; helo=mail-yb1-xb49.google.com; envelope-from=3optiywykdnspb7kg9dlldib.9ljifkrumm9-absifpqp.lwi78p.lod@flex--seanjc.bounces.google.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20210112 header.b=a1O/5J/w; dkim-atps=neutral Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4N2jY63YHRz3cMs for ; Thu, 3 Nov 2022 10:20:26 +1100 (AEDT) Received: by mail-yb1-xb49.google.com with SMTP id b14-20020a056902030e00b006a827d81fd8so335954ybs.17 for ; Wed, 02 Nov 2022 16:20:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:from:to:cc:subject:date:message-id:reply-to; bh=OIMXqLasKb2O7+CMjQgfYLOt6T5DmXDJujGgVXhHqFI=; b=a1O/5J/wwBHL5DdZlnXyBx3ad/wWPMoFwu33ObN+0h9wckbMWe8lEdCsf3vv9dBpgd Og1tkQtFVeN8Yaq0826BqmvMwQ236D77oay2sYYYSosRusWw8ffouoxC4D4VSugKPkv6 eFSXHeIkk4rxzIyC2jBSOKzzG944O7R7WfzIzWLb9f4+zjzqLEX7pMuEWnereIkmHCla z9Wi0z30aLvdj67Gj18fnOk19FrwVJlssI8qaBiE05oy40Xc5I9xoCB6VMFe6e9n0+Qe iXYDA9B6ROLH1CUwVEa9sH8tNUrl6u56cDEtM0Lb6ytwc6hrZDZzuAPDLoXNIWglaaUo bPHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=OIMXqLasKb2O7+CMjQgfYLOt6T5DmXDJujGgVXhHqFI=; b=yk5sNyEeeZW22OpIxwBLZjvfz1o9E4dwehNpAIV87wB6iw0sP8zGOYJ6mlFY+bzYRw B40IvwkfbAM6hUOgmFeE4iuveBCHTRuz0WfTlT+6qH63KY8R2f6cXa1t1BF6r2sQCwTR cUxkeVY4GbX7XZOSa1ahyoeEx06X+Ce9z854E3sY7svTgfYgHfGmD2novSsFZCQE2Tnz MlM/gvcalinE74F6H3uK0AKkO60j3YGTfT07+A2sb7BpE4sEfO9eGdR94M/KFPn0n4R8 NSmunjBaeEtpjXhR0Foa7z9oBuegelKIGh7vM1b4oVt681/4qXPiMCluX92jaebfAf2J RxyA== X-Gm-Message-State: ACrzQf2Qx1HZPfVaEREBQkQSqelk+VipqWCAY15qGB9Oa6oBlbafrh5d yUX94xesghP9WQ0/0ArlL0w/f8/fdiY= X-Google-Smtp-Source: AMsMyM6yCCCYKkQ3PoeOADSfJNFYsKEgxfAWyhUa6qA376xUH3n8/ExaQ+otuspws7tagrIY8b2ugf0DdfA= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a81:1713:0:b0:36b:3a6b:4b49 with SMTP id 19-20020a811713000000b0036b3a6b4b49mr26413129ywx.500.1667431224514; Wed, 02 Nov 2022 16:20:24 -0700 (PDT) Date: Wed, 2 Nov 2022 23:19:08 +0000 In-Reply-To: <20221102231911.3107438-1-seanjc@google.com> Mime-Version: 1.0 References: <20221102231911.3107438-1-seanjc@google.com> X-Mailer: git-send-email 2.38.1.431.g37b22c650d-goog Message-ID: <20221102231911.3107438-42-seanjc@google.com> Subject: [PATCH 41/44] KVM: Use a per-CPU variable to track which CPUs have enabled virtualization From: Sean Christopherson To: Paolo Bonzini , Marc Zyngier , Huacai Chen , Aleksandar Markovic , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , Matthew Rosato , Eric Farman , Sean Christopherson , Vitaly Kuznetsov Content-Type: text/plain; charset="UTF-8" X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Sean Christopherson Cc: kvm@vger.kernel.org, David Hildenbrand , Atish Patra , linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-s390@vger.kernel.org, Chao Gao , Suzuki K Poulose , Yuan Yao , kvmarm@lists.linux.dev, Thomas Gleixner , Alexandru Elisei , linux-arm-kernel@lists.infradead.org, Isaku Yamahata , Fabiano Rosas , linux-mips@vger.kernel.org, Oliver Upton , James Morse , kvm-riscv@lists.infradead.org, linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" Use a per-CPU variable instead of a shared bitmap to track which CPUs have successfully enabled virtualization hardware. Using a per-CPU bool avoids the need for an additional allocation, and arguably yields easier to read code. Using a bitmap would be advantageous if KVM used it to avoid generating IPIs to CPUs that failed to enable hardware, but that's an extreme edge case and not worth optimizing, and the low level helpers would still want to keep their individual checks as attempting to enable virtualization hardware when it's already enabled can be problematic, e.g. Intel's VMXON will fault. Opportunistically change the order in hardware_enable_nolock() to set the flag if and only if hardware enabling is successful, instead of speculatively setting the flag and then clearing it on failure. Add a comment explaining that the check in hardware_disable_nolock() isn't simply paranoia. Waaay back when, commit 1b6c016818a5 ("KVM: Keep track of which cpus have virtualization enabled"), added the logic as a guards against CPU hotplug racing with hardware enable/disable. Now that KVM has eliminated the race by taking cpu_hotplug_lock for read (via cpus_read_lock()) when enabling or disabling hardware, at first glance it appears that the check is now superfluous, i.e. it's tempting to remove the per-CPU flag entirely... Signed-off-by: Sean Christopherson --- virt/kvm/kvm_main.c | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 4a42b78bfb0e..31949a89fe25 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -102,7 +102,7 @@ EXPORT_SYMBOL_GPL(halt_poll_ns_shrink); DEFINE_MUTEX(kvm_lock); LIST_HEAD(vm_list); -static cpumask_var_t cpus_hardware_enabled; +static DEFINE_PER_CPU(bool, hardware_enabled); static int kvm_usage_count; static atomic_t hardware_enable_failed; @@ -5008,21 +5008,17 @@ static struct miscdevice kvm_dev = { static void hardware_enable_nolock(void *junk) { - int cpu = raw_smp_processor_id(); - int r; - - if (cpumask_test_cpu(cpu, cpus_hardware_enabled)) + if (__this_cpu_read(hardware_enabled)) return; - cpumask_set_cpu(cpu, cpus_hardware_enabled); - - r = kvm_arch_hardware_enable(); - - if (r) { - cpumask_clear_cpu(cpu, cpus_hardware_enabled); + if (kvm_arch_hardware_enable()) { atomic_inc(&hardware_enable_failed); - pr_info("kvm: enabling virtualization on CPU%d failed\n", cpu); + pr_info("kvm: enabling virtualization on CPU%d failed\n", + raw_smp_processor_id()); + return; } + + __this_cpu_write(hardware_enabled, true); } static int kvm_online_cpu(unsigned int cpu) @@ -5054,12 +5050,16 @@ static int kvm_online_cpu(unsigned int cpu) static void hardware_disable_nolock(void *junk) { - int cpu = raw_smp_processor_id(); - - if (!cpumask_test_cpu(cpu, cpus_hardware_enabled)) + /* + * Note, hardware_disable_all_nolock() tells all online CPUs to disable + * hardware, not just CPUs that successfully enabled hardware! + */ + if (!__this_cpu_read(hardware_enabled)) return; - cpumask_clear_cpu(cpu, cpus_hardware_enabled); + kvm_arch_hardware_disable(); + + __this_cpu_write(hardware_enabled, false); } static int kvm_offline_cpu(unsigned int cpu) @@ -5861,13 +5861,11 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) int r; int cpu; - if (!zalloc_cpumask_var(&cpus_hardware_enabled, GFP_KERNEL)) - return -ENOMEM; - r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_ONLINE, "kvm/cpu:online", kvm_online_cpu, kvm_offline_cpu); if (r) - goto out_free_2; + return r; + register_reboot_notifier(&kvm_reboot_notifier); /* A kmem cache lets us meet the alignment requirements of fx_save. */ @@ -5940,8 +5938,6 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) out_free_3: unregister_reboot_notifier(&kvm_reboot_notifier); cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE); -out_free_2: - free_cpumask_var(cpus_hardware_enabled); return r; } EXPORT_SYMBOL_GPL(kvm_init); @@ -5967,7 +5963,6 @@ void kvm_exit(void) unregister_reboot_notifier(&kvm_reboot_notifier); cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE); kvm_irqfd_exit(); - free_cpumask_var(cpus_hardware_enabled); } EXPORT_SYMBOL_GPL(kvm_exit); -- 2.38.1.431.g37b22c650d-goog 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 936DEC4332F for ; Wed, 2 Nov 2022 23:32:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:Reply-To:List-Subscribe:List-Help: List-Post:List-Archive:List-Unsubscribe:List-Id:Cc:To:From:Subject:Message-ID :References:Mime-Version:In-Reply-To:Date:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=rnaIaqZWvG2u6F3A49yDQ/haW3eGzBOv+cP8rz+wrdY=; b=nZcgtn8rnBDkUs g4EYi8/mWNDlp/ICwsB2Hc0M1bSlI8qqQwWtcRQIE/U8et5VU9Am+NUqIr6RGcMjYOF+MAs0mPkWX x2fHiiyuQ5umoqk/fO8aPw494hQqYuB6F+58ZBQFZaNxSSRQISOsbwQM3qk5sI2DozOuk4J8ZT3XL JaYWUXzbtsMA06NWqGfMRw0j5rxL8QIuo2yBjbBp3Mwv8IqyNwfYYqdTwbA4h9lYkbqWO7DySHvbF mD7psGMZkS5sL0EYoFxkjEqSvt4kkjAkBAxXEkdTztWQ+wj+0guB2uKXCWj5/o6UDvKhvcZwaOMjv 50F6NA/PqpdqCXSYLTHA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oqNCN-00Ezyn-H0; Wed, 02 Nov 2022 23:31:09 +0000 Received: from mail-yb1-xb49.google.com ([2607:f8b0:4864:20::b49]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oqN21-00EtpT-TF for linux-arm-kernel@lists.infradead.org; Wed, 02 Nov 2022 23:20:27 +0000 Received: by mail-yb1-xb49.google.com with SMTP id 69-20020a250548000000b006cc9d755afaso345061ybf.16 for ; Wed, 02 Nov 2022 16:20:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:from:to:cc:subject:date:message-id:reply-to; bh=OIMXqLasKb2O7+CMjQgfYLOt6T5DmXDJujGgVXhHqFI=; b=a1O/5J/wwBHL5DdZlnXyBx3ad/wWPMoFwu33ObN+0h9wckbMWe8lEdCsf3vv9dBpgd Og1tkQtFVeN8Yaq0826BqmvMwQ236D77oay2sYYYSosRusWw8ffouoxC4D4VSugKPkv6 eFSXHeIkk4rxzIyC2jBSOKzzG944O7R7WfzIzWLb9f4+zjzqLEX7pMuEWnereIkmHCla z9Wi0z30aLvdj67Gj18fnOk19FrwVJlssI8qaBiE05oy40Xc5I9xoCB6VMFe6e9n0+Qe iXYDA9B6ROLH1CUwVEa9sH8tNUrl6u56cDEtM0Lb6ytwc6hrZDZzuAPDLoXNIWglaaUo bPHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=OIMXqLasKb2O7+CMjQgfYLOt6T5DmXDJujGgVXhHqFI=; b=J2VGyaIpERuyu04+382GoehWllywaWwRZ6rukhlU0HyMX89zXIhSUdYg3B6W0fRIkl MLlDXdY/r/Jm1zKQuAJF8eG954r9fUJrmxaFgKGGNNW+QEfUXpMJ36jhBVM1yviWaUGk 5BmZNgRuhX2RRB476E6xlmvHTnFDcOCYGIVhT0ejQR4PrPJktE6JzHZXLQPoKITR7jKa Mf/krM4KJcR3nN3ft1lsCLLN23CicDsFOOg79dPktDNycbCgfHEkYoqfMVcMcRS/FmHE Pk3DS/qtZWyATWMMmVVKCnE8B+HhLlEoaleRVnjwIuHi/kyWHmcY6H729rjpNXTZ9UYG dF8Q== X-Gm-Message-State: ACrzQf2jbmbBFwSXCpRZPRGENamYvir8K8bJs5RfcYvKloS6uU7yCbLx /QNoHQqN5uo0IJ10kRvhSogpehZgs3Q= X-Google-Smtp-Source: AMsMyM6yCCCYKkQ3PoeOADSfJNFYsKEgxfAWyhUa6qA376xUH3n8/ExaQ+otuspws7tagrIY8b2ugf0DdfA= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a81:1713:0:b0:36b:3a6b:4b49 with SMTP id 19-20020a811713000000b0036b3a6b4b49mr26413129ywx.500.1667431224514; Wed, 02 Nov 2022 16:20:24 -0700 (PDT) Date: Wed, 2 Nov 2022 23:19:08 +0000 In-Reply-To: <20221102231911.3107438-1-seanjc@google.com> Mime-Version: 1.0 References: <20221102231911.3107438-1-seanjc@google.com> X-Mailer: git-send-email 2.38.1.431.g37b22c650d-goog Message-ID: <20221102231911.3107438-42-seanjc@google.com> Subject: [PATCH 41/44] KVM: Use a per-CPU variable to track which CPUs have enabled virtualization From: Sean Christopherson To: Paolo Bonzini , Marc Zyngier , Huacai Chen , Aleksandar Markovic , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , Matthew Rosato , Eric Farman , Sean Christopherson , Vitaly Kuznetsov Cc: James Morse , Alexandru Elisei , Suzuki K Poulose , Oliver Upton , Atish Patra , David Hildenbrand , kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, kvmarm@lists.cs.columbia.edu, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, Isaku Yamahata , Fabiano Rosas , Michael Ellerman , Chao Gao , Thomas Gleixner , Yuan Yao X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221102_162025_981072_123F9554 X-CRM114-Status: GOOD ( 18.72 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Sean Christopherson Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Use a per-CPU variable instead of a shared bitmap to track which CPUs have successfully enabled virtualization hardware. Using a per-CPU bool avoids the need for an additional allocation, and arguably yields easier to read code. Using a bitmap would be advantageous if KVM used it to avoid generating IPIs to CPUs that failed to enable hardware, but that's an extreme edge case and not worth optimizing, and the low level helpers would still want to keep their individual checks as attempting to enable virtualization hardware when it's already enabled can be problematic, e.g. Intel's VMXON will fault. Opportunistically change the order in hardware_enable_nolock() to set the flag if and only if hardware enabling is successful, instead of speculatively setting the flag and then clearing it on failure. Add a comment explaining that the check in hardware_disable_nolock() isn't simply paranoia. Waaay back when, commit 1b6c016818a5 ("KVM: Keep track of which cpus have virtualization enabled"), added the logic as a guards against CPU hotplug racing with hardware enable/disable. Now that KVM has eliminated the race by taking cpu_hotplug_lock for read (via cpus_read_lock()) when enabling or disabling hardware, at first glance it appears that the check is now superfluous, i.e. it's tempting to remove the per-CPU flag entirely... Signed-off-by: Sean Christopherson --- virt/kvm/kvm_main.c | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 4a42b78bfb0e..31949a89fe25 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -102,7 +102,7 @@ EXPORT_SYMBOL_GPL(halt_poll_ns_shrink); DEFINE_MUTEX(kvm_lock); LIST_HEAD(vm_list); -static cpumask_var_t cpus_hardware_enabled; +static DEFINE_PER_CPU(bool, hardware_enabled); static int kvm_usage_count; static atomic_t hardware_enable_failed; @@ -5008,21 +5008,17 @@ static struct miscdevice kvm_dev = { static void hardware_enable_nolock(void *junk) { - int cpu = raw_smp_processor_id(); - int r; - - if (cpumask_test_cpu(cpu, cpus_hardware_enabled)) + if (__this_cpu_read(hardware_enabled)) return; - cpumask_set_cpu(cpu, cpus_hardware_enabled); - - r = kvm_arch_hardware_enable(); - - if (r) { - cpumask_clear_cpu(cpu, cpus_hardware_enabled); + if (kvm_arch_hardware_enable()) { atomic_inc(&hardware_enable_failed); - pr_info("kvm: enabling virtualization on CPU%d failed\n", cpu); + pr_info("kvm: enabling virtualization on CPU%d failed\n", + raw_smp_processor_id()); + return; } + + __this_cpu_write(hardware_enabled, true); } static int kvm_online_cpu(unsigned int cpu) @@ -5054,12 +5050,16 @@ static int kvm_online_cpu(unsigned int cpu) static void hardware_disable_nolock(void *junk) { - int cpu = raw_smp_processor_id(); - - if (!cpumask_test_cpu(cpu, cpus_hardware_enabled)) + /* + * Note, hardware_disable_all_nolock() tells all online CPUs to disable + * hardware, not just CPUs that successfully enabled hardware! + */ + if (!__this_cpu_read(hardware_enabled)) return; - cpumask_clear_cpu(cpu, cpus_hardware_enabled); + kvm_arch_hardware_disable(); + + __this_cpu_write(hardware_enabled, false); } static int kvm_offline_cpu(unsigned int cpu) @@ -5861,13 +5861,11 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) int r; int cpu; - if (!zalloc_cpumask_var(&cpus_hardware_enabled, GFP_KERNEL)) - return -ENOMEM; - r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_ONLINE, "kvm/cpu:online", kvm_online_cpu, kvm_offline_cpu); if (r) - goto out_free_2; + return r; + register_reboot_notifier(&kvm_reboot_notifier); /* A kmem cache lets us meet the alignment requirements of fx_save. */ @@ -5940,8 +5938,6 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) out_free_3: unregister_reboot_notifier(&kvm_reboot_notifier); cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE); -out_free_2: - free_cpumask_var(cpus_hardware_enabled); return r; } EXPORT_SYMBOL_GPL(kvm_init); @@ -5967,7 +5963,6 @@ void kvm_exit(void) unregister_reboot_notifier(&kvm_reboot_notifier); cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE); kvm_irqfd_exit(); - free_cpumask_var(cpus_hardware_enabled); } EXPORT_SYMBOL_GPL(kvm_exit); -- 2.38.1.431.g37b22c650d-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel