2023-05-16 14:18:13

by Josh Poimboeuf

[permalink] [raw]
Subject: [PATCH] Revert "x86/orc: Make it callthunk aware"

Commit 396e0b8e09e8 ("x86/orc: Make it callthunk aware") attempted to
deal with the fact that function prefix code didn't have ORC coverage.
However, it didn't work as advertised. Use of the "null" ORC entry just
caused affected unwinds to end early.

The root cause has now been fixed with commit 5743654f5e2e ("objtool:
Generate ORC data for __pfx code").

Revert most of commit 396e0b8e09e8 ("x86/orc: Make it callthunk aware").
The is_callthunk() function remains as it's now used by other code.

Signed-off-by: Josh Poimboeuf <[email protected]>
---
arch/x86/include/asm/alternative.h | 5 -----
arch/x86/kernel/callthunks.c | 2 +-
arch/x86/kernel/unwind_orc.c | 21 +--------------------
3 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index d7da28fada87..6c15a622ad60 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -113,7 +113,6 @@ extern void callthunks_patch_builtin_calls(void);
extern void callthunks_patch_module_calls(struct callthunk_sites *sites,
struct module *mod);
extern void *callthunks_translate_call_dest(void *dest);
-extern bool is_callthunk(void *addr);
extern int x86_call_depth_emit_accounting(u8 **pprog, void *func);
#else
static __always_inline void callthunks_patch_builtin_calls(void) {}
@@ -124,10 +123,6 @@ static __always_inline void *callthunks_translate_call_dest(void *dest)
{
return dest;
}
-static __always_inline bool is_callthunk(void *addr)
-{
- return false;
-}
static __always_inline int x86_call_depth_emit_accounting(u8 **pprog,
void *func)
{
diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c
index 22ab13966427..8e0a9b637e23 100644
--- a/arch/x86/kernel/callthunks.c
+++ b/arch/x86/kernel/callthunks.c
@@ -293,7 +293,7 @@ void *callthunks_translate_call_dest(void *dest)
return target ? : dest;
}

-bool is_callthunk(void *addr)
+static bool is_callthunk(void *addr)
{
unsigned int tmpl_size = SKL_TMPL_SIZE;
void *tmpl = skl_call_thunk_template;
diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 3ac50b7298d1..183bc20f610e 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -136,21 +136,6 @@ static struct orc_entry null_orc_entry = {
.type = ORC_TYPE_CALL
};

-#ifdef CONFIG_CALL_THUNKS
-static struct orc_entry *orc_callthunk_find(unsigned long ip)
-{
- if (!is_callthunk((void *)ip))
- return NULL;
-
- return &null_orc_entry;
-}
-#else
-static struct orc_entry *orc_callthunk_find(unsigned long ip)
-{
- return NULL;
-}
-#endif
-
/* Fake frame pointer entry -- used as a fallback for generated code */
static struct orc_entry orc_fp_entry = {
.type = ORC_TYPE_CALL,
@@ -203,11 +188,7 @@ static struct orc_entry *orc_find(unsigned long ip)
if (orc)
return orc;

- orc = orc_ftrace_find(ip);
- if (orc)
- return orc;
-
- return orc_callthunk_find(ip);
+ return orc_ftrace_find(ip);
}

#ifdef CONFIG_MODULES
--
2.40.1



Subject: [tip: objtool/core] Revert "x86/orc: Make it callthunk aware"

The following commit has been merged into the objtool/core branch of tip:

Commit-ID: 020126239b8f376ed2f0bef9dc07d0b280a0b7f5
Gitweb: https://git.kernel.org/tip/020126239b8f376ed2f0bef9dc07d0b280a0b7f5
Author: Josh Poimboeuf <[email protected]>
AuthorDate: Tue, 16 May 2023 06:56:46 -07:00
Committer: Josh Poimboeuf <[email protected]>
CommitterDate: Wed, 07 Jun 2023 09:48:57 -07:00

Revert "x86/orc: Make it callthunk aware"

Commit 396e0b8e09e8 ("x86/orc: Make it callthunk aware") attempted to
deal with the fact that function prefix code didn't have ORC coverage.
However, it didn't work as advertised. Use of the "null" ORC entry just
caused affected unwinds to end early.

The root cause has now been fixed with commit 5743654f5e2e ("objtool:
Generate ORC data for __pfx code").

Revert most of commit 396e0b8e09e8 ("x86/orc: Make it callthunk aware").
The is_callthunk() function remains as it's now used by other code.

Link: https://lore.kernel.org/r/a05b916ef941da872cbece1ab3593eceabd05a79.1684245404.git.jpoimboe@kernel.org
Signed-off-by: Josh Poimboeuf <[email protected]>
---
arch/x86/include/asm/alternative.h | 5 -----
arch/x86/kernel/callthunks.c | 2 +-
arch/x86/kernel/unwind_orc.c | 21 +--------------------
3 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index d7da28f..6c15a62 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -113,7 +113,6 @@ extern void callthunks_patch_builtin_calls(void);
extern void callthunks_patch_module_calls(struct callthunk_sites *sites,
struct module *mod);
extern void *callthunks_translate_call_dest(void *dest);
-extern bool is_callthunk(void *addr);
extern int x86_call_depth_emit_accounting(u8 **pprog, void *func);
#else
static __always_inline void callthunks_patch_builtin_calls(void) {}
@@ -124,10 +123,6 @@ static __always_inline void *callthunks_translate_call_dest(void *dest)
{
return dest;
}
-static __always_inline bool is_callthunk(void *addr)
-{
- return false;
-}
static __always_inline int x86_call_depth_emit_accounting(u8 **pprog,
void *func)
{
diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c
index 22ab139..8e0a9b6 100644
--- a/arch/x86/kernel/callthunks.c
+++ b/arch/x86/kernel/callthunks.c
@@ -293,7 +293,7 @@ void *callthunks_translate_call_dest(void *dest)
return target ? : dest;
}

-bool is_callthunk(void *addr)
+static bool is_callthunk(void *addr)
{
unsigned int tmpl_size = SKL_TMPL_SIZE;
void *tmpl = skl_call_thunk_template;
diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 7891727..4ee16b0 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -183,21 +183,6 @@ static struct orc_entry null_orc_entry = {
.type = ORC_TYPE_CALL
};

-#ifdef CONFIG_CALL_THUNKS
-static struct orc_entry *orc_callthunk_find(unsigned long ip)
-{
- if (!is_callthunk((void *)ip))
- return NULL;
-
- return &null_orc_entry;
-}
-#else
-static struct orc_entry *orc_callthunk_find(unsigned long ip)
-{
- return NULL;
-}
-#endif
-
/* Fake frame pointer entry -- used as a fallback for generated code */
static struct orc_entry orc_fp_entry = {
.type = ORC_TYPE_CALL,
@@ -250,11 +235,7 @@ static struct orc_entry *orc_find(unsigned long ip)
if (orc)
return orc;

- orc = orc_ftrace_find(ip);
- if (orc)
- return orc;
-
- return orc_callthunk_find(ip);
+ return orc_ftrace_find(ip);
}

#ifdef CONFIG_MODULES