| Message ID | 1458911978-19430-4-git-send-email-guangrong.xiao@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 25/03/2016 14:19, Xiao Guangrong wrote: > The obsolete sp should not be used on current vCPUs and should not hurt > vCPU's running, so skip it from for_each_gfn_sp() and > for_each_gfn_indirect_valid_sp() > > The side effort is we will double check role.invalid in kvm_mmu_get_page() > but i think it is okay as role is well cached > > Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> Queued for 4.7. Paolo > --- > arch/x86/kvm/mmu.c | 28 ++++++++++++---------------- > 1 file changed, 12 insertions(+), 16 deletions(-) > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index c396e8b..4d66a9e 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -1906,18 +1906,17 @@ static void kvm_mmu_commit_zap_page(struct kvm *kvm, > * since it has been deleted from active_mmu_pages but still can be found > * at hast list. > * > - * for_each_gfn_indirect_valid_sp has skipped that kind of page and > - * kvm_mmu_get_page(), the only user of for_each_gfn_sp(), has skipped > - * all the obsolete pages. > + * for_each_gfn_valid_sp() has skipped that kind of pages. > */ > -#define for_each_gfn_sp(_kvm, _sp, _gfn) \ > +#define for_each_gfn_valid_sp(_kvm, _sp, _gfn) \ > hlist_for_each_entry(_sp, \ > &(_kvm)->arch.mmu_page_hash[kvm_page_table_hashfn(_gfn)], hash_link) \ > - if ((_sp)->gfn != (_gfn)) {} else > + if ((_sp)->gfn != (_gfn) || is_obsolete_sp((_kvm), (_sp)) \ > + || (_sp)->role.invalid) {} else > > #define for_each_gfn_indirect_valid_sp(_kvm, _sp, _gfn) \ > - for_each_gfn_sp(_kvm, _sp, _gfn) \ > - if ((_sp)->role.direct || (_sp)->role.invalid) {} else > + for_each_gfn_valid_sp(_kvm, _sp, _gfn) \ > + if ((_sp)->role.direct) {} else > > /* @sp->gfn should be write-protected at the call site */ > static bool __kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp, > @@ -1958,6 +1957,11 @@ static void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point) { } > static void mmu_audit_disable(void) { } > #endif > > +static bool is_obsolete_sp(struct kvm *kvm, struct kvm_mmu_page *sp) > +{ > + return unlikely(sp->mmu_valid_gen != kvm->arch.mmu_valid_gen); > +} > + > static bool kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp, > struct list_head *invalid_list) > { > @@ -2092,11 +2096,6 @@ static void clear_sp_write_flooding_count(u64 *spte) > __clear_sp_write_flooding_count(sp); > } > > -static bool is_obsolete_sp(struct kvm *kvm, struct kvm_mmu_page *sp) > -{ > - return unlikely(sp->mmu_valid_gen != kvm->arch.mmu_valid_gen); > -} > - > static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu, > gfn_t gfn, > gva_t gaddr, > @@ -2123,10 +2122,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu, > quadrant &= (1 << ((PT32_PT_BITS - PT64_PT_BITS) * level)) - 1; > role.quadrant = quadrant; > } > - for_each_gfn_sp(vcpu->kvm, sp, gfn) { > - if (is_obsolete_sp(vcpu->kvm, sp)) > - continue; > - > + for_each_gfn_valid_sp(vcpu->kvm, sp, gfn) { > if (!need_sync && sp->unsync) > need_sync = true; > > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index c396e8b..4d66a9e 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -1906,18 +1906,17 @@ static void kvm_mmu_commit_zap_page(struct kvm *kvm, * since it has been deleted from active_mmu_pages but still can be found * at hast list. * - * for_each_gfn_indirect_valid_sp has skipped that kind of page and - * kvm_mmu_get_page(), the only user of for_each_gfn_sp(), has skipped - * all the obsolete pages. + * for_each_gfn_valid_sp() has skipped that kind of pages. */ -#define for_each_gfn_sp(_kvm, _sp, _gfn) \ +#define for_each_gfn_valid_sp(_kvm, _sp, _gfn) \ hlist_for_each_entry(_sp, \ &(_kvm)->arch.mmu_page_hash[kvm_page_table_hashfn(_gfn)], hash_link) \ - if ((_sp)->gfn != (_gfn)) {} else + if ((_sp)->gfn != (_gfn) || is_obsolete_sp((_kvm), (_sp)) \ + || (_sp)->role.invalid) {} else #define for_each_gfn_indirect_valid_sp(_kvm, _sp, _gfn) \ - for_each_gfn_sp(_kvm, _sp, _gfn) \ - if ((_sp)->role.direct || (_sp)->role.invalid) {} else + for_each_gfn_valid_sp(_kvm, _sp, _gfn) \ + if ((_sp)->role.direct) {} else /* @sp->gfn should be write-protected at the call site */ static bool __kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp, @@ -1958,6 +1957,11 @@ static void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point) { } static void mmu_audit_disable(void) { } #endif +static bool is_obsolete_sp(struct kvm *kvm, struct kvm_mmu_page *sp) +{ + return unlikely(sp->mmu_valid_gen != kvm->arch.mmu_valid_gen); +} + static bool kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp, struct list_head *invalid_list) { @@ -2092,11 +2096,6 @@ static void clear_sp_write_flooding_count(u64 *spte) __clear_sp_write_flooding_count(sp); } -static bool is_obsolete_sp(struct kvm *kvm, struct kvm_mmu_page *sp) -{ - return unlikely(sp->mmu_valid_gen != kvm->arch.mmu_valid_gen); -} - static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu, gfn_t gfn, gva_t gaddr, @@ -2123,10 +2122,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu, quadrant &= (1 << ((PT32_PT_BITS - PT64_PT_BITS) * level)) - 1; role.quadrant = quadrant; } - for_each_gfn_sp(vcpu->kvm, sp, gfn) { - if (is_obsolete_sp(vcpu->kvm, sp)) - continue; - + for_each_gfn_valid_sp(vcpu->kvm, sp, gfn) { if (!need_sync && sp->unsync) need_sync = true;
The obsolete sp should not be used on current vCPUs and should not hurt vCPU's running, so skip it from for_each_gfn_sp() and for_each_gfn_indirect_valid_sp() The side effort is we will double check role.invalid in kvm_mmu_get_page() but i think it is okay as role is well cached Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> --- arch/x86/kvm/mmu.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-)