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

From: Ritesh Harjani (IBM) <[email-masked]>
Date: Wed, 04 Sep 2024 23:57:54 +0530

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

Christophe Leroy <[email-masked]> writes:

> Le 31/07/2024 à 09:56, Ritesh Harjani (IBM) a écrit :
>> [Vous ne recevez pas souvent de courriers de [email-masked]. 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-masked]>
>> ---
>>   arch/powerpc/include/asm/kfence.h     |  5 +++++
>>   arch/powerpc/mm/book3s64/hash_utils.c | 16 +++++++++++-----
>>   2 files changed, 16 insertions(+), 5 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/asm/kfence.h b/arch/powerpc/include/asm/kfence.h
>> index fab124ada1c7..f3a9476a71b3 100644
>> --- a/arch/powerpc/include/asm/kfence.h
>> +++ b/arch/powerpc/include/asm/kfence.h
>> @@ -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
>
> Avoid #ifdefs whenever possible. Here you can do:
>
> 	if (IS_ENABLED(CONFIG_PPC64) && !radix_enabled())
> 		return false;
>

Sure. I will change it.

>>          return !kfence_disabled;
>>   }
>>   #endif
>> diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
>> index 01c3b4b65241..1a1b50735fa0 100644
>> --- a/arch/powerpc/mm/book3s64/hash_utils.c
>> +++ b/arch/powerpc/mm/book3s64/hash_utils.c
>> @@ -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)
>
> Use #ifdef
>

Sure. Thanks!

-ritesh