Re: [PATCH 4/4] KVM: MMU: Split out the main body of walk_addr_generic()

[Avi Kivity <avi@redhat.com>]

On 06/09/2011 05:18 PM, Ingo Molnar wrote:
> * Takuya Yoshikawa<takuya.yoshikawa@gmail.com>  wrote:
>
> >  +/*
> >  + * do_walk() returns one of these.
> >  + *
> >  + * WALK_NEXT:		Continue the walk loop.
> >  + * WALK_DONE:		Break from the walk loop.
> >  + * WALK_RETRY:		Retry walk.
> >  + * WALK_NOT_PRESENT:		Set PFERR_PRESENT_MASK and goto error.
> >  + * WALK_RSVD_FAULT:		Set PFERR_RSVD_MASK and goto error.
> >  + * WALK_ERROR:		Goto error.
> >  + * WALK_ABORT:		Return immediately.
>
> hm, this iterator turned out to be more complex than i thought it
> would become. Avi, are you still happy with that?

No - a lot of code is spent just communicating between the two 
functions.  Best to leave it in a single function.

> >  +	if ((walker->level == PT_PAGE_TABLE_LEVEL) ||
> >  +	    ((walker->level == PT_DIRECTORY_LEVEL)&&  is_large_pte(*pte)&&
> >  +	     (PTTYPE == 64 || is_pse(vcpu))) ||
> >  +	    ((walker->level == PT_PDPE_LEVEL)&&  is_large_pte(*pte)&&
> >  +	     (mmu->root_level == PT64_ROOT_LEVEL))) {
>
> This condition wants to move into a is-pte-large inline function.

Better, is_last_gpte().  We already have an is_last_spte().

-- 
error compiling committee.c: too many arguments to function