Skip to contentSkip to conversation

powerpc/mm/keys: Move pte bits to correct headers

PATCH Applied Aneesh Kumar K.V 3 participants 6 messages Mar 7, 2018 – Apr 4, 2018 PowerPC

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 powerpc/mm/keys: Move pte bits to correct headers

Memory keys are supported only with hash translation mode. Instead of #ifdef in generic code move the key related pte bits to respective headers

---
 arch/powerpc/include/asm/book3s/64/hash-4k.h  |  7 +++++++
 arch/powerpc/include/asm/book3s/64/hash-64k.h |  7 +++++++
 arch/powerpc/include/asm/book3s/64/pgtable.h  | 19 -------------------
 3 files changed, 14 insertions(+), 19 deletions(-)
arch/powerpc/include/asm/book3s/64/hash-4k.h +7 -0
index fc3dc6a93939..4103bfc7c223 100644
@@ -33,6 +33,13 @@
 #define H_PAGE_THP_HUGE 0x0
 #define H_PAGE_COMBO	0x0
 
+/* memory key bits, only 8 keys supported */
+#define H_PTE_PKEY_BIT0	0
+#define H_PTE_PKEY_BIT1	0
+#define H_PTE_PKEY_BIT2	_RPAGE_RSV3
+#define H_PTE_PKEY_BIT3	_RPAGE_RSV4
+#define H_PTE_PKEY_BIT4	_RPAGE_RSV5
+
 /* 8 bytes per each pte entry */
 #define H_PTE_FRAG_SIZE_SHIFT  (H_PTE_INDEX_SIZE + 3)
 #define H_PTE_FRAG_NR	(PAGE_SIZE >> H_PTE_FRAG_SIZE_SHIFT)
arch/powerpc/include/asm/book3s/64/hash-64k.h +7 -0
index e53728ff29a0..bb880c97b87d 100644
@@ -16,6 +16,13 @@
 #define H_PAGE_BUSY	_RPAGE_RPN44     /* software: PTE & hash are busy */
 #define H_PAGE_HASHPTE	_RPAGE_RPN43	/* PTE has associated HPTE */
 
+/* memory key bits. */
+#define H_PTE_PKEY_BIT0	_RPAGE_RSV1
+#define H_PTE_PKEY_BIT1	_RPAGE_RSV2
+#define H_PTE_PKEY_BIT2	_RPAGE_RSV3
+#define H_PTE_PKEY_BIT3	_RPAGE_RSV4
+#define H_PTE_PKEY_BIT4	_RPAGE_RSV5
+
 /*
  * We need to differentiate between explicit huge page and THP huge
  * page, since THP huge page also need to track real subpage details
arch/powerpc/include/asm/book3s/64/pgtable.h +0 -19
index 973199bd4654..c233915abb68 100644
@@ -60,25 +60,6 @@
 /* Max physical address bit as per radix table */
 #define _RPAGE_PA_MAX		57
 
-#ifdef CONFIG_PPC_MEM_KEYS
-#ifdef CONFIG_PPC_64K_PAGES
-#define H_PTE_PKEY_BIT0	_RPAGE_RSV1
-#define H_PTE_PKEY_BIT1	_RPAGE_RSV2
-#else /* CONFIG_PPC_64K_PAGES */
-#define H_PTE_PKEY_BIT0	0 /* _RPAGE_RSV1 is not available */
-#define H_PTE_PKEY_BIT1	0 /* _RPAGE_RSV2 is not available */
-#endif /* CONFIG_PPC_64K_PAGES */
-#define H_PTE_PKEY_BIT2	_RPAGE_RSV3
-#define H_PTE_PKEY_BIT3	_RPAGE_RSV4
-#define H_PTE_PKEY_BIT4	_RPAGE_RSV5
-#else /*  CONFIG_PPC_MEM_KEYS */
-#define H_PTE_PKEY_BIT0	0
-#define H_PTE_PKEY_BIT1	0
-#define H_PTE_PKEY_BIT2	0
-#define H_PTE_PKEY_BIT3	0
-#define H_PTE_PKEY_BIT4	0
-#endif /*  CONFIG_PPC_MEM_KEYS */
-
 /*
  * Max physical address bit we will use for now.
  *
Signature · 1 line
2.14.3
#1 lore raw

PATCH 2/2 powerpc/mm/keys: Update documentation in key fault handling

No functionality change in this patch. Adds more code comments. We also remove an unnecessary pkey check after we check for pkey error in this patch.

---
 arch/powerpc/mm/fault.c | 28 ++++++++++++----------------
 arch/powerpc/mm/pkeys.c | 11 ++++-------
 2 files changed, 16 insertions(+), 23 deletions(-)
arch/powerpc/mm/fault.c +12 -16
index 866446cf2d9a..c01d627e687a 100644
@@ -297,7 +297,12 @@ static bool access_error(bool is_write, bool is_exec,
 
 	if (unlikely(!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE))))
 		return true;
-
+	/*
+	 * We should ideally do the vma pkey access check here. But in the
+	 * fault path, handle_mm_fault() also does the same check. To avoid
+	 * these multiple checks, we skip it here and handle access error due
+	 * to pkeys later.
+	 */
 	return false;
 }
 
@@ -518,25 +523,16 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
 
 #ifdef CONFIG_PPC_MEM_KEYS
 	/*
-	 * if the HPTE is not hashed, hardware will not detect
-	 * a key fault. Lets check if we failed because of a
-	 * software detected key fault.
+	 * we skipped checking for access error due to key earlier.
+	 * Check that using handle_mm_fault error return.
 	 */
 	if (unlikely(fault & VM_FAULT_SIGSEGV) &&
-		!arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
-			is_exec, 0)) {
-		/*
-		 * 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 let's use vma_pkey() to get the key; instead
-		 * of get_mm_addr_key().
-		 */
+		!arch_vma_access_permitted(vma, is_write, is_exec, 0)) {
+
 		int pkey = vma_pkey(vma);
 
-		if (likely(pkey)) {
-			up_read(&mm->mmap_sem);
-			return bad_key_fault_exception(regs, address, pkey);
-		}
+		up_read(&mm->mmap_sem);
+		return bad_key_fault_exception(regs, address, pkey);
 	}
 #endif /* CONFIG_PPC_MEM_KEYS */
 
arch/powerpc/mm/pkeys.c +4 -7
index ba71c5481f42..56d33056a559 100644
@@ -119,18 +119,15 @@ int pkey_initialize(void)
 #else
 	os_reserved = 0;
 #endif
+	initial_allocation_mask = ~0x0;
+	pkey_amr_uamor_mask = ~0x0ul;
+	pkey_iamr_mask = ~0x0ul;
 	/*
-	 * Bits are in LE format. NOTE: 1, 0 are reserved.
+	 * key 0, 1 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;
-
-	/* register mask is in BE format */
-	pkey_amr_uamor_mask = ~0x0ul;
-	pkey_iamr_mask = ~0x0ul;
-
 	for (i = 2; i < (pkeys_total - os_reserved); i++) {
 		initial_allocation_mask &= ~(0x1 << i);
 		pkey_amr_uamor_mask &= ~(0x3ul << pkeyshift(i));
Signature · 1 line
2.14.3
#3 lore raw
In reply to Aneesh Kumar K.V
Quoted 15 lines · Aneesh Kumar K.V

Memory keys are supported only with hash translation mode. Instead of #ifdef in generic code move the key related pte bits to respective headers

---
 arch/powerpc/include/asm/book3s/64/hash-4k.h  |  7 +++++++
 arch/powerpc/include/asm/book3s/64/hash-64k.h |  7 +++++++
 arch/powerpc/include/asm/book3s/64/pgtable.h  | 19 -------------------
 3 files changed, 14 insertions(+), 19 deletions(-)
arch/powerpc/include/asm/book3s/64/hash-4k.h +0 -0
index fc3dc6a93939..4103bfc7c223 100644

arch/powerpc/include/asm/book3s/64/hash-4k.h line 42

Quoted 10 lines
arch/powerpc/include/asm/book3s/64/hash-4k.h +7 -0
@@ -33,6 +33,13 @@
 #define H_PAGE_THP_HUGE 0x0
 #define H_PAGE_COMBO	0x0
 
+/* memory key bits, only 8 keys supported */
+#define H_PTE_PKEY_BIT0	0
+#define H_PTE_PKEY_BIT1	0
+#define H_PTE_PKEY_BIT2	_RPAGE_RSV3
+#define H_PTE_PKEY_BIT3	_RPAGE_RSV4
+#define H_PTE_PKEY_BIT4	_RPAGE_RSV5
+

If CONFIG_PPC_MEM_KEYS is not defined, all of them have to be 0. How is that handled here?

Quoted 8 lines
arch/powerpc/include/asm/book3s/64/hash-4k.h +0 -0
 /* 8 bytes per each pte entry */
 #define H_PTE_FRAG_SIZE_SHIFT  (H_PTE_INDEX_SIZE + 3)
 #define H_PTE_FRAG_NR	(PAGE_SIZE >> H_PTE_FRAG_SIZE_SHIFT)
arch/powerpc/include/asm/book3s/64/hash-64k.h +0 -0
index e53728ff29a0..bb880c97b87d 100644

arch/powerpc/include/asm/book3s/64/hash-64k.h line 25

Quoted 10 lines
arch/powerpc/include/asm/book3s/64/hash-64k.h +7 -0
@@ -16,6 +16,13 @@
 #define H_PAGE_BUSY	_RPAGE_RPN44     /* software: PTE & hash are busy */
 #define H_PAGE_HASHPTE	_RPAGE_RPN43	/* PTE has associated HPTE */
 
+/* memory key bits. */
+#define H_PTE_PKEY_BIT0	_RPAGE_RSV1
+#define H_PTE_PKEY_BIT1	_RPAGE_RSV2
+#define H_PTE_PKEY_BIT2	_RPAGE_RSV3
+#define H_PTE_PKEY_BIT3	_RPAGE_RSV4
+#define H_PTE_PKEY_BIT4	_RPAGE_RSV5
+

same comment as above.

RP

#4 lore raw
Quoted 19 lines · Ram Pai
Quoted 18 lines · Aneesh Kumar K.V

Memory keys are supported only with hash translation mode. Instead of #ifdef in generic code move the key related pte bits to respective headers

---
  arch/powerpc/include/asm/book3s/64/hash-4k.h  |  7 +++++++
  arch/powerpc/include/asm/book3s/64/hash-64k.h |  7 +++++++
  arch/powerpc/include/asm/book3s/64/pgtable.h  | 19 -------------------
  3 files changed, 14 insertions(+), 19 deletions(-)
arch/powerpc/include/asm/book3s/64/hash-4k.h +0 -0
index fc3dc6a93939..4103bfc7c223 100644
@@ -33,6 +33,13 @@
  #define H_PAGE_THP_HUGE 0x0
  #define H_PAGE_COMBO	0x0
 
Quoted 11 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h line 42

Quoted 7 lines
arch/powerpc/include/asm/book3s/64/hash-4k.h +7 -0
+/* memory key bits, only 8 keys supported */
+#define H_PTE_PKEY_BIT0	0
+#define H_PTE_PKEY_BIT1	0
+#define H_PTE_PKEY_BIT2	_RPAGE_RSV3
+#define H_PTE_PKEY_BIT3	_RPAGE_RSV4
+#define H_PTE_PKEY_BIT4	_RPAGE_RSV5
+

If CONFIG_PPC_MEM_KEYS is not defined, all of them have to be 0. How is that handled here?

why? conditional defines of pte bits always results in error, like we check for an overloaded key bit in some code path and taking wrong action.

Quoted 12 lines
Quoted 12 lines
arch/powerpc/include/asm/book3s/64/hash-4k.h +0 -0
  /* 8 bytes per each pte entry */
  #define H_PTE_FRAG_SIZE_SHIFT  (H_PTE_INDEX_SIZE + 3)
  #define H_PTE_FRAG_NR	(PAGE_SIZE >> H_PTE_FRAG_SIZE_SHIFT)
arch/powerpc/include/asm/book3s/64/hash-64k.h +0 -0
index e53728ff29a0..bb880c97b87d 100644
@@ -16,6 +16,13 @@
  #define H_PAGE_BUSY	_RPAGE_RPN44     /* software: PTE & hash are busy */
  #define H_PAGE_HASHPTE	_RPAGE_RPN43	/* PTE has associated HPTE */
 
Quoted 10 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h line 25

Quoted 7 lines
arch/powerpc/include/asm/book3s/64/hash-64k.h +7 -0
+/* memory key bits. */
+#define H_PTE_PKEY_BIT0	_RPAGE_RSV1
+#define H_PTE_PKEY_BIT1	_RPAGE_RSV2
+#define H_PTE_PKEY_BIT2	_RPAGE_RSV3
+#define H_PTE_PKEY_BIT3	_RPAGE_RSV4
+#define H_PTE_PKEY_BIT4	_RPAGE_RSV5
+

same comment as above.

-aneesh