Re: [PATCH] powerpc/ftrace: fix syscall tracing on PPC64_ELF_ABI_V1

[Christophe Leroy]

Le 06/12/2022 à 15:38, Mathieu Desnoyers a écrit :
> On 2022-12-05 17:50, Michael Ellerman wrote:
>> Michael Jeanson <mjeanson@xxxxxxxxxxxx> writes:
>>> On 2022-12-05 15:11, Michael Jeanson wrote:
>>>>>>> Michael Jeanson <mjeanson@xxxxxxxxxxxx> writes:
>>>>>>>> In v5.7 the powerpc syscall entry/exit logic was rewritten in C, on
>>>>>>>> PPC64_ELF_ABI_V1 this resulted in the symbols in the syscall table
>>>>>>>> changing from their dot prefixed variant to the non-prefixed ones.
>>>>>>>>
>>>>>>>> Since ftrace prefixes a dot to the syscall names when matching 
>>>>>>>> them to
>>>>>>>> build its syscall event list, this resulted in no syscall events 
>>>>>>>> being
>>>>>>>> available.
>>>>>>>>
>>>>>>>> Remove the PPC64_ELF_ABI_V1 specific version of
>>>>>>>> arch_syscall_match_sym_name to have the same behavior across all 
>>>>>>>> powerpc
>>>>>>>> variants.
>>>>>>>
>>>>>>> This doesn't seem to work for me.
>>>>>>>
>>>>>>> Event with it applied I still don't see anything in
>>>>>>> /sys/kernel/debug/tracing/events/syscalls
>>>>>>>
>>>>>>> Did we break it in some other way recently?
>>>>>>>
>>>>>>> cheers
>>>
>>> I did some further testing, my config also enabled KALLSYMS_ALL, when 
>>> I remove
>>> it there is indeed no syscall events.
>>
>> Aha, OK that explains it I guess.
>>
>> I was using ppc64_guest_defconfig which has ABI_V1 and FTRACE_SYSCALLS,
>> but does not have KALLSYMS_ALL. So I guess there's some other bug
>> lurking in there.
> 
> I don't have the setup handy to validate it, but I suspect it is caused 
> by the way scripts/kallsyms.c:symbol_valid() checks whether a symbol 
> entry needs to be integrated into the assembler output when 
> --all-symbols is not specified. It only keeps symbols which addresses 
> are in the text range. On PPC64_ELF_ABI_V1, this means only the 
> dot-prefixed symbols will be kept (those point to the function begin), 
> leaving out the non-dot-prefixed symbols (those point to the function 
> descriptors).
> 
> So I see two possible solutions there: either we ensure that 
> FTRACE_SYSCALLS selects KALLSYMS_ALL on PPC64_ELF_ABI_V1, or we modify 
> scripts/kallsyms.c:symbol_valid() to also include function descriptor 
> symbols. This would mean accepting symbols pointing into the .opd ELF 
> section.
> 
> IMHO the second option would be better because it does not increase the 
> kernel image size as much as KALLSYMS_ALL.
> 

Yes, seems to be the best solution.

Maybe the only thing to do is to add a new range to text_ranges, 
something like (untested):

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 03fa07ad45d9..decf31c497f5 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -64,6 +64,7 @@ static unsigned long long relative_base;
  static struct addr_range text_ranges[] = {
  	{ "_stext",     "_etext"     },
  	{ "_sinittext", "_einittext" },
+	{ "__start_opd", "__end_opd" },
  };
  #define text_range_text     (&text_ranges[0])
  #define text_range_inittext (&text_ranges[1])

---
Christophe