Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 12 Oct 2022 20:50:59 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     isaku.yamahata@intel.com
Cc:     linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        Paolo Bonzini <pbonzini@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Marc Zyngier <maz@kernel.org>, Will Deacon <will@kernel.org>,
        isaku.yamahata@gmail.com, Kai Huang <kai.huang@intel.com>,
        Chao Gao <chao.gao@intel.com>,
        Atish Patra <atishp@atishpatra.org>,
        Shaokun Zhang <zhangshaokun@hisilicon.com>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Huang Ying <ying.huang@intel.com>,
        Huacai Chen <chenhuacai@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Borislav Petkov <bp@alien8.de>
Subject: Re: [PATCH v5 16/30] KVM: Remove
 on_each_cpu(hardware_disable_nolock) in kvm_exit()
Message-ID: <Y0cos+UtxhvKSFIi@google.com>
References: <cover.1663869838.git.isaku.yamahata@intel.com>
 <54c7065fa08a65f1fcd7f47492f1a83e6f7a3746.1663869838.git.isaku.yamahata@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <54c7065fa08a65f1fcd7f47492f1a83e6f7a3746.1663869838.git.isaku.yamahata@intel.com>
Precedence: bulk

On Thu, Sep 22, 2022, isaku.yamahata@intel.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> hardware_enable/disable_nolock() check if the hardware is already
> enabled/disabled and work as nop when they are called multiple times.
> 
> When VM is created/destroyed, on_each_cpu(hardware_enable/disable_nolock)
> via kvm_arch_add/del_vm() and module_get/put() are called.  It means when
> kvm module is removed, it's guaranteed that there is no vm and that
> hardware_disable_nolock() was called on each cpus.
> 
> Although the module exit function, kvm_exit(), calls
> on_each_cpu(hardware_disable_nolock), it's essentially nop.  Eliminate nop
> call in kvm_exit().

Add a WARN to "prove" that this is a nop, to guard against future bugs, and to
document that it should be impossible for KVM to be unloaded with active VMs.
E.g. do this in addition to dropping the hardware disabling.

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1c1a2b0630bc..ca2251d02c77 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5855,6 +5855,8 @@ void kvm_exit(void)
 {
        int cpu;
 
+       WARN_ON_ONCE(kvm_usage_count);
+
        debugfs_remove_recursive(kvm_debugfs_dir);
        misc_deregister(&kvm_dev);
        for_each_possible_cpu(cpu)

> 
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
>  virt/kvm/kvm_main.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index ad9b8b7d21fa..d7c3bc14691f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -6034,7 +6034,6 @@ void kvm_exit(void)
>  	unregister_syscore_ops(&kvm_syscore_ops);
>  	unregister_reboot_notifier(&kvm_reboot_notifier);
>  	cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE);
> -	on_each_cpu(hardware_disable_nolock, NULL, 1);
>  	kvm_arch_hardware_unsetup();
>  	kvm_arch_exit();
>  	kvm_irqfd_exit();