[03/25] powerpc: track allocation status of all pkeys

Message ID 1504910713-7094-12-git-send-email-linuxram@us.ibm.com
State Changes Requested
Headers show
Series
  • Untitled series #2308
Related show

Commit Message

Ram Pai Sept. 8, 2017, 10:44 p.m.
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         |   72 ++++++++++++++++++++++++++++--
 arch/powerpc/mm/mmu_context_book3s64.c   |    2 +
 arch/powerpc/mm/pkeys.c                  |   28 ++++++++++++
 5 files changed, 108 insertions(+), 4 deletions(-)

Comments

Michael Ellerman Oct. 7, 2017, 10:02 a.m. | #1
Ram Pai <linuxram@us.ibm.com> writes:

> 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         |   72 ++++++++++++++++++++++++++++--
>  arch/powerpc/mm/mmu_context_book3s64.c   |    2 +
>  arch/powerpc/mm/pkeys.c                  |   28 ++++++++++++
>  5 files changed, 108 insertions(+), 4 deletions(-)

This doesn't build for me, with pseries_le_defconfig. I assume it built
for you. So something has changed upstream maybe?


In file included from ../include/linux/pkeys.h:8:0,
                 from ../mm/mprotect.c:26:
../mm/mprotect.c: In function ‘do_mprotect_pkey’:
../arch/powerpc/include/asm/pkeys.h:27:29: error: ‘VM_PKEY_BIT0’ undeclared (first use in this function)
 #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
                             ^
../mm/mprotect.c:470:6: note: in expansion of macro ‘ARCH_VM_PKEY_FLAGS’
      ARCH_VM_PKEY_FLAGS;
      ^~~~~~~~~~~~~~~~~~
../arch/powerpc/include/asm/pkeys.h:27:29: note: each undeclared identifier is reported only once for each function it appears in
 #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
                             ^
../mm/mprotect.c:470:6: note: in expansion of macro ‘ARCH_VM_PKEY_FLAGS’
      ARCH_VM_PKEY_FLAGS;
      ^~~~~~~~~~~~~~~~~~
../arch/powerpc/include/asm/pkeys.h:27:44: error: ‘VM_PKEY_BIT1’ undeclared (first use in this function)
 #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
                                            ^
../mm/mprotect.c:470:6: note: in expansion of macro ‘ARCH_VM_PKEY_FLAGS’
      ARCH_VM_PKEY_FLAGS;
      ^~~~~~~~~~~~~~~~~~
../arch/powerpc/include/asm/pkeys.h:27:59: error: ‘VM_PKEY_BIT2’ undeclared (first use in this function)
 #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
                                                           ^
../mm/mprotect.c:470:6: note: in expansion of macro ‘ARCH_VM_PKEY_FLAGS’
      ARCH_VM_PKEY_FLAGS;
      ^~~~~~~~~~~~~~~~~~
../arch/powerpc/include/asm/pkeys.h:28:5: error: ‘VM_PKEY_BIT3’ undeclared (first use in this function)
     VM_PKEY_BIT3 | VM_PKEY_BIT4)
     ^
../mm/mprotect.c:470:6: note: in expansion of macro ‘ARCH_VM_PKEY_FLAGS’
      ARCH_VM_PKEY_FLAGS;
      ^~~~~~~~~~~~~~~~~~
../arch/powerpc/include/asm/pkeys.h:28:20: error: ‘VM_PKEY_BIT4’ undeclared (first use in this function)
     VM_PKEY_BIT3 | VM_PKEY_BIT4)
                    ^
../mm/mprotect.c:470:6: note: in expansion of macro ‘ARCH_VM_PKEY_FLAGS’
      ARCH_VM_PKEY_FLAGS;
      ^~~~~~~~~~~~~~~~~~
../scripts/Makefile.build:311: recipe for target 'mm/mprotect.o' failed


cheers
Ram Pai Oct. 8, 2017, 11:02 p.m. | #2
On Sat, Oct 07, 2017 at 09:02:55PM +1100, Michael Ellerman wrote:
> Ram Pai <linuxram@us.ibm.com> writes:
> 
> > 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         |   72 ++++++++++++++++++++++++++++--
> >  arch/powerpc/mm/mmu_context_book3s64.c   |    2 +
> >  arch/powerpc/mm/pkeys.c                  |   28 ++++++++++++
> >  5 files changed, 108 insertions(+), 4 deletions(-)
> 
> This doesn't build for me, with pseries_le_defconfig. I assume it built
> for you. So something has changed upstream maybe?
> 

Yes. :(
The following commit upstream broke my patches.
df3735c5b40fad8d0d28eb8ab065fe955b3347ee

Will fix and send you a patch.

RP



> 
> In file included from ../include/linux/pkeys.h:8:0,
>                  from ../mm/mprotect.c:26:
> ../mm/mprotect.c: In function ‘do_mprotect_pkey’:
> ../arch/powerpc/include/asm/pkeys.h:27:29: error: ‘VM_PKEY_BIT0’ undeclared (first use in this function)
>  #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
>                              ^
..snip...
Balbir Singh Oct. 18, 2017, 2:47 a.m. | #3
On Fri,  8 Sep 2017 15:44:51 -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>

I ended up reviewing v7 of the patch. Is this v8?
I think the comments still apply to this revision

Balbir Singh.
Aneesh Kumar K.V Oct. 23, 2017, 9:41 a.m. | #4
Ram Pai <linuxram@us.ibm.com> writes:

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

When you say reserved, reserved by whom? Is that part of ISA or PAPR ?
Also do you expect that to change. If not why all these indirection?
Can we have the mask as a #define for 4K and 64K page size
config?

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


-aneesh
Ram Pai Oct. 23, 2017, 6:14 p.m. | #5
On Mon, Oct 23, 2017 at 03:11:28PM +0530, Aneesh Kumar K.V wrote:
> Ram Pai <linuxram@us.ibm.com> writes:
> 
> > Total 32 keys are available on power7 and above. However
> > pkey 0,1 are reserved. So effectively we  have  30 pkeys.
> 
> When you say reserved, reserved by whom? Is that part of ISA or PAPR ?
> Also do you expect that to change. If not why all these indirection?
> Can we have the mask as a #define for 4K and 64K page size
> config?

The reserved keys cannot be determined
statically. It depends on the platform and the key info exported to us
by the device-tree.  Hence it cannot be macro'd.

One of the subsequent patch, reads the device tree and sets pkeys_total
appropriately. FYI.

BTW: key 0 is reserved by the ISA.  key 1 is reserved, but its nebulous.
There is a programming note in the ISA to avoid key 1. Also testing
and experimentation with key 1 lead to unpredicatable behavior on
powervm. Key 31 may or may not be used by the hypervisor. The device
tree has to be referred to determine that.

Bottomline, the mask can be determined only during runtime.

RP
Aneesh Kumar K.V Oct. 24, 2017, 6:28 a.m. | #6
Ram Pai <linuxram@us.ibm.com> writes:
 +
> +#define mm_set_pkey_is_allocated(mm, pkey)	\
> +	(mm_pkey_allocation_map(mm) & 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));
>  }
>

that is confusing naming. what is mm_set_pkey_is_allocated()? . 'set' in
that name is confusing.

-aneesh
Ram Pai Oct. 24, 2017, 7:23 a.m. | #7
On Tue, Oct 24, 2017 at 11:58:29AM +0530, Aneesh Kumar K.V wrote:
> Ram Pai <linuxram@us.ibm.com> writes:
>  +
> > +#define mm_set_pkey_is_allocated(mm, pkey)	\
> > +	(mm_pkey_allocation_map(mm) & 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));
> >  }
> >
> 
> that is confusing naming. what is mm_set_pkey_is_allocated()? . 'set' in
> that name is confusing.

will change it by removing the 'set' in the names.

thanks,
RP

Patch

diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
index c3b00e8..55950f4 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu.h
@@ -107,6 +107,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 7badf29..c705a5d 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -144,6 +144,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 44e01a2..133f8c4 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -3,6 +3,8 @@ 
 
 extern bool pkey_inited;
 extern bool pkey_execute_disable_support;
+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.
@@ -21,21 +23,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;
 }
 
 /*
@@ -59,5 +116,12 @@  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;
+}
+
 extern void pkey_initialize(void);
 #endif /*_ASM_PPC64_PKEYS_H */
diff --git a/arch/powerpc/mm/mmu_context_book3s64.c b/arch/powerpc/mm/mmu_context_book3s64.c
index 05e1538..5df223a 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>
@@ -118,6 +119,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 418a05b..ebc9e84 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -16,9 +16,13 @@ 
 
 bool pkey_inited;
 bool pkey_execute_disable_support;
+int  pkeys_total;		/* total pkeys as per device tree */
+u32  initial_allocation_mask;	/* bits set for reserved keys */
 
 void __init 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.
@@ -30,4 +34,28 @@  void __init pkey_initialize(void)
 	 * A patch further down will enable it.
 	 */
 	pkey_execute_disable_support = 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);
 }