Skip to contentSkip to conversation

FRV: Change PML4 -> PUD

PATCH In review David Howells 5 participants 7 messages Jan 4, 2005 – Jan 5, 2005

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 FRV: Change PML4 -> PUD

The attached patch changes the PML4 bits of the FRV arch to the new PUD way.

---
warthog>diffstat frv-pml4-pud-2610mm1.diff

 arch/frv/kernel/gdb-stub.c    |    4 -
 arch/frv/mm/dma-alloc.c       |    6 +
 arch/frv/mm/fault.c           |   23 ++++---
 arch/frv/mm/init.c            |    4 -
 arch/frv/mm/mmu-context.c     |    4 -
 arch/frv/mm/pgalloc.c         |    2 
 include/asm-frv/mmu_context.h |    6 -
 include/asm-frv/page.h        |   13 ++--
 include/asm-frv/pgalloc.h     |    4 -
 include/asm-frv/pgtable.h     |  129 +++++++++++++++++++++++++++++++-----------
 10 files changed, 134 insertions(+), 61 deletions(-)
arch/frv/kernel/gdb-stub.c renamed from warthog/kernels/linux-2.6.10-mm1/arch/frv/kernel/gdb-stub.c +3 -1
diff -uNrp /warthog/kernels/linux-2.6.10-mm1/arch/frv/kernel/gdb-stub.c linux-2.6.10-mm1-frv/arch/frv/kernel/gdb-stub.c
@@ -449,12 +449,14 @@ static unsigned long __saved_dampr, __sa
 static inline unsigned long gdbstub_virt_to_pte(unsigned long vaddr)
 {
 	pgd_t *pgd;
+	pud_t *pud;
 	pmd_t *pmd;
 	pte_t *pte;
 	unsigned long val, dampr5;
 
 	pgd = (pgd_t *) __get_DAMLR(3) + pgd_index(vaddr);
-	pmd = pmd_offset(pgd, vaddr);
+	pud = pud_offset(pgd, vaddr);
+	pmd = pmd_offset(pud, vaddr);
 
 	if (pmd_bad(*pmd) || !pmd_present(*pmd))
 		return 0;
arch/frv/mm/dma-alloc.c renamed from warthog/kernels/linux-2.6.10-mm1/arch/frv/mm/dma-alloc.c +4 -2
diff -uNrp /warthog/kernels/linux-2.6.10-mm1/arch/frv/mm/dma-alloc.c linux-2.6.10-mm1-frv/arch/frv/mm/dma-alloc.c
@@ -50,6 +50,7 @@
 static int map_page(unsigned long va, unsigned long pa, pgprot_t prot)
 {
 	pgd_t *pge;
+	pud_t *pue;
 	pmd_t *pme;
 	pte_t *pte;
 	int err = -ENOMEM;
@@ -57,8 +58,9 @@ static int map_page(unsigned long va, un
 	spin_lock(&init_mm.page_table_lock);
 
 	/* Use upper 10 bits of VA to index the first level map */
-	pge = pml4_pgd_offset_k(pml4_offset_k(va), va);
-	pme = pmd_offset(pge, va);
+	pge = pgd_offset_k(va);
+	pue = pud_offset(pge, va);
+	pme = pmd_offset(pue, va);
 
 	/* Use middle 10 bits of VA to index the second-level map */
 	pte = pte_alloc_kernel(&init_mm, pme, va);
arch/frv/mm/fault.c renamed from warthog/kernels/linux-2.6.10-mm1/arch/frv/mm/fault.c +15 -8
diff -uNrp /warthog/kernels/linux-2.6.10-mm1/arch/frv/mm/fault.c linux-2.6.10-mm1-frv/arch/frv/mm/fault.c
@@ -36,8 +36,8 @@ asmlinkage void do_page_fault(int datamm
 	struct mm_struct *mm;
 	unsigned long _pme, lrai, lrad, fixup;
 	siginfo_t info;
-	pml4_t *pml4e;
 	pgd_t *pge;
+	pud_t *pue;
 	pte_t *pte;
 	int write;
 
@@ -230,9 +230,9 @@ asmlinkage void do_page_fault(int datamm
 
 	__break_hijack_kernel_event();
 
-	pml4e = pml4_offset(current->mm, ear0);
-	pge = pml4_pgd_offset(pml4e, ear0);
-	_pme = pge->pge[0].ste[0];
+	pge = pgd_offset(current->mm, ear0);
+	pue = pud_offset(pge, ear0);
+	_pme = pue->pue[0].ste[0];
 
 	printk(KERN_ALERT "  PGE : %8p { PME %08lx }\n", pge, _pme);
 
@@ -298,21 +298,28 @@ asmlinkage void do_page_fault(int datamm
 		 */
 		int index = pgd_index(ear0);
 		pgd_t *pgd, *pgd_k;
+		pud_t *pud, *pud_k;
 		pmd_t *pmd, *pmd_k;
 		pte_t *pte_k;
 
 		pgd = (pgd_t *) __get_TTBR();
 		pgd = (pgd_t *)__va(pgd) + index;
-		pgd_k = ((pgd_t *)(init_mm.pml4)) + index;
+		pgd_k = ((pgd_t *)(init_mm.pgd)) + index;
 
 		if (!pgd_present(*pgd_k))
 			goto no_context;
-		set_pgd(pgd, *pgd_k);
+		//set_pgd(pgd, *pgd_k); /////// gcc ICE's on this line
 
-		pmd = pmd_offset(pgd, ear0);
-		pmd_k = pmd_offset(pgd_k, ear0);
+		pud_k = pud_offset(pgd_k, ear0);
+		if (!pud_present(*pud_k))
+			goto no_context;
+
+		pmd_k = pmd_offset(pud_k, ear0);
 		if (!pmd_present(*pmd_k))
 			goto no_context;
+
+		pud = pud_offset(pgd, ear0);
+		pmd = pmd_offset(pud, ear0);
 		set_pmd(pmd, *pmd_k);
 
 		pte_k = pte_offset_kernel(pmd_k, ear0);
arch/frv/mm/init.c renamed from warthog/kernels/linux-2.6.10-mm1/arch/frv/mm/init.c +3 -1
diff -uNrp /warthog/kernels/linux-2.6.10-mm1/arch/frv/mm/init.c linux-2.6.10-mm1-frv/arch/frv/mm/init.c
@@ -111,6 +111,7 @@ void __init paging_init(void)
 #if CONFIG_HIGHMEM
 	if (num_physpages - num_mappedpages) {
 		pgd_t *pge;
+		pud_t *pue;
 		pmd_t *pme;
 
 		pkmap_page_table = alloc_bootmem_pages(PAGE_SIZE);
@@ -118,7 +119,8 @@ void __init paging_init(void)
 		memset(pkmap_page_table, 0, PAGE_SIZE);
 
 		pge = swapper_pg_dir + pgd_index_k(PKMAP_BASE);
-		pme = pmd_offset(pge, PKMAP_BASE);
+		pue = pud_offset(pge, PKMAP_BASE);
+		pme = pmd_offset(pue, PKMAP_BASE);
 		__set_pmd(pme, virt_to_phys(pkmap_page_table) | _PAGE_TABLE);
 	}
 #endif
arch/frv/mm/mmu-context.c renamed from warthog/kernels/linux-2.6.10-mm1/arch/frv/mm/mmu-context.c +1 -3
diff -uNrp /warthog/kernels/linux-2.6.10-mm1/arch/frv/mm/mmu-context.c linux-2.6.10-mm1-frv/arch/frv/mm/mmu-context.c
@@ -87,12 +87,10 @@ static unsigned get_cxn(mm_context_t *ct
  * restore the current TLB miss handler mapped page tables into the MMU context and set up a
  * mapping for the page directory
  */
-void change_mm_context(mm_context_t *old, mm_context_t *ctx, pml4_t *pml4)
+void change_mm_context(mm_context_t *old, mm_context_t *ctx, pgd_t *pgd)
 {
 	unsigned long _pgd;
-	pgd_t *pgd;
 
-	pgd = pml4_pgd_offset(pml4, 0);
 	_pgd = virt_to_phys(pgd);
 
 	/* save the state of the outgoing MMU context */
arch/frv/mm/pgalloc.c renamed from warthog/kernels/linux-2.6.10-mm1/arch/frv/mm/pgalloc.c +1 -1
diff -uNrp /warthog/kernels/linux-2.6.10-mm1/arch/frv/mm/pgalloc.c linux-2.6.10-mm1-frv/arch/frv/mm/pgalloc.c
@@ -129,7 +129,7 @@ void pgd_dtor(void *pgd, kmem_cache_t *c
 	spin_unlock_irqrestore(&pgd_lock, flags);
 }
 
-pgd_t *__pgd_alloc(struct mm_struct *mm, pml4_t *pml4, unsigned long address)
+pgd_t *pgd_alloc(struct mm_struct *mm)
 {
 	pgd_t *pgd;
 
include/asm-frv/mmu_context.h renamed from warthog/kernels/linux-2.6.10-mm1/include/asm-frv/mmu_context.h +3 -3
diff -uNrp /warthog/kernels/linux-2.6.10-mm1/include/asm-frv/mmu_context.h linux-2.6.10-mm1-frv/include/asm-frv/mmu_context.h
@@ -23,7 +23,7 @@ static inline void enter_lazy_tlb(struct
 
 #ifdef CONFIG_MMU
 extern int init_new_context(struct task_struct *tsk, struct mm_struct *mm);
-extern void change_mm_context(mm_context_t *old, mm_context_t *ctx, pml4_t *_pml4);
+extern void change_mm_context(mm_context_t *old, mm_context_t *ctx, pgd_t *_pgd);
 extern void destroy_context(struct mm_struct *mm);
 
 #else
@@ -35,12 +35,12 @@ extern void destroy_context(struct mm_st
 #define switch_mm(prev, next, tsk)						\
 do {										\
 	if (prev != next)							\
-		change_mm_context(&prev->context, &next->context, next->pml4);	\
+		change_mm_context(&prev->context, &next->context, next->pgd);	\
 } while(0)
 
 #define activate_mm(prev, next)						\
 do {									\
-	change_mm_context(&prev->context, &next->context, next->pml4);	\
+	change_mm_context(&prev->context, &next->context, next->pgd);	\
 } while(0)
 
 #define deactivate_mm(tsk, mm)			\
include/asm-frv/page.h renamed from warthog/kernels/linux-2.6.10-mm1/include/asm-frv/page.h +7 -6
diff -uNrp /warthog/kernels/linux-2.6.10-mm1/include/asm-frv/page.h linux-2.6.10-mm1-frv/include/asm-frv/page.h
@@ -23,18 +23,21 @@
 /*
  * These are used to make use of C type-checking..
  */
-typedef struct { unsigned long pte;	} pte_t;
-typedef struct { unsigned long ste[64];	} pmd_t;
-typedef struct { pmd_t         pge[1];	} pgd_t;
-typedef struct { unsigned long pgprot;	} pgprot_t;
+typedef struct { unsigned long	pte;	} pte_t;
+typedef struct { unsigned long	ste[64];} pmd_t;
+typedef struct { pmd_t		pue[1]; } pud_t;
+typedef struct { pud_t		pge[1];	} pgd_t;
+typedef struct { unsigned long	pgprot;	} pgprot_t;
 
 #define pte_val(x)	((x).pte)
 #define pmd_val(x)	((x).ste[0])
+#define pud_val(x)	((x).pue[0])
 #define pgd_val(x)	((x).pge[0])
 #define pgprot_val(x)	((x).pgprot)
 
 #define __pte(x)	((pte_t) { (x) } )
 #define __pmd(x)	((pmd_t) { (x) } )
+#define __pud(x)	((pud_t) { (x) } )
 #define __pgd(x)	((pgd_t) { (x) } )
 #define __pgprot(x)	((pgprot_t) { (x) } )
 #define PTE_MASK	PAGE_MASK
@@ -91,8 +94,6 @@ extern unsigned long max_pfn;
 		 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
 #endif
 
-#include <asm-generic/nopml4-page.h>
-
 #endif /* __ASSEMBLY__ */
 
 #endif /* __KERNEL__ */
include/asm-frv/pgalloc.h renamed from warthog/kernels/linux-2.6.10-mm1/include/asm-frv/pgalloc.h +1 -3
diff -uNrp /warthog/kernels/linux-2.6.10-mm1/include/asm-frv/pgalloc.h linux-2.6.10-mm1-frv/include/asm-frv/pgalloc.h
@@ -31,6 +31,7 @@ do {										\
  * Allocate and free page tables.
  */
 
+extern pgd_t *pgd_alloc(struct mm_struct *);
 extern void pgd_free(pgd_t *);
 
 extern pte_t *pte_alloc_one_kernel(struct mm_struct *, unsigned long);
@@ -57,9 +58,6 @@ static inline void pte_free(struct page 
 #define pmd_alloc_one(mm, addr)		({ BUG(); ((pmd_t *) 2); })
 #define pmd_free(x)			do { } while (0)
 #define __pmd_free_tlb(tlb,x)		do { } while (0)
-#define pgd_populate(mm, pmd, pte)	BUG()
-
-#include <asm-generic/nopml4-pgalloc.h>
 
 #endif /* CONFIG_MMU */
 
include/asm-frv/pgtable.h renamed from warthog/kernels/linux-2.6.10-mm1/include/asm-frv/pgtable.h +96 -33

220 lines not on this page · raw message

#1 lore raw
Quoted 2 lines · David Howells

The attached patch changes the PML4 bits of the FRV arch to the new PUD way.

Looks OK... any reason you aren't using the asm-generic folding headers? (asm-generic/pgtable-nopmd.h or asm-generic/pgtable-nopud.h). I sent some notes to the arch list about getting those working, but apparently it hasn't come though yet.

Of course I do think it is sensible that you just get it working first, before getting too fancy.

Nick

#2 lore raw
Quoted 6 lines · Nick Piggin
Quoted 2 lines · David Howells

The attached patch changes the PML4 bits of the FRV arch to the new PUD way.

Looks OK... any reason you aren't using the asm-generic folding headers? (asm-generic/pgtable-nopmd.h or asm-generic/pgtable-nopud.h).

The PMEs aren't that trivial. Technically, I think I should have one PGE containing 64 PMEs, each of which points to one chunk of a common page table, but I'm not sure the allocation assumptions will work right for that.

The way the page table tree structure is defined on this arch is interesting:
16KB PGD, 256B PTs and 16KB pages. I glue several PTs together into one page, which means that each PME actually contains 64 pointers and is 256B in size.

Trying to use the trivial PUD/PMD support buys me compilation errors about being unable to represent objects. It would be easier if the support wasn't inside out: pmd_t contains a pud_t which contains a pgd_t. This perhaps should be the other way around.

The problems seem to revolve around this:

	#define set_pgd(pgdptr, pgdval) \
		set_pud((pud_t *)(pgdptr), (pud_t) { pgdval })

It's probably possible to rewrite the thing so that the pgd_t contains the 64 pointers, but then set_pmd() ends up setting the pgd_t which seems wrong somehow. Not only that, but the code looks wrong: pmd->pud->pgd?????

It would seem better to me to start from the assumption that PMEs will always contain "pointers" to page tables. What the current method seems to do is that pointers to page tables are installed as high up the tree as possible, and the unnecessary dangly bits (PUDs/PMDs) are looped back on themselves.

Both methods work, I suppose, but it's not well documented; and because it's inside out, it's not immediately obvious. Whoever designed this system should write it up and stick a file in Documentation/ about it.

Quoted 2 lines

I sent some notes to the arch list about getting those working, but apparently it hasn't come though yet.

Sounds like there's a mailing list I should be on, but don't know about.

Quoted 2 lines

Of course I do think it is sensible that you just get it working first, before getting too fancy.

Definitely. The arch makes "fancy" tricky anyway.

David

#3 lore raw
Quoted 12 lines · David Howells
Quoted 7 lines · Nick Piggin
Quoted 2 lines · David Howells

The attached patch changes the PML4 bits of the FRV arch to the new PUD way.

Looks OK... any reason you aren't using the asm-generic folding headers? (asm-generic/pgtable-nopmd.h or asm-generic/pgtable-nopud.h).

Quoted 8 lines

The PMEs aren't that trivial. Technically, I think I should have one PGE containing 64 PMEs, each of which points to one chunk of a common page table, but I'm not sure the allocation assumptions will work right for that.

The way the page table tree structure is defined on this arch is interesting:
16KB PGD, 256B PTs and 16KB pages. I glue several PTs together into one page, which means that each PME actually contains 64 pointers and is 256B in size.

OK, I sort-of understand. I see you've just got frv merged now, so I'll have a better look soon.

Quoted 5 lines

Trying to use the trivial PUD/PMD support buys me compilation errors about being unable to represent objects. It would be easier if the support wasn't inside out: pmd_t contains a pud_t which contains a pgd_t. This perhaps should be the other way around.

That kind of is inside out. But it is the easiest way to get the sizes right and have a unique type at the same time AFAIKS.

Quoted 9 lines

The problems seem to revolve around this:

	#define set_pgd(pgdptr, pgdval) \
		set_pud((pud_t *)(pgdptr), (pud_t) { pgdval })

It's probably possible to rewrite the thing so that the pgd_t contains the 64 pointers, but then set_pmd() ends up setting the pgd_t which seems wrong somehow. Not only that, but the code looks wrong: pmd->pud->pgd?????

This kind of stems from the way page tables get folded... basically I just pulled out the common stuff into a single header file. I'd like to look at making everything nicer, but I think this system might just turn out to be as good as it gets.

But anyway, further changes would require big sweeps of arch code, so may be best left to 2.7, if ever.

Quoted 10 lines

It would seem better to me to start from the assumption that PMEs will always contain "pointers" to page tables. What the current method seems to do is that pointers to page tables are installed as high up the tree as possible, and the unnecessary dangly bits (PUDs/PMDs) are looped back on themselves.

Both methods work, I suppose, but it's not well documented; and because it's inside out, it's not immediately obvious. Whoever designed this system should write it up and stick a file in Documentation/ about it.

Quoted 6 lines
Quoted 2 lines

I sent some notes to the arch list about getting those working, but apparently it hasn't come though yet.

Sounds like there's a mailing list I should be on, but don't know about.

It is very exclusive. Not even a great hacker like myself is allowed there ;) Ask David Miller, I think.

Quoted 9 lines
Quoted 3 lines

Of course I do think it is sensible that you just get it working first, before getting too fancy.

Definitely. The arch makes "fancy" tricky anyway.

David

#4 lore raw

secret cabales, was: Re: [PATCH] FRV: Change PML4 -> PUD

Quoted 6 lines · Nick Piggin
Quoted 2 lines

Sounds like there's a mailing list I should be on, but don't know about.

It is very exclusive. Not even a great hacker like myself is allowed there ;)
Ask David Miller, I think.

Sad enough that's theres some super-l33t cabal, so please everyone Cc lkml on such postings. Thanks.

#5 lore raw

Re: secret cabales, was: Re: [PATCH] FRV: Change PML4 -> PUD

Quoted 2 lines · Christoph Hellwig

Sad enough that's theres some super-l33t cabal

There's no particular reason for the l33tness of linux-arch@vger - Dave set it up as some alias thingy and restricts the visibility so that he doesn't need to admin it for spam. IOW: letting the email address leak onto other mailing lists will cause the spammers to get a hold of it.

I do think it would be better if we had a regular old mailing list for discussing cross-arch issues. If someone wants to volunteer to set up and admin such a list we could probably migrate...

#6 lore raw

Re: secret cabales, was: Re: [PATCH] FRV: Change PML4 -> PUD

Quoted 10 lines · Andrew Morton
Quoted 1 line · Christoph Hellwig

Sad enough that's theres some super-l33t cabal

There's no particular reason for the l33tness of linux-arch@vger - Dave set it up as some alias thingy and restricts the visibility so that he doesn't need to admin it for spam. IOW: letting the email address leak onto other mailing lists will cause the spammers to get a hold of it.

Quoted 4 lines

I do think it would be better if we had a regular old mailing list for discussing cross-arch issues. If someone wants to volunteer to set up and admin such a list we could probably migrate...
-

I can/will make a request that it be hosted at OSDL (with our anti-spam filters :).

Signature · 1 line
~Randy