| Message ID | 20200611134914.765827-5-gregory.clement@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: Add support for large kernel page (from 8K to 64K) | expand |
On Thu, Jun 11, 2020 at 3:49 PM Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > > In pte_offset_kernel() the pte_index macro is used. This macro makes > the assumption that the address is aligned to a page size. > > In arm_pte_allocation, the size allocated is the size needed for 512 > entries. Actually this size was calculated to fit in a 4K page. When > using larger page, the size of the table allocated is no more > aligned which end to give a wrong physical address. > > The solution is to round up the allocation to a page size instead of > the exact size of the tables (which is 4KB). It allows to comply with > the assumption of pte_index() but the drawback is a waste of memory > for the early allocation if page size is bigger than 4KB. Have you considered increasing PTRS_PER_PTE instead to fill up a logical page instead? If that doesn't work, can you explain here why not? Arnd
On Fri, Jun 12, 2020 at 10:37:15AM +0200, Arnd Bergmann wrote: > On Thu, Jun 11, 2020 at 3:49 PM Gregory CLEMENT > <gregory.clement@bootlin.com> wrote: > > In pte_offset_kernel() the pte_index macro is used. This macro makes > > the assumption that the address is aligned to a page size. > > > > In arm_pte_allocation, the size allocated is the size needed for 512 > > entries. Actually this size was calculated to fit in a 4K page. When > > using larger page, the size of the table allocated is no more > > aligned which end to give a wrong physical address. > > > > The solution is to round up the allocation to a page size instead of > > the exact size of the tables (which is 4KB). It allows to comply with > > the assumption of pte_index() but the drawback is a waste of memory > > for the early allocation if page size is bigger than 4KB. > > Have you considered increasing PTRS_PER_PTE instead to fill up > a logical page instead? If that doesn't work, can you explain here > why not? From what I remember, increasing the PTRS_PER_PTE also requires changing the pgd_t array to cover them. As a side-effect, {PMD,PGDIR}_SHIFT would need to increase. cpu_v7_set_pte_ext() also needs to take care of the software pte offset (hard-coded at 2048 now). Many years ago I had some patches to get rid of the software pte offset but it wasn't really worth the maintenance hassle (only possible for ARMv6/7). I'm not even sure it's feasible now if we gained more L_PTE_* bits in the meantime.
Arnd Bergmann <arnd@arndb.de> writes: > On Thu, Jun 11, 2020 at 3:49 PM Gregory CLEMENT > <gregory.clement@bootlin.com> wrote: >> >> In pte_offset_kernel() the pte_index macro is used. This macro makes >> the assumption that the address is aligned to a page size. >> >> In arm_pte_allocation, the size allocated is the size needed for 512 >> entries. Actually this size was calculated to fit in a 4K page. When >> using larger page, the size of the table allocated is no more >> aligned which end to give a wrong physical address. >> >> The solution is to round up the allocation to a page size instead of >> the exact size of the tables (which is 4KB). It allows to comply with >> the assumption of pte_index() but the drawback is a waste of memory >> for the early allocation if page size is bigger than 4KB. > > Have you considered increasing PTRS_PER_PTE instead to fill up > a logical page instead? If that doesn't work, can you explain here > why not? Actually for this situation I didn't try to do better but it is only used very early during the boot. Then I'm expecting that the allocation is done though slab so with object at the exact size we need. However you also pointed modifying PTRS_PER_PTE for the overall memory consumption in the cover letter and in this case, it could worth modifying it. Gregory > > Arnd
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c index ec8d0008bfa1..b7fdea7e0cbe 100644 --- a/arch/arm/mm/mmu.c +++ b/arch/arm/mm/mmu.c @@ -715,7 +715,9 @@ static pte_t * __init arm_pte_alloc(pmd_t *pmd, unsigned long addr, void *(*alloc)(unsigned long sz)) { if (pmd_none(*pmd)) { - pte_t *pte = alloc(PTE_HWTABLE_OFF + PTE_HWTABLE_SIZE); + /* The PTE needs to be page to be page aligned */ + pte_t *pte = alloc(round_up(PTE_HWTABLE_OFF + PTE_HWTABLE_SIZE, + PAGE_SIZE)); __pmd_populate(pmd, __pa(pte), prot); } BUG_ON(pmd_bad(*pmd));
In pte_offset_kernel() the pte_index macro is used. This macro makes the assumption that the address is aligned to a page size. In arm_pte_allocation, the size allocated is the size needed for 512 entries. Actually this size was calculated to fit in a 4K page. When using larger page, the size of the table allocated is no more aligned which end to give a wrong physical address. The solution is to round up the allocation to a page size instead of the exact size of the tables (which is 4KB). It allows to comply with the assumption of pte_index() but the drawback is a waste of memory for the early allocation if page size is bigger than 4KB. This is inspired from fa0ca2726ea9 ("DSMP 64K support") and 4ef803e12baf ("mmu: large-page: Added support for multiple kernel page sizes") from https://github.com/MarvellEmbeddedProcessors/linux-marvell.git Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> --- arch/arm/mm/mmu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)