From: Yuri Tikhonov <yur@emcraft•com>
To: "prodyut hazarika" <prodyuth@gmail•com>
Cc: linuxppc-dev@ozlabs•org, dzu@denx•de, wd@denx•de,
Ilya Yanok <yanok@emcraft•com>
Subject: Re[2]: [PATCH] powerpc: add support for PAGE_SIZEs greater than 4KB for
Date: Thu, 11 Sep 2008 22:15:07 +0400 [thread overview]
Message-ID: <1715327082.20080911221507@emcraft.com> (raw)
In-Reply-To: <49c0ff980809110957h4b7b85d4ie39dbb27d1edbe39@mail.gmail.com>
=0D=0AHello,
On Thursday, September 11, 2008 you wrote:
> I was planning to post a similar patch. Good that you already posted
> it :-) I will try to finish off similar patch for 40x processors.
>>
>> +choice
>> + prompt "Page size"
>> + depends on 44x && PPC32
>> + default PPC32_4K_PAGES
>> + help
>> + The PAGE_SIZE definition. Increasing the page size may
>> + improve the system performance in some dedicated cases.
>> + If unsure, set it to 4 KB.
>> +
> You should mention an example of dedicated cases (eg. RAID).
ACK.
> I think this help should mention that for page size 256KB, you will
> need to have a special version of binutils, since the ELF standard
> mentions page sizes only upto 64KB.
Right. We use ELDK-4.2 for compiling applications to be run on 256K
PAGE_SIZE kernel. This toolchain includes necessary changes for
ELF_MAXPAGESIZE in binutils/bfd/elf32-ppc.c.
>> -#ifdef CONFIG_PPC_64K_PAGES
>> +#if defined(CONFIG_PPC32_256K_PAGES)
>> +#define PAGE_SHIFT 18
>> +#elif defined(CONFIG_PPC32_64K_PAGES) || defined(CONFIG_PPC_64K_PAGES)
>> #define PAGE_SHIFT 16
>> +#elif defined(CONFIG_PPC32_16K_PAGES)
>> +#define PAGE_SHIFT 14
>> #else
>> #define PAGE_SHIFT 12
>> #endif
> Why should the new defines be inside CONFIG_PPC_64K_PAGES? The
> definition CONFIG_PPC_64K_PAGES is repeated.
We decided to introduce new CONFIG_PPC32_64K_PAGES option to
distinguish using 64K pages on PPC32 and PPC64, so PAGE_SHIFT will be
defined as 16 when the CONFIG_PPC_64K_PAGES option is set on some PPC64
platform, and as 16 when the CONFIG_PPC32_64K_PAGES option is set on
some ppc44x PPC32 platform.
> Shouldn't these defines be like this:
> #if defined(CONFIG_PPC32_256K_PAGES)
> #define PAGE_SHIFT 18
> #elif defined(CONFIG_PPC32_64K_PAGES) || defined(CONFIG_PPC_64K_PAGES)
> #define PAGE_SHIFT 16
> #elif defined(CONFIG_PPC32_16K_PAGES)
> #define PAGE_SHIFT 14
> #else
> #define PAGE_SHIFT 12
> #endif
Admittedly, I don't see the difference between your version and
Ilya's one. Am I missing something ?
>> +#elif (PAGE_SHIFT =3D=3D 14)
>> +/*
>> + * PAGE_SIZE 16K
>> + * PAGE_SHIFT 14
>> + * PTE_SHIFT 11
>> + * PMD_SHIFT 25
>> + */
>> +#define PPC44x_TLBE_SIZE PPC44x_TLB_16K
>> +#define PPC44x_PGD_OFF_SH 9 /*(32 - PMD_SHIFT + 2)*/
>> +#define PPC44x_PGD_OFF_M1 23 /*(PMD_SHIFT - 2)*/
>> +#define PPC44x_PTE_ADD_SH 21 /*32 - PMD_SHIFT + PTE_SHIFT + 3*/
>> +#define PPC44x_PTE_ADD_M1 18 /*32 - 3 - PTE_SHIFT*/
>> +#define PPC44x_RPN_M2 17 /*31 - PAGE_SHIFT*/
> Please change PPC44x_PGD_OFF_SH to PPC44x_PGD_OFF_SHIFT. SH sounds
> very confusing. I don't like the MI and M2 names too. Change
> PPC44x_RPN_M2 to PPC44x_RPN_MASK. Change M1 to MASK in
> PPC44x_PGD_OFF_M1 and PPC44x_PTE_ADD_M1 .
> Is there no way a define like
> #define PPC44x_PGD_OFF_SH (32 - PMD_SHIFT + 2)
> be used in assembly file. If yes, we can avoid repeating the defines.
> I think these 44x specific defines should go to asm/mmu-44x.h since I
> am planning to post a patch for 40x. For those processors, the defines
> below will changes as:
> #define PPC44x_PTE_ADD_SH (32 - PMD_SHIFT + PTE_SHIFT + 2)
> #define PPC44x_PTE_ADD_M1 (32 - 2 - PTE_SHIFT)
> Since these defines are not generic, they should be put in the mmu
> specific header file rather than adding a new header file. When 40x
> processors are supported, the corresponding defines can go to
> include/asm/mmu-40x.h
>> +#elif (PAGE_SHIFT =3D=3D 18)
>> +/*
>> + * PAGE_SIZE 256K
>> + * PAGE_SHIFT 18
>> + * PTE_SHIFT 11
>> + * PMD_SHIFT 29
>> + */
>> +#define PPC44x_TLBE_SIZE PPC44x_TLB_256K
>> +#define PPC44x_PGD_OFF_SH 5 /*(32 - PMD_SHIFT + 2)*/
>> +#define PPC44x_PGD_OFF_M1 27 /*(PMD_SHIFT - 2)*/
>> +#define PPC44x_PTE_ADD_SH 17 /*32 - PMD_SHIFT + PTE_SHIFT + 3*/
>> +#define PPC44x_PTE_ADD_M1 18 /*32 - 3 - PTE_SHIFT*/
>> +#define PPC44x_RPN_M2 13 /*31 - PAGE_SHIFT*/
> For 256KB page size, I cannot understand why PTE_SHIFT is 11. Since
> each PTE entry is 8 byte, PTE_SHIFT should have been 15. But then
> there would be no bits in the Effective address for the 1st level
> PGDIR offset. On what basis PTE_SHIFT of 11 is chosen? This overflow
> problem happens only for 256KB page size.
We should use smaller PTE area in address to free some bits for PGDIR
part. I guess the only impact this approach has is ineffective usage
of memory pages allocated for PTE tables, since having PTE_SHIFT of 11
we use only 1/16 of pages with PTEs.
Regards, Yuri
--
Yuri Tikhonov, Senior Software Engineer
Emcraft Systems, www.emcraft.com
next prev parent reply other threads:[~2008-09-11 18:36 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-10 21:53 [RFC PATCH] Support for big page sizes on 44x Ilya Yanok
2008-09-10 21:53 ` [PATCH] powerpc: add support for PAGE_SIZEs greater than 4KB for Ilya Yanok
2008-09-11 16:57 ` prodyut hazarika
2008-09-11 18:15 ` Yuri Tikhonov [this message]
2008-09-11 20:09 ` Josh Boyer
2008-09-11 23:38 ` Ilya Yanok
2008-09-12 0:47 ` Josh Boyer
2008-09-11 18:28 ` Ilya Yanok
2008-09-11 18:38 ` prodyut hazarika
2008-09-11 22:44 ` Ilya Yanok
2008-09-11 23:52 ` Re[2]: " Yuri Tikhonov
2008-09-11 18:53 ` prodyut hazarika
2008-09-11 21:51 ` Ilya Yanok
2008-09-13 17:49 ` Benjamin Herrenschmidt
2008-09-13 23:37 ` Josh Boyer
2008-09-11 22:37 ` prodyut hazarika
2008-09-11 23:20 ` Re[2]: " Yuri Tikhonov
2008-09-12 3:48 ` David Gibson
2008-09-13 17:46 ` Benjamin Herrenschmidt
2008-09-26 23:43 ` Ilya Yanok
2008-09-26 23:35 ` Ilya Yanok
2008-09-29 2:58 ` David Gibson
2008-09-10 21:53 ` [PATCH] mm: fix ENTRIES_PER_PAGEPAGE overflow with 256KB pages Ilya Yanok
2008-09-12 3:50 ` David Gibson
2008-09-12 5:29 ` prodyut hazarika
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1715327082.20080911221507@emcraft.com \
--to=yur@emcraft$(echo .)com \
--cc=dzu@denx$(echo .)de \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=prodyuth@gmail$(echo .)com \
--cc=wd@denx$(echo .)de \
--cc=yanok@emcraft$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox