All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: mingo@kernel.org, tglx@linutronix.de, bp@alien8.de,
	dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
	jpoimboe@kernel.org, jbaron@akamai.com, ardb@kernel.org,
	linux-kernel@vger.kernel.org, erhard_f@mailbox.org,
	ndesaulniers@google.com, mhiramat@kernel.org,
	sandipan.das@amd.com
Subject: [PATCH v1.1 3/3] x86/static_call: Add support for Jcc tail-calls
Date: Thu, 26 Jan 2023 16:34:27 +0100	[thread overview]
Message-ID: <Y9Kdg9QjHkr9G5b5@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20230124100753.13608e1f@gandalf.local.home>

On Tue, Jan 24, 2023 at 10:07:53AM -0500, Steven Rostedt wrote:
> On Tue, 24 Jan 2023 14:06:49 +0100
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > > Just to confirm, as it's not clear if this is the static call site or one
> > > of the functions that is being called.  
> > 
> > Ah, you've not looked at enough asm then? ;-) Yes this is the static
> > call site, see the __SCT_ target (instruction at 0x35d).
> 
> Yeah, could you specify it a bit more in the change log such that those
> looking back at this don't have to have that requirement of staring at
> enough asm ;-)

How's this then?

---
Subject: x86/static_call: Add support for Jcc tail-calls
From: Peter Zijlstra <peterz@infradead.org>
Date: Fri Jan 20 16:40:33 CET 2023

Clang likes to create conditional tail calls like:

0000000000000350 <amd_pmu_add_event>:
350:       0f 1f 44 00 00          nopl   0x0(%rax,%rax,1) 351: R_X86_64_NONE      __fentry__-0x4
355:       48 83 bf 20 01 00 00 00         cmpq   $0x0,0x120(%rdi)
35d:       0f 85 00 00 00 00       jne    363 <amd_pmu_add_event+0x13>     35f: R_X86_64_PLT32     __SCT__amd_pmu_branch_add-0x4
363:       e9 00 00 00 00          jmp    368 <amd_pmu_add_event+0x18>     364: R_X86_64_PLT32     __x86_return_thunk-0x4

Where 0x35d is a static call site that's turned into a conditional
tail-call using the Jcc class of instructions.

Teach the in-line static call text patching about this.

Notably, since there is no conditional-ret, in that case patch the Jcc
to point at an empty stub function that does the ret -- or the return
thunk when needed.

Reported-by: "Erhard F." <erhard_f@mailbox.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 arch/x86/kernel/static_call.c |   50 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 3 deletions(-)

--- a/arch/x86/kernel/static_call.c
+++ b/arch/x86/kernel/static_call.c
@@ -9,6 +9,7 @@ enum insn_type {
 	NOP = 1,  /* site cond-call */
 	JMP = 2,  /* tramp / site tail-call */
 	RET = 3,  /* tramp / site cond-tail-call */
+	JCC = 4,
 };
 
 /*
@@ -25,12 +26,40 @@ static const u8 xor5rax[] = { 0x2e, 0x2e
 
 static const u8 retinsn[] = { RET_INSN_OPCODE, 0xcc, 0xcc, 0xcc, 0xcc };
 
+static u8 __is_Jcc(u8 *insn) /* Jcc.d32 */
+{
+	u8 ret = 0;
+
+	if (insn[0] == 0x0f) {
+		u8 tmp = insn[1];
+		if ((tmp & 0xf0) == 0x80)
+			ret = tmp;
+	}
+
+	return ret;
+}
+
+extern void __static_call_return(void);
+
+asm (".global __static_call_return\n\t"
+     ".type __static_call_return, @function\n\t"
+     ASM_FUNC_ALIGN "\n\t"
+     "__static_call_return:\n\t"
+     ANNOTATE_NOENDBR
+     ANNOTATE_RETPOLINE_SAFE
+     "ret; int3\n\t"
+     ".size __static_call_return, . - __static_call_return \n\t");
+
 static void __ref __static_call_transform(void *insn, enum insn_type type,
 					  void *func, bool modinit)
 {
 	const void *emulate = NULL;
 	int size = CALL_INSN_SIZE;
 	const void *code;
+	u8 op, buf[6];
+
+	if ((type == JMP || type == RET) && (op = __is_Jcc(insn)))
+		type = JCC;
 
 	switch (type) {
 	case CALL:
@@ -57,6 +86,20 @@ static void __ref __static_call_transfor
 		else
 			code = &retinsn;
 		break;
+
+	case JCC:
+		if (!func) {
+			func = __static_call_return;
+			if (cpu_feature_enabled(X86_FEATURE_RETHUNK))
+				func = x86_return_thunk;
+		}
+
+		buf[0] = 0x0f;
+		__text_gen_insn(buf+1, op, insn+1, func, 5);
+		code = buf;
+		size = 6;
+
+		break;
 	}
 
 	if (memcmp(insn, code, size) == 0)
@@ -68,9 +111,9 @@ static void __ref __static_call_transfor
 	text_poke_bp(insn, code, size, emulate);
 }
 
-static void __static_call_validate(void *insn, bool tail, bool tramp)
+static void __static_call_validate(u8 *insn, bool tail, bool tramp)
 {
-	u8 opcode = *(u8 *)insn;
+	u8 opcode = insn[0];
 
 	if (tramp && memcmp(insn+5, tramp_ud, 3)) {
 		pr_err("trampoline signature fail");
@@ -79,7 +122,8 @@ static void __static_call_validate(void
 
 	if (tail) {
 		if (opcode == JMP32_INSN_OPCODE ||
-		    opcode == RET_INSN_OPCODE)
+		    opcode == RET_INSN_OPCODE ||
+		    __is_Jcc(insn))
 			return;
 	} else {
 		if (opcode == CALL_INSN_OPCODE ||

  reply	other threads:[~2023-01-26 15:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-23 20:59 [PATCH 0/3] static_call/x86: Handle clang's conditional tail calls Peter Zijlstra
2023-01-23 20:59 ` [PATCH 1/3] x86/alternative: Introduce int3_emulate_jcc() Peter Zijlstra
2023-01-31 14:22   ` [tip: x86/alternatives] x86/alternatives: " tip-bot2 for Peter Zijlstra
2023-01-23 20:59 ` [PATCH 2/3] x86/alternative: Teach text_poke_bp() to patch Jcc.d32 instructions Peter Zijlstra
2023-01-31 14:22   ` [tip: x86/alternatives] x86/alternatives: " tip-bot2 for Peter Zijlstra
2023-01-23 20:59 ` [PATCH 3/3] x86/static_call: Add support for Jcc tail-calls Peter Zijlstra
2023-01-23 22:44   ` Steven Rostedt
2023-01-24 13:06     ` Peter Zijlstra
2023-01-24 15:07       ` Steven Rostedt
2023-01-26 15:34         ` Peter Zijlstra [this message]
2023-01-26 18:14           ` [PATCH v1.1 " Nick Desaulniers
2023-02-06 16:07             ` Steven Rostedt
2023-01-31 14:22           ` [tip: x86/alternatives] " tip-bot2 for Peter Zijlstra
2023-02-08 22:36 ` [PATCH 0/3] static_call/x86: Handle clang's conditional tail calls Nathan Chancellor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y9Kdg9QjHkr9G5b5@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=ardb@kernel.org \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=erhard_f@mailbox.org \
    --cc=hpa@zytor.com \
    --cc=jbaron@akamai.com \
    --cc=jpoimboe@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=rostedt@goodmis.org \
    --cc=sandipan.das@amd.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.