On 20.09.2011, at 19:54, Scott Wood wrote:
> 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...". :-)
Ahem :). Changed.
>
>> + 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).
Yeah, no need to micro-optimized those few bits. The reason I used the asm statement was that I copied the hugetlbfs code which does it that way :).
>
>> }
>>
>> up_read(¤t->mm->mmap_sem);
>> }
>>
>> if (likely(!pfnmap)) {
>> + unsigned long tsize_pages = 1 << (tsize - 2);
>
> 1 << (tsize + 10 - PAGE_SHIFT);
Are we getting variable page sizes anytime soon? Will change it nevertheless, just curious :).
>
>> 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()?
Yes, it would. Good catch!
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
