| Message ID | 20241011021051.1557902-15-seanjc@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: x86/mmu: A/D cleanups (on top of kvm_follow_pfn) | expand |
On 10/11/24 04:10, Sean Christopherson wrote: > Return immediately if a young SPTE is found when testing, but not updating, > SPTEs. The return value is a boolean, i.e. whether there is one young SPTE > or fifty is irrelevant (ignoring the fact that it's impossible for there to > be fifty SPTEs, as KVM has a hard limit on the number of valid TDP MMU > roots). > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/mmu/tdp_mmu.c | 84 ++++++++++++++++++-------------------- > 1 file changed, 40 insertions(+), 44 deletions(-) > > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c > index e8c061bf94ec..f412bca206c5 100644 > --- a/arch/x86/kvm/mmu/tdp_mmu.c > +++ b/arch/x86/kvm/mmu/tdp_mmu.c > @@ -1192,35 +1192,6 @@ bool kvm_tdp_mmu_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range, > return flush; > } > > -typedef bool (*tdp_handler_t)(struct kvm *kvm, struct tdp_iter *iter, > - struct kvm_gfn_range *range); > - > -static __always_inline bool kvm_tdp_mmu_handle_gfn(struct kvm *kvm, > - struct kvm_gfn_range *range, > - tdp_handler_t handler) > -{ > - struct kvm_mmu_page *root; > - struct tdp_iter iter; > - bool ret = false; > - > - /* > - * 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) { > - rcu_read_lock(); > - > - tdp_root_for_each_leaf_pte(iter, root, range->start, range->end) > - ret |= handler(kvm, &iter, range); > - > - rcu_read_unlock(); > - } > - > - return ret; > -} > - > /* > * Mark the SPTEs range of GFNs [start, end) unaccessed and return non-zero > * if any of the GFNs in the range have been accessed. > @@ -1229,15 +1200,10 @@ static __always_inline bool kvm_tdp_mmu_handle_gfn(struct kvm *kvm, > * from the clear_young() or clear_flush_young() notifier, which uses the > * return value to determine if the page has been accessed. > */ > -static bool age_gfn_range(struct kvm *kvm, struct tdp_iter *iter, > - struct kvm_gfn_range *range) > +static void kvm_tdp_mmu_age_spte(struct tdp_iter *iter) > { > u64 new_spte; > > - /* If we have a non-accessed entry we don't need to change the pte. */ > - if (!is_accessed_spte(iter->old_spte)) > - return false; > - > if (spte_ad_enabled(iter->old_spte)) { > iter->old_spte = tdp_mmu_clear_spte_bits(iter->sptep, > iter->old_spte, > @@ -1253,23 +1219,53 @@ static bool age_gfn_range(struct kvm *kvm, struct tdp_iter *iter, > > trace_kvm_tdp_mmu_spte_changed(iter->as_id, iter->gfn, iter->level, > iter->old_spte, new_spte); > - return true; > +} > + > +static bool __kvm_tdp_mmu_age_gfn_range(struct kvm *kvm, > + struct kvm_gfn_range *range, > + bool test_only) > +{ > + struct kvm_mmu_page *root; > + struct tdp_iter iter; > + bool ret = false; > + > + /* > + * 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) { > + rcu_read_lock(); > + > + tdp_root_for_each_leaf_pte(iter, root, range->start, range->end) { > + if (!is_accessed_spte(iter.old_spte)) > + continue; > + > + ret = true; > + if (test_only) > + break; > + > + kvm_tdp_mmu_age_spte(&iter); > + } > + > + rcu_read_unlock(); > + > + if (ret && test_only) > + break; > + } > + > + return ret; > } If you use guard(rcu)() you can avoid the repeated breaks: for_each_valid_tdp_mmu_root(kvm, root, range->slot->as_id) { guard(rcu)(); tdp_root_for_each_leaf_pte(iter, root, range->start, range->end) { if (!is_accessed_spte(iter.old_spte)) continue; ret = true; if (test_only) return ret; kvm_tdp_mmu_age_spte(&iter); } } return ret; Paolo
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c index e8c061bf94ec..f412bca206c5 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.c +++ b/arch/x86/kvm/mmu/tdp_mmu.c @@ -1192,35 +1192,6 @@ bool kvm_tdp_mmu_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range, return flush; } -typedef bool (*tdp_handler_t)(struct kvm *kvm, struct tdp_iter *iter, - struct kvm_gfn_range *range); - -static __always_inline bool kvm_tdp_mmu_handle_gfn(struct kvm *kvm, - struct kvm_gfn_range *range, - tdp_handler_t handler) -{ - struct kvm_mmu_page *root; - struct tdp_iter iter; - bool ret = false; - - /* - * 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) { - rcu_read_lock(); - - tdp_root_for_each_leaf_pte(iter, root, range->start, range->end) - ret |= handler(kvm, &iter, range); - - rcu_read_unlock(); - } - - return ret; -} - /* * Mark the SPTEs range of GFNs [start, end) unaccessed and return non-zero * if any of the GFNs in the range have been accessed. @@ -1229,15 +1200,10 @@ static __always_inline bool kvm_tdp_mmu_handle_gfn(struct kvm *kvm, * from the clear_young() or clear_flush_young() notifier, which uses the * return value to determine if the page has been accessed. */ -static bool age_gfn_range(struct kvm *kvm, struct tdp_iter *iter, - struct kvm_gfn_range *range) +static void kvm_tdp_mmu_age_spte(struct tdp_iter *iter) { u64 new_spte; - /* If we have a non-accessed entry we don't need to change the pte. */ - if (!is_accessed_spte(iter->old_spte)) - return false; - if (spte_ad_enabled(iter->old_spte)) { iter->old_spte = tdp_mmu_clear_spte_bits(iter->sptep, iter->old_spte, @@ -1253,23 +1219,53 @@ static bool age_gfn_range(struct kvm *kvm, struct tdp_iter *iter, trace_kvm_tdp_mmu_spte_changed(iter->as_id, iter->gfn, iter->level, iter->old_spte, new_spte); - return true; +} + +static bool __kvm_tdp_mmu_age_gfn_range(struct kvm *kvm, + struct kvm_gfn_range *range, + bool test_only) +{ + struct kvm_mmu_page *root; + struct tdp_iter iter; + bool ret = false; + + /* + * 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) { + rcu_read_lock(); + + tdp_root_for_each_leaf_pte(iter, root, range->start, range->end) { + if (!is_accessed_spte(iter.old_spte)) + continue; + + ret = true; + if (test_only) + break; + + kvm_tdp_mmu_age_spte(&iter); + } + + rcu_read_unlock(); + + if (ret && test_only) + break; + } + + return ret; } bool kvm_tdp_mmu_age_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range) { - return kvm_tdp_mmu_handle_gfn(kvm, range, age_gfn_range); -} - -static bool test_age_gfn(struct kvm *kvm, struct tdp_iter *iter, - struct kvm_gfn_range *range) -{ - return is_accessed_spte(iter->old_spte); + return __kvm_tdp_mmu_age_gfn_range(kvm, range, false); } bool kvm_tdp_mmu_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range) { - return kvm_tdp_mmu_handle_gfn(kvm, range, test_age_gfn); + return __kvm_tdp_mmu_age_gfn_range(kvm, range, true); } /*
Return immediately if a young SPTE is found when testing, but not updating, SPTEs. The return value is a boolean, i.e. whether there is one young SPTE or fifty is irrelevant (ignoring the fact that it's impossible for there to be fifty SPTEs, as KVM has a hard limit on the number of valid TDP MMU roots). Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/mmu/tdp_mmu.c | 84 ++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 44 deletions(-)