Strong Access Ordering page attributes for POWER7

PATCH In review [email protected] 5 participants 19 messages Jun 18, 2008 – Jul 8, 2008

Subsystems (4)

Memory management PowerPC MM core Memory mapping

lore

Share & tools

lore

[email protected]

author

2008-06-18 22:32 UTC #0 lore raw

cover Strong Access Ordering page attributes for POWER7

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

[email protected]

author

2008-06-18 22:32 UTC #1 lore raw

PATCH 1/6 mm: Allow architectures to define additional protection bits

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 <[email protected]>

---

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

include/linux/mman.h +27 -1

Index: 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);
 }
 
 /*

mm/mmap.c +3 -2

Index: 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);

mm/mprotect.c +1 -1

Index: 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;

Andrew Morton

maintainer Acked-by on v1 1/6

2008-07-01 08:53 UTC #2 lore raw

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

Quoted 6 lines · On Wed, 18 Jun 2008 17:32:55 -0500 [email protected] 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 <[email protected]>

Quoted 1 line

Note: vm_get_page_prot() is now pretty ugly.

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

Quoted 1 line

Suggestions?

nfi. Let us rub the Hugh-summoning lamp.

Quoted 28 lines

Signed-off-by Dave Kleikamp <[email protected]>

---

 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
===================================================================

include/linux/mman.h +12 -0

@@ -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

include/linux/mman.h static inline void vm_unacct_memory(long · line 58

Quoted 10 lines

include/linux/mman.h +10 -0

+/*
+ * 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.

Quoted 47 lines

include/linux/mman.h +5 -1

+#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
===================================================================

mm/mmap.c +3 -2

@@ -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
===================================================================

mm/mprotect.c +1 -1

@@ -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;

Signature · 1 line

--

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Dave Kleikamp

author

2008-07-01 13:54 UTC #3 lore raw

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

Quoted 11 lines · Andrew Morton

Quoted 6 lines · On Wed, 18 Jun 2008 17:32:55 -0500 [email protected] 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.

Quoted 51 lines

Acked-by Andrew Morton <[email protected]>
Quoted 1 line
Note: vm_get_page_prot() is now pretty ugly.
It is. But afacit it generates the same code for non-powerpc.
Quoted 1 line
Suggestions?
nfi. Let us rub the Hugh-summoning lamp.
Quoted 28 lines
Signed-off-by Dave Kleikamp <[email protected]>
---
 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
===================================================================
include/linux/mman.h +12 -0
@@ -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
include/linux/mman.h static inline void vm_unacct_memory(long · line 58
Quoted 10 lines
include/linux/mman.h +10 -0
+/*
+ * 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

Quoted 3 lines

#define arch_validate_prot arch_validate_prot
here.

No problem.

Quoted 47 lines

include/linux/mman.h +5 -1

+#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
===================================================================

mm/mmap.c +3 -2

@@ -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
===================================================================

mm/mprotect.c +1 -1

@@ -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;

Signature · 1 line

--

Signature · 2 lines

David Kleikamp
IBM Linux Technology Center

Benjamin Herrenschmidt

2008-07-07 05:52 UTC #4 lore raw

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

Quoted 6 lines · Dave Kleikamp

Quoted 5 lines · Andrew Morton
Quoted 3 lines · On Wed, 18 Jun 2008 17:32:55 -0500 [email protected] wrote:
This patch allows architectures to define functions to deal with additional protections bits for mmap() and mprotect().

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
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.

Quoted 9 lines

Quoted 9 lines
Acked-by Andrew Morton <[email protected]>
Quoted 1 line
Note: vm_get_page_prot() is now pretty ugly.
It is. But afacit it generates the same code for non-powerpc.
Quoted 1 line
Suggestions?
nfi. Let us rub the Hugh-summoning lamp.

Didn't rub hard enough ? :-)

Cheers,
Ben.

Quoted 91 lines

Quoted 41 lines

Quoted 28 lines

Signed-off-by Dave Kleikamp <[email protected]>

---

 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
===================================================================

include/linux/mman.h +12 -0

@@ -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

include/linux/mman.h static inline void vm_unacct_memory(long · line 58

Quoted 10 lines

include/linux/mman.h +10 -0

+/*
+ * 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

Quoted 3 lines

#define arch_validate_prot arch_validate_prot
here.

No problem.

Quoted 44 lines

include/linux/mman.h +5 -1

+#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
===================================================================

mm/mmap.c +3 -2

@@ -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
===================================================================

mm/mprotect.c +1 -1

@@ -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;

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
mm/mprotect.c +0 -0
 	reqprot = prot;
Signature · 1 line
-- 

Hugh Dickins

2008-07-07 21:11 UTC #5 lore raw

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

Quoted 22 lines · Benjamin Herrenschmidt

Quoted 6 lines · Dave Kleikamp
Quoted 5 lines · Andrew Morton
Quoted 3 lines · On Wed, 18 Jun 2008 17:32:55 -0500 [email protected] wrote:
This patch allows architectures to define functions to deal with additional protections bits for mmap() and mprotect().
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
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.
Quoted 4 lines
Quoted 4 lines
Acked-by Andrew Morton <[email protected]>
Quoted 1 line
Note: vm_get_page_prot() is now pretty ugly.

Quoted 7 lines

Quoted 5 lines
Quoted 5 lines
It is. But afacit it generates the same code for non-powerpc.
Quoted 1 line
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

Quoted 97 lines

Cheers,
Ben.

Quoted 93 lines

Quoted 41 lines

Quoted 28 lines

Signed-off-by Dave Kleikamp <[email protected]>

---

 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
===================================================================

include/linux/mman.h +12 -0

@@ -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

include/linux/mman.h static inline void vm_unacct_memory(long · line 58

Quoted 10 lines

include/linux/mman.h +10 -0

+/*
+ * 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

Quoted 3 lines

#define arch_validate_prot arch_validate_prot
here.

No problem.

Quoted 46 lines

include/linux/mman.h +5 -1

+#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
===================================================================

mm/mmap.c +3 -2

@@ -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
===================================================================

mm/mprotect.c +1 -1

@@ -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;

Quoted 8 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Benjamin Herrenschmidt

2008-07-07 22:24 UTC #6 lore raw

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

Quoted 3 lines · Hugh Dickins

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 :-)

Quoted 7 lines

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.

Quoted 5 lines

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.

Quoted 8 lines

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.

Benjamin Herrenschmidt

2008-07-08 06:18 UTC #7 lore raw

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

Quoted 12 lines · Benjamin Herrenschmidt

Quoted 12 lines
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.

Quoted 8 lines

Quoted 1 line
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.

Hugh Dickins

Acked-by

2008-07-08 13:00 UTC #8 lore raw

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

Quoted 15 lines · Benjamin Herrenschmidt

Quoted 14 lines · Benjamin Herrenschmidt
Quoted 13 lines
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?

Quoted 8 lines

Quoted 6 lines
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 <[email protected]>

Quoted 8 lines

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.

Dave Kleikamp

author

2008-07-08 13:35 UTC #9 lore raw

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

In reply to Benjamin Herrenschmidt

Quoted 3 lines · Benjamin Herrenschmidt

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

Quoted 2 lines

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

Signature · 2 lines

David Kleikamp
IBM Linux Technology Center

[email protected]

author

2008-06-18 22:32 UTC #10 lore raw

PATCH 2/6 powerpc: hash_huge_page() should get the WIMG bits from the lpte

In reply to [email protected]

Signed-off-by Dave Kleikamp <[email protected]>

Cc Jon Tollefson <[email protected]>

Cc Adam Litke <[email protected]>

Cc Benjamin Herrenschmidt <[email protected]>

---

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

arch/powerpc/mm/hugetlbpage.c +2 -3

Index: 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,

[email protected]

author

2008-06-18 22:28 UTC #11 lore raw

PATCH 3/6 powerpc: Define flags for Strong Access Ordering

In reply to [email protected]

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 <[email protected]>

---

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

include/asm-powerpc/mman.h +2 -0

Index: 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

include/asm-powerpc/pgtable-ppc64.h +3 -0

Index: 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)

include/linux/mm.h +1 -0

Index: 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

Signature · 5 lines

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

[email protected]

author

2008-06-18 22:32 UTC #12 lore raw

PATCH 4/6 powerpc: Add SAO Feature bit to the cputable

In reply to [email protected]

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

Signed-off-by Joel Schopp <[email protected]>

Signed-off-by Dave Kleikamp <[email protected]>

include/asm-powerpc/cputable.h +2 -1

Index: 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 | \

[email protected]

author

2008-06-18 22:32 UTC #13 lore raw

PATCH 5/6 powerpc: Add Strong Access Ordering

In reply to [email protected]

Signed-off-by Dave Kleikamp <[email protected]>

---

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

arch/powerpc/kernel/syscalls.c +3 -0

Index: 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;

include/asm-powerpc/mman.h +28 -0

Index: 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 */

[email protected]

author

2008-06-18 22:33 UTC #14 lore raw

PATCH 6/6 powerpc: Dont clear _PAGE_COHERENT when _PAGE_SAO is set

In reply to [email protected]

Signed-off-by Dave Kleikamp <[email protected]>

Cc Benjamin Herrenschmidt <[email protected]>

---

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

arch/powerpc/platforms/pseries/lpar.c +2 -1

Index: 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);

Benjamin Herrenschmidt

2008-07-03 23:39 UTC #15 lore raw

In reply to [email protected]

Quoted 7 lines · On Wed, 2008-06-18 at 17:32 -0500, [email protected] 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.

Dave Kleikamp

author

2008-07-07 14:05 UTC #16 lore raw

Quoted 9 lines · Benjamin Herrenschmidt

Quoted 7 lines · On Wed, 2008-06-18 at 17:32 -0500, [email protected] 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.

Quoted 5 lines

../..
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.

Quoted 4 lines

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

Signature · 2 lines

David Kleikamp
IBM Linux Technology Center

Joel Schopp

2008-07-07 21:23 UTC #17 lore raw

Quoted 9 lines

Quoted 6 lines
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.

Benjamin Herrenschmidt

2008-07-07 22:27 UTC #18 lore raw

Quoted 9 lines · Joel Schopp

Quoted 8 lines
Quoted 5 lines
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.