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

[Christophe Leroy]

Le 05/12/2022 à 19:19, Michael Jeanson a écrit :
> [Vous ne recevez pas souvent de courriers de mjeanson@xxxxxxxxxxxx. 
> Découvrez pourquoi ceci est important à 
> https://aka.ms/LearnAboutSenderIdentification ]
> 
> On 2022-12-05 00:34, Michael Ellerman 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've just tried this change on top of v6.1-rc8 in qemu with a base 
> config of
> 'corenet32_smp_defconfig' and these options on top:
> 
> CONFIG_FTRACE=y
> CONFIG_FTRACE_SYSCALLS=y
> 
> And I can trace syscalls with ftrace.
> 
> What kernel tree and config are you using?

If you are using a ppc32 config, CONFIG_PPC64_ELF_ABI_V1 won't be set, 
so it doesn't matter whether this change is there or not.

You should try corenet64_smp_defconfig if you want 
CONFIG_PPC64_ELF_ABI_V1 to be set.

You can also use ppc64_defconfig, that's a different platform but it 
also has CONFIG_PPC64_ELF_ABI_V1.

Christophe

> 
> Thanks for looking into this.
> 
>>
>>
>>> Fixes: 68b34588e202 ("powerpc/64/sycall: Implement syscall entry/exit 
>>> logic in C")
>>> Cc: stable@xxxxxxxxxxxxxxx # v5.7+
>>> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
>>> Cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
>>> Cc: Mark Rutland <mark.rutland@xxxxxxx>
>>> Cc: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
>>> Cc: Nicholas Piggin <npiggin@xxxxxxxxx>
>>> Cc: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
>>> Cc: Michal Suchanek <msuchanek@xxxxxxx>
>>> Cc: linuxppc-dev@xxxxxxxxxxxxxxxx
>>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>>> Signed-off-by: Michael Jeanson <mjeanson@xxxxxxxxxxxx>
>>> Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
>>> ---
>>>   arch/powerpc/include/asm/ftrace.h | 12 ------------
>>>   1 file changed, 12 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/ftrace.h 
>>> b/arch/powerpc/include/asm/ftrace.h
>>> index 3cee7115441b..e3d1f377bc5b 100644
>>> --- a/arch/powerpc/include/asm/ftrace.h
>>> +++ b/arch/powerpc/include/asm/ftrace.h
>>> @@ -64,17 +64,6 @@ void ftrace_graph_func(unsigned long ip, unsigned 
>>> long parent_ip,
>>>    * those.
>>>    */
>>>   #define ARCH_HAS_SYSCALL_MATCH_SYM_NAME
>>> -#ifdef CONFIG_PPC64_ELF_ABI_V1
>>> -static inline bool arch_syscall_match_sym_name(const char *sym, 
>>> const char *name)
>>> -{
>>> -    /* We need to skip past the initial dot, and the __se_sys alias */
>>> -    return !strcmp(sym + 1, name) ||
>>> -            (!strncmp(sym, ".__se_sys", 9) && !strcmp(sym + 6, 
>>> name)) ||
>>> -            (!strncmp(sym, ".ppc_", 5) && !strcmp(sym + 5, name + 
>>> 4)) ||
>>> -            (!strncmp(sym, ".ppc32_", 7) && !strcmp(sym + 7, name + 
>>> 4)) ||
>>> -            (!strncmp(sym, ".ppc64_", 7) && !strcmp(sym + 7, name + 
>>> 4));
>>> -}
>>> -#else
>>>   static inline bool arch_syscall_match_sym_name(const char *sym, 
>>> const char *name)
>>>   {
>>>      return !strcmp(sym, name) ||
>>> @@ -83,7 +72,6 @@ static inline bool 
>>> arch_syscall_match_sym_name(const char *sym, const char *name
>>>              (!strncmp(sym, "ppc32_", 6) && !strcmp(sym + 6, name + 
>>> 4)) ||
>>>              (!strncmp(sym, "ppc64_", 6) && !strcmp(sym + 6, name + 4));
>>>   }
>>> -#endif /* CONFIG_PPC64_ELF_ABI_V1 */
>>>   #endif /* CONFIG_FTRACE_SYSCALLS */
>>>
>>>   #if defined(CONFIG_PPC64) && defined(CONFIG_FUNCTION_TRACER)
>>> -- 
>>> 2.34.1
>