All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: mingo@redhat.com, tglx@linutronix.de
Cc: akpm@linux-foundation.org, hpa@zytor.com,
	linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org,
	x86@kernel.org, virtualization@lists.linux-foundation.org,
	astrachan@google.com, manojgupta@google.com,
	ghackmann@google.com, sedat.dilek@gmail.com, tstellar@redhat.com,
	keescook@google.com, yamada.masahiro@socionext.com,
	michal.lkml@markovi.net, linux-kbuild@vger.kernel.org,
	geert@linux-m68k.org, will.deacon@arm.com,
	mawilcox@microsoft.com, arnd@arndb.de, rientjes@google.com,
	acme@redhat.com, pombredanne@nexb.com, aryabinin@virtuozzo.com,
	kstewart@linuxfoundation.org, boris.ostrovsky@oracle.com,
	jan.kiszka@siemens.com, rostedt@goodmis.org,
	kirill.shutemov@linux.intel.com, ard.biesheuvel@linaro.org,
	akataria@vmware.com, brijesh.singh@amd.com,
	caoj.fnst@cn.fujitsu.com, gregkh@linuxfoundation.org,
	jarkko.sakkinen@linux.intel.com, jgross@suse.com,
	jpoimboe@redhat.com, mka@chromium.org, ndesaulniers@google.com,
	thomas.lendacky@amd.com, tweek@google.com, mjg59@google.com,
	joe@perches.com, andrea.parri@amarulasolutions.com
Subject: [PATCH v6 3/3] x86: paravirt: make native_save_fl extern inline
Date: Thu, 21 Jun 2018 09:23:24 -0700	[thread overview]
Message-ID: <20180621162324.36656-4-ndesaulniers@google.com> (raw)
In-Reply-To: <20180621162324.36656-1-ndesaulniers@google.com>

native_save_fl() is marked static inline, but by using it as
a function pointer in arch/x86/kernel/paravirt.c, it MUST be outlined.

paravirt's use of native_save_fl() also requires that no GPRs other than
%rax are clobbered.

Compilers have different heuristics which they use to emit stack guard
code, the emittance of which can break paravirt's callee saved assumption
by clobbering %rcx.

Marking a function definition extern inline means that if this version
cannot be inlined, then the out-of-line version will be preferred. By
having the out-of-line version be implemented in assembly, it cannot be
instrumented with a stack protector, which might violate custom calling
conventions that code like paravirt rely on.

The semantics of extern inline has changed since gnu89. This means that
folks using GCC versions >= 5.1 may see symbol redefinition errors at
link time for subdirs that override KBUILD_CFLAGS (making the C standard
used implicit) regardless of this patch. This has been cleaned up
earlier in the patch set, but is left as a note in the commit message
for future travelers.

Reports:
https://lkml.org/lkml/2018/5/7/534
https://github.com/ClangBuiltLinux/linux/issues/16

Discussion:
https://bugs.llvm.org/show_bug.cgi?id=37512
https://lkml.org/lkml/2018/5/24/1371

Thanks to the many folks that participated in the discussion.

Acked-by: Juergen Gross <jgross@suse.com>
Debugged-by: Alistair Strachan <astrachan@google.com>
Debugged-by: Matthias Kaehlcke <mka@chromium.org>
Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Suggested-by: H. Peter Anvin <hpa@zytor.com>
Suggested-by: Tom Stellar <tstellar@redhat.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 arch/x86/include/asm/irqflags.h |  2 +-
 arch/x86/kernel/Makefile        |  1 +
 arch/x86/kernel/irqflags.S      | 26 ++++++++++++++++++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/kernel/irqflags.S

diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
index 89f08955fff7..c4fc17220df9 100644
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -13,7 +13,7 @@
  * Interrupt control:
  */
 
-static inline unsigned long native_save_fl(void)
+extern inline unsigned long native_save_fl(void)
 {
 	unsigned long flags;
 
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 02d6f5cf4e70..8824d01c0c35 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -61,6 +61,7 @@ obj-y			+= alternative.o i8253.o hw_breakpoint.o
 obj-y			+= tsc.o tsc_msr.o io_delay.o rtc.o
 obj-y			+= pci-iommu_table.o
 obj-y			+= resource.o
+obj-y			+= irqflags.o
 
 obj-y				+= process.o
 obj-y				+= fpu/
diff --git a/arch/x86/kernel/irqflags.S b/arch/x86/kernel/irqflags.S
new file mode 100644
index 000000000000..ddeeaac8adda
--- /dev/null
+++ b/arch/x86/kernel/irqflags.S
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <asm/asm.h>
+#include <asm/export.h>
+#include <linux/linkage.h>
+
+/*
+ * unsigned long native_save_fl(void)
+ */
+ENTRY(native_save_fl)
+	pushf
+	pop %_ASM_AX
+	ret
+ENDPROC(native_save_fl)
+EXPORT_SYMBOL(native_save_fl)
+
+/*
+ * void native_restore_fl(unsigned long flags)
+ * %eax/%rdi: flags
+ */
+ENTRY(native_restore_fl)
+	push %_ASM_ARG1
+	popf
+	ret
+ENDPROC(native_restore_fl)
+EXPORT_SYMBOL(native_restore_fl)
-- 
2.18.0.rc2.346.g013aa6912e-goog


WARNING: multiple messages have this Message-ID (diff)
From: Nick Desaulniers <ndesaulniers@google.com>
To: mingo@redhat.com, tglx@linutronix.de
Cc: akpm@linux-foundation.org, hpa@zytor.com,
	linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org,
	x86@kernel.org, virtualization@lists.linux-foundation.org,
	astrachan@google.com, manojgupta@google.com,
	ghackmann@google.com, sedat.dilek@gmail.com, tstellar@redhat.com,
	keescook@google.com, yamada.masahiro@socionext.com,
	michal.lkml@markovi.net, linux-kbuild@vger.kernel.org,
	geert@linux-m68k.org, will.deacon@arm.com,
	mawilcox@microsoft.com, arnd@arndb.de, rientjes@google.com,
	acme@redhat.com, pombredanne@nexb.com, aryabinin@virtuozzo.com,
	kstewart@linuxfoundation.org, boris.ostrovsky@oracle.com,
	jan.kiszka@siemens.com, rostedt@goodmis.org,
	kirill.shutemov@linux.intel.com, ard.biesheuvel@linaro.org,
	akataria@vmware.com, brijesh.singh@amd.com,
	caoj.fnst@cn.fujitsu.com, gregkh@l
Subject: [PATCH v6 3/3] x86: paravirt: make native_save_fl extern inline
Date: Thu, 21 Jun 2018 09:23:24 -0700	[thread overview]
Message-ID: <20180621162324.36656-4-ndesaulniers@google.com> (raw)
In-Reply-To: <20180621162324.36656-1-ndesaulniers@google.com>

native_save_fl() is marked static inline, but by using it as
a function pointer in arch/x86/kernel/paravirt.c, it MUST be outlined.

paravirt's use of native_save_fl() also requires that no GPRs other than
%rax are clobbered.

Compilers have different heuristics which they use to emit stack guard
code, the emittance of which can break paravirt's callee saved assumption
by clobbering %rcx.

Marking a function definition extern inline means that if this version
cannot be inlined, then the out-of-line version will be preferred. By
having the out-of-line version be implemented in assembly, it cannot be
instrumented with a stack protector, which might violate custom calling
conventions that code like paravirt rely on.

The semantics of extern inline has changed since gnu89. This means that
folks using GCC versions >= 5.1 may see symbol redefinition errors at
link time for subdirs that override KBUILD_CFLAGS (making the C standard
used implicit) regardless of this patch. This has been cleaned up
earlier in the patch set, but is left as a note in the commit message
for future travelers.

Reports:
https://lkml.org/lkml/2018/5/7/534
https://github.com/ClangBuiltLinux/linux/issues/16

Discussion:
https://bugs.llvm.org/show_bug.cgi?id=37512
https://lkml.org/lkml/2018/5/24/1371

Thanks to the many folks that participated in the discussion.

Acked-by: Juergen Gross <jgross@suse.com>
Debugged-by: Alistair Strachan <astrachan@google.com>
Debugged-by: Matthias Kaehlcke <mka@chromium.org>
Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Suggested-by: H. Peter Anvin <hpa@zytor.com>
Suggested-by: Tom Stellar <tstellar@redhat.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 arch/x86/include/asm/irqflags.h |  2 +-
 arch/x86/kernel/Makefile        |  1 +
 arch/x86/kernel/irqflags.S      | 26 ++++++++++++++++++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/kernel/irqflags.S

diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
index 89f08955fff7..c4fc17220df9 100644
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -13,7 +13,7 @@
  * Interrupt control:
  */
 
-static inline unsigned long native_save_fl(void)
+extern inline unsigned long native_save_fl(void)
 {
 	unsigned long flags;
 
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 02d6f5cf4e70..8824d01c0c35 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -61,6 +61,7 @@ obj-y			+= alternative.o i8253.o hw_breakpoint.o
 obj-y			+= tsc.o tsc_msr.o io_delay.o rtc.o
 obj-y			+= pci-iommu_table.o
 obj-y			+= resource.o
+obj-y			+= irqflags.o
 
 obj-y				+= process.o
 obj-y				+= fpu/
diff --git a/arch/x86/kernel/irqflags.S b/arch/x86/kernel/irqflags.S
new file mode 100644
index 000000000000..ddeeaac8adda
--- /dev/null
+++ b/arch/x86/kernel/irqflags.S
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <asm/asm.h>
+#include <asm/export.h>
+#include <linux/linkage.h>
+
+/*
+ * unsigned long native_save_fl(void)
+ */
+ENTRY(native_save_fl)
+	pushf
+	pop %_ASM_AX
+	ret
+ENDPROC(native_save_fl)
+EXPORT_SYMBOL(native_save_fl)
+
+/*
+ * void native_restore_fl(unsigned long flags)
+ * %eax/%rdi: flags
+ */
+ENTRY(native_restore_fl)
+	push %_ASM_ARG1
+	popf
+	ret
+ENDPROC(native_restore_fl)
+EXPORT_SYMBOL(native_restore_fl)
-- 
2.18.0.rc2.346.g013aa6912e-goog

  parent reply	other threads:[~2018-06-21 16:23 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-21 16:23 [PATCH v6 0/3] extern inline native_save_fl for paravirt Nick Desaulniers
2018-06-21 16:23 ` Nick Desaulniers
2018-06-21 16:23 ` [PATCH v6 1/3] compiler-gcc.h: add gnu_inline to all inline declarations Nick Desaulniers
2018-06-21 16:23   ` Nick Desaulniers
2018-07-03 15:59   ` [tip:x86/urgent] compiler-gcc.h: Add __attribute__((gnu_inline)) " tip-bot for Nick Desaulniers
2018-06-21 16:23 ` [PATCH v6 2/3] x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h> Nick Desaulniers
2018-06-21 16:23   ` Nick Desaulniers
2018-07-03 15:59   ` [tip:x86/urgent] x86/asm: Add " tip-bot for H. Peter Anvin
2018-06-21 16:23 ` Nick Desaulniers [this message]
2018-06-21 16:23   ` [PATCH v6 3/3] x86: paravirt: make native_save_fl extern inline Nick Desaulniers
2018-06-22  2:24   ` Ingo Molnar
2018-06-22  2:24     ` Ingo Molnar
2018-06-22 17:10     ` Nick Desaulniers
2018-06-22 17:10       ` Nick Desaulniers
2018-06-26  7:13       ` Ingo Molnar
2018-06-26  7:13         ` Ingo Molnar
2018-06-26 16:22         ` Nick Desaulniers
2018-06-26 16:22           ` Nick Desaulniers
2018-07-03  7:21           ` Juergen Gross
2018-07-03  7:21             ` Juergen Gross
2018-07-03 16:00   ` [tip:x86/urgent] x86/paravirt: Make native_save_fl() " tip-bot for Nick Desaulniers
2018-07-13 10:16   ` [PATCH v6 3/3] x86: paravirt: make native_save_fl " David Laight
2018-07-16 17:27     ` Nick Desaulniers
2018-07-16 17:27       ` Nick Desaulniers
2018-07-17  8:44       ` David Laight

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=20180621162324.36656-4-ndesaulniers@google.com \
    --to=ndesaulniers@google.com \
    --cc=acme@redhat.com \
    --cc=akataria@vmware.com \
    --cc=akpm@linux-foundation.org \
    --cc=andrea.parri@amarulasolutions.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=arnd@arndb.de \
    --cc=aryabinin@virtuozzo.com \
    --cc=astrachan@google.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=brijesh.singh@amd.com \
    --cc=caoj.fnst@cn.fujitsu.com \
    --cc=geert@linux-m68k.org \
    --cc=ghackmann@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=jan.kiszka@siemens.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jgross@suse.com \
    --cc=joe@perches.com \
    --cc=jpoimboe@redhat.com \
    --cc=keescook@google.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kstewart@linuxfoundation.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manojgupta@google.com \
    --cc=mawilcox@microsoft.com \
    --cc=michal.lkml@markovi.net \
    --cc=mingo@redhat.com \
    --cc=mjg59@google.com \
    --cc=mka@chromium.org \
    --cc=pombredanne@nexb.com \
    --cc=rientjes@google.com \
    --cc=rostedt@goodmis.org \
    --cc=sedat.dilek@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.com \
    --cc=tstellar@redhat.com \
    --cc=tweek@google.com \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=will.deacon@arm.com \
    --cc=x86@kernel.org \
    --cc=yamada.masahiro@socionext.com \
    /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.