From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 3/3] KVM: MMU: Use helpers to clean up walk_addr_generic() Date: Mon, 20 Jun 2011 15:02:07 +0300 Message-ID: <4DFF36BF.7030304@redhat.com> References: <20110615020003.15722a29.takuya.yoshikawa@gmail.com> <20110615020343.991f0b86.takuya.yoshikawa@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org, yoshikawa.takuya@oss.ntt.co.jp, mingo@elte.hu To: Takuya Yoshikawa Return-path: Received: from mx1.redhat.com ([209.132.183.28]:32957 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751139Ab1FTMCS (ORCPT ); Mon, 20 Jun 2011 08:02:18 -0400 In-Reply-To: <20110615020343.991f0b86.takuya.yoshikawa@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On 06/14/2011 08:03 PM, Takuya Yoshikawa wrote: > From: Takuya Yoshikawa > > Introduce two new helpers: set_accessed_bit() and is_last_gpte(). > > These names were suggested by Ingo and Avi. > > Cc: Ingo Molnar > Signed-off-by: Takuya Yoshikawa > --- > arch/x86/kvm/paging_tmpl.h | 57 ++++++++++++++++++++++++++++++++----------- > 1 files changed, 42 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h > index 92fe275..d655a4b6 100644 > --- a/arch/x86/kvm/paging_tmpl.h > +++ b/arch/x86/kvm/paging_tmpl.h > @@ -113,6 +113,43 @@ static unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, pt_element_t gpte) > return access; > } > > +static int FNAME(set_accessed_bit)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, > + gfn_t table_gfn, unsigned index, > + pt_element_t __user *ptep_user, > + pt_element_t *ptep) > +{ > + int ret; > + > + trace_kvm_mmu_set_accessed_bit(table_gfn, index, sizeof(*ptep)); > + ret = FNAME(cmpxchg_gpte)(vcpu, mmu, ptep_user, index, > + *ptep, *ptep|PT_ACCESSED_MASK); > + if (unlikely(ret)) > + return ret; > + > + mark_page_dirty(vcpu->kvm, table_gfn); > + *ptep |= PT_ACCESSED_MASK; > + > + return 0; > +} I don't think this one is worthwhile, it takes 7 parameters! If there's so much communication between caller and callee, it means they are too heavily tied up. > + > +static bool FNAME(is_last_gpte)(struct guest_walker *walker, > + struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, > + pt_element_t gpte) > +{ > + if (walker->level == PT_PAGE_TABLE_LEVEL) > + return true; > + > + if ((walker->level == PT_DIRECTORY_LEVEL)&& is_large_pte(gpte)&& > + (PTTYPE == 64 || is_pse(vcpu))) > + return true; > + > + if ((walker->level == PT_PDPE_LEVEL)&& is_large_pte(gpte)&& > + (mmu->root_level == PT64_ROOT_LEVEL)) > + return true; > + > + return false; > +} > + This one is much better. -- error compiling committee.c: too many arguments to function