On Wednesday 22 June 2011 14:15:51 Izik Eidus wrote:
> On 6/22/2011 3:21 AM, Chris Wright wrote:
> > * Nai Xia (nai.xia@xxxxxxxxx) 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<nai.xia@xxxxxxxxx>
> >> Acked-by: Izik Eidus<izik.eidus@xxxxxxxxxxxxxxxxxx>
> >> ---
> >> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
