Re: [PATCH 3/6] s390: Add ftrace_stub_graph

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2088 bytes --]

> diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S
> index 1fec370fecf48..97e9d6bdc0174 100644
> --- a/arch/s390/kernel/mcount.S
> +++ b/arch/s390/kernel/mcount.S
> @@ -4,6 +4,7 @@
>   *
>   */
>
> +#include <linux/cfi_types.h>
>  #include <linux/linkage.h>
>  #include <asm/asm-offsets.h>
>  #include <asm/ftrace.h>

[ ... ]

> @@ -34,10 +35,14 @@
>
>  	.section .kprobes.text, "ax"
>
> -SYM_FUNC_START(ftrace_stub)
> +SYM_TYPED_FUNC_START(ftrace_stub)
>  	BR_EX	%r14
>  SYM_FUNC_END(ftrace_stub)
>
> +SYM_TYPED_FUNC_START(ftrace_stub_graph)
> +	BR_EX	%r14
> +SYM_FUNC_END(ftrace_stub_graph)

Will this link when CONFIG_CFI_CLANG=y and CONFIG_FUNCTION_GRAPH_TRACER=n?

When CFI is enabled, the SYM_TYPED_FUNC_START() macro emits a reference to
__kcfi_typeid_ftrace_stub_graph. However, this symbol is only generated if
ftrace_stub_graph is declared in C code, which only happens when
CONFIG_FUNCTION_GRAPH_TRACER is enabled in kernel/trace/fgraph.c.

Both the RISC-V reference implementation (commit f3a0c23f2539) and the x86
fix (commit aa69f814920d8) wrap ftrace_stub_graph inside:

  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
  SYM_TYPED_FUNC_START(ftrace_stub_graph)
  	BR_EX	%r14
  SYM_FUNC_END(ftrace_stub_graph)
  #endif

From commit aa69f814920d8:

  "When CONFIG_FUNCTION_GRAPH_TRACER is disabled,
  __kcfi_typeid_ftrace_stub_graph is missing, causing a link failure:
  ld.lld: error: undefined symbol: __kcfi_typeid_ftrace_stub_graph
  referenced by arch/x86/kernel/ftrace_64.o:(__cfi_ftrace_stub_graph) in
  archive vmlinux.a"

Should the s390 implementation include the CONFIG_FUNCTION_GRAPH_TRACER
guard?

> +
>  SYM_CODE_START(ftrace_stub_direct_tramp)
>  	lgr	%r1, %r0
>  	BR_EX	%r1

This looks like a bug fix. Should this include:

  Fixes: 883bbbffa5a4 ("ftrace,kcfi: Separate ftrace_stub() and ftrace_stub_graph()")


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30100858043