On Sun, Dec 22, 2024, Paolo Bonzini wrote: > From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx> > > Extract tdp_mmu_root_match() to check if the root has given types and use > it for the root page table iterator. It checks only_invalid now. > > TDX KVM operates on a shared page table only (Shared-EPT), a mirrored page > table only (Secure-EPT), or both based on the operation. KVM MMU notifier > operations only on shared page table. KVM guest_memfd invalidation > operations only on mirrored page table, and so on. Introduce a centralized > matching function instead of open coding matching logic in the iterator. > The next step is to extend the function to check whether the page is shared > or private > > Link: https://lore.kernel.org/kvm/ZivazWQw1oCU8VBC@xxxxxxxxxx/ > Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx> > Message-ID: <20240718211230.1492011-10-rick.p.edgecombe@xxxxxxxxx> > Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx> > --- > arch/x86/kvm/mmu/tdp_mmu.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c > index 5b01038ddce8..e0ccfdd4200b 100644 > --- a/arch/x86/kvm/mmu/tdp_mmu.c > +++ b/arch/x86/kvm/mmu/tdp_mmu.c > @@ -92,6 +92,14 @@ void kvm_tdp_mmu_put_root(struct kvm *kvm, struct kvm_mmu_page *root) > call_rcu(&root->rcu_head, tdp_mmu_free_sp_rcu_callback); > } > > +static bool tdp_mmu_root_match(struct kvm_mmu_page *root, bool only_valid) > +{ > + if (only_valid && root->role.invalid) > + return false; > + > + return true; Ugh, this is almost as bad as if (x) return true; return false; Just do return !only_valid || root->role.invalid; And I vote to drop the helper, "match" makes me think about things like roles and addresses, not just if a root is valid. > +} > + > /* > * Returns the next root after @prev_root (or the first root if @prev_root is > * NULL). A reference to the returned root is acquired, and the reference to > @@ -125,7 +133,7 @@ static struct kvm_mmu_page *tdp_mmu_next_root(struct kvm *kvm, > typeof(*next_root), link); > > while (next_root) { > - if ((!only_valid || !next_root->role.invalid) && > + if (tdp_mmu_root_match(next_root, only_valid) && E.g. this can just as easily be !only_valid || root->role.invalid && which is amusingly even fewer characters :-D > kvm_tdp_mmu_get_root(next_root)) > break; > > @@ -176,7 +184,7 @@ static struct kvm_mmu_page *tdp_mmu_next_root(struct kvm *kvm, > list_for_each_entry(_root, &_kvm->arch.tdp_mmu_roots, link) \ > if (kvm_lockdep_assert_mmu_lock_held(_kvm, false) && \ > ((_as_id >= 0 && kvm_mmu_page_as_id(_root) != _as_id) || \ > - ((_only_valid) && (_root)->role.invalid))) { \ > + !tdp_mmu_root_match((_root), (_only_valid)))) { \ > } else > > #define for_each_tdp_mmu_root(_kvm, _root, _as_id) \ > -- > 2.43.5 > >
