Re: [PATCH 2/2] kvm: change the dirty page tracking to work with dirty bit instead of page fault

[Izik Eidus <ieidus@redhat.com>]

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);
> +		}
>