linux-kernel - Re: [PATCH 20/30] KVM: TDX: create/destroy VM structure

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: <5da952c534b68bba9fbfddf2197209cb719f5e41.camel@intel.com>
Date: Sat, 22 Feb 2025 00:30:37 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "pbonzini@...hat.com" <pbonzini@...hat.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>
CC: "Huang, Kai" <kai.huang@...el.com>, "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>, "sean.j.christopherson@...el.com"
	<sean.j.christopherson@...el.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Zhao, Yan Y" <yan.y.zhao@...el.com>,
	"Yamahata, Isaku" <isaku.yamahata@...el.com>, "tony.lindgren@...ux.intel.com"
	<tony.lindgren@...ux.intel.com>
Subject: Re: [PATCH 20/30] KVM: TDX: create/destroy VM structure

On Thu, 2025-02-20 at 17:08 -0800, Sean Christopherson wrote:
> Argh, after digging more, this isn't actually true.  The separate "unload MMUs"
> code is leftover from when KVM rather stupidly tried to free all MMU pages when
> freeing a vCPU.
> 
> Commit 7b53aa565084 ("KVM: Fix vcpu freeing for guest smp") "fixed" things by
> unloading MMUs before destroying vCPUs, but the underlying problem was trying to
> free _all_ MMU pages when destroying a single vCPU.  That eventually got fixed
> for good (haven't checked when), but the separate MMU unloading never got cleaned
> up.
> 
> So, scratch the mmu_destroy() idea.  But I still want/need to move vCPU destruction
> before vm_destroy.
> 
> Now that kvm_arch_pre_destroy_vm() is a thing, one idea would be to add
> kvm_x86_ops.pre_destroy_vm(), which would pair decently well with the existing
> call to kvm_mmu_pre_destroy_vm().

So:
kvm_x86_call(vm_destroy)(kvm); -> tdx_mmu_release_hkid()
kvm_destroy_vcpus(kvm); -> tdx_vcpu_free() -> reclaim
static_call_cond(kvm_x86_vm_free)(kvm); -> reclaim

To:
(pre_destroy_vm)() -> tdx_mmu_release_hkid()
kvm_destroy_vcpus(kvm); -> reclaim
kvm_x86_call(vm_destroy)(kvm); -> nothing
static_call_cond(kvm_x86_vm_free)(kvm); -> reclaim

I'm not seeing a problem. We can follow up with a real check once you post the
patches. I'm not 100% confident on the shape of the proposal. But in general if
we can add more callbacks it seems likely that we can reproduce the current
order. At this stage it seems safer to do that then anything more clever