Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Nadav Amit <namit@vmware.com>
To:     Peter Zijlstra <peterz@infradead.org>
CC:     the arch/x86 maintainers <x86@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Ard Biesheuvel <ard.biesheuvel@linaro.org>,
        Andy Lutomirski <luto@kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ingo Molnar <mingo@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Jason Baron <jbaron@akamai.com>, Jiri Kosina <jkosina@suse.cz>,
        David Laight <David.Laight@ACULAB.COM>,
        Borislav Petkov <bp@alien8.de>,
        Julia Cartwright <julia@ni.com>, Jessica Yu <jeyu@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        Edward Cree <ecree@solarflare.com>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [PATCH 13/15] x86/static_call: Add inline static call
 implementation for x86-64
Thread-Topic: [PATCH 13/15] x86/static_call: Add inline static call
 implementation for x86-64
Thread-Index: AQHVG6HbUZEsVsjdrUOVxRZQdAn/WKaPsoOA
Date:   Fri, 7 Jun 2019 05:50:22 +0000
Message-ID: <47AED7D2-4727-4EA1-84DB-4B119673A4FA@vmware.com>
References: <20190605130753.327195108@infradead.org>
 <20190605131945.313688119@infradead.org>
In-Reply-To: <20190605131945.313688119@infradead.org>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: vmware.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3293540FC5084F4FA3C147479223B19A@namprd05.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 741d5cb7-72bc-469f-1e3e-08d6eb0c07da
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Jun 2019 05:50:22.1564
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: namit@vmware.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR05MB5570
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> On Jun 5, 2019, at 6:08 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>=20
> From: Josh Poimboeuf <jpoimboe@redhat.com>
>=20
> Add the inline static call implementation for x86-64.  For each key, a
> temporary trampoline is created, named __static_call_tramp_<key>.  The
> trampoline has an indirect jump to the destination function.
>=20
> Objtool uses the trampoline naming convention to detect all the call
> sites.  It then annotates those call sites in the .static_call_sites
> section.
>=20
> During boot (and module init), the call sites are patched to call
> directly into the destination function.  The temporary trampoline is
> then no longer used.
>=20
> Cc: x86@kernel.org
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Julia Cartwright <julia@ni.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jason Baron <jbaron@akamai.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: David Laight <David.Laight@ACULAB.COM>
> Cc: Jessica Yu <jeyu@kernel.org>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Link: https://nam04.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2=
Flkml.kernel.org%2Fr%2F62188c62f6dda49ca2e20629ee8e5a62a6c0b500.1543200841.=
git.jpoimboe%40redhat.com&amp;data=3D02%7C01%7Cnamit%40vmware.com%7C3a349bb=
2a7e042ef9d9d08d6e9b8fc2d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C6369=
53378066316039&amp;sdata=3DJ%2BsCYwRi8GpP5GrJaLo8nM5jN2KNZlfwq7RDuKok%2FmE%=
3D&amp;reserved=3D0
> ---
> arch/x86/Kconfig                                |    3=20
> arch/x86/include/asm/static_call.h              |   28 ++++-
> arch/x86/kernel/asm-offsets.c                   |    6 +
> arch/x86/kernel/static_call.c                   |   12 +-
> include/linux/static_call.h                     |    2=20
> tools/objtool/Makefile                          |    3=20
> tools/objtool/check.c                           |  125 ++++++++++++++++++=
+++++-
> tools/objtool/check.h                           |    2=20
> tools/objtool/elf.h                             |    1=20
> tools/objtool/include/linux/static_call_types.h |   19 +++
> tools/objtool/sync-check.sh                     |    1=20
> 11 files changed, 193 insertions(+), 9 deletions(-)
> create mode 100644 tools/objtool/include/linux/static_call_types.h
>=20
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -199,6 +199,7 @@ config X86
> 	select HAVE_STACKPROTECTOR		if CC_HAS_SANE_STACKPROTECTOR
> 	select HAVE_STACK_VALIDATION		if X86_64
> 	select HAVE_STATIC_CALL
> +	select HAVE_STATIC_CALL_INLINE		if HAVE_STACK_VALIDATION
> 	select HAVE_RSEQ
> 	select HAVE_SYSCALL_TRACEPOINTS
> 	select HAVE_UNSTABLE_SCHED_CLOCK
> @@ -213,6 +214,7 @@ config X86
> 	select RTC_MC146818_LIB
> 	select SPARSE_IRQ
> 	select SRCU
> +	select STACK_VALIDATION			if HAVE_STACK_VALIDATION && (HAVE_STATIC_CALL=
_INLINE || RETPOLINE)
> 	select SYSCTL_EXCEPTION_TRACE
> 	select THREAD_INFO_IN_TASK
> 	select USER_STACKTRACE_SUPPORT
> @@ -439,7 +441,6 @@ config GOLDFISH
> config RETPOLINE
> 	bool "Avoid speculative indirect branches in kernel"
> 	default y
> -	select STACK_VALIDATION if HAVE_STACK_VALIDATION
> 	help
> 	  Compile kernel with the retpoline compiler options to guard against
> 	  kernel-to-user data leaks by avoiding speculative indirect
> --- a/arch/x86/include/asm/static_call.h
> +++ b/arch/x86/include/asm/static_call.h
> @@ -2,6 +2,20 @@
> #ifndef _ASM_STATIC_CALL_H
> #define _ASM_STATIC_CALL_H
>=20
> +#include <asm/asm-offsets.h>
> +
> +#ifdef CONFIG_HAVE_STATIC_CALL_INLINE
> +
> +/*
> + * This trampoline is only used during boot / module init, so it's safe =
to use
> + * the indirect branch without a retpoline.
> + */
> +#define __ARCH_STATIC_CALL_TRAMP_JMP(key, func)				\
> +	ANNOTATE_RETPOLINE_SAFE						\
> +	"jmpq *" __stringify(key) "+" __stringify(SC_KEY_func) "(%rip) \n"
> +
> +#else /* !CONFIG_HAVE_STATIC_CALL_INLINE */
> +
> /*
>  * Manually construct a 5-byte direct JMP to prevent the assembler from
>  * optimizing it into a 2-byte JMP.
> @@ -12,9 +26,19 @@
> 	".long " #func " - " __ARCH_STATIC_CALL_JMP_LABEL(key) "\n"	\
> 	__ARCH_STATIC_CALL_JMP_LABEL(key) ":"
>=20
> +#endif /* !CONFIG_HAVE_STATIC_CALL_INLINE */
> +
> /*
> - * This is a permanent trampoline which does a direct jump to the functi=
on.
> - * The direct jump get patched by static_call_update().
> + * For CONFIG_HAVE_STATIC_CALL_INLINE, this is a temporary trampoline wh=
ich
> + * uses the current value of the key->func pointer to do an indirect jum=
p to
> + * the function.  This trampoline is only used during boot, before the c=
all
> + * sites get patched by static_call_update().  The name of this trampoli=
ne has
> + * a magical aspect: objtool uses it to find static call sites so it can=
 create
> + * the .static_call_sites section.
> + *
> + * For CONFIG_HAVE_STATIC_CALL, this is a permanent trampoline which
> + * does a direct jump to the function.  The direct jump gets patched by
> + * static_call_update().
>  */
> #define ARCH_DEFINE_STATIC_CALL_TRAMP(key, func)			\
> 	asm(".pushsection .text, \"ax\"				\n"	\
> --- a/arch/x86/kernel/asm-offsets.c
> +++ b/arch/x86/kernel/asm-offsets.c
> @@ -12,6 +12,7 @@
> #include <linux/hardirq.h>
> #include <linux/suspend.h>
> #include <linux/kbuild.h>
> +#include <linux/static_call.h>
> #include <asm/processor.h>
> #include <asm/thread_info.h>
> #include <asm/sigframe.h>
> @@ -104,4 +105,9 @@ static void __used common(void)
> 	OFFSET(TSS_sp0, tss_struct, x86_tss.sp0);
> 	OFFSET(TSS_sp1, tss_struct, x86_tss.sp1);
> 	OFFSET(TSS_sp2, tss_struct, x86_tss.sp2);
> +
> +#ifdef CONFIG_HAVE_STATIC_CALL_INLINE
> +	BLANK();
> +	OFFSET(SC_KEY_func, static_call_key, func);
> +#endif
> }
> --- a/arch/x86/kernel/static_call.c
> +++ b/arch/x86/kernel/static_call.c
> @@ -10,16 +10,22 @@
> void arch_static_call_transform(void *site, void *tramp, void *func)
> {
> 	unsigned char opcodes[CALL_INSN_SIZE];
> -	unsigned char insn_opcode;
> +	unsigned char insn_opcode, expected;
> 	unsigned long insn;
> 	s32 dest_relative;
>=20
> 	mutex_lock(&text_mutex);
>=20
> -	insn =3D (unsigned long)tramp;
> +	if (IS_ENABLED(CONFIG_HAVE_STATIC_CALL_INLINE)) {
> +		insn =3D (unsigned long)site;
> +		expected =3D 0xE8; /* CALL */

RELATIVECALL_OPCODE ?

> +	} else {
> +		insn =3D (unsigned long)tramp;
> +		expected =3D 0xE9; /* JMP */

RELATIVEJUMP_OPCODE ?

( I did not review the objtool parts )=