Re: [PATCH v22 19/20] ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Mon, 3 Feb 2025 22:33:48 +0100
Gabriel de Perthuis <g2p.code@gmail.com> wrote:

> Hello,
> 
> I got errors building Linux 6.14-rc1 that were solved by reverting this 
> patch and the one after (19/20 and 20/20).
> 
> Errors look like:
> 
> In file included from ./arch/x86/include/asm/asm-prototypes.h:2,
>                   from <stdin>:3:
> ./arch/x86/include/asm/ftrace.h: In function 'arch_ftrace_get_symaddr':
> ./arch/x86/include/asm/ftrace.h:46:21: error: implicit declaration of 
> function 'get_kernel_nofault' [-Wimplicit-function-declaration]
>     46 |                 if (get_kernel_nofault(instr, (u32 *)(fentry_ip 
> - ENDBR_INSN_SIZE)))
>        | ^~~~~~~~~~~~~~~~~~
> 
> Will send .config on request if needed.


Thanks for the report!

-------<arch/x86/include/asm/asm-prototypes.h>
/* SPDX-License-Identifier: GPL-2.0 */
#include <asm/ftrace.h>
#include <linux/uaccess.h>
-----

Ah, that's why... get_kernel_nofault() is defined in linux/uaccess.h.
I also found that is_endbr() is in asm/ibt.h.

Can you try this?

Thank you,

diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index f9cb4d07df58..d24d7c71253f 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -16,24 +16,9 @@
 # include <asm/ibt.h>
 /* Add offset for endbr64 if IBT enabled */
 # define FTRACE_MCOUNT_MAX_OFFSET	ENDBR_INSN_SIZE
-#endif
-
-#ifdef CONFIG_DYNAMIC_FTRACE
-#define ARCH_SUPPORTS_FTRACE_OPS 1
-#endif
-
-#ifndef __ASSEMBLY__
-extern void __fentry__(void);
-
-static inline unsigned long ftrace_call_adjust(unsigned long addr)
-{
-	/*
-	 * addr is the address of the mcount call instruction.
-	 * recordmcount does the necessary offset calculation.
-	 */
-	return addr;
-}
 
+#include <linux/uaccess.h>
+/* This only supports fentry based ftrace. */
 static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
 {
 #ifdef CONFIG_X86_KERNEL_IBT
@@ -55,6 +40,24 @@ static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
 }
 #define ftrace_get_symaddr(fentry_ip)	arch_ftrace_get_symaddr(fentry_ip)
 
+#endif
+
+#ifdef CONFIG_DYNAMIC_FTRACE
+#define ARCH_SUPPORTS_FTRACE_OPS 1
+#endif
+
+#ifndef __ASSEMBLY__
+extern void __fentry__(void);
+
+static inline unsigned long ftrace_call_adjust(unsigned long addr)
+{
+	/*
+	 * addr is the address of the mcount call instruction.
+	 * recordmcount does the necessary offset calculation.
+	 */
+	return addr;
+}
+
 #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
 
 #include <linux/ftrace_regs.h>


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>