linux-kernel - Re: [RFC PATCH v5 021/104] KVM: x86: Introduce hooks to free VM callback prezap and vm

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20220331195419.GB2084469@ls.amr.corp.intel.com>
Date:   Thu, 31 Mar 2022 12:54:19 -0700
From:   Isaku Yamahata <isaku.yamahata@...il.com>
To:     Kai Huang <kai.huang@...el.com>
Cc:     isaku.yamahata@...el.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, isaku.yamahata@...il.com,
        Paolo Bonzini <pbonzini@...hat.com>,
        Jim Mattson <jmattson@...gle.com>, erdemaktas@...gle.com,
        Connor Kuehl <ckuehl@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>
Subject: Re: [RFC PATCH v5 021/104] KVM: x86: Introduce hooks to free VM
 callback prezap and vm_free

On Thu, Mar 31, 2022 at 04:02:24PM +1300,
Kai Huang <kai.huang@...el.com> wrote:

> On Fri, 2022-03-04 at 11:48 -0800, isaku.yamahata@...el.com wrote:
> > From: Kai Huang <kai.huang@...el.com>
> > 
> > Before tearing down private page tables, TDX requires some resources of the
> > guest TD to be destroyed (i.e. keyID must have been reclaimed, etc).  Add
> > prezap callback before tearing down private page tables for it.
> > 
> > TDX needs to free some resources after other resources (i.e. vcpu related
> > resources).  Add vm_free callback at the end of kvm_arch_destroy_vm().
> > 
> > Signed-off-by: Kai Huang <kai.huang@...el.com>
> > Signed-off-by: Isaku Yamahata <isaku.yamahata@...el.com>
> > ---
> >  arch/x86/include/asm/kvm-x86-ops.h | 2 ++
> >  arch/x86/include/asm/kvm_host.h    | 2 ++
> >  arch/x86/kvm/x86.c                 | 8 ++++++++
> >  3 files changed, 12 insertions(+)
> > 
> > diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> > index 8125d43d3566..ef48dcc98cfc 100644
> > --- a/arch/x86/include/asm/kvm-x86-ops.h
> > +++ b/arch/x86/include/asm/kvm-x86-ops.h
> > @@ -20,7 +20,9 @@ KVM_X86_OP(has_emulated_msr)
> >  KVM_X86_OP(vcpu_after_set_cpuid)
> >  KVM_X86_OP(is_vm_type_supported)
> >  KVM_X86_OP(vm_init)
> > +KVM_X86_OP_NULL(mmu_prezap)
> >  KVM_X86_OP_NULL(vm_destroy)
> > +KVM_X86_OP_NULL(vm_free)
> >  KVM_X86_OP(vcpu_create)
> >  KVM_X86_OP(vcpu_free)
> >  KVM_X86_OP(vcpu_reset)
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 8de357a9ad30..5ff7a0fba311 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1326,7 +1326,9 @@ struct kvm_x86_ops {
> >  	bool (*is_vm_type_supported)(unsigned long vm_type);
> >  	unsigned int vm_size;
> >  	int (*vm_init)(struct kvm *kvm);
> > +	void (*mmu_prezap)(struct kvm *kvm);
> >  	void (*vm_destroy)(struct kvm *kvm);
> > +	void (*vm_free)(struct kvm *kvm);
> >  
> >  	/* Create, but do not attach this VCPU */
> >  	int (*vcpu_create)(struct kvm_vcpu *vcpu);
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index f6438750d190..a48f5c69fadb 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -11779,6 +11779,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
> >  	kvm_page_track_cleanup(kvm);
> >  	kvm_xen_destroy_vm(kvm);
> >  	kvm_hv_destroy_vm(kvm);
> > +	static_call_cond(kvm_x86_vm_free)(kvm);
> >  }
> >  
> >  static void memslot_rmap_free(struct kvm_memory_slot *slot)
> > @@ -12036,6 +12037,13 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
> >  
> >  void kvm_arch_flush_shadow_all(struct kvm *kvm)
> >  {
> > +	/*
> > +	 * kvm_mmu_zap_all() zaps both private and shared page tables.  Before
> > +	 * tearing down private page tables, TDX requires some TD resources to
> > +	 * be destroyed (i.e. keyID must have been reclaimed, etc).  Invoke
> > +	 * kvm_x86_mmu_prezap() for this.
> > +	 */
> > +	static_call_cond(kvm_x86_mmu_prezap)(kvm);
> >  	kvm_mmu_zap_all(kvm);
> >  }
> >  
> 
> The two callbacks are introduced here but they are actually implemented in 2
> patches later (patch 24 KVM: TDX: create/destroy VM structure).  Why not just
> squash this patch to patch 24?  Or at least you can put this patch right before
> patch 24.

Ok. I'll squash this patch into it.
-- 
Isaku Yamahata <isaku.yamahata@...il.com>