On Thu, Sep 01, 2022 at 07:17:51PM -0700, isaku.yamahata@xxxxxxxxx wrote:
> From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
>
> A global variable hardware_enable_failed in kvm_arch.c is used only by
> kvm_arch_add_vm() and hardware_enable(). It doesn't have to be a global
> variable. Make it function local.
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> ---
> virt/kvm/kvm_arch.c | 49 +++++++++++++++++++++------------------------
> 1 file changed, 23 insertions(+), 26 deletions(-)
>
> diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
> index de39d0127584..f7dcde842eb5 100644
> --- a/virt/kvm/kvm_arch.c
> +++ b/virt/kvm/kvm_arch.c
> @@ -13,14 +13,13 @@
> #include <linux/kvm_host.h>
>
> static cpumask_t cpus_hardware_enabled = CPU_MASK_NONE;
> -static atomic_t hardware_enable_failed;
>
> __weak int kvm_arch_post_init_vm(struct kvm *kvm)
> {
> return 0;
> }
>
> -static void hardware_enable(void *caller_name)
> +static int __hardware_enable(void *caller_name)
> {
> int cpu = raw_smp_processor_id();
> int r;
> @@ -28,18 +27,22 @@ static void hardware_enable(void *caller_name)
> WARN_ON_ONCE(preemptible());
>
> if (cpumask_test_cpu(cpu, &cpus_hardware_enabled))
> - return;
> -
> - cpumask_set_cpu(cpu, &cpus_hardware_enabled);
> -
> + return 0;
> r = kvm_arch_hardware_enable();
> -
> - if (r) {
> - cpumask_clear_cpu(cpu, &cpus_hardware_enabled);
> - atomic_inc(&hardware_enable_failed);
> + if (r)
> pr_warn("kvm: enabling virtualization on CPU%d failed during %s()\n",
> cpu, (const char *)caller_name);
> - }
> + else
> + cpumask_set_cpu(cpu, &cpus_hardware_enabled);
> + return r;
> +}
> +
> +static void hardware_enable(void *arg)
> +{
> + atomic_t *failed = arg;
> +
> + if (__hardware_enable((void *)__func__))
> + atomic_inc(failed);
> }
A side effect: The actual caller_name information introduced in Patch
3 for hardware_enable() is lost. I tend to keep the information, but
depends on you and other guys. :-)
>
> static void hardware_disable(void *junk)
> @@ -65,15 +68,16 @@ __weak void kvm_arch_pre_hardware_unsetup(void)
> */
> __weak int kvm_arch_add_vm(struct kvm *kvm, int usage_count)
> {
> + atomic_t failed;
> int r = 0;
>
> if (usage_count != 1)
> return 0;
>
> - atomic_set(&hardware_enable_failed, 0);
> - on_each_cpu(hardware_enable, (void *)__func__, 1);
> + atomic_set(&failed, 0);
> + on_each_cpu(hardware_enable, &failed, 1);
>
> - if (atomic_read(&hardware_enable_failed)) {
> + if (atomic_read(&failed)) {
> r = -EBUSY;
> goto err;
> }
> @@ -96,27 +100,20 @@ __weak int kvm_arch_del_vm(int usage_count)
>
> __weak int kvm_arch_online_cpu(unsigned int cpu, int usage_count)
> {
> - int ret = 0;
> + int ret;
>
> ret = kvm_arch_check_processor_compat();
> if (ret)
> return ret;
>
> + if (!usage_count)
> + return 0;
> /*
> * Abort the CPU online process if hardware virtualization cannot
> * be enabled. Otherwise running VMs would encounter unrecoverable
> * errors when scheduled to this CPU.
> */
> - if (usage_count) {
> - WARN_ON_ONCE(atomic_read(&hardware_enable_failed));
> -
> - hardware_enable((void *)__func__);
> - if (atomic_read(&hardware_enable_failed)) {
> - atomic_set(&hardware_enable_failed, 0);
> - ret = -EIO;
> - }
> - }
> - return ret;
> + return __hardware_enable((void *)__func__);
> }
>
> __weak int kvm_arch_offline_cpu(unsigned int cpu, int usage_count)
> @@ -149,5 +146,5 @@ __weak void kvm_arch_resume(int usage_count)
> return; /* FIXME: disable KVM */
>
> if (usage_count)
> - hardware_enable((void *)__func__);
> + (void)__hardware_enable((void *)__func__);
> }
> --
> 2.25.1
>
