linux-kernel - Re: [PATCH v5 10/30] KVM: Add arch hooks when VM is added/deleted

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221004001621.GJ2414580@ls.amr.corp.intel.com>
Date:   Mon, 3 Oct 2022 17:16:21 -0700
From:   Isaku Yamahata <isaku.yamahata@...il.com>
To:     isaku.yamahata@...el.com
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Marc Zyngier <maz@...nel.org>, Will Deacon <will@...nel.org>,
        isaku.yamahata@...il.com, Kai Huang <kai.huang@...el.com>,
        Chao Gao <chao.gao@...el.com>,
        Atish Patra <atishp@...shpatra.org>,
        Shaokun Zhang <zhangshaokun@...ilicon.com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Huang Ying <ying.huang@...el.com>,
        Huacai Chen <chenhuacai@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH v5 10/30] KVM: Add arch hooks when VM is added/deleted

On Thu, Sep 22, 2022 at 11:20:39AM -0700,
isaku.yamahata@...el.com wrote:

> From: Isaku Yamahata <isaku.yamahata@...el.com>
> 
> and pass kvm_usage_count with kvm_lock.  Move kvm_arch_post_init_vm() under
> kvm_arch_add_vm().  Replace enable/disable_hardware_all() with the default
> implementation of kvm_arch_add/del_vm().  Later kvm_arch_post_init_vm() is
> deleted once x86 overrides kvm_arch_add_vm().
> 
> Suggested-by: Sean Christopherson <seanjc@...gle.com>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@...el.com>
> ---
>  include/linux/kvm_host.h |   2 +
>  virt/kvm/kvm_main.c      | 121 ++++++++++++++++++++-------------------
>  2 files changed, 65 insertions(+), 58 deletions(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index eab352902de7..3fbb01bbac98 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1445,6 +1445,8 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu);
>  bool kvm_arch_dy_runnable(struct kvm_vcpu *vcpu);
>  bool kvm_arch_dy_has_pending_interrupt(struct kvm_vcpu *vcpu);
>  int kvm_arch_post_init_vm(struct kvm *kvm);
> +int kvm_arch_add_vm(struct kvm *kvm, int usage_count);
> +int kvm_arch_del_vm(int usage_count);
>  void kvm_arch_pre_destroy_vm(struct kvm *kvm);
>  int kvm_arch_create_vm_debugfs(struct kvm *kvm);
>  
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index c4b908553726..e2c8823786ff 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -142,8 +142,9 @@ static int kvm_no_compat_open(struct inode *inode, struct file *file)
>  #define KVM_COMPAT(c)	.compat_ioctl	= kvm_no_compat_ioctl,	\
>  			.open		= kvm_no_compat_open
>  #endif
> -static int hardware_enable_all(void);
> -static void hardware_disable_all(void);
> +static void hardware_enable_nolock(void *junk);
> +static void hardware_disable_nolock(void *junk);
> +static void kvm_del_vm(void);
>  
>  static void kvm_io_bus_destroy(struct kvm_io_bus *bus);
>  
> @@ -1106,6 +1107,41 @@ int __weak kvm_arch_post_init_vm(struct kvm *kvm)
>  	return 0;
>  }
>  
> +/*
> + * Called after the VM is otherwise initialized, but just before adding it to
> + * the vm_list.
> + */
> +int __weak kvm_arch_add_vm(struct kvm *kvm, int usage_count)
> +{
> +	int r = 0;
> +
> +	if (usage_count != 1)
> +		return 0;

Oops. This line should be.
+               return kvm_arch_post_init_vm(kvm);

"KVM: x86: Duplicate arch callbacks related to pm events and compat check"
should have same line. "KVM: Eliminate kvm_arch_post_init_vm()" should include
kvm_mmu_post_init_vm().
It creates a kernel thread. kvm unit test didn't catch it.

Thanks,

> +
> +	atomic_set(&hardware_enable_failed, 0);
> +	on_each_cpu(hardware_enable_nolock, NULL, 1);
> +
> +	if (atomic_read(&hardware_enable_failed)) {
> +		r = -EBUSY;
> +		goto err;
> +	}
> +
> +	r = kvm_arch_post_init_vm(kvm);
> +err:
> +	if (r)
> +		on_each_cpu(hardware_disable_nolock, NULL, 1);
> +	return r;
> +}
> +
> +int __weak kvm_arch_del_vm(int usage_count)
> +{
> +	if (usage_count)
> +		return 0;
> +
> +	on_each_cpu(hardware_disable_nolock, NULL, 1);
> +	return 0;
> +}
> +
>  /*
>   * Called just after removing the VM from the vm_list, but before doing any
>   * other destruction.
-- 
Isaku Yamahata <isaku.yamahata@...il.com>