public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Nadav Amit <namit@vmware.com>
To: <linux-kernel@vger.kernel.org>, <x86@kernel.org>
Cc: <nadav.amit@gmail.com>, Nadav Amit <namit@vmware.com>,
	Juergen Gross <jgross@suse.com>,
	Alok Kataria <akataria@vmware.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	<virtualization@lists.linux-foundation.org>
Subject: [PATCH 4/6] x86: prevent inline distortion by paravirt ops
Date: Thu, 17 May 2018 09:14:00 -0700	[thread overview]
Message-ID: <20180517161402.78089-5-namit@vmware.com> (raw)
In-Reply-To: <20180517161402.78089-1-namit@vmware.com>

GCC considers the number of statements in inlined assembly blocks,
according to new-lines and semicolons, as an indication to the cost of
the block in time and space. This data is distorted by the kernel code,
which puts information in alternative sections. As a result, the
compiler may perform incorrect inlining and branch optimizations.

The solution is to set an assembly macro and call it from the inlined
assembly block. As a result GCC considers the inline assembly block as
a single instruction.

The effect of the patch is a more aggressive inlining, which also
causes a size increase of kernel.

   text	   data	    bss	    dec	    hex	filename
18131468 10068488 2936832 31136788 1db1c14 ./vmlinux before
18146418 10064100 2936832 31147350 1db4556 ./vmlinux after (+10562)

Static text symbols:
Before:	39788
After:	39673	(-115)

Cc: Juergen Gross <jgross@suse.com>
Cc: Alok Kataria <akataria@vmware.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: virtualization@lists.linux-foundation.org

Signed-off-by: Nadav Amit <namit@vmware.com>
---
 arch/x86/include/asm/paravirt_types.h | 63 +++++++++++++++++----------
 1 file changed, 39 insertions(+), 24 deletions(-)

diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 180bc0bff0fb..ea62204c2ee6 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -346,20 +346,45 @@ extern struct pv_lock_ops pv_lock_ops;
 /*
  * Generate some code, and mark it as patchable by the
  * apply_paravirt() alternate instruction patcher.
+ *
+ * This generates an indirect call based on the operation type number.
+ * The type number, computed in PARAVIRT_PATCH, is derived from the
+ * offset into the paravirt_patch_template structure, and can therefore be
+ * freely converted back into a structure offset.
+ *
+ * The paravirtual alternative logic and data are encapsulated within an
+ * assembly macro, which is then called on each use. This hack is necessary to
+ * prevent GCC from considering the inline assembly blocks as costly in time and
+ * space, which can prevent function inlining and lead to other bad compilation
+ * decisions. GCC computes inline assembly cost according to the number of
+ * perceived number of assembly instruction, based on the number of new-lines
+ * and semicolons in the assembly block. The macro will eventually be compiled
+ * into a single instruction (and some data). This scheme allows GCC to better
+ * understand the inline asm cost.
  */
-#define _paravirt_alt(insn_string, type, clobber)	\
-	"771:\n\t" insn_string "\n" "772:\n"		\
-	".pushsection .parainstructions,\"a\"\n"	\
-	_ASM_ALIGN "\n"					\
-	_ASM_PTR " 771b\n"				\
-	"  .byte " type "\n"				\
-	"  .byte 772b-771b\n"				\
-	"  .short " clobber "\n"			\
-	".popsection\n"
+asm(".macro __paravirt_alt type:req clobber:req pv_opptr:req\n"
+    "771:\n\t"
+    ANNOTATE_RETPOLINE_SAFE "\n\t"
+    "call *\\pv_opptr\n"
+    "772:\n\t"
+    ".pushsection .parainstructions,\"a\"\n\t"
+    _ASM_ALIGN "\n\t"
+    _ASM_PTR " 771b\n\t"
+    ".byte \\type\n\t"
+    ".byte 772b-771b\n\t"
+    ".short \\clobber\n\t"
+    ".popsection\n\t"
+    ".endm");
+
+#define _paravirt_alt(type, clobber, pv_opptr)				\
+	"__paravirt_alt type=" __stringify(type)			\
+	" clobber=" __stringify(clobber)				\
+	" pv_opptr=" __stringify(pv_opptr) "\n\t"
 
 /* Generate patchable code, with the default asm parameters. */
-#define paravirt_alt(insn_string)					\
-	_paravirt_alt(insn_string, "%c[paravirt_typenum]", "%c[paravirt_clobber]")
+#define paravirt_alt							\
+	_paravirt_alt("%c[paravirt_typenum]", "%c[paravirt_clobber]",	\
+		      "%c[paravirt_opptr]")
 
 /* Simple instruction patching code. */
 #define NATIVE_LABEL(a,x,b) "\n\t.globl " a #x "_" #b "\n" a #x "_" #b ":\n\t"
@@ -387,16 +412,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
 
 int paravirt_disable_iospace(void);
 
-/*
- * This generates an indirect call based on the operation type number.
- * The type number, computed in PARAVIRT_PATCH, is derived from the
- * offset into the paravirt_patch_template structure, and can therefore be
- * freely converted back into a structure offset.
- */
-#define PARAVIRT_CALL					\
-	ANNOTATE_RETPOLINE_SAFE				\
-	"call *%c[paravirt_opptr];"
-
 /*
  * These macros are intended to wrap calls through one of the paravirt
  * ops structs, so that they can be later identified and patched at
@@ -534,7 +549,7 @@ int paravirt_disable_iospace(void);
 		/* since this condition will never hold */		\
 		if (sizeof(rettype) > sizeof(unsigned long)) {		\
 			asm volatile(pre				\
-				     paravirt_alt(PARAVIRT_CALL)	\
+				     paravirt_alt			\
 				     post				\
 				     : call_clbr, ASM_CALL_CONSTRAINT	\
 				     : paravirt_type(op),		\
@@ -544,7 +559,7 @@ int paravirt_disable_iospace(void);
 			__ret = (rettype)((((u64)__edx) << 32) | __eax); \
 		} else {						\
 			asm volatile(pre				\
-				     paravirt_alt(PARAVIRT_CALL)	\
+				     paravirt_alt			\
 				     post				\
 				     : call_clbr, ASM_CALL_CONSTRAINT	\
 				     : paravirt_type(op),		\
@@ -571,7 +586,7 @@ int paravirt_disable_iospace(void);
 		PVOP_VCALL_ARGS;					\
 		PVOP_TEST_NULL(op);					\
 		asm volatile(pre					\
-			     paravirt_alt(PARAVIRT_CALL)		\
+			     paravirt_alt				\
 			     post					\
 			     : call_clbr, ASM_CALL_CONSTRAINT		\
 			     : paravirt_type(op),			\
-- 
2.17.0

  parent reply	other threads:[~2018-05-17 23:28 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-17 16:13 [PATCH 0/6] Macrofying inline assembly for better compilation Nadav Amit
2018-05-17 16:13 ` [PATCH 1/6] x86: objtool: use asm macro for better compiler decisions Nadav Amit
2018-05-17 16:13 ` [PATCH 2/6] x86: bug: prevent gcc distortions Nadav Amit
2018-05-18  7:58   ` Peter Zijlstra
2018-05-18  8:13     ` Ingo Molnar
2018-05-18 10:11       ` Borislav Petkov
2018-05-18 14:36         ` Nadav Amit
2018-05-18 15:40           ` Borislav Petkov
2018-05-18 15:46             ` Nadav Amit
2018-05-18 15:53               ` Borislav Petkov
2018-05-18 16:29                 ` Nadav Amit
2018-05-18 17:41                   ` Boris Petkov
2018-05-18 14:30       ` Nadav Amit
2018-05-18 14:22     ` Nadav Amit
2018-05-18 17:52       ` Joe Perches
2018-05-18 16:24     ` Linus Torvalds
2018-05-18 17:24       ` Nadav Amit
2018-05-18 18:25         ` Linus Torvalds
2018-05-18 18:33           ` hpa
2018-05-18 18:50             ` Linus Torvalds
2018-05-18 18:53               ` hpa
2018-05-18 19:02                 ` Nadav Amit
2018-05-18 19:05                   ` hpa
2018-05-18 19:11                   ` Linus Torvalds
2018-05-18 19:18                     ` Nadav Amit
2018-05-18 19:21                       ` Linus Torvalds
2018-05-18 19:22                         ` hpa
2018-05-18 19:36                           ` Nadav Amit
2018-05-18 19:41                             ` hpa
2018-05-17 16:13 ` [PATCH 3/6] x86: alternative: macrofy locks for better inlining Nadav Amit
2018-05-17 16:14 ` Nadav Amit [this message]
2018-05-17 16:14 ` [PATCH 5/6] x86: refcount: prevent gcc distortions Nadav Amit
2018-05-19  4:27   ` kbuild test robot
2018-05-17 16:14 ` [PATCH 6/6] x86: removing unneeded new-lines Nadav Amit
2018-05-18  9:20 ` [PATCH 0/6] Macrofying inline assembly for better compilation David Laight
2018-05-18 14:15   ` Nadav Amit

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=20180517161402.78089-5-namit@vmware.com \
    --to=namit@vmware.com \
    --cc=akataria@vmware.com \
    --cc=hpa@zytor.com \
    --cc=jgross@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=nadav.amit@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=virtualization@lists.linux-foundation.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox