linux-kernel - Re: [PATCH v3 17/17] KVM: x86/tdp_mmu: Take root types for kvm_tdp_mmu_invalidate_all

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: <ZodJeS3qhO6nJ0di@yzhao56-desk.sh.intel.com>
Date: Fri, 5 Jul 2024 09:16:41 +0800
From: Yan Zhao <yan.y.zhao@...el.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
CC: "Gao, Chao" <chao.gao@...el.com>, "seanjc@...gle.com" <seanjc@...gle.com>,
	"Huang, Kai" <kai.huang@...el.com>, "sagis@...gle.com" <sagis@...gle.com>,
	"isaku.yamahata@...il.com" <isaku.yamahata@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Aktas, Erdem"
	<erdemaktas@...gle.com>, "dmatlack@...gle.com" <dmatlack@...gle.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "Yamahata, Isaku"
	<isaku.yamahata@...el.com>, "pbonzini@...hat.com" <pbonzini@...hat.com>
Subject: Re: [PATCH v3 17/17] KVM: x86/tdp_mmu: Take root types for
 kvm_tdp_mmu_invalidate_all_roots()

On Thu, Jul 04, 2024 at 04:00:18AM +0800, Edgecombe, Rick P wrote:
> On Wed, 2024-06-26 at 10:25 +0800, Yan Zhao wrote:
> > > Maybe better than a comment...? Need to put the whole thing together and
> > > test it
> > > still.
> > Hmm, I think passing KVM_DIRECT_ROOTS only in kvm_tdp_mmu_zap_all() is ok,
> > because kvm_mmu_uninit_tdp_mmu() will zap all invalidated roots eventually,
> > though KVM_DIRECT_ROOTS | KVM_INVALID_ROOTS matches more to the function name
> > zap_all.
> > 
> > I'm not convinced that the change in tdp_mmu_root_match() above is needed.
> > Once a root is marked invalid, it won't be restored later. Distinguishing
> > between invalid direct roots and invalid mirror roots will only result in more
> > unused roots lingering unnecessarily.
> 
> The logic for direct roots is for normal VMs as well. In any case, we should not
> mix generic KVM MMU changes in with mirrored memory additions. So let's keep the
> existing direct root behavior the same.
To keep the existing direct root behavior the same, I think specifying
KVM_DIRECT_ROOTS | KVM_INVALID_ROOTS in kvm_tdp_mmu_zap_all() is enough.

No need to modify tdp_mmu_root_match() do distinguish between invalid direct
roots and invalid mirror roots. As long as a root is invalid, guest is no longer
affected by it and KVM will not use it any more. The last remaining operation
to the invalid root is only zapping.

Distinguishing between invalid direct roots and invalid mirror roots would
make the code to zap invalid roots unnecessarily complex, e.g.

kvm_tdp_mmu_zap_invalidated_roots() is called both in kvm_mmu_uninit_tdp_mmu()
and kvm_mmu_zap_all_fast().

- When called in the former, both invalid direct and invalid mirror roots are
  required to zap;
- when called in the latter, only invalid direct roots are required to zap.