[RFC v7 00/25] powerpc: Memory Protection Keys

[RFC v7 00/25] powerpc: Memory Protection Keys

[Ram Pai]

Memory protection keys enable applications to protect its
address  space from inadvertent access from or corruption
by itself.

The overall idea:
-----------------
 A process allocates a   key  and associates it with
 an  address  range  within    its   address   space.
 The process  then  can  dynamically  set read/write 
 permissions on  the   key   without  involving  the 
 kernel. Any  code that  violates   the  permissions
 of  the address space; as defined by its associated
 key, will receive a segmentation fault.

This  patch series enables the feature on PPC64 HPTE
platform.

ISA3.0   section  5.7.13   describes  the  detailed
specifications.


Highlevel view of the design:
---------------------------
When  an  application associates a key with a address
address  range,  program  the key in    the Linux PTE.
When the MMU   detects  a page fault, allocate a hash
page  and   program  the  key into HPTE.  And finally
when the  MMU    detects  a  key  violation;  due  to
invalid    application  access, invoke the registered
signal   handler and provide the violated  key number
as   well  as the state of the key register (AMR), at
the time it faulted.


Testing:
-------
This  patch  series has passed all the protection key
tests   available    in   the selftests directory.The
tests are updated  to    work on both x86 and powerpc.
NOTE: All the selftest related patches will be   part
of  a separate patch series.


Outstanding issues:
-------------------
How will the application know if pkey is  enabled, if
so   how   many     pkeys     are       available? Is
PKEY_DISABLE_EXECUTE supported?  - Ben.


History:
-------
version v7:
	(1) refers to device tree property to enable
		protection keys.
	(2) adds 4K PTE support.
	(3) fixes a couple of bugs noticed by Thiago
	(4) decouples this patch series from   arch-
	    independent code. This patch series can
	    now stand by itself, with one kludge
	    patch(2).

version v6:
	(1) selftest changes  are broken down into 20
		incremental patches.
	(2) A  separate   key  allocation  mask  that
       		includes    PKEY_DISABLE_EXECUTE   is 
		added for powerpc
	(3) pkey feature  is enabled for 64K HPT case
		only.  RPT and 4k HPT is disabled.
	(4) Documentation   is   updated   to  better 
		capture the semantics.
	(5) introduced   arch_pkeys_enabled() to find
       		if an arch enables pkeys.  Correspond-
		ing change the  logic   that displays
		key value in smaps.
	(6) code  rearranged  in many places based on
       		comments from   Dave Hansen,   Balbir,
	       	Anshuman.	
	(7) fixed  one bug where a bogus key could be
		associated     successfully        in
		pkey_mprotect().

version v5:
	(1) reverted back to the old  design -- store
	    the key in the pte,  instead of bypassing
	    it.  The v4  design  slowed down the hash
	    page path.
	(2) detects key violation when kernel is told
       		to access user pages.
	(3) further  refined the patches into smaller
       		consumable units
	(4) page faults   handlers captures the fault-
		ing key 
	    from the pte   instead of   the vma. This
	    closes  a  race  between  where  the  key 
	    update in the  vma and a key fault caused
	    by the key programmed in the pte.
	(5) a key created   with access-denied should
	    also set it up to deny write. Fixed it.
	(6) protection-key   number   is displayed in
       		smaps the x86 way.

version v4:
	(1) patches no more depend on the pte bits
       		to program the hpte
			-- comment by Balbir
	(2) documentation updates
	(3) fixed a bug in the selftest.
	(4) unlike x86, powerpc   lets signal handler
		change   key   permission   bits; the
	       	change   will   persist across signal
	       	handler   boundaries.   Earlier    we
	       	allowed   the   signal   handler   to
	       	modify   a   field   in   the siginfo
		structure   which would  than be used
       		by  the  kernel  to  program  the key
		protection register (AMR)
       		  -- resolves a issue raised by Ben.
    		"Calls  to  sys_swapcontext  with   a
		made-up  context  will  end up with a
		crap  AMR  if done by code who didn't
	       	know about that register".
	(5) these  changes  enable protection keys on
       		4k-page kernel aswell.

version v3:
	(1) split the patches into smaller consumable
		patches.
	(2) added  the  ability  to  disable  execute
       		permission  on  a  key  at   creation.
	(3) rename    calc_pte_to_hpte_pkey_bits() to
	    pte_to_hpte_pkey_bits()
		-- suggested by Anshuman
	(4) some   code   optimization and clarity in
		do_page_fault()  
	(5) A bug fix while  invalidating a hpte slot
		in __hash_page_4K()
       		-- noticed by Aneesh
	

version v2:
	(1) documentation and selftest added.
 	(2) fixed a  bug  in 4k  hpte  backed 64k pte
       		where  page    invalidation   was not
		done  correctly,  and  initialization
	       	of    second-part-of-the-pte  was not
		done    correctly  if the pte was not
	       	yet Hashed with a hpte.
	       	   --	Reported by Aneesh.
	(3) Fixed  ABI  breakage  caused in siginfo
       		structure.
		-- Reported by Anshuman.
	

version v1: Initial version


Ram Pai (25):
  powerpc: define an additional vma bit for protection keys.
  powerpc: track allocation status of all pkeys
  powerpc: helper function to read,write AMR,IAMR,UAMOR registers
  powerpc: helper functions to initialize AMR, IAMR and UAMOR registers
  powerpc: cleaup AMR,iAMR when a key is allocated or freed
  powerpc: implementation for arch_set_user_pkey_access()
  powerpc: sys_pkey_alloc() and sys_pkey_free() system calls
  powerpc: ability to create execute-disabled pkeys
  powerpc: store and restore the pkey state across context switches
  powerpc: introduce execute-only pkey
  powerpc: ability to associate pkey to a vma
  powerpc: implementation for arch_override_mprotect_pkey()
  powerpc: map vma key-protection bits to pte key bits.
  powerpc: sys_pkey_mprotect() system call
  powerpc: Program HPTE key protection bits
  powerpc: helper to validate key-access permissions of a pte
  powerpc: check key protection for user page access
  powerpc: Macro the mask used for checking DSI exception
  powerpc: implementation for arch_vma_access_permitted()
  powerpc: Handle exceptions caused by pkey violation
  powerpc: capture AMR register content on pkey violation
  powerpc: introduce get_pte_pkey() helper
  powerpc: capture the violated protection key on fault
  powerpc: Deliver SEGV signal on pkey violation
  powerpc: Enable pkey subsystem

 arch/powerpc/include/asm/book3s/64/mmu-hash.h |   10 +
 arch/powerpc/include/asm/book3s/64/mmu.h      |   10 +
 arch/powerpc/include/asm/book3s/64/pgtable.h  |   69 +++++++-
 arch/powerpc/include/asm/cputable.h           |    8 +-
 arch/powerpc/include/asm/mman.h               |   16 ++-
 arch/powerpc/include/asm/mmu_context.h        |   18 ++-
 arch/powerpc/include/asm/paca.h               |    4 +
 arch/powerpc/include/asm/pkeys.h              |  255 ++++++++++++++++++++++++-
 arch/powerpc/include/asm/processor.h          |    5 +
 arch/powerpc/include/asm/reg.h                |    8 +-
 arch/powerpc/include/asm/systbl.h             |    3 +
 arch/powerpc/include/asm/unistd.h             |    6 +-
 arch/powerpc/include/uapi/asm/ptrace.h        |    1 +
 arch/powerpc/include/uapi/asm/unistd.h        |    3 +
 arch/powerpc/kernel/asm-offsets.c             |    6 +
 arch/powerpc/kernel/exceptions-64s.S          |    2 +-
 arch/powerpc/kernel/process.c                 |   25 +++
 arch/powerpc/kernel/prom.c                    |   19 ++
 arch/powerpc/kernel/signal_32.c               |    5 +
 arch/powerpc/kernel/signal_64.c               |    4 +
 arch/powerpc/kernel/traps.c                   |   15 ++
 arch/powerpc/mm/fault.c                       |   31 +++
 arch/powerpc/mm/hash_utils_64.c               |   26 +++
 arch/powerpc/mm/mmu_context_book3s64.c        |    2 +
 arch/powerpc/mm/pkeys.c                       |  256 +++++++++++++++++++++++++
 25 files changed, 787 insertions(+), 20 deletions(-)

[RFC v7 01/25] powerpc: define an additional vma bit for protection keys.

[Ram Pai]

powerpc needs an additional vma bit to support 32 keys.
Till the additional vma bit lands in include/linux/mm.h
we have to define  it  in powerpc specific header file.
This is  needed to get pkeys working on power.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/pkeys.h |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index acad7c2..4ccb8f5 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -2,6 +2,24 @@
 #define _ASM_PPC64_PKEYS_H
 
 extern bool pkey_inited;
+
+/*
+ * powerpc needs an additional vma bit to support 32 keys.
+ * Till the additional vma bit lands in include/linux/mm.h
+ * we have to carry the hunk below. This is  needed to get
+ * pkeys working on power. -- Ram
+ */
+#ifndef VM_HIGH_ARCH_BIT_4
+#define VM_HIGH_ARCH_BIT_4	36
+#define VM_HIGH_ARCH_4	BIT(VM_HIGH_ARCH_BIT_4)
+#define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
+#define VM_PKEY_BIT0	VM_HIGH_ARCH_0
+#define VM_PKEY_BIT1	VM_HIGH_ARCH_1
+#define VM_PKEY_BIT2	VM_HIGH_ARCH_2
+#define VM_PKEY_BIT3	VM_HIGH_ARCH_3
+#define VM_PKEY_BIT4	VM_HIGH_ARCH_4
+#endif
+
 #define ARCH_VM_PKEY_FLAGS 0
 
 static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
-- 
1.7.1

[RFC v7 02/25] powerpc: track allocation status of all pkeys

[Ram Pai]

Total 32 keys are available on power7 and above. However
pkey 0,1 are reserved. So effectively we  have  30 pkeys.

On 4K kernels, we do not  have  5  bits  in  the  PTE to
represent  all the keys; we only have 3bits.Two of those
keys are reserved; pkey 0 and pkey 1. So effectively  we
have 6 pkeys.

This patch keeps track of reserved keys, allocated  keys
and keys that are currently free.

Also it  adds  skeletal  functions  and macros, that the
architecture-independent code expects to be available.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/mmu.h |    9 +++
 arch/powerpc/include/asm/mmu_context.h   |    1 +
 arch/powerpc/include/asm/pkeys.h         |   98 ++++++++++++++++++++++++++++-
 arch/powerpc/mm/mmu_context_book3s64.c   |    2 +
 arch/powerpc/mm/pkeys.c                  |    2 +
 5 files changed, 108 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
index 77529a3..104ad72 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu.h
@@ -108,6 +108,15 @@ struct patb_entry {
 #ifdef CONFIG_SPAPR_TCE_IOMMU
 	struct list_head iommu_group_mem_list;
 #endif
+
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	/*
+	 * Each bit represents one protection key.
+	 * bit set   -> key allocated
+	 * bit unset -> key available for allocation
+	 */
+	u32 pkey_allocation_map;
+#endif
 } mm_context_t;
 
 /*
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 4b93547..4705dab 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -184,6 +184,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
 
 #ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
 #define pkey_initialize()
+#define pkey_mm_init(mm)
 #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
 
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 4ccb8f5..def385f 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -2,6 +2,8 @@
 #define _ASM_PPC64_PKEYS_H
 
 extern bool pkey_inited;
+extern int pkeys_total; /* total pkeys as per device tree */
+extern u32 initial_allocation_mask;/* bits set for reserved keys */
 
 /*
  * powerpc needs an additional vma bit to support 32 keys.
@@ -20,21 +22,76 @@
 #define VM_PKEY_BIT4	VM_HIGH_ARCH_4
 #endif
 
-#define ARCH_VM_PKEY_FLAGS 0
+#define arch_max_pkey()  pkeys_total
+#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
+				VM_PKEY_BIT3 | VM_PKEY_BIT4)
+
+#define pkey_alloc_mask(pkey) (0x1 << pkey)
+
+#define mm_pkey_allocation_map(mm)	(mm->context.pkey_allocation_map)
+
+#define mm_set_pkey_allocated(mm, pkey) {	\
+	mm_pkey_allocation_map(mm) |= pkey_alloc_mask(pkey); \
+}
+
+#define mm_set_pkey_free(mm, pkey) {	\
+	mm_pkey_allocation_map(mm) &= ~pkey_alloc_mask(pkey);	\
+}
+
+#define mm_set_pkey_is_allocated(mm, pkey)	\
+	(mm_pkey_allocation_map(mm) & pkey_alloc_mask(pkey))
+
+#define mm_set_pkey_is_reserved(mm, pkey) (initial_allocation_mask & \
+					pkey_alloc_mask(pkey))
 
 static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
 {
-	return (pkey == 0);
+	/* a reserved key is never considered as 'explicitly allocated' */
+	return ((pkey < arch_max_pkey()) &&
+		!mm_set_pkey_is_reserved(mm, pkey) &&
+		mm_set_pkey_is_allocated(mm, pkey));
 }
 
+/*
+ * Returns a positive, 5-bit key on success, or -1 on failure.
+ */
 static inline int mm_pkey_alloc(struct mm_struct *mm)
 {
-	return -1;
+	/*
+	 * Note: this is the one and only place we make sure
+	 * that the pkey is valid as far as the hardware is
+	 * concerned.  The rest of the kernel trusts that
+	 * only good, valid pkeys come out of here.
+	 */
+	u32 all_pkeys_mask = (u32)(~(0x0));
+	int ret;
+
+	if (!pkey_inited)
+		return -1;
+	/*
+	 * Are we out of pkeys?  We must handle this specially
+	 * because ffz() behavior is undefined if there are no
+	 * zeros.
+	 */
+	if (mm_pkey_allocation_map(mm) == all_pkeys_mask)
+		return -1;
+
+	ret = ffz((u32)mm_pkey_allocation_map(mm));
+	mm_set_pkey_allocated(mm, ret);
+	return ret;
 }
 
 static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
 {
-	return -EINVAL;
+	if (!pkey_inited)
+		return -1;
+
+	if (!mm_pkey_is_allocated(mm, pkey))
+		return -EINVAL;
+
+	mm_set_pkey_free(mm, pkey);
+
+	return 0;
 }
 
 /*
@@ -58,12 +115,45 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 	return 0;
 }
 
+static inline void pkey_mm_init(struct mm_struct *mm)
+{
+	if (!pkey_inited)
+		return;
+	mm_pkey_allocation_map(mm) = initial_allocation_mask;
+}
+
 static inline void pkey_initialize(void)
 {
+	int os_reserved, i;
+
 	/* disable the pkey system till everything
 	 * is in place. A patch further down the
 	 * line will enable it.
 	 */
 	pkey_inited = false;
+
+	/* Lets assume 32 keys */
+	pkeys_total = 32;
+
+#ifdef CONFIG_PPC_4K_PAGES
+	/*
+	 * the OS can manage only 8 pkeys
+	 * due to its inability to represent
+	 * them in the linux 4K-PTE.
+	 */
+	os_reserved = pkeys_total-8;
+#else
+	os_reserved = 0;
+#endif
+	/*
+	 * Bits are in LE format.
+	 * NOTE: 1, 0 are reserved.
+	 * key 0 is the default key, which allows read/write/execute.
+	 * key 1 is recommended not to be used.
+	 * PowerISA(3.0) page 1015, programming note.
+	 */
+	initial_allocation_mask = ~0x0;
+	for (i = 2; i < (pkeys_total - os_reserved); i++)
+		initial_allocation_mask &= ~(0x1<<i);
 }
 #endif /*_ASM_PPC64_PKEYS_H */
diff --git a/arch/powerpc/mm/mmu_context_book3s64.c b/arch/powerpc/mm/mmu_context_book3s64.c
index a3edf81..34a16f3 100644
--- a/arch/powerpc/mm/mmu_context_book3s64.c
+++ b/arch/powerpc/mm/mmu_context_book3s64.c
@@ -16,6 +16,7 @@
 #include <linux/string.h>
 #include <linux/types.h>
 #include <linux/mm.h>
+#include <linux/pkeys.h>
 #include <linux/spinlock.h>
 #include <linux/idr.h>
 #include <linux/export.h>
@@ -120,6 +121,7 @@ static int hash__init_new_context(struct mm_struct *mm)
 
 	subpage_prot_init_new_context(mm);
 
+	pkey_mm_init(mm);
 	return index;
 }
 
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index c3acee1..37dacc5 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -16,3 +16,5 @@
 #include <linux/pkeys.h>                /* PKEY_*                       */
 
 bool pkey_inited;
+int  pkeys_total;		/* total pkeys as per device tree */
+u32  initial_allocation_mask;	/* bits set for reserved keys */
-- 
1.7.1

[RFC v7 03/25] powerpc: helper function to read, write AMR, IAMR, UAMOR registers

[Ram Pai]

Implements helper functions to read and write the key related
registers; AMR, IAMR, UAMOR.

AMR register tracks the read,write permission of a key
IAMR register tracks the execute permission of a key
UAMOR register enables and disables a key

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/pgtable.h |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 85bc987..d4da0e9 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -428,6 +428,32 @@ static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,
 		pte_update(mm, addr, ptep, 0, _PAGE_PRIVILEGED, 1);
 }
 
+#include <asm/reg.h>
+static inline u64 read_amr(void)
+{
+	return mfspr(SPRN_AMR);
+}
+static inline void write_amr(u64 value)
+{
+	mtspr(SPRN_AMR, value);
+}
+static inline u64 read_iamr(void)
+{
+	return mfspr(SPRN_IAMR);
+}
+static inline void write_iamr(u64 value)
+{
+	mtspr(SPRN_IAMR, value);
+}
+static inline u64 read_uamor(void)
+{
+	return mfspr(SPRN_UAMOR);
+}
+static inline void write_uamor(u64 value)
+{
+	mtspr(SPRN_UAMOR, value);
+}
+
 #define __HAVE_ARCH_PTEP_GET_AND_CLEAR
 static inline pte_t ptep_get_and_clear(struct mm_struct *mm,
 				       unsigned long addr, pte_t *ptep)
-- 
1.7.1

[RFC v7 04/25] powerpc: helper functions to initialize AMR, IAMR and UAMOR registers

[Ram Pai]

Introduce  helper functions that can initialize the bits in the AMR,
IAMR and UAMOR register; the bits that correspond to the given pkey.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/pkeys.h |    1 +
 arch/powerpc/mm/pkeys.c          |   46 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index def385f..f1c55e0 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -25,6 +25,7 @@
 #define arch_max_pkey()  pkeys_total
 #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
 				VM_PKEY_BIT3 | VM_PKEY_BIT4)
+#define AMR_BITS_PER_PKEY 2
 
 #define pkey_alloc_mask(pkey) (0x1 << pkey)
 
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index 37dacc5..846d984 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -18,3 +18,49 @@
 bool pkey_inited;
 int  pkeys_total;		/* total pkeys as per device tree */
 u32  initial_allocation_mask;	/* bits set for reserved keys */
+
+#define PKEY_REG_BITS (sizeof(u64)*8)
+#define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey+1) * AMR_BITS_PER_PKEY))
+
+static inline void init_amr(int pkey, u8 init_bits)
+{
+	u64 new_amr_bits = (((u64)init_bits & 0x3UL) << pkeyshift(pkey));
+	u64 old_amr = read_amr() & ~((u64)(0x3ul) << pkeyshift(pkey));
+
+	write_amr(old_amr | new_amr_bits);
+}
+
+static inline void init_iamr(int pkey, u8 init_bits)
+{
+	u64 new_iamr_bits = (((u64)init_bits & 0x3UL) << pkeyshift(pkey));
+	u64 old_iamr = read_iamr() & ~((u64)(0x3ul) << pkeyshift(pkey));
+
+	write_iamr(old_iamr | new_iamr_bits);
+}
+
+static void pkey_status_change(int pkey, bool enable)
+{
+	u64 old_uamor;
+
+	/* reset the AMR and IAMR bits for this key */
+	init_amr(pkey, 0x0);
+	init_iamr(pkey, 0x0);
+
+	/* enable/disable key */
+	old_uamor = read_uamor();
+	if (enable)
+		old_uamor |= (0x3ul << pkeyshift(pkey));
+	else
+		old_uamor &= ~(0x3ul << pkeyshift(pkey));
+	write_uamor(old_uamor);
+}
+
+void __arch_activate_pkey(int pkey)
+{
+	pkey_status_change(pkey, true);
+}
+
+void __arch_deactivate_pkey(int pkey)
+{
+	pkey_status_change(pkey, false);
+}
-- 
1.7.1

[RFC v7 05/25] powerpc: cleaup AMR, iAMR when a key is allocated or freed

[Ram Pai]

cleanup the bits corresponding to a key in the AMR, and IAMR
register, when the key is newly allocated/activated or is freed.
We dont want some residual bits cause the hardware enforce
unintended behavior when the key is activated or freed.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/pkeys.h |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index f1c55e0..40bfaac 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -53,6 +53,8 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
 		mm_set_pkey_is_allocated(mm, pkey));
 }
 
+extern void __arch_activate_pkey(int pkey);
+extern void __arch_deactivate_pkey(int pkey);
 /*
  * Returns a positive, 5-bit key on success, or -1 on failure.
  */
@@ -79,6 +81,12 @@ static inline int mm_pkey_alloc(struct mm_struct *mm)
 
 	ret = ffz((u32)mm_pkey_allocation_map(mm));
 	mm_set_pkey_allocated(mm, ret);
+
+	/*
+	 * enable the key in the hardware
+	 */
+	if (ret > 0)
+		__arch_activate_pkey(ret);
 	return ret;
 }
 
@@ -90,6 +98,10 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
 	if (!mm_pkey_is_allocated(mm, pkey))
 		return -EINVAL;
 
+	/*
+	 * Disable the key in the hardware
+	 */
+	__arch_deactivate_pkey(pkey);
 	mm_set_pkey_free(mm, pkey);
 
 	return 0;
-- 
1.7.1

[RFC v7 06/25] powerpc: implementation for arch_set_user_pkey_access()

[Ram Pai]

This patch provides the detailed implementation for
a user to allocate a key and enable it in the hardware.

It provides the plumbing, but it cannot be used till
the system call is implemented. The next patch  will
do so.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/pkeys.h |    9 ++++++++-
 arch/powerpc/mm/pkeys.c          |   28 ++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 40bfaac..aec3f49 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -23,6 +23,9 @@
 #endif
 
 #define arch_max_pkey()  pkeys_total
+#define AMR_RD_BIT 0x1UL
+#define AMR_WR_BIT 0x2UL
+#define IAMR_EX_BIT 0x1UL
 #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
 				VM_PKEY_BIT3 | VM_PKEY_BIT4)
 #define AMR_BITS_PER_PKEY 2
@@ -122,10 +125,14 @@ static inline int arch_override_mprotect_pkey(struct vm_area_struct *vma,
 	return 0;
 }
 
+extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
+		unsigned long init_val);
 static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 		unsigned long init_val)
 {
-	return 0;
+	if (!pkey_inited)
+		return -EINVAL;
+	return __arch_set_user_pkey_access(tsk, pkey, init_val);
 }
 
 static inline void pkey_mm_init(struct mm_struct *mm)
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index 846d984..5b3531e 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -22,6 +22,11 @@
 #define PKEY_REG_BITS (sizeof(u64)*8)
 #define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey+1) * AMR_BITS_PER_PKEY))
 
+static bool is_pkey_enabled(int pkey)
+{
+	return !!(read_uamor() & (0x3ul << pkeyshift(pkey)));
+}
+
 static inline void init_amr(int pkey, u8 init_bits)
 {
 	u64 new_amr_bits = (((u64)init_bits & 0x3UL) << pkeyshift(pkey));
@@ -64,3 +69,26 @@ void __arch_deactivate_pkey(int pkey)
 {
 	pkey_status_change(pkey, false);
 }
+
+/*
+ * set the access right in AMR IAMR and UAMOR register
+ * for @pkey to that specified in @init_val.
+ */
+int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
+		unsigned long init_val)
+{
+	u64 new_amr_bits = 0x0ul;
+
+	if (!is_pkey_enabled(pkey))
+		return -EINVAL;
+
+	/* Set the bits we need in AMR:  */
+	if (init_val & PKEY_DISABLE_ACCESS)
+		new_amr_bits |= AMR_RD_BIT | AMR_WR_BIT;
+	else if (init_val & PKEY_DISABLE_WRITE)
+		new_amr_bits |= AMR_WR_BIT;
+
+	init_amr(pkey, new_amr_bits);
+
+	return 0;
+}
-- 
1.7.1

[RFC v7 07/25] powerpc: sys_pkey_alloc() and sys_pkey_free() system calls

[Ram Pai]

Finally this patch provides the ability for a process to
allocate and free a protection key.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/systbl.h      |    2 ++
 arch/powerpc/include/asm/unistd.h      |    4 +---
 arch/powerpc/include/uapi/asm/unistd.h |    2 ++
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index 1c94708..22dd776 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -388,3 +388,5 @@
 COMPAT_SYS_SPU(pwritev2)
 SYSCALL(kexec_file_load)
 SYSCALL(statx)
+SYSCALL(pkey_alloc)
+SYSCALL(pkey_free)
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index 9ba11db..e0273bc 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -12,13 +12,11 @@
 #include <uapi/asm/unistd.h>
 
 
-#define NR_syscalls		384
+#define NR_syscalls		386
 
 #define __NR__exit __NR_exit
 
 #define __IGNORE_pkey_mprotect
-#define __IGNORE_pkey_alloc
-#define __IGNORE_pkey_free
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/powerpc/include/uapi/asm/unistd.h b/arch/powerpc/include/uapi/asm/unistd.h
index b85f142..7993a07 100644
--- a/arch/powerpc/include/uapi/asm/unistd.h
+++ b/arch/powerpc/include/uapi/asm/unistd.h
@@ -394,5 +394,7 @@
 #define __NR_pwritev2		381
 #define __NR_kexec_file_load	382
 #define __NR_statx		383
+#define __NR_pkey_alloc		384
+#define __NR_pkey_free		385
 
 #endif /* _UAPI_ASM_POWERPC_UNISTD_H_ */
-- 
1.7.1

[RFC v7 08/25] powerpc: ability to create execute-disabled pkeys

[Ram Pai]

powerpc has hardware support to disable execute on a pkey.
This patch enables the ability to create execute-disabled
keys.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/pkeys.h |   12 ++++++++++++
 arch/powerpc/mm/pkeys.c          |    5 +++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index aec3f49..9d59619 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -22,6 +22,18 @@
 #define VM_PKEY_BIT4	VM_HIGH_ARCH_4
 #endif
 
+/* override any generic PKEY Permission defines */
+#undef  PKEY_DISABLE_ACCESS
+#define PKEY_DISABLE_ACCESS    0x1
+#undef  PKEY_DISABLE_WRITE
+#define PKEY_DISABLE_WRITE     0x2
+#undef  PKEY_DISABLE_EXECUTE
+#define PKEY_DISABLE_EXECUTE   0x4
+#undef  PKEY_ACCESS_MASK
+#define PKEY_ACCESS_MASK       (PKEY_DISABLE_ACCESS |\
+				PKEY_DISABLE_WRITE  |\
+				PKEY_DISABLE_EXECUTE)
+
 #define arch_max_pkey()  pkeys_total
 #define AMR_RD_BIT 0x1UL
 #define AMR_WR_BIT 0x2UL
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index 5b3531e..c1c40c3 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -78,6 +78,7 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 		unsigned long init_val)
 {
 	u64 new_amr_bits = 0x0ul;
+	u64 new_iamr_bits = 0x0ul;
 
 	if (!is_pkey_enabled(pkey))
 		return -EINVAL;
@@ -90,5 +91,9 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 
 	init_amr(pkey, new_amr_bits);
 
+	if ((init_val & PKEY_DISABLE_EXECUTE))
+		new_iamr_bits |= IAMR_EX_BIT;
+
+	init_iamr(pkey, new_iamr_bits);
 	return 0;
 }
-- 
1.7.1

[RFC v7 09/25] powerpc: store and restore the pkey state across context switches

[Ram Pai]

Store and restore the AMR, IAMR and UAMOR register state of the task
before scheduling out and after scheduling in, respectively.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/pkeys.h     |    5 +++++
 arch/powerpc/include/asm/processor.h |    5 +++++
 arch/powerpc/kernel/process.c        |   25 +++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 9d59619..dac8751 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -147,6 +147,11 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 	return __arch_set_user_pkey_access(tsk, pkey, init_val);
 }
 
+static inline bool arch_pkeys_enabled(void)
+{
+	return pkey_inited;
+}
+
 static inline void pkey_mm_init(struct mm_struct *mm)
 {
 	if (!pkey_inited)
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 1189d04..dcb1cf0 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -309,6 +309,11 @@ struct thread_struct {
 	struct thread_vr_state ckvr_state; /* Checkpointed VR state */
 	unsigned long	ckvrsave; /* Checkpointed VRSAVE */
 #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	unsigned long	amr;
+	unsigned long	iamr;
+	unsigned long	uamor;
+#endif
 #ifdef CONFIG_KVM_BOOK3S_32_HANDLER
 	void*		kvm_shadow_vcpu; /* KVM internal data */
 #endif /* CONFIG_KVM_BOOK3S_32_HANDLER */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 2ad725e..af373f4 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -42,6 +42,7 @@
 #include <linux/hw_breakpoint.h>
 #include <linux/uaccess.h>
 #include <linux/elf-randomize.h>
+#include <linux/pkeys.h>
 
 #include <asm/pgtable.h>
 #include <asm/io.h>
@@ -1096,6 +1097,13 @@ static inline void save_sprs(struct thread_struct *t)
 		t->tar = mfspr(SPRN_TAR);
 	}
 #endif
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	if (arch_pkeys_enabled()) {
+		t->amr = mfspr(SPRN_AMR);
+		t->iamr = mfspr(SPRN_IAMR);
+		t->uamor = mfspr(SPRN_UAMOR);
+	}
+#endif
 }
 
 static inline void restore_sprs(struct thread_struct *old_thread,
@@ -1131,6 +1139,16 @@ static inline void restore_sprs(struct thread_struct *old_thread,
 			mtspr(SPRN_TAR, new_thread->tar);
 	}
 #endif
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	if (arch_pkeys_enabled()) {
+		if (old_thread->amr != new_thread->amr)
+			mtspr(SPRN_AMR, new_thread->amr);
+		if (old_thread->iamr != new_thread->iamr)
+			mtspr(SPRN_IAMR, new_thread->iamr);
+		if (old_thread->uamor != new_thread->uamor)
+			mtspr(SPRN_UAMOR, new_thread->uamor);
+	}
+#endif
 }
 
 struct task_struct *__switch_to(struct task_struct *prev,
@@ -1689,6 +1707,13 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)
 	current->thread.tm_tfiar = 0;
 	current->thread.load_tm = 0;
 #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	if (arch_pkeys_enabled()) {
+		current->thread.amr   = 0x0ul;
+		current->thread.iamr  = 0x0ul;
+		current->thread.uamor = 0x0ul;
+	}
+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
 }
 EXPORT_SYMBOL(start_thread);
 
-- 
1.7.1

[RFC v7 10/25] powerpc: introduce execute-only pkey

[Ram Pai]

This patch provides the implementation of execute-only pkey.
The architecture-independent  expects the ability to create
and manage a special key which has execute-only permission.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/mmu.h |    1 +
 arch/powerpc/include/asm/pkeys.h         |    8 ++++-
 arch/powerpc/mm/pkeys.c                  |   57 ++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
index 104ad72..0c0a2a8 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu.h
@@ -116,6 +116,7 @@ struct patb_entry {
 	 * bit unset -> key available for allocation
 	 */
 	u32 pkey_allocation_map;
+	s16 execute_only_pkey; /* key holding execute-only protection */
 #endif
 } mm_context_t;
 
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index dac8751..5b17d93 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -126,11 +126,15 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
  * Try to dedicate one of the protection keys to be used as an
  * execute-only protection key.
  */
+extern int __execute_only_pkey(struct mm_struct *mm);
 static inline int execute_only_pkey(struct mm_struct *mm)
 {
-	return 0;
+	if (!pkey_inited)
+		return -1;
+	return __execute_only_pkey(mm);
 }
 
+
 static inline int arch_override_mprotect_pkey(struct vm_area_struct *vma,
 		int prot, int pkey)
 {
@@ -157,6 +161,8 @@ static inline void pkey_mm_init(struct mm_struct *mm)
 	if (!pkey_inited)
 		return;
 	mm_pkey_allocation_map(mm) = initial_allocation_mask;
+	/* -1 means unallocated or invalid */
+	mm->context.execute_only_pkey = -1;
 }
 
 static inline void pkey_initialize(void)
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index c1c40c3..25749bf 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -97,3 +97,60 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 	init_iamr(pkey, new_iamr_bits);
 	return 0;
 }
+
+static inline bool pkey_allows_readwrite(int pkey)
+{
+	int pkey_shift = pkeyshift(pkey);
+
+	if (!(read_uamor() & (0x3UL << pkey_shift)))
+		return true;
+
+	return !(read_amr() & ((AMR_RD_BIT|AMR_WR_BIT) << pkey_shift));
+}
+
+int __execute_only_pkey(struct mm_struct *mm)
+{
+	bool need_to_set_mm_pkey = false;
+	int execute_only_pkey = mm->context.execute_only_pkey;
+	int ret;
+
+	/* Do we need to assign a pkey for mm's execute-only maps? */
+	if (execute_only_pkey == -1) {
+		/* Go allocate one to use, which might fail */
+		execute_only_pkey = mm_pkey_alloc(mm);
+		if (execute_only_pkey < 0)
+			return -1;
+		need_to_set_mm_pkey = true;
+	}
+
+	/*
+	 * We do not want to go through the relatively costly
+	 * dance to set AMR if we do not need to.  Check it
+	 * first and assume that if the execute-only pkey is
+	 * readwrite-disabled than we do not have to set it
+	 * ourselves.
+	 */
+	if (!need_to_set_mm_pkey &&
+	    !pkey_allows_readwrite(execute_only_pkey))
+		return execute_only_pkey;
+
+	/*
+	 * Set up AMR so that it denies access for everything
+	 * other than execution.
+	 */
+	ret = __arch_set_user_pkey_access(current, execute_only_pkey,
+			(PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE));
+	/*
+	 * If the AMR-set operation failed somehow, just return
+	 * 0 and effectively disable execute-only support.
+	 */
+	if (ret) {
+		mm_set_pkey_free(mm, execute_only_pkey);
+		return -1;
+	}
+
+	/* We got one, store it and use it from here on out */
+	if (need_to_set_mm_pkey)
+		mm->context.execute_only_pkey = execute_only_pkey;
+	return execute_only_pkey;
+}
-- 
1.7.1

[RFC v7 11/25] powerpc: ability to associate pkey to a vma

[Ram Pai]

arch-independent code expects the arch to  map
a  pkey  into the vma's protection bit setting.
The patch provides that ability.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/mman.h  |    8 +++++++-
 arch/powerpc/include/asm/pkeys.h |   12 ++++++++++++
 2 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
index 30922f6..067eec2 100644
--- a/arch/powerpc/include/asm/mman.h
+++ b/arch/powerpc/include/asm/mman.h
@@ -13,6 +13,7 @@
 
 #include <asm/cputable.h>
 #include <linux/mm.h>
+#include <linux/pkeys.h>
 #include <asm/cpu_has_feature.h>
 
 /*
@@ -22,7 +23,12 @@
 static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot,
 		unsigned long pkey)
 {
-	return (prot & PROT_SAO) ? VM_SAO : 0;
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	return (((prot & PROT_SAO) ? VM_SAO : 0) |
+			pkey_to_vmflag_bits(pkey));
+#else
+	return ((prot & PROT_SAO) ? VM_SAO : 0);
+#endif
 }
 #define arch_calc_vm_prot_bits(prot, pkey) arch_calc_vm_prot_bits(prot, pkey)
 
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 5b17d93..a715a08 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -34,6 +34,18 @@
 				PKEY_DISABLE_WRITE  |\
 				PKEY_DISABLE_EXECUTE)
 
+static inline u64 pkey_to_vmflag_bits(u16 pkey)
+{
+	if (!pkey_inited)
+		return 0x0UL;
+
+	return (((pkey & 0x1UL) ? VM_PKEY_BIT0 : 0x0UL) |
+		((pkey & 0x2UL) ? VM_PKEY_BIT1 : 0x0UL) |
+		((pkey & 0x4UL) ? VM_PKEY_BIT2 : 0x0UL) |
+		((pkey & 0x8UL) ? VM_PKEY_BIT3 : 0x0UL) |
+		((pkey & 0x10UL) ? VM_PKEY_BIT4 : 0x0UL));
+}
+
 #define arch_max_pkey()  pkeys_total
 #define AMR_RD_BIT 0x1UL
 #define AMR_WR_BIT 0x2UL
-- 
1.7.1

[RFC v7 12/25] powerpc: implementation for arch_override_mprotect_pkey()

[Ram Pai]

arch independent code calls arch_override_mprotect_pkey()
to return a pkey that best matches the requested protection.

This patch provides the implementation.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/mmu_context.h |    5 +++
 arch/powerpc/include/asm/pkeys.h       |   17 ++++++++++-
 arch/powerpc/mm/pkeys.c                |   47 ++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 4705dab..7232484 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -185,6 +185,11 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
 #ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
 #define pkey_initialize()
 #define pkey_mm_init(mm)
+
+static inline int vma_pkey(struct vm_area_struct *vma)
+{
+	return 0;
+}
 #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
 
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index a715a08..03f7ea2 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -46,6 +46,16 @@ static inline u64 pkey_to_vmflag_bits(u16 pkey)
 		((pkey & 0x10UL) ? VM_PKEY_BIT4 : 0x0UL));
 }
 
+#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
+				VM_PKEY_BIT3 | VM_PKEY_BIT4)
+
+static inline int vma_pkey(struct vm_area_struct *vma)
+{
+	if (!pkey_inited)
+		return 0;
+	return (vma->vm_flags & ARCH_VM_PKEY_FLAGS) >> VM_PKEY_SHIFT;
+}
+
 #define arch_max_pkey()  pkeys_total
 #define AMR_RD_BIT 0x1UL
 #define AMR_WR_BIT 0x2UL
@@ -146,11 +156,14 @@ static inline int execute_only_pkey(struct mm_struct *mm)
 	return __execute_only_pkey(mm);
 }
 
-
+extern int __arch_override_mprotect_pkey(struct vm_area_struct *vma,
+		int prot, int pkey);
 static inline int arch_override_mprotect_pkey(struct vm_area_struct *vma,
 		int prot, int pkey)
 {
-	return 0;
+	if (!pkey_inited)
+		return 0;
+	return __arch_override_mprotect_pkey(vma, prot, pkey);
 }
 
 extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index 25749bf..edcbf48 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -154,3 +154,50 @@ int __execute_only_pkey(struct mm_struct *mm)
 		mm->context.execute_only_pkey = execute_only_pkey;
 	return execute_only_pkey;
 }
+
+static inline bool vma_is_pkey_exec_only(struct vm_area_struct *vma)
+{
+	/* Do this check first since the vm_flags should be hot */
+	if ((vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)) != VM_EXEC)
+		return false;
+
+	return (vma_pkey(vma) == vma->vm_mm->context.execute_only_pkey);
+}
+
+/*
+ * This should only be called for *plain* mprotect calls.
+ */
+int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot,
+		int pkey)
+{
+	/*
+	 * Is this an mprotect_pkey() call?  If so, never
+	 * override the value that came from the user.
+	 */
+	if (pkey != -1)
+		return pkey;
+
+	/*
+	 * If the currently associated pkey is execute-only,
+	 * but the requested protection requires read or write,
+	 * move it back to the default pkey.
+	 */
+	if (vma_is_pkey_exec_only(vma) &&
+	    (prot & (PROT_READ|PROT_WRITE)))
+		return 0;
+
+	/*
+	 * the requested protection is execute-only. Hence
+	 * lets use a execute-only pkey.
+	 */
+	if (prot == PROT_EXEC) {
+		pkey = execute_only_pkey(vma->vm_mm);
+		if (pkey > 0)
+			return pkey;
+	}
+
+	/*
+	 * nothing to override.
+	 */
+	return vma_pkey(vma);
+}
-- 
1.7.1

[RFC v7 13/25] powerpc: map vma key-protection bits to pte key bits.

[Ram Pai]

map  the  key  protection  bits of the vma to the pkey bits in
the PTE.

The Pte  bits used  for pkey  are  3,4,5,6  and 57. The  first
four bits are the same four bits that were freed up  initially
in this patch series. remember? :-) Without those four bits
this patch would'nt be possible.

BUT, On 4k kernel, bit 3, and 4 could not be freed up. remember?
Hence we have to be satisfied with 5,6 and 7.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/pgtable.h |   25 ++++++++++++++++++++++++-
 arch/powerpc/include/asm/mman.h              |    8 ++++++++
 arch/powerpc/include/asm/pkeys.h             |   12 ++++++++++++
 3 files changed, 44 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index d4da0e9..060a1b2 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -37,6 +37,7 @@
 #define _RPAGE_RSV2		0x0800000000000000UL
 #define _RPAGE_RSV3		0x0400000000000000UL
 #define _RPAGE_RSV4		0x0200000000000000UL
+#define _RPAGE_RSV5		0x00040UL
 
 #define _PAGE_PTE		0x4000000000000000UL	/* distinguishes PTEs from pointers */
 #define _PAGE_PRESENT		0x8000000000000000UL	/* pte contains a translation */
@@ -56,6 +57,25 @@
 /* Max physical address bit as per radix table */
 #define _RPAGE_PA_MAX		57
 
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+#ifdef CONFIG_PPC_64K_PAGES
+#define H_PAGE_PKEY_BIT0	_RPAGE_RSV1
+#define H_PAGE_PKEY_BIT1	_RPAGE_RSV2
+#else /* CONFIG_PPC_64K_PAGES */
+#define H_PAGE_PKEY_BIT0	0 /* _RPAGE_RSV1 is not available */
+#define H_PAGE_PKEY_BIT1	0 /* _RPAGE_RSV2 is not available */
+#endif /* CONFIG_PPC_64K_PAGES */
+#define H_PAGE_PKEY_BIT2	_RPAGE_RSV3
+#define H_PAGE_PKEY_BIT3	_RPAGE_RSV4
+#define H_PAGE_PKEY_BIT4	_RPAGE_RSV5
+#else /*  CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+#define H_PAGE_PKEY_BIT0	0
+#define H_PAGE_PKEY_BIT1	0
+#define H_PAGE_PKEY_BIT2	0
+#define H_PAGE_PKEY_BIT3	0
+#define H_PAGE_PKEY_BIT4	0
+#endif /*  CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+
 /*
  * Max physical address bit we will use for now.
  *
@@ -116,13 +136,16 @@
 #define _PAGE_CHG_MASK	(PTE_RPN_MASK | _PAGE_HPTEFLAGS | _PAGE_DIRTY | \
 			 _PAGE_ACCESSED | _PAGE_SPECIAL | _PAGE_PTE |	\
 			 _PAGE_SOFT_DIRTY)
+
+#define H_PAGE_PKEY  (H_PAGE_PKEY_BIT0 | H_PAGE_PKEY_BIT1 | H_PAGE_PKEY_BIT2 | \
+			H_PAGE_PKEY_BIT3 | H_PAGE_PKEY_BIT4)
 /*
  * Mask of bits returned by pte_pgprot()
  */
 #define PAGE_PROT_BITS  (_PAGE_SAO | _PAGE_NON_IDEMPOTENT | _PAGE_TOLERANT | \
 			 H_PAGE_4K_PFN | _PAGE_PRIVILEGED | _PAGE_ACCESSED | \
 			 _PAGE_READ | _PAGE_WRITE |  _PAGE_DIRTY | _PAGE_EXEC | \
-			 _PAGE_SOFT_DIRTY)
+			 _PAGE_SOFT_DIRTY | H_PAGE_PKEY)
 /*
  * We define 2 sets of base prot bits, one for basic pages (ie,
  * cacheable kernel and user pages) and one for non cacheable
diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
index 067eec2..3f7220f 100644
--- a/arch/powerpc/include/asm/mman.h
+++ b/arch/powerpc/include/asm/mman.h
@@ -32,12 +32,20 @@ static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot,
 }
 #define arch_calc_vm_prot_bits(prot, pkey) arch_calc_vm_prot_bits(prot, pkey)
 
+
 static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
 {
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	return (vm_flags & VM_SAO) ?
+		__pgprot(_PAGE_SAO | vmflag_to_page_pkey_bits(vm_flags)) :
+		__pgprot(0 | vmflag_to_page_pkey_bits(vm_flags));
+#else
 	return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
+#endif
 }
 #define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
 
+
 static inline bool arch_validate_prot(unsigned long prot)
 {
 	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 03f7ea2..1ded6df 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -46,6 +46,18 @@ static inline u64 pkey_to_vmflag_bits(u16 pkey)
 		((pkey & 0x10UL) ? VM_PKEY_BIT4 : 0x0UL));
 }
 
+static inline u64 vmflag_to_page_pkey_bits(u64 vm_flags)
+{
+	if (!pkey_inited)
+		return 0x0UL;
+
+	return (((vm_flags & VM_PKEY_BIT0) ? H_PAGE_PKEY_BIT4 : 0x0UL) |
+		((vm_flags & VM_PKEY_BIT1) ? H_PAGE_PKEY_BIT3 : 0x0UL) |
+		((vm_flags & VM_PKEY_BIT2) ? H_PAGE_PKEY_BIT2 : 0x0UL) |
+		((vm_flags & VM_PKEY_BIT3) ? H_PAGE_PKEY_BIT1 : 0x0UL) |
+		((vm_flags & VM_PKEY_BIT4) ? H_PAGE_PKEY_BIT0 : 0x0UL));
+}
+
 #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
 				VM_PKEY_BIT3 | VM_PKEY_BIT4)
 
-- 
1.7.1

[RFC v7 14/25] powerpc: sys_pkey_mprotect() system call

[Ram Pai]

Patch provides the ability for a process to
associate a pkey with a address range.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/systbl.h      |    1 +
 arch/powerpc/include/asm/unistd.h      |    4 +---
 arch/powerpc/include/uapi/asm/unistd.h |    1 +
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index 22dd776..b33b551 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -390,3 +390,4 @@
 SYSCALL(statx)
 SYSCALL(pkey_alloc)
 SYSCALL(pkey_free)
+SYSCALL(pkey_mprotect)
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index e0273bc..daf1ba9 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -12,12 +12,10 @@
 #include <uapi/asm/unistd.h>
 
 
-#define NR_syscalls		386
+#define NR_syscalls		387
 
 #define __NR__exit __NR_exit
 
-#define __IGNORE_pkey_mprotect
-
 #ifndef __ASSEMBLY__
 
 #include <linux/types.h>
diff --git a/arch/powerpc/include/uapi/asm/unistd.h b/arch/powerpc/include/uapi/asm/unistd.h
index 7993a07..71ae45e 100644
--- a/arch/powerpc/include/uapi/asm/unistd.h
+++ b/arch/powerpc/include/uapi/asm/unistd.h
@@ -396,5 +396,6 @@
 #define __NR_statx		383
 #define __NR_pkey_alloc		384
 #define __NR_pkey_free		385
+#define __NR_pkey_mprotect	386
 
 #endif /* _UAPI_ASM_POWERPC_UNISTD_H_ */
-- 
1.7.1

[RFC v7 15/25] powerpc: Program HPTE key protection bits

[Ram Pai]

Map the PTE protection key bits to the HPTE key protection bits,
while creating HPTE  entries.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/mmu-hash.h |    5 +++++
 arch/powerpc/include/asm/mmu_context.h        |    6 ++++++
 arch/powerpc/include/asm/pkeys.h              |   13 +++++++++++++
 arch/powerpc/mm/hash_utils_64.c               |    1 +
 4 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
index 6981a52..f7a6ed3 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
@@ -90,6 +90,8 @@
 #define HPTE_R_PP0		ASM_CONST(0x8000000000000000)
 #define HPTE_R_TS		ASM_CONST(0x4000000000000000)
 #define HPTE_R_KEY_HI		ASM_CONST(0x3000000000000000)
+#define HPTE_R_KEY_BIT0		ASM_CONST(0x2000000000000000)
+#define HPTE_R_KEY_BIT1		ASM_CONST(0x1000000000000000)
 #define HPTE_R_RPN_SHIFT	12
 #define HPTE_R_RPN		ASM_CONST(0x0ffffffffffff000)
 #define HPTE_R_RPN_3_0		ASM_CONST(0x01fffffffffff000)
@@ -104,6 +106,9 @@
 #define HPTE_R_C		ASM_CONST(0x0000000000000080)
 #define HPTE_R_R		ASM_CONST(0x0000000000000100)
 #define HPTE_R_KEY_LO		ASM_CONST(0x0000000000000e00)
+#define HPTE_R_KEY_BIT2		ASM_CONST(0x0000000000000800)
+#define HPTE_R_KEY_BIT3		ASM_CONST(0x0000000000000400)
+#define HPTE_R_KEY_BIT4		ASM_CONST(0x0000000000000200)
 
 #define HPTE_V_1TB_SEG		ASM_CONST(0x4000000000000000)
 #define HPTE_V_VRMA_MASK	ASM_CONST(0x4001ffffff000000)
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 7232484..2eb7f80 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -190,6 +190,12 @@ static inline int vma_pkey(struct vm_area_struct *vma)
 {
 	return 0;
 }
+
+static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
+{
+	return 0x0UL;
+}
+
 #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
 
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 1ded6df..4b7e3f4 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -72,6 +72,19 @@ static inline int vma_pkey(struct vm_area_struct *vma)
 #define AMR_RD_BIT 0x1UL
 #define AMR_WR_BIT 0x2UL
 #define IAMR_EX_BIT 0x1UL
+
+static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
+{
+	if (!pkey_inited)
+		return 0x0UL;
+
+	return (((pteflags & H_PAGE_PKEY_BIT0) ? HPTE_R_KEY_BIT0 : 0x0UL) |
+		((pteflags & H_PAGE_PKEY_BIT1) ? HPTE_R_KEY_BIT1 : 0x0UL) |
+		((pteflags & H_PAGE_PKEY_BIT2) ? HPTE_R_KEY_BIT2 : 0x0UL) |
+		((pteflags & H_PAGE_PKEY_BIT3) ? HPTE_R_KEY_BIT3 : 0x0UL) |
+		((pteflags & H_PAGE_PKEY_BIT4) ? HPTE_R_KEY_BIT4 : 0x0UL));
+}
+
 #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
 				VM_PKEY_BIT3 | VM_PKEY_BIT4)
 #define AMR_BITS_PER_PKEY 2
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index f88423b..545f291 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -231,6 +231,7 @@ unsigned long htab_convert_pte_flags(unsigned long pteflags)
 		 */
 		rflags |= HPTE_R_M;
 
+	rflags |= pte_to_hpte_pkey_bits(pteflags);
 	return rflags;
 }
 
-- 
1.7.1

[RFC v7 16/25] powerpc: helper to validate key-access permissions of a pte

[Ram Pai]

helper function that checks if the read/write/execute is allowed
on the pte.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/pgtable.h |    4 +++
 arch/powerpc/include/asm/pkeys.h             |   12 +++++++++++
 arch/powerpc/mm/pkeys.c                      |   28 ++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 060a1b2..2bec0f6 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -477,6 +477,10 @@ static inline void write_uamor(u64 value)
 	mtspr(SPRN_UAMOR, value);
 }
 
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+extern bool arch_pte_access_permitted(u64 pte, bool write, bool execute);
+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+
 #define __HAVE_ARCH_PTEP_GET_AND_CLEAR
 static inline pte_t ptep_get_and_clear(struct mm_struct *mm,
 				       unsigned long addr, pte_t *ptep)
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 4b7e3f4..ba7bff6 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -85,6 +85,18 @@ static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
 		((pteflags & H_PAGE_PKEY_BIT4) ? HPTE_R_KEY_BIT4 : 0x0UL));
 }
 
+static inline u16 pte_to_pkey_bits(u64 pteflags)
+{
+	if (!pkey_inited)
+		return 0x0UL;
+
+	return (((pteflags & H_PAGE_PKEY_BIT0) ? 0x10 : 0x0UL) |
+		((pteflags & H_PAGE_PKEY_BIT1) ? 0x8 : 0x0UL) |
+		((pteflags & H_PAGE_PKEY_BIT2) ? 0x4 : 0x0UL) |
+		((pteflags & H_PAGE_PKEY_BIT3) ? 0x2 : 0x0UL) |
+		((pteflags & H_PAGE_PKEY_BIT4) ? 0x1 : 0x0UL));
+}
+
 #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
 				VM_PKEY_BIT3 | VM_PKEY_BIT4)
 #define AMR_BITS_PER_PKEY 2
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index edcbf48..8d756ef 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -201,3 +201,31 @@ int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot,
 	 */
 	return vma_pkey(vma);
 }
+
+static bool pkey_access_permitted(int pkey, bool write, bool execute)
+{
+	int pkey_shift;
+	u64 amr;
+
+	if (!pkey)
+		return true;
+
+	pkey_shift = pkeyshift(pkey);
+	if (!(read_uamor() & (0x3UL << pkey_shift)))
+		return true;
+
+	if (execute && !(read_iamr() & (IAMR_EX_BIT << pkey_shift)))
+		return true;
+
+	amr = read_amr(); /* delay reading amr uptil absolutely needed*/
+	return ((!write && !(amr & (AMR_RD_BIT << pkey_shift))) ||
+		(write &&  !(amr & (AMR_WR_BIT << pkey_shift))));
+}
+
+bool arch_pte_access_permitted(u64 pte, bool write, bool execute)
+{
+	if (!pkey_inited)
+		return true;
+	return pkey_access_permitted(pte_to_pkey_bits(pte),
+			write, execute);
+}
-- 
1.7.1

[RFC v7 17/25] powerpc: check key protection for user page access

[Ram Pai]

Make sure that the kernel does not access user pages without
checking their key-protection.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/pgtable.h |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 2bec0f6..d1e1460 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -479,6 +479,20 @@ static inline void write_uamor(u64 value)
 
 #ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
 extern bool arch_pte_access_permitted(u64 pte, bool write, bool execute);
+
+#define pte_access_permitted(pte, write) \
+	(pte_present(pte) && \
+	 ((!(write) || pte_write(pte)) && \
+	  arch_pte_access_permitted(pte_val(pte), !!write, 0)))
+
+/*
+ * We store key in pmd for huge tlb pages. So need
+ * to check for key protection.
+ */
+#define pmd_access_permitted(pmd, write) \
+	(pmd_present(pmd) && \
+	 ((!(write) || pmd_write(pmd)) && \
+	  arch_pte_access_permitted(pmd_val(pmd), !!write, 0)))
 #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
 
 #define __HAVE_ARCH_PTEP_GET_AND_CLEAR
-- 
1.7.1

[RFC v7 18/25] powerpc: Macro the mask used for checking DSI exception

[Ram Pai]

Replace the magic number used to check for DSI exception
with a meaningful value.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/reg.h       |    7 ++++++-
 arch/powerpc/kernel/exceptions-64s.S |    2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 7e50e47..ee04bc0 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -272,16 +272,21 @@
 #define SPRN_DAR	0x013	/* Data Address Register */
 #define SPRN_DBCR	0x136	/* e300 Data Breakpoint Control Reg */
 #define SPRN_DSISR	0x012	/* Data Storage Interrupt Status Register */
+#define   DSISR_BIT32		0x80000000	/* not defined */
 #define   DSISR_NOHPTE		0x40000000	/* no translation found */
+#define   DSISR_PAGEATTR_CONFLT	0x20000000	/* page attribute conflict */
+#define   DSISR_BIT35		0x10000000	/* not defined */
 #define   DSISR_PROTFAULT	0x08000000	/* protection fault */
 #define   DSISR_BADACCESS	0x04000000	/* bad access to CI or G */
 #define   DSISR_ISSTORE		0x02000000	/* access was a store */
 #define   DSISR_DABRMATCH	0x00400000	/* hit data breakpoint */
-#define   DSISR_NOSEGMENT	0x00200000	/* SLB miss */
 #define   DSISR_KEYFAULT	0x00200000	/* Key fault */
+#define   DSISR_BIT43		0x00100000	/* not defined */
 #define   DSISR_UNSUPP_MMU	0x00080000	/* Unsupported MMU config */
 #define   DSISR_SET_RC		0x00040000	/* Failed setting of R/C bits */
 #define   DSISR_PGDIRFAULT      0x00020000      /* Fault on page directory */
+#define   DSISR_PAGE_FAULT_MASK (DSISR_BIT32 | DSISR_PAGEATTR_CONFLT | \
+				DSISR_BADACCESS | DSISR_DABRMATCH | DSISR_BIT43)
 #define SPRN_TBRL	0x10C	/* Time Base Read Lower Register (user, R/O) */
 #define SPRN_TBRU	0x10D	/* Time Base Read Upper Register (user, R/O) */
 #define SPRN_CIR	0x11B	/* Chip Information Register (hyper, R/0) */
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index b886795..e154bfe 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1411,7 +1411,7 @@ USE_TEXT_SECTION()
 	.balign	IFETCH_ALIGN_BYTES
 do_hash_page:
 #ifdef CONFIG_PPC_STD_MMU_64
-	andis.	r0,r4,0xa450		/* weird error? */
+	andis.	r0,r4,DSISR_PAGE_FAULT_MASK@h
 	bne-	handle_page_fault	/* if not, try to insert a HPTE */
 	CURRENT_THREAD_INFO(r11, r1)
 	lwz	r0,TI_PREEMPT(r11)	/* If we're in an "NMI" */
-- 
1.7.1

[RFC v7 19/25] powerpc: implementation for arch_vma_access_permitted()

[Ram Pai]

This patch provides the implementation for
arch_vma_access_permitted(). Returns true if the
requested access is allowed by pkey associated with the
vma.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/mmu_context.h |    5 +++-
 arch/powerpc/mm/pkeys.c                |   43 ++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 2eb7f80..a1cfcca 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -175,6 +175,10 @@ static inline void arch_bprm_mm_init(struct mm_struct *mm,
 {
 }
 
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+bool arch_vma_access_permitted(struct vm_area_struct *vma,
+			bool write, bool execute, bool foreign);
+#else /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
 static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
 		bool write, bool execute, bool foreign)
 {
@@ -182,7 +186,6 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
 	return true;
 }
 
-#ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
 #define pkey_initialize()
 #define pkey_mm_init(mm)
 
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index 8d756ef..1424c79 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -229,3 +229,46 @@ bool arch_pte_access_permitted(u64 pte, bool write, bool execute)
 	return pkey_access_permitted(pte_to_pkey_bits(pte),
 			write, execute);
 }
+
+/*
+ * We only want to enforce protection keys on the current process
+ * because we effectively have no access to AMR/IAMR for other
+ * processes or any way to tell *which * AMR/IAMR in a threaded
+ * process we could use.
+ *
+ * So do not enforce things if the VMA is not from the current
+ * mm, or if we are in a kernel thread.
+ */
+static inline bool vma_is_foreign(struct vm_area_struct *vma)
+{
+	if (!current->mm)
+		return true;
+	/*
+	 * if the VMA is from another process, then AMR/IAMR has no
+	 * relevance and should not be enforced.
+	 */
+	if (current->mm != vma->vm_mm)
+		return true;
+
+	return false;
+}
+
+bool arch_vma_access_permitted(struct vm_area_struct *vma,
+		bool write, bool execute, bool foreign)
+{
+	int pkey;
+
+	if (!pkey_inited)
+		return true;
+
+	/* allow access if the VMA is not one from this process */
+	if (foreign || vma_is_foreign(vma))
+		return true;
+
+	pkey = vma_pkey(vma);
+
+	if (!pkey)
+		return true;
+
+	return pkey_access_permitted(pkey, write, execute);
+}
-- 
1.7.1

[RFC v7 20/25] powerpc: Handle exceptions caused by pkey violation

[Ram Pai]

Handle Data and  Instruction exceptions caused by memory
protection-key.

The CPU will detect the key fault if the HPTE is already
programmed with the key.

However if the HPTE is not  hashed, a key fault will not
be detected by the  hardware. The   software will detect
pkey violation in such a case.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/reg.h |    3 ++-
 arch/powerpc/mm/fault.c        |   21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index ee04bc0..b7cbc8c 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -286,7 +286,8 @@
 #define   DSISR_SET_RC		0x00040000	/* Failed setting of R/C bits */
 #define   DSISR_PGDIRFAULT      0x00020000      /* Fault on page directory */
 #define   DSISR_PAGE_FAULT_MASK (DSISR_BIT32 | DSISR_PAGEATTR_CONFLT | \
-				DSISR_BADACCESS | DSISR_DABRMATCH | DSISR_BIT43)
+				DSISR_BADACCESS | DSISR_KEYFAULT | \
+				DSISR_DABRMATCH | DSISR_BIT43)
 #define SPRN_TBRL	0x10C	/* Time Base Read Lower Register (user, R/O) */
 #define SPRN_TBRU	0x10D	/* Time Base Read Upper Register (user, R/O) */
 #define SPRN_CIR	0x11B	/* Chip Information Register (hyper, R/0) */
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 3a7d580..ea74fe2 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -261,6 +261,13 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
 	}
 #endif
 
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	if (error_code & DSISR_KEYFAULT) {
+		code = SEGV_PKUERR;
+		goto bad_area_nosemaphore;
+	}
+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+
 	/* We restore the interrupt state now */
 	if (!arch_irq_disabled_regs(regs))
 		local_irq_enable();
@@ -441,6 +448,20 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
 		WARN_ON_ONCE(error_code & DSISR_PROTFAULT);
 #endif /* CONFIG_PPC_STD_MMU */
 
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	if (!arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
+			is_exec, 0)) {
+		code = SEGV_PKUERR;
+		goto bad_area;
+	}
+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+
+
+	/* handle_mm_fault() needs to know if its a instruction access
+	 * fault.
+	 */
+	if (is_exec)
+		flags |= FAULT_FLAG_INSTRUCTION;
 	/*
 	 * If for any reason at all we couldn't handle the fault,
 	 * make sure we exit gracefully rather than endlessly redo
-- 
1.7.1

[RFC v7 21/25] powerpc: capture AMR register content on pkey violation

[Ram Pai]

capture AMR register contents, and save it in paca
whenever a pkey violation is detected.

This value will be needed to deliver pkey-violation
signal to the task.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/paca.h   |    3 +++
 arch/powerpc/kernel/asm-offsets.c |    5 +++++
 arch/powerpc/mm/fault.c           |    2 ++
 3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 1c09f8f..c8bd1fc 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -92,6 +92,9 @@ struct paca_struct {
 	struct dtl_entry *dispatch_log_end;
 #endif /* CONFIG_PPC_STD_MMU_64 */
 	u64 dscr_default;		/* per-CPU default DSCR */
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	u64 paca_amr;			/* value of amr at exception */
+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
 
 #ifdef CONFIG_PPC_STD_MMU_64
 	/*
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 709e234..17f5d8a 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -241,6 +241,11 @@ int main(void)
 	OFFSET(PACAHWCPUID, paca_struct, hw_cpu_id);
 	OFFSET(PACAKEXECSTATE, paca_struct, kexec_state);
 	OFFSET(PACA_DSCR_DEFAULT, paca_struct, dscr_default);
+
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	OFFSET(PACA_AMR, paca_struct, paca_amr);
+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+
 	OFFSET(ACCOUNT_STARTTIME, paca_struct, accounting.starttime);
 	OFFSET(ACCOUNT_STARTTIME_USER, paca_struct, accounting.starttime_user);
 	OFFSET(ACCOUNT_USER_TIME, paca_struct, accounting.utime);
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index ea74fe2..a6710f5 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -264,6 +264,7 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
 #ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
 	if (error_code & DSISR_KEYFAULT) {
 		code = SEGV_PKUERR;
+		get_paca()->paca_amr = read_amr();
 		goto bad_area_nosemaphore;
 	}
 #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
@@ -451,6 +452,7 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
 #ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
 	if (!arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
 			is_exec, 0)) {
+		get_paca()->paca_amr = read_amr();
 		code = SEGV_PKUERR;
 		goto bad_area;
 	}
-- 
1.7.1

[RFC v7 22/25] powerpc: introduce get_pte_pkey() helper

[Ram Pai]

get_pte_pkey() helper returns the pkey associated with
a address corresponding to a given mm_struct.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/mmu-hash.h |    5 +++++
 arch/powerpc/mm/hash_utils_64.c               |   25 +++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
index f7a6ed3..369f9ff 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
@@ -450,6 +450,11 @@ extern int hash_page(unsigned long ea, unsigned long access, unsigned long trap,
 int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
 		     pte_t *ptep, unsigned long trap, unsigned long flags,
 		     int ssize, unsigned int shift, unsigned int mmu_psize);
+
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+u16 get_pte_pkey(struct mm_struct *mm, unsigned long address);
+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 extern int __hash_page_thp(unsigned long ea, unsigned long access,
 			   unsigned long vsid, pmd_t *pmdp, unsigned long trap,
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index 545f291..44ebf2a 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -1570,6 +1570,31 @@ void hash_preload(struct mm_struct *mm, unsigned long ea,
 	local_irq_restore(flags);
 }
 
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+/*
+ * return the protection key associated with the given address
+ * and the mm_struct.
+ */
+u16 get_pte_pkey(struct mm_struct *mm, unsigned long address)
+{
+	pte_t *ptep;
+	u16 pkey = 0;
+	unsigned long flags;
+
+	if (!mm || !mm->pgd)
+		return 0;
+
+	local_irq_save(flags);
+	ptep = find_linux_pte_or_hugepte(mm->pgd, address,
+			NULL, NULL);
+	if (ptep)
+		pkey = pte_to_pkey_bits(pte_val(READ_ONCE(*ptep)));
+	local_irq_restore(flags);
+
+	return pkey;
+}
+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 static inline void tm_flush_hash_page(int local)
 {
-- 
1.7.1

[RFC v7 23/25] powerpc: capture the violated protection key on fault

[Ram Pai]

Capture the protection key that got violated in paca.
This value will be later used to inform the signal
handler.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/paca.h   |    1 +
 arch/powerpc/kernel/asm-offsets.c |    1 +
 arch/powerpc/mm/fault.c           |    8 ++++++++
 3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index c8bd1fc..0c06188 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -94,6 +94,7 @@ struct paca_struct {
 	u64 dscr_default;		/* per-CPU default DSCR */
 #ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
 	u64 paca_amr;			/* value of amr at exception */
+	u16 paca_pkey;                  /* exception causing pkey */
 #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
 
 #ifdef CONFIG_PPC_STD_MMU_64
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 17f5d8a..7dff862 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -244,6 +244,7 @@ int main(void)
 
 #ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
 	OFFSET(PACA_AMR, paca_struct, paca_amr);
+	OFFSET(PACA_PKEY, paca_struct, paca_pkey);
 #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
 
 	OFFSET(ACCOUNT_STARTTIME, paca_struct, accounting.starttime);
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index a6710f5..7fee303 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -265,6 +265,7 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
 	if (error_code & DSISR_KEYFAULT) {
 		code = SEGV_PKUERR;
 		get_paca()->paca_amr = read_amr();
+		get_paca()->paca_pkey = get_pte_pkey(current->mm, address);
 		goto bad_area_nosemaphore;
 	}
 #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
@@ -453,6 +454,13 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
 	if (!arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
 			is_exec, 0)) {
 		get_paca()->paca_amr = read_amr();
+		/*
+		 * The pgd-pdt...pmd-pte tree may not  have  been fully setup.
+		 * Hence we cannot walk the tree to locate the pte, to locate
+		 * the key. Hence lets use vma_pkey() to get the key; instead
+		 * of get_pte_pkey().
+		 */
+		get_paca()->paca_pkey = vma_pkey(vma);
 		code = SEGV_PKUERR;
 		goto bad_area;
 	}
-- 
1.7.1

[RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation

[Ram Pai]

The value of the AMR register at the time of exception
is made available in gp_regs[PT_AMR] of the siginfo.

The value of the pkey, whose protection got violated,
is made available in si_pkey field of the siginfo structure.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/uapi/asm/ptrace.h |    1 +
 arch/powerpc/kernel/signal_32.c        |    5 +++++
 arch/powerpc/kernel/signal_64.c        |    4 ++++
 arch/powerpc/kernel/traps.c            |   15 +++++++++++++++
 4 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
index 8036b38..fc9c9c0 100644
--- a/arch/powerpc/include/uapi/asm/ptrace.h
+++ b/arch/powerpc/include/uapi/asm/ptrace.h
@@ -110,6 +110,7 @@ struct pt_regs {
 #define PT_RESULT 43
 #define PT_DSCR 44
 #define PT_REGS_COUNT 44
+#define PT_AMR	45
 
 #define PT_FPR0	48	/* each FP reg occupies 2 slots in this space */
 
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 97bb138..9c4a7f3 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -500,6 +500,11 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
 				   (unsigned long) &frame->tramp[2]);
 	}
 
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	if (__put_user(get_paca()->paca_amr, &frame->mc_gregs[PT_AMR]))
+		return 1;
+#endif /*  CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+
 	return 0;
 }
 
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index c83c115..86a4262 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -174,6 +174,10 @@ static long setup_sigcontext(struct sigcontext __user *sc,
 	if (set != NULL)
 		err |=  __put_user(set->sig[0], &sc->oldmask);
 
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	err |= __put_user(get_paca()->paca_amr, &sc->gp_regs[PT_AMR]);
+#endif /*  CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+
 	return err;
 }
 
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index d4e545d..fe1e7c7 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -20,6 +20,7 @@
 #include <linux/sched/debug.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
+#include <linux/pkeys.h>
 #include <linux/stddef.h>
 #include <linux/unistd.h>
 #include <linux/ptrace.h>
@@ -247,6 +248,15 @@ void user_single_step_siginfo(struct task_struct *tsk,
 	info->si_addr = (void __user *)regs->nip;
 }
 
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+static void fill_sig_info_pkey(int si_code, siginfo_t *info, unsigned long addr)
+{
+	if (si_code != SEGV_PKUERR)
+		return;
+	info->si_pkey = get_paca()->paca_pkey;
+}
+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+
 void _exception(int signr, struct pt_regs *regs, int code, unsigned long addr)
 {
 	siginfo_t info;
@@ -274,6 +284,11 @@ void _exception(int signr, struct pt_regs *regs, int code, unsigned long addr)
 	info.si_signo = signr;
 	info.si_code = code;
 	info.si_addr = (void __user *) addr;
+
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+	fill_sig_info_pkey(code, &info, addr);
+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+
 	force_sig_info(signr, &info, current);
 }
 
-- 
1.7.1

[RFC v7 25/25] powerpc: Enable pkey subsystem

[Ram Pai]

PAPR defines 'ibm,processor-storage-keys' property. It exports
two values.The first value indicates the number of data-access
keys and the second indicates the number of instruction-access
keys. Though this alludes  that keys can be either data access
or instructon access only, that is not the case in reality.Any
key can be of any kind.  This patch adds all the keys and uses
that as the total number of keys available to us.

Non PAPR platforms do not define this property   in the device
tree yet. Here, we   hardcode   CPUs   that   support  pkey by
consulting PowerISA3.0

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/cputable.h    |    8 +++++---
 arch/powerpc/include/asm/mmu_context.h |    1 +
 arch/powerpc/include/asm/pkeys.h       |   32 ++++++++++++++++++++++++++++++--
 arch/powerpc/kernel/prom.c             |   19 +++++++++++++++++++
 4 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index d02ad93..1d9ed84 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -214,6 +214,7 @@ enum {
 #define CPU_FTR_DAWR			LONG_ASM_CONST(0x0400000000000000)
 #define CPU_FTR_DABRX			LONG_ASM_CONST(0x0800000000000000)
 #define CPU_FTR_PMAO_BUG		LONG_ASM_CONST(0x1000000000000000)
+#define CPU_FTR_PKEY			LONG_ASM_CONST(0x2000000000000000)
 #define CPU_FTR_POWER9_DD1		LONG_ASM_CONST(0x4000000000000000)
 
 #ifndef __ASSEMBLY__
@@ -452,7 +453,7 @@ enum {
 	    CPU_FTR_DSCR | CPU_FTR_SAO  | CPU_FTR_ASYM_SMT | \
 	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
 	    CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | \
-	    CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX)
+	    CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX | CPU_FTR_PKEY)
 #define CPU_FTRS_POWER8 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
 	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\
 	    CPU_FTR_MMCRA | CPU_FTR_SMT | \
@@ -462,7 +463,7 @@ enum {
 	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
 	    CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
 	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
-	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP)
+	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_PKEY)
 #define CPU_FTRS_POWER8E (CPU_FTRS_POWER8 | CPU_FTR_PMAO_BUG)
 #define CPU_FTRS_POWER8_DD1 (CPU_FTRS_POWER8 & ~CPU_FTR_DBELL)
 #define CPU_FTRS_POWER9 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
@@ -474,7 +475,8 @@ enum {
 	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
 	    CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
 	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
-	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300)
+	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | \
+	    CPU_FTR_PKEY)
 #define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \
 			     (~CPU_FTR_SAO))
 #define CPU_FTRS_CELL	(CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index a1cfcca..acd59d8 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -188,6 +188,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
 
 #define pkey_initialize()
 #define pkey_mm_init(mm)
+#define pkey_mmu_values(total_data, total_execute)
 
 static inline int vma_pkey(struct vm_area_struct *vma)
 {
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index ba7bff6..e61ed6c 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -1,6 +1,8 @@
 #ifndef _ASM_PPC64_PKEYS_H
 #define _ASM_PPC64_PKEYS_H
 
+#include <asm/firmware.h>
+
 extern bool pkey_inited;
 extern int pkeys_total; /* total pkeys as per device tree */
 extern u32 initial_allocation_mask;/* bits set for reserved keys */
@@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm)
 	mm->context.execute_only_pkey = -1;
 }
 
+static inline void pkey_mmu_values(int total_data, int total_execute)
+{
+	/*
+	 * since any pkey can be used for data or execute, we
+	 * will  just  treat all keys as equal and track them
+	 * as one entity.
+	 */
+	pkeys_total = total_data + total_execute;
+}
+
+static inline bool pkey_mmu_enabled(void)
+{
+	if (firmware_has_feature(FW_FEATURE_LPAR))
+		return pkeys_total;
+	else
+		return cpu_has_feature(CPU_FTR_PKEY);
+}
+
 static inline void pkey_initialize(void)
 {
 	int os_reserved, i;
@@ -236,9 +256,17 @@ static inline void pkey_initialize(void)
 	 * line will enable it.
 	 */
 	pkey_inited = false;
+	if (pkey_mmu_enabled())
+		pkey_inited = !radix_enabled();
+
+	if (!pkey_inited)
+		return;
 
-	/* Lets assume 32 keys */
-	pkeys_total = 32;
+	/* Lets assume 32 keys if we are not told
+	 * the number of pkeys.
+	 */
+	if (!pkeys_total)
+		pkeys_total = 32;
 
 #ifdef CONFIG_PPC_4K_PAGES
 	/*
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index f830562..f61da26 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -35,6 +35,7 @@
 #include <linux/of_fdt.h>
 #include <linux/libfdt.h>
 #include <linux/cpu.h>
+#include <linux/pkeys.h>
 
 #include <asm/prom.h>
 #include <asm/rtas.h>
@@ -228,6 +229,23 @@ static void __init check_cpu_pa_features(unsigned long node)
 		      ibm_pa_features, ARRAY_SIZE(ibm_pa_features));
 }
 
+static void __init check_cpu_pkey_feature(unsigned long node)
+{
+	const __be32 *ftrs;
+	int len, total_data, total_execute;
+
+	ftrs = of_get_flat_dt_prop(node,
+		"ibm,processor-storage-keys", &len);
+	if (ftrs == NULL)
+		return;
+
+	len /= sizeof(int);
+	total_execute = (len >= 2) ? be32_to_cpu(ftrs[1]) : 0;
+	total_data = (len >= 1) ? be32_to_cpu(ftrs[0]) : 0;
+	pkey_mmu_values(total_data, total_execute);
+}
+
+
 #ifdef CONFIG_PPC_STD_MMU_64
 static void __init init_mmu_slb_size(unsigned long node)
 {
@@ -391,6 +409,7 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
 
 		check_cpu_feature_properties(node);
 		check_cpu_pa_features(node);
+		check_cpu_pkey_feature(node);
 	}
 
 	identical_pvr_fixup(node);
-- 
1.7.1

Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys

[Thiago Jung Bauermann]

Ram Pai <linuxram@us.ibm.com> writes:
>  static inline void pkey_initialize(void)
>  {
> +	int os_reserved, i;
> +
>  	/* disable the pkey system till everything
>  	 * is in place. A patch further down the
>  	 * line will enable it.
>  	 */
>  	pkey_inited = false;
> +
> +	/* Lets assume 32 keys */
> +	pkeys_total = 32;
> +
> +#ifdef CONFIG_PPC_4K_PAGES
> +	/*
> +	 * the OS can manage only 8 pkeys
> +	 * due to its inability to represent
> +	 * them in the linux 4K-PTE.
> +	 */
> +	os_reserved = pkeys_total-8;
> +#else
> +	os_reserved = 0;
> +#endif
> +	/*
> +	 * Bits are in LE format.
> +	 * NOTE: 1, 0 are reserved.
> +	 * key 0 is the default key, which allows read/write/execute.
> +	 * key 1 is recommended not to be used.
> +	 * PowerISA(3.0) page 1015, programming note.
> +	 */
> +	initial_allocation_mask = ~0x0;
> +	for (i = 2; i < (pkeys_total - os_reserved); i++)
> +		initial_allocation_mask &= ~(0x1<<i);
>  }
>  #endif /*_ASM_PPC64_PKEYS_H */

In v6, key 31 was also reserved, but it's not in this version. Is this
intentional?

Isn't it better for this function to be in pkeys.c? Ideally, functions
should be in .c files not in headers unless they're very small or
performance sensitive IMHO.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC v7 09/25] powerpc: store and restore the pkey state across context switches

[Thiago Jung Bauermann]

Ram Pai <linuxram@us.ibm.com> writes:
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -42,6 +42,7 @@
>  #include <linux/hw_breakpoint.h>
>  #include <linux/uaccess.h>
>  #include <linux/elf-randomize.h>
> +#include <linux/pkeys.h>
>
>  #include <asm/pgtable.h>
>  #include <asm/io.h>
> @@ -1096,6 +1097,13 @@ static inline void save_sprs(struct thread_struct *t)
>  		t->tar = mfspr(SPRN_TAR);
>  	}
>  #endif
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> +	if (arch_pkeys_enabled()) {
> +		t->amr = mfspr(SPRN_AMR);
> +		t->iamr = mfspr(SPRN_IAMR);
> +		t->uamor = mfspr(SPRN_UAMOR);
> +	}
> +#endif
>  }

Is it worth having a flag in thread_struct saying whether it has every
called pkey_alloc and only do the mfsprs if it did?

> @@ -1131,6 +1139,16 @@ static inline void restore_sprs(struct thread_struct *old_thread,
>  			mtspr(SPRN_TAR, new_thread->tar);
>  	}
>  #endif
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> +	if (arch_pkeys_enabled()) {
> +		if (old_thread->amr != new_thread->amr)
> +			mtspr(SPRN_AMR, new_thread->amr);
> +		if (old_thread->iamr != new_thread->iamr)
> +			mtspr(SPRN_IAMR, new_thread->iamr);
> +		if (old_thread->uamor != new_thread->uamor)
> +			mtspr(SPRN_UAMOR, new_thread->uamor);
> +	}
> +#endif
>  }
>
>  struct task_struct *__switch_to(struct task_struct *prev,
> @@ -1689,6 +1707,13 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)
>  	current->thread.tm_tfiar = 0;
>  	current->thread.load_tm = 0;
>  #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> +	if (arch_pkeys_enabled()) {
> +		current->thread.amr   = 0x0ul;
> +		current->thread.iamr  = 0x0ul;
> +		current->thread.uamor = 0x0ul;
> +	}
> +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
>  }
>  EXPORT_SYMBOL(start_thread);

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation

[Thiago Jung Bauermann]

Ram Pai <linuxram@us.ibm.com> writes:

> The value of the AMR register at the time of exception
> is made available in gp_regs[PT_AMR] of the siginfo.
>
> The value of the pkey, whose protection got violated,
> is made available in si_pkey field of the siginfo structure.

Should the IAMR also be made available?

Also, should the AMR and IAMR be accesible to userspace (e.g., to GDB)
via ptrace and the core file?

> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -500,6 +500,11 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
>  				   (unsigned long) &frame->tramp[2]);
>  	}
>
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> +	if (__put_user(get_paca()->paca_amr, &frame->mc_gregs[PT_AMR]))
> +		return 1;
> +#endif /*  CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> +
>  	return 0;
>  }

frame->mc_gregs[PT_AMR] has 32 bits, but paca_amr has 64 bits. Does this
work as intended?

> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> index c83c115..86a4262 100644
> --- a/arch/powerpc/kernel/signal_64.c
> +++ b/arch/powerpc/kernel/signal_64.c
> @@ -174,6 +174,10 @@ static long setup_sigcontext(struct sigcontext __user *sc,
>  	if (set != NULL)
>  		err |=  __put_user(set->sig[0], &sc->oldmask);
>
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> +	err |= __put_user(get_paca()->paca_amr, &sc->gp_regs[PT_AMR]);
> +#endif /*  CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> +
>  	return err;
>  }

Isn't a corresponding change needed in restore_sigcontext? And in the
corresponding TM versions setup_tm_sigcontexts and restore_tm_sigcontexts?

Ditto for the equivalent functions in signal_32.c.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC v7 25/25] powerpc: Enable pkey subsystem

[Thiago Jung Bauermann]

Ram Pai <linuxram@us.ibm.com> writes:
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -214,6 +214,7 @@ enum {
>  #define CPU_FTR_DAWR			LONG_ASM_CONST(0x0400000000000000)
>  #define CPU_FTR_DABRX			LONG_ASM_CONST(0x0800000000000000)
>  #define CPU_FTR_PMAO_BUG		LONG_ASM_CONST(0x1000000000000000)
> +#define CPU_FTR_PKEY			LONG_ASM_CONST(0x2000000000000000)
>  #define CPU_FTR_POWER9_DD1		LONG_ASM_CONST(0x4000000000000000)
>
>  #ifndef __ASSEMBLY__
> @@ -452,7 +453,7 @@ enum {
>  	    CPU_FTR_DSCR | CPU_FTR_SAO  | CPU_FTR_ASYM_SMT | \
>  	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
>  	    CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | \
> -	    CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX)
> +	    CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX | CPU_FTR_PKEY)

P7 supports protection keys for data access (AMR) but not for
instruction access (IAMR), right? There's nothing in the code making
this distinction, so either CPU_FTR_PKEY shouldn't be enabled in P7 or
separate feature bits for AMR and IAMR should be used and checked before
trying to access the IAMR.

>  #define CPU_FTRS_POWER8 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
>  	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\
>  	    CPU_FTR_MMCRA | CPU_FTR_SMT | \
> @@ -462,7 +463,7 @@ enum {
>  	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
>  	    CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
>  	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
> -	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP)
> +	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_PKEY)
>  #define CPU_FTRS_POWER8E (CPU_FTRS_POWER8 | CPU_FTR_PMAO_BUG)
>  #define CPU_FTRS_POWER8_DD1 (CPU_FTRS_POWER8 & ~CPU_FTR_DBELL)
>  #define CPU_FTRS_POWER9 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> @@ -474,7 +475,8 @@ enum {
>  	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
>  	    CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
>  	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
> -	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300)
> +	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | \
> +	    CPU_FTR_PKEY)
>  #define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \
>  			     (~CPU_FTR_SAO))
>  #define CPU_FTRS_CELL	(CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index a1cfcca..acd59d8 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -188,6 +188,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
>
>  #define pkey_initialize()
>  #define pkey_mm_init(mm)
> +#define pkey_mmu_values(total_data, total_execute)
>
>  static inline int vma_pkey(struct vm_area_struct *vma)
>  {
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index ba7bff6..e61ed6c 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -1,6 +1,8 @@
>  #ifndef _ASM_PPC64_PKEYS_H
>  #define _ASM_PPC64_PKEYS_H
>
> +#include <asm/firmware.h>
> +
>  extern bool pkey_inited;
>  extern int pkeys_total; /* total pkeys as per device tree */
>  extern u32 initial_allocation_mask;/* bits set for reserved keys */
> @@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm)
>  	mm->context.execute_only_pkey = -1;
>  }
>
> +static inline void pkey_mmu_values(int total_data, int total_execute)
> +{
> +	/*
> +	 * since any pkey can be used for data or execute, we
> +	 * will  just  treat all keys as equal and track them
> +	 * as one entity.
> +	 */
> +	pkeys_total = total_data + total_execute;
> +}

Right now this works because the firmware reports 0 execute keys in the
device tree, but if (when?) it is fixed to report 32 execute keys as
well as 32 data keys (which are the same keys), any place using
pkeys_total expecting it to mean the number of keys that are available
will be broken. This includes pkey_initialize and mm_pkey_is_allocated.

Perhaps pkeys_total should use total_data as the number of keys
supported in the system, and total_execute just as a flag to say whether
there's a IAMR? Or, since P8 and later have IAMR and P7 is unlikely to
have the firmware fixed, maybe the kernel should just ignore
total_execute altogether?

> +static inline bool pkey_mmu_enabled(void)
> +{
> +	if (firmware_has_feature(FW_FEATURE_LPAR))
> +		return pkeys_total;
> +	else
> +		return cpu_has_feature(CPU_FTR_PKEY);
> +}
> +
>  static inline void pkey_initialize(void)
>  {
>  	int os_reserved, i;
> @@ -236,9 +256,17 @@ static inline void pkey_initialize(void)
>  	 * line will enable it.
>  	 */
>  	pkey_inited = false;
> +	if (pkey_mmu_enabled())
> +		pkey_inited = !radix_enabled();
> +
> +	if (!pkey_inited)
> +		return;
>
> -	/* Lets assume 32 keys */
> -	pkeys_total = 32;
> +	/* Lets assume 32 keys if we are not told
> +	 * the number of pkeys.
> +	 */
> +	if (!pkeys_total)
> +		pkeys_total = 32;
>
>  #ifdef CONFIG_PPC_4K_PAGES
>  	/*

This patch should remove the comment "disable the pkey system till
everything is in place. A patch further down the line will enable it.".

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys

[Michael Ellerman]

Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:

> Ram Pai <linuxram@us.ibm.com> writes:
>>  static inline void pkey_initialize(void)
>>  {
>> +	int os_reserved, i;
>> +
>>  	/* disable the pkey system till everything
>>  	 * is in place. A patch further down the
>>  	 * line will enable it.
>>  	 */
>>  	pkey_inited = false;
>> +
>> +	/* Lets assume 32 keys */
>> +	pkeys_total = 32;
>> +
>> +#ifdef CONFIG_PPC_4K_PAGES
>> +	/*
>> +	 * the OS can manage only 8 pkeys
>> +	 * due to its inability to represent
>> +	 * them in the linux 4K-PTE.
>> +	 */
>> +	os_reserved = pkeys_total-8;
>> +#else
>> +	os_reserved = 0;
>> +#endif
>> +	/*
>> +	 * Bits are in LE format.
>> +	 * NOTE: 1, 0 are reserved.
>> +	 * key 0 is the default key, which allows read/write/execute.
>> +	 * key 1 is recommended not to be used.
>> +	 * PowerISA(3.0) page 1015, programming note.
>> +	 */
>> +	initial_allocation_mask = ~0x0;
>> +	for (i = 2; i < (pkeys_total - os_reserved); i++)
>> +		initial_allocation_mask &= ~(0x1<<i);
>>  }
>>  #endif /*_ASM_PPC64_PKEYS_H */
>
> In v6, key 31 was also reserved, but it's not in this version. Is this
> intentional?

That whole thing could be replaced with two constants.

Except it can't, because we can't just hard code the number of keys. It
needs to come either from the device tree or be based on the CPU we're
running on.

> Isn't it better for this function to be in pkeys.c? Ideally, functions
> should be in .c files not in headers unless they're very small or
> performance sensitive IMHO.

Yes. No reason for that to be in a header AFAICS.

cheers

Re: [RFC v7 09/25] powerpc: store and restore the pkey state across context switches

[Michael Ellerman]

Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:

> Ram Pai <linuxram@us.ibm.com> writes:
>> --- a/arch/powerpc/kernel/process.c
>> +++ b/arch/powerpc/kernel/process.c
>> @@ -42,6 +42,7 @@
>>  #include <linux/hw_breakpoint.h>
>>  #include <linux/uaccess.h>
>>  #include <linux/elf-randomize.h>
>> +#include <linux/pkeys.h>
>>
>>  #include <asm/pgtable.h>
>>  #include <asm/io.h>
>> @@ -1096,6 +1097,13 @@ static inline void save_sprs(struct thread_struct *t)
>>  		t->tar = mfspr(SPRN_TAR);
>>  	}
>>  #endif
>> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
>> +	if (arch_pkeys_enabled()) {
>> +		t->amr = mfspr(SPRN_AMR);
>> +		t->iamr = mfspr(SPRN_IAMR);
>> +		t->uamor = mfspr(SPRN_UAMOR);
>> +	}
>> +#endif
>>  }
>
> Is it worth having a flag in thread_struct saying whether it has every
> called pkey_alloc and only do the mfsprs if it did?

Yes, in fact there's a programming note in the UAMOR section of the arch
that says exactly that.

On the write side you have to be a bit more careful. You have to make
sure you set the UAMOR to 0 when you're switching from a process that
has used keys to one that isn't.

cheers

Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation

[Michael Ellerman]

Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:

> Ram Pai <linuxram@us.ibm.com> writes:
>
>> The value of the AMR register at the time of exception
>> is made available in gp_regs[PT_AMR] of the siginfo.
>>
>> The value of the pkey, whose protection got violated,
>> is made available in si_pkey field of the siginfo structure.
>
> Should the IAMR also be made available?
>
> Also, should the AMR and IAMR be accesible to userspace (e.g., to GDB)
> via ptrace and the core file?

Yes if they're part of the thread's context they should be accessible
via ptrace and in core files.

>> --- a/arch/powerpc/kernel/signal_32.c
>> +++ b/arch/powerpc/kernel/signal_32.c
>> @@ -500,6 +500,11 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
>>  				   (unsigned long) &frame->tramp[2]);
>>  	}
>>
>> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
>> +	if (__put_user(get_paca()->paca_amr, &frame->mc_gregs[PT_AMR]))
>> +		return 1;
>> +#endif /*  CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
>> +
>>  	return 0;
>>  }
>
> frame->mc_gregs[PT_AMR] has 32 bits, but paca_amr has 64 bits. Does this
> work as intended?

I don't understand why we are putting it in there at all?

Is there some special handling of the actual register on signals? I
haven't seen it. In which case the process can get the value of AMR by
reading the register. ??

cheers

[RFC v7 26/25] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface

[Thiago Jung Bauermann]

Expose useful information for programs using memory protection keys.
Provide implementation for powerpc and x86.

On a powerpc system with pkeys support, here is what is shown:

$ head /sys/kernel/mm/protection_keys/*
==> /sys/kernel/mm/protection_keys/disable_execute_supported <==
true

==> /sys/kernel/mm/protection_keys/total_keys <==
32

==> /sys/kernel/mm/protection_keys/usable_keys <==
30

And on an x86 without pkeys support:

$ head /sys/kernel/mm/protection_keys/*
==> /sys/kernel/mm/protection_keys/disable_execute_supported <==
false

==> /sys/kernel/mm/protection_keys/total_keys <==
1

==> /sys/kernel/mm/protection_keys/usable_keys <==
0

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---

Ram asked me to add a sysfs interface for the memory protection keys
feature. Here it is.

If you have suggestions on what should be exposed, please let me know.

 arch/powerpc/include/asm/pkeys.h   |  2 ++
 arch/powerpc/mm/pkeys.c            | 12 ++++++++
 arch/x86/include/asm/mmu_context.h | 34 +++++++++++-----------
 arch/x86/include/asm/pkeys.h       |  1 +
 arch/x86/mm/pkeys.c                |  5 ++++
 mm/mprotect.c                      | 58 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 96 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index e61ed6c332db..bbc5a34cc6d6 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -215,6 +215,8 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 	return __arch_set_user_pkey_access(tsk, pkey, init_val);
 }
 
+unsigned int arch_usable_pkeys(void);
+
 static inline bool arch_pkeys_enabled(void)
 {
 	return pkey_inited;
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index 1424c79f45f6..54efbb133049 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -272,3 +272,15 @@ bool arch_vma_access_permitted(struct vm_area_struct *vma,
 
 	return pkey_access_permitted(pkey, write, execute);
 }
+
+unsigned int arch_usable_pkeys(void)
+{
+	unsigned int reserved;
+
+	if (!pkey_inited)
+		return 0;
+
+	reserved = hweight32(initial_allocation_mask);
+
+	return (pkeys_total > reserved) ? pkeys_total - reserved : 0;
+}
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index 68b329d77b3a..d2eabedd583a 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -105,13 +105,30 @@ static inline void enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)
 #endif
 }
 
+#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
+#define PKEY_INITIAL_ALLOCATION_MAP	1
+
+static inline int vma_pkey(struct vm_area_struct *vma)
+{
+	unsigned long vma_pkey_mask = VM_PKEY_BIT0 | VM_PKEY_BIT1 |
+				      VM_PKEY_BIT2 | VM_PKEY_BIT3;
+
+	return (vma->vm_flags & vma_pkey_mask) >> VM_PKEY_SHIFT;
+}
+#else
+static inline int vma_pkey(struct vm_area_struct *vma)
+{
+	return 0;
+}
+#endif
+
 static inline int init_new_context(struct task_struct *tsk,
 				   struct mm_struct *mm)
 {
 	#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
 	if (cpu_feature_enabled(X86_FEATURE_OSPKE)) {
 		/* pkey 0 is the default and always allocated */
-		mm->context.pkey_allocation_map = 0x1;
+		mm->context.pkey_allocation_map = PKEY_INITIAL_ALLOCATION_MAP;
 		/* -1 means unallocated or invalid */
 		mm->context.execute_only_pkey = -1;
 	}
@@ -205,21 +222,6 @@ static inline void arch_unmap(struct mm_struct *mm, struct vm_area_struct *vma,
 		mpx_notify_unmap(mm, vma, start, end);
 }
 
-#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
-static inline int vma_pkey(struct vm_area_struct *vma)
-{
-	unsigned long vma_pkey_mask = VM_PKEY_BIT0 | VM_PKEY_BIT1 |
-				      VM_PKEY_BIT2 | VM_PKEY_BIT3;
-
-	return (vma->vm_flags & vma_pkey_mask) >> VM_PKEY_SHIFT;
-}
-#else
-static inline int vma_pkey(struct vm_area_struct *vma)
-{
-	return 0;
-}
-#endif
-
 static inline bool __pkru_allows_pkey(u16 pkey, bool write)
 {
 	u32 pkru = read_pkru();
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
index fa8279972ddf..e1b25aa60530 100644
--- a/arch/x86/include/asm/pkeys.h
+++ b/arch/x86/include/asm/pkeys.h
@@ -105,5 +105,6 @@ extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 		unsigned long init_val);
 extern void copy_init_pkru_to_fpregs(void);
+extern unsigned int arch_usable_pkeys(void);
 
 #endif /*_ASM_X86_PKEYS_H */
diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index 2dab69a706ec..a3acca15ff83 100644
--- a/arch/x86/mm/pkeys.c
+++ b/arch/x86/mm/pkeys.c
@@ -123,6 +123,11 @@ int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot, int pkey
 	return vma_pkey(vma);
 }
 
+unsigned int arch_usable_pkeys(void)
+{
+	return arch_max_pkey() - hweight32(PKEY_INITIAL_ALLOCATION_MAP);
+}
+
 #define PKRU_AD_KEY(pkey)	(PKRU_AD_BIT << ((pkey) * PKRU_BITS_PER_PKEY))
 
 /*
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 8edd0d576254..855744b9f7d6 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -554,4 +554,62 @@ SYSCALL_DEFINE1(pkey_free, int, pkey)
 	return ret;
 }
 
+#ifdef CONFIG_SYSFS
+
+#define PKEYS_ATTR_RO(_name)						\
+	static struct kobj_attribute _name##_attr = __ATTR_RO(_name)
+
+static ssize_t total_keys_show(struct kobject *kobj,
+			       struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%u\n", arch_max_pkey());
+}
+PKEYS_ATTR_RO(total_keys);
+
+static ssize_t usable_keys_show(struct kobject *kobj,
+				struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%u\n", arch_usable_pkeys());
+}
+PKEYS_ATTR_RO(usable_keys);
+
+static ssize_t disable_execute_supported_show(struct kobject *kobj,
+					      struct kobj_attribute *attr,
+					      char *buf)
+{
+#ifdef PKEY_DISABLE_EXECUTE
+	if (arch_pkeys_enabled()) {
+		strcpy(buf, "true\n");
+		return sizeof("true\n") - 1;
+	}
+#endif
+
+	strcpy(buf, "false\n");
+	return sizeof("false\n") - 1;
+}
+PKEYS_ATTR_RO(disable_execute_supported);
+
+static struct attribute *pkeys_attrs[] = {
+	&total_keys_attr.attr,
+	&usable_keys_attr.attr,
+	&disable_execute_supported_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group pkeys_attr_group = {
+	.attrs = pkeys_attrs,
+	.name = "protection_keys",
+};
+
+static int __init pkeys_sysfs_init(void)
+{
+	int err;
+
+	err = sysfs_create_group(mm_kobj, &pkeys_attr_group);
+
+	return err;
+}
+late_initcall(pkeys_sysfs_init);
+#endif /* CONFIG_SYSFS */
+
 #endif /* CONFIG_ARCH_HAS_PKEYS */

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

[RFC v7 26/25] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface

[Thiago Jung Bauermann]

Expose useful information for programs using memory protection keys.
Provide implementation for powerpc and x86.

On a powerpc system with pkeys support, here is what is shown:

$ head /sys/kernel/mm/protection_keys/*
==> /sys/kernel/mm/protection_keys/disable_execute_supported <==
true

==> /sys/kernel/mm/protection_keys/total_keys <==
32

==> /sys/kernel/mm/protection_keys/usable_keys <==
30

And on an x86 without pkeys support:

$ head /sys/kernel/mm/protection_keys/*
==> /sys/kernel/mm/protection_keys/disable_execute_supported <==
false

==> /sys/kernel/mm/protection_keys/total_keys <==
1

==> /sys/kernel/mm/protection_keys/usable_keys <==
0

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---

Ram asked me to add a sysfs interface for the memory protection keys
feature. Here it is.

If you have suggestions on what should be exposed, please let me know.

 arch/powerpc/include/asm/pkeys.h   |  2 ++
 arch/powerpc/mm/pkeys.c            | 12 ++++++++
 arch/x86/include/asm/mmu_context.h | 34 +++++++++++-----------
 arch/x86/include/asm/pkeys.h       |  1 +
 arch/x86/mm/pkeys.c                |  5 ++++
 mm/mprotect.c                      | 58 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 96 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index e61ed6c332db..bbc5a34cc6d6 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -215,6 +215,8 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 	return __arch_set_user_pkey_access(tsk, pkey, init_val);
 }
 
+unsigned int arch_usable_pkeys(void);
+
 static inline bool arch_pkeys_enabled(void)
 {
 	return pkey_inited;
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index 1424c79f45f6..54efbb133049 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -272,3 +272,15 @@ bool arch_vma_access_permitted(struct vm_area_struct *vma,
 
 	return pkey_access_permitted(pkey, write, execute);
 }
+
+unsigned int arch_usable_pkeys(void)
+{
+	unsigned int reserved;
+
+	if (!pkey_inited)
+		return 0;
+
+	reserved = hweight32(initial_allocation_mask);
+
+	return (pkeys_total > reserved) ? pkeys_total - reserved : 0;
+}
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index 68b329d77b3a..d2eabedd583a 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -105,13 +105,30 @@ static inline void enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)
 #endif
 }
 
+#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
+#define PKEY_INITIAL_ALLOCATION_MAP	1
+
+static inline int vma_pkey(struct vm_area_struct *vma)
+{
+	unsigned long vma_pkey_mask = VM_PKEY_BIT0 | VM_PKEY_BIT1 |
+				      VM_PKEY_BIT2 | VM_PKEY_BIT3;
+
+	return (vma->vm_flags & vma_pkey_mask) >> VM_PKEY_SHIFT;
+}
+#else
+static inline int vma_pkey(struct vm_area_struct *vma)
+{
+	return 0;
+}
+#endif
+
 static inline int init_new_context(struct task_struct *tsk,
 				   struct mm_struct *mm)
 {
 	#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
 	if (cpu_feature_enabled(X86_FEATURE_OSPKE)) {
 		/* pkey 0 is the default and always allocated */
-		mm->context.pkey_allocation_map = 0x1;
+		mm->context.pkey_allocation_map = PKEY_INITIAL_ALLOCATION_MAP;
 		/* -1 means unallocated or invalid */
 		mm->context.execute_only_pkey = -1;
 	}
@@ -205,21 +222,6 @@ static inline void arch_unmap(struct mm_struct *mm, struct vm_area_struct *vma,
 		mpx_notify_unmap(mm, vma, start, end);
 }
 
-#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
-static inline int vma_pkey(struct vm_area_struct *vma)
-{
-	unsigned long vma_pkey_mask = VM_PKEY_BIT0 | VM_PKEY_BIT1 |
-				      VM_PKEY_BIT2 | VM_PKEY_BIT3;
-
-	return (vma->vm_flags & vma_pkey_mask) >> VM_PKEY_SHIFT;
-}
-#else
-static inline int vma_pkey(struct vm_area_struct *vma)
-{
-	return 0;
-}
-#endif
-
 static inline bool __pkru_allows_pkey(u16 pkey, bool write)
 {
 	u32 pkru = read_pkru();
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
index fa8279972ddf..e1b25aa60530 100644
--- a/arch/x86/include/asm/pkeys.h
+++ b/arch/x86/include/asm/pkeys.h
@@ -105,5 +105,6 @@ extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 		unsigned long init_val);
 extern void copy_init_pkru_to_fpregs(void);
+extern unsigned int arch_usable_pkeys(void);
 
 #endif /*_ASM_X86_PKEYS_H */
diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index 2dab69a706ec..a3acca15ff83 100644
--- a/arch/x86/mm/pkeys.c
+++ b/arch/x86/mm/pkeys.c
@@ -123,6 +123,11 @@ int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot, int pkey
 	return vma_pkey(vma);
 }
 
+unsigned int arch_usable_pkeys(void)
+{
+	return arch_max_pkey() - hweight32(PKEY_INITIAL_ALLOCATION_MAP);
+}
+
 #define PKRU_AD_KEY(pkey)	(PKRU_AD_BIT << ((pkey) * PKRU_BITS_PER_PKEY))
 
 /*
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 8edd0d576254..855744b9f7d6 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -554,4 +554,62 @@ SYSCALL_DEFINE1(pkey_free, int, pkey)
 	return ret;
 }
 
+#ifdef CONFIG_SYSFS
+
+#define PKEYS_ATTR_RO(_name)						\
+	static struct kobj_attribute _name##_attr = __ATTR_RO(_name)
+
+static ssize_t total_keys_show(struct kobject *kobj,
+			       struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%u\n", arch_max_pkey());
+}
+PKEYS_ATTR_RO(total_keys);
+
+static ssize_t usable_keys_show(struct kobject *kobj,
+				struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%u\n", arch_usable_pkeys());
+}
+PKEYS_ATTR_RO(usable_keys);
+
+static ssize_t disable_execute_supported_show(struct kobject *kobj,
+					      struct kobj_attribute *attr,
+					      char *buf)
+{
+#ifdef PKEY_DISABLE_EXECUTE
+	if (arch_pkeys_enabled()) {
+		strcpy(buf, "true\n");
+		return sizeof("true\n") - 1;
+	}
+#endif
+
+	strcpy(buf, "false\n");
+	return sizeof("false\n") - 1;
+}
+PKEYS_ATTR_RO(disable_execute_supported);
+
+static struct attribute *pkeys_attrs[] = {
+	&total_keys_attr.attr,
+	&usable_keys_attr.attr,
+	&disable_execute_supported_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group pkeys_attr_group = {
+	.attrs = pkeys_attrs,
+	.name = "protection_keys",
+};
+
+static int __init pkeys_sysfs_init(void)
+{
+	int err;
+
+	err = sysfs_create_group(mm_kobj, &pkeys_attr_group);
+
+	return err;
+}
+late_initcall(pkeys_sysfs_init);
+#endif /* CONFIG_SYSFS */
+
 #endif /* CONFIG_ARCH_HAS_PKEYS */

Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys

[Ram Pai]

On Thu, Aug 10, 2017 at 05:25:39PM -0300, Thiago Jung Bauermann wrote:
> 
> Ram Pai <linuxram@us.ibm.com> writes:
> >  static inline void pkey_initialize(void)
> >  {
> > +	int os_reserved, i;
> > +
> >  	/* disable the pkey system till everything
> >  	 * is in place. A patch further down the
> >  	 * line will enable it.
> >  	 */
> >  	pkey_inited = false;
> > +
> > +	/* Lets assume 32 keys */
> > +	pkeys_total = 32;
> > +
> > +#ifdef CONFIG_PPC_4K_PAGES
> > +	/*
> > +	 * the OS can manage only 8 pkeys
> > +	 * due to its inability to represent
> > +	 * them in the linux 4K-PTE.
> > +	 */
> > +	os_reserved = pkeys_total-8;
> > +#else
> > +	os_reserved = 0;
> > +#endif
> > +	/*
> > +	 * Bits are in LE format.
> > +	 * NOTE: 1, 0 are reserved.
> > +	 * key 0 is the default key, which allows read/write/execute.
> > +	 * key 1 is recommended not to be used.
> > +	 * PowerISA(3.0) page 1015, programming note.
> > +	 */
> > +	initial_allocation_mask = ~0x0;
> > +	for (i = 2; i < (pkeys_total - os_reserved); i++)
> > +		initial_allocation_mask &= ~(0x1<<i);
> >  }
> >  #endif /*_ASM_PPC64_PKEYS_H */
> 
> In v6, key 31 was also reserved, but it's not in this version. Is this
> intentional?

On powernv platform, there is no hypervisor and hence the hypervisor
will not reserve key 31 for its own use. Wherease on PAPR guest
the hypervisor takes away key 31.  

Its not possible to determine at compile time which keys are used
or not. Hence the above code.  pkeys_total is 32 in this patch,
but will be set to whatever value the device tree tells us. That will
be done in a subsequent patch.


RP

Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys

[Ram Pai]

On Fri, Aug 11, 2017 at 03:39:14PM +1000, Michael Ellerman wrote:
> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
> 
> > Ram Pai <linuxram@us.ibm.com> writes:
> >>  static inline void pkey_initialize(void)
> >>  {
> >> +	int os_reserved, i;
> >> +
> >>  	/* disable the pkey system till everything
> >>  	 * is in place. A patch further down the
> >>  	 * line will enable it.
> >>  	 */
> >>  	pkey_inited = false;
> >> +
> >> +	/* Lets assume 32 keys */
> >> +	pkeys_total = 32;
> >> +
> >> +#ifdef CONFIG_PPC_4K_PAGES
> >> +	/*
> >> +	 * the OS can manage only 8 pkeys
> >> +	 * due to its inability to represent
> >> +	 * them in the linux 4K-PTE.
> >> +	 */
> >> +	os_reserved = pkeys_total-8;
> >> +#else
> >> +	os_reserved = 0;
> >> +#endif
> >> +	/*
> >> +	 * Bits are in LE format.
> >> +	 * NOTE: 1, 0 are reserved.
> >> +	 * key 0 is the default key, which allows read/write/execute.
> >> +	 * key 1 is recommended not to be used.
> >> +	 * PowerISA(3.0) page 1015, programming note.
> >> +	 */
> >> +	initial_allocation_mask = ~0x0;
> >> +	for (i = 2; i < (pkeys_total - os_reserved); i++)
> >> +		initial_allocation_mask &= ~(0x1<<i);
> >>  }
> >>  #endif /*_ASM_PPC64_PKEYS_H */
> >
> > In v6, key 31 was also reserved, but it's not in this version. Is this
> > intentional?
> 
> That whole thing could be replaced with two constants.
> 
> Except it can't, because we can't just hard code the number of keys. It
> needs to come either from the device tree or be based on the CPU we're
> running on.
> 
> > Isn't it better for this function to be in pkeys.c? Ideally, functions
> > should be in .c files not in headers unless they're very small or
> > performance sensitive IMHO.
> 
> Yes. No reason for that to be in a header AFAICS.

Yes can be moved into the pkeys.c file.  It was a simple function
to begin with....but not so any more.

RP

Re: [RFC v7 09/25] powerpc: store and restore the pkey state across context switches

[Ram Pai]

On Fri, Aug 11, 2017 at 04:34:19PM +1000, Michael Ellerman wrote:
> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
> 
> > Ram Pai <linuxram@us.ibm.com> writes:
> >> --- a/arch/powerpc/kernel/process.c
> >> +++ b/arch/powerpc/kernel/process.c
> >> @@ -42,6 +42,7 @@
> >>  #include <linux/hw_breakpoint.h>
> >>  #include <linux/uaccess.h>
> >>  #include <linux/elf-randomize.h>
> >> +#include <linux/pkeys.h>
> >>
> >>  #include <asm/pgtable.h>
> >>  #include <asm/io.h>
> >> @@ -1096,6 +1097,13 @@ static inline void save_sprs(struct thread_struct *t)
> >>  		t->tar = mfspr(SPRN_TAR);
> >>  	}
> >>  #endif
> >> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> >> +	if (arch_pkeys_enabled()) {
> >> +		t->amr = mfspr(SPRN_AMR);
> >> +		t->iamr = mfspr(SPRN_IAMR);
> >> +		t->uamor = mfspr(SPRN_UAMOR);
> >> +	}
> >> +#endif
> >>  }
> >
> > Is it worth having a flag in thread_struct saying whether it has every
> > called pkey_alloc and only do the mfsprs if it did?

Yes. This will further optimize the code; a great thing!

> 
> Yes, in fact there's a programming note in the UAMOR section of the arch
> that says exactly that.
> 
> On the write side you have to be a bit more careful. You have to make
> sure you set the UAMOR to 0 when you're switching from a process that
> has used keys to one that isn't.

Currently we save and restore AMR/IAMR/UAMOR if the OS has enabled pkeys.
This means the UAMOR will get restored to 0 if the application has not
used any keys.

But if we do optimize the code further; as suggested by Thiago, we will
have to be careful with initializing UAMOR while switching back task
that has not used the keys yet.

RP

> 
> cheers

-- 
Ram Pai

Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation

[Ram Pai]

On Fri, Aug 11, 2017 at 08:26:30PM +1000, Michael Ellerman wrote:
> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
> 
> > Ram Pai <linuxram@us.ibm.com> writes:
> >
> >> The value of the AMR register at the time of exception
> >> is made available in gp_regs[PT_AMR] of the siginfo.
> >>
> >> The value of the pkey, whose protection got violated,
> >> is made available in si_pkey field of the siginfo structure.
> >
> > Should the IAMR also be made available?
> >
> > Also, should the AMR and IAMR be accesible to userspace (e.g., to GDB)
> > via ptrace and the core file?
> 
> Yes if they're part of the thread's context they should be accessible
> via ptrace and in core files.

ok. Some more code needed. :(

> 
> >> --- a/arch/powerpc/kernel/signal_32.c
> >> +++ b/arch/powerpc/kernel/signal_32.c
> >> @@ -500,6 +500,11 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
> >>  				   (unsigned long) &frame->tramp[2]);
> >>  	}
> >>
> >> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> >> +	if (__put_user(get_paca()->paca_amr, &frame->mc_gregs[PT_AMR]))
> >> +		return 1;
> >> +#endif /*  CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> >> +
> >>  	return 0;
> >>  }
> >
> > frame->mc_gregs[PT_AMR] has 32 bits, but paca_amr has 64 bits. Does this
> > work as intended?

hmm..i think we should just disable pkey support for 32 bit apps, till
we figure out all the edge cases.

> 
> I don't understand why we are putting it in there at all?
> 
> Is there some special handling of the actual register on signals? I
> haven't seen it. In which case the process can get the value of AMR by
> reading the register. ??

The value of AMR register at the time of the key-exception may not be
the same when the signal handler is invoked. 

RP

Re: [RFC v7 25/25] powerpc: Enable pkey subsystem

[Ram Pai]

On Thu, Aug 10, 2017 at 06:27:34PM -0300, Thiago Jung Bauermann wrote:
> 
> Ram Pai <linuxram@us.ibm.com> writes:
> > --- a/arch/powerpc/include/asm/cputable.h
> > +++ b/arch/powerpc/include/asm/cputable.h
> > @@ -214,6 +214,7 @@ enum {
> >  #define CPU_FTR_DAWR			LONG_ASM_CONST(0x0400000000000000)
> >  #define CPU_FTR_DABRX			LONG_ASM_CONST(0x0800000000000000)
> >  #define CPU_FTR_PMAO_BUG		LONG_ASM_CONST(0x1000000000000000)
> > +#define CPU_FTR_PKEY			LONG_ASM_CONST(0x2000000000000000)
> >  #define CPU_FTR_POWER9_DD1		LONG_ASM_CONST(0x4000000000000000)
> >
> >  #ifndef __ASSEMBLY__
> > @@ -452,7 +453,7 @@ enum {
> >  	    CPU_FTR_DSCR | CPU_FTR_SAO  | CPU_FTR_ASYM_SMT | \
> >  	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
> >  	    CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | \
> > -	    CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX)
> > +	    CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX | CPU_FTR_PKEY)
> 
> P7 supports protection keys for data access (AMR) but not for
> instruction access (IAMR), right? There's nothing in the code making
> this distinction, so either CPU_FTR_PKEY shouldn't be enabled in P7 or
> separate feature bits for AMR and IAMR should be used and checked before
> trying to access the IAMR.

did'nt David say P7 supports both? P6, i think, only support data.
my pkey tests have passed on p7.

> 
> >  #define CPU_FTRS_POWER8 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> >  	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\
> >  	    CPU_FTR_MMCRA | CPU_FTR_SMT | \
> > @@ -462,7 +463,7 @@ enum {
> >  	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
> >  	    CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
> >  	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
> > -	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP)
> > +	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_PKEY)
> >  #define CPU_FTRS_POWER8E (CPU_FTRS_POWER8 | CPU_FTR_PMAO_BUG)
> >  #define CPU_FTRS_POWER8_DD1 (CPU_FTRS_POWER8 & ~CPU_FTR_DBELL)
> >  #define CPU_FTRS_POWER9 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> > @@ -474,7 +475,8 @@ enum {
> >  	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
> >  	    CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
> >  	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
> > -	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300)
> > +	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | \
> > +	    CPU_FTR_PKEY)
> >  #define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \
> >  			     (~CPU_FTR_SAO))
> >  #define CPU_FTRS_CELL	(CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> > index a1cfcca..acd59d8 100644
> > --- a/arch/powerpc/include/asm/mmu_context.h
> > +++ b/arch/powerpc/include/asm/mmu_context.h
> > @@ -188,6 +188,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
> >
> >  #define pkey_initialize()
> >  #define pkey_mm_init(mm)
> > +#define pkey_mmu_values(total_data, total_execute)
> >
> >  static inline int vma_pkey(struct vm_area_struct *vma)
> >  {
> > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> > index ba7bff6..e61ed6c 100644
> > --- a/arch/powerpc/include/asm/pkeys.h
> > +++ b/arch/powerpc/include/asm/pkeys.h
> > @@ -1,6 +1,8 @@
> >  #ifndef _ASM_PPC64_PKEYS_H
> >  #define _ASM_PPC64_PKEYS_H
> >
> > +#include <asm/firmware.h>
> > +
> >  extern bool pkey_inited;
> >  extern int pkeys_total; /* total pkeys as per device tree */
> >  extern u32 initial_allocation_mask;/* bits set for reserved keys */
> > @@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm)
> >  	mm->context.execute_only_pkey = -1;
> >  }
> >
> > +static inline void pkey_mmu_values(int total_data, int total_execute)
> > +{
> > +	/*
> > +	 * since any pkey can be used for data or execute, we
> > +	 * will  just  treat all keys as equal and track them
> > +	 * as one entity.
> > +	 */
> > +	pkeys_total = total_data + total_execute;
> > +}
> 
> Right now this works because the firmware reports 0 execute keys in the
> device tree, but if (when?) it is fixed to report 32 execute keys as
> well as 32 data keys (which are the same keys), any place using
> pkeys_total expecting it to mean the number of keys that are available
> will be broken. This includes pkey_initialize and mm_pkey_is_allocated.

Good point. we should just ignore total_execute. It should
be the same value as total_data on the latest platforms.
On older platforms it will continue to be zero.

> 
> Perhaps pkeys_total should use total_data as the number of keys
> supported in the system, and total_execute just as a flag to say whether
> there's a IAMR? Or, since P8 and later have IAMR and P7 is unlikely to
> have the firmware fixed, maybe the kernel should just ignore
> total_execute altogether?
> 
> > +static inline bool pkey_mmu_enabled(void)
> > +{
> > +	if (firmware_has_feature(FW_FEATURE_LPAR))
> > +		return pkeys_total;
> > +	else
> > +		return cpu_has_feature(CPU_FTR_PKEY);
> > +}
> > +
> >  static inline void pkey_initialize(void)
> >  {
> >  	int os_reserved, i;
> > @@ -236,9 +256,17 @@ static inline void pkey_initialize(void)
> >  	 * line will enable it.
> >  	 */
> >  	pkey_inited = false;
> > +	if (pkey_mmu_enabled())
> > +		pkey_inited = !radix_enabled();
> > +
> > +	if (!pkey_inited)
> > +		return;
> >
> > -	/* Lets assume 32 keys */
> > -	pkeys_total = 32;
> > +	/* Lets assume 32 keys if we are not told
> > +	 * the number of pkeys.
> > +	 */
> > +	if (!pkeys_total)
> > +		pkeys_total = 32;
> >
> >  #ifdef CONFIG_PPC_4K_PAGES
> >  	/*
> 
> This patch should remove the comment "disable the pkey system till
> everything is in place. A patch further down the line will enable it.".

Yes.

RP

Re: [RFC v7 25/25] powerpc: Enable pkey subsystem

[Thiago Jung Bauermann]

Ram Pai <linuxram@us.ibm.com> writes:

> On Thu, Aug 10, 2017 at 06:27:34PM -0300, Thiago Jung Bauermann wrote:
>> 
>> Ram Pai <linuxram@us.ibm.com> writes:
>> > --- a/arch/powerpc/include/asm/cputable.h
>> > +++ b/arch/powerpc/include/asm/cputable.h
>> > @@ -214,6 +214,7 @@ enum {
>> >  #define CPU_FTR_DAWR			LONG_ASM_CONST(0x0400000000000000)
>> >  #define CPU_FTR_DABRX			LONG_ASM_CONST(0x0800000000000000)
>> >  #define CPU_FTR_PMAO_BUG		LONG_ASM_CONST(0x1000000000000000)
>> > +#define CPU_FTR_PKEY			LONG_ASM_CONST(0x2000000000000000)
>> >  #define CPU_FTR_POWER9_DD1		LONG_ASM_CONST(0x4000000000000000)
>> >
>> >  #ifndef __ASSEMBLY__
>> > @@ -452,7 +453,7 @@ enum {
>> >  	    CPU_FTR_DSCR | CPU_FTR_SAO  | CPU_FTR_ASYM_SMT | \
>> >  	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
>> >  	    CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | \
>> > -	    CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX)
>> > +	    CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX | CPU_FTR_PKEY)
>> 
>> P7 supports protection keys for data access (AMR) but not for
>> instruction access (IAMR), right? There's nothing in the code making
>> this distinction, so either CPU_FTR_PKEY shouldn't be enabled in P7 or
>> separate feature bits for AMR and IAMR should be used and checked before
>> trying to access the IAMR.
>
> did'nt David say P7 supports both? P6, i think, only support data.
> my pkey tests have passed on p7.

He said that P7 was the first processor to support 32 keys, but if you
look at the Virtual Page Class Key Protection section in ISA 2.06,
there's no IAMR.

There was a bug in the code where init_iamr was calling write_amr
instead of write_iamr, perhaps that's why it worked when you tested on P7?

>> 
>> >  #define CPU_FTRS_POWER8 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
>> >  	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\
>> >  	    CPU_FTR_MMCRA | CPU_FTR_SMT | \
>> > @@ -462,7 +463,7 @@ enum {
>> >  	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
>> >  	    CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
>> >  	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
>> > -	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP)
>> > +	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_PKEY)
>> >  #define CPU_FTRS_POWER8E (CPU_FTRS_POWER8 | CPU_FTR_PMAO_BUG)
>> >  #define CPU_FTRS_POWER8_DD1 (CPU_FTRS_POWER8 & ~CPU_FTR_DBELL)
>> >  #define CPU_FTRS_POWER9 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
>> > @@ -474,7 +475,8 @@ enum {
>> >  	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
>> >  	    CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
>> >  	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
>> > -	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300)
>> > +	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | \
>> > +	    CPU_FTR_PKEY)
>> >  #define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \
>> >  			     (~CPU_FTR_SAO))
>> >  #define CPU_FTRS_CELL	(CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
>> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
>> > index a1cfcca..acd59d8 100644
>> > --- a/arch/powerpc/include/asm/mmu_context.h
>> > +++ b/arch/powerpc/include/asm/mmu_context.h
>> > @@ -188,6 +188,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
>> >
>> >  #define pkey_initialize()
>> >  #define pkey_mm_init(mm)
>> > +#define pkey_mmu_values(total_data, total_execute)
>> >
>> >  static inline int vma_pkey(struct vm_area_struct *vma)
>> >  {
>> > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
>> > index ba7bff6..e61ed6c 100644
>> > --- a/arch/powerpc/include/asm/pkeys.h
>> > +++ b/arch/powerpc/include/asm/pkeys.h
>> > @@ -1,6 +1,8 @@
>> >  #ifndef _ASM_PPC64_PKEYS_H
>> >  #define _ASM_PPC64_PKEYS_H
>> >
>> > +#include <asm/firmware.h>
>> > +
>> >  extern bool pkey_inited;
>> >  extern int pkeys_total; /* total pkeys as per device tree */
>> >  extern u32 initial_allocation_mask;/* bits set for reserved keys */
>> > @@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm)
>> >  	mm->context.execute_only_pkey = -1;
>> >  }
>> >
>> > +static inline void pkey_mmu_values(int total_data, int total_execute)
>> > +{
>> > +	/*
>> > +	 * since any pkey can be used for data or execute, we
>> > +	 * will  just  treat all keys as equal and track them
>> > +	 * as one entity.
>> > +	 */
>> > +	pkeys_total = total_data + total_execute;
>> > +}
>> 
>> Right now this works because the firmware reports 0 execute keys in the
>> device tree, but if (when?) it is fixed to report 32 execute keys as
>> well as 32 data keys (which are the same keys), any place using
>> pkeys_total expecting it to mean the number of keys that are available
>> will be broken. This includes pkey_initialize and mm_pkey_is_allocated.
>
> Good point. we should just ignore total_execute. It should
> be the same value as total_data on the latest platforms.
> On older platforms it will continue to be zero.

Indeed. There should just be a special case to disable execute
protection for P7.

>> Perhaps pkeys_total should use total_data as the number of keys
>> supported in the system, and total_execute just as a flag to say whether
>> there's a IAMR? Or, since P8 and later have IAMR and P7 is unlikely to
>> have the firmware fixed, maybe the kernel should just ignore
>> total_execute altogether?

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys

[Thiago Jung Bauermann]

Ram Pai <linuxram@us.ibm.com> writes:

> On Thu, Aug 10, 2017 at 05:25:39PM -0300, Thiago Jung Bauermann wrote:
>> 
>> Ram Pai <linuxram@us.ibm.com> writes:
>> >  static inline void pkey_initialize(void)
>> >  {
>> > +	int os_reserved, i;
>> > +
>> >  	/* disable the pkey system till everything
>> >  	 * is in place. A patch further down the
>> >  	 * line will enable it.
>> >  	 */
>> >  	pkey_inited = false;
>> > +
>> > +	/* Lets assume 32 keys */
>> > +	pkeys_total = 32;
>> > +
>> > +#ifdef CONFIG_PPC_4K_PAGES
>> > +	/*
>> > +	 * the OS can manage only 8 pkeys
>> > +	 * due to its inability to represent
>> > +	 * them in the linux 4K-PTE.
>> > +	 */
>> > +	os_reserved = pkeys_total-8;
>> > +#else
>> > +	os_reserved = 0;
>> > +#endif
>> > +	/*
>> > +	 * Bits are in LE format.
>> > +	 * NOTE: 1, 0 are reserved.
>> > +	 * key 0 is the default key, which allows read/write/execute.
>> > +	 * key 1 is recommended not to be used.
>> > +	 * PowerISA(3.0) page 1015, programming note.
>> > +	 */
>> > +	initial_allocation_mask = ~0x0;
>> > +	for (i = 2; i < (pkeys_total - os_reserved); i++)
>> > +		initial_allocation_mask &= ~(0x1<<i);
>> >  }
>> >  #endif /*_ASM_PPC64_PKEYS_H */
>> 
>> In v6, key 31 was also reserved, but it's not in this version. Is this
>> intentional?
>
> On powernv platform, there is no hypervisor and hence the hypervisor
> will not reserve key 31 for its own use. Wherease on PAPR guest
> the hypervisor takes away key 31.  
>
> Its not possible to determine at compile time which keys are used
> or not. Hence the above code.  pkeys_total is 32 in this patch,
> but will be set to whatever value the device tree tells us. That will
> be done in a subsequent patch.

You're right. At the time I made that comment I didn't realize that the
hypervisor would subtract its reserved key from the device property.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC v7 25/25] powerpc: Enable pkey subsystem

[Ram Pai]

On Thu, Aug 17, 2017 at 05:30:27PM -0300, Thiago Jung Bauermann wrote:
> 
> Ram Pai <linuxram@us.ibm.com> writes:
> 
> > On Thu, Aug 10, 2017 at 06:27:34PM -0300, Thiago Jung Bauermann wrote:
> >> 
> >> Ram Pai <linuxram@us.ibm.com> writes:
> >> > --- a/arch/powerpc/include/asm/cputable.h
> >> > +++ b/arch/powerpc/include/asm/cputable.h
> >> > @@ -214,6 +214,7 @@ enum {
> >> >  #define CPU_FTR_DAWR			LONG_ASM_CONST(0x0400000000000000)
> >> >  #define CPU_FTR_DABRX			LONG_ASM_CONST(0x0800000000000000)
> >> >  #define CPU_FTR_PMAO_BUG		LONG_ASM_CONST(0x1000000000000000)
> >> > +#define CPU_FTR_PKEY			LONG_ASM_CONST(0x2000000000000000)
> >> >  #define CPU_FTR_POWER9_DD1		LONG_ASM_CONST(0x4000000000000000)
> >> >
> >> >  #ifndef __ASSEMBLY__
> >> > @@ -452,7 +453,7 @@ enum {
> >> >  	    CPU_FTR_DSCR | CPU_FTR_SAO  | CPU_FTR_ASYM_SMT | \
> >> >  	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
> >> >  	    CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | \
> >> > -	    CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX)
> >> > +	    CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX | CPU_FTR_PKEY)
> >> 
> >> P7 supports protection keys for data access (AMR) but not for
> >> instruction access (IAMR), right? There's nothing in the code making
> >> this distinction, so either CPU_FTR_PKEY shouldn't be enabled in P7 or
> >> separate feature bits for AMR and IAMR should be used and checked before
> >> trying to access the IAMR.
> >
> > did'nt David say P7 supports both? P6, i think, only support data.
> > my pkey tests have passed on p7.
> 
> He said that P7 was the first processor to support 32 keys, but if you
> look at the Virtual Page Class Key Protection section in ISA 2.06,
> there's no IAMR.
> 
> There was a bug in the code where init_iamr was calling write_amr
> instead of write_iamr, perhaps that's why it worked when you tested on P7?
> 
> >> 
> >> >  #define CPU_FTRS_POWER8 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> >> >  	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\
> >> >  	    CPU_FTR_MMCRA | CPU_FTR_SMT | \
> >> > @@ -462,7 +463,7 @@ enum {
> >> >  	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
> >> >  	    CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
> >> >  	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
> >> > -	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP)
> >> > +	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_PKEY)
> >> >  #define CPU_FTRS_POWER8E (CPU_FTRS_POWER8 | CPU_FTR_PMAO_BUG)
> >> >  #define CPU_FTRS_POWER8_DD1 (CPU_FTRS_POWER8 & ~CPU_FTR_DBELL)
> >> >  #define CPU_FTRS_POWER9 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> >> > @@ -474,7 +475,8 @@ enum {
> >> >  	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
> >> >  	    CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
> >> >  	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
> >> > -	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300)
> >> > +	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | \
> >> > +	    CPU_FTR_PKEY)
> >> >  #define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \
> >> >  			     (~CPU_FTR_SAO))
> >> >  #define CPU_FTRS_CELL	(CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> >> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> >> > index a1cfcca..acd59d8 100644
> >> > --- a/arch/powerpc/include/asm/mmu_context.h
> >> > +++ b/arch/powerpc/include/asm/mmu_context.h
> >> > @@ -188,6 +188,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
> >> >
> >> >  #define pkey_initialize()
> >> >  #define pkey_mm_init(mm)
> >> > +#define pkey_mmu_values(total_data, total_execute)
> >> >
> >> >  static inline int vma_pkey(struct vm_area_struct *vma)
> >> >  {
> >> > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> >> > index ba7bff6..e61ed6c 100644
> >> > --- a/arch/powerpc/include/asm/pkeys.h
> >> > +++ b/arch/powerpc/include/asm/pkeys.h
> >> > @@ -1,6 +1,8 @@
> >> >  #ifndef _ASM_PPC64_PKEYS_H
> >> >  #define _ASM_PPC64_PKEYS_H
> >> >
> >> > +#include <asm/firmware.h>
> >> > +
> >> >  extern bool pkey_inited;
> >> >  extern int pkeys_total; /* total pkeys as per device tree */
> >> >  extern u32 initial_allocation_mask;/* bits set for reserved keys */
> >> > @@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm)
> >> >  	mm->context.execute_only_pkey = -1;
> >> >  }
> >> >
> >> > +static inline void pkey_mmu_values(int total_data, int total_execute)
> >> > +{
> >> > +	/*
> >> > +	 * since any pkey can be used for data or execute, we
> >> > +	 * will  just  treat all keys as equal and track them
> >> > +	 * as one entity.
> >> > +	 */
> >> > +	pkeys_total = total_data + total_execute;
> >> > +}
> >> 
> >> Right now this works because the firmware reports 0 execute keys in the
> >> device tree, but if (when?) it is fixed to report 32 execute keys as
> >> well as 32 data keys (which are the same keys), any place using
> >> pkeys_total expecting it to mean the number of keys that are available
> >> will be broken. This includes pkey_initialize and mm_pkey_is_allocated.
> >
> > Good point. we should just ignore total_execute. It should
> > be the same value as total_data on the latest platforms.
> > On older platforms it will continue to be zero.
> 
> Indeed. There should just be a special case to disable execute
> protection for P7.

Ok. we should disable execute protection for P7 and earlier generations of CPU.

RP.

Re: [RFC v7 26/25] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface

[Ram Pai]

On Fri, Aug 11, 2017 at 02:34:43PM -0300, Thiago Jung Bauermann wrote:
> Expose useful information for programs using memory protection keys.
> Provide implementation for powerpc and x86.
> 
> On a powerpc system with pkeys support, here is what is shown:
> 
> $ head /sys/kernel/mm/protection_keys/*
> ==> /sys/kernel/mm/protection_keys/disable_execute_supported <==
> true

We should not just call out disable_execute_supported.
disable_access_supported and disable_write_supported should also 
be called out.

> 
> ==> /sys/kernel/mm/protection_keys/total_keys <==
> 32
> 

> ==> /sys/kernel/mm/protection_keys/usable_keys <==
> 30

This is little nebulous.  It depends on how we define
usable as.  Is it the number of keys that are available
to the app?  If that is the case that value is dynamic.
Sometime the OS steals one key for execute-only key.
And anything that is dynamic can be inherently racy.
So I think we should define 'usable' as guaranteed number
of keys available to the app and display a value that is
one less than what is available.

in the above example the value should be 29.

RP

--
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: [RFC v7 26/25] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface

[Ram Pai]

On Fri, Aug 11, 2017 at 02:34:43PM -0300, Thiago Jung Bauermann wrote:
> Expose useful information for programs using memory protection keys.
> Provide implementation for powerpc and x86.
> 
> On a powerpc system with pkeys support, here is what is shown:
> 
> $ head /sys/kernel/mm/protection_keys/*
> ==> /sys/kernel/mm/protection_keys/disable_execute_supported <==
> true

We should not just call out disable_execute_supported.
disable_access_supported and disable_write_supported should also 
be called out.

> 
> ==> /sys/kernel/mm/protection_keys/total_keys <==
> 32
> 

> ==> /sys/kernel/mm/protection_keys/usable_keys <==
> 30

This is little nebulous.  It depends on how we define
usable as.  Is it the number of keys that are available
to the app?  If that is the case that value is dynamic.
Sometime the OS steals one key for execute-only key.
And anything that is dynamic can be inherently racy.
So I think we should define 'usable' as guaranteed number
of keys available to the app and display a value that is
one less than what is available.

in the above example the value should be 29.

RP

Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation

[Michael Ellerman]

Ram Pai <linuxram@us.ibm.com> writes:
> On Fri, Aug 11, 2017 at 08:26:30PM +1000, Michael Ellerman wrote:
>> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
>> 
>> > Ram Pai <linuxram@us.ibm.com> writes:
>> >
>> >> The value of the AMR register at the time of exception
>> >> is made available in gp_regs[PT_AMR] of the siginfo.
...
>> 
>> I don't understand why we are putting it in there at all?
>> 
>> Is there some special handling of the actual register on signals? I
>> haven't seen it. In which case the process can get the value of AMR by
>> reading the register. ??
>
> The value of AMR register at the time of the key-exception may not be
> the same when the signal handler is invoked. 

Why not?

cheers

Re: [RFC v7 25/25] powerpc: Enable pkey subsystem

[Michael Ellerman]

Ram Pai <linuxram@us.ibm.com> writes:
> On Thu, Aug 17, 2017 at 05:30:27PM -0300, Thiago Jung Bauermann wrote:
>> Ram Pai <linuxram@us.ibm.com> writes:
>> > On Thu, Aug 10, 2017 at 06:27:34PM -0300, Thiago Jung Bauermann wrote:
>> >> Ram Pai <linuxram@us.ibm.com> writes:
>> >> > @@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm)
>> >> >  	mm->context.execute_only_pkey = -1;
>> >> >  }
>> >> >
>> >> > +static inline void pkey_mmu_values(int total_data, int total_execute)
>> >> > +{
>> >> > +	/*
>> >> > +	 * since any pkey can be used for data or execute, we
>> >> > +	 * will  just  treat all keys as equal and track them
>> >> > +	 * as one entity.
>> >> > +	 */
>> >> > +	pkeys_total = total_data + total_execute;
>> >> > +}
>> >> 
>> >> Right now this works because the firmware reports 0 execute keys in the
>> >> device tree, but if (when?) it is fixed to report 32 execute keys as
>> >> well as 32 data keys (which are the same keys), any place using
>> >> pkeys_total expecting it to mean the number of keys that are available
>> >> will be broken. This includes pkey_initialize and mm_pkey_is_allocated.
>> >
>> > Good point. we should just ignore total_execute. It should
>> > be the same value as total_data on the latest platforms.
>> > On older platforms it will continue to be zero.
>> 
>> Indeed. There should just be a special case to disable execute
>> protection for P7.
>
> Ok. we should disable execute protection for P7 and earlier generations of CPU.

You should do what the device tree says you can do.

If it says there are no execute keys then you shouldn't touch the IAMR.

If you don't want to handle the case where there are 0 execute keys but
some data keys then you should do:

  total_keys = min(data_keys, exec_keys);


cheers

Re: [RFC v7 25/25] powerpc: Enable pkey subsystem

[Thiago Jung Bauermann]

Michael Ellerman <mpe@ellerman.id.au> writes:

> Ram Pai <linuxram@us.ibm.com> writes:
>> On Thu, Aug 17, 2017 at 05:30:27PM -0300, Thiago Jung Bauermann wrote:
>>> Ram Pai <linuxram@us.ibm.com> writes:
>>> > On Thu, Aug 10, 2017 at 06:27:34PM -0300, Thiago Jung Bauermann wrote:
>>> >> Ram Pai <linuxram@us.ibm.com> writes:
>>> >> > @@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm)
>>> >> >  	mm->context.execute_only_pkey = -1;
>>> >> >  }
>>> >> >
>>> >> > +static inline void pkey_mmu_values(int total_data, int total_execute)
>>> >> > +{
>>> >> > +	/*
>>> >> > +	 * since any pkey can be used for data or execute, we
>>> >> > +	 * will  just  treat all keys as equal and track them
>>> >> > +	 * as one entity.
>>> >> > +	 */
>>> >> > +	pkeys_total = total_data + total_execute;
>>> >> > +}
>>> >> 
>>> >> Right now this works because the firmware reports 0 execute keys in the
>>> >> device tree, but if (when?) it is fixed to report 32 execute keys as
>>> >> well as 32 data keys (which are the same keys), any place using
>>> >> pkeys_total expecting it to mean the number of keys that are available
>>> >> will be broken. This includes pkey_initialize and mm_pkey_is_allocated.
>>> >
>>> > Good point. we should just ignore total_execute. It should
>>> > be the same value as total_data on the latest platforms.
>>> > On older platforms it will continue to be zero.
>>> 
>>> Indeed. There should just be a special case to disable execute
>>> protection for P7.
>>
>> Ok. we should disable execute protection for P7 and earlier generations of CPU.
>
> You should do what the device tree says you can do.
>
> If it says there are no execute keys then you shouldn't touch the IAMR.

The downside of that approach is that the device tree in P8 LPARs
currently says there are no execute keys even though there are. We'd
have to require customers to upgrade their firmware to a fixed version
if they want to use execute keys.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC v7 25/25] powerpc: Enable pkey subsystem

[Ram Pai]

On Fri, Aug 18, 2017 at 12:26:33PM -0300, Thiago Jung Bauermann wrote:
> 
> Michael Ellerman <mpe@ellerman.id.au> writes:
> 
> > Ram Pai <linuxram@us.ibm.com> writes:
> >> On Thu, Aug 17, 2017 at 05:30:27PM -0300, Thiago Jung Bauermann wrote:
> >>> Ram Pai <linuxram@us.ibm.com> writes:
> >>> > On Thu, Aug 10, 2017 at 06:27:34PM -0300, Thiago Jung Bauermann wrote:
> >>> >> Ram Pai <linuxram@us.ibm.com> writes:
> >>> >> > @@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm)
> >>> >> >  	mm->context.execute_only_pkey = -1;
> >>> >> >  }
> >>> >> >
> >>> >> > +static inline void pkey_mmu_values(int total_data, int total_execute)
> >>> >> > +{
> >>> >> > +	/*
> >>> >> > +	 * since any pkey can be used for data or execute, we
> >>> >> > +	 * will  just  treat all keys as equal and track them
> >>> >> > +	 * as one entity.
> >>> >> > +	 */
> >>> >> > +	pkeys_total = total_data + total_execute;
> >>> >> > +}
> >>> >> 
> >>> >> Right now this works because the firmware reports 0 execute keys in the
> >>> >> device tree, but if (when?) it is fixed to report 32 execute keys as
> >>> >> well as 32 data keys (which are the same keys), any place using
> >>> >> pkeys_total expecting it to mean the number of keys that are available
> >>> >> will be broken. This includes pkey_initialize and mm_pkey_is_allocated.
> >>> >
> >>> > Good point. we should just ignore total_execute. It should
> >>> > be the same value as total_data on the latest platforms.
> >>> > On older platforms it will continue to be zero.
> >>> 
> >>> Indeed. There should just be a special case to disable execute
> >>> protection for P7.
> >>
> >> Ok. we should disable execute protection for P7 and earlier generations of CPU.
> >
> > You should do what the device tree says you can do.
> >
> > If it says there are no execute keys then you shouldn't touch the IAMR.
> 
> The downside of that approach is that the device tree in P8 LPARs
> currently says there are no execute keys even though there are. We'd
> have to require customers to upgrade their firmware to a fixed version
> if they want to use execute keys.

Correct. the device tree for this property currently does not correctly
capture the number of execute keys.

On skiboot based systems, there is not device tree property to refer to
aswell. Thiago has a patch to fix it, but existing systems without the
skiboot fix, will not expose that property.

So unfortunately we will have to rely on multiple peices of information
to enable the pkey system in the kernel.


RP

> 
> -- 
> Thiago Jung Bauermann
> IBM Linux Technology Center

-- 
Ram Pai

Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation

[Ram Pai]

On Fri, Aug 18, 2017 at 02:48:31PM +1000, Michael Ellerman wrote:
> Ram Pai <linuxram@us.ibm.com> writes:
> > On Fri, Aug 11, 2017 at 08:26:30PM +1000, Michael Ellerman wrote:
> >> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
> >> 
> >> > Ram Pai <linuxram@us.ibm.com> writes:
> >> >
> >> >> The value of the AMR register at the time of exception
> >> >> is made available in gp_regs[PT_AMR] of the siginfo.
> ...
> >> 
> >> I don't understand why we are putting it in there at all?
> >> 
> >> Is there some special handling of the actual register on signals? I
> >> haven't seen it. In which case the process can get the value of AMR by
> >> reading the register. ??
> >
> > The value of AMR register at the time of the key-exception may not be
> > the same when the signal handler is invoked. 
> 
> Why not?

Assume two threads of a task.

T1:  mprotect_key(foo, PAGE_SIZE, pkey=4);
T1:  set AMR to disable access for pkey 4;
T1:  key fault
    T2: set AMR to enable access to pkey 4;
T1:  fault handler called.
	This fault handler will see the new AMR and not the
	one at the time of the fault.

RP

Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation

[Benjamin Herrenschmidt]

On Fri, 2017-08-18 at 10:04 -0700, Ram Pai wrote:
> Assume two threads of a task.
> 
> T1:  mprotect_key(foo, PAGE_SIZE, pkey=4);
> T1:  set AMR to disable access for pkey 4;
> T1:  key fault
>     T2: set AMR to enable access to pkey 4;
> T1:  fault handler called.
>         This fault handler will see the new AMR and not the
>         one at the time of the fault.

You aren't context switching AMR with the threads ? Ugh... something is
very wrong then.

Ben.

Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation

[Ram Pai]

On Sat, Aug 19, 2017 at 07:54:20AM +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2017-08-18 at 10:04 -0700, Ram Pai wrote:
> > Assume two threads of a task.
> > 
> > T1:  mprotect_key(foo, PAGE_SIZE, pkey=4);
> > T1:  set AMR to disable access for pkey 4;
> > T1:  key fault
> >     T2: set AMR to enable access to pkey 4;
> > T1:  fault handler called.
> >         This fault handler will see the new AMR and not the
> >         one at the time of the fault.
> 
> You aren't context switching AMR with the threads ? Ugh... something is
> very wrong then.

I do store and restore AMR accross context switch. So nevermind; the
above problem cannot happen.

RP

Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation

[Ram Pai]

On Fri, Aug 18, 2017 at 10:04:10AM -0700, Ram Pai wrote:
> On Fri, Aug 18, 2017 at 02:48:31PM +1000, Michael Ellerman wrote:
> > Ram Pai <linuxram@us.ibm.com> writes:
> > > On Fri, Aug 11, 2017 at 08:26:30PM +1000, Michael Ellerman wrote:
> > >> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
> > >> 
> > >> > Ram Pai <linuxram@us.ibm.com> writes:
> > >> >
> > >> >> The value of the AMR register at the time of exception
> > >> >> is made available in gp_regs[PT_AMR] of the siginfo.
> > ...
> > >> 
> > >> I don't understand why we are putting it in there at all?
> > >> 
> > >> Is there some special handling of the actual register on signals? I
> > >> haven't seen it. In which case the process can get the value of AMR by
> > >> reading the register. ??
> > >
> > > The value of AMR register at the time of the key-exception may not be
> > > the same when the signal handler is invoked. 
> > 
> > Why not?
> 
> Assume two threads of a task.
> 
> T1:  mprotect_key(foo, PAGE_SIZE, pkey=4);
> T1:  set AMR to disable access for pkey 4;
> T1:  key fault
>     T2: set AMR to enable access to pkey 4;
> T1:  fault handler called.
> 	This fault handler will see the new AMR and not the
> 	one at the time of the fault.

Ok. Ben debunked my above reason. So at this point I dont have a solid
reason to defend my statement --
"The value of AMR register at the time of the key-exception may not be
the same when the signal handler is invoked."

Coming back to the your main question, "why we need to provide the
contents of AMR register to the signal handler?" --   the only reason
i can see is, probably tools like gdb and ptrace may find it useful.

And since it was suggested that content of IAMR is also useful to the
application, the value of which cannot be accessed from userspace,
it may make sense to provide both the contents.

Please suggest.
RP

Re: [RFC v7 26/25] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface

[Thiago Jung Bauermann]

Ram Pai <linuxram@us.ibm.com> writes:

> On Fri, Aug 11, 2017 at 02:34:43PM -0300, Thiago Jung Bauermann wrote:
>> Expose useful information for programs using memory protection keys.
>> Provide implementation for powerpc and x86.
>> 
>> On a powerpc system with pkeys support, here is what is shown:
>> 
>> $ head /sys/kernel/mm/protection_keys/*
>> ==> /sys/kernel/mm/protection_keys/disable_execute_supported <==
>> true
>
> We should not just call out disable_execute_supported.
> disable_access_supported and disable_write_supported should also 
> be called out.

Ok, will do in the next version.

>> ==> /sys/kernel/mm/protection_keys/total_keys <==
>> 32
>> 
>
>> ==> /sys/kernel/mm/protection_keys/usable_keys <==
>> 30
>
> This is little nebulous.  It depends on how we define
> usable as.  Is it the number of keys that are available
> to the app?  If that is the case that value is dynamic.
> Sometime the OS steals one key for execute-only key.
> And anything that is dynamic can be inherently racy.
> So I think we should define 'usable' as guaranteed number
> of keys available to the app

Yes, that is how I defined it: the difference between the number of keys
provided by the platform and the keys reserved by the OS. I do need to
spell it out somewhere inside Documentation/ though.

> and display a value that is one less than what is available.
>
> in the above example the value should be 29.

Good point, I didn't account for the execute-only key. I will make that
change in the next version.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

--
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: [RFC v7 26/25] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface

[Thiago Jung Bauermann]

Ram Pai <linuxram@us.ibm.com> writes:

> On Fri, Aug 11, 2017 at 02:34:43PM -0300, Thiago Jung Bauermann wrote:
>> Expose useful information for programs using memory protection keys.
>> Provide implementation for powerpc and x86.
>> 
>> On a powerpc system with pkeys support, here is what is shown:
>> 
>> $ head /sys/kernel/mm/protection_keys/*
>> ==> /sys/kernel/mm/protection_keys/disable_execute_supported <==
>> true
>
> We should not just call out disable_execute_supported.
> disable_access_supported and disable_write_supported should also 
> be called out.

Ok, will do in the next version.

>> ==> /sys/kernel/mm/protection_keys/total_keys <==
>> 32
>> 
>
>> ==> /sys/kernel/mm/protection_keys/usable_keys <==
>> 30
>
> This is little nebulous.  It depends on how we define
> usable as.  Is it the number of keys that are available
> to the app?  If that is the case that value is dynamic.
> Sometime the OS steals one key for execute-only key.
> And anything that is dynamic can be inherently racy.
> So I think we should define 'usable' as guaranteed number
> of keys available to the app

Yes, that is how I defined it: the difference between the number of keys
provided by the platform and the keys reserved by the OS. I do need to
spell it out somewhere inside Documentation/ though.

> and display a value that is one less than what is available.
>
> in the above example the value should be 29.

Good point, I didn't account for the execute-only key. I will make that
change in the next version.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation

[Benjamin Herrenschmidt]

On Fri, 2017-08-18 at 15:49 -0700, Ram Pai wrote:
> Coming back to the your main question, "why we need to provide the
> contents of AMR register to the signal handler?" --   the only reason
> i can see is, probably tools like gdb and ptrace may find it useful.
> 
> And since it was suggested that content of IAMR is also useful to the
> application, the value of which cannot be accessed from userspace,
> it may make sense to provide both the contents.

I'd rather you added ptrace calls to obtain & set it. I don't think we
need to touch the signal frame, it's too much of an ABI compatibility
burden.

Cheers,
Ben.

Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation

[Balbir Singh]

On Fri, 18 Aug 2017 15:36:55 -0700
Ram Pai <linuxram@us.ibm.com> wrote:

> On Sat, Aug 19, 2017 at 07:54:20AM +1000, Benjamin Herrenschmidt wrote:
> > On Fri, 2017-08-18 at 10:04 -0700, Ram Pai wrote:  
> > > Assume two threads of a task.
> > > 
> > > T1:  mprotect_key(foo, PAGE_SIZE, pkey=4);
> > > T1:  set AMR to disable access for pkey 4;
> > > T1:  key fault
> > >     T2: set AMR to enable access to pkey 4;
> > > T1:  fault handler called.
> > >         This fault handler will see the new AMR and not the
> > >         one at the time of the fault.  
> > 
> > You aren't context switching AMR with the threads ? Ugh... something is
> > very wrong then.  
> 
> I do store and restore AMR accross context switch. So nevermind; the
> above problem cannot happen.
>

I think the assumption is that pkey_alloc() will do the right thing
while allocating keys across threads

Balbir Singh.

Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys

[Balbir Singh]

On Sun, 30 Jul 2017 17:12:03 -0700
Ram Pai <linuxram@us.ibm.com> wrote:

> Total 32 keys are available on power7 and above. However
> pkey 0,1 are reserved. So effectively we  have  30 pkeys.
> 
> On 4K kernels, we do not  have  5  bits  in  the  PTE to
> represent  all the keys; we only have 3bits.Two of those
> keys are reserved; pkey 0 and pkey 1. So effectively  we
> have 6 pkeys.
> 
> This patch keeps track of reserved keys, allocated  keys
> and keys that are currently free.
> 
> Also it  adds  skeletal  functions  and macros, that the
> architecture-independent code expects to be available.
> 
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> ---
>  arch/powerpc/include/asm/book3s/64/mmu.h |    9 +++
>  arch/powerpc/include/asm/mmu_context.h   |    1 +
>  arch/powerpc/include/asm/pkeys.h         |   98 ++++++++++++++++++++++++++++-
>  arch/powerpc/mm/mmu_context_book3s64.c   |    2 +
>  arch/powerpc/mm/pkeys.c                  |    2 +
>  5 files changed, 108 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
> index 77529a3..104ad72 100644
> --- a/arch/powerpc/include/asm/book3s/64/mmu.h
> +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
> @@ -108,6 +108,15 @@ struct patb_entry {
>  #ifdef CONFIG_SPAPR_TCE_IOMMU
>  	struct list_head iommu_group_mem_list;
>  #endif
> +
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> +	/*
> +	 * Each bit represents one protection key.
> +	 * bit set   -> key allocated
> +	 * bit unset -> key available for allocation
> +	 */
> +	u32 pkey_allocation_map;
> +#endif
>  } mm_context_t;
>  
>  /*
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index 4b93547..4705dab 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -184,6 +184,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
>  
>  #ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
>  #define pkey_initialize()
> +#define pkey_mm_init(mm)
>  #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
>  
>  #endif /* __KERNEL__ */
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 4ccb8f5..def385f 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -2,6 +2,8 @@
>  #define _ASM_PPC64_PKEYS_H
>  
>  extern bool pkey_inited;
> +extern int pkeys_total; /* total pkeys as per device tree */
> +extern u32 initial_allocation_mask;/* bits set for reserved keys */
>  
>  /*
>   * powerpc needs an additional vma bit to support 32 keys.
> @@ -20,21 +22,76 @@
>  #define VM_PKEY_BIT4	VM_HIGH_ARCH_4
>  #endif
>  
> -#define ARCH_VM_PKEY_FLAGS 0
> +#define arch_max_pkey()  pkeys_total
> +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
> +				VM_PKEY_BIT3 | VM_PKEY_BIT4)
> +
> +#define pkey_alloc_mask(pkey) (0x1 << pkey)
> +
> +#define mm_pkey_allocation_map(mm)	(mm->context.pkey_allocation_map)
> +
> +#define mm_set_pkey_allocated(mm, pkey) {	\
> +	mm_pkey_allocation_map(mm) |= pkey_alloc_mask(pkey); \
> +}
> +
> +#define mm_set_pkey_free(mm, pkey) {	\
> +	mm_pkey_allocation_map(mm) &= ~pkey_alloc_mask(pkey);	\
> +}
> +
> +#define mm_set_pkey_is_allocated(mm, pkey)	\
> +	(mm_pkey_allocation_map(mm) & pkey_alloc_mask(pkey))
> +
> +#define mm_set_pkey_is_reserved(mm, pkey) (initial_allocation_mask & \
> +					pkey_alloc_mask(pkey))
>  
>  static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
>  {
> -	return (pkey == 0);
> +	/* a reserved key is never considered as 'explicitly allocated' */
> +	return ((pkey < arch_max_pkey()) &&
> +		!mm_set_pkey_is_reserved(mm, pkey) &&
> +		mm_set_pkey_is_allocated(mm, pkey));

Sounds like this should be called mm_pkey_is_free()

>  }
>  
> +/*
> + * Returns a positive, 5-bit key on success, or -1 on failure.
> + */
>  static inline int mm_pkey_alloc(struct mm_struct *mm)
>  {
> -	return -1;
> +	/*
> +	 * Note: this is the one and only place we make sure
> +	 * that the pkey is valid as far as the hardware is
> +	 * concerned.  The rest of the kernel trusts that
> +	 * only good, valid pkeys come out of here.
> +	 */
> +	u32 all_pkeys_mask = (u32)(~(0x0));
> +	int ret;
> +
> +	if (!pkey_inited)
> +		return -1;
> +	/*
> +	 * Are we out of pkeys?  We must handle this specially
> +	 * because ffz() behavior is undefined if there are no
> +	 * zeros.
> +	 */
Point A

> +	if (mm_pkey_allocation_map(mm) == all_pkeys_mask)
> +		return -1;
> +
> +	ret = ffz((u32)mm_pkey_allocation_map(mm));

Point B

So the allocation occurs from MSB to LSB?  I also think you need
a preempt disable between points A, B.

Is this code reentrant?

> +	mm_set_pkey_allocated(mm, ret);
> +	return ret;
>  }
>  
>  static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
>  {
> -	return -EINVAL;
> +	if (!pkey_inited)
> +		return -1;
> +
> +	if (!mm_pkey_is_allocated(mm, pkey))
> +		return -EINVAL;
> +
> +	mm_set_pkey_free(mm, pkey);
> +
> +	return 0;
>  }
>  
>  /*
> @@ -58,12 +115,45 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
>  	return 0;
>  }
>  
> +static inline void pkey_mm_init(struct mm_struct *mm)
> +{
> +	if (!pkey_inited)
> +		return;
> +	mm_pkey_allocation_map(mm) = initial_allocation_mask;
> +}
> +
>  static inline void pkey_initialize(void)
>  {
> +	int os_reserved, i;
> +
>  	/* disable the pkey system till everything
>  	 * is in place. A patch further down the
>  	 * line will enable it.
>  	 */
>  	pkey_inited = false;

Why are we doing it this way?

> +
> +	/* Lets assume 32 keys */
> +	pkeys_total = 32;
> +
> +#ifdef CONFIG_PPC_4K_PAGES
> +	/*
> +	 * the OS can manage only 8 pkeys
> +	 * due to its inability to represent
> +	 * them in the linux 4K-PTE.
> +	 */
> +	os_reserved = pkeys_total-8;
> +#else
> +	os_reserved = 0;
> +#endif
> +	/*
> +	 * Bits are in LE format.
> +	 * NOTE: 1, 0 are reserved.
> +	 * key 0 is the default key, which allows read/write/execute.
> +	 * key 1 is recommended not to be used.
> +	 * PowerISA(3.0) page 1015, programming note.
> +	 */
> +	initial_allocation_mask = ~0x0;
> +	for (i = 2; i < (pkeys_total - os_reserved); i++)
> +		initial_allocation_mask &= ~(0x1<<i);

On radix key 0 is used for supervisor mode to implement SMEP
and SMAP. It would be nice to reserve key 0 for the OS for
hash as well. Why are the bits in LE format? The limitations
of the adjunct partition apply to Linux?

>  }
>  #endif /*_ASM_PPC64_PKEYS_H */
> diff --git a/arch/powerpc/mm/mmu_context_book3s64.c b/arch/powerpc/mm/mmu_context_book3s64.c
> index a3edf81..34a16f3 100644
> --- a/arch/powerpc/mm/mmu_context_book3s64.c
> +++ b/arch/powerpc/mm/mmu_context_book3s64.c
> @@ -16,6 +16,7 @@
>  #include <linux/string.h>
>  #include <linux/types.h>
>  #include <linux/mm.h>
> +#include <linux/pkeys.h>
>  #include <linux/spinlock.h>
>  #include <linux/idr.h>
>  #include <linux/export.h>
> @@ -120,6 +121,7 @@ static int hash__init_new_context(struct mm_struct *mm)
>  
>  	subpage_prot_init_new_context(mm);
>  
> +	pkey_mm_init(mm);
>  	return index;
>  }
>  
> diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
> index c3acee1..37dacc5 100644
> --- a/arch/powerpc/mm/pkeys.c
> +++ b/arch/powerpc/mm/pkeys.c
> @@ -16,3 +16,5 @@
>  #include <linux/pkeys.h>                /* PKEY_*                       */
>  
>  bool pkey_inited;
> +int  pkeys_total;		/* total pkeys as per device tree */
> +u32  initial_allocation_mask;	/* bits set for reserved keys */

Balbir

Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation

[Ram Pai]

On Wed, Oct 18, 2017 at 01:25:48PM +1100, Balbir Singh wrote:
> On Fri, 18 Aug 2017 15:36:55 -0700
> Ram Pai <linuxram@us.ibm.com> wrote:
> 
> > On Sat, Aug 19, 2017 at 07:54:20AM +1000, Benjamin Herrenschmidt wrote:
> > > On Fri, 2017-08-18 at 10:04 -0700, Ram Pai wrote:  
> > > > Assume two threads of a task.
> > > > 
> > > > T1:  mprotect_key(foo, PAGE_SIZE, pkey=4);
> > > > T1:  set AMR to disable access for pkey 4;
> > > > T1:  key fault
> > > >     T2: set AMR to enable access to pkey 4;
> > > > T1:  fault handler called.
> > > >         This fault handler will see the new AMR and not the
> > > >         one at the time of the fault.  
> > > 
> > > You aren't context switching AMR with the threads ? Ugh... something is
> > > very wrong then.  
> > 
> > I do store and restore AMR accross context switch. So nevermind; the
> > above problem cannot happen.
> >
> 
> I think the assumption is that pkey_alloc() will do the right thing
> while allocating keys across threads

It does.  A key allocated to a thread will never be allocated to another
thread.

RP

Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys

[Ram Pai]

On Wed, Oct 18, 2017 at 01:42:49PM +1100, Balbir Singh wrote:
> On Sun, 30 Jul 2017 17:12:03 -0700
> Ram Pai <linuxram@us.ibm.com> wrote:
> 
> > Total 32 keys are available on power7 and above. However
> > pkey 0,1 are reserved. So effectively we  have  30 pkeys.
> > 
> > On 4K kernels, we do not  have  5  bits  in  the  PTE to
> > represent  all the keys; we only have 3bits.Two of those
> > keys are reserved; pkey 0 and pkey 1. So effectively  we
> > have 6 pkeys.
> > 
> > This patch keeps track of reserved keys, allocated  keys
> > and keys that are currently free.
> > 
> > Also it  adds  skeletal  functions  and macros, that the
> > architecture-independent code expects to be available.
> > 
> > Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> > ---
> >  arch/powerpc/include/asm/book3s/64/mmu.h |    9 +++
> >  arch/powerpc/include/asm/mmu_context.h   |    1 +
> >  arch/powerpc/include/asm/pkeys.h         |   98 ++++++++++++++++++++++++++++-
> >  arch/powerpc/mm/mmu_context_book3s64.c   |    2 +
> >  arch/powerpc/mm/pkeys.c                  |    2 +
> >  5 files changed, 108 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
> > index 77529a3..104ad72 100644
> > --- a/arch/powerpc/include/asm/book3s/64/mmu.h
> > +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
> > @@ -108,6 +108,15 @@ struct patb_entry {
> >  #ifdef CONFIG_SPAPR_TCE_IOMMU
> >  	struct list_head iommu_group_mem_list;
> >  #endif
> > +
> > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> > +	/*
> > +	 * Each bit represents one protection key.
> > +	 * bit set   -> key allocated
> > +	 * bit unset -> key available for allocation
> > +	 */
> > +	u32 pkey_allocation_map;
> > +#endif
> >  } mm_context_t;
> >  
> >  /*
> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> > index 4b93547..4705dab 100644
> > --- a/arch/powerpc/include/asm/mmu_context.h
> > +++ b/arch/powerpc/include/asm/mmu_context.h
> > @@ -184,6 +184,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
> >  
> >  #ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> >  #define pkey_initialize()
> > +#define pkey_mm_init(mm)
> >  #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> >  
> >  #endif /* __KERNEL__ */
> > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> > index 4ccb8f5..def385f 100644
> > --- a/arch/powerpc/include/asm/pkeys.h
> > +++ b/arch/powerpc/include/asm/pkeys.h
> > @@ -2,6 +2,8 @@
> >  #define _ASM_PPC64_PKEYS_H
> >  
> >  extern bool pkey_inited;
> > +extern int pkeys_total; /* total pkeys as per device tree */
> > +extern u32 initial_allocation_mask;/* bits set for reserved keys */
> >  
> >  /*
> >   * powerpc needs an additional vma bit to support 32 keys.
> > @@ -20,21 +22,76 @@
> >  #define VM_PKEY_BIT4	VM_HIGH_ARCH_4
> >  #endif
> >  
> > -#define ARCH_VM_PKEY_FLAGS 0
> > +#define arch_max_pkey()  pkeys_total
> > +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
> > +				VM_PKEY_BIT3 | VM_PKEY_BIT4)
> > +
> > +#define pkey_alloc_mask(pkey) (0x1 << pkey)
> > +
> > +#define mm_pkey_allocation_map(mm)	(mm->context.pkey_allocation_map)
> > +
> > +#define mm_set_pkey_allocated(mm, pkey) {	\
> > +	mm_pkey_allocation_map(mm) |= pkey_alloc_mask(pkey); \
> > +}
> > +
> > +#define mm_set_pkey_free(mm, pkey) {	\
> > +	mm_pkey_allocation_map(mm) &= ~pkey_alloc_mask(pkey);	\
> > +}
> > +
> > +#define mm_set_pkey_is_allocated(mm, pkey)	\
> > +	(mm_pkey_allocation_map(mm) & pkey_alloc_mask(pkey))
> > +
> > +#define mm_set_pkey_is_reserved(mm, pkey) (initial_allocation_mask & \
> > +					pkey_alloc_mask(pkey))
> >  
> >  static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
> >  {
> > -	return (pkey == 0);
> > +	/* a reserved key is never considered as 'explicitly allocated' */
> > +	return ((pkey < arch_max_pkey()) &&
> > +		!mm_set_pkey_is_reserved(mm, pkey) &&
> > +		mm_set_pkey_is_allocated(mm, pkey));
> 
> Sounds like this should be called mm_pkey_is_free()

hmm. why?

> 
> >  }
> >  
> > +/*
> > + * Returns a positive, 5-bit key on success, or -1 on failure.
> > + */
> >  static inline int mm_pkey_alloc(struct mm_struct *mm)
> >  {
> > -	return -1;
> > +	/*
> > +	 * Note: this is the one and only place we make sure
> > +	 * that the pkey is valid as far as the hardware is
> > +	 * concerned.  The rest of the kernel trusts that
> > +	 * only good, valid pkeys come out of here.
> > +	 */
> > +	u32 all_pkeys_mask = (u32)(~(0x0));
> > +	int ret;
> > +
> > +	if (!pkey_inited)
> > +		return -1;
> > +	/*
> > +	 * Are we out of pkeys?  We must handle this specially
> > +	 * because ffz() behavior is undefined if there are no
> > +	 * zeros.
> > +	 */
> Point A
> 
> > +	if (mm_pkey_allocation_map(mm) == all_pkeys_mask)
> > +		return -1;
> > +
> > +	ret = ffz((u32)mm_pkey_allocation_map(mm));
> 
> Point B
> 
> So the allocation occurs from MSB to LSB?

ffz() allocates bits from right to left. On BE systems
it will be from MSB to LSB, and on LE systems it will be from LSB
to MSB. right?


> I also think you need
> a preempt disable between points A, B.
> 
> Is this code reentrant?


mm_pkey_alloc() is called holding the mmap_sem. So it cannot race
with itself.

> 
> > +	mm_set_pkey_allocated(mm, ret);
> > +	return ret;
> >  }
> >  
> >  static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
> >  {
> > -	return -EINVAL;
> > +	if (!pkey_inited)
> > +		return -1;
> > +
> > +	if (!mm_pkey_is_allocated(mm, pkey))
> > +		return -EINVAL;
> > +
> > +	mm_set_pkey_free(mm, pkey);
> > +
> > +	return 0;
> >  }
> >  
> >  /*
> > @@ -58,12 +115,45 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> >  	return 0;
> >  }
> >  
> > +static inline void pkey_mm_init(struct mm_struct *mm)
> > +{
> > +	if (!pkey_inited)
> > +		return;
> > +	mm_pkey_allocation_map(mm) = initial_allocation_mask;
> > +}
> > +
> >  static inline void pkey_initialize(void)
> >  {
> > +	int os_reserved, i;
> > +
> >  	/* disable the pkey system till everything
> >  	 * is in place. A patch further down the
> >  	 * line will enable it.
> >  	 */
> >  	pkey_inited = false;
> 
> Why are we doing it this way?

many factors .... led to this organization. It could have been done
differently, i suppose.  but it organically grew to this state.

Running down the memory lane, i recall, I had to introduce many skeletal
functions as soon as the PROTECTION_KEY configuration option got
enabled.  The arch neutral code called into them, and since these
skeletal function did nothing, bad thing could happen. So had to
disable everything till everything was in place.

> 
> > +
> > +	/* Lets assume 32 keys */
> > +	pkeys_total = 32;
> > +
> > +#ifdef CONFIG_PPC_4K_PAGES
> > +	/*
> > +	 * the OS can manage only 8 pkeys
> > +	 * due to its inability to represent
> > +	 * them in the linux 4K-PTE.
> > +	 */
> > +	os_reserved = pkeys_total-8;
> > +#else
> > +	os_reserved = 0;
> > +#endif
> > +	/*
> > +	 * Bits are in LE format.
> > +	 * NOTE: 1, 0 are reserved.
> > +	 * key 0 is the default key, which allows read/write/execute.
> > +	 * key 1 is recommended not to be used.
> > +	 * PowerISA(3.0) page 1015, programming note.
> > +	 */
> > +	initial_allocation_mask = ~0x0;
> > +	for (i = 2; i < (pkeys_total - os_reserved); i++)
> > +		initial_allocation_mask &= ~(0x1<<i);
> 
> On radix key 0 is used for supervisor mode to implement SMEP
> and SMAP. It would be nice to reserve key 0 for the OS for
> hash as well. Why are the bits in LE format? The limitations
> of the adjunct partition apply to Linux?

We dont allocate Key-0 to the application. Key-0 is considered as reserved.

> 
> >  }
> >  #endif /*_ASM_PPC64_PKEYS_H */
> > diff --git a/arch/powerpc/mm/mmu_context_book3s64.c b/arch/powerpc/mm/mmu_context_book3s64.c
> > index a3edf81..34a16f3 100644
> > --- a/arch/powerpc/mm/mmu_context_book3s64.c
> > +++ b/arch/powerpc/mm/mmu_context_book3s64.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/string.h>
> >  #include <linux/types.h>
> >  #include <linux/mm.h>
> > +#include <linux/pkeys.h>
> >  #include <linux/spinlock.h>
> >  #include <linux/idr.h>
> >  #include <linux/export.h>
> > @@ -120,6 +121,7 @@ static int hash__init_new_context(struct mm_struct *mm)
> >  
> >  	subpage_prot_init_new_context(mm);
> >  
> > +	pkey_mm_init(mm);
> >  	return index;
> >  }
> >  
> > diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
> > index c3acee1..37dacc5 100644
> > --- a/arch/powerpc/mm/pkeys.c
> > +++ b/arch/powerpc/mm/pkeys.c
> > @@ -16,3 +16,5 @@
> >  #include <linux/pkeys.h>                /* PKEY_*                       */
> >  
> >  bool pkey_inited;
> > +int  pkeys_total;		/* total pkeys as per device tree */
> > +u32  initial_allocation_mask;	/* bits set for reserved keys */

Thanks for the review.
RP

Re: [RFC v7 12/25] powerpc: implementation for arch_override_mprotect_pkey()

[Laurent Dufour]

Hi Ram,

On 31/07/2017 02:12, Ram Pai wrote:
> arch independent code calls arch_override_mprotect_pkey()
> to return a pkey that best matches the requested protection.
> 
> This patch provides the implementation.
> 
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> ---
>  arch/powerpc/include/asm/mmu_context.h |    5 +++
>  arch/powerpc/include/asm/pkeys.h       |   17 ++++++++++-
>  arch/powerpc/mm/pkeys.c                |   47 ++++++++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index 4705dab..7232484 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -185,6 +185,11 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
>  #ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
>  #define pkey_initialize()
>  #define pkey_mm_init(mm)
> +
> +static inline int vma_pkey(struct vm_area_struct *vma)
> +{
> +	return 0;
> +}
>  #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> 
>  #endif /* __KERNEL__ */
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index a715a08..03f7ea2 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -46,6 +46,16 @@ static inline u64 pkey_to_vmflag_bits(u16 pkey)
>  		((pkey & 0x10UL) ? VM_PKEY_BIT4 : 0x0UL));
>  }
> 
> +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
> +				VM_PKEY_BIT3 | VM_PKEY_BIT4)
> +
> +static inline int vma_pkey(struct vm_area_struct *vma)
> +{
> +	if (!pkey_inited)
> +		return 0;
> +	return (vma->vm_flags & ARCH_VM_PKEY_FLAGS) >> VM_PKEY_SHIFT;
> +}
> +
>  #define arch_max_pkey()  pkeys_total
>  #define AMR_RD_BIT 0x1UL
>  #define AMR_WR_BIT 0x2UL
> @@ -146,11 +156,14 @@ static inline int execute_only_pkey(struct mm_struct *mm)
>  	return __execute_only_pkey(mm);
>  }
> 
> -
> +extern int __arch_override_mprotect_pkey(struct vm_area_struct *vma,
> +		int prot, int pkey);
>  static inline int arch_override_mprotect_pkey(struct vm_area_struct *vma,
>  		int prot, int pkey)
>  {
> -	return 0;
> +	if (!pkey_inited)
> +		return 0;
> +	return __arch_override_mprotect_pkey(vma, prot, pkey);
>  }
> 
>  extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
> index 25749bf..edcbf48 100644
> --- a/arch/powerpc/mm/pkeys.c
> +++ b/arch/powerpc/mm/pkeys.c
> @@ -154,3 +154,50 @@ int __execute_only_pkey(struct mm_struct *mm)
>  		mm->context.execute_only_pkey = execute_only_pkey;
>  	return execute_only_pkey;
>  }
> +
> +static inline bool vma_is_pkey_exec_only(struct vm_area_struct *vma)
> +{
> +	/* Do this check first since the vm_flags should be hot */
> +	if ((vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)) != VM_EXEC)
> +		return false;
> +
> +	return (vma_pkey(vma) == vma->vm_mm->context.execute_only_pkey);
> +}
> +
> +/*
> + * This should only be called for *plain* mprotect calls.
> + */
> +int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot,
> +		int pkey)
> +{
> +	/*
> +	 * Is this an mprotect_pkey() call?  If so, never
> +	 * override the value that came from the user.
> +	 */
> +	if (pkey != -1)
> +		return pkey;

This check should be moved in arch_override_mprotect_pkey() in
arch/powerpc/include/asm/pkeys.h

> +
> +	/*
> +	 * If the currently associated pkey is execute-only,
> +	 * but the requested protection requires read or write,
> +	 * move it back to the default pkey.
> +	 */
> +	if (vma_is_pkey_exec_only(vma) &&
> +	    (prot & (PROT_READ|PROT_WRITE)))
> +		return 0;
> +
> +	/*
> +	 * the requested protection is execute-only. Hence
> +	 * lets use a execute-only pkey.
> +	 */
> +	if (prot == PROT_EXEC) {
> +		pkey = execute_only_pkey(vma->vm_mm);
> +		if (pkey > 0)
> +			return pkey;
> +	}
> +
> +	/*
> +	 * nothing to override.
> +	 */
> +	return vma_pkey(vma);
> +}
> 

Re: [RFC v7 16/25] powerpc: helper to validate key-access permissions of a pte

[Laurent Dufour]

On 31/07/2017 02:12, Ram Pai wrote:
> helper function that checks if the read/write/execute is allowed
> on the pte.
> 
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> ---
>  arch/powerpc/include/asm/book3s/64/pgtable.h |    4 +++
>  arch/powerpc/include/asm/pkeys.h             |   12 +++++++++++
>  arch/powerpc/mm/pkeys.c                      |   28 ++++++++++++++++++++++++++
>  3 files changed, 44 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index 060a1b2..2bec0f6 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -477,6 +477,10 @@ static inline void write_uamor(u64 value)
>  	mtspr(SPRN_UAMOR, value);
>  }
> 
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> +extern bool arch_pte_access_permitted(u64 pte, bool write, bool execute);
> +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> +
>  #define __HAVE_ARCH_PTEP_GET_AND_CLEAR
>  static inline pte_t ptep_get_and_clear(struct mm_struct *mm,
>  				       unsigned long addr, pte_t *ptep)
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 4b7e3f4..ba7bff6 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -85,6 +85,18 @@ static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
>  		((pteflags & H_PAGE_PKEY_BIT4) ? HPTE_R_KEY_BIT4 : 0x0UL));
>  }
> 
> +static inline u16 pte_to_pkey_bits(u64 pteflags)
> +{
> +	if (!pkey_inited)
> +		return 0x0UL;

Is it really needed to make such a check in this low level function ?
The only caller is already checking for pkey_inited before making the call.

> +
> +	return (((pteflags & H_PAGE_PKEY_BIT0) ? 0x10 : 0x0UL) |
> +		((pteflags & H_PAGE_PKEY_BIT1) ? 0x8 : 0x0UL) |
> +		((pteflags & H_PAGE_PKEY_BIT2) ? 0x4 : 0x0UL) |
> +		((pteflags & H_PAGE_PKEY_BIT3) ? 0x2 : 0x0UL) |
> +		((pteflags & H_PAGE_PKEY_BIT4) ? 0x1 : 0x0UL));
> +}
> +
>  #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
>  				VM_PKEY_BIT3 | VM_PKEY_BIT4)
>  #define AMR_BITS_PER_PKEY 2
> diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
> index edcbf48..8d756ef 100644
> --- a/arch/powerpc/mm/pkeys.c
> +++ b/arch/powerpc/mm/pkeys.c
> @@ -201,3 +201,31 @@ int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot,
>  	 */
>  	return vma_pkey(vma);
>  }
> +
> +static bool pkey_access_permitted(int pkey, bool write, bool execute)
> +{
> +	int pkey_shift;
> +	u64 amr;
> +
> +	if (!pkey)
> +		return true;
> +
> +	pkey_shift = pkeyshift(pkey);
> +	if (!(read_uamor() & (0x3UL << pkey_shift)))
> +		return true;
> +
> +	if (execute && !(read_iamr() & (IAMR_EX_BIT << pkey_shift)))
> +		return true;
> +
> +	amr = read_amr(); /* delay reading amr uptil absolutely needed*/
> +	return ((!write && !(amr & (AMR_RD_BIT << pkey_shift))) ||
> +		(write &&  !(amr & (AMR_WR_BIT << pkey_shift))));
> +}
> +
> +bool arch_pte_access_permitted(u64 pte, bool write, bool execute)
> +{
> +	if (!pkey_inited)
> +		return true;
> +	return pkey_access_permitted(pte_to_pkey_bits(pte),
> +			write, execute);
> +}
> 

Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys

[Laurent Dufour]

Hi Ram,

On 31/07/2017 02:12, Ram Pai wrote:
> Total 32 keys are available on power7 and above. However
> pkey 0,1 are reserved. So effectively we  have  30 pkeys.
> 
> On 4K kernels, we do not  have  5  bits  in  the  PTE to
> represent  all the keys; we only have 3bits.Two of those
> keys are reserved; pkey 0 and pkey 1. So effectively  we
> have 6 pkeys.

IIUC, the pkey 0 and 1 are reserved by the hardware, and the kernel PTE has
only 5 bits to keep track of the pkey. Why hw pkey 0 and 1 has to be
represented in the kernel PTE ?

> This patch keeps track of reserved keys, allocated  keys
> and keys that are currently free.
> 
> Also it  adds  skeletal  functions  and macros, that the
> architecture-independent code expects to be available.
> 
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> ---
>  arch/powerpc/include/asm/book3s/64/mmu.h |    9 +++
>  arch/powerpc/include/asm/mmu_context.h   |    1 +
>  arch/powerpc/include/asm/pkeys.h         |   98 ++++++++++++++++++++++++++++-
>  arch/powerpc/mm/mmu_context_book3s64.c   |    2 +
>  arch/powerpc/mm/pkeys.c                  |    2 +
>  5 files changed, 108 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
> index 77529a3..104ad72 100644
> --- a/arch/powerpc/include/asm/book3s/64/mmu.h
> +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
> @@ -108,6 +108,15 @@ struct patb_entry {
>  #ifdef CONFIG_SPAPR_TCE_IOMMU
>  	struct list_head iommu_group_mem_list;
>  #endif
> +
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> +	/*
> +	 * Each bit represents one protection key.
> +	 * bit set   -> key allocated
> +	 * bit unset -> key available for allocation
> +	 */
> +	u32 pkey_allocation_map;
> +#endif
>  } mm_context_t;
> 
>  /*
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index 4b93547..4705dab 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -184,6 +184,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
> 
>  #ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
>  #define pkey_initialize()
> +#define pkey_mm_init(mm)
>  #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> 
>  #endif /* __KERNEL__ */
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 4ccb8f5..def385f 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -2,6 +2,8 @@
>  #define _ASM_PPC64_PKEYS_H
> 
>  extern bool pkey_inited;
> +extern int pkeys_total; /* total pkeys as per device tree */
> +extern u32 initial_allocation_mask;/* bits set for reserved keys */
> 
>  /*
>   * powerpc needs an additional vma bit to support 32 keys.
> @@ -20,21 +22,76 @@
>  #define VM_PKEY_BIT4	VM_HIGH_ARCH_4
>  #endif
> 
> -#define ARCH_VM_PKEY_FLAGS 0
> +#define arch_max_pkey()  pkeys_total
> +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
> +				VM_PKEY_BIT3 | VM_PKEY_BIT4)
> +
> +#define pkey_alloc_mask(pkey) (0x1 << pkey)
> +
> +#define mm_pkey_allocation_map(mm)	(mm->context.pkey_allocation_map)
> +
> +#define mm_set_pkey_allocated(mm, pkey) {	\
> +	mm_pkey_allocation_map(mm) |= pkey_alloc_mask(pkey); \
> +}
> +
> +#define mm_set_pkey_free(mm, pkey) {	\
> +	mm_pkey_allocation_map(mm) &= ~pkey_alloc_mask(pkey);	\
> +}
> +
> +#define mm_set_pkey_is_allocated(mm, pkey)	\
> +	(mm_pkey_allocation_map(mm) & pkey_alloc_mask(pkey))
> +
> +#define mm_set_pkey_is_reserved(mm, pkey) (initial_allocation_mask & \
> +					pkey_alloc_mask(pkey))

This macro doesn't need a 'mm' argument.

>  static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
>  {
> -	return (pkey == 0);
> +	/* a reserved key is never considered as 'explicitly allocated' */
> +	return ((pkey < arch_max_pkey()) &&
> +		!mm_set_pkey_is_reserved(mm, pkey) &&
> +		mm_set_pkey_is_allocated(mm, pkey));
>  }
> 
> +/*
> + * Returns a positive, 5-bit key on success, or -1 on failure.

I guess you rely on the mmap_sem to protect against concurrency in
mm_pkey_alloc() and mm_pkey_free().
As this is not explicit in the code, it should at least be mentioned in the
comment describing the function.

> + */
>  static inline int mm_pkey_alloc(struct mm_struct *mm)
>  {
> -	return -1;
> +	/*
> +	 * Note: this is the one and only place we make sure
> +	 * that the pkey is valid as far as the hardware is
> +	 * concerned.  The rest of the kernel trusts that
> +	 * only good, valid pkeys come out of here.
> +	 */
> +	u32 all_pkeys_mask = (u32)(~(0x0));
> +	int ret;
> +
> +	if (!pkey_inited)
> +		return -1;
> +	/*
> +	 * Are we out of pkeys?  We must handle this specially
> +	 * because ffz() behavior is undefined if there are no
> +	 * zeros.
> +	 */
> +	if (mm_pkey_allocation_map(mm) == all_pkeys_mask)
> +		return -1;
> +
> +	ret = ffz((u32)mm_pkey_allocation_map(mm));
> +	mm_set_pkey_allocated(mm, ret);
> +	return ret;
>  }
> 
>  static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
>  {
> -	return -EINVAL;
> +	if (!pkey_inited)
> +		return -1;
> +
> +	if (!mm_pkey_is_allocated(mm, pkey))
> +		return -EINVAL;
> +
> +	mm_set_pkey_free(mm, pkey);
> +
> +	return 0;
>  }
> 
>  /*
> @@ -58,12 +115,45 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
>  	return 0;
>  }
> 
> +static inline void pkey_mm_init(struct mm_struct *mm)
> +{
> +	if (!pkey_inited)
> +		return;
> +	mm_pkey_allocation_map(mm) = initial_allocation_mask;
> +}
> +
>  static inline void pkey_initialize(void)
>  {
> +	int os_reserved, i;
> +
>  	/* disable the pkey system till everything
>  	 * is in place. A patch further down the
>  	 * line will enable it.
>  	 */
>  	pkey_inited = false;
> +
> +	/* Lets assume 32 keys */
> +	pkeys_total = 32;
> +
> +#ifdef CONFIG_PPC_4K_PAGES
> +	/*
> +	 * the OS can manage only 8 pkeys
> +	 * due to its inability to represent
> +	 * them in the linux 4K-PTE.
> +	 */
> +	os_reserved = pkeys_total-8;
> +#else
> +	os_reserved = 0;
> +#endif
> +	/*
> +	 * Bits are in LE format.
> +	 * NOTE: 1, 0 are reserved.
> +	 * key 0 is the default key, which allows read/write/execute.
> +	 * key 1 is recommended not to be used.
> +	 * PowerISA(3.0) page 1015, programming note.
> +	 */
> +	initial_allocation_mask = ~0x0;
> +	for (i = 2; i < (pkeys_total - os_reserved); i++)
> +		initial_allocation_mask &= ~(0x1<<i);
>  }
>  #endif /*_ASM_PPC64_PKEYS_H */
> diff --git a/arch/powerpc/mm/mmu_context_book3s64.c b/arch/powerpc/mm/mmu_context_book3s64.c
> index a3edf81..34a16f3 100644
> --- a/arch/powerpc/mm/mmu_context_book3s64.c
> +++ b/arch/powerpc/mm/mmu_context_book3s64.c
> @@ -16,6 +16,7 @@
>  #include <linux/string.h>
>  #include <linux/types.h>
>  #include <linux/mm.h>
> +#include <linux/pkeys.h>
>  #include <linux/spinlock.h>
>  #include <linux/idr.h>
>  #include <linux/export.h>
> @@ -120,6 +121,7 @@ static int hash__init_new_context(struct mm_struct *mm)
> 
>  	subpage_prot_init_new_context(mm);
> 
> +	pkey_mm_init(mm);
>  	return index;
>  }
> 
> diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
> index c3acee1..37dacc5 100644
> --- a/arch/powerpc/mm/pkeys.c
> +++ b/arch/powerpc/mm/pkeys.c
> @@ -16,3 +16,5 @@
>  #include <linux/pkeys.h>                /* PKEY_*                       */
> 
>  bool pkey_inited;
> +int  pkeys_total;		/* total pkeys as per device tree */
> +u32  initial_allocation_mask;	/* bits set for reserved keys */
> 

Re: [RFC v7 15/25] powerpc: Program HPTE key protection bits

[Laurent Dufour]

On 31/07/2017 02:12, Ram Pai wrote:
> Map the PTE protection key bits to the HPTE key protection bits,
> while creating HPTE  entries.
> 
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> ---
>  arch/powerpc/include/asm/book3s/64/mmu-hash.h |    5 +++++
>  arch/powerpc/include/asm/mmu_context.h        |    6 ++++++
>  arch/powerpc/include/asm/pkeys.h              |   13 +++++++++++++
>  arch/powerpc/mm/hash_utils_64.c               |    1 +
>  4 files changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> index 6981a52..f7a6ed3 100644
> --- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> +++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> @@ -90,6 +90,8 @@
>  #define HPTE_R_PP0		ASM_CONST(0x8000000000000000)
>  #define HPTE_R_TS		ASM_CONST(0x4000000000000000)
>  #define HPTE_R_KEY_HI		ASM_CONST(0x3000000000000000)
> +#define HPTE_R_KEY_BIT0		ASM_CONST(0x2000000000000000)
> +#define HPTE_R_KEY_BIT1		ASM_CONST(0x1000000000000000)
>  #define HPTE_R_RPN_SHIFT	12
>  #define HPTE_R_RPN		ASM_CONST(0x0ffffffffffff000)
>  #define HPTE_R_RPN_3_0		ASM_CONST(0x01fffffffffff000)
> @@ -104,6 +106,9 @@
>  #define HPTE_R_C		ASM_CONST(0x0000000000000080)
>  #define HPTE_R_R		ASM_CONST(0x0000000000000100)
>  #define HPTE_R_KEY_LO		ASM_CONST(0x0000000000000e00)
> +#define HPTE_R_KEY_BIT2		ASM_CONST(0x0000000000000800)
> +#define HPTE_R_KEY_BIT3		ASM_CONST(0x0000000000000400)
> +#define HPTE_R_KEY_BIT4		ASM_CONST(0x0000000000000200)
> 
>  #define HPTE_V_1TB_SEG		ASM_CONST(0x4000000000000000)
>  #define HPTE_V_VRMA_MASK	ASM_CONST(0x4001ffffff000000)
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index 7232484..2eb7f80 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -190,6 +190,12 @@ static inline int vma_pkey(struct vm_area_struct *vma)
>  {
>  	return 0;
>  }
> +
> +static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
> +{
> +	return 0x0UL;
> +}
> +
>  #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> 
>  #endif /* __KERNEL__ */
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 1ded6df..4b7e3f4 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -72,6 +72,19 @@ static inline int vma_pkey(struct vm_area_struct *vma)
>  #define AMR_RD_BIT 0x1UL
>  #define AMR_WR_BIT 0x2UL
>  #define IAMR_EX_BIT 0x1UL
> +
> +static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
> +{
> +	if (!pkey_inited)
> +		return 0x0UL;

Why making such a check here, is it to avoid the following check during the
boot process only ?
IIUC, there is no way to get H_PAGE_PKEY_BIT* set when pkey_inited is false.

> +
> +	return (((pteflags & H_PAGE_PKEY_BIT0) ? HPTE_R_KEY_BIT0 : 0x0UL) |
> +		((pteflags & H_PAGE_PKEY_BIT1) ? HPTE_R_KEY_BIT1 : 0x0UL) |
> +		((pteflags & H_PAGE_PKEY_BIT2) ? HPTE_R_KEY_BIT2 : 0x0UL) |
> +		((pteflags & H_PAGE_PKEY_BIT3) ? HPTE_R_KEY_BIT3 : 0x0UL) |
> +		((pteflags & H_PAGE_PKEY_BIT4) ? HPTE_R_KEY_BIT4 : 0x0UL));
> +}
> +
>  #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
>  				VM_PKEY_BIT3 | VM_PKEY_BIT4)
>  #define AMR_BITS_PER_PKEY 2
> diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> index f88423b..545f291 100644
> --- a/arch/powerpc/mm/hash_utils_64.c
> +++ b/arch/powerpc/mm/hash_utils_64.c
> @@ -231,6 +231,7 @@ unsigned long htab_convert_pte_flags(unsigned long pteflags)
>  		 */
>  		rflags |= HPTE_R_M;
> 
> +	rflags |= pte_to_hpte_pkey_bits(pteflags);
>  	return rflags;
>  }
> 

Re: [RFC v7 12/25] powerpc: implementation for arch_override_mprotect_pkey()

[Ram Pai]

On Wed, Oct 18, 2017 at 05:58:18PM +0200, Laurent Dufour wrote:
> Hi Ram,
> 
> On 31/07/2017 02:12, Ram Pai wrote:
> > arch independent code calls arch_override_mprotect_pkey()
> > to return a pkey that best matches the requested protection.
> > 
> > This patch provides the implementation.
> > 
> > Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> > ---
> >  arch/powerpc/include/asm/mmu_context.h |    5 +++
> >  arch/powerpc/include/asm/pkeys.h       |   17 ++++++++++-
> >  arch/powerpc/mm/pkeys.c                |   47 ++++++++++++++++++++++++++++++++
> >  3 files changed, 67 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> > index 4705dab..7232484 100644
> > --- a/arch/powerpc/include/asm/mmu_context.h
> > +++ b/arch/powerpc/include/asm/mmu_context.h
> > @@ -185,6 +185,11 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
> >  #ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> >  #define pkey_initialize()
> >  #define pkey_mm_init(mm)
> > +
> > +static inline int vma_pkey(struct vm_area_struct *vma)
> > +{
> > +	return 0;
> > +}
> >  #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> > 
> >  #endif /* __KERNEL__ */
> > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> > index a715a08..03f7ea2 100644
> > --- a/arch/powerpc/include/asm/pkeys.h
> > +++ b/arch/powerpc/include/asm/pkeys.h
> > @@ -46,6 +46,16 @@ static inline u64 pkey_to_vmflag_bits(u16 pkey)
> >  		((pkey & 0x10UL) ? VM_PKEY_BIT4 : 0x0UL));
> >  }
> > 
> > +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
> > +				VM_PKEY_BIT3 | VM_PKEY_BIT4)
> > +
> > +static inline int vma_pkey(struct vm_area_struct *vma)
> > +{
> > +	if (!pkey_inited)
> > +		return 0;
> > +	return (vma->vm_flags & ARCH_VM_PKEY_FLAGS) >> VM_PKEY_SHIFT;
> > +}
> > +
> >  #define arch_max_pkey()  pkeys_total
> >  #define AMR_RD_BIT 0x1UL
> >  #define AMR_WR_BIT 0x2UL
> > @@ -146,11 +156,14 @@ static inline int execute_only_pkey(struct mm_struct *mm)
> >  	return __execute_only_pkey(mm);
> >  }
> > 
> > -
> > +extern int __arch_override_mprotect_pkey(struct vm_area_struct *vma,
> > +		int prot, int pkey);
> >  static inline int arch_override_mprotect_pkey(struct vm_area_struct *vma,
> >  		int prot, int pkey)
> >  {
> > -	return 0;
> > +	if (!pkey_inited)
> > +		return 0;
> > +	return __arch_override_mprotect_pkey(vma, prot, pkey);
> >  }
> > 
> >  extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> > diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
> > index 25749bf..edcbf48 100644
> > --- a/arch/powerpc/mm/pkeys.c
> > +++ b/arch/powerpc/mm/pkeys.c
> > @@ -154,3 +154,50 @@ int __execute_only_pkey(struct mm_struct *mm)
> >  		mm->context.execute_only_pkey = execute_only_pkey;
> >  	return execute_only_pkey;
> >  }
> > +
> > +static inline bool vma_is_pkey_exec_only(struct vm_area_struct *vma)
> > +{
> > +	/* Do this check first since the vm_flags should be hot */
> > +	if ((vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)) != VM_EXEC)
> > +		return false;
> > +
> > +	return (vma_pkey(vma) == vma->vm_mm->context.execute_only_pkey);
> > +}
> > +
> > +/*
> > + * This should only be called for *plain* mprotect calls.
> > + */
> > +int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot,
> > +		int pkey)
> > +{
> > +	/*
> > +	 * Is this an mprotect_pkey() call?  If so, never
> > +	 * override the value that came from the user.
> > +	 */
> > +	if (pkey != -1)
> > +		return pkey;
> 
> This check should be moved in arch_override_mprotect_pkey() in
> arch/powerpc/include/asm/pkeys.h

ok. will do.

> 
> > +
> > +	/*
> > +	 * If the currently associated pkey is execute-only,
> > +	 * but the requested protection requires read or write,
> > +	 * move it back to the default pkey.
> > +	 */
> > +	if (vma_is_pkey_exec_only(vma) &&
> > +	    (prot & (PROT_READ|PROT_WRITE)))
> > +		return 0;
> > +
> > +	/*
> > +	 * the requested protection is execute-only. Hence
> > +	 * lets use a execute-only pkey.
> > +	 */
> > +	if (prot == PROT_EXEC) {
> > +		pkey = execute_only_pkey(vma->vm_mm);
> > +		if (pkey > 0)
> > +			return pkey;
> > +	}
> > +
> > +	/*
> > +	 * nothing to override.
> > +	 */
> > +	return vma_pkey(vma);
> > +}
> > 

Thanks,
-- 
Ram Pai

Re: [RFC v7 16/25] powerpc: helper to validate key-access permissions of a pte

[Ram Pai]

On Wed, Oct 18, 2017 at 06:08:34PM +0200, Laurent Dufour wrote:
> 
> 
> On 31/07/2017 02:12, Ram Pai wrote:
> > helper function that checks if the read/write/execute is allowed
> > on the pte.
> > 
> > Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> > ---
> >  arch/powerpc/include/asm/book3s/64/pgtable.h |    4 +++
> >  arch/powerpc/include/asm/pkeys.h             |   12 +++++++++++
> >  arch/powerpc/mm/pkeys.c                      |   28 ++++++++++++++++++++++++++
> >  3 files changed, 44 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> > index 060a1b2..2bec0f6 100644
> > --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> > +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> > @@ -477,6 +477,10 @@ static inline void write_uamor(u64 value)
> >  	mtspr(SPRN_UAMOR, value);
> >  }
> > 
> > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> > +extern bool arch_pte_access_permitted(u64 pte, bool write, bool execute);
> > +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> > +
> >  #define __HAVE_ARCH_PTEP_GET_AND_CLEAR
> >  static inline pte_t ptep_get_and_clear(struct mm_struct *mm,
> >  				       unsigned long addr, pte_t *ptep)
> > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> > index 4b7e3f4..ba7bff6 100644
> > --- a/arch/powerpc/include/asm/pkeys.h
> > +++ b/arch/powerpc/include/asm/pkeys.h
> > @@ -85,6 +85,18 @@ static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
> >  		((pteflags & H_PAGE_PKEY_BIT4) ? HPTE_R_KEY_BIT4 : 0x0UL));
> >  }
> > 
> > +static inline u16 pte_to_pkey_bits(u64 pteflags)
> > +{
> > +	if (!pkey_inited)
> > +		return 0x0UL;
> 
> Is it really needed to make such a check in this low level function ?
> The only caller is already checking for pkey_inited before making the call.

There are two callers to this function. get_pte_pkey() is one among
them and it calls this function ignorant of the status of the
pkey-subsystem.

Thanks,
RP

Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys

[Ram Pai]

On Wed, Oct 18, 2017 at 06:08:46PM +0200, Laurent Dufour wrote:
> Hi Ram,
> 
> On 31/07/2017 02:12, Ram Pai wrote:
> > Total 32 keys are available on power7 and above. However
> > pkey 0,1 are reserved. So effectively we  have  30 pkeys.
> > 
> > On 4K kernels, we do not  have  5  bits  in  the  PTE to
> > represent  all the keys; we only have 3bits.Two of those
> > keys are reserved; pkey 0 and pkey 1. So effectively  we
> > have 6 pkeys.
> 
> IIUC, the pkey 0 and 1 are reserved by the hardware, and the kernel PTE has
> only 5 bits to keep track of the pkey. Why hw pkey 0 and 1 has to be
> represented in the kernel PTE ?


Key 0 is the default key. It is reserved in the sense, it cannot
be allocated or freed. but its there and will be used as the default.
So when no key is associated with a pte, it is infact using key 0.

Good question for the hardware designers. :)

key 1 is suggested not be used because it can be used by hypervisor
(powervm) or something else. Since linux kernel does not use it,
we dont program the key in the pte.



> 
> > This patch keeps track of reserved keys, allocated  keys
> > and keys that are currently free.
> > 
> > Also it  adds  skeletal  functions  and macros, that the
> > architecture-independent code expects to be available.
> > 
> > Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> > ---
> >  arch/powerpc/include/asm/book3s/64/mmu.h |    9 +++
> >  arch/powerpc/include/asm/mmu_context.h   |    1 +
> >  arch/powerpc/include/asm/pkeys.h         |   98 ++++++++++++++++++++++++++++-
> >  arch/powerpc/mm/mmu_context_book3s64.c   |    2 +
> >  arch/powerpc/mm/pkeys.c                  |    2 +
> >  5 files changed, 108 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
> > index 77529a3..104ad72 100644
> > --- a/arch/powerpc/include/asm/book3s/64/mmu.h
> > +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
> > @@ -108,6 +108,15 @@ struct patb_entry {
> >  #ifdef CONFIG_SPAPR_TCE_IOMMU
> >  	struct list_head iommu_group_mem_list;
> >  #endif
> > +
> > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> > +	/*
> > +	 * Each bit represents one protection key.
> > +	 * bit set   -> key allocated
> > +	 * bit unset -> key available for allocation
> > +	 */
> > +	u32 pkey_allocation_map;
> > +#endif
> >  } mm_context_t;
> > 
> >  /*
> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> > index 4b93547..4705dab 100644
> > --- a/arch/powerpc/include/asm/mmu_context.h
> > +++ b/arch/powerpc/include/asm/mmu_context.h
> > @@ -184,6 +184,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
> > 
> >  #ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> >  #define pkey_initialize()
> > +#define pkey_mm_init(mm)
> >  #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> > 
> >  #endif /* __KERNEL__ */
> > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> > index 4ccb8f5..def385f 100644
> > --- a/arch/powerpc/include/asm/pkeys.h
> > +++ b/arch/powerpc/include/asm/pkeys.h
> > @@ -2,6 +2,8 @@
> >  #define _ASM_PPC64_PKEYS_H
> > 
> >  extern bool pkey_inited;
> > +extern int pkeys_total; /* total pkeys as per device tree */
> > +extern u32 initial_allocation_mask;/* bits set for reserved keys */
> > 
> >  /*
> >   * powerpc needs an additional vma bit to support 32 keys.
> > @@ -20,21 +22,76 @@
> >  #define VM_PKEY_BIT4	VM_HIGH_ARCH_4
> >  #endif
> > 
> > -#define ARCH_VM_PKEY_FLAGS 0
> > +#define arch_max_pkey()  pkeys_total
> > +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
> > +				VM_PKEY_BIT3 | VM_PKEY_BIT4)
> > +
> > +#define pkey_alloc_mask(pkey) (0x1 << pkey)
> > +
> > +#define mm_pkey_allocation_map(mm)	(mm->context.pkey_allocation_map)
> > +
> > +#define mm_set_pkey_allocated(mm, pkey) {	\
> > +	mm_pkey_allocation_map(mm) |= pkey_alloc_mask(pkey); \
> > +}
> > +
> > +#define mm_set_pkey_free(mm, pkey) {	\
> > +	mm_pkey_allocation_map(mm) &= ~pkey_alloc_mask(pkey);	\
> > +}
> > +
> > +#define mm_set_pkey_is_allocated(mm, pkey)	\
> > +	(mm_pkey_allocation_map(mm) & pkey_alloc_mask(pkey))
> > +
> > +#define mm_set_pkey_is_reserved(mm, pkey) (initial_allocation_mask & \
> > +					pkey_alloc_mask(pkey))
> 
> This macro doesn't need a 'mm' argument.
> 
> >  static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
> >  {
> > -	return (pkey == 0);
> > +	/* a reserved key is never considered as 'explicitly allocated' */
> > +	return ((pkey < arch_max_pkey()) &&
> > +		!mm_set_pkey_is_reserved(mm, pkey) &&
> > +		mm_set_pkey_is_allocated(mm, pkey));
> >  }
> > 
> > +/*
> > + * Returns a positive, 5-bit key on success, or -1 on failure.
> 
> I guess you rely on the mmap_sem to protect against concurrency in
> mm_pkey_alloc() and mm_pkey_free().
> As this is not explicit in the code, it should at least be mentioned in the
> comment describing the function.


Yes. will do. good point.

> 
> > + */
> >  static inline int mm_pkey_alloc(struct mm_struct *mm)
> >  {
> > -	return -1;
> > +	/*
> > +	 * Note: this is the one and only place we make sure
> > +	 * that the pkey is valid as far as the hardware is
> > +	 * concerned.  The rest of the kernel trusts that
> > +	 * only good, valid pkeys come out of here.
> > +	 */
> > +	u32 all_pkeys_mask = (u32)(~(0x0));
> > +	int ret;
> > +
> > +	if (!pkey_inited)
> > +		return -1;
> > +	/*
> > +	 * Are we out of pkeys?  We must handle this specially
> > +	 * because ffz() behavior is undefined if there are no
> > +	 * zeros.
> > +	 */
> > +	if (mm_pkey_allocation_map(mm) == all_pkeys_mask)
> > +		return -1;
> > +
> > +	ret = ffz((u32)mm_pkey_allocation_map(mm));
> > +	mm_set_pkey_allocated(mm, ret);
> > +	return ret;
> >  }
> > 
> >  static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
> >  {
> > -	return -EINVAL;
> > +	if (!pkey_inited)
> > +		return -1;
> > +
> > +	if (!mm_pkey_is_allocated(mm, pkey))
> > +		return -EINVAL;
> > +
> > +	mm_set_pkey_free(mm, pkey);
> > +
> > +	return 0;
> >  }
> > 
> >  /*
> > @@ -58,12 +115,45 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> >  	return 0;
> >  }
> > 
> > +static inline void pkey_mm_init(struct mm_struct *mm)
> > +{
> > +	if (!pkey_inited)
> > +		return;
> > +	mm_pkey_allocation_map(mm) = initial_allocation_mask;
> > +}
> > +
> >  static inline void pkey_initialize(void)
> >  {
> > +	int os_reserved, i;
> > +
> >  	/* disable the pkey system till everything
> >  	 * is in place. A patch further down the
> >  	 * line will enable it.
> >  	 */
> >  	pkey_inited = false;
> > +
> > +	/* Lets assume 32 keys */
> > +	pkeys_total = 32;
> > +
> > +#ifdef CONFIG_PPC_4K_PAGES
> > +	/*
> > +	 * the OS can manage only 8 pkeys
> > +	 * due to its inability to represent
> > +	 * them in the linux 4K-PTE.
> > +	 */
> > +	os_reserved = pkeys_total-8;
> > +#else
> > +	os_reserved = 0;
> > +#endif
> > +	/*
> > +	 * Bits are in LE format.
> > +	 * NOTE: 1, 0 are reserved.
> > +	 * key 0 is the default key, which allows read/write/execute.
> > +	 * key 1 is recommended not to be used.
> > +	 * PowerISA(3.0) page 1015, programming note.
> > +	 */
> > +	initial_allocation_mask = ~0x0;
> > +	for (i = 2; i < (pkeys_total - os_reserved); i++)
> > +		initial_allocation_mask &= ~(0x1<<i);
> >  }
> >  #endif /*_ASM_PPC64_PKEYS_H */
> > diff --git a/arch/powerpc/mm/mmu_context_book3s64.c b/arch/powerpc/mm/mmu_context_book3s64.c
> > index a3edf81..34a16f3 100644
> > --- a/arch/powerpc/mm/mmu_context_book3s64.c
> > +++ b/arch/powerpc/mm/mmu_context_book3s64.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/string.h>
> >  #include <linux/types.h>
> >  #include <linux/mm.h>
> > +#include <linux/pkeys.h>
> >  #include <linux/spinlock.h>
> >  #include <linux/idr.h>
> >  #include <linux/export.h>
> > @@ -120,6 +121,7 @@ static int hash__init_new_context(struct mm_struct *mm)
> > 
> >  	subpage_prot_init_new_context(mm);
> > 
> > +	pkey_mm_init(mm);
> >  	return index;
> >  }
> > 
> > diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
> > index c3acee1..37dacc5 100644
> > --- a/arch/powerpc/mm/pkeys.c
> > +++ b/arch/powerpc/mm/pkeys.c
> > @@ -16,3 +16,5 @@
> >  #include <linux/pkeys.h>                /* PKEY_*                       */
> > 
> >  bool pkey_inited;
> > +int  pkeys_total;		/* total pkeys as per device tree */
> > +u32  initial_allocation_mask;	/* bits set for reserved keys */
> > 

-- 
Ram Pai

Re: [RFC v7 15/25] powerpc: Program HPTE key protection bits

[Ram Pai]

On Wed, Oct 18, 2017 at 06:15:40PM +0200, Laurent Dufour wrote:
> 
> 
> On 31/07/2017 02:12, Ram Pai wrote:
> > Map the PTE protection key bits to the HPTE key protection bits,
> > while creating HPTE  entries.
> > 
> > Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> > ---
> >  arch/powerpc/include/asm/book3s/64/mmu-hash.h |    5 +++++
> >  arch/powerpc/include/asm/mmu_context.h        |    6 ++++++
> >  arch/powerpc/include/asm/pkeys.h              |   13 +++++++++++++
> >  arch/powerpc/mm/hash_utils_64.c               |    1 +
> >  4 files changed, 25 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> > index 6981a52..f7a6ed3 100644
> > --- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> > +++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> > @@ -90,6 +90,8 @@
> >  #define HPTE_R_PP0		ASM_CONST(0x8000000000000000)
> >  #define HPTE_R_TS		ASM_CONST(0x4000000000000000)
> >  #define HPTE_R_KEY_HI		ASM_CONST(0x3000000000000000)
> > +#define HPTE_R_KEY_BIT0		ASM_CONST(0x2000000000000000)
> > +#define HPTE_R_KEY_BIT1		ASM_CONST(0x1000000000000000)
> >  #define HPTE_R_RPN_SHIFT	12
> >  #define HPTE_R_RPN		ASM_CONST(0x0ffffffffffff000)
> >  #define HPTE_R_RPN_3_0		ASM_CONST(0x01fffffffffff000)
> > @@ -104,6 +106,9 @@
> >  #define HPTE_R_C		ASM_CONST(0x0000000000000080)
> >  #define HPTE_R_R		ASM_CONST(0x0000000000000100)
> >  #define HPTE_R_KEY_LO		ASM_CONST(0x0000000000000e00)
> > +#define HPTE_R_KEY_BIT2		ASM_CONST(0x0000000000000800)
> > +#define HPTE_R_KEY_BIT3		ASM_CONST(0x0000000000000400)
> > +#define HPTE_R_KEY_BIT4		ASM_CONST(0x0000000000000200)
> > 
> >  #define HPTE_V_1TB_SEG		ASM_CONST(0x4000000000000000)
> >  #define HPTE_V_VRMA_MASK	ASM_CONST(0x4001ffffff000000)
> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> > index 7232484..2eb7f80 100644
> > --- a/arch/powerpc/include/asm/mmu_context.h
> > +++ b/arch/powerpc/include/asm/mmu_context.h
> > @@ -190,6 +190,12 @@ static inline int vma_pkey(struct vm_area_struct *vma)
> >  {
> >  	return 0;
> >  }
> > +
> > +static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
> > +{
> > +	return 0x0UL;
> > +}
> > +
> >  #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> > 
> >  #endif /* __KERNEL__ */
> > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> > index 1ded6df..4b7e3f4 100644
> > --- a/arch/powerpc/include/asm/pkeys.h
> > +++ b/arch/powerpc/include/asm/pkeys.h
> > @@ -72,6 +72,19 @@ static inline int vma_pkey(struct vm_area_struct *vma)
> >  #define AMR_RD_BIT 0x1UL
> >  #define AMR_WR_BIT 0x2UL
> >  #define IAMR_EX_BIT 0x1UL
> > +
> > +static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
> > +{
> > +	if (!pkey_inited)
> > +		return 0x0UL;
> 
> Why making such a check here, is it to avoid the following check during the
> boot process only ?
> IIUC, there is no way to get H_PAGE_PKEY_BIT* set when pkey_inited is false.

I know its a little paronia. Trying to avoid a case where the
uninitialized pkey bits in the pte are erroneously interpreted as valid
by this function. Remember that the caller of this function will use the
return value to program the hpte. Nothing really bad should happen
since none of the keys are enabled. But ... just playing it safe.

thanks,
RP

Re: [RFC v7 15/25] powerpc: Program HPTE key protection bits

[Michael Ellerman]

Ram Pai <linuxram@us.ibm.com> writes:
> On Wed, Oct 18, 2017 at 06:15:40PM +0200, Laurent Dufour wrote:
>> On 31/07/2017 02:12, Ram Pai wrote:
>> > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
>> > index 1ded6df..4b7e3f4 100644
>> > --- a/arch/powerpc/include/asm/pkeys.h
>> > +++ b/arch/powerpc/include/asm/pkeys.h
>> > @@ -72,6 +72,19 @@ static inline int vma_pkey(struct vm_area_struct *vma)
>> >  #define AMR_RD_BIT 0x1UL
>> >  #define AMR_WR_BIT 0x2UL
>> >  #define IAMR_EX_BIT 0x1UL
>> > +
>> > +static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
>> > +{
>> > +	if (!pkey_inited)
>> > +		return 0x0UL;
>> 
>> Why making such a check here, is it to avoid the following check during the
>> boot process only ?
>> IIUC, there is no way to get H_PAGE_PKEY_BIT* set when pkey_inited is false.
>
> I know its a little paronia. Trying to avoid a case where the
> uninitialized pkey bits in the pte are erroneously interpreted as valid
> by this function. Remember that the caller of this function will use the
> return value to program the hpte. Nothing really bad should happen
> since none of the keys are enabled. But ... just playing it safe.

I think that's probably over-paranoid. It's not like it adds much
overhead, but it is a hot path, so no need to make it slower than it
needs to be.

cheers

Re: [RFC v7 16/25] powerpc: helper to validate key-access permissions of a pte

[Michael Ellerman]

Ram Pai <linuxram@us.ibm.com> writes:
> On Wed, Oct 18, 2017 at 06:08:34PM +0200, Laurent Dufour wrote:
>> On 31/07/2017 02:12, Ram Pai wrote:
>> > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
>> > index 4b7e3f4..ba7bff6 100644
>> > --- a/arch/powerpc/include/asm/pkeys.h
>> > +++ b/arch/powerpc/include/asm/pkeys.h
>> > @@ -85,6 +85,18 @@ static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
>> >  		((pteflags & H_PAGE_PKEY_BIT4) ? HPTE_R_KEY_BIT4 : 0x0UL));
>> >  }
>> > 
>> > +static inline u16 pte_to_pkey_bits(u64 pteflags)
>> > +{
>> > +	if (!pkey_inited)
>> > +		return 0x0UL;
>> 
>> Is it really needed to make such a check in this low level function ?
>> The only caller is already checking for pkey_inited before making the call.
>
> There are two callers to this function. get_pte_pkey() is one among
> them and it calls this function ignorant of the status of the
> pkey-subsystem.

But if none of the bits are set it will return 0 anyway right?

cheers

[RFC v7 16/25] powerpc: helper to validate key-access permissions of a pte

[Ram Pai]

helper function that checks if the read/write/execute is allowed
on the pte.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/pgtable.h |    4 +++
 arch/powerpc/include/asm/pkeys.h             |   12 +++++++++++
 arch/powerpc/mm/pkeys.c                      |   28 ++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 060a1b2..2bec0f6 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -477,6 +477,10 @@ static inline void write_uamor(u64 value)
 	mtspr(SPRN_UAMOR, value);
 }
 
+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+extern bool arch_pte_access_permitted(u64 pte, bool write, bool execute);
+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+
 #define __HAVE_ARCH_PTEP_GET_AND_CLEAR
 static inline pte_t ptep_get_and_clear(struct mm_struct *mm,
 				       unsigned long addr, pte_t *ptep)
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 4b7e3f4..ba7bff6 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -85,6 +85,18 @@ static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
 		((pteflags & H_PAGE_PKEY_BIT4) ? HPTE_R_KEY_BIT4 : 0x0UL));
 }
 
+static inline u16 pte_to_pkey_bits(u64 pteflags)
+{
+	if (!pkey_inited)
+		return 0x0UL;
+
+	return (((pteflags & H_PAGE_PKEY_BIT0) ? 0x10 : 0x0UL) |
+		((pteflags & H_PAGE_PKEY_BIT1) ? 0x8 : 0x0UL) |
+		((pteflags & H_PAGE_PKEY_BIT2) ? 0x4 : 0x0UL) |
+		((pteflags & H_PAGE_PKEY_BIT3) ? 0x2 : 0x0UL) |
+		((pteflags & H_PAGE_PKEY_BIT4) ? 0x1 : 0x0UL));
+}
+
 #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
 				VM_PKEY_BIT3 | VM_PKEY_BIT4)
 #define AMR_BITS_PER_PKEY 2
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index edcbf48..8d756ef 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -201,3 +201,31 @@ int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot,
 	 */
 	return vma_pkey(vma);
 }
+
+static bool pkey_access_permitted(int pkey, bool write, bool execute)
+{
+	int pkey_shift;
+	u64 amr;
+
+	if (!pkey)
+		return true;
+
+	pkey_shift = pkeyshift(pkey);
+	if (!(read_uamor() & (0x3UL << pkey_shift)))
+		return true;
+
+	if (execute && !(read_iamr() & (IAMR_EX_BIT << pkey_shift)))
+		return true;
+
+	amr = read_amr(); /* delay reading amr uptil absolutely needed*/
+	return ((!write && !(amr & (AMR_RD_BIT << pkey_shift))) ||
+		(write &&  !(amr & (AMR_WR_BIT << pkey_shift))));
+}
+
+bool arch_pte_access_permitted(u64 pte, bool write, bool execute)
+{
+	if (!pkey_inited)
+		return true;
+	return pkey_access_permitted(pte_to_pkey_bits(pte),
+			write, execute);
+}
-- 
1.7.1