* [4.4 STABLE BACKPORT] x86: paravirt: make native_save_fl extern inline
[not found] <20180718090702.GA6212@kroah.com>
@ 2018-07-20 22:36 ` Nick Desaulniers
2018-07-21 0:04 ` kbuild test robot
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Nick Desaulniers @ 2018-07-20 22:36 UTC (permalink / raw)
To: gregkh
Cc: stable, Nick Desaulniers, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, x86, Juergen Gross, linux-kernel
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>
---
Backport for 4.4. 4.4 is missing commit 784d5699eddc "x86: move exports to
actual definitions" which doesn't apply cleanly, and not really worth
backporting IMO. It's simpler to change this patch from upstream:
+ #include <asm-generic/export.h>
rather than
+ #include <asm/export.h>
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 b77f5edb03b0..0056bc945cd1 100644
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -8,7 +8,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 b1b78ffe01d0..7947cee61f61 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -41,6 +41,7 @@ obj-y += alternative.o i8253.o pci-nommu.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..3817eb748eb4
--- /dev/null
+++ b/arch/x86/kernel/irqflags.S
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <asm/asm.h>
+#include <asm-generic/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.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [4.4 STABLE BACKPORT] x86: paravirt: make native_save_fl extern inline
2018-07-20 22:36 ` [4.4 STABLE BACKPORT] x86: paravirt: make native_save_fl extern inline Nick Desaulniers
@ 2018-07-21 0:04 ` kbuild test robot
2018-07-21 0:06 ` Nick Desaulniers
2018-07-21 5:08 ` Greg KH
2018-07-23 7:57 ` Greg KH
2 siblings, 1 reply; 6+ messages in thread
From: kbuild test robot @ 2018-07-21 0:04 UTC (permalink / raw)
To: Nick Desaulniers
Cc: kbuild-all, gregkh, stable, Nick Desaulniers, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86, Juergen Gross, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1077 bytes --]
Hi Nick,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tip/x86/core]
[cannot apply to v4.18-rc5 next-20180720]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nick-Desaulniers/x86-paravirt-make-native_save_fl-extern-inline/20180721-073112
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
arch/x86/kernel/irqflags.S: Assembler messages:
>> arch/x86/kernel/irqflags.S:22: Error: bad register name `%_ASM_ARG1'
vim +22 arch/x86/kernel/irqflags.S
16
17 /*
18 * void native_restore_fl(unsigned long flags)
19 * %eax/%rdi: flags
20 */
21 ENTRY(native_restore_fl)
> 22 push %_ASM_ARG1
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6567 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [4.4 STABLE BACKPORT] x86: paravirt: make native_save_fl extern inline
2018-07-21 0:04 ` kbuild test robot
@ 2018-07-21 0:06 ` Nick Desaulniers
2018-07-21 5:07 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2018-07-21 0:06 UTC (permalink / raw)
To: lkp
Cc: kbuild-all, Greg KH, stable, Thomas Gleixner, mingo, hpa, x86,
jgross, LKML
On Fri, Jul 20, 2018 at 5:04 PM kbuild test robot <lkp@intel.com> wrote:
>
> Hi Nick,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on tip/x86/core]
> [cannot apply to v4.18-rc5 next-20180720]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Nick-Desaulniers/x86-paravirt-make-native_save_fl-extern-inline/20180721-073112
> config: i386-tinyconfig (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> All errors (new ones prefixed by >>):
>
> arch/x86/kernel/irqflags.S: Assembler messages:
> >> arch/x86/kernel/irqflags.S:22: Error: bad register name `%_ASM_ARG1'
>
> vim +22 arch/x86/kernel/irqflags.S
>
> 16
> 17 /*
> 18 * void native_restore_fl(unsigned long flags)
> 19 * %eax/%rdi: flags
> 20 */
> 21 ENTRY(native_restore_fl)
> > 22 push %_ASM_ARG1
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
oops, probably should not have cc'ed vger.kernel.org for a patch meant
for stable.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [4.4 STABLE BACKPORT] x86: paravirt: make native_save_fl extern inline
2018-07-21 0:06 ` Nick Desaulniers
@ 2018-07-21 5:07 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2018-07-21 5:07 UTC (permalink / raw)
To: Nick Desaulniers
Cc: lkp, kbuild-all, stable, Thomas Gleixner, mingo, hpa, x86, jgross, LKML
On Fri, Jul 20, 2018 at 05:06:07PM -0700, Nick Desaulniers wrote:
> On Fri, Jul 20, 2018 at 5:04 PM kbuild test robot <lkp@intel.com> wrote:
> >
> > Hi Nick,
> >
> > Thank you for the patch! Yet something to improve:
> >
> > [auto build test ERROR on tip/x86/core]
> > [cannot apply to v4.18-rc5 next-20180720]
> > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> >
> > url: https://github.com/0day-ci/linux/commits/Nick-Desaulniers/x86-paravirt-make-native_save_fl-extern-inline/20180721-073112
> > config: i386-tinyconfig (attached as .config)
> > compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> > reproduce:
> > # save the attached .config to linux build tree
> > make ARCH=i386
> >
> > All errors (new ones prefixed by >>):
> >
> > arch/x86/kernel/irqflags.S: Assembler messages:
> > >> arch/x86/kernel/irqflags.S:22: Error: bad register name `%_ASM_ARG1'
> >
> > vim +22 arch/x86/kernel/irqflags.S
> >
> > 16
> > 17 /*
> > 18 * void native_restore_fl(unsigned long flags)
> > 19 * %eax/%rdi: flags
> > 20 */
> > 21 ENTRY(native_restore_fl)
> > > 22 push %_ASM_ARG1
> >
> > ---
> > 0-DAY kernel test infrastructure Open Source Technology Center
> > https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
> oops, probably should not have cc'ed vger.kernel.org for a patch meant
> for stable.
stable@vger.kernel.org is the right address, you did it correct. I
think 0-day just got a bit trigger-happy on this one for some reason,
don't worry about it.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [4.4 STABLE BACKPORT] x86: paravirt: make native_save_fl extern inline
2018-07-20 22:36 ` [4.4 STABLE BACKPORT] x86: paravirt: make native_save_fl extern inline Nick Desaulniers
2018-07-21 0:04 ` kbuild test robot
@ 2018-07-21 5:08 ` Greg KH
2018-07-23 7:57 ` Greg KH
2 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2018-07-21 5:08 UTC (permalink / raw)
To: Nick Desaulniers
Cc: stable, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
Juergen Gross, linux-kernel
On Fri, Jul 20, 2018 at 03:36:41PM -0700, Nick Desaulniers wrote:
> 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>
> ---
> Backport for 4.4. 4.4 is missing commit 784d5699eddc "x86: move exports to
> actual definitions" which doesn't apply cleanly, and not really worth
> backporting IMO. It's simpler to change this patch from upstream:
> + #include <asm-generic/export.h>
> rather than
> + #include <asm/export.h>
Yeah, that makes sense, thanks for the backport. I'll queue it up after
the next round of stable kernels comes out in a few days.
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [4.4 STABLE BACKPORT] x86: paravirt: make native_save_fl extern inline
2018-07-20 22:36 ` [4.4 STABLE BACKPORT] x86: paravirt: make native_save_fl extern inline Nick Desaulniers
2018-07-21 0:04 ` kbuild test robot
2018-07-21 5:08 ` Greg KH
@ 2018-07-23 7:57 ` Greg KH
2 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2018-07-23 7:57 UTC (permalink / raw)
To: Nick Desaulniers
Cc: stable, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
Juergen Gross, linux-kernel
On Fri, Jul 20, 2018 at 03:36:41PM -0700, Nick Desaulniers wrote:
> 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>
> ---
> Backport for 4.4. 4.4 is missing commit 784d5699eddc "x86: move exports to
> actual definitions" which doesn't apply cleanly, and not really worth
> backporting IMO. It's simpler to change this patch from upstream:
> + #include <asm-generic/export.h>
> rather than
> + #include <asm/export.h>
Now applied, thanks.
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-07-23 7:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20180718090702.GA6212@kroah.com>
2018-07-20 22:36 ` [4.4 STABLE BACKPORT] x86: paravirt: make native_save_fl extern inline Nick Desaulniers
2018-07-21 0:04 ` kbuild test robot
2018-07-21 0:06 ` Nick Desaulniers
2018-07-21 5:07 ` Greg KH
2018-07-21 5:08 ` Greg KH
2018-07-23 7:57 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).