Re: [PATCH] KVM: PPC: E500: Support hugetlbfs

[Scott Wood <scottwood@freescale.com>]

On 09/19/2011 06:35 PM, Alexander Graf wrote:
> +			/*
> +			 * e500 doesn't implement the lowest tsize bit,
> +			 * or 1K pages.
> +			 */
> +			tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);
> +
> +			/* take the smallest page size that satisfies host and
> +			   guest mapping */

kernel comment style...

I don't personally care much, but this leaves a checkpatch complaint for
the next person to modify the comment.  Such as to make it say "take the
largest page size that...". :-)

> +			asm (PPC_CNTLZL "%0,%1" : "=r" (lz) : "r" (psize));
> +			tsize = min(21 - lz, tsize);

No need to open-code the cntlz and subtract-from-21:

	tsize = min(ilog2(psize) - 10, tsize);

	/*
	 * e500 doesn't implement the lowest tsize bit,
	 * or 1K pages.
	 */
	tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);

There's still an open-coded subtraction of 10, but that relates more
straightforwardly to the definition of tsize (and could be factored out
into a size-to-tsize function).

>  		}
>  
>  		up_read(&current->mm->mmap_sem);
>  	}
>  
>  	if (likely(!pfnmap)) {
> +		unsigned long tsize_pages = 1 << (tsize - 2);

1 << (tsize + 10 - PAGE_SHIFT);

>  		pfn = gfn_to_pfn_memslot(vcpu_e500->vcpu.kvm, slot, gfn);
> +		pfn &= ~(tsize_pages - 1);
> +		gvaddr &= ~((tsize_pages << PAGE_SHIFT) - 1);
>  		if (is_error_pfn(pfn)) {

Won't the masking of pfn affect is_error_pfn()?

-Scott