Re: [RFC PATCH 1/7] static_call/x86: Add __static_call_returnl0()

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Nov 10, 2020 at 10:55:15AM +0100, Peter Zijlstra wrote:
> On Tue, Nov 10, 2020 at 01:56:03AM +0100, Frederic Weisbecker wrote:
> 
> > [fweisbec: s/disp16/data16, integrate into text_get_insn(), elaborate
> >  comment on the resulting insn, emulate on int3 trap, provide validation,
> >  uninline __static_call_return0() for HAVE_STATIC_CALL]

> Why did you add full emulation of this? The patch I send to you used the
> text_poke_bp(.emulate) argument to have it emulate an actual call to the
> out-of-line version of that function.
> 
> That should work fine and is a lot less code.

For reference; the below is what I send you. Actually doing the
__static_call_return0() call while we poke the magic XOR instruction is
much simpler.

---
Subject: static_call/x86: Add __static_call_return0
From: Peter Zijlstra <peterz@infradead.org>
Date: Mon Oct 12 11:43:32 CEST 2020

Provide a stub function that return 0 and wire up the static call site
patching to replace the CALL with a single 5 byte instruction that
clears %RAX, the return value register.

The function can be cast to any function pointer type that has a
single %RAX return (including pointers).

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kernel/static_call.c |   11 ++++++++++-
 include/linux/static_call.h   |    6 ++++++
 kernel/static_call.c          |    5 +++++
 3 files changed, 21 insertions(+), 1 deletion(-)

--- a/arch/x86/kernel/static_call.c
+++ b/arch/x86/kernel/static_call.c
@@ -13,12 +13,21 @@ enum insn_type {
 
 static void __ref __static_call_transform(void *insn, enum insn_type type, void *func)
 {
+	/*
+	 * disp16 disp16 xorq %rax, %rax - a single 5 byte instruction that clears %rax
+	 */
+	static const u8 ret0[5] = { 0x66, 0x66, 0x48, 0x31, 0xc0 };
 	int size = CALL_INSN_SIZE;
+	const void *emulate = NULL;
 	const void *code;
 
 	switch (type) {
 	case CALL:
 		code = text_gen_insn(CALL_INSN_OPCODE, insn, func);
+		if (func == &__static_call_return0) {
+			emulate = code;
+			code = ret0;
+		}
 		break;
 
 	case NOP:
@@ -41,7 +50,7 @@ static void __ref __static_call_transfor
 	if (unlikely(system_state == SYSTEM_BOOTING))
 		return text_poke_early(insn, code, size);
 
-	text_poke_bp(insn, code, size, NULL);
+	text_poke_bp(insn, code, size, emulate);
 }
 
 static void __static_call_validate(void *insn, bool tail)
--- a/include/linux/static_call.h
+++ b/include/linux/static_call.h
@@ -136,6 +136,8 @@ extern void arch_static_call_transform(v
 
 #ifdef CONFIG_HAVE_STATIC_CALL_INLINE
 
+extern long __static_call_return0(void);
+
 extern int __init static_call_init(void);
 
 struct static_call_mod {
@@ -187,6 +189,8 @@ extern int static_call_text_reserved(voi
 
 #elif defined(CONFIG_HAVE_STATIC_CALL)
 
+static inline long __static_call_return0(void) { return 0; }
+
 static inline int static_call_init(void) { return 0; }
 
 struct static_call_key {
@@ -234,6 +238,8 @@ static inline int static_call_text_reser
 
 #else /* Generic implementation */
 
+static inline long __static_call_return0(void) { return 0; }
+
 static inline int static_call_init(void) { return 0; }
 
 struct static_call_key {
--- a/kernel/static_call.c
+++ b/kernel/static_call.c
@@ -438,6 +438,11 @@ int __init static_call_init(void)
 }
 early_initcall(static_call_init);
 
+long __static_call_return0(void)
+{
+	return 0;
+}
+
 #ifdef CONFIG_STATIC_CALL_SELFTEST
 
 static int func_a(int x)