| Message ID | 20211119235759.1304274-9-dmatlack@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86/mmu: Eager Page Splitting for the TDP MMU | expand |
On Fri, Nov 19, 2021 at 3:58 PM David Matlack <dmatlack@google.com> wrote: > > Consolidate is_large_pte and is_present_pte into a single helper. This > will be used in a follow-up commit to check for present large-pages > during Eager Page Splitting. > > No functional change intended. > > Signed-off-by: David Matlack <dmatlack@google.com> Reviewed-by: Ben Gardon <bgardon@google.com> > --- > arch/x86/kvm/mmu/spte.h | 5 +++++ > arch/x86/kvm/mmu/tdp_mmu.c | 3 +-- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h > index cc432f9a966b..e73c41d31816 100644 > --- a/arch/x86/kvm/mmu/spte.h > +++ b/arch/x86/kvm/mmu/spte.h > @@ -257,6 +257,11 @@ static inline bool is_large_pte(u64 pte) > return pte & PT_PAGE_SIZE_MASK; > } > > +static inline bool is_large_present_pte(u64 pte) > +{ > + return is_shadow_present_pte(pte) && is_large_pte(pte); > +} > + > static inline bool is_last_spte(u64 pte, int level) > { > return (level == PG_LEVEL_4K) || is_large_pte(pte); > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c > index ff4d83ad7580..f8c4337f1fcf 100644 > --- a/arch/x86/kvm/mmu/tdp_mmu.c > +++ b/arch/x86/kvm/mmu/tdp_mmu.c > @@ -1011,8 +1011,7 @@ int kvm_tdp_mmu_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault) > * than the target, that SPTE must be cleared and replaced > * with a non-leaf SPTE. > */ > - if (is_shadow_present_pte(iter.old_spte) && > - is_large_pte(iter.old_spte)) { > + if (is_large_present_pte(iter.old_spte)) { I'm amazed there's only one instance of a check for present and large. > if (!tdp_mmu_zap_spte_atomic(vcpu->kvm, &iter)) > break; > } > -- > 2.34.0.rc2.393.gf8c9666880-goog >
On Fri, Nov 19, 2021, David Matlack wrote: > Consolidate is_large_pte and is_present_pte into a single helper. This > will be used in a follow-up commit to check for present large-pages > during Eager Page Splitting. > > No functional change intended. > > Signed-off-by: David Matlack <dmatlack@google.com> > --- > arch/x86/kvm/mmu/spte.h | 5 +++++ > arch/x86/kvm/mmu/tdp_mmu.c | 3 +-- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h > index cc432f9a966b..e73c41d31816 100644 > --- a/arch/x86/kvm/mmu/spte.h > +++ b/arch/x86/kvm/mmu/spte.h > @@ -257,6 +257,11 @@ static inline bool is_large_pte(u64 pte) > return pte & PT_PAGE_SIZE_MASK; > } > > +static inline bool is_large_present_pte(u64 pte) > +{ > + return is_shadow_present_pte(pte) && is_large_pte(pte); > +} > + > static inline bool is_last_spte(u64 pte, int level) > { > return (level == PG_LEVEL_4K) || is_large_pte(pte); > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c > index ff4d83ad7580..f8c4337f1fcf 100644 > --- a/arch/x86/kvm/mmu/tdp_mmu.c > +++ b/arch/x86/kvm/mmu/tdp_mmu.c > @@ -1011,8 +1011,7 @@ int kvm_tdp_mmu_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault) > * than the target, that SPTE must be cleared and replaced > * with a non-leaf SPTE. > */ > - if (is_shadow_present_pte(iter.old_spte) && > - is_large_pte(iter.old_spte)) { > + if (is_large_present_pte(iter.old_spte)) { I strongly object to this helper. PRESENT in hardware and shadow-present are two very different things, the name is_large_present_pte() doesn't capture that detail. Yeah, we could name it is_large_shadow_present_pte(), but for me at least that requires more effort to read, and it's not like this is replacing 10s of instances. > if (!tdp_mmu_zap_spte_atomic(vcpu->kvm, &iter)) > break; > } > -- > 2.34.0.rc2.393.gf8c9666880-goog >
On Wed, Dec 1, 2021 at 10:34 AM Sean Christopherson <seanjc@google.com> wrote: > > On Fri, Nov 19, 2021, David Matlack wrote: > > Consolidate is_large_pte and is_present_pte into a single helper. This > > will be used in a follow-up commit to check for present large-pages > > during Eager Page Splitting. > > > > No functional change intended. > > > > Signed-off-by: David Matlack <dmatlack@google.com> > > --- > > arch/x86/kvm/mmu/spte.h | 5 +++++ > > arch/x86/kvm/mmu/tdp_mmu.c | 3 +-- > > 2 files changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h > > index cc432f9a966b..e73c41d31816 100644 > > --- a/arch/x86/kvm/mmu/spte.h > > +++ b/arch/x86/kvm/mmu/spte.h > > @@ -257,6 +257,11 @@ static inline bool is_large_pte(u64 pte) > > return pte & PT_PAGE_SIZE_MASK; > > } > > > > +static inline bool is_large_present_pte(u64 pte) > > +{ > > + return is_shadow_present_pte(pte) && is_large_pte(pte); > > +} > > + > > static inline bool is_last_spte(u64 pte, int level) > > { > > return (level == PG_LEVEL_4K) || is_large_pte(pte); > > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c > > index ff4d83ad7580..f8c4337f1fcf 100644 > > --- a/arch/x86/kvm/mmu/tdp_mmu.c > > +++ b/arch/x86/kvm/mmu/tdp_mmu.c > > @@ -1011,8 +1011,7 @@ int kvm_tdp_mmu_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault) > > * than the target, that SPTE must be cleared and replaced > > * with a non-leaf SPTE. > > */ > > - if (is_shadow_present_pte(iter.old_spte) && > > - is_large_pte(iter.old_spte)) { > > + if (is_large_present_pte(iter.old_spte)) { > > I strongly object to this helper. PRESENT in hardware and shadow-present are two > very different things, the name is_large_present_pte() doesn't capture that detail. > Yeah, we could name it is_large_shadow_present_pte(), but for me at least that > requires more effort to read, and it's not like this is replacing 10s of instances. Ok I'll drop it in v1. > > > if (!tdp_mmu_zap_spte_atomic(vcpu->kvm, &iter)) > > break; > > } > > -- > > 2.34.0.rc2.393.gf8c9666880-goog > >
diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h index cc432f9a966b..e73c41d31816 100644 --- a/arch/x86/kvm/mmu/spte.h +++ b/arch/x86/kvm/mmu/spte.h @@ -257,6 +257,11 @@ static inline bool is_large_pte(u64 pte) return pte & PT_PAGE_SIZE_MASK; } +static inline bool is_large_present_pte(u64 pte) +{ + return is_shadow_present_pte(pte) && is_large_pte(pte); +} + static inline bool is_last_spte(u64 pte, int level) { return (level == PG_LEVEL_4K) || is_large_pte(pte); diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c index ff4d83ad7580..f8c4337f1fcf 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.c +++ b/arch/x86/kvm/mmu/tdp_mmu.c @@ -1011,8 +1011,7 @@ int kvm_tdp_mmu_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault) * than the target, that SPTE must be cleared and replaced * with a non-leaf SPTE. */ - if (is_shadow_present_pte(iter.old_spte) && - is_large_pte(iter.old_spte)) { + if (is_large_present_pte(iter.old_spte)) { if (!tdp_mmu_zap_spte_atomic(vcpu->kvm, &iter)) break; }
Consolidate is_large_pte and is_present_pte into a single helper. This will be used in a follow-up commit to check for present large-pages during Eager Page Splitting. No functional change intended. Signed-off-by: David Matlack <dmatlack@google.com> --- arch/x86/kvm/mmu/spte.h | 5 +++++ arch/x86/kvm/mmu/tdp_mmu.c | 3 +-- 2 files changed, 6 insertions(+), 2 deletions(-)