Re[4]: [2/2] powerpc: support for 256K pages on PPC 44x

From: Yuri Tikhonov <yur@emcraft.com>
To: Milton Miller <miltonm@bga.com>
Cc: dzu@denx.de, Ilya Yanok <yanok@emcraft.com>,
	linux-ppc <linuxppc-dev@ozlabs.org>,
	Vladimir Panfilov <pvr@emcraft.com>, Wolfgang Denk <wd@denx.de>
Subject: Re[4]: [2/2] powerpc: support for 256K pages on PPC 44x
Date: Thu, 27 Nov 2008 03:30:01 +0300	[thread overview]
Message-ID: <364074843.20081127033001@emcraft.com> (raw)
In-Reply-To: <983a8cd0a573e97059371081e61663cc@bga.com>

=0D=0A Hello Milton,

On Friday, November 14, 2008 you wrote:

> On Nov 13, 2008, at 10:32 PM, Yuri Tikhonov wrote:
>> On Tuesday, November 11, 2008 Milton Miller wrote:
>>>>>>  #ifdef CONFIG_PTE_64BIT
>>>>>>  typedef unsigned long long pte_basic_t;
>>>>>> +#ifdef CONFIG_PPC_256K_PAGES
>>>>>> +#define PTE_SHIFT       (PAGE_SHIFT - 7)
>>>>>
>>>>> This seems to be missing the comment on how many ptes are actually=20
>>>>> in
>>>>> the page that are in the other if and else cases.
>>>>
>>>> Ok. I'll fix this. Actually it's another hack: we don't use full page
>>>> for PTE table because we need to reserve something for PGD
>>
>>> I don't understand "we need to reserve something for PGD".   Do you
>>> mean that you would not require a second page for the PGD because the
>>> full pagetable could fit in one page?
> ...
>>> That does imply you want to allocate the pte page from a slab instead
>>> of pgalloc.  Is that covered?
>>
>>  Well, in case of 256K PAGE_SIZE we do not need the PGD level indeed
>> (18 bits are used for offset, and remaining 14 bits are for PTE index
>> inside the PTE table). Even the full 256K PTE page isn't necessary to
>> cover the full range: only half of it would be enough (with 14 bits we
>> can address only 16K PTEs).
>>
>>  But the head_44x.S code is essentially based on the assumption of
>> 2-level page addressing. Also, I may guess that eliminating of the
>> PGD level won't be as easy as just a re-implementation of the TLB-miss
>> handlers in head_44x.S. So, the current approach for 256K-pages
>> support was just a compromise between the required for the project
>> functionality, and the effort necessary to achieve it.

> So are you allocating the < PAGE_SIZE levels from slabs (either kmalloc
> or dedicated) instead of allocating pages?   Or are you wasting the=20
> extra space?

 Wasting the extra space has a place here.

> At a very minimum you need to comment this in the code.  If I were=20
> maintiner I would say not wasting large fractions of pages when the=20
> page size is 256k would be my merge requirement.  As I said, I'm fine=20
> with keeping the page table two levels, but the tradeoff needs to be=20
> documented.

 Agree, we'll document this fact, and re-submit the patch.

 Regards, Yuri

 --
 Yuri Tikhonov, Senior Software Engineer
 Emcraft Systems, www.emcraft.com