Re: [PATCH 13/18] KVM: x86/tdp_mmu: Propagate attr_filter to MMU notifier callbacks

[Yan Zhao <yan.y.zhao@intel.com>]

On Fri, Dec 13, 2024 at 02:57:06PM -0500, Paolo Bonzini wrote:
> Teach the MMU notifier callbacks how to check kvm_gfn_range.process to
> filter which KVM MMU root types to operate on.
> 
> The private GPAs are backed by guest memfd. Such memory is not subjected
> to MMU notifier callbacks because it can't be mapped into the host user
> address space. Now kvm_gfn_range conveys info about which root to operate
> on. Enhance the callback to filter the root page table type.
> 
> The KVM MMU notifier comes down to two functions.
> kvm_tdp_mmu_unmap_gfn_range() and __kvm_tdp_mmu_age_gfn_range():
> - invalidate_range_start() calls kvm_tdp_mmu_unmap_gfn_range()
> - invalidate_range_end() doesn't call into arch code
> - the other callbacks call __kvm_tdp_mmu_age_gfn_range()
> 
> For VM's without a private/shared split in the EPT, all operations
> should target the normal(direct) root.
> 
> With the switch from for_each_tdp_mmu_root() to
> __for_each_tdp_mmu_root() in kvm_tdp_mmu_handle_gfn(), there are no
> longer any users of for_each_tdp_mmu_root(). Remove it.
> 
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Message-ID: <20240718211230.1492011-14-rick.p.edgecombe@intel.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/mmu/tdp_mmu.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index d17a3cf1596d..9a004c999a28 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -193,9 +193,6 @@ static struct kvm_mmu_page *tdp_mmu_next_root(struct kvm *kvm,
>  		     !tdp_mmu_root_match((_root), (_types)))) {			\
>  		} else
>  
> -#define for_each_tdp_mmu_root(_kvm, _root, _as_id)			\
> -	__for_each_tdp_mmu_root(_kvm, _root, _as_id, KVM_ALL_ROOTS)
> -
>  #define for_each_valid_tdp_mmu_root(_kvm, _root, _as_id)		\
>  	__for_each_tdp_mmu_root(_kvm, _root, _as_id, KVM_VALID_ROOTS)
>  
> @@ -1172,12 +1169,16 @@ int kvm_tdp_mmu_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
>  	return ret;
>  }
>  
> +/* Used by mmu notifier via kvm_unmap_gfn_range() */
>  bool kvm_tdp_mmu_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range,
>  				 bool flush)
>  {
> +	enum kvm_tdp_mmu_root_types types;
>  	struct kvm_mmu_page *root;
>  
> -	__for_each_tdp_mmu_root_yield_safe(kvm, root, range->slot->as_id, KVM_ALL_ROOTS)
> +	types = kvm_gfn_range_filter_to_root_types(kvm, range->attr_filter) | KVM_INVALID_ROOTS;
Just curious: 
though adding KVM_INVALID_ROOTS matches the old KVM_ALL_ROOTS, why do we need
to unmap an invalid root?

> +
> +	__for_each_tdp_mmu_root_yield_safe(kvm, root, range->slot->as_id, types)
>  		flush = tdp_mmu_zap_leafs(kvm, root, range->start, range->end,
>  					  range->may_block, flush);
>  
> @@ -1217,17 +1218,21 @@ static bool __kvm_tdp_mmu_age_gfn_range(struct kvm *kvm,
>  					struct kvm_gfn_range *range,
>  					bool test_only)
>  {
> +	enum kvm_tdp_mmu_root_types types;
>  	struct kvm_mmu_page *root;
>  	struct tdp_iter iter;
>  	bool ret = false;
>  
> +	types = kvm_gfn_range_filter_to_root_types(kvm, range->attr_filter);
> +
>  	/*
>  	 * Don't support rescheduling, none of the MMU notifiers that funnel
>  	 * into this helper allow blocking; it'd be dead, wasteful code.  Note,
>  	 * this helper must NOT be used to unmap GFNs, as it processes only
>  	 * valid roots!
>  	 */
> -	for_each_valid_tdp_mmu_root(kvm, root, range->slot->as_id) {
> +	WARN_ON(types & ~KVM_VALID_ROOTS);
> +	__for_each_tdp_mmu_root(kvm, root, range->slot->as_id, types) {
>  		guard(rcu)();
>  
>  		tdp_root_for_each_leaf_pte(iter, kvm, root, range->start, range->end) {
> -- 
> 2.43.5
> 
>