Re: [PATCH 3/6] KVM: x86/mmu: Speed up dirty logging in tdp_mmu_map_handle_target_level

[David Matlack <dmatlack@google.com>]

On Mon, Aug 02, 2021 at 04:58:17PM +0200, Paolo Bonzini wrote:
> On 31/07/21 00:37, David Matlack wrote:
> > -	if (new_spte == iter->old_spte)
> > +	if (new_spte == iter->old_spte) {
> >   		ret = RET_PF_SPURIOUS;
> > -	else if (!tdp_mmu_set_spte_atomic(vcpu->kvm, iter, new_spte))
> > -		return RET_PF_RETRY;
> > +	} else {
> > +		if (!tdp_mmu_set_spte_atomic_no_dirty_log(vcpu->kvm, iter, new_spte))
> > +			return RET_PF_RETRY;
> > +
> > +		/*
> > +		 * Mark the gfn dirty here rather that through the vcpu-agnostic
> > +		 * handle_changed_spte_dirty_log to leverage vcpu->lru_slot_index.
> > +		 */
> > +		if (is_writable_pte(new_spte))
> > +			kvm_vcpu_mark_page_dirty(vcpu, iter->gfn);
> > +	}
> 
> Looking at the remaining callers of tdp_mmu_set_spte_atomic we have:
> 
> * tdp_mmu_zap_spte_atomic calls it with REMOVED_SPTE as the new_spte, which
> is never writable
> 
> * kvm_tdp_mmu_map calls it for nonleaf SPTEs, which are always writable but
> should not be dirty.
> 
> 
> So I think you should:
> 
> * change those two to tdp_mmu_set_spte_atomic_no_dirty_log
> 
> * add a WARN_ON_ONCE(iter->level > PG_LEVEL_4K) to tdp_mmu_set_spte_atomic
> 
> * put the kvm_vcpu_mark_page_dirty code directly in tdp_mmu_set_spte_atomic,
> instead of the call to handle_changed_spte_dirty_log
> 
> (I can't exclude I'm missing something though).

Makes sense. I'll take a look to confirm and make those changes in v2.

> 
> Paolo
>