linux-kernel - Re: [PATCH 13/18] KVM: x86/tdp_mmu: Propagate attr

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: <Z2F0MMiHf/CHwGjd@yzhao56-desk.sh.intel.com>
Date: Tue, 17 Dec 2024 20:53:04 +0800
From: Yan Zhao <yan.y.zhao@...el.com>
To: Paolo Bonzini <pbonzini@...hat.com>
CC: <linux-kernel@...r.kernel.org>, <kvm@...r.kernel.org>,
	<isaku.yamahata@...el.com>, <binbin.wu@...ux.intel.com>,
	<rick.p.edgecombe@...el.com>
Subject: Re: [PATCH 13/18] KVM: x86/tdp_mmu: Propagate attr_filter to MMU
 notifier callbacks

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@...el.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
> Message-ID: <20240718211230.1492011-14-rick.p.edgecombe@...el.com>
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.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
> 
>