From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [PATCH v3 05/19] KVM: MMU: optimize to handle dirty bit Date: Thu, 07 Jul 2011 03:12:26 +0800 Message-ID: <4E14B39A.5020401@cn.fujitsu.com> References: <4E0C3178.2080603@cn.fujitsu.com> <4E0C320A.8080104@cn.fujitsu.com> <20110705192703.GB24417@amt.cnet> <4E13B8D4.3050704@cn.fujitsu.com> <20110706165134.GC24417@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , LKML , KVM To: Marcelo Tosatti Return-path: In-Reply-To: <20110706165134.GC24417@amt.cnet> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 07/07/2011 12:51 AM, Marcelo Tosatti wrote: > On Wed, Jul 06, 2011 at 09:22:28AM +0800, Xiao Guangrong wrote: >> On 07/06/2011 03:27 AM, Marcelo Tosatti wrote: >>> On Thu, Jun 30, 2011 at 04:21:30PM +0800, Xiao Guangrong wrote: >>>> If dirty bit is not set, we can make the pte access read-only to avoid handing >>>> dirty bit everywhere >>>> >>>> Signed-off-by: Xiao Guangrong >>> >>>> -static unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, pt_element_t gpte) >>>> +static unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, pt_element_t gpte, >>>> + bool last) >>>> { >>>> unsigned access; >>>> >>>> access = (gpte & (PT_WRITABLE_MASK | PT_USER_MASK)) | ACC_EXEC_MASK; >>>> + if (last && !is_dirty_gpte(gpte)) >>>> + access &= ~ACC_WRITE_MASK; >>>> + >>> >>> What if the walker marks the dirty bit on the gpte? >>> >> >> Actually, we get guest pte access after mark the dirty bit: >> >> if (write_fault && unlikely(!is_dirty_gpte(pte))) { >> int ret; >> >> trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte)); >> ret = FNAME(cmpxchg_gpte)(vcpu, mmu, ptep_user, index, >> pte, pte|PT_DIRTY_MASK); >> if (unlikely(ret < 0)) { >> present = false; >> goto error; >> } else if (ret) >> goto walk; >> >> mark_page_dirty(vcpu->kvm, table_gfn); >> pte |= PT_DIRTY_MASK; >> walker->ptes[walker->level - 1] = pte; >> } >> >> pte_access = pt_access & FNAME(gpte_access)(vcpu, pte, true); >> >> So, i think it works well :-) > > I think you meant after marking the accessed bit. The dirty bit is set > just before returning. > In this patch, i moved getting pte_access to the behind of setting dirty bit set