Re: [PATCH 3/3] KVM: MMU: Use helpers to clean up walk_addr_generic()

[Avi Kivity <avi@redhat.com>]

On 06/14/2011 08:03 PM, Takuya Yoshikawa wrote:
> From: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp>
>
> Introduce two new helpers: set_accessed_bit() and is_last_gpte().
>
> These names were suggested by Ingo and Avi.
>
> Cc: Ingo Molnar<mingo@elte.hu>
> Signed-off-by: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp>
> ---
>   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