Re: [PATCH] powerpc/radix: Move some functions into #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE

[Christophe Leroy]

Le 14/08/2023 à 08:24, Nicholas Piggin a écrit :
> On Wed Aug 9, 2023 at 6:01 PM AEST, Christophe Leroy wrote:
>> With skiboot_defconfig, Clang reports:
>>
>>    CC      arch/powerpc/mm/book3s64/radix_tlb.o
>> arch/powerpc/mm/book3s64/radix_tlb.c:419:20: error: unused function '_tlbie_pid_lpid' [-Werror,-Wunused-function]
>> static inline void _tlbie_pid_lpid(unsigned long pid, unsigned long lpid,
>>                     ^
>> arch/powerpc/mm/book3s64/radix_tlb.c:663:20: error: unused function '_tlbie_va_range_lpid' [-Werror,-Wunused-function]
>> static inline void _tlbie_va_range_lpid(unsigned long start, unsigned long end,
>>                     ^
>>
>> This is because those functions are only called from functions
>> enclosed in a #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
>>
>> Move below functions inside that #ifdef
>> * __tlbie_pid_lpid(unsigned long pid,
>> * __tlbie_va_lpid(unsigned long va, unsigned long pid,
>> * fixup_tlbie_pid_lpid(unsigned long pid, unsigned long lpid)
>> * _tlbie_pid_lpid(unsigned long pid, unsigned long lpid,
>> * fixup_tlbie_va_range_lpid(unsigned long va,
>> * __tlbie_va_range_lpid(unsigned long start, unsigned long end,
>> * _tlbie_va_range_lpid(unsigned long start, unsigned long end,
> 
> Thanks for doing this. Functions vaguely belong where they are, which
> makes it slightly annoying to move them. Is it also annoying to add
> ifdefs for each one where they are?

Looking at it once more, we can even move all those functions out in a 
separate file that would only be built when 
CONFIG_KVM_BOOK3S_HV_POSSIBLE is selected. The only dependency with 
other part of the file is the static tlb_single_page_flush_ceiling, 
which can easily be made global.

In principle we try to avoid #ifdefs in C-files.

Why would we want to keep those functions at the current place and 
spreat several more #ifdefs inside the file ?

Christophe