[patch 0/6] Strong Access Ordering page attributes for POWER7

[patch 0/6] Strong Access Ordering page attributes for POWER7

[shaggy]

Andrew,

The first patch in this series hits architecture independent code, but the
rest is contained in the powerpc subtree.  Could you pick up the first
patch into -mm?  I can send the rest of them through the powerpc git tree.
The first patch and the rest of the set are independent and can be merged
in either order.

Changes since I posted on June 10:
- Fixed reversed logic in arch_validate_prot() in include/asm-powerpc/mman.h
- Replace binary & with logical && in arch_validate_prot()
- Got rid of HAVE_ARCH_PROT_BITS

Allow an application to enable Strong Access Ordering on specific pages of
memory on Power 7 hardware. Currently, power has a weaker memory model than
x86. Implementing a stronger memory model allows an emulator to more
efficiently translate x86 code into power code, resulting in faster code
execution.

On Power 7 hardware, storing 0b1110 in the WIMG bits of the hpte enables
strong access ordering mode for the memory page.  This patchset allows a
user to specify which pages are thus enabled by passing a new protection
bit through mmap() and mprotect().  I have tentatively defined this bit,
PROT_SAO, as 0x10.

In order to accomplish this, I had to modify the architecture-independent
code to allow the architecture to deal with additional protection bits.

Thanks,
Shaggy
-- 

[patch 1/6] mm: Allow architectures to define additional protection bits

[shaggy]

This patch allows architectures to define functions to deal with
additional protections bits for mmap() and mprotect().

arch_calc_vm_prot_bits() maps additonal protection bits to vm_flags
arch_vm_get_page_prot() maps additional vm_flags to the vma's vm_page_prot
arch_validate_prot() checks for valid values of the protection bits

Note: vm_get_page_prot() is now pretty ugly.  Suggestions?

Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
---

 include/linux/mman.h |   28 +++++++++++++++++++++++++++-
 mm/mmap.c            |    5 +++--
 mm/mprotect.c        |    2 +-
 3 files changed, 31 insertions(+), 4 deletions(-)

Index: linux-2.6.26-rc5/include/linux/mman.h
===================================================================
--- linux-2.6.26-rc5.orig/include/linux/mman.h
+++ linux-2.6.26-rc5/include/linux/mman.h
@@ -34,6 +34,31 @@ static inline void vm_unacct_memory(long
 }
 
 /*
+ * Allow architectures to handle additional protection bits
+ */
+
+#ifndef arch_calc_vm_prot_bits
+#define arch_calc_vm_prot_bits(prot) 0
+#endif
+
+#ifndef arch_vm_get_page_prot
+#define arch_vm_get_page_prot(vm_flags) __pgprot(0)
+#endif
+
+#ifndef arch_validate_prot
+/*
+ * This is called from mprotect().  PROT_GROWSDOWN and PROT_GROWSUP have
+ * already been masked out.
+ *
+ * Returns true if the prot flags are valid
+ */
+static inline int arch_validate_prot(unsigned long prot)
+{
+	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
+}
+#endif
+
+/*
  * Optimisation macro.  It is equivalent to:
  *      (x & bit1) ? bit2 : 0
  * but this version is faster.
@@ -51,7 +76,8 @@ calc_vm_prot_bits(unsigned long prot)
 {
 	return _calc_vm_trans(prot, PROT_READ,  VM_READ ) |
 	       _calc_vm_trans(prot, PROT_WRITE, VM_WRITE) |
-	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC );
+	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC) |
+	       arch_calc_vm_prot_bits(prot);
 }
 
 /*
Index: linux-2.6.26-rc5/mm/mmap.c
===================================================================
--- linux-2.6.26-rc5.orig/mm/mmap.c
+++ linux-2.6.26-rc5/mm/mmap.c
@@ -72,8 +72,9 @@ pgprot_t protection_map[16] = {
 
 pgprot_t vm_get_page_prot(unsigned long vm_flags)
 {
-	return protection_map[vm_flags &
-				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
+	return __pgprot(pgprot_val(protection_map[vm_flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
+			pgprot_val(arch_vm_get_page_prot(vm_flags)));
 }
 EXPORT_SYMBOL(vm_get_page_prot);
 
Index: linux-2.6.26-rc5/mm/mprotect.c
===================================================================
--- linux-2.6.26-rc5.orig/mm/mprotect.c
+++ linux-2.6.26-rc5/mm/mprotect.c
@@ -239,7 +239,7 @@ sys_mprotect(unsigned long start, size_t
 	end = start + len;
 	if (end <= start)
 		return -ENOMEM;
-	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM))
+	if (!arch_validate_prot(prot))
 		return -EINVAL;
 
 	reqprot = prot;

-- 

[patch 2/6] powerpc: hash_huge_page() should get the WIMG bits from the lpte

[shaggy]

Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
Cc: Jon Tollefson <kniht@linux.vnet.ibm.com>
Cc: Adam Litke <agl@us.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 arch/powerpc/mm/hugetlbpage.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Index: linux-2.6.26-rc5/arch/powerpc/mm/hugetlbpage.c
===================================================================
--- linux-2.6.26-rc5.orig/arch/powerpc/mm/hugetlbpage.c
+++ linux-2.6.26-rc5/arch/powerpc/mm/hugetlbpage.c
@@ -502,9 +502,8 @@ repeat:
 		new_pte = (new_pte & ~_PAGE_HPTEFLAGS) | _PAGE_HASHPTE;
 
 		/* Add in WIMG bits */
-		/* XXX We should store these in the pte */
-		/* --BenH: I think they are ... */
-		rflags |= _PAGE_COHERENT;
+		rflags |= (new_pte & (_PAGE_WRITETHRU | _PAGE_NO_CACHE |
+				      _PAGE_COHERENT | _PAGE_GUARDED));
 
 		/* Insert into the hash table, primary slot */
 		slot = ppc_md.hpte_insert(hpte_group, va, pa, rflags, 0,

-- 

[patch 3/6] powerpc: Define flags for Strong Access Ordering

[shaggy]

This patch defines:

- PROT_SAO, which is passed into mmap() and mprotect() in the prot field
- VM_SAO in vma->vm_flags, and
- _PAGE_SAO, the combination of WIMG bits in the pte that enables strong
access ordering for the page.

NOTE: There doesn't seem to be a precedent for architecture-dependent vm_flags.
It may be better to define VM_SAO somewhere in include/asm-powerpc/.  Since
vm_flags is a long, defining it in the high-order word would help prevent a
collision with any newly added values in architecture-independent code.

Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
---

 include/asm-powerpc/mman.h          |    2 ++
 include/asm-powerpc/pgtable-ppc64.h |    3 +++
 include/linux/mm.h                  |    1 +
 3 files changed, 6 insertions(+)

Index: linux-2.6.26-rc5/include/asm-powerpc/mman.h
===================================================================
--- linux-2.6.26-rc5.orig/include/asm-powerpc/mman.h
+++ linux-2.6.26-rc5/include/asm-powerpc/mman.h
@@ -10,6 +10,8 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#define PROT_SAO	0x10		/* Strong Access Ordering */
+
 #define MAP_RENAME      MAP_ANONYMOUS   /* In SunOS terminology */
 #define MAP_NORESERVE   0x40            /* don't reserve swap pages */
 #define MAP_LOCKED	0x80
Index: linux-2.6.26-rc5/include/asm-powerpc/pgtable-ppc64.h
===================================================================
--- linux-2.6.26-rc5.orig/include/asm-powerpc/pgtable-ppc64.h
+++ linux-2.6.26-rc5/include/asm-powerpc/pgtable-ppc64.h
@@ -94,6 +94,9 @@
 #define _PAGE_HASHPTE	0x0400 /* software: pte has an associated HPTE */
 #define _PAGE_BUSY	0x0800 /* software: PTE & hash are busy */
 
+/* Strong Access Ordering */
+#define _PAGE_SAO	(_PAGE_WRITETHRU | _PAGE_NO_CACHE | _PAGE_COHERENT)
+
 #define _PAGE_BASE	(_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_COHERENT)
 
 #define _PAGE_WRENABLE	(_PAGE_RW | _PAGE_DIRTY)
Index: linux-2.6.26-rc5/include/linux/mm.h
===================================================================
--- linux-2.6.26-rc5.orig/include/linux/mm.h
+++ linux-2.6.26-rc5/include/linux/mm.h
@@ -108,6 +108,7 @@ extern unsigned int kobjsize(const void 
 
 #define VM_CAN_NONLINEAR 0x08000000	/* Has ->fault & does nonlinear pages */
 #define VM_MIXEDMAP	0x10000000	/* Can contain "struct page" and pure PFN pages */
+#define VM_SAO		0x20000000	/* Strong Access Ordering (powerpc) */
 
 #ifndef VM_STACK_DEFAULT_FLAGS		/* arch can override this */
 #define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS

-- 

[patch 4/6] powerpc: Add SAO Feature bit to the cputable

[shaggy]

This patch applies on top of the patches posted today to linuxppc-dev by
Michael Neuling and Joel Schopp.

Signed-off-by: Joel Schopp <jschopp@austin.ibm.com>
Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>

Index: linux-2.6.26-rc5/include/asm-powerpc/cputable.h
===================================================================
--- linux-2.6.26-rc5.orig/include/asm-powerpc/cputable.h
+++ linux-2.6.26-rc5/include/asm-powerpc/cputable.h
@@ -183,6 +183,7 @@ extern void do_feature_fixups(unsigned l
 #define CPU_FTR_1T_SEGMENT		LONG_ASM_CONST(0x0004000000000000)
 #define CPU_FTR_NO_SLBIE_B		LONG_ASM_CONST(0x0008000000000000)
 #define CPU_FTR_VSX			LONG_ASM_CONST(0x0010000000000000)
+#define CPU_FTR_SAO			LONG_ASM_CONST(0x0020000000000000)
 
 #ifndef __ASSEMBLY__
 
@@ -395,7 +396,7 @@ extern void do_feature_fixups(unsigned l
 	    CPU_FTR_MMCRA | CPU_FTR_SMT | \
 	    CPU_FTR_COHERENT_ICACHE | CPU_FTR_LOCKLESS_TLBIE | \
 	    CPU_FTR_PURR | CPU_FTR_SPURR | CPU_FTR_REAL_LE | \
-	    CPU_FTR_DSCR)
+	    CPU_FTR_DSCR | CPU_FTR_SAO)
 #define CPU_FTRS_CELL	(CPU_FTR_USE_TB | \
 	    CPU_FTR_HPTE_TABLE | CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
 	    CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \

-- 

[patch 5/6] powerpc: Add Strong Access Ordering

[shaggy]

Allow an application to enable Strong Access Ordering on specific pages of
memory on Power 7 hardware. Currently, power has a weaker memory model than
x86. Implementing a stronger memory model allows an emulator to more
efficiently translate x86 code into power code, resulting in faster code
execution.

On Power 7 hardware, storing 0b1110 in the WIMG bits of the hpte enables
strong access ordering mode for the memory page.  This patchset allows a
user to specify which pages are thus enabled by passing a new protection
bit through mmap() and mprotect().  I have tentatively defined this bit,
PROT_SAO, as 0x10.

Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
---

 arch/powerpc/kernel/syscalls.c |    3 +++
 include/asm-powerpc/mman.h     |   28 ++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

Index: linux-2.6.26-rc5/arch/powerpc/kernel/syscalls.c
===================================================================
--- linux-2.6.26-rc5.orig/arch/powerpc/kernel/syscalls.c
+++ linux-2.6.26-rc5/arch/powerpc/kernel/syscalls.c
@@ -143,6 +143,9 @@ static inline unsigned long do_mmap2(uns
 	struct file * file = NULL;
 	unsigned long ret = -EINVAL;
 
+	if (!arch_validate_prot(prot))
+		goto out;
+
 	if (shift) {
 		if (off & ((1 << shift) - 1))
 			goto out;
Index: linux-2.6.26-rc5/include/asm-powerpc/mman.h
===================================================================
--- linux-2.6.26-rc5.orig/include/asm-powerpc/mman.h
+++ linux-2.6.26-rc5/include/asm-powerpc/mman.h
@@ -1,7 +1,9 @@
 #ifndef _ASM_POWERPC_MMAN_H
 #define _ASM_POWERPC_MMAN_H
 
+#include <asm/cputable.h>
 #include <asm-generic/mman.h>
+#include <linux/mm.h>
 
 /*
  * This program is free software; you can redistribute it and/or
@@ -26,4 +28,30 @@
 #define MAP_POPULATE	0x8000		/* populate (prefault) pagetables */
 #define MAP_NONBLOCK	0x10000		/* do not block on IO */
 
+/*
+ * This file is included by linux/mman.h, so we can't use cacl_vm_prot_bits()
+ * here.  How important is the optimization?
+ */
+static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot)
+{
+	return (prot & PROT_SAO) ? VM_SAO : 0;
+}
+#define arch_calc_vm_prot_bits(prot) arch_calc_vm_prot_bits(prot)
+
+static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
+{
+	return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : 0;
+}
+#define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
+
+static inline int arch_validate_prot(unsigned long prot)
+{
+	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
+		return 0;
+	if ((prot & PROT_SAO) && !cpu_has_feature(CPU_FTR_SAO))
+		return 0;
+	return 1;
+}
+#define arch_validate_prot(prot) arch_validate_prot(prot)
+
 #endif	/* _ASM_POWERPC_MMAN_H */

-- 

[patch 6/6] powerpc: Dont clear _PAGE_COHERENT when _PAGE_SAO is set

[shaggy]

Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 arch/powerpc/platforms/pseries/lpar.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6.26-rc5/arch/powerpc/platforms/pseries/lpar.c
===================================================================
--- linux-2.6.26-rc5.orig/arch/powerpc/platforms/pseries/lpar.c
+++ linux-2.6.26-rc5/arch/powerpc/platforms/pseries/lpar.c
@@ -305,7 +305,8 @@ static long pSeries_lpar_hpte_insert(uns
 	flags = 0;
 
 	/* Make pHyp happy */
-	if (rflags & (_PAGE_GUARDED|_PAGE_NO_CACHE))
+	if ((rflags & _PAGE_GUARDED) ||
+	    ((rflags & _PAGE_NO_CACHE) & !(rflags & _PAGE_WRITETHRU)))
 		hpte_r &= ~_PAGE_COHERENT;
 
 	lpar_rc = plpar_pte_enter(flags, hpte_group, hpte_v, hpte_r, &slot);

-- 

Re: [patch 1/6] mm: Allow architectures to define additional protection bits

[Andrew Morton]

On Wed, 18 Jun 2008 17:32:55 -0500 shaggy@linux.vnet.ibm.com wrote:

> This patch allows architectures to define functions to deal with
> additional protections bits for mmap() and mprotect().
> 
> arch_calc_vm_prot_bits() maps additonal protection bits to vm_flags
> arch_vm_get_page_prot() maps additional vm_flags to the vma's vm_page_prot
> arch_validate_prot() checks for valid values of the protection bits

It'd be simpler if Paul were to merge this.  It doesn't conflict with
any pending work.

Acked-by: Andrew Morton <akpm@linux-foundation.org>

> Note: vm_get_page_prot() is now pretty ugly.

It is.  But afacit it generates the same code for non-powerpc.

> Suggestions?

nfi.  Let us rub the Hugh-summoning lamp.

> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> ---
> 
>  include/linux/mman.h |   28 +++++++++++++++++++++++++++-
>  mm/mmap.c            |    5 +++--
>  mm/mprotect.c        |    2 +-
>  3 files changed, 31 insertions(+), 4 deletions(-)
> 
> Index: linux-2.6.26-rc5/include/linux/mman.h
> ===================================================================
> --- linux-2.6.26-rc5.orig/include/linux/mman.h
> +++ linux-2.6.26-rc5/include/linux/mman.h
> @@ -34,6 +34,31 @@ static inline void vm_unacct_memory(long
>  }
>  
>  /*
> + * Allow architectures to handle additional protection bits
> + */
> +
> +#ifndef arch_calc_vm_prot_bits
> +#define arch_calc_vm_prot_bits(prot) 0
> +#endif
> +
> +#ifndef arch_vm_get_page_prot
> +#define arch_vm_get_page_prot(vm_flags) __pgprot(0)
> +#endif
> +
> +#ifndef arch_validate_prot
> +/*
> + * This is called from mprotect().  PROT_GROWSDOWN and PROT_GROWSUP have
> + * already been masked out.
> + *
> + * Returns true if the prot flags are valid
> + */
> +static inline int arch_validate_prot(unsigned long prot)
> +{
> +	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
> +}

Officially we should now have

#define arch_validate_prot arch_validate_prot

here.

> +#endif
> +
> +/*
>   * Optimisation macro.  It is equivalent to:
>   *      (x & bit1) ? bit2 : 0
>   * but this version is faster.
> @@ -51,7 +76,8 @@ calc_vm_prot_bits(unsigned long prot)
>  {
>  	return _calc_vm_trans(prot, PROT_READ,  VM_READ ) |
>  	       _calc_vm_trans(prot, PROT_WRITE, VM_WRITE) |
> -	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC );
> +	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC) |
> +	       arch_calc_vm_prot_bits(prot);
>  }
>  
>  /*
> Index: linux-2.6.26-rc5/mm/mmap.c
> ===================================================================
> --- linux-2.6.26-rc5.orig/mm/mmap.c
> +++ linux-2.6.26-rc5/mm/mmap.c
> @@ -72,8 +72,9 @@ pgprot_t protection_map[16] = {
>  
>  pgprot_t vm_get_page_prot(unsigned long vm_flags)
>  {
> -	return protection_map[vm_flags &
> -				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
> +	return __pgprot(pgprot_val(protection_map[vm_flags &
> +				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
> +			pgprot_val(arch_vm_get_page_prot(vm_flags)));
>  }
>  EXPORT_SYMBOL(vm_get_page_prot);
>  
> Index: linux-2.6.26-rc5/mm/mprotect.c
> ===================================================================
> --- linux-2.6.26-rc5.orig/mm/mprotect.c
> +++ linux-2.6.26-rc5/mm/mprotect.c
> @@ -239,7 +239,7 @@ sys_mprotect(unsigned long start, size_t
>  	end = start + len;
>  	if (end <= start)
>  		return -ENOMEM;
> -	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM))
> +	if (!arch_validate_prot(prot))
>  		return -EINVAL;
>  
>  	reqprot = prot;
> 
> -- 

Re: [patch 1/6] mm: Allow architectures to define additional protection bits

[Dave Kleikamp]

On Tue, 2008-07-01 at 01:53 -0700, Andrew Morton wrote:
> On Wed, 18 Jun 2008 17:32:55 -0500 shaggy@linux.vnet.ibm.com wrote:
> 
> > This patch allows architectures to define functions to deal with
> > additional protections bits for mmap() and mprotect().
> > 
> > arch_calc_vm_prot_bits() maps additonal protection bits to vm_flags
> > arch_vm_get_page_prot() maps additional vm_flags to the vma's vm_page_prot
> > arch_validate_prot() checks for valid values of the protection bits
> 
> It'd be simpler if Paul were to merge this.  It doesn't conflict with
> any pending work.

That works for me.  Paul, I'll send you an updated patchset.

> Acked-by: Andrew Morton <akpm@linux-foundation.org>
> 
> > Note: vm_get_page_prot() is now pretty ugly.
> 
> It is.  But afacit it generates the same code for non-powerpc.
> 
> > Suggestions?
> 
> nfi.  Let us rub the Hugh-summoning lamp.
> 
> > Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> > ---
> > 
> >  include/linux/mman.h |   28 +++++++++++++++++++++++++++-
> >  mm/mmap.c            |    5 +++--
> >  mm/mprotect.c        |    2 +-
> >  3 files changed, 31 insertions(+), 4 deletions(-)
> > 
> > Index: linux-2.6.26-rc5/include/linux/mman.h
> > ===================================================================
> > --- linux-2.6.26-rc5.orig/include/linux/mman.h
> > +++ linux-2.6.26-rc5/include/linux/mman.h
> > @@ -34,6 +34,31 @@ static inline void vm_unacct_memory(long
> >  }
> >  
> >  /*
> > + * Allow architectures to handle additional protection bits
> > + */
> > +
> > +#ifndef arch_calc_vm_prot_bits
> > +#define arch_calc_vm_prot_bits(prot) 0
> > +#endif
> > +
> > +#ifndef arch_vm_get_page_prot
> > +#define arch_vm_get_page_prot(vm_flags) __pgprot(0)
> > +#endif
> > +
> > +#ifndef arch_validate_prot
> > +/*
> > + * This is called from mprotect().  PROT_GROWSDOWN and PROT_GROWSUP have
> > + * already been masked out.
> > + *
> > + * Returns true if the prot flags are valid
> > + */
> > +static inline int arch_validate_prot(unsigned long prot)
> > +{
> > +	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
> > +}
> 
> Officially we should now have
> 
> #define arch_validate_prot arch_validate_prot
> 
> here.

No problem.

> > +#endif
> > +
> > +/*
> >   * Optimisation macro.  It is equivalent to:
> >   *      (x & bit1) ? bit2 : 0
> >   * but this version is faster.
> > @@ -51,7 +76,8 @@ calc_vm_prot_bits(unsigned long prot)
> >  {
> >  	return _calc_vm_trans(prot, PROT_READ,  VM_READ ) |
> >  	       _calc_vm_trans(prot, PROT_WRITE, VM_WRITE) |
> > -	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC );
> > +	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC) |
> > +	       arch_calc_vm_prot_bits(prot);
> >  }
> >  
> >  /*
> > Index: linux-2.6.26-rc5/mm/mmap.c
> > ===================================================================
> > --- linux-2.6.26-rc5.orig/mm/mmap.c
> > +++ linux-2.6.26-rc5/mm/mmap.c
> > @@ -72,8 +72,9 @@ pgprot_t protection_map[16] = {
> >  
> >  pgprot_t vm_get_page_prot(unsigned long vm_flags)
> >  {
> > -	return protection_map[vm_flags &
> > -				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
> > +	return __pgprot(pgprot_val(protection_map[vm_flags &
> > +				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
> > +			pgprot_val(arch_vm_get_page_prot(vm_flags)));
> >  }
> >  EXPORT_SYMBOL(vm_get_page_prot);
> >  
> > Index: linux-2.6.26-rc5/mm/mprotect.c
> > ===================================================================
> > --- linux-2.6.26-rc5.orig/mm/mprotect.c
> > +++ linux-2.6.26-rc5/mm/mprotect.c
> > @@ -239,7 +239,7 @@ sys_mprotect(unsigned long start, size_t
> >  	end = start + len;
> >  	if (end <= start)
> >  		return -ENOMEM;
> > -	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM))
> > +	if (!arch_validate_prot(prot))
> >  		return -EINVAL;
> >  
> >  	reqprot = prot;
> > 
> > -- 
-- 
David Kleikamp
IBM Linux Technology Center

Re: [patch 0/6] Strong Access Ordering page attributes for POWER7

[Benjamin Herrenschmidt]

On Wed, 2008-06-18 at 17:32 -0500, shaggy@linux.vnet.ibm.com wrote:
> Andrew,
> 
> The first patch in this series hits architecture independent code, but the
> rest is contained in the powerpc subtree.  Could you pick up the first
> patch into -mm?  I can send the rest of them through the powerpc git tree.
> The first patch and the rest of the set are independent and can be merged
> in either order.

 ../..

I was wondering... how do we inform userspace that this is available ?
Same question with adding the endian bit on 4xx which I plan to do using
your infrastructure...

We haven't defined a user-visible feature bit (and besides, we're really
getting short on these...). This is becoming a bit of concern btw (the
running out of bits). Maybe we should start defining an AT_HWCAP2 for
powerpc and get libc updated to pick it up ?

Ben.

Re: [patch 1/6] mm: Allow architectures to define additional protection bits

[Benjamin Herrenschmidt]

On Tue, 2008-07-01 at 13:54 +0000, Dave Kleikamp wrote:
> On Tue, 2008-07-01 at 01:53 -0700, Andrew Morton wrote:
> > On Wed, 18 Jun 2008 17:32:55 -0500 shaggy@linux.vnet.ibm.com wrote:
> > 
> > > This patch allows architectures to define functions to deal with
> > > additional protections bits for mmap() and mprotect().
> > > 
> > > arch_calc_vm_prot_bits() maps additonal protection bits to vm_flags
> > > arch_vm_get_page_prot() maps additional vm_flags to the vma's vm_page_prot
> > > arch_validate_prot() checks for valid values of the protection bits
> > 
> > It'd be simpler if Paul were to merge this.  It doesn't conflict with
> > any pending work.
> 
> That works for me.  Paul, I'll send you an updated patchset.

Please, CC me as I'll handle this merge window.

> > Acked-by: Andrew Morton <akpm@linux-foundation.org>
> > 
> > > Note: vm_get_page_prot() is now pretty ugly.
> > 
> > It is.  But afacit it generates the same code for non-powerpc.
> > 
> > > Suggestions?
> > 
> > nfi.  Let us rub the Hugh-summoning lamp.

Didn't rub hard enough ? :-)

Cheers,
Ben.

> > > Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> > > ---
> > > 
> > >  include/linux/mman.h |   28 +++++++++++++++++++++++++++-
> > >  mm/mmap.c            |    5 +++--
> > >  mm/mprotect.c        |    2 +-
> > >  3 files changed, 31 insertions(+), 4 deletions(-)
> > > 
> > > Index: linux-2.6.26-rc5/include/linux/mman.h
> > > ===================================================================
> > > --- linux-2.6.26-rc5.orig/include/linux/mman.h
> > > +++ linux-2.6.26-rc5/include/linux/mman.h
> > > @@ -34,6 +34,31 @@ static inline void vm_unacct_memory(long
> > >  }
> > >  
> > >  /*
> > > + * Allow architectures to handle additional protection bits
> > > + */
> > > +
> > > +#ifndef arch_calc_vm_prot_bits
> > > +#define arch_calc_vm_prot_bits(prot) 0
> > > +#endif
> > > +
> > > +#ifndef arch_vm_get_page_prot
> > > +#define arch_vm_get_page_prot(vm_flags) __pgprot(0)
> > > +#endif
> > > +
> > > +#ifndef arch_validate_prot
> > > +/*
> > > + * This is called from mprotect().  PROT_GROWSDOWN and PROT_GROWSUP have
> > > + * already been masked out.
> > > + *
> > > + * Returns true if the prot flags are valid
> > > + */
> > > +static inline int arch_validate_prot(unsigned long prot)
> > > +{
> > > +	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
> > > +}
> > 
> > Officially we should now have
> > 
> > #define arch_validate_prot arch_validate_prot
> > 
> > here.
> 
> No problem.
> 
> > > +#endif
> > > +
> > > +/*
> > >   * Optimisation macro.  It is equivalent to:
> > >   *      (x & bit1) ? bit2 : 0
> > >   * but this version is faster.
> > > @@ -51,7 +76,8 @@ calc_vm_prot_bits(unsigned long prot)
> > >  {
> > >  	return _calc_vm_trans(prot, PROT_READ,  VM_READ ) |
> > >  	       _calc_vm_trans(prot, PROT_WRITE, VM_WRITE) |
> > > -	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC );
> > > +	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC) |
> > > +	       arch_calc_vm_prot_bits(prot);
> > >  }
> > >  
> > >  /*
> > > Index: linux-2.6.26-rc5/mm/mmap.c
> > > ===================================================================
> > > --- linux-2.6.26-rc5.orig/mm/mmap.c
> > > +++ linux-2.6.26-rc5/mm/mmap.c
> > > @@ -72,8 +72,9 @@ pgprot_t protection_map[16] = {
> > >  
> > >  pgprot_t vm_get_page_prot(unsigned long vm_flags)
> > >  {
> > > -	return protection_map[vm_flags &
> > > -				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
> > > +	return __pgprot(pgprot_val(protection_map[vm_flags &
> > > +				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
> > > +			pgprot_val(arch_vm_get_page_prot(vm_flags)));
> > >  }
> > >  EXPORT_SYMBOL(vm_get_page_prot);
> > >  
> > > Index: linux-2.6.26-rc5/mm/mprotect.c
> > > ===================================================================
> > > --- linux-2.6.26-rc5.orig/mm/mprotect.c
> > > +++ linux-2.6.26-rc5/mm/mprotect.c
> > > @@ -239,7 +239,7 @@ sys_mprotect(unsigned long start, size_t
> > >  	end = start + len;
> > >  	if (end <= start)
> > >  		return -ENOMEM;
> > > -	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM))
> > > +	if (!arch_validate_prot(prot))
> > >  		return -EINVAL;
> > >  
> > >  	reqprot = prot;
> > > 
> > > -- 

Re: [patch 0/6] Strong Access Ordering page attributes for POWER7

[Dave Kleikamp]

On Fri, 2008-07-04 at 09:39 +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2008-06-18 at 17:32 -0500, shaggy@linux.vnet.ibm.com wrote:
> > Andrew,
> > 
> > The first patch in this series hits architecture independent code, but the
> > rest is contained in the powerpc subtree.  Could you pick up the first
> > patch into -mm?  I can send the rest of them through the powerpc git tree.
> > The first patch and the rest of the set are independent and can be merged
> > in either order.
> 
>  ../..
> 
> I was wondering... how do we inform userspace that this is available ?
> Same question with adding the endian bit on 4xx which I plan to do using
> your infrastructure...

I hadn't really given it much thought.  Is there a simple way to
determine if the cpu is power 7 or newer?

It's not elegant, but a program could call mmap() with PROT_SAO set and
check for errno == EINVAL.  Then call again without PROT_SAO, if it
needs to.

> We haven't defined a user-visible feature bit (and besides, we're really
> getting short on these...). This is becoming a bit of concern btw (the
> running out of bits). Maybe we should start defining an AT_HWCAP2 for
> powerpc and get libc updated to pick it up ?

Joel,
Any thoughts?

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

Re: [patch 1/6] mm: Allow architectures to define additional protection bits

[Hugh Dickins]

On Mon, 7 Jul 2008, Benjamin Herrenschmidt wrote:
> On Tue, 2008-07-01 at 13:54 +0000, Dave Kleikamp wrote:
> > On Tue, 2008-07-01 at 01:53 -0700, Andrew Morton wrote:
> > > On Wed, 18 Jun 2008 17:32:55 -0500 shaggy@linux.vnet.ibm.com wrote:
> > > 
> > > > This patch allows architectures to define functions to deal with
> > > > additional protections bits for mmap() and mprotect().
> > > > 
> > > > arch_calc_vm_prot_bits() maps additonal protection bits to vm_flags
> > > > arch_vm_get_page_prot() maps additional vm_flags to the vma's vm_page_prot
> > > > arch_validate_prot() checks for valid values of the protection bits
> > > 
> > > It'd be simpler if Paul were to merge this.  It doesn't conflict with
> > > any pending work.
> > 
> > That works for me.  Paul, I'll send you an updated patchset.
> 
> Please, CC me as I'll handle this merge window.
> 
> > > Acked-by: Andrew Morton <akpm@linux-foundation.org>
> > > 
> > > > Note: vm_get_page_prot() is now pretty ugly.
> > > 
> > > It is.  But afacit it generates the same code for non-powerpc.
> > > 
> > > > Suggestions?
> > > 
> > > nfi.  Let us rub the Hugh-summoning lamp.
> 
> Didn't rub hard enough ? :-)

Sorry, Andrew got the wrong pantomime: I was appearing in Aladdin
a couple of years ago, but this year I'm the Sleeping Beauty.
(Did I hear a grumble of dissent from the back stalls?)

I don't find Dave's patch very handsome, but it gets the job done
so I'd better not carp.  The ugliness in vm_get_page_prot is just
an inevitable consequence of growing beyond the traditional neat
pairing of VM_xxx flags with VM_MAYxxx flags, along with the way
that opaque pgprot_t type becomes occasionally tiresome, as such
opaque types do: I don't think there's a better way of handling
it than Dave has done.

There is a little inconsistency, that arch_calc_vm_prot_bits
and arch_vm_get_page_prot just handle the exceptional flag (SAO),
whereas arch_validate_prot handles all of them; but I don't feel
so strongly about that to suggest resubmission.

And regarding VM_SAO added to include/linux/mm.h in 3/6: although
it's odd to be weaving back and forth between arch-specific and
common, it's already the case that mman definitions and pgtable
definitions are arch-specific but mm.h common: I'm much happier
to have VM_SAO defined once there as Dave has it, than get into
arch-specific vm_flags.

Is someone going to be asking for PROT_WC shortly?

Hugh

> 
> Cheers,
> Ben.
> 
> > > > Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> > > > ---
> > > > 
> > > >  include/linux/mman.h |   28 +++++++++++++++++++++++++++-
> > > >  mm/mmap.c            |    5 +++--
> > > >  mm/mprotect.c        |    2 +-
> > > >  3 files changed, 31 insertions(+), 4 deletions(-)
> > > > 
> > > > Index: linux-2.6.26-rc5/include/linux/mman.h
> > > > ===================================================================
> > > > --- linux-2.6.26-rc5.orig/include/linux/mman.h
> > > > +++ linux-2.6.26-rc5/include/linux/mman.h
> > > > @@ -34,6 +34,31 @@ static inline void vm_unacct_memory(long
> > > >  }
> > > >  
> > > >  /*
> > > > + * Allow architectures to handle additional protection bits
> > > > + */
> > > > +
> > > > +#ifndef arch_calc_vm_prot_bits
> > > > +#define arch_calc_vm_prot_bits(prot) 0
> > > > +#endif
> > > > +
> > > > +#ifndef arch_vm_get_page_prot
> > > > +#define arch_vm_get_page_prot(vm_flags) __pgprot(0)
> > > > +#endif
> > > > +
> > > > +#ifndef arch_validate_prot
> > > > +/*
> > > > + * This is called from mprotect().  PROT_GROWSDOWN and PROT_GROWSUP have
> > > > + * already been masked out.
> > > > + *
> > > > + * Returns true if the prot flags are valid
> > > > + */
> > > > +static inline int arch_validate_prot(unsigned long prot)
> > > > +{
> > > > +	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
> > > > +}
> > > 
> > > Officially we should now have
> > > 
> > > #define arch_validate_prot arch_validate_prot
> > > 
> > > here.
> > 
> > No problem.
> > 
> > > > +#endif
> > > > +
> > > > +/*
> > > >   * Optimisation macro.  It is equivalent to:
> > > >   *      (x & bit1) ? bit2 : 0
> > > >   * but this version is faster.
> > > > @@ -51,7 +76,8 @@ calc_vm_prot_bits(unsigned long prot)
> > > >  {
> > > >  	return _calc_vm_trans(prot, PROT_READ,  VM_READ ) |
> > > >  	       _calc_vm_trans(prot, PROT_WRITE, VM_WRITE) |
> > > > -	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC );
> > > > +	       _calc_vm_trans(prot, PROT_EXEC,  VM_EXEC) |
> > > > +	       arch_calc_vm_prot_bits(prot);
> > > >  }
> > > >  
> > > >  /*
> > > > Index: linux-2.6.26-rc5/mm/mmap.c
> > > > ===================================================================
> > > > --- linux-2.6.26-rc5.orig/mm/mmap.c
> > > > +++ linux-2.6.26-rc5/mm/mmap.c
> > > > @@ -72,8 +72,9 @@ pgprot_t protection_map[16] = {
> > > >  
> > > >  pgprot_t vm_get_page_prot(unsigned long vm_flags)
> > > >  {
> > > > -	return protection_map[vm_flags &
> > > > -				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
> > > > +	return __pgprot(pgprot_val(protection_map[vm_flags &
> > > > +				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
> > > > +			pgprot_val(arch_vm_get_page_prot(vm_flags)));
> > > >  }
> > > >  EXPORT_SYMBOL(vm_get_page_prot);
> > > >  
> > > > Index: linux-2.6.26-rc5/mm/mprotect.c
> > > > ===================================================================
> > > > --- linux-2.6.26-rc5.orig/mm/mprotect.c
> > > > +++ linux-2.6.26-rc5/mm/mprotect.c
> > > > @@ -239,7 +239,7 @@ sys_mprotect(unsigned long start, size_t
> > > >  	end = start + len;
> > > >  	if (end <= start)
> > > >  		return -ENOMEM;
> > > > -	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM))
> > > > +	if (!arch_validate_prot(prot))
> > > >  		return -EINVAL;
> > > >  
> > > >  	reqprot = prot;
> > > > 
> > > > -- 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 

Re: [patch 0/6] Strong Access Ordering page attributes for POWER7

[Joel Schopp]

>> We haven't defined a user-visible feature bit (and besides, we're really
>> getting short on these...). This is becoming a bit of concern btw (the
>> running out of bits). Maybe we should start defining an AT_HWCAP2 for
>> powerpc and get libc updated to pick it up ?
>>     
>
> Joel,
> Any thoughts?
Is it a required or optional feature of the 2.06 architecture spec?  If it's required you could just use that.  It doesn't solve the problem more generically if other archs decide to implement it though.

Re: [patch 1/6] mm: Allow architectures to define additional protection bits

[Benjamin Herrenschmidt]

On Mon, 2008-07-07 at 22:11 +0100, Hugh Dickins wrote:
> Sorry, Andrew got the wrong pantomime: I was appearing in Aladdin
> a couple of years ago, but this year I'm the Sleeping Beauty.
> (Did I hear a grumble of dissent from the back stalls?)

No comment :-)

> I don't find Dave's patch very handsome, but it gets the job done
> so I'd better not carp.  The ugliness in vm_get_page_prot is just
> an inevitable consequence of growing beyond the traditional neat
> pairing of VM_xxx flags with VM_MAYxxx flags, along with the way
> that opaque pgprot_t type becomes occasionally tiresome, as such
> opaque types do: I don't think there's a better way of handling
> it than Dave has done.

That was also my conclusion. It didn't look pretty but I couldn't come
up with something prettier.
 
> There is a little inconsistency, that arch_calc_vm_prot_bits
> and arch_vm_get_page_prot just handle the exceptional flag (SAO),
> whereas arch_validate_prot handles all of them; but I don't feel
> so strongly about that to suggest resubmission.
> 
> And regarding VM_SAO added to include/linux/mm.h in 3/6: although
> it's odd to be weaving back and forth between arch-specific and
> common, it's already the case that mman definitions and pgtable
> definitions are arch-specific but mm.h common: I'm much happier
> to have VM_SAO defined once there as Dave has it, than get into
> arch-specific vm_flags.
> 
> Is someone going to be asking for PROT_WC shortly?

I'll definitely come with PROT_ENDIAN soon :-) (ie, some powerpc
processors can have a per-page endian flag that when set causes all
load/store instructions on this are to be byte-flipped, support for this
feature has been requested for some time, and now I have the
infrastructure to do it).

Cheers,
Ben.

Re: [patch 0/6] Strong Access Ordering page attributes for POWER7

[Benjamin Herrenschmidt]

On Mon, 2008-07-07 at 16:23 -0500, Joel Schopp wrote:
> >> We haven't defined a user-visible feature bit (and besides, we're really
> >> getting short on these...). This is becoming a bit of concern btw (the
> >> running out of bits). Maybe we should start defining an AT_HWCAP2 for
> >> powerpc and get libc updated to pick it up ?
> >>     
> >
> > Joel,
> > Any thoughts?
> Is it a required or optional feature of the 2.06 architecture spec?  If it's required you could just use that.  It doesn't solve the problem more generically if other archs decide to implement it though.

And then we start having to expose 2.06S vs. 2.06E .. nah.

I think for now, for SAO, the idea that one can "try" and if -EINVAL,
try again without might work fine.

Cheers,
Ben.

Re: [patch 1/6] mm: Allow architectures to define additional protection bits

[Benjamin Herrenschmidt]

On Tue, 2008-07-08 at 08:24 +1000, Benjamin Herrenschmidt wrote:
> > There is a little inconsistency, that arch_calc_vm_prot_bits
> > and arch_vm_get_page_prot just handle the exceptional flag (SAO),
> > whereas arch_validate_prot handles all of them; but I don't feel
> > so strongly about that to suggest resubmission.
> > 
> > And regarding VM_SAO added to include/linux/mm.h in 3/6: although
> > it's odd to be weaving back and forth between arch-specific and
> > common, it's already the case that mman definitions and pgtable
> > definitions are arch-specific but mm.h common: I'm much happier
> > to have VM_SAO defined once there as Dave has it, than get into
> > arch-specific vm_flags.
> > 
> > Is someone going to be asking for PROT_WC shortly?
> 
> I'll definitely come with PROT_ENDIAN soon :-) (ie, some powerpc
> processors can have a per-page endian flag that when set causes all
> load/store instructions on this are to be byte-flipped, support for
> this
> feature has been requested for some time, and now I have the
> infrastructure to do it).

BTW. Do we have your ack ?

Andrew, what tree should this go via ? I have further powerpc patches
depending on this one... so on one hand I'd be happy to take it, but
on the other hand, it's more likely to clash with other things...

Maybe I should check how it applies on top of linux-next.

Ben.

Re: [patch 1/6] mm: Allow architectures to define additional protection bits

[Hugh Dickins]

On Tue, 8 Jul 2008, Benjamin Herrenschmidt wrote:
> On Tue, 2008-07-08 at 08:24 +1000, Benjamin Herrenschmidt wrote:
> > > There is a little inconsistency, that arch_calc_vm_prot_bits
> > > and arch_vm_get_page_prot just handle the exceptional flag (SAO),
> > > whereas arch_validate_prot handles all of them; but I don't feel
> > > so strongly about that to suggest resubmission.
> > > 
> > > And regarding VM_SAO added to include/linux/mm.h in 3/6: although
> > > it's odd to be weaving back and forth between arch-specific and
> > > common, it's already the case that mman definitions and pgtable
> > > definitions are arch-specific but mm.h common: I'm much happier
> > > to have VM_SAO defined once there as Dave has it, than get into
> > > arch-specific vm_flags.
> > > 
> > > Is someone going to be asking for PROT_WC shortly?
> > 
> > I'll definitely come with PROT_ENDIAN soon :-) (ie, some powerpc
> > processors can have a per-page endian flag that when set causes all
> > load/store instructions on this are to be byte-flipped, support for
> > this
> > feature has been requested for some time, and now I have the
> > infrastructure to do it).
> 
> BTW. Do we have your ack ?

To PROT_SAO?  Okay,
Acked-by: Hugh Dickins <hugh@veritas.com>

> 
> Andrew, what tree should this go via ? I have further powerpc patches
> depending on this one... so on one hand I'd be happy to take it, but
> on the other hand, it's more likely to clash with other things...
> 
> Maybe I should check how it applies on top of linux-next.
> 
> Ben.

Re: [patch 1/6] mm: Allow architectures to define additional protection bits

[Dave Kleikamp]

On Tue, 2008-07-08 at 16:18 +1000, Benjamin Herrenschmidt wrote:

> Andrew, what tree should this go via ? I have further powerpc patches
> depending on this one... so on one hand I'd be happy to take it, but
> on the other hand, it's more likely to clash with other things...

Andrew has asked that it go through Paul, which now means you.

"It'd be simpler if Paul were to merge this.  It doesn't conflict with
any pending work."
http://ozlabs.org/pipermail/linuxppc-dev/2008-July/058948.html

> 
> Maybe I should check how it applies on top of linux-next.

Looks pretty clean:

patching file include/linux/mman.h
patching file mm/mmap.c
patching file mm/mprotect.c
Hunk #1 succeeded at 237 (offset -2 lines).

Thanks,
Shaggy
-- 
David Kleikamp
IBM Linux Technology Center