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

From: Ritesh Harjani (IBM) <[email-masked]>
Date: Thu, 05 Sep 2024 00:04:44 +0530

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 ]
>> 
>> 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-masked]>
>> ---
>>   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(-)
>> 
>> diff --git a/arch/powerpc/include/asm/kfence.h b/arch/powerpc/include/asm/kfence.h
>> index fab124ada1c7..5975688d8de1 100644
>> --- a/arch/powerpc/include/asm/kfence.h
>> +++ b/arch/powerpc/include/asm/kfence.h
>> @@ -15,6 +15,8 @@
>>   #define ARCH_FUNC_PREFIX "."
>>   #endif
>> 
>> +extern bool kfence_early_init;
>> +
>>   #ifdef CONFIG_KFENCE
>>   extern bool kfence_disabled;
>> 
>> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
>> index b0d927009af8..311e2112d782 100644
>> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
>> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
>> @@ -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;
>> diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
>> index 21131b96d209..259821a4db62 100644
>> --- a/arch/powerpc/mm/init-common.c
>> +++ b/arch/powerpc/mm/init-common.c
>> @@ -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 ?
>

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.

>> +{
>> +       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 ?
>

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