powerpc: Memory Protection Keys

Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6 in the 4K backed hpte pages. These bits continue to be used for 64K backed hpte pages in this patch, but will be freed up in the next patch.

The patch does the following change to the 64K PTE format

H_PAGE_BUSY moves from bit 3 to bit 9

H_PAGE_F_SECOND which occupied bit 4 moves to the second part
of the pte.
H_PAGE_F_GIX which occupied bit 5, 6 and 7 also moves to the
second part of the pte.

the four bits((H_PAGE_F_SECOND|H_PAGE_F_GIX) that represent a slot is initialized to 0xF indicating an invalid slot. If a hpte
gets cached in a 0xF slot(i.e 7th slot of secondary), it is
released immediately. In other words, even though 0xF is a

valid slot we discard and consider it as an invalid slot;i.e hpte_soft_invalid(). This gives us an opportunity to not depend on a bit in the primary PTE in order to determine the validity of a slot.

When we release a hpte in the 0xF slot we also release a
legitimate primary slot and unmap that entry. This is to
ensure that we do get a legimate non-0xF slot the next time we
retry for a slot.

Though treating 0xF slot as invalid reduces the number of available slots and may have an effect on the performance, the probabilty of hitting a 0xF is extermely low.

Compared to the current scheme, the above described scheme reduces the number of false hash table updates significantly and has the

added advantage of releasing four valuable PTE bits for other purpose.

This idea was jointly developed by Paul Mackerras, Aneesh, Michael Ellermen and myself.

4K PTE format remain unchanged currently.

Signed-off-by Ram Pai <[email protected]>

---
 arch/powerpc/include/asm/book3s/64/hash-4k.h  | 20 +++++++
 arch/powerpc/include/asm/book3s/64/hash-64k.h | 32 +++++++----
 arch/powerpc/include/asm/book3s/64/hash.h     | 15 +++--
 arch/powerpc/include/asm/book3s/64/mmu-hash.h |  5 ++
 arch/powerpc/mm/dump_linuxpagetables.c        |  3 +-
 arch/powerpc/mm/hash64_4k.c                   | 14 ++---
 arch/powerpc/mm/hash64_64k.c                  | 81 ++++++++++++---------------
 arch/powerpc/mm/hash_utils_64.c               | 30 +++++++---
 8 files changed, 122 insertions(+), 78 deletions(-)

arch/powerpc/include/asm/book3s/64/hash-4k.h +2 -0

index b4b5e6b..5ef1d81 100644
@@ -16,6 +16,18 @@
 #define H_PUD_TABLE_SIZE	(sizeof(pud_t) << H_PUD_INDEX_SIZE)
 #define H_PGD_TABLE_SIZE	(sizeof(pgd_t) << H_PGD_INDEX_SIZE)
 
+
+/*

arch/powerpc/include/asm/book3s/64/hash-4k.h +2 -0

+
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
 static inline char *get_hpte_slot_array(pmd_t *pmdp)

arch/powerpc/include/asm/book3s/64/hash-64k.h +0 -0

index 9732837..0eb3c89 100644

arch/powerpc/include/asm/book3s/64/hash-64k.h +0 -0

 /*
  * we support 16 fragments per PTE page of 64K size.
  */

@@ -74,6 +76,16 @@ static inline unsigned long __rpte_to_hidx(real_pte_t rpte, unsigned long index)
 	return (pte_val(rpte.pte) >> H_PAGE_F_GIX_SHIFT) & 0xf;
 }

arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -0

+
 #define __rpte_to_pte(r)	((r).pte)
 extern bool __rpte_sub_valid(real_pte_t rpte, unsigned long index);
 /*

arch/powerpc/include/asm/book3s/64/hash.h +0 -0

index 4e957b0..e7cf03a 100644

arch/powerpc/include/asm/book3s/64/hash.h +10 -0

+
+#define INIT_HIDX (~0x0UL)
 
 #ifdef CONFIG_PPC_64K_PAGES
 #include <asm/book3s/64/hash-64k.h>

@@ -160,6 +157,14 @@ static inline int hash__pte_none(pte_t pte)
 	return (pte_val(pte) & ~H_PTE_NONE_MASK) == 0;
 }
 
+static inline bool hpte_soft_invalid(unsigned long slot)
+{
+	return ((slot & 0xfUL) == 0xfUL);
+}
+
+unsigned long get_hidx_gslot(unsigned long vpn, unsigned long shift,
+		int ssize, real_pte_t rpte, unsigned int subpg_index);
+
 /* This low level function performs the actual PTE insertion
  * Setting the PTE depends on the MMU type and other factors. It's
  * an horrible mess that I'm not going to try to clean up now but

arch/powerpc/include/asm/book3s/64/mmu-hash.h +0 -0

index 6981a52..cfb8169 100644

arch/powerpc/include/asm/book3s/64/mmu-hash.h +1 -0

+
 struct mm_struct;
 unsigned int hash_page_do_lazy_icache(unsigned int pp, pte_t pte, int trap);
 extern int hash_page_mm(struct mm_struct *mm, unsigned long ea,

arch/powerpc/mm/dump_linuxpagetables.c +1 -1

index 44fe483..b832ed3 100644
@@ -213,7 +213,7 @@ struct flag_info {
 		.val	= H_PAGE_4K_PFN,
 		.set	= "4K_pfn",
 	}, {
-#endif
+#else
 		.mask	= H_PAGE_F_GIX,
 		.val	= H_PAGE_F_GIX,
 		.set	= "f_gix",

arch/powerpc/mm/dump_linuxpagetables.c +0 -0

 #endif
 		.mask	= _PAGE_SPECIAL,
 		.val	= _PAGE_SPECIAL,

arch/powerpc/mm/hash64_4k.c +2 -0

index 6fa450c..c673829 100644
@@ -20,6 +20,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		   pte_t *ptep, unsigned long trap, unsigned long flags,
 		   int ssize, int subpg_prot)
 {
+	real_pte_t rpte;
 	unsigned long hpte_group;
 	unsigned long rflags, pa;
 	unsigned long old_pte, new_pte;

@@ -54,6 +55,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 	 * need to add in 0x1 if it's a read-only user page
 	 */
 	rflags = htab_convert_pte_flags(new_pte);
+	rpte = __real_pte(__pte(old_pte), ptep);
 
 	if (cpu_has_feature(CPU_FTR_NOEXECUTE) &&
 	    !cpu_has_feature(CPU_FTR_COHERENT_ICACHE))

Quoted 4 lines · Ram Pai

Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6 in the 4K backed hpte pages. These bits continue to be used for 64K backed hpte pages in this patch, but will be freed up in the next patch.

The counting 3, 4, 5 and 6 are in BE format I believe, I was initially trying to see that from right to left as we normally do in the kernel and was getting confused. So basically these bits (which are only applicable for 64K mapping IIUC) are going to be freed up from the PTE format.

#define _RPAGE_RSV1 0x1000000000000000UL
#define _RPAGE_RSV2 0x0800000000000000UL
#define _RPAGE_RSV3 0x0400000000000000UL
#define _RPAGE_RSV4 0x0200000000000000UL

As you have mentioned before this feature is available for 64K page size only and not for 4K mappings. So I assume we support both the combinations.

* 64K mapping on 64K
* 64K mapping on 4K

These are the current users of the above bits

#define H_PAGE_BUSY _RPAGE_RSV1 /* software: PTE & hash are busy */
#define H_PAGE_F_SECOND _RPAGE_RSV2 /* HPTE is in 2ndary HPTEG */
#define H_PAGE_F_GIX (_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
#define H_PAGE_HASHPTE _RPAGE_RPN43 /* PTE has associated HPTE */

Quoted 4 lines

The patch does the following change to the 64K PTE format

H_PAGE_BUSY moves from bit 3 to bit 9

and what is in there on bit 9 now ? This ?

#define _RPAGE_SW2 0x00400

which is used as

#define _PAGE_SPECIAL _RPAGE_SW2 /* software: special page */

which will not be required any more ?

Quoted 6 lines

H_PAGE_F_SECOND which occupied bit 4 moves to the second part
of the pte.
H_PAGE_F_GIX which occupied bit 5, 6 and 7 also moves to the
second part of the pte.

Quoted 4 lines

the four bits((H_PAGE_F_SECOND|H_PAGE_F_GIX) that represent a slot is initialized to 0xF indicating an invalid slot. If a hpte
gets cached in a 0xF slot(i.e 7th slot of secondary), it is
released immediately. In other words, even though 0xF is a

Release immediately means we attempt again for a new hash slot ?

Quoted 5 lines

valid slot we discard and consider it as an invalid slot;i.e hpte_soft_invalid(). This gives us an opportunity to not depend on a bit in the primary PTE in order to determine the validity of a slot.

So we have to see the slot number in the second half for each PTE to figure out if it has got a valid slot in the hash page table.

Quoted 6 lines

When we release a hpte in the 0xF slot we also release a
legitimate primary slot and unmap that entry. This is to
ensure that we do get a legimate non-0xF slot the next time we
retry for a slot.

Okay.

Quoted 1 line

Quoted 3 lines

Though treating 0xF slot as invalid reduces the number of available slots and may have an effect on the performance, the probabilty of hitting a 0xF is extermely low.

Why you say that ? I thought every slot number has the same probability of hit from the hash function.

Quoted 4 lines

Compared to the current scheme, the above described scheme reduces the number of false hash table updates significantly and has the

How it reduces false hash table updates ?

Quoted 29 lines

added advantage of releasing four valuable PTE bits for other purpose.

This idea was jointly developed by Paul Mackerras, Aneesh, Michael Ellermen and myself.

4K PTE format remain unchanged currently.

Signed-off-by Ram Pai <[email protected]>

---
 arch/powerpc/include/asm/book3s/64/hash-4k.h  | 20 +++++++
 arch/powerpc/include/asm/book3s/64/hash-64k.h | 32 +++++++----
 arch/powerpc/include/asm/book3s/64/hash.h     | 15 +++--
 arch/powerpc/include/asm/book3s/64/mmu-hash.h |  5 ++
 arch/powerpc/mm/dump_linuxpagetables.c        |  3 +-
 arch/powerpc/mm/hash64_4k.c                   | 14 ++---
 arch/powerpc/mm/hash64_64k.c                  | 81 ++++++++++++---------------
 arch/powerpc/mm/hash_utils_64.c               | 30 +++++++---
 8 files changed, 122 insertions(+), 78 deletions(-)

arch/powerpc/include/asm/book3s/64/hash-4k.h +0 -0

index b4b5e6b..5ef1d81 100644
@@ -16,6 +16,18 @@
 #define H_PUD_TABLE_SIZE	(sizeof(pud_t) << H_PUD_INDEX_SIZE)
 #define H_PGD_TABLE_SIZE	(sizeof(pgd_t) << H_PGD_INDEX_SIZE)
 

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

Quoted 12 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +12 -0

+
+/*
+ * Only supported by 4k linux page size
+ */
+#define H_PAGE_F_SECOND        _RPAGE_RSV2     /* HPTE is in 2ndary HPTEG */
+#define H_PAGE_F_GIX           (_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
+#define H_PAGE_F_GIX_SHIFT     56
+
+#define H_PAGE_BUSY	_RPAGE_RSV1     /* software: PTE & hash are busy */
+#define H_PAGE_HASHPTE	_RPAGE_RPN43    /* PTE has associated HPTE */
+
+

So we moved the common 64K definitions here.

Quoted 9 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +0 -0

 /* PTE flags to conserve for HPTE identification */
 #define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_HASHPTE | \
 			 H_PAGE_F_SECOND | H_PAGE_F_GIX)

@@ -48,6 +60,14 @@ static inline int hash__hugepd_ok(hugepd_t hpd)
 }
 #endif
 

arch/powerpc/include/asm/book3s/64/hash-4k.h static inline int hash__hugepd_ok(hugepd_t hpd) · line 68

Quoted 6 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +6 -0

+static inline unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+			unsigned int subpg_index, unsigned long slot)
+{
+	return (slot << H_PAGE_F_GIX_SHIFT) &
+		(H_PAGE_F_SECOND | H_PAGE_F_GIX);
+}

Why we are passing the first 3 arguments of the function if we never use it inside. Is the caller expected to take care of it ?

Quoted 5 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +2 -0

+
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 

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

Quoted 13 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +0 -0

 static inline char *get_hpte_slot_array(pmd_t *pmdp)

arch/powerpc/include/asm/book3s/64/hash-64k.h +2 -2

index 9732837..0eb3c89 100644
@@ -10,23 +10,25 @@
  * 64k aligned address free up few of the lower bits of RPN for us
  * We steal that here. For more deatils look at pte_pfn/pfn_pte()
  */
-#define H_PAGE_COMBO	_RPAGE_RPN0 /* this is a combo 4k page */
-#define H_PAGE_4K_PFN	_RPAGE_RPN1 /* PFN is for a single 4k page */
+#define H_PAGE_COMBO   _RPAGE_RPN0 /* this is a combo 4k page */
+#define H_PAGE_4K_PFN  _RPAGE_RPN1 /* PFN is for a single 4k page */

Its the same thing, changes nothing.

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

Quoted 8 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +7 -0

+#define H_PAGE_F_SECOND	_RPAGE_RSV2	/* HPTE is in 2ndary HPTEG */
+#define H_PAGE_F_GIX	(_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
+#define H_PAGE_F_GIX_SHIFT	56
+
+
+#define H_PAGE_BUSY	_RPAGE_RPN42     /* software: PTE & hash are busy */
+#define H_PAGE_HASHPTE	_RPAGE_RPN43    /* PTE has associated HPTE */

H_PAGE_BUSY seems to be differently defined here.

Quoted 8 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -0

+
 /*
  * We need to differentiate between explicit huge page and THP huge
  * page, since THP huge page also need to track real subpage details
  */
 #define H_PAGE_THP_HUGE  H_PAGE_4K_PFN
 

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

Quoted 5 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +0 -5

-/*
- * Used to track subpage group valid if H_PAGE_COMBO is set
- * This overloads H_PAGE_F_GIX and H_PAGE_F_SECOND
- */
-#define H_PAGE_COMBO_VALID	(H_PAGE_F_GIX | H_PAGE_F_SECOND)

H_PAGE_COMBO_VALID is not defined alternately ?

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

Quoted 7 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +2 -3

-
 /* PTE flags to conserve for HPTE identification */
-#define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_F_SECOND | \
-			 H_PAGE_F_GIX | H_PAGE_HASHPTE | H_PAGE_COMBO)
+#define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_HASHPTE | H_PAGE_COMBO)
+

Slot information has moved to the second half, hence _PAGE_HPTEFLAGS need not carry that.

Quoted 8 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +0 -0

 /*
  * we support 16 fragments per PTE page of 64K size.
  */

@@ -74,6 +76,16 @@ static inline unsigned long __rpte_to_hidx(real_pte_t rpte, unsigned long index)
 	return (pte_val(rpte.pte) >> H_PAGE_F_GIX_SHIFT) & 0xf;
 }
 

arch/powerpc/include/asm/book3s/64/hash-64k.h static inline unsigned long __rpte_to_hidx(real_pte_t rpte, unsigned long index) · line 87

Quoted 9 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +9 -0

+static inline unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+		unsigned int subpg_index, unsigned long slot)
+{
+	unsigned long *hidxp = (unsigned long *)(ptep + PTRS_PER_PTE);
+
+	rpte.hidx &= ~(0xfUL << (subpg_index << 2));
+	*hidxp = rpte.hidx  | (slot << (subpg_index << 2));
+	return 0x0UL;
+}

New method to insert the slot information in the second half.

Quoted 10 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -0

+
 #define __rpte_to_pte(r)	((r).pte)
 extern bool __rpte_sub_valid(real_pte_t rpte, unsigned long index);
 /*

arch/powerpc/include/asm/book3s/64/hash.h +0 -0

index 4e957b0..e7cf03a 100644

arch/powerpc/include/asm/book3s/64/hash.h line 15

Quoted 8 lines

arch/powerpc/include/asm/book3s/64/hash.h +0 -5

@@ -8,11 +8,8 @@
  *
  */
 #define H_PTE_NONE_MASK		_PAGE_HPTEFLAGS
-#define H_PAGE_F_GIX_SHIFT	56
-#define H_PAGE_BUSY		_RPAGE_RSV1 /* software: PTE & hash are busy */
-#define H_PAGE_F_SECOND		_RPAGE_RSV2	/* HPTE is in 2ndary HPTEG */
-#define H_PAGE_F_GIX		(_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
-#define H_PAGE_HASHPTE		_RPAGE_RPN43	/* PTE has associated HPTE */

Removing the common definitions.

Quoted 25 lines

arch/powerpc/include/asm/book3s/64/hash.h +10 -0

+
+#define INIT_HIDX (~0x0UL)
 
 #ifdef CONFIG_PPC_64K_PAGES
 #include <asm/book3s/64/hash-64k.h>

@@ -160,6 +157,14 @@ static inline int hash__pte_none(pte_t pte)
 	return (pte_val(pte) & ~H_PTE_NONE_MASK) == 0;
 }
 
+static inline bool hpte_soft_invalid(unsigned long slot)
+{
+	return ((slot & 0xfUL) == 0xfUL);
+}
+
+unsigned long get_hidx_gslot(unsigned long vpn, unsigned long shift,
+		int ssize, real_pte_t rpte, unsigned int subpg_index);
+
 /* This low level function performs the actual PTE insertion
  * Setting the PTE depends on the MMU type and other factors. It's
  * an horrible mess that I'm not going to try to clean up now but

arch/powerpc/include/asm/book3s/64/mmu-hash.h +0 -0

index 6981a52..cfb8169 100644

arch/powerpc/include/asm/book3s/64/mmu-hash.h extern int __hash_page_4K(unsigned long ea, unsigned long access, · line 441

Quoted 7 lines

arch/powerpc/include/asm/book3s/64/mmu-hash.h +4 -0

@@ -435,6 +435,11 @@ extern int __hash_page_4K(unsigned long ea, unsigned long access,
 extern int __hash_page_64K(unsigned long ea, unsigned long access,
 			   unsigned long vsid, pte_t *ptep, unsigned long trap,
 			   unsigned long flags, int ssize);
+extern unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+			unsigned int subpg_index, unsigned long slot);
+extern unsigned long get_hidx_slot(unsigned long vpn, unsigned long shift,
+			int ssize, real_pte_t rpte, unsigned int subpg_index);

I wonder what purpose set_hidx_slot() defined previously, served.

Quoted 16 lines

arch/powerpc/include/asm/book3s/64/mmu-hash.h +1 -0

+
 struct mm_struct;
 unsigned int hash_page_do_lazy_icache(unsigned int pp, pte_t pte, int trap);
 extern int hash_page_mm(struct mm_struct *mm, unsigned long ea,

arch/powerpc/mm/dump_linuxpagetables.c +1 -1

index 44fe483..b832ed3 100644
@@ -213,7 +213,7 @@ struct flag_info {
 		.val	= H_PAGE_4K_PFN,
 		.set	= "4K_pfn",
 	}, {
-#endif
+#else
 		.mask	= H_PAGE_F_GIX,
 		.val	= H_PAGE_F_GIX,

arch/powerpc/mm/dump_linuxpagetables.c struct flag_info { · line 227

Quoted 6 lines

arch/powerpc/mm/dump_linuxpagetables.c +1 -0

 		.set	= "f_gix",

@@ -224,6 +224,7 @@ struct flag_info {
 		.val	= H_PAGE_F_SECOND,
 		.set	= "f_second",
 	}, {
+#endif /* CONFIG_PPC_64K_PAGES */

Are we adding H_PAGE_F_GIX as an element for 4K mapping ?

Quoted 22 lines

arch/powerpc/mm/dump_linuxpagetables.c +0 -0

 #endif
 		.mask	= _PAGE_SPECIAL,
 		.val	= _PAGE_SPECIAL,

arch/powerpc/mm/hash64_4k.c +2 -0

index 6fa450c..c673829 100644
@@ -20,6 +20,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		   pte_t *ptep, unsigned long trap, unsigned long flags,
 		   int ssize, int subpg_prot)
 {
+	real_pte_t rpte;
 	unsigned long hpte_group;
 	unsigned long rflags, pa;
 	unsigned long old_pte, new_pte;

@@ -54,6 +55,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 	 * need to add in 0x1 if it's a read-only user page
 	 */
 	rflags = htab_convert_pte_flags(new_pte);
+	rpte = __real_pte(__pte(old_pte), ptep);
 

arch/powerpc/mm/hash64_4k.c int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid, · line 70

Quoted 13 lines

arch/powerpc/mm/hash64_4k.c +2 -5

 	if (cpu_has_feature(CPU_FTR_NOEXECUTE) &&
 	    !cpu_has_feature(CPU_FTR_COHERENT_ICACHE))

@@ -64,13 +66,10 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		/*
 		 * There MIGHT be an HPTE for this pte
 		 */
-		hash = hpt_hash(vpn, shift, ssize);
-		if (old_pte & H_PAGE_F_SECOND)
-			hash = ~hash;
-		slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
-		slot += (old_pte & H_PAGE_F_GIX) >> H_PAGE_F_GIX_SHIFT;
+		unsigned long gslot = get_hidx_gslot(vpn, shift,
+						ssize, rpte, 0);

I am wondering why there is a 'g' before the slot in all these functions.

Its already too much of changes in a single patch. Being a single logical change it needs to be inside a single change but then we need much more description in the commit message for some one to understand what all changed and how.

Quoted 17 lines · Anshuman Khandual

Quoted 4 lines · Ram Pai

Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6 in the 4K backed hpte pages. These bits continue to be used for 64K backed hpte pages in this patch, but will be freed up in the next patch.

The counting 3, 4, 5 and 6 are in BE format I believe, I was initially trying to see that from right to left as we normally do in the kernel and was getting confused. So basically these bits (which are only applicable for 64K mapping IIUC) are going to be freed up from the PTE format.

#define _RPAGE_RSV1 0x1000000000000000UL
#define _RPAGE_RSV2 0x0800000000000000UL
#define _RPAGE_RSV3 0x0400000000000000UL
#define _RPAGE_RSV4 0x0200000000000000UL

Quoted 6 lines

As you have mentioned before this feature is available for 64K page size only and not for 4K mappings. So I assume we support both the combinations.

* 64K mapping on 64K
* 64K mapping on 4K

yes.

Quoted 17 lines

These are the current users of the above bits

#define H_PAGE_BUSY _RPAGE_RSV1 /* software: PTE & hash are busy */
#define H_PAGE_F_SECOND _RPAGE_RSV2 /* HPTE is in 2ndary HPTEG */
#define H_PAGE_F_GIX (_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
#define H_PAGE_HASHPTE _RPAGE_RPN43 /* PTE has associated HPTE */

Quoted 4 lines

The patch does the following change to the 64K PTE format

H_PAGE_BUSY moves from bit 3 to bit 9

and what is in there on bit 9 now ? This ?

#define _RPAGE_SW2 0x00400

Quoted 5 lines

which is used as

#define _PAGE_SPECIAL _RPAGE_SW2 /* software: special page */

which will not be required any more ?

i think you are reading bit 9 from right to left. the bit 9 i refer to is from left to right. Using the same numbering convention the ISA3.0 uses.

I know it is confusing, will make a mention in the comment of this patch, to read it the big-endian way.

BTW: Bit 9 is not used currently. so using it in this patch. But this is a temporary move. the H_PAGE_BUSY will move to bit 7 in the next patch.

Had to keep at bit 9, because bit 7 is not yet entirely freed up. it is used by 64K PTE backed by 64k htpe.

Quoted 7 lines

Quoted 7 lines

H_PAGE_F_SECOND which occupied bit 4 moves to the second part
of the pte.
H_PAGE_F_GIX which occupied bit 5, 6 and 7 also moves to the
second part of the pte.

Quoted 6 lines

Quoted 4 lines

the four bits((H_PAGE_F_SECOND|H_PAGE_F_GIX) that represent a slot is initialized to 0xF indicating an invalid slot. If a hpte
gets cached in a 0xF slot(i.e 7th slot of secondary), it is
released immediately. In other words, even though 0xF is a

Release immediately means we attempt again for a new hash slot ?

yes.

Quoted 8 lines

Quoted 5 lines

valid slot we discard and consider it as an invalid slot;i.e hpte_soft_invalid(). This gives us an opportunity to not depend on a bit in the primary PTE in order to determine the validity of a slot.

So we have to see the slot number in the second half for each PTE to figure out if it has got a valid slot in the hash page table.

yes.

Quoted 14 lines

Quoted 6 lines

When we release a hpte in the 0xF slot we also release a
legitimate primary slot and unmap that entry. This is to
ensure that we do get a legimate non-0xF slot the next time we
retry for a slot.

Okay.

Quoted 4 lines

Though treating 0xF slot as invalid reduces the number of available slots and may have an effect on the performance, the probabilty of hitting a 0xF is extermely low.

Quoted 2 lines

Why you say that ? I thought every slot number has the same probability of hit from the hash function.

Every hash bucket has the same probability. But every slot within the hash bucket is filled in sequentially. so it takes 15 hptes to hash to the same bucket before we get to the 15th slot in the secondary.

Quoted 6 lines

Quoted 5 lines

Compared to the current scheme, the above described scheme reduces the number of false hash table updates significantly and has the

Quoted 1 line

How it reduces false hash table updates ?

earlier, we had 1 bit allocated in the first-part-of-the 64K-PTE for four consecutive 4K hptes. If any one 4k hpte got hashed-in, the bit got set. Which means anytime it faulted on the remaining three 4k hpte, we saw the bit already set and tried to erroneously update that hpte. So we had a 75% update error rate. Funcationally not bad, but bad from a performance point of view.

With the current scheme, we decide if a 4k slot is valid by looking at its value rather than depending on a bit in the main-pte. So there is no chance of getting mislead. And hence no chance of trying to update a invalid hpte. Should improve performance and at the same time give us four valuable PTE bits.

Quoted 31 lines

Quoted 31 lines

added advantage of releasing four valuable PTE bits for other purpose.

This idea was jointly developed by Paul Mackerras, Aneesh, Michael Ellermen and myself.

4K PTE format remain unchanged currently.

Signed-off-by Ram Pai <[email protected]>

---
 arch/powerpc/include/asm/book3s/64/hash-4k.h  | 20 +++++++
 arch/powerpc/include/asm/book3s/64/hash-64k.h | 32 +++++++----
 arch/powerpc/include/asm/book3s/64/hash.h     | 15 +++--
 arch/powerpc/include/asm/book3s/64/mmu-hash.h |  5 ++
 arch/powerpc/mm/dump_linuxpagetables.c        |  3 +-
 arch/powerpc/mm/hash64_4k.c                   | 14 ++---
 arch/powerpc/mm/hash64_64k.c                  | 81 ++++++++++++---------------
 arch/powerpc/mm/hash_utils_64.c               | 30 +++++++---
 8 files changed, 122 insertions(+), 78 deletions(-)

arch/powerpc/include/asm/book3s/64/hash-4k.h +0 -0

index b4b5e6b..5ef1d81 100644
@@ -16,6 +16,18 @@
 #define H_PUD_TABLE_SIZE	(sizeof(pud_t) << H_PUD_INDEX_SIZE)
 #define H_PGD_TABLE_SIZE	(sizeof(pgd_t) << H_PGD_INDEX_SIZE)
 

Quoted 14 lines

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

Quoted 12 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +12 -0

+
+/*
+ * Only supported by 4k linux page size
+ */
+#define H_PAGE_F_SECOND        _RPAGE_RSV2     /* HPTE is in 2ndary HPTEG */
+#define H_PAGE_F_GIX           (_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
+#define H_PAGE_F_GIX_SHIFT     56
+
+#define H_PAGE_BUSY	_RPAGE_RSV1     /* software: PTE & hash are busy */
+#define H_PAGE_HASHPTE	_RPAGE_RPN43    /* PTE has associated HPTE */
+
+

So we moved the common 64K definitions here.

yes.

Quoted 9 lines

Quoted 9 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +0 -0

 /* PTE flags to conserve for HPTE identification */
 #define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_HASHPTE | \
 			 H_PAGE_F_SECOND | H_PAGE_F_GIX)

@@ -48,6 +60,14 @@ static inline int hash__hugepd_ok(hugepd_t hpd)
 }
 #endif
 

Quoted 9 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h static inline int hash__hugepd_ok(hugepd_t hpd) · line 68

Quoted 6 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +6 -0

+static inline unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+			unsigned int subpg_index, unsigned long slot)
+{
+	return (slot << H_PAGE_F_GIX_SHIFT) &
+		(H_PAGE_F_SECOND | H_PAGE_F_GIX);
+}

Why we are passing the first 3 arguments of the function if we never use it inside. Is the caller expected to take care of it ?

trying to keep the same prototype for the 4K-pte and 64K-pte cases. Otherwise the caller has to wonder which parameter scheme to use.

Quoted 5 lines

Quoted 5 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +2 -0

+
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 

Quoted 15 lines

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

Quoted 13 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +0 -0

 static inline char *get_hpte_slot_array(pmd_t *pmdp)

arch/powerpc/include/asm/book3s/64/hash-64k.h +2 -2

index 9732837..0eb3c89 100644
@@ -10,23 +10,25 @@
  * 64k aligned address free up few of the lower bits of RPN for us
  * We steal that here. For more deatils look at pte_pfn/pfn_pte()
  */
-#define H_PAGE_COMBO	_RPAGE_RPN0 /* this is a combo 4k page */
-#define H_PAGE_4K_PFN	_RPAGE_RPN1 /* PFN is for a single 4k page */
+#define H_PAGE_COMBO   _RPAGE_RPN0 /* this is a combo 4k page */
+#define H_PAGE_4K_PFN  _RPAGE_RPN1 /* PFN is for a single 4k page */

Its the same thing, changes nothing.

it fixes some space/tab problem.

Quoted 10 lines

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

Quoted 8 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +7 -0

+#define H_PAGE_F_SECOND	_RPAGE_RSV2	/* HPTE is in 2ndary HPTEG */
+#define H_PAGE_F_GIX	(_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
+#define H_PAGE_F_GIX_SHIFT	56
+
+
+#define H_PAGE_BUSY	_RPAGE_RPN42     /* software: PTE & hash are busy */
+#define H_PAGE_HASHPTE	_RPAGE_RPN43    /* PTE has associated HPTE */

H_PAGE_BUSY seems to be differently defined here.

Yes. it is using two different bits depending on 4K hpte v/s 64k hpte case. But in the next patch all will be same and consistent.

Quoted 8 lines

Quoted 8 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -0

+
 /*
  * We need to differentiate between explicit huge page and THP huge
  * page, since THP huge page also need to track real subpage details
  */
 #define H_PAGE_THP_HUGE  H_PAGE_4K_PFN
 

Quoted 7 lines

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

Quoted 5 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +0 -5

-/*
- * Used to track subpage group valid if H_PAGE_COMBO is set
- * This overloads H_PAGE_F_GIX and H_PAGE_F_SECOND
- */
-#define H_PAGE_COMBO_VALID	(H_PAGE_F_GIX | H_PAGE_F_SECOND)

H_PAGE_COMBO_VALID is not defined alternately ?

it is not needed anymore.

Quoted 10 lines

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

Quoted 7 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +2 -3

-
 /* PTE flags to conserve for HPTE identification */
-#define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_F_SECOND | \
-			 H_PAGE_F_GIX | H_PAGE_HASHPTE | H_PAGE_COMBO)
+#define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_HASHPTE | H_PAGE_COMBO)
+

Slot information has moved to the second half, hence _PAGE_HPTEFLAGS need not carry that.

yes.

Quoted 8 lines

Quoted 8 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +0 -0

 /*
  * we support 16 fragments per PTE page of 64K size.
  */

@@ -74,6 +76,16 @@ static inline unsigned long __rpte_to_hidx(real_pte_t rpte, unsigned long index)
 	return (pte_val(rpte.pte) >> H_PAGE_F_GIX_SHIFT) & 0xf;
 }
 

Quoted 11 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h static inline unsigned long __rpte_to_hidx(real_pte_t rpte, unsigned long index) · line 87

Quoted 9 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +9 -0

+static inline unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+		unsigned int subpg_index, unsigned long slot)
+{
+	unsigned long *hidxp = (unsigned long *)(ptep + PTRS_PER_PTE);
+
+	rpte.hidx &= ~(0xfUL << (subpg_index << 2));
+	*hidxp = rpte.hidx  | (slot << (subpg_index << 2));
+	return 0x0UL;
+}

New method to insert the slot information in the second half.

yes. well it basically trying to reduce code redundancy. Too many places using exactly the same code to accomplish the same thing. Makes sense to bring it all in one place.

Quoted 73 lines

Quoted 11 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -0

+
 #define __rpte_to_pte(r)	((r).pte)
 extern bool __rpte_sub_valid(real_pte_t rpte, unsigned long index);
 /*

arch/powerpc/include/asm/book3s/64/hash.h +0 -0

index 4e957b0..e7cf03a 100644

arch/powerpc/include/asm/book3s/64/hash.h line 15

Quoted 8 lines

arch/powerpc/include/asm/book3s/64/hash.h +0 -5

@@ -8,11 +8,8 @@
  *
  */
 #define H_PTE_NONE_MASK		_PAGE_HPTEFLAGS
-#define H_PAGE_F_GIX_SHIFT	56
-#define H_PAGE_BUSY		_RPAGE_RSV1 /* software: PTE & hash are busy */
-#define H_PAGE_F_SECOND		_RPAGE_RSV2	/* HPTE is in 2ndary HPTEG */
-#define H_PAGE_F_GIX		(_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
-#define H_PAGE_HASHPTE		_RPAGE_RPN43	/* PTE has associated HPTE */

Removing the common definitions.

Quoted 25 lines

arch/powerpc/include/asm/book3s/64/hash.h +10 -0

+
+#define INIT_HIDX (~0x0UL)
 
 #ifdef CONFIG_PPC_64K_PAGES
 #include <asm/book3s/64/hash-64k.h>

@@ -160,6 +157,14 @@ static inline int hash__pte_none(pte_t pte)
 	return (pte_val(pte) & ~H_PTE_NONE_MASK) == 0;
 }
 
+static inline bool hpte_soft_invalid(unsigned long slot)
+{
+	return ((slot & 0xfUL) == 0xfUL);
+}
+
+unsigned long get_hidx_gslot(unsigned long vpn, unsigned long shift,
+		int ssize, real_pte_t rpte, unsigned int subpg_index);
+
 /* This low level function performs the actual PTE insertion
  * Setting the PTE depends on the MMU type and other factors. It's
  * an horrible mess that I'm not going to try to clean up now but

arch/powerpc/include/asm/book3s/64/mmu-hash.h +0 -0

index 6981a52..cfb8169 100644

arch/powerpc/include/asm/book3s/64/mmu-hash.h extern int __hash_page_4K(unsigned long ea, unsigned long access, · line 441

Quoted 7 lines

arch/powerpc/include/asm/book3s/64/mmu-hash.h +4 -0

@@ -435,6 +435,11 @@ extern int __hash_page_4K(unsigned long ea, unsigned long access,
 extern int __hash_page_64K(unsigned long ea, unsigned long access,
 			   unsigned long vsid, pte_t *ptep, unsigned long trap,
 			   unsigned long flags, int ssize);
+extern unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+			unsigned int subpg_index, unsigned long slot);
+extern unsigned long get_hidx_slot(unsigned long vpn, unsigned long shift,
+			int ssize, real_pte_t rpte, unsigned int subpg_index);

I wonder what purpose set_hidx_slot() defined previously, served.

Quoted 16 lines

arch/powerpc/include/asm/book3s/64/mmu-hash.h +1 -0

+
 struct mm_struct;
 unsigned int hash_page_do_lazy_icache(unsigned int pp, pte_t pte, int trap);
 extern int hash_page_mm(struct mm_struct *mm, unsigned long ea,

arch/powerpc/mm/dump_linuxpagetables.c +1 -1

index 44fe483..b832ed3 100644
@@ -213,7 +213,7 @@ struct flag_info {
 		.val	= H_PAGE_4K_PFN,
 		.set	= "4K_pfn",
 	}, {
-#endif
+#else
 		.mask	= H_PAGE_F_GIX,
 		.val	= H_PAGE_F_GIX,

Quoted 8 lines

arch/powerpc/mm/dump_linuxpagetables.c struct flag_info { · line 227

Quoted 6 lines

arch/powerpc/mm/dump_linuxpagetables.c +1 -0

 		.set	= "f_gix",

@@ -224,6 +224,7 @@ struct flag_info {
 		.val	= H_PAGE_F_SECOND,
 		.set	= "f_second",
 	}, {
+#endif /* CONFIG_PPC_64K_PAGES */

Are we adding H_PAGE_F_GIX as an element for 4K mapping ?

I think there is mistake here.
In the next patch when these bits are divorsed from 64K ptes entirely, we will not need the above code for 64K ptes. But good catch. Will fix the error in this patch.

Quoted 22 lines

Quoted 22 lines

arch/powerpc/mm/dump_linuxpagetables.c +0 -0

 #endif
 		.mask	= _PAGE_SPECIAL,
 		.val	= _PAGE_SPECIAL,

arch/powerpc/mm/hash64_4k.c +2 -0

index 6fa450c..c673829 100644
@@ -20,6 +20,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		   pte_t *ptep, unsigned long trap, unsigned long flags,
 		   int ssize, int subpg_prot)
 {
+	real_pte_t rpte;
 	unsigned long hpte_group;
 	unsigned long rflags, pa;
 	unsigned long old_pte, new_pte;

@@ -54,6 +55,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 	 * need to add in 0x1 if it's a read-only user page
 	 */
 	rflags = htab_convert_pte_flags(new_pte);
+	rpte = __real_pte(__pte(old_pte), ptep);
 

Quoted 16 lines

arch/powerpc/mm/hash64_4k.c int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid, · line 70

Quoted 13 lines

arch/powerpc/mm/hash64_4k.c +2 -5

 	if (cpu_has_feature(CPU_FTR_NOEXECUTE) &&
 	    !cpu_has_feature(CPU_FTR_COHERENT_ICACHE))

@@ -64,13 +66,10 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		/*
 		 * There MIGHT be an HPTE for this pte
 		 */
-		hash = hpt_hash(vpn, shift, ssize);
-		if (old_pte & H_PAGE_F_SECOND)
-			hash = ~hash;
-		slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
-		slot += (old_pte & H_PAGE_F_GIX) >> H_PAGE_F_GIX_SHIFT;
+		unsigned long gslot = get_hidx_gslot(vpn, shift,
+						ssize, rpte, 0);

I am wondering why there is a 'g' before the slot in all these functions.

Right. even i was confused initially. :)

hash table slots are originized as one big table. 8 consecutive entires in that table form a bucket. the term slot is used to refer to the slot within the bucket. the term gslot is used to refer to an entry in the table. roughly speaking slot 2 in bucket 2, will be gslot 2*8+2=18.

Quoted 6 lines

Its already too much of changes in a single patch. Being a single logical change it needs to be inside a single change but then we need much more description in the commit message for some one to understand what all changed and how.

I have further broken down this patch, one to introduce get_hidx_gslot() one to introduce set_hidx_slot() . Hopefully that will reduce the size of the patch to graspable level. let me know,

Quoted 44 lines · Ram Pai

Quoted 17 lines · Anshuman Khandual

Quoted 4 lines · Ram Pai

Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6 in the 4K backed hpte pages. These bits continue to be used for 64K backed hpte pages in this patch, but will be freed up in the next patch.

The counting 3, 4, 5 and 6 are in BE format I believe, I was initially trying to see that from right to left as we normally do in the kernel and was getting confused. So basically these bits (which are only applicable for 64K mapping IIUC) are going to be freed up from the PTE format.

#define _RPAGE_RSV1 0x1000000000000000UL
#define _RPAGE_RSV2 0x0800000000000000UL
#define _RPAGE_RSV3 0x0400000000000000UL
#define _RPAGE_RSV4 0x0200000000000000UL

Quoted 6 lines

As you have mentioned before this feature is available for 64K page size only and not for 4K mappings. So I assume we support both the combinations.

* 64K mapping on 64K
* 64K mapping on 4K

yes.

Quoted 17 lines

These are the current users of the above bits

#define H_PAGE_BUSY _RPAGE_RSV1 /* software: PTE & hash are busy */
#define H_PAGE_F_SECOND _RPAGE_RSV2 /* HPTE is in 2ndary HPTEG */
#define H_PAGE_F_GIX (_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
#define H_PAGE_HASHPTE _RPAGE_RPN43 /* PTE has associated HPTE */

Quoted 4 lines

The patch does the following change to the 64K PTE format

H_PAGE_BUSY moves from bit 3 to bit 9

and what is in there on bit 9 now ? This ?

#define _RPAGE_SW2 0x00400

Quoted 8 lines

Quoted 5 lines

which is used as

#define _PAGE_SPECIAL _RPAGE_SW2 /* software: special page */

which will not be required any more ?

i think you are reading bit 9 from right to left. the bit 9 i refer to is from left to right. Using the same numbering convention the ISA3.0 uses.

Right, my bad. Then it would be this one.

'#define _RPAGE_RPN42 0x0040000000000000UL'

Quoted 2 lines

I know it is confusing, will make a mention in the comment of this patch, to read it the big-endian way.

Right.

Quoted 6 lines

BTW: Bit 9 is not used currently. so using it in this patch. But this is a temporary move. the H_PAGE_BUSY will move to bit 7 in the next patch.

Had to keep at bit 9, because bit 7 is not yet entirely freed up. it is used by 64K PTE backed by 64k htpe.

Got it.

Quoted 41 lines

Quoted 7 lines

Quoted 7 lines

H_PAGE_F_SECOND which occupied bit 4 moves to the second part
of the pte.
H_PAGE_F_GIX which occupied bit 5, 6 and 7 also moves to the
second part of the pte.

Quoted 6 lines

Quoted 4 lines

the four bits((H_PAGE_F_SECOND|H_PAGE_F_GIX) that represent a slot is initialized to 0xF indicating an invalid slot. If a hpte
gets cached in a 0xF slot(i.e 7th slot of secondary), it is
released immediately. In other words, even though 0xF is a

Release immediately means we attempt again for a new hash slot ?

yes.

Quoted 8 lines

Quoted 5 lines

valid slot we discard and consider it as an invalid slot;i.e hpte_soft_invalid(). This gives us an opportunity to not depend on a bit in the primary PTE in order to determine the validity of a slot.

So we have to see the slot number in the second half for each PTE to figure out if it has got a valid slot in the hash page table.

yes.

Quoted 14 lines

Quoted 6 lines

When we release a hpte in the 0xF slot we also release a
legitimate primary slot and unmap that entry. This is to
ensure that we do get a legimate non-0xF slot the next time we
retry for a slot.

Okay.

Quoted 4 lines

Though treating 0xF slot as invalid reduces the number of available slots and may have an effect on the performance, the probabilty of hitting a 0xF is extermely low.

Quoted 6 lines

Quoted 2 lines

Why you say that ? I thought every slot number has the same probability of hit from the hash function.

Every hash bucket has the same probability. But every slot within the hash bucket is filled in sequentially. so it takes 15 hptes to hash to the same bucket before we get to the 15th slot in the secondary.

Okay, would the last one be 16th instead ?

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

Compared to the current scheme, the above described scheme reduces the number of false hash table updates significantly and has the

Quoted 8 lines

Quoted 1 line

How it reduces false hash table updates ?

earlier, we had 1 bit allocated in the first-part-of-the 64K-PTE for four consecutive 4K hptes. If any one 4k hpte got hashed-in, the bit got set. Which means anytime it faulted on the remaining three 4k hpte, we saw the bit already set and tried to erroneously update that hpte. So we had a 75% update error rate. Funcationally not bad, but bad from a performance point of view.

I am bit out of sync regarding these PTE bits, after Aneesh's radix changes went in :) Will look into this bit closer.

Quoted 1 line

Quoted 5 lines

With the current scheme, we decide if a 4k slot is valid by looking at its value rather than depending on a bit in the main-pte. So there is no chance of getting mislead. And hence no chance of trying to update a invalid hpte. Should improve performance and at the same time give us four valuable PTE bits.

I am not sure why you say 'invalid hpte'. IIUC

* We will require 16 '64K on 4K' mappings to actually cover 64K on 64K

* A single (64K on 4K)'s TLB can cover 64K on 64K as long as the TLB is present and not flushed. That gets us performance. Once flushed, a new HPTE entry covering new (64K on 4K) is inserted. As long as the PFN for the 4K is different HPTE will be different and it cannot collide with any existing ones and create problems (ERAT error ?)

As you are pointing out, I am not sure whether the existing design had more probability for an invalid HPTE insert. Will look into this in detail.

Quoted 157 lines

Quoted 31 lines

Quoted 31 lines

added advantage of releasing four valuable PTE bits for other purpose.

This idea was jointly developed by Paul Mackerras, Aneesh, Michael Ellermen and myself.

4K PTE format remain unchanged currently.

Signed-off-by Ram Pai <[email protected]>

---
 arch/powerpc/include/asm/book3s/64/hash-4k.h  | 20 +++++++
 arch/powerpc/include/asm/book3s/64/hash-64k.h | 32 +++++++----
 arch/powerpc/include/asm/book3s/64/hash.h     | 15 +++--
 arch/powerpc/include/asm/book3s/64/mmu-hash.h |  5 ++
 arch/powerpc/mm/dump_linuxpagetables.c        |  3 +-
 arch/powerpc/mm/hash64_4k.c                   | 14 ++---
 arch/powerpc/mm/hash64_64k.c                  | 81 ++++++++++++---------------
 arch/powerpc/mm/hash_utils_64.c               | 30 +++++++---
 8 files changed, 122 insertions(+), 78 deletions(-)

arch/powerpc/include/asm/book3s/64/hash-4k.h +0 -0

index b4b5e6b..5ef1d81 100644
@@ -16,6 +16,18 @@
 #define H_PUD_TABLE_SIZE	(sizeof(pud_t) << H_PUD_INDEX_SIZE)
 #define H_PGD_TABLE_SIZE	(sizeof(pgd_t) << H_PGD_INDEX_SIZE)
 

Quoted 14 lines

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

Quoted 12 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +12 -0

+
+/*
+ * Only supported by 4k linux page size
+ */
+#define H_PAGE_F_SECOND        _RPAGE_RSV2     /* HPTE is in 2ndary HPTEG */
+#define H_PAGE_F_GIX           (_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
+#define H_PAGE_F_GIX_SHIFT     56
+
+#define H_PAGE_BUSY	_RPAGE_RSV1     /* software: PTE & hash are busy */
+#define H_PAGE_HASHPTE	_RPAGE_RPN43    /* PTE has associated HPTE */
+
+

So we moved the common 64K definitions here.

yes.

Quoted 9 lines

Quoted 9 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +0 -0

 /* PTE flags to conserve for HPTE identification */
 #define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_HASHPTE | \
 			 H_PAGE_F_SECOND | H_PAGE_F_GIX)

@@ -48,6 +60,14 @@ static inline int hash__hugepd_ok(hugepd_t hpd)
 }
 #endif
 

Quoted 9 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h static inline int hash__hugepd_ok(hugepd_t hpd) · line 68

Quoted 6 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +6 -0

+static inline unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+			unsigned int subpg_index, unsigned long slot)
+{
+	return (slot << H_PAGE_F_GIX_SHIFT) &
+		(H_PAGE_F_SECOND | H_PAGE_F_GIX);
+}

Why we are passing the first 3 arguments of the function if we never use it inside. Is the caller expected to take care of it ?

trying to keep the same prototype for the 4K-pte and 64K-pte cases. Otherwise the caller has to wonder which parameter scheme to use.

Quoted 5 lines

Quoted 5 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +2 -0

+
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 

Quoted 15 lines

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

Quoted 13 lines

arch/powerpc/include/asm/book3s/64/hash-4k.h +0 -0

 static inline char *get_hpte_slot_array(pmd_t *pmdp)

arch/powerpc/include/asm/book3s/64/hash-64k.h +2 -2

index 9732837..0eb3c89 100644
@@ -10,23 +10,25 @@
  * 64k aligned address free up few of the lower bits of RPN for us
  * We steal that here. For more deatils look at pte_pfn/pfn_pte()
  */
-#define H_PAGE_COMBO	_RPAGE_RPN0 /* this is a combo 4k page */
-#define H_PAGE_4K_PFN	_RPAGE_RPN1 /* PFN is for a single 4k page */
+#define H_PAGE_COMBO   _RPAGE_RPN0 /* this is a combo 4k page */
+#define H_PAGE_4K_PFN  _RPAGE_RPN1 /* PFN is for a single 4k page */

Its the same thing, changes nothing.

it fixes some space/tab problem.

Quoted 10 lines

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

Quoted 8 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +7 -0

+#define H_PAGE_F_SECOND	_RPAGE_RSV2	/* HPTE is in 2ndary HPTEG */
+#define H_PAGE_F_GIX	(_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
+#define H_PAGE_F_GIX_SHIFT	56
+
+
+#define H_PAGE_BUSY	_RPAGE_RPN42     /* software: PTE & hash are busy */
+#define H_PAGE_HASHPTE	_RPAGE_RPN43    /* PTE has associated HPTE */

H_PAGE_BUSY seems to be differently defined here.

Yes. it is using two different bits depending on 4K hpte v/s 64k hpte case. But in the next patch all will be same and consistent.

Quoted 8 lines

Quoted 8 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -0

+
 /*
  * We need to differentiate between explicit huge page and THP huge
  * page, since THP huge page also need to track real subpage details
  */
 #define H_PAGE_THP_HUGE  H_PAGE_4K_PFN
 

Quoted 7 lines

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

Quoted 5 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +0 -5

-/*
- * Used to track subpage group valid if H_PAGE_COMBO is set
- * This overloads H_PAGE_F_GIX and H_PAGE_F_SECOND
- */
-#define H_PAGE_COMBO_VALID	(H_PAGE_F_GIX | H_PAGE_F_SECOND)

H_PAGE_COMBO_VALID is not defined alternately ?

it is not needed anymore.

Quoted 10 lines

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

Quoted 7 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +2 -3

-
 /* PTE flags to conserve for HPTE identification */
-#define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_F_SECOND | \
-			 H_PAGE_F_GIX | H_PAGE_HASHPTE | H_PAGE_COMBO)
+#define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_HASHPTE | H_PAGE_COMBO)
+

Slot information has moved to the second half, hence _PAGE_HPTEFLAGS need not carry that.

yes.

Quoted 8 lines

Quoted 8 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +0 -0

 /*
  * we support 16 fragments per PTE page of 64K size.
  */

@@ -74,6 +76,16 @@ static inline unsigned long __rpte_to_hidx(real_pte_t rpte, unsigned long index)
 	return (pte_val(rpte.pte) >> H_PAGE_F_GIX_SHIFT) & 0xf;
 }
 

Quoted 11 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h static inline unsigned long __rpte_to_hidx(real_pte_t rpte, unsigned long index) · line 87

Quoted 9 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +9 -0

+static inline unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+		unsigned int subpg_index, unsigned long slot)
+{
+	unsigned long *hidxp = (unsigned long *)(ptep + PTRS_PER_PTE);
+
+	rpte.hidx &= ~(0xfUL << (subpg_index << 2));
+	*hidxp = rpte.hidx  | (slot << (subpg_index << 2));
+	return 0x0UL;
+}

New method to insert the slot information in the second half.

Quoted 3 lines

yes. well it basically trying to reduce code redundancy. Too many places using exactly the same code to accomplish the same thing. Makes sense to bring it all in one place.

Right.

Quoted 126 lines

Quoted 73 lines

Quoted 11 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -0

+
 #define __rpte_to_pte(r)	((r).pte)
 extern bool __rpte_sub_valid(real_pte_t rpte, unsigned long index);
 /*

arch/powerpc/include/asm/book3s/64/hash.h +0 -0

index 4e957b0..e7cf03a 100644

arch/powerpc/include/asm/book3s/64/hash.h line 15

Quoted 8 lines

arch/powerpc/include/asm/book3s/64/hash.h +0 -5

@@ -8,11 +8,8 @@
  *
  */
 #define H_PTE_NONE_MASK		_PAGE_HPTEFLAGS
-#define H_PAGE_F_GIX_SHIFT	56
-#define H_PAGE_BUSY		_RPAGE_RSV1 /* software: PTE & hash are busy */
-#define H_PAGE_F_SECOND		_RPAGE_RSV2	/* HPTE is in 2ndary HPTEG */
-#define H_PAGE_F_GIX		(_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
-#define H_PAGE_HASHPTE		_RPAGE_RPN43	/* PTE has associated HPTE */

Removing the common definitions.

Quoted 25 lines

arch/powerpc/include/asm/book3s/64/hash.h +10 -0

+
+#define INIT_HIDX (~0x0UL)
 
 #ifdef CONFIG_PPC_64K_PAGES
 #include <asm/book3s/64/hash-64k.h>

@@ -160,6 +157,14 @@ static inline int hash__pte_none(pte_t pte)
 	return (pte_val(pte) & ~H_PTE_NONE_MASK) == 0;
 }
 
+static inline bool hpte_soft_invalid(unsigned long slot)
+{
+	return ((slot & 0xfUL) == 0xfUL);
+}
+
+unsigned long get_hidx_gslot(unsigned long vpn, unsigned long shift,
+		int ssize, real_pte_t rpte, unsigned int subpg_index);
+
 /* This low level function performs the actual PTE insertion
  * Setting the PTE depends on the MMU type and other factors. It's
  * an horrible mess that I'm not going to try to clean up now but

arch/powerpc/include/asm/book3s/64/mmu-hash.h +0 -0

index 6981a52..cfb8169 100644

arch/powerpc/include/asm/book3s/64/mmu-hash.h extern int __hash_page_4K(unsigned long ea, unsigned long access, · line 441

Quoted 7 lines

arch/powerpc/include/asm/book3s/64/mmu-hash.h +4 -0

@@ -435,6 +435,11 @@ extern int __hash_page_4K(unsigned long ea, unsigned long access,
 extern int __hash_page_64K(unsigned long ea, unsigned long access,
 			   unsigned long vsid, pte_t *ptep, unsigned long trap,
 			   unsigned long flags, int ssize);
+extern unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+			unsigned int subpg_index, unsigned long slot);
+extern unsigned long get_hidx_slot(unsigned long vpn, unsigned long shift,
+			int ssize, real_pte_t rpte, unsigned int subpg_index);

I wonder what purpose set_hidx_slot() defined previously, served.

Quoted 16 lines

arch/powerpc/include/asm/book3s/64/mmu-hash.h +1 -0

+
 struct mm_struct;
 unsigned int hash_page_do_lazy_icache(unsigned int pp, pte_t pte, int trap);
 extern int hash_page_mm(struct mm_struct *mm, unsigned long ea,

arch/powerpc/mm/dump_linuxpagetables.c +1 -1

index 44fe483..b832ed3 100644
@@ -213,7 +213,7 @@ struct flag_info {
 		.val	= H_PAGE_4K_PFN,
 		.set	= "4K_pfn",
 	}, {
-#endif
+#else
 		.mask	= H_PAGE_F_GIX,
 		.val	= H_PAGE_F_GIX,

Quoted 8 lines

arch/powerpc/mm/dump_linuxpagetables.c struct flag_info { · line 227

Quoted 6 lines

arch/powerpc/mm/dump_linuxpagetables.c +1 -0

 		.set	= "f_gix",

@@ -224,6 +224,7 @@ struct flag_info {
 		.val	= H_PAGE_F_SECOND,
 		.set	= "f_second",
 	}, {
+#endif /* CONFIG_PPC_64K_PAGES */

Are we adding H_PAGE_F_GIX as an element for 4K mapping ?

I think there is mistake here.
In the next patch when these bits are divorsed from 64K ptes entirely, we will not need the above code for 64K ptes. But good catch. Will fix the error in this patch.

Quoted 22 lines

Quoted 22 lines

arch/powerpc/mm/dump_linuxpagetables.c +0 -0

 #endif
 		.mask	= _PAGE_SPECIAL,
 		.val	= _PAGE_SPECIAL,

arch/powerpc/mm/hash64_4k.c +2 -0

index 6fa450c..c673829 100644
@@ -20,6 +20,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		   pte_t *ptep, unsigned long trap, unsigned long flags,
 		   int ssize, int subpg_prot)
 {
+	real_pte_t rpte;
 	unsigned long hpte_group;
 	unsigned long rflags, pa;
 	unsigned long old_pte, new_pte;

@@ -54,6 +55,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 	 * need to add in 0x1 if it's a read-only user page
 	 */
 	rflags = htab_convert_pte_flags(new_pte);
+	rpte = __real_pte(__pte(old_pte), ptep);
 

Quoted 16 lines

arch/powerpc/mm/hash64_4k.c int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid, · line 70

Quoted 13 lines

arch/powerpc/mm/hash64_4k.c +2 -5

 	if (cpu_has_feature(CPU_FTR_NOEXECUTE) &&
 	    !cpu_has_feature(CPU_FTR_COHERENT_ICACHE))

@@ -64,13 +66,10 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		/*
 		 * There MIGHT be an HPTE for this pte
 		 */
-		hash = hpt_hash(vpn, shift, ssize);
-		if (old_pte & H_PAGE_F_SECOND)
-			hash = ~hash;
-		slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
-		slot += (old_pte & H_PAGE_F_GIX) >> H_PAGE_F_GIX_SHIFT;
+		unsigned long gslot = get_hidx_gslot(vpn, shift,
+						ssize, rpte, 0);

I am wondering why there is a 'g' before the slot in all these functions.

Quoted 6 lines

Right. even i was confused initially. :)

hash table slots are originized as one big table. 8 consecutive entires in that table form a bucket. the term slot is used to refer to the slot within the bucket. the term gslot is used to refer to an entry in the table. roughly speaking slot 2 in bucket 2, will be gslot 2*8+2=18.

Global slot as it can point any where on that two dimensional table ?

Quoted 7 lines

Quoted 6 lines

Its already too much of changes in a single patch. Being a single logical change it needs to be inside a single change but then we need much more description in the commit message for some one to understand what all changed and how.

Quoted 3 lines

I have further broken down this patch, one to introduce get_hidx_gslot() one to introduce set_hidx_slot() . Hopefully that will reduce the size of the patch to graspable level. let me know,

I did some experiments with the first two patches.

* First of all the first patch does not compile without this.

arch/powerpc/mm/hash_utils_64.c +1 -1

@@ -1612,7 +1612,7 @@ unsigned long get_hidx_gslot(unsigned long vpn, unsigned long shift,
 void flush_hash_page(unsigned long vpn, real_pte_t pte, int psize, int ssize,
                     unsigned long flags)
 {
-       unsigned long hash, index, shift, hidx, gslot;
+       unsigned long index, shift, gslot;
        int local = flags & HPTE_LOCAL_UPDATE;
 
        DBG_LOW("flush_hash_page(vpn=%016lx)\n", vpn);

* Though it boots the kernel, system is kind of unresponsive while attempting to compile a kernel. Though I did not dig further on this, seems like the first patch is not self sufficient yet.

* With both first and second patch, the kernel boots fine and compiles a kernel.

We need to sort out issues in the first two patches before looking into the rest of the patch series.

Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6 in the 4K backed hpte pages. These bits continue to be used for 64K backed hpte pages in this patch, but will be freed up in the next patch.

The patch does the following change to the 64K PTE format

H_PAGE_BUSY moves from bit 3 to bit 9 H_PAGE_F_SECOND which occupied bit 4 moves to the second part
of the pte.
H_PAGE_F_GIX which occupied bit 5, 6 and 7 also moves to the
second part of the pte.

the four bits((H_PAGE_F_SECOND|H_PAGE_F_GIX) that represent a slot is initialized to 0xF indicating an invalid slot. If a hpte
gets cached in a 0xF slot(i.e 7th slot of secondary), it is
released immediately. In other words, even though 0xF is a
valid slot we discard and consider it as an invalid slot;i.e hpte_soft_invalid(). This gives us an opportunity to not depend on a bit in the primary PTE in order to determine the validity of a slot.

When we release a hpte in the 0xF slot we also release a
legitimate primary slot and unmap that entry. This is to
ensure that we do get a legimate non-0xF slot the next time we
retry for a slot.

Though treating 0xF slot as invalid reduces the number of available slots and may have an effect on the performance, the probabilty of hitting a 0xF is extermely low.

Compared to the current scheme, the above described scheme reduces the number of false hash table updates significantly and has the added advantage of releasing four valuable PTE bits for other purpose.

This idea was jointly developed by Paul Mackerras, Aneesh, Michael Ellermen and myself.

4K PTE format remain unchanged currently.

Signed-off-by Ram Pai <[email protected]>

---
 arch/powerpc/include/asm/book3s/64/hash-4k.h  | 20 +++++++
 arch/powerpc/include/asm/book3s/64/hash-64k.h | 32 +++++++----
 arch/powerpc/include/asm/book3s/64/hash.h     | 15 +++--
 arch/powerpc/include/asm/book3s/64/mmu-hash.h |  5 ++
 arch/powerpc/mm/dump_linuxpagetables.c        |  3 +-
 arch/powerpc/mm/hash64_4k.c                   | 14 ++---
 arch/powerpc/mm/hash64_64k.c                  | 81 ++++++++++++---------------
 arch/powerpc/mm/hash_utils_64.c               | 30 +++++++---
 8 files changed, 122 insertions(+), 78 deletions(-)

arch/powerpc/include/asm/book3s/64/hash-4k.h +20 -0

index b4b5e6b..5ef1d81 100644
@@ -16,6 +16,18 @@
 #define H_PUD_TABLE_SIZE	(sizeof(pud_t) << H_PUD_INDEX_SIZE)
 #define H_PGD_TABLE_SIZE	(sizeof(pgd_t) << H_PGD_INDEX_SIZE)
 
+
+/*
+ * Only supported by 4k linux page size
+ */
+#define H_PAGE_F_SECOND        _RPAGE_RSV2     /* HPTE is in 2ndary HPTEG */
+#define H_PAGE_F_GIX           (_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
+#define H_PAGE_F_GIX_SHIFT     56
+
+#define H_PAGE_BUSY	_RPAGE_RSV1     /* software: PTE & hash are busy */
+#define H_PAGE_HASHPTE	_RPAGE_RPN43    /* PTE has associated HPTE */
+
+
 /* PTE flags to conserve for HPTE identification */
 #define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_HASHPTE | \
 			 H_PAGE_F_SECOND | H_PAGE_F_GIX)

@@ -48,6 +60,14 @@ static inline int hash__hugepd_ok(hugepd_t hpd)
 }
 #endif
 
+static inline unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+			unsigned int subpg_index, unsigned long slot)
+{
+	return (slot << H_PAGE_F_GIX_SHIFT) &
+		(H_PAGE_F_SECOND | H_PAGE_F_GIX);
+}
+
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
 static inline char *get_hpte_slot_array(pmd_t *pmdp)

arch/powerpc/include/asm/book3s/64/hash-64k.h +10 -2

index 9732837..0eb3c89 100644
@@ -10,23 +10,25 @@
  * 64k aligned address free up few of the lower bits of RPN for us
  * We steal that here. For more deatils look at pte_pfn/pfn_pte()
  */
-#define H_PAGE_COMBO	_RPAGE_RPN0 /* this is a combo 4k page */
-#define H_PAGE_4K_PFN	_RPAGE_RPN1 /* PFN is for a single 4k page */
+#define H_PAGE_COMBO   _RPAGE_RPN0 /* this is a combo 4k page */
+#define H_PAGE_4K_PFN  _RPAGE_RPN1 /* PFN is for a single 4k page */
+#define H_PAGE_F_SECOND	_RPAGE_RSV2	/* HPTE is in 2ndary HPTEG */
+#define H_PAGE_F_GIX	(_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
+#define H_PAGE_F_GIX_SHIFT	56
+
+
+#define H_PAGE_BUSY	_RPAGE_RPN42     /* software: PTE & hash are busy */
+#define H_PAGE_HASHPTE	_RPAGE_RPN43    /* PTE has associated HPTE */
+
 /*
  * We need to differentiate between explicit huge page and THP huge
  * page, since THP huge page also need to track real subpage details
  */
 #define H_PAGE_THP_HUGE  H_PAGE_4K_PFN
 

arch/powerpc/include/asm/book3s/64/hash-64k.h +11 -0

+
 /*
  * we support 16 fragments per PTE page of 64K size.
  */

@@ -74,6 +76,16 @@ static inline unsigned long __rpte_to_hidx(real_pte_t rpte, unsigned long index)
 	return (pte_val(rpte.pte) >> H_PAGE_F_GIX_SHIFT) & 0xf;
 }
 
+static inline unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+		unsigned int subpg_index, unsigned long slot)
+{
+	unsigned long *hidxp = (unsigned long *)(ptep + PTRS_PER_PTE);
+
+	rpte.hidx &= ~(0xfUL << (subpg_index << 2));
+	*hidxp = rpte.hidx  | (slot << (subpg_index << 2));
+	return 0x0UL;
+}
+
 #define __rpte_to_pte(r)	((r).pte)
 extern bool __rpte_sub_valid(real_pte_t rpte, unsigned long index);
 /*

arch/powerpc/include/asm/book3s/64/hash.h +10 -5

index 4e957b0..e7cf03a 100644
@@ -8,11 +8,8 @@
  *
  */
 #define H_PTE_NONE_MASK		_PAGE_HPTEFLAGS
-#define H_PAGE_F_GIX_SHIFT	56
-#define H_PAGE_BUSY		_RPAGE_RSV1 /* software: PTE & hash are busy */
-#define H_PAGE_F_SECOND		_RPAGE_RSV2	/* HPTE is in 2ndary HPTEG */
-#define H_PAGE_F_GIX		(_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
-#define H_PAGE_HASHPTE		_RPAGE_RPN43	/* PTE has associated HPTE */
+
+#define INIT_HIDX (~0x0UL)
 
 #ifdef CONFIG_PPC_64K_PAGES
 #include <asm/book3s/64/hash-64k.h>

@@ -160,6 +157,14 @@ static inline int hash__pte_none(pte_t pte)
 	return (pte_val(pte) & ~H_PTE_NONE_MASK) == 0;
 }
 
+static inline bool hpte_soft_invalid(unsigned long slot)
+{
+	return ((slot & 0xfUL) == 0xfUL);
+}
+
+unsigned long get_hidx_gslot(unsigned long vpn, unsigned long shift,
+		int ssize, real_pte_t rpte, unsigned int subpg_index);
+
 /* This low level function performs the actual PTE insertion
  * Setting the PTE depends on the MMU type and other factors. It's
  * an horrible mess that I'm not going to try to clean up now but

arch/powerpc/include/asm/book3s/64/mmu-hash.h +5 -0

index 6981a52..cfb8169 100644
@@ -435,6 +435,11 @@ extern int __hash_page_4K(unsigned long ea, unsigned long access,
 extern int __hash_page_64K(unsigned long ea, unsigned long access,
 			   unsigned long vsid, pte_t *ptep, unsigned long trap,
 			   unsigned long flags, int ssize);
+extern unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+			unsigned int subpg_index, unsigned long slot);
+extern unsigned long get_hidx_slot(unsigned long vpn, unsigned long shift,
+			int ssize, real_pte_t rpte, unsigned int subpg_index);
+
 struct mm_struct;
 unsigned int hash_page_do_lazy_icache(unsigned int pp, pte_t pte, int trap);
 extern int hash_page_mm(struct mm_struct *mm, unsigned long ea,

arch/powerpc/mm/dump_linuxpagetables.c +2 -1

index 44fe483..b832ed3 100644
@@ -213,7 +213,7 @@ struct flag_info {
 		.val	= H_PAGE_4K_PFN,
 		.set	= "4K_pfn",
 	}, {
-#endif
+#else
 		.mask	= H_PAGE_F_GIX,
 		.val	= H_PAGE_F_GIX,
 		.set	= "f_gix",

@@ -224,6 +224,7 @@ struct flag_info {
 		.val	= H_PAGE_F_SECOND,
 		.set	= "f_second",
 	}, {
+#endif /* CONFIG_PPC_64K_PAGES */
 #endif
 		.mask	= _PAGE_SPECIAL,
 		.val	= _PAGE_SPECIAL,

arch/powerpc/mm/hash64_4k.c +5 -6

index 6fa450c..c673829 100644
@@ -20,6 +20,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		   pte_t *ptep, unsigned long trap, unsigned long flags,
 		   int ssize, int subpg_prot)
 {
+	real_pte_t rpte;
 	unsigned long hpte_group;
 	unsigned long rflags, pa;
 	unsigned long old_pte, new_pte;

@@ -54,6 +55,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 	 * need to add in 0x1 if it's a read-only user page
 	 */
 	rflags = htab_convert_pte_flags(new_pte);
+	rpte = __real_pte(__pte(old_pte), ptep);
 
 	if (cpu_has_feature(CPU_FTR_NOEXECUTE) &&
 	    !cpu_has_feature(CPU_FTR_COHERENT_ICACHE))

@@ -64,13 +66,10 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		/*
 		 * There MIGHT be an HPTE for this pte
 		 */
-		hash = hpt_hash(vpn, shift, ssize);
-		if (old_pte & H_PAGE_F_SECOND)
-			hash = ~hash;
-		slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
-		slot += (old_pte & H_PAGE_F_GIX) >> H_PAGE_F_GIX_SHIFT;
+		unsigned long gslot = get_hidx_gslot(vpn, shift,
+						ssize, rpte, 0);
 
-		if (mmu_hash_ops.hpte_updatepp(slot, rflags, vpn, MMU_PAGE_4K,
+		if (mmu_hash_ops.hpte_updatepp(gslot, rflags, vpn, MMU_PAGE_4K,
 					       MMU_PAGE_4K, ssize, flags) == -1)
 			old_pte &= ~_PAGE_HPTEFLAGS;
 	}

arch/powerpc/mm/hash64_64k.c +5 -27

index 1a68cb1..3702a3c 100644
@@ -15,34 +15,13 @@
 #include <linux/mm.h>
 #include <asm/machdep.h>
 #include <asm/mmu.h>
+
 /*
  * index from 0 - 15
  */
 bool __rpte_sub_valid(real_pte_t rpte, unsigned long index)
 {
-	unsigned long g_idx;
-	unsigned long ptev = pte_val(rpte.pte);
-
-	g_idx = (ptev & H_PAGE_COMBO_VALID) >> H_PAGE_F_GIX_SHIFT;
-	index = index >> 2;
-	if (g_idx & (0x1 << index))
-		return true;
-	else
-		return false;
-}
-/*
- * index from 0 - 15
- */
-static unsigned long mark_subptegroup_valid(unsigned long ptev, unsigned long index)
-{
-	unsigned long g_idx;
-
-	if (!(ptev & H_PAGE_COMBO))
-		return ptev;
-	index = index >> 2;
-	g_idx = 0x1 << index;
-
-	return ptev | (g_idx << H_PAGE_F_GIX_SHIFT);
+	return !(hpte_soft_invalid(rpte.hidx >> (index << 2)));
 }
 
 int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,

@@ -50,10 +29,9 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		   int ssize, int subpg_prot)
 {
 	real_pte_t rpte;
-	unsigned long *hidxp;
 	unsigned long hpte_group;
 	unsigned int subpg_index;
-	unsigned long rflags, pa, hidx;
+	unsigned long rflags, pa;
 	unsigned long old_pte, new_pte, subpg_pte;
 	unsigned long vpn, hash, slot;
 	unsigned long shift = mmu_psize_defs[MMU_PAGE_4K].shift;

@@ -116,28 +94,23 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		 * On hash insert failure we use old pte value and we don't
 		 * want slot information there if we have a insert failure.
 		 */
-		old_pte &= ~(H_PAGE_HASHPTE | H_PAGE_F_GIX | H_PAGE_F_SECOND);
-		new_pte &= ~(H_PAGE_HASHPTE | H_PAGE_F_GIX | H_PAGE_F_SECOND);
+		old_pte &= ~(H_PAGE_HASHPTE);
+		new_pte &= ~(H_PAGE_HASHPTE);
 		goto htab_insert_hpte;
 	}
 	/*
 	 * Check for sub page valid and update
 	 */
 	if (__rpte_sub_valid(rpte, subpg_index)) {

arch/powerpc/mm/hash64_64k.c +11 -2

-		ret = mmu_hash_ops.hpte_updatepp(slot, rflags, vpn,
+		int ret = mmu_hash_ops.hpte_updatepp(gslot, rflags, vpn,
 						 MMU_PAGE_4K, MMU_PAGE_4K,
 						 ssize, flags);
 		/*
-		 *if we failed because typically the HPTE wasn't really here
+		 * if we failed because typically the HPTE wasn't really here
 		 * we try an insertion.
 		 */
 		if (ret == -1)

@@ -148,6 +121,15 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 	}
 
 htab_insert_hpte:
+
+	/*
+	 * initialize all hidx entries to a invalid value,
+	 * the first time the PTE is about to allocate
+	 * a 4K hpte
+	 */
+	if (!(old_pte & H_PAGE_COMBO))
+		rpte.hidx = INIT_HIDX;
+
 	/*
 	 * handle H_PAGE_4K_PFN case
 	 */

arch/powerpc/mm/hash64_64k.c +5 -5

+			if (hpte_soft_invalid(slot) || (mftb() & 0x1))
 				hpte_group = ((hash & htab_hash_mask) *
 					      HPTES_PER_GROUP) & ~0x7UL;
+
 			mmu_hash_ops.hpte_remove(hpte_group);
 			/*
 			 * FIXME!! Should be try the group from which we removed ?

@@ -204,11 +196,9 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 	 * Since we have H_PAGE_BUSY set on ptep, we can be sure
 	 * nobody is undating hidx.
 	 */
-	hidxp = (unsigned long *)(ptep + PTRS_PER_PTE);
-	rpte.hidx &= ~(0xfUL << (subpg_index << 2));
-	*hidxp = rpte.hidx  | (slot << (subpg_index << 2));
-	new_pte = mark_subptegroup_valid(new_pte, subpg_index);
-	new_pte |=  H_PAGE_HASHPTE;
+	new_pte |= set_hidx_slot(ptep, rpte, subpg_index, slot);
+	new_pte |= H_PAGE_HASHPTE;
+
 	/*
 	 * check __real_pte for details on matching smp_rmb()
 	 */

arch/powerpc/mm/hash64_64k.c +0 -0

 	}
 	*ptep = __pte(new_pte & ~H_PAGE_BUSY);
 	return 0;

arch/powerpc/mm/hash_utils_64.c +6 -1

index f2095ce..c0f4b46 100644
@@ -975,8 +975,9 @@ void __init hash__early_init_devtree(void)
 
 void __init hash__early_init_mmu(void)
 {
+#ifndef CONFIG_PPC_64K_PAGES
 	/*
-	 * We have code in __hash_page_64K() and elsewhere, which assumes it can
+	 * We have code in __hash_page_4K() and elsewhere, which assumes it can
 	 * do the following:
 	 *   new_pte |= (slot << H_PAGE_F_GIX_SHIFT) & (H_PAGE_F_SECOND | H_PAGE_F_GIX);
 	 *

@@ -987,6 +988,7 @@ void __init hash__early_init_mmu(void)
 	 * with a BUILD_BUG_ON().
 	 */
 	BUILD_BUG_ON(H_PAGE_F_SECOND != (1ul  << (H_PAGE_F_GIX_SHIFT + 3)));
+#endif /* CONFIG_PPC_64K_PAGES */
 
 	htab_init_page_sizes();
 

@@ -1589,29 +1591,39 @@ static inline void tm_flush_hash_page(int local)
 }
 #endif
 
+unsigned long get_hidx_gslot(unsigned long vpn, unsigned long shift,
+			int ssize, real_pte_t rpte, unsigned int subpg_index)
+{

arch/powerpc/mm/hash_utils_64.c +4 -7

+
+
 /* WARNING: This is called from hash_low_64.S, if you change this prototype,
  *          do not forget to update the assembly call site !
  */
 void flush_hash_page(unsigned long vpn, real_pte_t pte, int psize, int ssize,
 		     unsigned long flags)
 {
-	unsigned long hash, index, shift, hidx, slot;
+	unsigned long hash, index, shift, hidx, gslot;
 	int local = flags & HPTE_LOCAL_UPDATE;
 
 	DBG_LOW("flush_hash_page(vpn=%016lx)\n", vpn);
 	pte_iterate_hashed_subpages(pte, psize, vpn, index, shift) {
-		hash = hpt_hash(vpn, shift, ssize);
-		hidx = __rpte_to_hidx(pte, index);
-		if (hidx & _PTEIDX_SECONDARY)
-			hash = ~hash;
-		slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
-		slot += hidx & _PTEIDX_GROUP_IX;
+		gslot = get_hidx_gslot(vpn, shift, ssize, pte, index);

Quoted 117 lines · Ram Pai

Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6 in the 4K backed hpte pages. These bits continue to be used for 64K backed hpte pages in this patch, but will be freed up in the next patch.

The patch does the following change to the 64K PTE format

H_PAGE_BUSY moves from bit 3 to bit 9 H_PAGE_F_SECOND which occupied bit 4 moves to the second part
of the pte.
H_PAGE_F_GIX which occupied bit 5, 6 and 7 also moves to the
second part of the pte.

the four bits((H_PAGE_F_SECOND|H_PAGE_F_GIX) that represent a slot is initialized to 0xF indicating an invalid slot. If a hpte
gets cached in a 0xF slot(i.e 7th slot of secondary), it is
released immediately. In other words, even though 0xF is a
valid slot we discard and consider it as an invalid slot;i.e hpte_soft_invalid(). This gives us an opportunity to not depend on a bit in the primary PTE in order to determine the validity of a slot.

When we release a hpte in the 0xF slot we also release a
legitimate primary slot and unmap that entry. This is to
ensure that we do get a legimate non-0xF slot the next time we
retry for a slot.

Though treating 0xF slot as invalid reduces the number of available slots and may have an effect on the performance, the probabilty of hitting a 0xF is extermely low.

Compared to the current scheme, the above described scheme reduces the number of false hash table updates significantly and has the added advantage of releasing four valuable PTE bits for other purpose.

This idea was jointly developed by Paul Mackerras, Aneesh, Michael Ellermen and myself.

4K PTE format remain unchanged currently.

Signed-off-by Ram Pai <[email protected]>

---
 arch/powerpc/include/asm/book3s/64/hash-4k.h  | 20 +++++++
 arch/powerpc/include/asm/book3s/64/hash-64k.h | 32 +++++++----
 arch/powerpc/include/asm/book3s/64/hash.h     | 15 +++--
 arch/powerpc/include/asm/book3s/64/mmu-hash.h |  5 ++
 arch/powerpc/mm/dump_linuxpagetables.c        |  3 +-
 arch/powerpc/mm/hash64_4k.c                   | 14 ++---
 arch/powerpc/mm/hash64_64k.c                  | 81 ++++++++++++---------------
 arch/powerpc/mm/hash_utils_64.c               | 30 +++++++---
 8 files changed, 122 insertions(+), 78 deletions(-)

arch/powerpc/include/asm/book3s/64/hash-4k.h +20 -0

index b4b5e6b..5ef1d81 100644
@@ -16,6 +16,18 @@
 #define H_PUD_TABLE_SIZE	(sizeof(pud_t) << H_PUD_INDEX_SIZE)
 #define H_PGD_TABLE_SIZE	(sizeof(pgd_t) << H_PGD_INDEX_SIZE)
 
+
+/*
+ * Only supported by 4k linux page size
+ */
+#define H_PAGE_F_SECOND        _RPAGE_RSV2     /* HPTE is in 2ndary HPTEG */
+#define H_PAGE_F_GIX           (_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
+#define H_PAGE_F_GIX_SHIFT     56
+
+#define H_PAGE_BUSY	_RPAGE_RSV1     /* software: PTE & hash are busy */
+#define H_PAGE_HASHPTE	_RPAGE_RPN43    /* PTE has associated HPTE */
+
+
 /* PTE flags to conserve for HPTE identification */
 #define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_HASHPTE | \
 			 H_PAGE_F_SECOND | H_PAGE_F_GIX)

@@ -48,6 +60,14 @@ static inline int hash__hugepd_ok(hugepd_t hpd)
 }
 #endif
 
+static inline unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+			unsigned int subpg_index, unsigned long slot)
+{
+	return (slot << H_PAGE_F_GIX_SHIFT) &
+		(H_PAGE_F_SECOND | H_PAGE_F_GIX);
+}
+
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
 static inline char *get_hpte_slot_array(pmd_t *pmdp)

arch/powerpc/include/asm/book3s/64/hash-64k.h +10 -2

index 9732837..0eb3c89 100644
@@ -10,23 +10,25 @@
  * 64k aligned address free up few of the lower bits of RPN for us
  * We steal that here. For more deatils look at pte_pfn/pfn_pte()
  */
-#define H_PAGE_COMBO	_RPAGE_RPN0 /* this is a combo 4k page */
-#define H_PAGE_4K_PFN	_RPAGE_RPN1 /* PFN is for a single 4k page */
+#define H_PAGE_COMBO   _RPAGE_RPN0 /* this is a combo 4k page */
+#define H_PAGE_4K_PFN  _RPAGE_RPN1 /* PFN is for a single 4k page */
+#define H_PAGE_F_SECOND	_RPAGE_RSV2	/* HPTE is in 2ndary HPTEG */
+#define H_PAGE_F_GIX	(_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
+#define H_PAGE_F_GIX_SHIFT	56
+
+
+#define H_PAGE_BUSY	_RPAGE_RPN42     /* software: PTE & hash are busy */
+#define H_PAGE_HASHPTE	_RPAGE_RPN43    /* PTE has associated HPTE */
+
 /*
  * We need to differentiate between explicit huge page and THP huge
  * page, since THP huge page also need to track real subpage details
  */
 #define H_PAGE_THP_HUGE  H_PAGE_4K_PFN
 

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

Quoted 10 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -8

-/*
- * Used to track subpage group valid if H_PAGE_COMBO is set
- * This overloads H_PAGE_F_GIX and H_PAGE_F_SECOND
- */
-#define H_PAGE_COMBO_VALID	(H_PAGE_F_GIX | H_PAGE_F_SECOND)
-
 /* PTE flags to conserve for HPTE identification */
-#define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_F_SECOND | \
-			 H_PAGE_F_GIX | H_PAGE_HASHPTE | H_PAGE_COMBO)
+#define _PAGE_HPTEFLAGS (H_PAGE_BUSY | H_PAGE_HASHPTE | H_PAGE_COMBO)

Why in this patch ? This is related to 64K pte

Quoted 136 lines

arch/powerpc/include/asm/book3s/64/hash-64k.h +11 -0

+
 /*
  * we support 16 fragments per PTE page of 64K size.
  */

@@ -74,6 +76,16 @@ static inline unsigned long __rpte_to_hidx(real_pte_t rpte, unsigned long index)
 	return (pte_val(rpte.pte) >> H_PAGE_F_GIX_SHIFT) & 0xf;
 }
 
+static inline unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+		unsigned int subpg_index, unsigned long slot)
+{
+	unsigned long *hidxp = (unsigned long *)(ptep + PTRS_PER_PTE);
+
+	rpte.hidx &= ~(0xfUL << (subpg_index << 2));
+	*hidxp = rpte.hidx  | (slot << (subpg_index << 2));
+	return 0x0UL;
+}
+
 #define __rpte_to_pte(r)	((r).pte)
 extern bool __rpte_sub_valid(real_pte_t rpte, unsigned long index);
 /*

arch/powerpc/include/asm/book3s/64/hash.h +10 -5

index 4e957b0..e7cf03a 100644
@@ -8,11 +8,8 @@
  *
  */
 #define H_PTE_NONE_MASK		_PAGE_HPTEFLAGS
-#define H_PAGE_F_GIX_SHIFT	56
-#define H_PAGE_BUSY		_RPAGE_RSV1 /* software: PTE & hash are busy */
-#define H_PAGE_F_SECOND		_RPAGE_RSV2	/* HPTE is in 2ndary HPTEG */
-#define H_PAGE_F_GIX		(_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
-#define H_PAGE_HASHPTE		_RPAGE_RPN43	/* PTE has associated HPTE */
+
+#define INIT_HIDX (~0x0UL)
 
 #ifdef CONFIG_PPC_64K_PAGES
 #include <asm/book3s/64/hash-64k.h>

@@ -160,6 +157,14 @@ static inline int hash__pte_none(pte_t pte)
 	return (pte_val(pte) & ~H_PTE_NONE_MASK) == 0;
 }
 
+static inline bool hpte_soft_invalid(unsigned long slot)
+{
+	return ((slot & 0xfUL) == 0xfUL);
+}
+
+unsigned long get_hidx_gslot(unsigned long vpn, unsigned long shift,
+		int ssize, real_pte_t rpte, unsigned int subpg_index);
+
 /* This low level function performs the actual PTE insertion
  * Setting the PTE depends on the MMU type and other factors. It's
  * an horrible mess that I'm not going to try to clean up now but

arch/powerpc/include/asm/book3s/64/mmu-hash.h +5 -0

index 6981a52..cfb8169 100644
@@ -435,6 +435,11 @@ extern int __hash_page_4K(unsigned long ea, unsigned long access,
 extern int __hash_page_64K(unsigned long ea, unsigned long access,
 			   unsigned long vsid, pte_t *ptep, unsigned long trap,
 			   unsigned long flags, int ssize);
+extern unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,
+			unsigned int subpg_index, unsigned long slot);
+extern unsigned long get_hidx_slot(unsigned long vpn, unsigned long shift,
+			int ssize, real_pte_t rpte, unsigned int subpg_index);
+
 struct mm_struct;
 unsigned int hash_page_do_lazy_icache(unsigned int pp, pte_t pte, int trap);
 extern int hash_page_mm(struct mm_struct *mm, unsigned long ea,

arch/powerpc/mm/dump_linuxpagetables.c +2 -1

index 44fe483..b832ed3 100644
@@ -213,7 +213,7 @@ struct flag_info {
 		.val	= H_PAGE_4K_PFN,
 		.set	= "4K_pfn",
 	}, {
-#endif
+#else
 		.mask	= H_PAGE_F_GIX,
 		.val	= H_PAGE_F_GIX,
 		.set	= "f_gix",

@@ -224,6 +224,7 @@ struct flag_info {
 		.val	= H_PAGE_F_SECOND,
 		.set	= "f_second",
 	}, {
+#endif /* CONFIG_PPC_64K_PAGES */
 #endif
 		.mask	= _PAGE_SPECIAL,
 		.val	= _PAGE_SPECIAL,

arch/powerpc/mm/hash64_4k.c +6 -8

index 6fa450c..c673829 100644
@@ -20,6 +20,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		   pte_t *ptep, unsigned long trap, unsigned long flags,
 		   int ssize, int subpg_prot)
 {
+	real_pte_t rpte;
 	unsigned long hpte_group;
 	unsigned long rflags, pa;
 	unsigned long old_pte, new_pte;

@@ -54,6 +55,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 	 * need to add in 0x1 if it's a read-only user page
 	 */
 	rflags = htab_convert_pte_flags(new_pte);
+	rpte = __real_pte(__pte(old_pte), ptep);
 
 	if (cpu_has_feature(CPU_FTR_NOEXECUTE) &&
 	    !cpu_has_feature(CPU_FTR_COHERENT_ICACHE))

@@ -64,13 +66,10 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		/*
 		 * There MIGHT be an HPTE for this pte
 		 */
-		hash = hpt_hash(vpn, shift, ssize);
-		if (old_pte & H_PAGE_F_SECOND)
-			hash = ~hash;
-		slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
-		slot += (old_pte & H_PAGE_F_GIX) >> H_PAGE_F_GIX_SHIFT;
+		unsigned long gslot = get_hidx_gslot(vpn, shift,
+						ssize, rpte, 0);
 
-		if (mmu_hash_ops.hpte_updatepp(slot, rflags, vpn, MMU_PAGE_4K,
+		if (mmu_hash_ops.hpte_updatepp(gslot, rflags, vpn, MMU_PAGE_4K,
 					       MMU_PAGE_4K, ssize, flags) == -1)
 			old_pte &= ~_PAGE_HPTEFLAGS;
 	}

@@ -118,8 +117,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 			return -1;
 		}
 		new_pte = (new_pte & ~_PAGE_HPTEFLAGS) | H_PAGE_HASHPTE;
-		new_pte |= (slot << H_PAGE_F_GIX_SHIFT) &
-			(H_PAGE_F_SECOND | H_PAGE_F_GIX);
+		new_pte |= set_hidx_slot(ptep, rpte, 0, slot);

arch/powerpc/mm/hash64_4k.c int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid, · line 123

Quoted 3 lines

arch/powerpc/mm/hash64_4k.c +0 -0

 	}
 	*ptep = __pte(new_pte & ~H_PAGE_BUSY);
 	return 0;

None of the above changes are needed. We are not changing anything w.r.t 4k linux page table yet. So we can drop this.

Quoted 70 lines

arch/powerpc/mm/hash64_64k.c +5 -28

index 1a68cb1..3702a3c 100644
@@ -15,34 +15,13 @@
 #include <linux/mm.h>
 #include <asm/machdep.h>
 #include <asm/mmu.h>
+
 /*
  * index from 0 - 15
  */
 bool __rpte_sub_valid(real_pte_t rpte, unsigned long index)
 {
-	unsigned long g_idx;
-	unsigned long ptev = pte_val(rpte.pte);
-
-	g_idx = (ptev & H_PAGE_COMBO_VALID) >> H_PAGE_F_GIX_SHIFT;
-	index = index >> 2;
-	if (g_idx & (0x1 << index))
-		return true;
-	else
-		return false;
-}
-/*
- * index from 0 - 15
- */
-static unsigned long mark_subptegroup_valid(unsigned long ptev, unsigned long index)
-{
-	unsigned long g_idx;
-
-	if (!(ptev & H_PAGE_COMBO))
-		return ptev;
-	index = index >> 2;
-	g_idx = 0x1 << index;
-
-	return ptev | (g_idx << H_PAGE_F_GIX_SHIFT);
+	return !(hpte_soft_invalid(rpte.hidx >> (index << 2)));
 }
 
 int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,

@@ -50,10 +29,9 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		   int ssize, int subpg_prot)
 {
 	real_pte_t rpte;
-	unsigned long *hidxp;
 	unsigned long hpte_group;
 	unsigned int subpg_index;
-	unsigned long rflags, pa, hidx;
+	unsigned long rflags, pa;
 	unsigned long old_pte, new_pte, subpg_pte;
 	unsigned long vpn, hash, slot;
 	unsigned long shift = mmu_psize_defs[MMU_PAGE_4K].shift;

@@ -116,28 +94,23 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		 * On hash insert failure we use old pte value and we don't
 		 * want slot information there if we have a insert failure.
 		 */
-		old_pte &= ~(H_PAGE_HASHPTE | H_PAGE_F_GIX | H_PAGE_F_SECOND);
-		new_pte &= ~(H_PAGE_HASHPTE | H_PAGE_F_GIX | H_PAGE_F_SECOND);
+		old_pte &= ~(H_PAGE_HASHPTE);
+		new_pte &= ~(H_PAGE_HASHPTE);
 		goto htab_insert_hpte;
 	}
 	/*
 	 * Check for sub page valid and update
 	 */
 	if (__rpte_sub_valid(rpte, subpg_index)) {
-		int ret;
 

arch/powerpc/mm/hash64_64k.c int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid, · line 107

Quoted 8 lines

arch/powerpc/mm/hash64_64k.c +2 -6

-		hash = hpt_hash(vpn, shift, ssize);
-		hidx = __rpte_to_hidx(rpte, subpg_index);
-		if (hidx & _PTEIDX_SECONDARY)
-			hash = ~hash;
-		slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
-		slot += hidx & _PTEIDX_GROUP_IX;
+		unsigned long gslot = get_hidx_gslot(vpn, shift,
+				ssize, rpte, subpg_index);

Converting that to helper is also not needed in this patch. Leave it as it is. It is much easier to review.

Quoted 30 lines

arch/powerpc/mm/hash64_64k.c +11 -2

-		ret = mmu_hash_ops.hpte_updatepp(slot, rflags, vpn,
+		int ret = mmu_hash_ops.hpte_updatepp(gslot, rflags, vpn,
 						 MMU_PAGE_4K, MMU_PAGE_4K,
 						 ssize, flags);
 		/*
-		 *if we failed because typically the HPTE wasn't really here
+		 * if we failed because typically the HPTE wasn't really here
 		 * we try an insertion.
 		 */
 		if (ret == -1)

@@ -148,6 +121,15 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 	}
 
 htab_insert_hpte:
+
+	/*
+	 * initialize all hidx entries to a invalid value,
+	 * the first time the PTE is about to allocate
+	 * a 4K hpte
+	 */
+	if (!(old_pte & H_PAGE_COMBO))
+		rpte.hidx = INIT_HIDX;
+
 	/*
 	 * handle H_PAGE_4K_PFN case
 	 */

@@ -177,10 +159,20 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 						rflags, HPTE_V_SECONDARY,

arch/powerpc/mm/hash64_64k.c int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid, · line 163

Quoted 6 lines

arch/powerpc/mm/hash64_64k.c +2 -2

 						MMU_PAGE_4K, MMU_PAGE_4K,
 						ssize);
-		if (slot == -1) {
-			if (mftb() & 0x1)
+
+		if (unlikely(hpte_soft_invalid(slot))) {

Should we name that hpte_slot_invalid() ? ie. s/soft/slot/ ?

arch/powerpc/mm/hash64_64k.c int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid, · line 167

Quoted 6 lines

arch/powerpc/mm/hash64_64k.c +4 -0

+			slot = slot & _PTEIDX_GROUP_IX;
+			mmu_hash_ops.hpte_invalidate(hpte_group+slot, vpn,
+				MMU_PAGE_4K, MMU_PAGE_4K,
+				ssize, flags);

What is the last arg flags here ? I guess we need to pass 0 there ? We can't do a local = 1 invalidate, because we don't know whether anybody did really access this address in between and has got the entry in TLB.

arch/powerpc/mm/hash64_64k.c int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid, · line 171

Quoted 6 lines

arch/powerpc/mm/hash64_64k.c +4 -0

+		}
+
+		if (unlikely(slot == -1 || hpte_soft_invalid(slot))) {
+

Can you add a comment around explaining invalid slot always result in removing from primary ? Also do we want to store that invalid slot details in a variable ? instead of doing that conditional again and again ? This is hotpath.

Quoted 26 lines

arch/powerpc/mm/hash64_64k.c +5 -5

+			if (hpte_soft_invalid(slot) || (mftb() & 0x1))
 				hpte_group = ((hash & htab_hash_mask) *
 					      HPTES_PER_GROUP) & ~0x7UL;
+
 			mmu_hash_ops.hpte_remove(hpte_group);
 			/*
 			 * FIXME!! Should be try the group from which we removed ?

@@ -204,11 +196,9 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 	 * Since we have H_PAGE_BUSY set on ptep, we can be sure
 	 * nobody is undating hidx.
 	 */
-	hidxp = (unsigned long *)(ptep + PTRS_PER_PTE);
-	rpte.hidx &= ~(0xfUL << (subpg_index << 2));
-	*hidxp = rpte.hidx  | (slot << (subpg_index << 2));
-	new_pte = mark_subptegroup_valid(new_pte, subpg_index);
-	new_pte |=  H_PAGE_HASHPTE;
+	new_pte |= set_hidx_slot(ptep, rpte, subpg_index, slot);
+	new_pte |= H_PAGE_HASHPTE;
+
 	/*
 	 * check __real_pte for details on matching smp_rmb()
 	 */

@@ -322,9 +312,10 @@ int __hash_page_64K(unsigned long ea, unsigned long access,
 					   MMU_PAGE_64K, MMU_PAGE_64K, old_pte);
 			return -1;
 		}

arch/powerpc/mm/hash64_64k.c int __hash_page_64K(unsigned long ea, unsigned long access, · line 318

Quoted 6 lines

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

-		new_pte = (new_pte & ~_PAGE_HPTEFLAGS) | H_PAGE_HASHPTE;
+
 		new_pte |= (slot << H_PAGE_F_GIX_SHIFT) &
-			(H_PAGE_F_SECOND | H_PAGE_F_GIX);
+				(H_PAGE_F_SECOND | H_PAGE_F_GIX);
+		new_pte = (new_pte & ~_PAGE_HPTEFLAGS) | H_PAGE_HASHPTE;

What is this change ? i guess we want this in second patch ?

Quoted 32 lines

arch/powerpc/mm/hash64_64k.c +0 -0

 	}
 	*ptep = __pte(new_pte & ~H_PAGE_BUSY);
 	return 0;

arch/powerpc/mm/hash_utils_64.c +3 -1

index f2095ce..c0f4b46 100644
@@ -975,8 +975,9 @@ void __init hash__early_init_devtree(void)
 
 void __init hash__early_init_mmu(void)
 {
+#ifndef CONFIG_PPC_64K_PAGES
 	/*
-	 * We have code in __hash_page_64K() and elsewhere, which assumes it can
+	 * We have code in __hash_page_4K() and elsewhere, which assumes it can
 	 * do the following:
 	 *   new_pte |= (slot << H_PAGE_F_GIX_SHIFT) & (H_PAGE_F_SECOND | H_PAGE_F_GIX);
 	 *

@@ -987,6 +988,7 @@ void __init hash__early_init_mmu(void)
 	 * with a BUILD_BUG_ON().
 	 */
 	BUILD_BUG_ON(H_PAGE_F_SECOND != (1ul  << (H_PAGE_F_GIX_SHIFT + 3)));
+#endif /* CONFIG_PPC_64K_PAGES */
 
 	htab_init_page_sizes();
 

@@ -1589,29 +1591,39 @@ static inline void tm_flush_hash_page(int local)
 }
 #endif
 

arch/powerpc/mm/hash_utils_64.c static inline void tm_flush_hash_page(int local) · line 1606

Quoted 13 lines

arch/powerpc/mm/hash_utils_64.c +13 -0

+unsigned long get_hidx_gslot(unsigned long vpn, unsigned long shift,
+			int ssize, real_pte_t rpte, unsigned int subpg_index)
+{
+	unsigned long hash, slot, hidx;
+
+	hash = hpt_hash(vpn, shift, ssize);
+	hidx = __rpte_to_hidx(rpte, subpg_index);
+	if (hidx & _PTEIDX_SECONDARY)
+		hash = ~hash;
+	slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
+	slot += hidx & _PTEIDX_GROUP_IX;
+	return slot;
+}

We don't need this helper for this patch series ?

Quoted 21 lines

arch/powerpc/mm/hash_utils_64.c +4 -7

+
+
 /* WARNING: This is called from hash_low_64.S, if you change this prototype,
  *          do not forget to update the assembly call site !
  */
 void flush_hash_page(unsigned long vpn, real_pte_t pte, int psize, int ssize,
 		     unsigned long flags)
 {
-	unsigned long hash, index, shift, hidx, slot;
+	unsigned long hash, index, shift, hidx, gslot;
 	int local = flags & HPTE_LOCAL_UPDATE;
 
 	DBG_LOW("flush_hash_page(vpn=%016lx)\n", vpn);
 	pte_iterate_hashed_subpages(pte, psize, vpn, index, shift) {
-		hash = hpt_hash(vpn, shift, ssize);
-		hidx = __rpte_to_hidx(pte, index);
-		if (hidx & _PTEIDX_SECONDARY)
-			hash = ~hash;
-		slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
-		slot += hidx & _PTEIDX_GROUP_IX;
+		gslot = get_hidx_gslot(vpn, shift, ssize, pte, index);

arch/powerpc/mm/hash_utils_64.c static inline void tm_flush_hash_page(int local) · line 1629

Quoted 10 lines

arch/powerpc/mm/hash_utils_64.c +1 -1

 		DBG_LOW(" sub %ld: hash=%lx, hidx=%lx\n", index, slot, hidx);
 		/*
 		 * We use same base page size and actual psize, because we don't
 		 * use these functions for hugepage
 		 */
-		mmu_hash_ops.hpte_invalidate(slot, vpn, psize, psize,
+		mmu_hash_ops.hpte_invalidate(gslot, vpn, psize, psize,
 					     ssize, local);
 	} pte_iterate_hashed_end();
 

And if we avoid adding that helper, changes like this can be avoided in the patch.

-aneesh

Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6 in the 4K backed hpte pages. These bits continue to be used for 64K backed hpte pages in this patch, but will be freed up in the next patch.

The patch does the following change to the 64K PTE format

H_PAGE_BUSY moves from bit 3 to bit 9 H_PAGE_F_SECOND which occupied bit 4 moves to the second part
of the pte.
H_PAGE_F_GIX which occupied bit 5, 6 and 7 also moves to the
second part of the pte.

the four bits((H_PAGE_F_SECOND|H_PAGE_F_GIX) that represent a slot is initialized to 0xF indicating an invalid slot. If a hpte
gets cached in a 0xF slot(i.e 7th slot of secondary), it is
released immediately. In other words, even though 0xF is a
valid slot we discard and consider it as an invalid slot;i.e hpte_soft_invalid(). This gives us an opportunity to not depend on a bit in the primary PTE in order to determine the validity of a slot.

When we release a hpte in the 0xF slot we also release a
legitimate primary slot and unmap that entry. This is to
ensure that we do get a legimate non-0xF slot the next time we
retry for a slot.

Though treating 0xF slot as invalid reduces the number of available slots and may have an effect on the performance, the probabilty of hitting a 0xF is extermely low.

Compared to the current scheme, the above described scheme reduces the number of false hash table updates significantly and has the added advantage of releasing four valuable PTE bits for other purpose.

This idea was jointly developed by Paul Mackerras, Aneesh, Michael Ellermen and myself.

4K PTE format remain unchanged currently.

Quoted 36 lines · Ram Pai

Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6 in the 4K backed hpte pages. These bits continue to be used for 64K backed hpte pages in this patch, but will be freed up in the next patch.

The patch does the following change to the 64K PTE format

H_PAGE_BUSY moves from bit 3 to bit 9 H_PAGE_F_SECOND which occupied bit 4 moves to the second part
of the pte.
H_PAGE_F_GIX which occupied bit 5, 6 and 7 also moves to the
second part of the pte.

the four bits((H_PAGE_F_SECOND|H_PAGE_F_GIX) that represent a slot is initialized to 0xF indicating an invalid slot. If a hpte
gets cached in a 0xF slot(i.e 7th slot of secondary), it is
released immediately. In other words, even though 0xF is a
valid slot we discard and consider it as an invalid slot;i.e hpte_soft_invalid(). This gives us an opportunity to not depend on a bit in the primary PTE in order to determine the validity of a slot.

When we release a hpte in the 0xF slot we also release a
legitimate primary slot and unmap that entry. This is to
ensure that we do get a legimate non-0xF slot the next time we
retry for a slot.

Though treating 0xF slot as invalid reduces the number of available slots and may have an effect on the performance, the probabilty of hitting a 0xF is extermely low.

Compared to the current scheme, the above described scheme reduces the number of false hash table updates significantly and has the added advantage of releasing four valuable PTE bits for other purpose.

Quoted 4 lines

This idea was jointly developed by Paul Mackerras, Aneesh, Michael Ellermen and myself.

4K PTE format remain unchanged currently.

Scanned through the PTE format again for hash 64K and 4K. It seems to me that there might be 5 free bits already present on the PTE format. I might have seriously mistaken something here :) Please correct me if that is not the case. _RPAGE_RPN* I think is applicable only for hash page table format and will not be available for radix later.

Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6 in the 64K backed hpte pages. This along with the earlier patch will entirely free up the four bits from 64K PTE.

This patch does the following change to 64K PTE that is backed by 64K hpte.

H_PAGE_F_SECOND which occupied bit 4 moves to the second part
of the pte.
H_PAGE_F_GIX which occupied bit 5, 6 and 7 also moves to the
second part of the pte.

since bit 7 is now freed up, we move H_PAGE_BUSY from bit 9 to bit 7. Trying to minimize gaps so that contiguous bits can be allocated if needed in the future.

The second part of the PTE will hold (H_PAGE_F_SECOND|H_PAGE_F_GIX) at bit 60,61,62,63.

Quoted 16 lines · Ram Pai

Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6 in the 64K backed hpte pages. This along with the earlier patch will entirely free up the four bits from 64K PTE.

This patch does the following change to 64K PTE that is backed by 64K hpte.

H_PAGE_F_SECOND which occupied bit 4 moves to the second part
of the pte.
H_PAGE_F_GIX which occupied bit 5, 6 and 7 also moves to the
second part of the pte.

since bit 7 is now freed up, we move H_PAGE_BUSY from bit 9 to bit 7. Trying to minimize gaps so that contiguous bits can be allocated if needed in the future.

Quoted 2 lines

The second part of the PTE will hold (H_PAGE_F_SECOND|H_PAGE_F_GIX) at bit 60,61,62,63.

I still dont understand how we freed up the 5th bit which is used in the 5th patch. Was that bit never used for any thing on 64K page size (64K and 4K mappings) ?

Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6 in the 64K backed hpte pages. This along with the earlier patch will entirely free up the four bits from 64K PTE.

This patch does the following change to 64K PTE that is backed by 64K hpte.

H_PAGE_F_SECOND which occupied bit 4 moves to the second part
of the pte.
H_PAGE_F_GIX which occupied bit 5, 6 and 7 also moves to the
second part of the pte.

since bit 7 is now freed up, we move H_PAGE_BUSY from bit 9 to bit 7. Trying to minimize gaps so that contiguous bits can be allocated if needed in the future.

The second part of the PTE will hold (H_PAGE_F_SECOND|H_PAGE_F_GIX) at bit 60,61,62,63.

Signed-off-by Ram Pai <[email protected]>

---
 arch/powerpc/include/asm/book3s/64/hash-64k.h | 26 ++++++++------------------
 arch/powerpc/mm/hash64_64k.c                  | 16 +++++++---------
 arch/powerpc/mm/hugetlbpage-hash64.c          | 16 ++++++----------
 3 files changed, 21 insertions(+), 37 deletions(-)

arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -8

index 0eb3c89..2fa5c60 100644
@@ -12,12 +12,8 @@
  */
 #define H_PAGE_COMBO   _RPAGE_RPN0 /* this is a combo 4k page */
 #define H_PAGE_4K_PFN  _RPAGE_RPN1 /* PFN is for a single 4k page */
-#define H_PAGE_F_SECOND	_RPAGE_RSV2	/* HPTE is in 2ndary HPTEG */
-#define H_PAGE_F_GIX	(_RPAGE_RSV3 | _RPAGE_RSV4 | _RPAGE_RPN44)
-#define H_PAGE_F_GIX_SHIFT	56
 
-
-#define H_PAGE_BUSY	_RPAGE_RPN42     /* software: PTE & hash are busy */
+#define H_PAGE_BUSY	_RPAGE_RPN44     /* software: PTE & hash are busy */
 #define H_PAGE_HASHPTE	_RPAGE_RPN43    /* PTE has associated HPTE */
 
 /*

@@ -56,24 +52,18 @@ static inline real_pte_t __real_pte(pte_t pte, pte_t *ptep)
 	unsigned long *hidxp;
 
 	rpte.pte = pte;
-	rpte.hidx = 0;
-	if (pte_val(pte) & H_PAGE_COMBO) {
-		/*

arch/powerpc/include/asm/book3s/64/hash-64k.h +4 -3

+	smp_rmb();
+	hidxp = (unsigned long *)(ptep + PTRS_PER_PTE);
+	rpte.hidx = *hidxp;
 	return rpte;
 }
 
 static inline unsigned long __rpte_to_hidx(real_pte_t rpte, unsigned long index)
 {
-	if ((pte_val(rpte.pte) & H_PAGE_COMBO))
-		return (rpte.hidx >> (index<<2)) & 0xf;
-	return (pte_val(rpte.pte) >> H_PAGE_F_GIX_SHIFT) & 0xf;
+	return ((rpte.hidx >> (index<<2)) & 0xfUL);
 }
 
 static inline unsigned long set_hidx_slot(pte_t *ptep, real_pte_t rpte,

arch/powerpc/mm/hash64_64k.c +7 -9

index 3702a3c..1c25ec2 100644
@@ -211,6 +211,7 @@ int __hash_page_64K(unsigned long ea, unsigned long access,
 		    unsigned long vsid, pte_t *ptep, unsigned long trap,
 		    unsigned long flags, int ssize)
 {
+	real_pte_t rpte;
 	unsigned long hpte_group;
 	unsigned long rflags, pa;
 	unsigned long old_pte, new_pte;

@@ -247,6 +248,7 @@ int __hash_page_64K(unsigned long ea, unsigned long access,
 	} while (!pte_xchg(ptep, __pte(old_pte), __pte(new_pte)));
 
 	rflags = htab_convert_pte_flags(new_pte);
+	rpte = __real_pte(__pte(old_pte), ptep);
 
 	if (cpu_has_feature(CPU_FTR_NOEXECUTE) &&
 	    !cpu_has_feature(CPU_FTR_COHERENT_ICACHE))

@@ -254,16 +256,13 @@ int __hash_page_64K(unsigned long ea, unsigned long access,
 
 	vpn  = hpt_vpn(ea, vsid, ssize);
 	if (unlikely(old_pte & H_PAGE_HASHPTE)) {
+		unsigned long gslot;
+
 		/*
 		 * There MIGHT be an HPTE for this pte
 		 */
-		hash = hpt_hash(vpn, shift, ssize);
-		if (old_pte & H_PAGE_F_SECOND)
-			hash = ~hash;
-		slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
-		slot += (old_pte & H_PAGE_F_GIX) >> H_PAGE_F_GIX_SHIFT;
-
-		if (mmu_hash_ops.hpte_updatepp(slot, rflags, vpn, MMU_PAGE_64K,
+		gslot = get_hidx_gslot(vpn, shift, ssize, rpte, 0);
+		if (mmu_hash_ops.hpte_updatepp(gslot, rflags, vpn, MMU_PAGE_64K,
 					       MMU_PAGE_64K, ssize,
 					       flags) == -1)
 			old_pte &= ~_PAGE_HPTEFLAGS;

@@ -313,8 +312,7 @@ int __hash_page_64K(unsigned long ea, unsigned long access,
 			return -1;
 		}
 
-		new_pte |= (slot << H_PAGE_F_GIX_SHIFT) &
-				(H_PAGE_F_SECOND | H_PAGE_F_GIX);
+		set_hidx_slot(ptep, rpte, 0, slot);
 		new_pte = (new_pte & ~_PAGE_HPTEFLAGS) | H_PAGE_HASHPTE;
 	}
 	*ptep = __pte(new_pte & ~H_PAGE_BUSY);

arch/powerpc/mm/hugetlbpage-hash64.c +6 -10

index a84bb44..239ca86 100644
@@ -22,6 +22,7 @@ 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)
 {
+	real_pte_t rpte;
 	unsigned long vpn;
 	unsigned long old_pte, new_pte;
 	unsigned long rflags, pa, sz;

@@ -61,6 +62,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
 	} while(!pte_xchg(ptep, __pte(old_pte), __pte(new_pte)));
 
 	rflags = htab_convert_pte_flags(new_pte);
+	rpte = __real_pte(__pte(old_pte), ptep);
 
 	sz = ((1UL) << shift);
 	if (!cpu_has_feature(CPU_FTR_COHERENT_ICACHE))

@@ -71,15 +73,10 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
 	/* Check if pte already has an hpte (case 2) */
 	if (unlikely(old_pte & H_PAGE_HASHPTE)) {
 		/* There MIGHT be an HPTE for this pte */
-		unsigned long hash, slot;
+		unsigned long gslot;
 
-		hash = hpt_hash(vpn, shift, ssize);
-		if (old_pte & H_PAGE_F_SECOND)
-			hash = ~hash;
-		slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
-		slot += (old_pte & H_PAGE_F_GIX) >> H_PAGE_F_GIX_SHIFT;
-
-		if (mmu_hash_ops.hpte_updatepp(slot, rflags, vpn, mmu_psize,
+		gslot = get_hidx_gslot(vpn, shift, ssize, rpte, 0);
+		if (mmu_hash_ops.hpte_updatepp(gslot, rflags, vpn, mmu_psize,
 					       mmu_psize, ssize, flags) == -1)
 			old_pte &= ~_PAGE_HPTEFLAGS;
 	}

@@ -106,8 +103,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
 			return -1;
 		}
 
-		new_pte |= (slot << H_PAGE_F_GIX_SHIFT) &
-			(H_PAGE_F_SECOND | H_PAGE_F_GIX);
+		new_pte |= set_hidx_slot(ptep, rpte, 0, slot);
 	}
 
 	/*

Signature · 1 line

1.8.3.1