book3s64/hash: Improve kfence support

RFC In review Ritesh Harjani (IBM) 2 participants 17 messages Jul 31, 2024 – Sep 4, 2024

Subsystems (2)

PowerPC Kfence

lore

Share & tools

lore

Ritesh Harjani (IBM)

author reviewer

2024-07-31 07:57 UTC #0 lore raw

cover book3s64/hash: Improve kfence support

Kfence on book3s64 Hash is broken. Kfence depends upon debug_pagealloc infrastructure on Hash. debug_pagealloc allocates a linear map based on the size of the DRAM i.e. 1 byte for every 64k page. That means for a 16TB DRAM, it will need 256MB memory for linear map. Memory for linear map on pseries comes from RMA region which has size limitation. On P8 RMA is 512MB, in which we also fit crash kernel at 256MB, paca allocations and emergency stacks. That means there is not enough memory in the RMA region for the linear map based on DRAM size (required by debug_pagealloc).

Now kfence only requires memory for it's kfence objects. kfence by default requires only (255 + 1) * 2 i.e. 32 MB for 64k pagesize.

This patch series removes the direct dependency of kfence on debug_pagealloc infrastructure. We separate the Hash kernel linear map functions to take linear map array as a parameter so that it can support debug_pagealloc and kfence individually. That means we don't need to keep the linear map region of size DRAM_SIZE >> PAGE_SHIFT anymore for kfence.

Hence, the current patch series solves the boot failure problem when kfence is enabled by optimizing the memory it requires for linear map within RMA region.

On radix we don't have this problem because no SLB and no RMA region size limitation.

Testing:
========
The patch series is still undergoing some testing. However, given that it's in good shape, I wanted to send out for review.
Note: It passes kfence kunit tests.
<dmesg results>

  [   48.715649][    T1] # kfence: pass:23 fail:0 skip:2 total:25
  [   48.716697][    T1] # Totals: pass:23 fail:0 skip:2 total:25
  [   48.717842][    T1] ok 1 kfence

TODOs: (for future patches)
===========================
However, there is still another problem which IMO makes kfence not suitable to be enabled by default on production kernels with Hash MMU i.e.
When kfence is enabled the kernel linear map uses PAGE_SIZE mapping rather than 16MB mapping as in the original case. Correct me if I am wrong, but theoretically at least this could cause TLB pressure in certain cases, which makes it not really suitable to be enabled by default on production kernels on Hash.

This is because on P8 book3s64, we don't support mapping multiple pagesizes (MPSS) within the kernel linear map segment. Is this understanding correct?

Ritesh Harjani (IBM) (10):
book3s64/hash: Remove kfence support temporarily book3s64/hash: Refactor kernel linear map related calls book3s64/hash: Add hash_debug_pagealloc_add_slot() function book3s64/hash: Add hash_debug_pagealloc_alloc_slots() function book3s64/hash: Refactor hash__kernel_map_pages() function book3s64/hash: Make kernel_map_linear_page() generic book3s64/hash: Disable debug_pagealloc if it requires more memory book3s64/hash: Add kfence functionality book3s64/radix: Refactoring common kfence related functions book3s64/hash: Disable kfence if not early init

 arch/powerpc/include/asm/kfence.h        |   2 +
 arch/powerpc/mm/book3s64/hash_utils.c    | 364 +++++++++++++++++------
 arch/powerpc/mm/book3s64/radix_pgtable.c |  12 -
 arch/powerpc/mm/init-common.c            |  12 +
 4 files changed, 286 insertions(+), 104 deletions(-)

--
2.45.2

Ritesh Harjani (IBM)

author reviewer

2024-07-31 07:57 UTC #1 lore raw

RFC 1/10 book3s64/hash: Remove kfence support temporarily

Kfence on book3s Hash on pseries is anyways broken. It fails to boot due to RMA size limitation. That is because, kfence with Hash uses debug_pagealloc infrastructure. debug_pagealloc allocates linear map for entire dram size instead of just kfence relevant objects. This means for 16TB of DRAM it will require (16TB >> PAGE_SHIFT) which is 256MB which is half of RMA region on P8.
crash kernel reserves 256MB and we also need 2048 * 16KB * 3 for emergency stack and some more for paca allocations.
That means there is not enough memory for reserving the full linear map in the RMA region, if the DRAM size is too big (>=16TB)
(The issue is seen above 8TB with crash kernel 256 MB reservation).

Now Kfence does not require linear memory map for entire DRAM. It only needs for kfence objects. So this patch temporarily removes the kfence functionality since debug_pagealloc code needs some refactoring. We will bring in kfence on Hash support in later patches.

Signed-off-by Ritesh Harjani (IBM) <[email protected]>

---
 arch/powerpc/include/asm/kfence.h     |  5 +++++
 arch/powerpc/mm/book3s64/hash_utils.c | 16 +++++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

arch/powerpc/include/asm/kfence.h +5 -0

index fab124ada1c7..f3a9476a71b3 100644
@@ -10,6 +10,7 @@
 
 #include <linux/mm.h>
 #include <asm/pgtable.h>
+#include <asm/mmu.h>
 
 #ifdef CONFIG_PPC64_ELF_ABI_V1
 #define ARCH_FUNC_PREFIX "."

@@ -25,6 +26,10 @@ static inline void disable_kfence(void)
 
 static inline bool arch_kfence_init_pool(void)
 {
+#ifdef CONFIG_PPC64
+	if (!radix_enabled())
+		return false;
+#endif
 	return !kfence_disabled;
 }
 #endif

arch/powerpc/mm/book3s64/hash_utils.c +11 -5

index 01c3b4b65241..1a1b50735fa0 100644
@@ -431,7 +431,7 @@ int htab_bolt_mapping(unsigned long vstart, unsigned long vend,
 			break;
 
 		cond_resched();
-		if (debug_pagealloc_enabled_or_kfence() &&
+		if (debug_pagealloc_enabled() &&
 			(paddr >> PAGE_SHIFT) < linear_map_hash_count)
 			linear_map_hash_slots[paddr >> PAGE_SHIFT] = ret | 0x80;
 	}

@@ -814,7 +814,7 @@ static void __init htab_init_page_sizes(void)
 	bool aligned = true;
 	init_hpte_page_sizes();
 
-	if (!debug_pagealloc_enabled_or_kfence()) {
+	if (!debug_pagealloc_enabled()) {
 		/*
 		 * Pick a size for the linear mapping. Currently, we only
 		 * support 16M, 1M and 4K which is the default

@@ -1134,7 +1134,7 @@ static void __init htab_initialize(void)
 
 	prot = pgprot_val(PAGE_KERNEL);
 
-	if (debug_pagealloc_enabled_or_kfence()) {
+	if (debug_pagealloc_enabled()) {
 		linear_map_hash_count = memblock_end_of_DRAM() >> PAGE_SHIFT;
 		linear_map_hash_slots = memblock_alloc_try_nid(
 				linear_map_hash_count, 1, MEMBLOCK_LOW_LIMIT,

@@ -2117,7 +2117,7 @@ void hpt_do_stress(unsigned long ea, unsigned long hpte_group)
 	}
 }
 
-#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE)
+#if defined(CONFIG_DEBUG_PAGEALLOC)
 static DEFINE_RAW_SPINLOCK(linear_map_hash_lock);
 
 static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)

@@ -2191,7 +2191,13 @@ int hash__kernel_map_pages(struct page *page, int numpages, int enable)
 	local_irq_restore(flags);
 	return 0;
 }
-#endif /* CONFIG_DEBUG_PAGEALLOC || CONFIG_KFENCE */
+#else /* CONFIG_DEBUG_PAGEALLOC */
+int hash__kernel_map_pages(struct page *page, int numpages,
+					 int enable)
+{
+	return 0;
+}
+#endif /* CONFIG_DEBUG_PAGEALLOC */
 
 void hash__setup_initial_memory_limit(phys_addr_t first_memblock_base,
 				phys_addr_t first_memblock_size)

Signature · 1 line

2.45.2

Christophe Leroy

2024-08-14 22:08 UTC #2 lore raw

Re: [RFC v1 01/10] book3s64/hash: Remove kfence support temporarily

Le 31/07/2024 à 09:56, Ritesh Harjani (IBM) a écrit :

Quoted 38 lines

[Vous ne recevez pas souvent de courriers de [email protected]. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
Kfence on book3s Hash on pseries is anyways broken. It fails to boot due to RMA size limitation. That is because, kfence with Hash uses debug_pagealloc infrastructure. debug_pagealloc allocates linear map for entire dram size instead of just kfence relevant objects. This means for 16TB of DRAM it will require (16TB >> PAGE_SHIFT) which is 256MB which is half of RMA region on P8.
crash kernel reserves 256MB and we also need 2048 * 16KB * 3 for emergency stack and some more for paca allocations.
That means there is not enough memory for reserving the full linear map in the RMA region, if the DRAM size is too big (>=16TB)
(The issue is seen above 8TB with crash kernel 256 MB reservation).
Now Kfence does not require linear memory map for entire DRAM. It only needs for kfence objects. So this patch temporarily removes the kfence functionality since debug_pagealloc code needs some refactoring. We will bring in kfence on Hash support in later patches.
Signed-off-by Ritesh Harjani (IBM) <[email protected]>
---
  arch/powerpc/include/asm/kfence.h     |  5 +++++
  arch/powerpc/mm/book3s64/hash_utils.c | 16 +++++++++++-----
  2 files changed, 16 insertions(+), 5 deletions(-)
arch/powerpc/include/asm/kfence.h +1 -0
index fab124ada1c7..f3a9476a71b3 100644
@@ -10,6 +10,7 @@
 
  #include <linux/mm.h>
  #include <asm/pgtable.h>
+#include <asm/mmu.h>
 
  #ifdef CONFIG_PPC64_ELF_ABI_V1
  #define ARCH_FUNC_PREFIX "."

arch/powerpc/include/asm/kfence.h static inline void disable_kfence(void) · line 32

Quoted 7 lines

arch/powerpc/include/asm/kfence.h +4 -0

@@ -25,6 +26,10 @@ static inline void disable_kfence(void)
 
  static inline bool arch_kfence_init_pool(void)
  {
+#ifdef CONFIG_PPC64
+       if (!radix_enabled())
+               return false;
+#endif

Avoid #ifdefs whenever possible. Here you can do:

	if (IS_ENABLED(CONFIG_PPC64) && !radix_enabled())
		return false;

Quoted 35 lines

arch/powerpc/include/asm/kfence.h +0 -0

         return !kfence_disabled;
  }
  #endif

arch/powerpc/mm/book3s64/hash_utils.c +3 -3

index 01c3b4b65241..1a1b50735fa0 100644
@@ -431,7 +431,7 @@ int htab_bolt_mapping(unsigned long vstart, unsigned long vend,
                         break;
 
                 cond_resched();
-               if (debug_pagealloc_enabled_or_kfence() &&
+               if (debug_pagealloc_enabled() &&
                         (paddr >> PAGE_SHIFT) < linear_map_hash_count)
                         linear_map_hash_slots[paddr >> PAGE_SHIFT] = ret | 0x80;
         }

@@ -814,7 +814,7 @@ static void __init htab_init_page_sizes(void)
         bool aligned = true;
         init_hpte_page_sizes();
 
-       if (!debug_pagealloc_enabled_or_kfence()) {
+       if (!debug_pagealloc_enabled()) {
                 /*
                  * Pick a size for the linear mapping. Currently, we only
                  * support 16M, 1M and 4K which is the default

@@ -1134,7 +1134,7 @@ static void __init htab_initialize(void)
 
         prot = pgprot_val(PAGE_KERNEL);
 
-       if (debug_pagealloc_enabled_or_kfence()) {
+       if (debug_pagealloc_enabled()) {
                 linear_map_hash_count = memblock_end_of_DRAM() >> PAGE_SHIFT;
                 linear_map_hash_slots = memblock_alloc_try_nid(
                                 linear_map_hash_count, 1, MEMBLOCK_LOW_LIMIT,

arch/powerpc/mm/book3s64/hash_utils.c void hpt_do_stress(unsigned long ea, unsigned long hpte_group) · line 2120

Quoted 5 lines

arch/powerpc/mm/book3s64/hash_utils.c +1 -1

@@ -2117,7 +2117,7 @@ void hpt_do_stress(unsigned long ea, unsigned long hpte_group)
         }
  }
 
-#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE)
+#if defined(CONFIG_DEBUG_PAGEALLOC)

Use #ifdef

Quoted 21 lines

arch/powerpc/mm/book3s64/hash_utils.c +7 -2

  static DEFINE_RAW_SPINLOCK(linear_map_hash_lock);
 
  static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)

@@ -2191,7 +2191,13 @@ int hash__kernel_map_pages(struct page *page, int numpages, int enable)
         local_irq_restore(flags);
         return 0;
  }
-#endif /* CONFIG_DEBUG_PAGEALLOC || CONFIG_KFENCE */
+#else /* CONFIG_DEBUG_PAGEALLOC */
+int hash__kernel_map_pages(struct page *page, int numpages,
+                                        int enable)
+{
+       return 0;
+}
+#endif /* CONFIG_DEBUG_PAGEALLOC */
 
  void hash__setup_initial_memory_limit(phys_addr_t first_memblock_base,
                                 phys_addr_t first_memblock_size)
--

2.45.2

Ritesh Harjani (IBM)

author reviewer

2024-09-04 18:31 UTC #3 lore raw

Re: [RFC v1 01/10] book3s64/hash: Remove kfence support temporarily

Sorry for the delayed response. Was pulled into something else.

Quoted 49 lines · Christophe Leroy

Le 31/07/2024 à 09:56, Ritesh Harjani (IBM) a écrit :
Quoted 38 lines
[Vous ne recevez pas souvent de courriers de [email protected]. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
Kfence on book3s Hash on pseries is anyways broken. It fails to boot due to RMA size limitation. That is because, kfence with Hash uses debug_pagealloc infrastructure. debug_pagealloc allocates linear map for entire dram size instead of just kfence relevant objects. This means for 16TB of DRAM it will require (16TB >> PAGE_SHIFT) which is 256MB which is half of RMA region on P8.
crash kernel reserves 256MB and we also need 2048 * 16KB * 3 for emergency stack and some more for paca allocations.
That means there is not enough memory for reserving the full linear map in the RMA region, if the DRAM size is too big (>=16TB)
(The issue is seen above 8TB with crash kernel 256 MB reservation).
Now Kfence does not require linear memory map for entire DRAM. It only needs for kfence objects. So this patch temporarily removes the kfence functionality since debug_pagealloc code needs some refactoring. We will bring in kfence on Hash support in later patches.
Signed-off-by Ritesh Harjani (IBM) <[email protected]>
---
  arch/powerpc/include/asm/kfence.h     |  5 +++++
  arch/powerpc/mm/book3s64/hash_utils.c | 16 +++++++++++-----
  2 files changed, 16 insertions(+), 5 deletions(-)
arch/powerpc/include/asm/kfence.h +1 -0
index fab124ada1c7..f3a9476a71b3 100644
@@ -10,6 +10,7 @@
 
  #include <linux/mm.h>
  #include <asm/pgtable.h>
+#include <asm/mmu.h>
 
  #ifdef CONFIG_PPC64_ELF_ABI_V1
  #define ARCH_FUNC_PREFIX "."
arch/powerpc/include/asm/kfence.h static inline void disable_kfence(void) · line 32
Quoted 7 lines
arch/powerpc/include/asm/kfence.h +4 -0
@@ -25,6 +26,10 @@ static inline void disable_kfence(void)
 
  static inline bool arch_kfence_init_pool(void)
  {
+#ifdef CONFIG_PPC64
+       if (!radix_enabled())
+               return false;
+#endif
Avoid #ifdefs whenever possible. Here you can do:

Quoted 3 lines

	if (IS_ENABLED(CONFIG_PPC64) && !radix_enabled())
		return false;

Sure. I will change it.

Quoted 38 lines

arch/powerpc/include/asm/kfence.h +0 -0

         return !kfence_disabled;
  }
  #endif

arch/powerpc/mm/book3s64/hash_utils.c +3 -3

index 01c3b4b65241..1a1b50735fa0 100644
@@ -431,7 +431,7 @@ int htab_bolt_mapping(unsigned long vstart, unsigned long vend,
                         break;
 
                 cond_resched();
-               if (debug_pagealloc_enabled_or_kfence() &&
+               if (debug_pagealloc_enabled() &&
                         (paddr >> PAGE_SHIFT) < linear_map_hash_count)
                         linear_map_hash_slots[paddr >> PAGE_SHIFT] = ret | 0x80;
         }

@@ -814,7 +814,7 @@ static void __init htab_init_page_sizes(void)
         bool aligned = true;
         init_hpte_page_sizes();
 
-       if (!debug_pagealloc_enabled_or_kfence()) {
+       if (!debug_pagealloc_enabled()) {
                 /*
                  * Pick a size for the linear mapping. Currently, we only
                  * support 16M, 1M and 4K which is the default

@@ -1134,7 +1134,7 @@ static void __init htab_initialize(void)
 
         prot = pgprot_val(PAGE_KERNEL);
 
-       if (debug_pagealloc_enabled_or_kfence()) {
+       if (debug_pagealloc_enabled()) {
                 linear_map_hash_count = memblock_end_of_DRAM() >> PAGE_SHIFT;
                 linear_map_hash_slots = memblock_alloc_try_nid(
                                 linear_map_hash_count, 1, MEMBLOCK_LOW_LIMIT,

@@ -2117,7 +2117,7 @@ void hpt_do_stress(unsigned long ea, unsigned long hpte_group)
         }
  }

Quoted 5 lines

arch/powerpc/mm/book3s64/hash_utils.c void hpt_do_stress(unsigned long ea, unsigned long hpte_group) · line 2120
Quoted 2 lines
arch/powerpc/mm/book3s64/hash_utils.c +1 -1
-#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE)
+#if defined(CONFIG_DEBUG_PAGEALLOC)
Use #ifdef

Sure. Thanks!

-ritesh

Ritesh Harjani (IBM)

author reviewer

2024-07-31 07:58 UTC #4 lore raw

RFC 2/10 book3s64/hash: Refactor kernel linear map related calls

In reply to Ritesh Harjani (IBM)

This just brings all linear map related handling at one place instead of having those functions scattered in hash_utils file.
Makes it easy for review.

No functionality changes in this patch.

Signed-off-by Ritesh Harjani (IBM) <[email protected]>

---
 arch/powerpc/mm/book3s64/hash_utils.c | 164 +++++++++++++-------------
 1 file changed, 82 insertions(+), 82 deletions(-)

arch/powerpc/mm/book3s64/hash_utils.c +82 ~~-82~~

index 1a1b50735fa0..b6ae955971bf 100644
@@ -273,6 +273,88 @@ void hash__tlbiel_all(unsigned int action)
 		WARN(1, "%s called on pre-POWER7 CPU\n", __func__);
 }
 
+#if defined(CONFIG_DEBUG_PAGEALLOC)
+static DEFINE_RAW_SPINLOCK(linear_map_hash_lock);
+
+static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)
+{
+	unsigned long hash;
+	unsigned long vsid = get_kernel_vsid(vaddr, mmu_kernel_ssize);
+	unsigned long vpn = hpt_vpn(vaddr, vsid, mmu_kernel_ssize);
+	unsigned long mode = htab_convert_pte_flags(pgprot_val(PAGE_KERNEL), HPTE_USE_KERNEL_KEY);
+	long ret;
+
+	hash = hpt_hash(vpn, PAGE_SHIFT, mmu_kernel_ssize);
+
+	/* Don't create HPTE entries for bad address */
+	if (!vsid)
+		return;
+
+	if (linear_map_hash_slots[lmi] & 0x80)
+		return;
+
+	ret = hpte_insert_repeating(hash, vpn, __pa(vaddr), mode,
+				    HPTE_V_BOLTED,
+				    mmu_linear_psize, mmu_kernel_ssize);
+
+	BUG_ON (ret < 0);
+	raw_spin_lock(&linear_map_hash_lock);
+	BUG_ON(linear_map_hash_slots[lmi] & 0x80);
+	linear_map_hash_slots[lmi] = ret | 0x80;
+	raw_spin_unlock(&linear_map_hash_lock);
+}
+
+static void kernel_unmap_linear_page(unsigned long vaddr, unsigned long lmi)
+{
+	unsigned long hash, hidx, slot;
+	unsigned long vsid = get_kernel_vsid(vaddr, mmu_kernel_ssize);
+	unsigned long vpn = hpt_vpn(vaddr, vsid, mmu_kernel_ssize);
+
+	hash = hpt_hash(vpn, PAGE_SHIFT, mmu_kernel_ssize);
+	raw_spin_lock(&linear_map_hash_lock);
+	if (!(linear_map_hash_slots[lmi] & 0x80)) {
+		raw_spin_unlock(&linear_map_hash_lock);
+		return;
+	}
+	hidx = linear_map_hash_slots[lmi] & 0x7f;
+	linear_map_hash_slots[lmi] = 0;
+	raw_spin_unlock(&linear_map_hash_lock);
+	if (hidx & _PTEIDX_SECONDARY)
+		hash = ~hash;
+	slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
+	slot += hidx & _PTEIDX_GROUP_IX;
+	mmu_hash_ops.hpte_invalidate(slot, vpn, mmu_linear_psize,
+				     mmu_linear_psize,
+				     mmu_kernel_ssize, 0);
+}
+
+int hash__kernel_map_pages(struct page *page, int numpages, int enable)
+{
+	unsigned long flags, vaddr, lmi;
+	int i;
+
+	local_irq_save(flags);
+	for (i = 0; i < numpages; i++, page++) {
+		vaddr = (unsigned long)page_address(page);
+		lmi = __pa(vaddr) >> PAGE_SHIFT;
+		if (lmi >= linear_map_hash_count)
+			continue;
+		if (enable)
+			kernel_map_linear_page(vaddr, lmi);
+		else
+			kernel_unmap_linear_page(vaddr, lmi);
+	}
+	local_irq_restore(flags);
+	return 0;
+}
+#else /* CONFIG_DEBUG_PAGEALLOC */
+int hash__kernel_map_pages(struct page *page, int numpages,
+					 int enable)
+{
+	return 0;
+}
+#endif /* CONFIG_DEBUG_PAGEALLOC */
+
 /*
  * 'R' and 'C' update notes:
  *  - Under pHyp or KVM, the updatepp path will not set C, thus it *will*

@@ -2117,88 +2199,6 @@ void hpt_do_stress(unsigned long ea, unsigned long hpte_group)
 	}
 }
 
-#if defined(CONFIG_DEBUG_PAGEALLOC)
-static DEFINE_RAW_SPINLOCK(linear_map_hash_lock);
-
-static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)
-{
-	unsigned long hash;
-	unsigned long vsid = get_kernel_vsid(vaddr, mmu_kernel_ssize);
-	unsigned long vpn = hpt_vpn(vaddr, vsid, mmu_kernel_ssize);
-	unsigned long mode = htab_convert_pte_flags(pgprot_val(PAGE_KERNEL), HPTE_USE_KERNEL_KEY);
-	long ret;
-
-	hash = hpt_hash(vpn, PAGE_SHIFT, mmu_kernel_ssize);
-
-	/* Don't create HPTE entries for bad address */
-	if (!vsid)
-		return;
-
-	if (linear_map_hash_slots[lmi] & 0x80)
-		return;
-
-	ret = hpte_insert_repeating(hash, vpn, __pa(vaddr), mode,
-				    HPTE_V_BOLTED,
-				    mmu_linear_psize, mmu_kernel_ssize);
-
-	BUG_ON (ret < 0);
-	raw_spin_lock(&linear_map_hash_lock);
-	BUG_ON(linear_map_hash_slots[lmi] & 0x80);
-	linear_map_hash_slots[lmi] = ret | 0x80;
-	raw_spin_unlock(&linear_map_hash_lock);
-}
-
-static void kernel_unmap_linear_page(unsigned long vaddr, unsigned long lmi)
-{
-	unsigned long hash, hidx, slot;
-	unsigned long vsid = get_kernel_vsid(vaddr, mmu_kernel_ssize);
-	unsigned long vpn = hpt_vpn(vaddr, vsid, mmu_kernel_ssize);
-
-	hash = hpt_hash(vpn, PAGE_SHIFT, mmu_kernel_ssize);
-	raw_spin_lock(&linear_map_hash_lock);
-	if (!(linear_map_hash_slots[lmi] & 0x80)) {
-		raw_spin_unlock(&linear_map_hash_lock);
-		return;
-	}
-	hidx = linear_map_hash_slots[lmi] & 0x7f;
-	linear_map_hash_slots[lmi] = 0;
-	raw_spin_unlock(&linear_map_hash_lock);
-	if (hidx & _PTEIDX_SECONDARY)
-		hash = ~hash;
-	slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
-	slot += hidx & _PTEIDX_GROUP_IX;
-	mmu_hash_ops.hpte_invalidate(slot, vpn, mmu_linear_psize,
-				     mmu_linear_psize,
-				     mmu_kernel_ssize, 0);
-}
-
-int hash__kernel_map_pages(struct page *page, int numpages, int enable)
-{
-	unsigned long flags, vaddr, lmi;
-	int i;
-
-	local_irq_save(flags);
-	for (i = 0; i < numpages; i++, page++) {
-		vaddr = (unsigned long)page_address(page);
-		lmi = __pa(vaddr) >> PAGE_SHIFT;
-		if (lmi >= linear_map_hash_count)
-			continue;
-		if (enable)
-			kernel_map_linear_page(vaddr, lmi);
-		else
-			kernel_unmap_linear_page(vaddr, lmi);
-	}
-	local_irq_restore(flags);
-	return 0;
-}
-#else /* CONFIG_DEBUG_PAGEALLOC */
-int hash__kernel_map_pages(struct page *page, int numpages,
-					 int enable)
-{
-	return 0;
-}
-#endif /* CONFIG_DEBUG_PAGEALLOC */
-
 void hash__setup_initial_memory_limit(phys_addr_t first_memblock_base,
 				phys_addr_t first_memblock_size)
 {

Signature · 1 line

2.45.2

Ritesh Harjani (IBM)

author reviewer

2024-07-31 07:59 UTC #5 lore raw

RFC 3/10 book3s64/hash: Add hash_debug_pagealloc_add_slot() function

In reply to Ritesh Harjani (IBM)

This adds hash_debug_pagealloc_add_slot() function instead of open coding that in htab_bolt_mapping(). This is required since we will be separating kfence functionality to not depend upon debug_pagealloc.

No functionality change in this patch.

Signed-off-by Ritesh Harjani (IBM) <[email protected]>

---
 arch/powerpc/mm/book3s64/hash_utils.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

arch/powerpc/mm/book3s64/hash_utils.c +10 -3

index b6ae955971bf..47b40b9b49d6 100644
@@ -328,6 +328,14 @@ static void kernel_unmap_linear_page(unsigned long vaddr, unsigned long lmi)
 				     mmu_kernel_ssize, 0);
 }
 
+static inline void hash_debug_pagealloc_add_slot(phys_addr_t paddr, int slot)
+{
+	if (!debug_pagealloc_enabled())
+		return;
+	if ((paddr >> PAGE_SHIFT) < linear_map_hash_count)
+		linear_map_hash_slots[paddr >> PAGE_SHIFT] = slot | 0x80;
+}
+
 int hash__kernel_map_pages(struct page *page, int numpages, int enable)
 {
 	unsigned long flags, vaddr, lmi;

@@ -353,6 +361,7 @@ int hash__kernel_map_pages(struct page *page, int numpages,
 {
 	return 0;
 }
+static inline void hash_debug_pagealloc_add_slot(phys_addr_t paddr, int slot) {}
 #endif /* CONFIG_DEBUG_PAGEALLOC */
 
 /*

@@ -513,9 +522,7 @@ int htab_bolt_mapping(unsigned long vstart, unsigned long vend,
 			break;
 
 		cond_resched();
-		if (debug_pagealloc_enabled() &&
-			(paddr >> PAGE_SHIFT) < linear_map_hash_count)
-			linear_map_hash_slots[paddr >> PAGE_SHIFT] = ret | 0x80;
+		hash_debug_pagealloc_add_slot(paddr, ret);
 	}
 	return ret < 0 ? ret : 0;
 }

Signature · 1 line

2.45.2

Ritesh Harjani (IBM)

author reviewer

2024-07-31 08:00 UTC #6 lore raw

RFC 4/10 book3s64/hash: Add hash_debug_pagealloc_alloc_slots() function

In reply to Ritesh Harjani (IBM)

This adds hash_debug_pagealloc_alloc_slots() function instead of open coding that in htab_initialize(). This is required since we will be separating the kfence functionality to not depend upon debug_pagealloc.

Now that everything required for debug_pagealloc is under a #ifdef config. Bring in linear_map_hash_slots and linear_map_hash_count variables under the same config too.

Signed-off-by Ritesh Harjani (IBM) <[email protected]>

---
 arch/powerpc/mm/book3s64/hash_utils.c | 29 ++++++++++++++++-----------
 1 file changed, 17 insertions(+), 12 deletions(-)

arch/powerpc/mm/book3s64/hash_utils.c +17 ~~-12~~

index 47b40b9b49d6..6af47b996e79 100644
@@ -123,8 +123,6 @@ EXPORT_SYMBOL_GPL(mmu_slb_size);
 #ifdef CONFIG_PPC_64K_PAGES
 int mmu_ci_restrictions;
 #endif
-static u8 *linear_map_hash_slots;
-static unsigned long linear_map_hash_count;
 struct mmu_hash_ops mmu_hash_ops;
 EXPORT_SYMBOL(mmu_hash_ops);

@@ -274,6 +272,8 @@ void hash__tlbiel_all(unsigned int action)
 }
 
 #if defined(CONFIG_DEBUG_PAGEALLOC)
+static u8 *linear_map_hash_slots;
+static unsigned long linear_map_hash_count;
 static DEFINE_RAW_SPINLOCK(linear_map_hash_lock);
 
 static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)

@@ -328,6 +328,19 @@ static void kernel_unmap_linear_page(unsigned long vaddr, unsigned long lmi)
 				     mmu_kernel_ssize, 0);
 }
 
+static inline void hash_debug_pagealloc_alloc_slots(void)
+{
+	if (!debug_pagealloc_enabled())
+		return;
+	linear_map_hash_count = memblock_end_of_DRAM() >> PAGE_SHIFT;
+	linear_map_hash_slots = memblock_alloc_try_nid(
+			linear_map_hash_count, 1, MEMBLOCK_LOW_LIMIT,
+			ppc64_rma_size,	NUMA_NO_NODE);
+	if (!linear_map_hash_slots)
+		panic("%s: Failed to allocate %lu bytes max_addr=%pa\n",
+		      __func__, linear_map_hash_count, &ppc64_rma_size);
+}
+
 static inline void hash_debug_pagealloc_add_slot(phys_addr_t paddr, int slot)
 {
 	if (!debug_pagealloc_enabled())

@@ -361,6 +374,7 @@ int hash__kernel_map_pages(struct page *page, int numpages,
 {
 	return 0;
 }
+static inline void hash_debug_pagealloc_alloc_slots(void) {}
 static inline void hash_debug_pagealloc_add_slot(phys_addr_t paddr, int slot) {}
 #endif /* CONFIG_DEBUG_PAGEALLOC */

@@ -1223,16 +1237,7 @@ static void __init htab_initialize(void)
 
 	prot = pgprot_val(PAGE_KERNEL);
 
-	if (debug_pagealloc_enabled()) {
-		linear_map_hash_count = memblock_end_of_DRAM() >> PAGE_SHIFT;
-		linear_map_hash_slots = memblock_alloc_try_nid(
-				linear_map_hash_count, 1, MEMBLOCK_LOW_LIMIT,
-				ppc64_rma_size,	NUMA_NO_NODE);
-		if (!linear_map_hash_slots)
-			panic("%s: Failed to allocate %lu bytes max_addr=%pa\n",
-			      __func__, linear_map_hash_count, &ppc64_rma_size);
-	}
-
+	hash_debug_pagealloc_alloc_slots();
 	/* create bolted the linear mapping in the hash table */
 	for_each_mem_range(i, &base, &end) {
 		size = end - base;

Signature · 1 line

2.45.2

Ritesh Harjani (IBM)

author reviewer

2024-07-31 08:00 UTC #7 lore raw

RFC 5/10 book3s64/hash: Refactor hash__kernel_map_pages() function

In reply to Ritesh Harjani (IBM)

This refactors hash__kernel_map_pages() function to call hash_debug_pagealloc_map_pages(). This will come useful when we will add kfence support.

No functionality changes in this patch.

Signed-off-by Ritesh Harjani (IBM) <[email protected]>

---
 arch/powerpc/mm/book3s64/hash_utils.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

arch/powerpc/mm/book3s64/hash_utils.c +8 -1

index 6af47b996e79..b96bbb0025fb 100644
@@ -349,7 +349,8 @@ static inline void hash_debug_pagealloc_add_slot(phys_addr_t paddr, int slot)
 		linear_map_hash_slots[paddr >> PAGE_SHIFT] = slot | 0x80;
 }
 
-int hash__kernel_map_pages(struct page *page, int numpages, int enable)
+static int hash_debug_pagealloc_map_pages(struct page *page, int numpages,
+					  int enable)
 {
 	unsigned long flags, vaddr, lmi;
 	int i;

@@ -368,6 +369,12 @@ int hash__kernel_map_pages(struct page *page, int numpages, int enable)
 	local_irq_restore(flags);
 	return 0;
 }
+
+int hash__kernel_map_pages(struct page *page, int numpages, int enable)
+{
+	return hash_debug_pagealloc_map_pages(page, numpages, enable);
+}
+
 #else /* CONFIG_DEBUG_PAGEALLOC */
 int hash__kernel_map_pages(struct page *page, int numpages,
 					 int enable)

Signature · 1 line

2.45.2

Ritesh Harjani (IBM)

author reviewer

2024-07-31 08:01 UTC #8 lore raw

RFC 6/10 book3s64/hash: Make kernel_map_linear_page() generic

In reply to Ritesh Harjani (IBM)

Currently kernel_map_linear_page() function assumes to be working on linear_map_hash_slots array. But since in later patches we need a separate linear map array for kfence, hence make kernel_map_linear_page() take a linear map array and lock in it's function argument.

This is needed to separate out kfence from debug_pagealloc infrastructure.

Signed-off-by Ritesh Harjani (IBM) <[email protected]>

---
 arch/powerpc/mm/book3s64/hash_utils.c | 47 ++++++++++++++-------------
 1 file changed, 25 insertions(+), 22 deletions(-)

arch/powerpc/mm/book3s64/hash_utils.c +25 ~~-22~~

index b96bbb0025fb..3f3eaf0a254b 100644
@@ -272,11 +272,8 @@ void hash__tlbiel_all(unsigned int action)
 }
 
 #if defined(CONFIG_DEBUG_PAGEALLOC)
-static u8 *linear_map_hash_slots;
-static unsigned long linear_map_hash_count;
-static DEFINE_RAW_SPINLOCK(linear_map_hash_lock);
-
-static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)
+static void kernel_map_linear_page(unsigned long vaddr, unsigned long idx,
+				   u8 *slots, raw_spinlock_t *lock)
 {
 	unsigned long hash;
 	unsigned long vsid = get_kernel_vsid(vaddr, mmu_kernel_ssize);

@@ -290,7 +287,7 @@ static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)
 	if (!vsid)
 		return;
 
-	if (linear_map_hash_slots[lmi] & 0x80)
+	if (slots[idx] & 0x80)
 		return;
 
 	ret = hpte_insert_repeating(hash, vpn, __pa(vaddr), mode,

@@ -298,36 +295,40 @@ static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)
 				    mmu_linear_psize, mmu_kernel_ssize);
 
 	BUG_ON (ret < 0);
-	raw_spin_lock(&linear_map_hash_lock);
-	BUG_ON(linear_map_hash_slots[lmi] & 0x80);
-	linear_map_hash_slots[lmi] = ret | 0x80;
-	raw_spin_unlock(&linear_map_hash_lock);
+	raw_spin_lock(lock);
+	BUG_ON(slots[idx] & 0x80);
+	slots[idx] = ret | 0x80;
+	raw_spin_unlock(lock);
 }
 
-static void kernel_unmap_linear_page(unsigned long vaddr, unsigned long lmi)
+static void kernel_unmap_linear_page(unsigned long vaddr, unsigned long idx,
+				     u8 *slots, raw_spinlock_t *lock)
 {
-	unsigned long hash, hidx, slot;
+	unsigned long hash, hslot, slot;
 	unsigned long vsid = get_kernel_vsid(vaddr, mmu_kernel_ssize);
 	unsigned long vpn = hpt_vpn(vaddr, vsid, mmu_kernel_ssize);
 
 	hash = hpt_hash(vpn, PAGE_SHIFT, mmu_kernel_ssize);
-	raw_spin_lock(&linear_map_hash_lock);
-	if (!(linear_map_hash_slots[lmi] & 0x80)) {
-		raw_spin_unlock(&linear_map_hash_lock);
+	raw_spin_lock(lock);
+	if (!(slots[idx] & 0x80)) {
+		raw_spin_unlock(lock);
 		return;
 	}
-	hidx = linear_map_hash_slots[lmi] & 0x7f;
-	linear_map_hash_slots[lmi] = 0;
-	raw_spin_unlock(&linear_map_hash_lock);
-	if (hidx & _PTEIDX_SECONDARY)
+	hslot = slots[idx] & 0x7f;
+	slots[idx] = 0;
+	raw_spin_unlock(lock);
+	if (hslot & _PTEIDX_SECONDARY)
 		hash = ~hash;
 	slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
-	slot += hidx & _PTEIDX_GROUP_IX;
+	slot += hslot & _PTEIDX_GROUP_IX;
 	mmu_hash_ops.hpte_invalidate(slot, vpn, mmu_linear_psize,
 				     mmu_linear_psize,
 				     mmu_kernel_ssize, 0);
 }
 
+static u8 *linear_map_hash_slots;
+static unsigned long linear_map_hash_count;
+static DEFINE_RAW_SPINLOCK(linear_map_hash_lock);
 static inline void hash_debug_pagealloc_alloc_slots(void)
 {
 	if (!debug_pagealloc_enabled())

@@ -362,9 +363,11 @@ static int hash_debug_pagealloc_map_pages(struct page *page, int numpages,
 		if (lmi >= linear_map_hash_count)
 			continue;
 		if (enable)
-			kernel_map_linear_page(vaddr, lmi);
+			kernel_map_linear_page(vaddr, lmi,
+				linear_map_hash_slots, &linear_map_hash_lock);
 		else
-			kernel_unmap_linear_page(vaddr, lmi);
+			kernel_unmap_linear_page(vaddr, lmi,
+				linear_map_hash_slots, &linear_map_hash_lock);
 	}
 	local_irq_restore(flags);
 	return 0;

Signature · 1 line

2.45.2

Ritesh Harjani (IBM)

author reviewer

2024-07-31 08:02 UTC #9 lore raw

RFC 7/10 book3s64/hash: Disable debug_pagealloc if it requires more memory

In reply to Ritesh Harjani (IBM)

Make size of the linear map to be allocated in RMA region to be of ppc64_rma_size / 4. If debug_pagealloc requires more memory than that then do not allocate any memory and disable debug_pagealloc.

Signed-off-by Ritesh Harjani (IBM) <[email protected]>

---
 arch/powerpc/mm/book3s64/hash_utils.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

arch/powerpc/mm/book3s64/hash_utils.c +14 -1

index 3f3eaf0a254b..906cd167180a 100644
@@ -331,9 +331,19 @@ static unsigned long linear_map_hash_count;
 static DEFINE_RAW_SPINLOCK(linear_map_hash_lock);
 static inline void hash_debug_pagealloc_alloc_slots(void)
 {
+	unsigned long max_hash_count = (ppc64_rma_size / 4) >> PAGE_SHIFT;
+
 	if (!debug_pagealloc_enabled())
 		return;
 	linear_map_hash_count = memblock_end_of_DRAM() >> PAGE_SHIFT;
+	if (unlikely(linear_map_hash_count > max_hash_count)) {
+		pr_info("linear map size (%llu) greater than 4 times RMA region (%llu). Disabling debug_pagealloc\n",
+			((u64)linear_map_hash_count << PAGE_SHIFT),
+			ppc64_rma_size);
+		linear_map_hash_count = 0;
+		return;
+	}
+
 	linear_map_hash_slots = memblock_alloc_try_nid(
 			linear_map_hash_count, 1, MEMBLOCK_LOW_LIMIT,
 			ppc64_rma_size,	NUMA_NO_NODE);

@@ -344,7 +354,7 @@ static inline void hash_debug_pagealloc_alloc_slots(void)
 
 static inline void hash_debug_pagealloc_add_slot(phys_addr_t paddr, int slot)
 {
-	if (!debug_pagealloc_enabled())
+	if (!debug_pagealloc_enabled() || !linear_map_hash_count)
 		return;
 	if ((paddr >> PAGE_SHIFT) < linear_map_hash_count)
 		linear_map_hash_slots[paddr >> PAGE_SHIFT] = slot | 0x80;

@@ -356,6 +366,9 @@ static int hash_debug_pagealloc_map_pages(struct page *page, int numpages,
 	unsigned long flags, vaddr, lmi;
 	int i;
 
+	if (!debug_pagealloc_enabled() || !linear_map_hash_count)
+		return 0;
+
 	local_irq_save(flags);
 	for (i = 0; i < numpages; i++, page++) {
 		vaddr = (unsigned long)page_address(page);

Signature · 1 line

2.45.2

Ritesh Harjani (IBM)

author reviewer

2024-07-31 08:02 UTC #10 lore raw

RFC 8/10 book3s64/hash: Add kfence functionality

In reply to Ritesh Harjani (IBM)

Now that linear map functionality of debug_pagealloc is made generic, enable kfence to use this generic infrastructure.

1. Define kfence related linear map variables.
- u8 *linear_map_kf_hash_slots;
- unsigned long linear_map_kf_hash_count;
- DEFINE_RAW_SPINLOCK(linear_map_kf_hash_lock);
2. The linear map size allocated in RMA region is quite small (KFENCE_POOL_SIZE >> PAGE_SHIFT) which is 512 bytes by default.
3. kfence pool memory is reserved using memblock_phys_alloc() which has can come from anywhere.
(default 255 objects => ((1+255) * 2) << PAGE_SHIFT = 32MB)
4. The hash slot information for kfence memory gets added in linear map in hash_linear_map_add_slot() (which also adds for debug_pagealloc).

Reported-by Pavithra Prakash <[email protected]>

Signed-off-by Ritesh Harjani (IBM) <[email protected]>

---
 arch/powerpc/include/asm/kfence.h     |   5 -
 arch/powerpc/mm/book3s64/hash_utils.c | 162 +++++++++++++++++++++++---
 2 files changed, 149 insertions(+), 18 deletions(-)

arch/powerpc/include/asm/kfence.h +0 -5

index f3a9476a71b3..fab124ada1c7 100644
@@ -10,7 +10,6 @@
 
 #include <linux/mm.h>
 #include <asm/pgtable.h>
-#include <asm/mmu.h>
 
 #ifdef CONFIG_PPC64_ELF_ABI_V1
 #define ARCH_FUNC_PREFIX "."

@@ -26,10 +25,6 @@ static inline void disable_kfence(void)
 
 static inline bool arch_kfence_init_pool(void)
 {
-#ifdef CONFIG_PPC64
-	if (!radix_enabled())
-		return false;
-#endif
 	return !kfence_disabled;
 }
 #endif

arch/powerpc/mm/book3s64/hash_utils.c +149 ~~-13~~

232 lines not on this page · raw message

Signature · 1 line

2.45.2

Ritesh Harjani (IBM)

author reviewer

2024-07-31 08:03 UTC #11 lore raw

RFC 9/10 book3s64/radix: Refactoring common kfence related functions

In reply to Ritesh Harjani (IBM)

Both radix and hash on book3s requires to detect if kfence early init is enabled or not. Hash needs to disable kfence if early init is not enabled because with kfence the linear map is mapped using PAGE_SIZE rather than 16M mapping.
We don't support multiple page sizes for slb entry used for kernel linear map in book3s64.

This patch refactors out the common functions required to detect kfence early init is enabled or not.

Signed-off-by Ritesh Harjani (IBM) <[email protected]>

---
 arch/powerpc/include/asm/kfence.h        |  2 ++
 arch/powerpc/mm/book3s64/radix_pgtable.c | 12 ------------
 arch/powerpc/mm/init-common.c            | 12 ++++++++++++
 3 files changed, 14 insertions(+), 12 deletions(-)

arch/powerpc/include/asm/kfence.h +2 -0

index fab124ada1c7..5975688d8de1 100644
@@ -15,6 +15,8 @@
 #define ARCH_FUNC_PREFIX "."
 #endif
 
+extern bool kfence_early_init;
+
 #ifdef CONFIG_KFENCE
 extern bool kfence_disabled;

arch/powerpc/mm/book3s64/radix_pgtable.c +0 ~~-12~~

index b0d927009af8..311e2112d782 100644
@@ -363,18 +363,6 @@ static int __meminit create_physical_mapping(unsigned long start,
 }
 
 #ifdef CONFIG_KFENCE
-static bool __ro_after_init kfence_early_init = !!CONFIG_KFENCE_SAMPLE_INTERVAL;
-
-static int __init parse_kfence_early_init(char *arg)
-{
-	int val;
-
-	if (get_option(&arg, &val))
-		kfence_early_init = !!val;
-	return 0;
-}
-early_param("kfence.sample_interval", parse_kfence_early_init);
-
 static inline phys_addr_t alloc_kfence_pool(void)
 {
 	phys_addr_t kfence_pool;

arch/powerpc/mm/init-common.c +12 -0

index 21131b96d209..259821a4db62 100644
@@ -33,6 +33,18 @@ bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP);
 bool disable_kuap = !IS_ENABLED(CONFIG_PPC_KUAP);
 #ifdef CONFIG_KFENCE
 bool __ro_after_init kfence_disabled;
+bool __ro_after_init kfence_early_init = !!CONFIG_KFENCE_SAMPLE_INTERVAL;
+static int __init parse_kfence_early_init(char *arg)
+{
+	int val;
+
+	if (get_option(&arg, &val))
+		kfence_early_init = !!val;
+	return 0;
+}
+early_param("kfence.sample_interval", parse_kfence_early_init);
+#else
+bool __ro_after_init kfence_early_init;
 #endif
 
 static int __init parse_nosmep(char *p)

Signature · 1 line

2.45.2

Christophe Leroy

2024-08-14 22:10 UTC #12 lore raw

Re: [RFC v1 09/10] book3s64/radix: Refactoring common kfence related functions

Le 31/07/2024 à 09:56, Ritesh Harjani (IBM) a écrit :

Quoted 60 lines

[Vous ne recevez pas souvent de courriers de [email protected]. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
Both radix and hash on book3s requires to detect if kfence early init is enabled or not. Hash needs to disable kfence if early init is not enabled because with kfence the linear map is mapped using PAGE_SIZE rather than 16M mapping.
We don't support multiple page sizes for slb entry used for kernel linear map in book3s64.
This patch refactors out the common functions required to detect kfence early init is enabled or not.
Signed-off-by Ritesh Harjani (IBM) <[email protected]>
---
  arch/powerpc/include/asm/kfence.h        |  2 ++
  arch/powerpc/mm/book3s64/radix_pgtable.c | 12 ------------
  arch/powerpc/mm/init-common.c            | 12 ++++++++++++
  3 files changed, 14 insertions(+), 12 deletions(-)
arch/powerpc/include/asm/kfence.h +2 -0
index fab124ada1c7..5975688d8de1 100644
@@ -15,6 +15,8 @@
  #define ARCH_FUNC_PREFIX "."
  #endif
 
+extern bool kfence_early_init;
+
  #ifdef CONFIG_KFENCE
  extern bool kfence_disabled;
 
arch/powerpc/mm/book3s64/radix_pgtable.c +0 ~~-12~~
index b0d927009af8..311e2112d782 100644
@@ -363,18 +363,6 @@ static int __meminit create_physical_mapping(unsigned long start,
  }
 
  #ifdef CONFIG_KFENCE
-static bool __ro_after_init kfence_early_init = !!CONFIG_KFENCE_SAMPLE_INTERVAL;
-
-static int __init parse_kfence_early_init(char *arg)
-{
-       int val;
-
-       if (get_option(&arg, &val))
-               kfence_early_init = !!val;
-       return 0;
-}
-early_param("kfence.sample_interval", parse_kfence_early_init);
-
  static inline phys_addr_t alloc_kfence_pool(void)
  {
         phys_addr_t kfence_pool;
arch/powerpc/mm/init-common.c +0 -0
index 21131b96d209..259821a4db62 100644

arch/powerpc/mm/init-common.c bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP); · line 37

Quoted 5 lines

arch/powerpc/mm/init-common.c +2 -0

@@ -33,6 +33,18 @@ bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP);
  bool disable_kuap = !IS_ENABLED(CONFIG_PPC_KUAP);
  #ifdef CONFIG_KFENCE
  bool __ro_after_init kfence_disabled;
+bool __ro_after_init kfence_early_init = !!CONFIG_KFENCE_SAMPLE_INTERVAL;
+static int __init parse_kfence_early_init(char *arg)

If I understand correctly, previously it was only for radix, now it is for every platform including PPC32 ?

arch/powerpc/mm/init-common.c bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP); · line 47

Quoted 10 lines

arch/powerpc/mm/init-common.c +10 -0

+{
+       int val;
+
+       if (get_option(&arg, &val))
+               kfence_early_init = !!val;
+       return 0;
+}
+early_param("kfence.sample_interval", parse_kfence_early_init);
+#else
+bool __ro_after_init kfence_early_init;

I don't understand, why do you need that in the #else case ?

Quoted 6 lines

arch/powerpc/mm/init-common.c +0 -1
  #endif
 
  static int __init parse_nosmep(char *p)
--
2.45.2

Ritesh Harjani (IBM)

author reviewer

2024-09-04 18:44 UTC #13 lore raw

Re: [RFC v1 09/10] book3s64/radix: Refactoring common kfence related functions

Quoted 61 lines · Christophe Leroy

Le 31/07/2024 à 09:56, Ritesh Harjani (IBM) a écrit :
Quoted 60 lines
[Vous ne recevez pas souvent de courriers de [email protected]. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
Both radix and hash on book3s requires to detect if kfence early init is enabled or not. Hash needs to disable kfence if early init is not enabled because with kfence the linear map is mapped using PAGE_SIZE rather than 16M mapping.
We don't support multiple page sizes for slb entry used for kernel linear map in book3s64.
This patch refactors out the common functions required to detect kfence early init is enabled or not.
Signed-off-by Ritesh Harjani (IBM) <[email protected]>
---
  arch/powerpc/include/asm/kfence.h        |  2 ++
  arch/powerpc/mm/book3s64/radix_pgtable.c | 12 ------------
  arch/powerpc/mm/init-common.c            | 12 ++++++++++++
  3 files changed, 14 insertions(+), 12 deletions(-)
arch/powerpc/include/asm/kfence.h +2 -0
index fab124ada1c7..5975688d8de1 100644
@@ -15,6 +15,8 @@
  #define ARCH_FUNC_PREFIX "."
  #endif
 
+extern bool kfence_early_init;
+
  #ifdef CONFIG_KFENCE
  extern bool kfence_disabled;
 
arch/powerpc/mm/book3s64/radix_pgtable.c +0 ~~-12~~
index b0d927009af8..311e2112d782 100644
@@ -363,18 +363,6 @@ static int __meminit create_physical_mapping(unsigned long start,
  }
 
  #ifdef CONFIG_KFENCE
-static bool __ro_after_init kfence_early_init = !!CONFIG_KFENCE_SAMPLE_INTERVAL;
-
-static int __init parse_kfence_early_init(char *arg)
-{
-       int val;
-
-       if (get_option(&arg, &val))
-               kfence_early_init = !!val;
-       return 0;
-}
-early_param("kfence.sample_interval", parse_kfence_early_init);
-
  static inline phys_addr_t alloc_kfence_pool(void)
  {
         phys_addr_t kfence_pool;
arch/powerpc/mm/init-common.c +0 -0
index 21131b96d209..259821a4db62 100644
@@ -33,6 +33,18 @@ bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP);

Quoted 9 lines

arch/powerpc/mm/init-common.c bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP); · line 37
Quoted 5 lines
arch/powerpc/mm/init-common.c +2 -0
  bool disable_kuap = !IS_ENABLED(CONFIG_PPC_KUAP);
  #ifdef CONFIG_KFENCE
  bool __ro_after_init kfence_disabled;
+bool __ro_after_init kfence_early_init = !!CONFIG_KFENCE_SAMPLE_INTERVAL;
+static int __init parse_kfence_early_init(char *arg)
If I understand correctly, previously it was only for radix, now it is for every platform including PPC32 ?

Ok. I see what you mean. Let me see how can I limit this cmdline parsing of kfence and/or special case kfence handling to book3s64 only.

Quoted 4 lines

Quoted 4 lines
arch/powerpc/mm/init-common.c +4 -0
+{
+       int val;
+
+       if (get_option(&arg, &val))

Quoted 9 lines

arch/powerpc/mm/init-common.c bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP); · line 47
Quoted 6 lines
arch/powerpc/mm/init-common.c +6 -0
+               kfence_early_init = !!val;
+       return 0;
+}
+early_param("kfence.sample_interval", parse_kfence_early_init);
+#else
+bool __ro_after_init kfence_early_init;
I don't understand, why do you need that in the #else case ?

Yes, I don't like it either. Let me clean this up. this was required in htab_init_page_sizes().

Thanks for pointing out.

-ritesh

Ritesh Harjani (IBM)

author reviewer

2024-07-31 08:04 UTC #14 lore raw

RFC 10/10 book3s64/hash: Disable kfence if not early init

In reply to Ritesh Harjani (IBM)

Enable kfence on book3s64 hash only when early init is enabled. This is because, kfence could cause the kernel linear map to be mapped at PAGE_SIZE level instead of 16M (which I guess we don't want).

Also currently there is no way to -
1. Make multiple page size entries for the SLB used for kernel linear map.
2. No easy way of getting the hash slot details after the page table mapping for kernel linear setup. So even if kfence allocate the pool in late init, we won't be able to get the hash slot details in kfence linear map.

Thus this patch disables kfence on hash if kfence early init is not enabled.

Signed-off-by Ritesh Harjani (IBM) <[email protected]>

---
 arch/powerpc/mm/book3s64/hash_utils.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

arch/powerpc/mm/book3s64/hash_utils.c +4 -1

index c66b9921fc7d..759dbcbf1483 100644
@@ -410,6 +410,8 @@ static phys_addr_t kfence_pool;
 
 static inline void hash_kfence_alloc_pool(void)
 {
+	if (!kfence_early_init)
+		goto err;
 
 	// allocate linear map for kfence within RMA region
 	linear_map_kf_hash_count = KFENCE_POOL_SIZE >> PAGE_SHIFT;

@@ -1074,7 +1076,8 @@ static void __init htab_init_page_sizes(void)
 	bool aligned = true;
 	init_hpte_page_sizes();
 
-	if (!debug_pagealloc_enabled_or_kfence()) {
+	if (!debug_pagealloc_enabled() &&
+	    !(IS_ENABLED(CONFIG_KFENCE) && kfence_early_init)) {
 		/*
 		 * Pick a size for the linear mapping. Currently, we only
 		 * support 16M, 1M and 4K which is the default

--
2.45.2

Christophe Leroy

2024-08-14 22:10 UTC #15 lore raw

Re: [RFC v1 10/10] book3s64/hash: Disable kfence if not early init

Le 31/07/2024 à 09:56, Ritesh Harjani (IBM) a écrit :

Quoted 36 lines

[Vous ne recevez pas souvent de courriers de [email protected]. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
Enable kfence on book3s64 hash only when early init is enabled. This is because, kfence could cause the kernel linear map to be mapped at PAGE_SIZE level instead of 16M (which I guess we don't want).
Also currently there is no way to -
1. Make multiple page size entries for the SLB used for kernel linear map.
2. No easy way of getting the hash slot details after the page table mapping for kernel linear setup. So even if kfence allocate the pool in late init, we won't be able to get the hash slot details in kfence linear map.
Thus this patch disables kfence on hash if kfence early init is not enabled.
Signed-off-by Ritesh Harjani (IBM) <[email protected]>
---
  arch/powerpc/mm/book3s64/hash_utils.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)
arch/powerpc/mm/book3s64/hash_utils.c +2 -0
index c66b9921fc7d..759dbcbf1483 100644
@@ -410,6 +410,8 @@ static phys_addr_t kfence_pool;
 
  static inline void hash_kfence_alloc_pool(void)
  {
+       if (!kfence_early_init)
+               goto err;
 
         // allocate linear map for kfence within RMA region
         linear_map_kf_hash_count = KFENCE_POOL_SIZE >> PAGE_SHIFT;

arch/powerpc/mm/book3s64/hash_utils.c static void __init htab_init_page_sizes(void) · line 1080

Quoted 6 lines

arch/powerpc/mm/book3s64/hash_utils.c +2 -1

@@ -1074,7 +1076,8 @@ static void __init htab_init_page_sizes(void)
         bool aligned = true;
         init_hpte_page_sizes();
 
-       if (!debug_pagealloc_enabled_or_kfence()) {
+       if (!debug_pagealloc_enabled() &&
+           !(IS_ENABLED(CONFIG_KFENCE) && kfence_early_init)) {

Looks complex, can we do simpler ?

Quoted 6 lines

arch/powerpc/mm/book3s64/hash_utils.c +0 -1

                 /*
                  * Pick a size for the linear mapping. Currently, we only
                  * support 16M, 1M and 4K which is the default
--

2.45.2

Ritesh Harjani (IBM)

author reviewer

2024-09-04 18:45 UTC #16 lore raw

Re: [RFC v1 10/10] book3s64/hash: Disable kfence if not early init

Quoted 40 lines · Christophe Leroy

Le 31/07/2024 à 09:56, Ritesh Harjani (IBM) a écrit :
Quoted 39 lines
[Vous ne recevez pas souvent de courriers de [email protected]. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
Enable kfence on book3s64 hash only when early init is enabled. This is because, kfence could cause the kernel linear map to be mapped at PAGE_SIZE level instead of 16M (which I guess we don't want).
Also currently there is no way to -
1. Make multiple page size entries for the SLB used for kernel linear map.
2. No easy way of getting the hash slot details after the page table mapping for kernel linear setup. So even if kfence allocate the pool in late init, we won't be able to get the hash slot details in kfence linear map.
Thus this patch disables kfence on hash if kfence early init is not enabled.
Signed-off-by Ritesh Harjani (IBM) <[email protected]>
---
  arch/powerpc/mm/book3s64/hash_utils.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)
arch/powerpc/mm/book3s64/hash_utils.c +2 -0
index c66b9921fc7d..759dbcbf1483 100644
@@ -410,6 +410,8 @@ static phys_addr_t kfence_pool;
 
  static inline void hash_kfence_alloc_pool(void)
  {
+       if (!kfence_early_init)
+               goto err;
 
         // allocate linear map for kfence within RMA region
         linear_map_kf_hash_count = KFENCE_POOL_SIZE >> PAGE_SHIFT;
@@ -1074,7 +1076,8 @@ static void __init htab_init_page_sizes(void)
         bool aligned = true;
         init_hpte_page_sizes();
 

Quoted 6 lines

arch/powerpc/mm/book3s64/hash_utils.c static void __init htab_init_page_sizes(void) · line 1080
Quoted 3 lines
arch/powerpc/mm/book3s64/hash_utils.c +2 -1
-       if (!debug_pagealloc_enabled_or_kfence()) {
+       if (!debug_pagealloc_enabled() &&
+           !(IS_ENABLED(CONFIG_KFENCE) && kfence_early_init)) {
Looks complex, can we do simpler ?

Yes, kfence_early_init anyway needs clean up. Will make it simpler.

Thanks for the review!

-ritesh