x86_64: some minor paging cleanups

PATCH Applied Joerg Roedel 3 participants 16 messages Dec 2, 2007 – Dec 5, 2007

Subsystems (2)

x86 x86 MM

lore

Share & tools

lore

Joerg Roedel

author

2007-12-02 13:22 UTC #0 lore raw

cover x86_64: some minor paging cleanups

This small series of patches cleans up the definition and use of the _PAGE_* and __PAGE_KERNEL* permissions for the x86_64 code in the x86 architecture a little bit.

Joerg Roedel

author

2007-12-02 13:22 UTC #1 lore raw

PATCH 1/3 x86_64: define all _PAGE_* in terms of _PAGE_BIT_*

This patch defines the _PAGE_* paging attributes in pgtable_64.h in terms of the former defined _PAGE_BIT_* values.

Signed-off-by Joerg Roedel <[email protected]>

---
 include/asm-x86/pgtable_64.h |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

include/asm-x86/pgtable_64.h +15 ~~-12~~

index 9b0ff47..9d4f11d 100644
@@ -150,19 +150,23 @@ static inline pte_t ptep_get_and_clear_full(struct mm_struct *mm, unsigned long
 #define _PAGE_BIT_ACCESSED	5
 #define _PAGE_BIT_DIRTY		6
 #define _PAGE_BIT_PSE		7	/* 4 MB (or 2MB) page */
+#define _PAGE_BIT_FILE		6
 #define _PAGE_BIT_GLOBAL	8	/* Global TLB entry PPro+ */
 #define _PAGE_BIT_NX           63       /* No execute: only valid after cpuid check */
 
-#define _PAGE_PRESENT	0x001
-#define _PAGE_RW	0x002
-#define _PAGE_USER	0x004
-#define _PAGE_PWT	0x008
-#define _PAGE_PCD	0x010
-#define _PAGE_ACCESSED	0x020
-#define _PAGE_DIRTY	0x040
-#define _PAGE_PSE	0x080	/* 2MB page */
-#define _PAGE_FILE	0x040	/* nonlinear file mapping, saved PTE; unset:swap */
-#define _PAGE_GLOBAL	0x100	/* Global TLB entry */
+#define _PAGE_PRESENT	(_AC(1,UL)<<_PAGE_BIT_PRESENT)
+#define _PAGE_RW	(_AC(1,UL)<<_PAGE_BIT_RW)
+#define _PAGE_USER	(_AC(1,UL)<<_PAGE_BIT_USER)
+#define _PAGE_PWT	(_AC(1,UL)<<_PAGE_BIT_PWT)
+#define _PAGE_PCD	(_AC(1,UL)<<_PAGE_BIT_PCD)
+#define _PAGE_ACCESSED	(_AC(1,UL)<<_PAGE_BIT_ACCESSED)
+#define _PAGE_DIRTY	(_AC(1,UL)<<_PAGE_BIT_DIRTY)
+/* 2MB page */
+#define _PAGE_PSE	(_AC(1,UL)<<_PAGE_BIT_PSE)
+/* nonlinear file mapping, saved PTE; unset:swap */
+#define _PAGE_FILE	(_AC(1,UL)<<_PAGE_BIT_FILE)
+/* Global TLB entry */
+#define _PAGE_GLOBAL	(_AC(1,UL)<<_PAGE_BIT_GLOBAL)
 
 #define _PAGE_PROTNONE	0x080	/* If not present */
 #define _PAGE_NX        (_AC(1,UL)<<_PAGE_BIT_NX)

@@ -248,8 +252,7 @@ static inline unsigned long pmd_bad(pmd_t pmd)
 #define pte_present(x)	(pte_val(x) & (_PAGE_PRESENT | _PAGE_PROTNONE))
 #define pte_clear(mm,addr,xp)	do { set_pte_at(mm, addr, xp, __pte(0)); } while (0)
 
-#define pages_to_mb(x) ((x) >> (20-PAGE_SHIFT))	/* FIXME: is this
-						   right? */
+#define pages_to_mb(x) ((x) >> (20-PAGE_SHIFT))	/* FIXME: is this right? */
 #define pte_page(x)	pfn_to_page(pte_pfn(x))
 #define pte_pfn(x)  ((pte_val(x) & __PHYSICAL_MASK) >> PAGE_SHIFT)

Signature · 1 line

1.5.2.5

Ingo Molnar

Applied

2007-12-04 12:36 UTC #2 lore raw

Re: [PATCH 1/3] x86_64: define all _PAGE_* in terms of _PAGE_BIT_*

Quoted 2 lines · Joerg Roedel

This patch defines the _PAGE_* paging attributes in pgtable_64.h in terms of the former defined _PAGE_BIT_* values.

thanks, applied to x86.git.

Ingo

Ingo Molnar

2007-12-04 12:38 UTC #3 lore raw

Re: [PATCH 1/3] x86_64: define all _PAGE_* in terms of _PAGE_BIT_*

In reply to Joerg Roedel

Quoted 1 line · Joerg Roedel

+#define _PAGE_PRESENT (_AC(1,UL)<<_PAGE_BIT_PRESENT)

please run patches through scripts/checkpatch.pl, it gives:

total: 10 errors, 0 warnings, 42 lines checked

(please send a followup cleanup patch - i picked up your series and this is a minor cleanliness issue)

Ingo

Joerg Roedel

author

2007-12-04 15:11 UTC #4 lore raw

Re: [PATCH 1/3] x86_64: define all _PAGE_* in terms of _PAGE_BIT_*

Quoted 11 lines · Ingo Molnar

Quoted 1 line · Joerg Roedel
+#define _PAGE_PRESENT (_AC(1,UL)<<_PAGE_BIT_PRESENT)
please run patches through scripts/checkpatch.pl, it gives:
total: 10 errors, 0 warnings, 42 lines checked
(please send a followup cleanup patch - i picked up your series and this is a minor cleanliness issue)

Sorry, forgot to check the patches before submission. Will cleanup and resubmit soon.

Joerg

Signature · 6 lines

           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Ingo Molnar

2007-12-04 21:14 UTC #5 lore raw

Re: [PATCH 1/3] x86_64: define all _PAGE_* in terms of _PAGE_BIT_*

Quoted 8 lines · Joerg Roedel

Quoted 7 lines · Ingo Molnar
Quoted 1 line · Joerg Roedel
+#define _PAGE_PRESENT (_AC(1,UL)<<_PAGE_BIT_PRESENT)
please run patches through scripts/checkpatch.pl, it gives:

Quoted 7 lines

Quoted 4 lines
total: 10 errors, 0 warnings, 42 lines checked
(please send a followup cleanup patch - i picked up your series and this is a minor cleanliness issue)
Sorry, forgot to check the patches before submission. Will cleanup and resubmit soon.

please if possible send a delta against x86.git, not a replacement patch. It's easier for the patchflow.

Ingo

Joerg Roedel

author

2007-12-05 10:53 UTC #6 lore raw

Re: [PATCH 1/3] x86_64: define all _PAGE_* in terms of _PAGE_BIT_*

Quoted 13 lines · Ingo Molnar

Quoted 10 lines · Joerg Roedel
Quoted 9 lines · Ingo Molnar
Quoted 1 line · Joerg Roedel
+#define _PAGE_PRESENT (_AC(1,UL)<<_PAGE_BIT_PRESENT)
please run patches through scripts/checkpatch.pl, it gives:
total: 10 errors, 0 warnings, 42 lines checked

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
(please send a followup cleanup patch - i picked up your series and this is a minor cleanliness issue)
Sorry, forgot to check the patches before submission. Will cleanup and resubmit soon.
please if possible send a delta against x86.git, not a replacement patch. It's easier for the patchflow.

Not sure I understand you right here. Do you mean a patch on top of my previous patches?

Signature · 6 lines

           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Ingo Molnar

2007-12-05 11:28 UTC #7 lore raw

Re: [PATCH 1/3] x86_64: define all _PAGE_* in terms of _PAGE_BIT_*

Quoted 7 lines · Joerg Roedel

Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
please run patches through scripts/checkpatch.pl, it gives:
total: 10 errors, 0 warnings, 42 lines checked
(please send a followup cleanup patch - i picked up your series and this is a minor cleanliness issue)

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
Sorry, forgot to check the patches before submission. Will cleanup and resubmit soon.
please if possible send a delta against x86.git, not a replacement patch. It's easier for the patchflow.
Not sure I understand you right here. Do you mean a patch on top of my previous patches?

yeah - that's easier to pick up for me (because i've already got your series applied), and it's also easier for you as well. It also makes sense to generally split style fixes from substantial patches. (in the cases where there are pre-existing style problems in the code)

Ingo

Joerg Roedel

author

2007-12-05 11:42 UTC #8 lore raw

Re: [PATCH 1/3] x86_64: define all _PAGE_* in terms of _PAGE_BIT_*

Quoted 16 lines · Ingo Molnar

Quoted 13 lines · Joerg Roedel
Quoted 12 lines
Quoted 9 lines
Quoted 6 lines
please run patches through scripts/checkpatch.pl, it gives:
total: 10 errors, 0 warnings, 42 lines checked
(please send a followup cleanup patch - i picked up your series and this is a minor cleanliness issue)
Sorry, forgot to check the patches before submission. Will cleanup and resubmit soon.
please if possible send a delta against x86.git, not a replacement patch. It's easier for the patchflow.

Quoted 7 lines

Quoted 2 lines
Not sure I understand you right here. Do you mean a patch on top of my previous patches?
yeah - that's easier to pick up for me (because i've already got your series applied), and it's also easier for you as well. It also makes sense to generally split style fixes from substantial patches. (in the cases where there are pre-existing style problems in the code)

Ok, thanks. Will prepare a patch on top of my first patch series.

Signature · 6 lines

           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Ingo Molnar

2007-12-05 12:30 UTC #9 lore raw

Re: [PATCH 1/3] x86_64: define all _PAGE_* in terms of _PAGE_BIT_*

Quoted 9 lines · Joerg Roedel

Quoted 9 lines
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Sorry, forgot to check the patches before submission. Will cleanup and resubmit soon.
please if possible send a delta against x86.git, not a replacement patch. It's easier for the patchflow.
Not sure I understand you right here. Do you mean a patch on top of my previous patches?

Quoted 7 lines

Quoted 5 lines
yeah - that's easier to pick up for me (because i've already got your series applied), and it's also easier for you as well. It also makes sense to generally split style fixes from substantial patches. (in the cases where there are pre-existing style problems in the code)
Ok, thanks. Will prepare a patch on top of my first patch series.

it's all in the mm branch of x86.git:

git-clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git linux-2.6.git cd linux-2.6.git
git-branch x86 # (1)
git-checkout x86
git-pull git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86.git mm

so the easiest is to do patches just against that tree.

NOTE: the x86.git tree is frequently regenerated, so it can not be rolling upgraded the usual git way via typing 'git-pull' - you have to zap your x86 branch and re-pull:

git-checkout master
git-branch -D x86 # this zaps everything!

and repeat from (1) above.

Ingo

Joerg Roedel

author

2007-12-05 14:44 UTC #10 lore raw

PATCH 1/1 x86_64: some whitespace cleanups in paging code

This patch does some whitespace cleanups in the paging code to fix some checkpatch.pl warnings of my formerly merged cleanup patches.

Signed-off-by Joerg Roedel <[email protected]>

---
 arch/x86/mm/init_64.c        |    2 +-
 include/asm-x86/pgtable_64.h |   22 +++++++++++-----------
 2 files changed, 12 insertions(+), 12 deletions(-)

arch/x86/mm/init_64.c +1 -1

index cfb357c..baef0d1 100644
@@ -226,7 +226,7 @@ __meminit void *early_ioremap(unsigned long addr, unsigned long size)
 		vaddr += addr & ~PMD_MASK;
 		addr &= PMD_MASK;
 		for (i = 0; i < pmds; i++, addr += PMD_SIZE)
-			set_pmd(pmd + i,__pmd(addr | __PAGE_KERNEL_LARGE_EXEC));
+			set_pmd(pmd+i, __pmd(addr | __PAGE_KERNEL_LARGE_EXEC));
 		__flush_tlb();
 		return (void *)vaddr;
 	next:

include/asm-x86/pgtable_64.h +11 ~~-11~~

index 9d4f11d..687eaa6 100644
@@ -154,22 +154,22 @@ static inline pte_t ptep_get_and_clear_full(struct mm_struct *mm, unsigned long
 #define _PAGE_BIT_GLOBAL	8	/* Global TLB entry PPro+ */
 #define _PAGE_BIT_NX           63       /* No execute: only valid after cpuid check */
 
-#define _PAGE_PRESENT	(_AC(1,UL)<<_PAGE_BIT_PRESENT)
-#define _PAGE_RW	(_AC(1,UL)<<_PAGE_BIT_RW)
-#define _PAGE_USER	(_AC(1,UL)<<_PAGE_BIT_USER)
-#define _PAGE_PWT	(_AC(1,UL)<<_PAGE_BIT_PWT)
-#define _PAGE_PCD	(_AC(1,UL)<<_PAGE_BIT_PCD)
-#define _PAGE_ACCESSED	(_AC(1,UL)<<_PAGE_BIT_ACCESSED)
-#define _PAGE_DIRTY	(_AC(1,UL)<<_PAGE_BIT_DIRTY)
+#define _PAGE_PRESENT	(_AC(1, UL)<<_PAGE_BIT_PRESENT)
+#define _PAGE_RW	(_AC(1, UL)<<_PAGE_BIT_RW)
+#define _PAGE_USER	(_AC(1, UL)<<_PAGE_BIT_USER)
+#define _PAGE_PWT	(_AC(1, UL)<<_PAGE_BIT_PWT)
+#define _PAGE_PCD	(_AC(1, UL)<<_PAGE_BIT_PCD)
+#define _PAGE_ACCESSED	(_AC(1, UL)<<_PAGE_BIT_ACCESSED)
+#define _PAGE_DIRTY	(_AC(1, UL)<<_PAGE_BIT_DIRTY)
 /* 2MB page */
-#define _PAGE_PSE	(_AC(1,UL)<<_PAGE_BIT_PSE)
+#define _PAGE_PSE	(_AC(1, UL)<<_PAGE_BIT_PSE)
 /* nonlinear file mapping, saved PTE; unset:swap */
-#define _PAGE_FILE	(_AC(1,UL)<<_PAGE_BIT_FILE)
+#define _PAGE_FILE	(_AC(1, UL)<<_PAGE_BIT_FILE)
 /* Global TLB entry */
-#define _PAGE_GLOBAL	(_AC(1,UL)<<_PAGE_BIT_GLOBAL)
+#define _PAGE_GLOBAL	(_AC(1, UL)<<_PAGE_BIT_GLOBAL)
 
 #define _PAGE_PROTNONE	0x080	/* If not present */
-#define _PAGE_NX        (_AC(1,UL)<<_PAGE_BIT_NX)
+#define _PAGE_NX        (_AC(1, UL)<<_PAGE_BIT_NX)
 
 #define _PAGE_TABLE	(_PAGE_PRESENT | _PAGE_RW | _PAGE_USER | _PAGE_ACCESSED | _PAGE_DIRTY)
 #define _KERNPG_TABLE	(_PAGE_PRESENT | _PAGE_RW | _PAGE_ACCESSED | _PAGE_DIRTY)

Signature · 1 line

1.5.2.5

Ingo Molnar

Applied

2007-12-05 14:51 UTC #11 lore raw

Re: [PATCH] x86_64: some whitespace cleanups in paging code

Quoted 2 lines · Joerg Roedel

This patch does some whitespace cleanups in the paging code to fix some checkpatch.pl warnings of my formerly merged cleanup patches.

thanks, applied.

btw., if there's any file you are particularly interested in cleaning up as a whole, you can use the --file option:

  scripts/checkpatch.pl --file include/asm-x86/pgtable_64.h

there's no hard policy on whether cleanups should be done pre-unification or post-unification. I've been using the following pretty sensible 3-step path when doing manual unifications:

- clean up the _32 and _64 file first (this is also easy to check - the resulting .o or vmlinux must not differ with/without the patch)

- remove artificial differences (in a separate patch)

- do the unification (in a third patch)

the general rule, more patches are better than fewer patches. Most of our testing/bisetion infrastructure is per-patch/per-commit, so if some mistake happens (and mistakes happen all the time), the finer grained approach is always easier to sort out.

Ingo

Heiko Carstens

2007-12-05 16:37 UTC #12 lore raw

Re: [PATCH] x86_64: some whitespace cleanups in paging code

In reply to Joerg Roedel

arch/x86/mm/init_64.c __meminit void *early_ioremap(unsigned long addr, unsigned long size) · line 229

Quoted 5 lines · Joerg Roedel

This patch does some whitespace cleanups in the paging code to fix some checkpatch.pl warnings of my formerly merged cleanup patches.
...
arch/x86/mm/init_64.c +1 -1
-			set_pmd(pmd + i,__pmd(addr | __PAGE_KERNEL_LARGE_EXEC));
+			set_pmd(pmd+i, __pmd(addr | __PAGE_KERNEL_LARGE_EXEC));

Did checkpatch enforce you to remove the spaces around '+'? I thought there was/is a common consensus that having spaces around operators is a good thing to have. IMHO it's much more readable...
But anyway, it's not that I want to start a new thread about coding style :)

Joerg Roedel

author

2007-12-05 17:28 UTC #13 lore raw

Re: [PATCH] x86_64: some whitespace cleanups in paging code

Quoted 11 lines · Heiko Carstens

arch/x86/mm/init_64.c __meminit void *early_ioremap(unsigned long addr, unsigned long size) · line 229
Quoted 5 lines · Joerg Roedel
This patch does some whitespace cleanups in the paging code to fix some checkpatch.pl warnings of my formerly merged cleanup patches.
...
arch/x86/mm/init_64.c +1 -1
-			set_pmd(pmd + i,__pmd(addr | __PAGE_KERNEL_LARGE_EXEC));
+			set_pmd(pmd+i, __pmd(addr | __PAGE_KERNEL_LARGE_EXEC));
Did checkpatch enforce you to remove the spaces around '+'? I thought there was/is a common consensus that having spaces around operators is a good thing to have. IMHO it's much more readable...
But anyway, it's not that I want to start a new thread about coding style :)

Checkpatch complained about a missing space after the comma. After I added one it complained about a line longer than 80 characters. But luckily it didn't complain about missing spaces around the plus sign ;)

Signature · 6 lines

           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Joerg Roedel

author

2007-12-02 13:22 UTC #14 lore raw

PATCH 2/3 x86_64: use __PAGE_KERNEL* instead of _KERNPG_TABLE for 2MB PTEs in memory initialization

In reply to Joerg Roedel

This minor cleanup replaces _KERNPG_TABLE with the __PAGE_KERNEL* for 2MB PTEs in the x86_64 memory initialization code. The __PAGE_KERNEL* defines are more appropriate for PTEs.

Signed-off-by Joerg Roedel <[email protected]>

---
 arch/x86/mm/init_64.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

arch/x86/mm/init_64.c +2 -2

index 0f9c8c8..8db7348 100644
@@ -224,7 +224,7 @@ __meminit void *early_ioremap(unsigned long addr, unsigned long size)
 		vaddr += addr & ~PMD_MASK;
 		addr &= PMD_MASK;
 		for (i = 0; i < pmds; i++, addr += PMD_SIZE)
-			set_pmd(pmd + i,__pmd(addr | _KERNPG_TABLE | _PAGE_PSE));
+			set_pmd(pmd + i,__pmd(addr | __PAGE_KERNEL_LARGE_EXEC));
 		__flush_tlb();
 		return (void *)vaddr;
 	next:

@@ -268,7 +268,7 @@ phys_pmd_init(pmd_t *pmd_page, unsigned long address, unsigned long end)
 		if (pmd_val(*pmd))
 			continue;
 
-		entry = _PAGE_NX|_PAGE_PSE|_KERNPG_TABLE|_PAGE_GLOBAL|address;
+		entry = __PAGE_KERNEL_LARGE|_PAGE_GLOBAL|address;
 		entry &= __supported_pte_mask;
 		set_pmd(pmd, __pmd(entry));
 	}

Signature · 1 line

1.5.2.5

Joerg Roedel

author

2007-12-02 13:21 UTC #15 lore raw

PATCH 3/3 x86_64: use __PAGE_KERNEL_EXEC in ioremap_64.c

In reply to Joerg Roedel

This patch replaces the manual permission setup for pages in ioremap_64.c with the pre-defined __PAGE_KERNEL_EXEC value.

Signed-off-by Joerg Roedel <[email protected]>

---
 arch/x86/mm/ioremap_64.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

arch/x86/mm/ioremap_64.c +1 -2

index 6cac90a..9a67fc6 100644
@@ -103,8 +103,7 @@ void __iomem * __ioremap(unsigned long phys_addr, unsigned long size, unsigned l
 	}
 #endif
 
-	pgprot = __pgprot(_PAGE_PRESENT | _PAGE_RW | _PAGE_GLOBAL
-			  | _PAGE_DIRTY | _PAGE_ACCESSED | flags);
+	pgprot = __pgprot(__PAGE_KERNEL_EXEC | _PAGE_GLOBAL | flags);
 	/*
 	 * Mappings have to be page-aligned
 	 */

Signature · 1 line

1.5.2.5