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.
