From mboxrd@z Thu Jan  1 00:00:00 1970
From: Izik Eidus <ieidus@redhat.com>
Subject: Re: [PATCH 2/2] kvm: change the dirty page tracking to work with
 dirty bit instead of page fault
Date: Wed, 10 Jun 2009 15:02:47 +0300
Message-ID: <4A2FA0E7.6070909@redhat.com>
References: <1244634691-9765-1-git-send-email-ieidus@redhat.com> <1244634691-9765-2-git-send-email-ieidus@redhat.com> <1244634691-9765-3-git-send-email-ieidus@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: avi@redhat.com
To: kvm@vger.kernel.org
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:43732 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751157AbZFJMCy (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 10 Jun 2009 08:02:54 -0400
Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26])
	by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n5AC2vep030293
	for <kvm@vger.kernel.org>; Wed, 10 Jun 2009 08:02:57 -0400
In-Reply-To: <1244634691-9765-3-git-send-email-ieidus@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Few quick thoughts:


>  
> +void kvm_arch_get_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot)
> +{
> +}
> +
>  long kvm_arch_vm_ioctl(struct file *filp,
>  		       unsigned int ioctl, unsigned long arg)
>  {
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index c7b0cc2..8a24149 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -527,6 +527,7 @@ struct kvm_x86_ops {
>  	int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
>  	int (*get_tdp_level)(void);
>  	u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
> +	int (*dirty_bit_support)(void);
>  };
>  
>  extern struct kvm_x86_ops *kvm_x86_ops;
> @@ -796,4 +797,6 @@ int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
>  int kvm_age_hva(struct kvm *kvm, unsigned long hva);
>  int cpuid_maxphyaddr(struct kvm_vcpu *vcpu);
>  
> +int is_dirty_and_clean_rmapp(struct kvm *kvm, unsigned long *rmapp);
> +
>  #endif /* _ASM_X86_KVM_HOST_H */
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 809cce0..3ec6a7d 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -140,6 +140,8 @@ module_param(oos_shadow, bool, 0644);
>  #define ACC_USER_MASK    PT_USER_MASK
>  #define ACC_ALL          (ACC_EXEC_MASK | ACC_WRITE_MASK | ACC_USER_MASK)
>  
> +#define SPTE_DONT_DIRTY (1ULL << PT_FIRST_AVAIL_BITS_SHIFT)
> +
>  #define SHADOW_PT_INDEX(addr, level) PT64_INDEX(addr, level)
>  
>  struct kvm_rmap_desc {
> @@ -629,6 +631,25 @@ static u64 *rmap_next(struct kvm *kvm, unsigned long *rmapp, u64 *spte)
>  	return NULL;
>  }
>  
> +int is_dirty_and_clean_rmapp(struct kvm *kvm, unsigned long *rmapp)
> +{
> +	u64 *spte;
> +	int dirty = 0;
> +
>   

Here we should add:

if (!shadow_dirty_mask)
    return 0;


> +	spte = rmap_next(kvm, rmapp, NULL);
> +	while (spte) {
> +		if (*spte & PT_DIRTY_MASK) {
> +			set_shadow_pte(spte, (*spte &= ~PT_DIRTY_MASK) |
> +				       SPTE_DONT_DIRTY);
> +			dirty = 1;
> +		}
> +		spte = rmap_next(kvm, rmapp, spte);
> +	}
> +
> +	return dirty;
> +}
> +


>   */
> @@ -1982,9 +1995,11 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
>  
>  	/* If nothing is dirty, don't bother messing with page tables. */
>  	if (is_dirty) {
> -		spin_lock(&kvm->mmu_lock);
> -		kvm_mmu_slot_remove_write_access(kvm, log->slot);
> -		spin_unlock(&kvm->mmu_lock);
> +		if (kvm_x86_ops->dirty_bit_support()) {
>   

This should be  if (kvm_x86_ops->dirty_bit_support() -> if 
(!kvm_x86_ops->dirty_bit_support())

> +			spin_lock(&kvm->mmu_lock);
> +			kvm_mmu_slot_remove_write_access(kvm, log->slot);
> +			spin_unlock(&kvm->mmu_lock);
> +		}
>