On Wednesday 22 June 2011 14:15:51 Izik Eidus wrote: > On 6/22/2011 3:21 AM, Chris Wright wrote: > > * Nai Xia ([email protected]) wrote: > >> Introduced kvm_mmu_notifier_test_and_clear_dirty(), > >> kvm_mmu_notifier_dirty_update() > >> and their mmu_notifier interfaces to support KSM dirty bit tracking, which > >> brings > >> significant performance gain in volatile pages scanning in KSM. > >> Currently, kvm_mmu_notifier_dirty_update() returns 0 if and only if intel > >> EPT is > >> enabled to indicate that the dirty bits of underlying sptes are not > >> updated by > >> hardware. > > Did you test with each of EPT, NPT and shadow? > > > >> Signed-off-by: Nai Xia<[email protected]> > >> Acked-by: Izik Eidus<[email protected]> > >> --- > >> arch/x86/include/asm/kvm_host.h | 1 + > >> arch/x86/kvm/mmu.c | 36 +++++++++++++++++++++++++++++ > >> arch/x86/kvm/mmu.h | 3 +- > >> arch/x86/kvm/vmx.c | 1 + > >> include/linux/kvm_host.h | 2 +- > >> include/linux/mmu_notifier.h | 48 > >> +++++++++++++++++++++++++++++++++++++++ > >> mm/mmu_notifier.c | 33 ++++++++++++++++++++++++++ > >> virt/kvm/kvm_main.c | 27 ++++++++++++++++++++++ > >> 8 files changed, 149 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/x86/include/asm/kvm_host.h > >> b/arch/x86/include/asm/kvm_host.h > >> index d2ac8e2..f0d7aa0 100644 > >> --- a/arch/x86/include/asm/kvm_host.h > >> +++ b/arch/x86/include/asm/kvm_host.h > >> @@ -848,6 +848,7 @@ extern bool kvm_rebooting; > >> int kvm_unmap_hva(struct kvm *kvm, unsigned long hva); > >> int kvm_age_hva(struct kvm *kvm, unsigned long hva); > >> int kvm_test_age_hva(struct kvm *kvm, unsigned long hva); > >> +int kvm_test_and_clear_dirty_hva(struct kvm *kvm, unsigned long hva); > >> void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte); > >> int cpuid_maxphyaddr(struct kvm_vcpu *vcpu); > >> int kvm_cpu_has_interrupt(struct kvm_vcpu *vcpu); > >> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > >> index aee3862..a5a0c51 100644 > >> --- a/arch/x86/kvm/mmu.c > >> +++ b/arch/x86/kvm/mmu.c > >> @@ -979,6 +979,37 @@ out: > >> return young; > >> } > >> > >> +/* > >> + * Caller is supposed to SetPageDirty(), it's not done inside this. > >> + */ > >> +static > >> +int kvm_test_and_clear_dirty_rmapp(struct kvm *kvm, unsigned long *rmapp, > >> + unsigned long data) > >> +{ > >> + u64 *spte; > >> + int dirty = 0; > >> + > >> + if (!shadow_dirty_mask) { > >> + WARN(1, "KVM: do NOT try to test dirty bit in EPT\n"); > >> + goto out; > >> + } > > This should never fire with the dirty_update() notifier test, right? > > And that means that this whole optimization is for the shadow mmu case, > > arguably the legacy case. > > > > Hi Chris, > AMD npt does track the dirty bit in the nested page tables, > so the shadow_dirty_mask should not be 0 in that case... > Hi Izik, I think he meant that if the caller is doing right && (!shadow_dirty_mask), the kvm_test_and_clear_dirty_rmapp() will never be called at all. So this test inside kvm_test_and_clear_dirty_rmapp() is useless...as I said I added this test in any case of this interface abused by others, just like a softer BUG_ON() --- dirty bit is not that critical to bump into BUG().
-- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
