Please cherry-pick upstream commits:
d03db2b "compiler-gcc.h: Add __attribute__((gnu_inline)) to all inline declarations" 0e2e160 "x86/asm: Add _ASM_ARG* constants for argument registers to <asm/asm.h>" d0a8d93 "x86/paravirt: Make native_save_fl() extern inline"
To stable branches 4.4+. They will allow 4.4+ x86 kernels compiled with Clang and have the configs CONFIG_STACK_PROTECTOR_STRONG and CONFIG_PARAVIRT to boot. They also allow gcc 5.1+ users to have consistent `extern inline` semantics.
In response to: https://android-review.googlesource.com/c/kernel/common/+/716477#message-291...
One of these days I'll remember to cc stable in the commit message properly...sorry!
On Tue, Jul 17, 2018 at 04:48:01PM -0700, Nick Desaulniers wrote:
Please cherry-pick upstream commits:
d03db2b "compiler-gcc.h: Add __attribute__((gnu_inline)) to all inline declarations"
This took a bunch of work to get it to work for 4.9, I didn't mess with it for 4.4.y, can you provide a patch series for that tree? Hm, I wonder if the same commits I did for 4.9.y would work there, let me try that first...
0e2e160 "x86/asm: Add _ASM_ARG* constants for argument registers to <asm/asm.h>"
This didn't work for 4.4.y either :(
d0a8d93 "x86/paravirt: Make native_save_fl() extern inline"
This worked for all.
To stable branches 4.4+. They will allow 4.4+ x86 kernels compiled with Clang and have the configs CONFIG_STACK_PROTECTOR_STRONG and CONFIG_PARAVIRT to boot. They also allow gcc 5.1+ users to have consistent `extern inline` semantics.
In response to: https://android-review.googlesource.com/c/kernel/common/+/716477#message-291...
One of these days I'll remember to cc stable in the commit message properly...sorry!
Not a problem.
Also, a minor thing, when doing kernel work you should have: [core] abbrev = 12
in your .gitconfig as our sha1 numbers are usually longer than the 7 you sent above.
Also, here's a handy alias lots of us use: git show -s --abbrev-commit --abbrev=12 --pretty=format:"%h ("%s")%n" to output the sha1 and commit description.
It would do this for one of your commits above: d0a8d9378d16 ("x86/paravirt: Make native_save_fl() extern inline")
thanks,
greg k-h
On Wed, Jul 18, 2018 at 10:48:37AM +0200, Greg KH wrote:
On Tue, Jul 17, 2018 at 04:48:01PM -0700, Nick Desaulniers wrote:
Please cherry-pick upstream commits:
d03db2b "compiler-gcc.h: Add __attribute__((gnu_inline)) to all inline declarations"
This took a bunch of work to get it to work for 4.9, I didn't mess with it for 4.4.y, can you provide a patch series for that tree? Hm, I wonder if the same commits I did for 4.9.y would work there, let me try that first...
Ok, nevermind, I got all of the above to work for 4.4.y now as well. Hopefully it passes my test builds...
0e2e160 "x86/asm: Add _ASM_ARG* constants for argument registers to <asm/asm.h>"
This didn't work for 4.4.y either :(
I got this to apply as well now, so all should be good.
thanks,
greg k-h
On Wed, Jul 18, 2018 at 10:48:37AM +0200, Greg KH wrote:
On Tue, Jul 17, 2018 at 04:48:01PM -0700, Nick Desaulniers wrote:
Please cherry-pick upstream commits:
d03db2b "compiler-gcc.h: Add __attribute__((gnu_inline)) to all inline declarations"
This took a bunch of work to get it to work for 4.9, I didn't mess with it for 4.4.y, can you provide a patch series for that tree? Hm, I wonder if the same commits I did for 4.9.y would work there, let me try that first...
0e2e160 "x86/asm: Add _ASM_ARG* constants for argument registers to <asm/asm.h>"
This didn't work for 4.4.y either :(
d0a8d93 "x86/paravirt: Make native_save_fl() extern inline"
This worked for all.
Oops, nope, it broke the build on 4.4.y, can you provide a working backport for there?
thanks,
greg k-h
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)
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-... 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
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-... 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.
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-... 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
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
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
On Wed, Jul 18, 2018 at 1:48 AM Greg KH gregkh@linuxfoundation.org wrote:
Also, a minor thing, when doing kernel work you should have: [core] abbrev = 12
in your .gitconfig as our sha1 numbers are usually longer than the 7 you sent above.
Also, here's a handy alias lots of us use: git show -s --abbrev-commit --abbrev=12 --pretty=format:"%h ("%s")%n" to output the sha1 and commit description.
It would do this for one of your commits above: d0a8d9378d16 ("x86/paravirt: Make native_save_fl() extern inline")
Neat, thanks for the tip, I was wondering if folks were always counting out 12 characters when copying SHAs...
On Wed, Jul 18, 2018 at 2:07 AM Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Jul 18, 2018 at 10:48:37AM +0200, Greg KH wrote:
On Tue, Jul 17, 2018 at 04:48:01PM -0700, Nick Desaulniers wrote:
Please cherry-pick upstream commits:
d03db2b "compiler-gcc.h: Add __attribute__((gnu_inline)) to all inline declarations"
This took a bunch of work to get it to work for 4.9, I didn't mess with it for 4.4.y, can you provide a patch series for that tree? Hm, I wonder if the same commits I did for 4.9.y would work there, let me try that first...
0e2e160 "x86/asm: Add _ASM_ARG* constants for argument registers to <asm/asm.h>"
This didn't work for 4.4.y either :(
d0a8d93 "x86/paravirt: Make native_save_fl() extern inline"
This worked for all.
Oops, nope, it broke the build on 4.4.y, can you provide a working backport for there?
Was there anything special I should have put in the commit message to show it was changed from upstream? I sent it in reply to this email I'm responding to, but gmail doesn't display it as a child. It can also be found: https://lkml.org/lkml/2018/7/20/1142 but please let me know if there's a better way I should send it.
On Fri, Jul 20, 2018 at 03:45:29PM -0700, Nick Desaulniers wrote:
On Wed, Jul 18, 2018 at 1:48 AM Greg KH gregkh@linuxfoundation.org wrote:
Also, a minor thing, when doing kernel work you should have: [core] abbrev = 12
in your .gitconfig as our sha1 numbers are usually longer than the 7 you sent above.
Also, here's a handy alias lots of us use: git show -s --abbrev-commit --abbrev=12 --pretty=format:"%h ("%s")%n" to output the sha1 and commit description.
It would do this for one of your commits above: d0a8d9378d16 ("x86/paravirt: Make native_save_fl() extern inline")
Neat, thanks for the tip, I was wondering if folks were always counting out 12 characters when copying SHAs...
On Wed, Jul 18, 2018 at 2:07 AM Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Jul 18, 2018 at 10:48:37AM +0200, Greg KH wrote:
On Tue, Jul 17, 2018 at 04:48:01PM -0700, Nick Desaulniers wrote:
Please cherry-pick upstream commits:
d03db2b "compiler-gcc.h: Add __attribute__((gnu_inline)) to all inline declarations"
This took a bunch of work to get it to work for 4.9, I didn't mess with it for 4.4.y, can you provide a patch series for that tree? Hm, I wonder if the same commits I did for 4.9.y would work there, let me try that first...
0e2e160 "x86/asm: Add _ASM_ARG* constants for argument registers to <asm/asm.h>"
This didn't work for 4.4.y either :(
d0a8d93 "x86/paravirt: Make native_save_fl() extern inline"
This worked for all.
Oops, nope, it broke the build on 4.4.y, can you provide a working backport for there?
Was there anything special I should have put in the commit message to show it was changed from upstream? I sent it in reply to this email I'm responding to, but gmail doesn't display it as a child. It can also be found: https://lkml.org/lkml/2018/7/20/1142 but please let me know if there's a better way I should send it.
Well, gmail sucks for mailing lists :)
And no, you did it great, but it would be nice to have the git commit id of the patch itself somewhere in there so I know what it is in Linus's tree so I don't have to go dig it up manually. Not a big deal, I can find it easily this time, but for the next time you send stuff in.
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
kbuild test robot -
Nick Desaulniers