linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/3] extern inline native_save_fl for paravirt
@ 2018-06-13 21:05 Nick Desaulniers
  2018-06-13 21:05 ` [PATCH v5 1/3] compiler-gcc.h: add gnu_inline to all inline declarations Nick Desaulniers
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Nick Desaulniers @ 2018-06-13 21:05 UTC (permalink / raw)
  To: akpm, hpa, mingo, tglx
  Cc: linux-efi, linux-kernel, x86, virtualization, astrachan,
	manojgupta, ghackmann, sedat.dilek, tstellar, keescook,
	yamada.masahiro, michal.lkml, linux-kbuild, geert, will.deacon,
	mawilcox, arnd, rientjes, acme, pombredanne, aryabinin, kstewart,
	boris.ostrovsky, jan.kiszka, rostedt, kirill.shutemov,
	ard.biesheuvel, akataria, brijesh.singh, caoj.fnst, gregkh,
	jarkko.sakkinen, jgross, jpoimboe, mka, ndesaulniers,
	thomas.lendacky, tweek, mjg59, joe, andrea.parri

paravirt depends on a custom calling convention (callee saved), but
expects this from a static inline function that it then forces to be
outlined. This is problematic because different compilers or flags can
then add a stack guard that violates the calling conventions.

Uses extern inline with the out-of-line definition in assembly to
prevent compilers from adding stack guards to the outlined version.

Other parts of the codebase overwrite KBUILD_CFLAGS, which is *extremely
problematic* for extern inline, as the sematics are completely the
opposite depending on what C standard is used.
http://blahg.josefsipek.net/?p=529

Changes since v4:
  Take Arnd's and hpa's suggestions properly feature detect gnu_inline
  attribute to support gcc 4.1.

Changes since v3:
  Take Joe's suggestion to hoist __inline__ and __inline out of
  conditional block.

Changes since v2:
  Take hpa's _ASM_ARG patch into the set in order to simplify cross
  32b/64b x86 assembly and rebase my final patch to use it.  Apply
  Sedat's typo fix to commit message and sign off on it. Take Joe's
  suggestion to simplify __inline__ and __inline. Add Arnd to list of
  folks who made helpful suggestions.

Changes since v1:
  Prefer gnu_inline function attribute instead of explicitly setting C
  standard compiler flag in problematic Makefiles. We should instead
  carefully evaluate if those Makefiles should be overwriting
  KBUILD_CFLAGS at all. Dropped the previous first two patches and added
  a new first patch.

H. Peter Anvin (1):
  x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h>

Nick Desaulniers (2):
  compiler-gcc.h: add gnu_inline to all inline declarations
  x86: paravirt: make native_save_fl extern inline

 arch/x86/include/asm/asm.h      | 59 +++++++++++++++++++++++++++++++++
 arch/x86/include/asm/irqflags.h |  2 +-
 arch/x86/kernel/Makefile        |  1 +
 arch/x86/kernel/irqflags.S      | 26 +++++++++++++++
 include/linux/compiler-gcc.h    | 29 ++++++++++++----
 5 files changed, 109 insertions(+), 8 deletions(-)
 create mode 100644 arch/x86/kernel/irqflags.S

-- 
2.18.0.rc1.244.gcf134e6275-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5 1/3] compiler-gcc.h: add gnu_inline to all inline declarations
  2018-06-13 21:05 [PATCH v5 0/3] extern inline native_save_fl for paravirt Nick Desaulniers
@ 2018-06-13 21:05 ` Nick Desaulniers
  2018-06-13 21:05 ` [PATCH v5 2/3] x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h> Nick Desaulniers
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Nick Desaulniers @ 2018-06-13 21:05 UTC (permalink / raw)
  To: akpm, hpa, mingo, tglx
  Cc: linux-efi, linux-kernel, x86, virtualization, astrachan,
	manojgupta, ghackmann, sedat.dilek, tstellar, keescook,
	yamada.masahiro, michal.lkml, linux-kbuild, geert, will.deacon,
	mawilcox, arnd, rientjes, acme, pombredanne, aryabinin, kstewart,
	boris.ostrovsky, jan.kiszka, rostedt, kirill.shutemov,
	ard.biesheuvel, akataria, brijesh.singh, caoj.fnst, gregkh,
	jarkko.sakkinen, jgross, jpoimboe, mka, ndesaulniers,
	thomas.lendacky, tweek, mjg59, joe, andrea.parri

Functions marked extern inline do not emit an externally visible
function when the gnu89 C standard is used. Some KBUILD Makefiles
overwrite KBUILD_CFLAGS. This is an issue for GCC 5.1+ users as without
an explicit C standard specified, the default is gnu11. Since c99, the
semantics of extern inline have changed such that an externally visible
function is always emitted. This can lead to multiple definition errors
of extern inline functions at link time of compilation units whose build
files have removed an explicit C standard compiler flag for users of GCC
5.1+ or Clang.

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: Joe Perches <joe@perches.com>
---
 include/linux/compiler-gcc.h | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index b4bf73f5e38f..c7cfca4ba02b 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -65,6 +65,18 @@
 #define __must_be_array(a)	BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
 #endif
 
+/*
+ * Feature detection for gnu_inline (gnu89 extern inline semantics). Either
+ * __GNUC_STDC_INLINE__ is defined (not using gnu89 extern inline semantics,
+ * and we opt in to the gnu89 semantics), or __GNUC_STDC_INLINE__ is not
+ * defined so the gnu89 semantics are the default.
+ */
+#ifdef __GNUC_STDC_INLINE__
+#define __gnu_inline	__attribute__((gnu_inline))
+#else
+#define __gnu_inline
+#endif
+
 /*
  * Force always-inline if the user requests it so via the .config,
  * or if gcc is too old.
@@ -72,19 +84,22 @@
  * -Wunused-function.  This turns out to avoid the need for complex #ifdef
  * directives.  Suppress the warning in clang as well by using "unused"
  * function attribute, which is redundant but not harmful for gcc.
+ * Prefer gnu_inline, so that extern inline functions do not emit an
+ * externally visible function. This makes extern inline behave as per gnu89
+ * semantics rather than c99. This prevents multiple symbol definition errors
+ * of extern inline functions at link time.
+ * A lot of inline functions can cause havoc with function tracing.
  */
 #if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) ||		\
     !defined(CONFIG_OPTIMIZE_INLINING) || (__GNUC__ < 4)
-#define inline inline		__attribute__((always_inline,unused)) notrace
-#define __inline__ __inline__	__attribute__((always_inline,unused)) notrace
-#define __inline __inline	__attribute__((always_inline,unused)) notrace
+#define inline \
+	inline __attribute__((always_inline, unused)) notrace __gnu_inline
 #else
-/* A lot of inline functions can cause havoc with function tracing */
-#define inline inline		__attribute__((unused)) notrace
-#define __inline__ __inline__	__attribute__((unused)) notrace
-#define __inline __inline	__attribute__((unused)) notrace
+#define inline inline		__attribute__((unused)) notrace __gnu_inline
 #endif
 
+#define __inline__ inline
+#define __inline inline
 #define __always_inline	inline __attribute__((always_inline))
 #define  noinline	__attribute__((noinline))
 
-- 
2.18.0.rc1.244.gcf134e6275-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v5 2/3] x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h>
  2018-06-13 21:05 [PATCH v5 0/3] extern inline native_save_fl for paravirt Nick Desaulniers
  2018-06-13 21:05 ` [PATCH v5 1/3] compiler-gcc.h: add gnu_inline to all inline declarations Nick Desaulniers
@ 2018-06-13 21:05 ` Nick Desaulniers
  2018-06-14 20:47   ` H. Peter Anvin
  2018-06-13 21:05 ` [PATCH v5 3/3] x86: paravirt: make native_save_fl extern inline Nick Desaulniers
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Nick Desaulniers @ 2018-06-13 21:05 UTC (permalink / raw)
  To: akpm, hpa, mingo, tglx
  Cc: linux-efi, linux-kernel, x86, virtualization, astrachan,
	manojgupta, ghackmann, sedat.dilek, tstellar, keescook,
	yamada.masahiro, michal.lkml, linux-kbuild, geert, will.deacon,
	mawilcox, arnd, rientjes, acme, pombredanne, aryabinin, kstewart,
	boris.ostrovsky, jan.kiszka, rostedt, kirill.shutemov,
	ard.biesheuvel, akataria, brijesh.singh, caoj.fnst, gregkh,
	jarkko.sakkinen, jgross, jpoimboe, mka, ndesaulniers,
	thomas.lendacky, tweek, mjg59, joe, andrea.parri, H. Peter Anvin

From: "H. Peter Anvin" <hpa@linux.intel.com>

i386 and x86-64 uses different registers for arguments; make them
available so we don't have to #ifdef in the actual code.

Native size and specified size (q, l, w, b) versions are provided.

Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/x86/include/asm/asm.h | 59 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
index 219faaec51df..990770f9e76b 100644
--- a/arch/x86/include/asm/asm.h
+++ b/arch/x86/include/asm/asm.h
@@ -46,6 +46,65 @@
 #define _ASM_SI		__ASM_REG(si)
 #define _ASM_DI		__ASM_REG(di)
 
+#ifndef __x86_64__
+/* 32 bit */
+
+#define _ASM_ARG1	_ASM_AX
+#define _ASM_ARG2	_ASM_DX
+#define _ASM_ARG3	_ASM_CX
+
+#define _ASM_ARG1L	eax
+#define _ASM_ARG2L	edx
+#define _ASM_ARG3L	ecx
+
+#define _ASM_ARG1W	ax
+#define _ASM_ARG2W	dx
+#define _ASM_ARG3W	cx
+
+#define _ASM_ARG1B	al
+#define _ASM_ARG2B	dl
+#define _ASM_ARG3B	cl
+
+#else
+/* 64 bit */
+
+#define _ASM_ARG1	_ASM_DI
+#define _ASM_ARG2	_ASM_SI
+#define _ASM_ARG3	_ASM_DX
+#define _ASM_ARG4	_ASM_CX
+#define _ASM_ARG5	r8
+#define _ASM_ARG6	r9
+
+#define _ASM_ARG1Q	rdi
+#define _ASM_ARG2Q	rsi
+#define _ASM_ARG3Q	rdx
+#define _ASM_ARG4Q	rcx
+#define _ASM_ARG5Q	r8
+#define _ASM_ARG6Q	r9
+
+#define _ASM_ARG1L	edi
+#define _ASM_ARG2L	esi
+#define _ASM_ARG3L	edx
+#define _ASM_ARG4L	ecx
+#define _ASM_ARG5L	r8d
+#define _ASM_ARG6L	r9d
+
+#define _ASM_ARG1W	di
+#define _ASM_ARG2W	si
+#define _ASM_ARG3W	dx
+#define _ASM_ARG4W	cx
+#define _ASM_ARG5W	r8w
+#define _ASM_ARG6W	r9w
+
+#define _ASM_ARG1B	dil
+#define _ASM_ARG2B	sil
+#define _ASM_ARG3B	dl
+#define _ASM_ARG4B	cl
+#define _ASM_ARG5B	r8b
+#define _ASM_ARG6B	r9b
+
+#endif
+
 /*
  * Macros to generate condition code outputs from inline assembly,
  * The output operand must be type "bool".
-- 
2.18.0.rc1.244.gcf134e6275-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v5 3/3] x86: paravirt: make native_save_fl extern inline
  2018-06-13 21:05 [PATCH v5 0/3] extern inline native_save_fl for paravirt Nick Desaulniers
  2018-06-13 21:05 ` [PATCH v5 1/3] compiler-gcc.h: add gnu_inline to all inline declarations Nick Desaulniers
  2018-06-13 21:05 ` [PATCH v5 2/3] x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h> Nick Desaulniers
@ 2018-06-13 21:05 ` Nick Desaulniers
  2018-06-14 13:20 ` [PATCH v5 0/3] extern inline native_save_fl for paravirt Sedat Dilek
  2018-06-19 15:03 ` Juergen Gross
  4 siblings, 0 replies; 12+ messages in thread
From: Nick Desaulniers @ 2018-06-13 21:05 UTC (permalink / raw)
  To: akpm, hpa, mingo, tglx
  Cc: linux-efi, linux-kernel, x86, virtualization, astrachan,
	manojgupta, ghackmann, sedat.dilek, tstellar, keescook,
	yamada.masahiro, michal.lkml, linux-kbuild, geert, will.deacon,
	mawilcox, arnd, rientjes, acme, pombredanne, aryabinin, kstewart,
	boris.ostrovsky, jan.kiszka, rostedt, kirill.shutemov,
	ard.biesheuvel, akataria, brijesh.singh, caoj.fnst, gregkh,
	jarkko.sakkinen, jgross, jpoimboe, mka, ndesaulniers,
	thomas.lendacky, tweek, mjg59, joe, andrea.parri

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.

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.rc1.244.gcf134e6275-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/3] extern inline native_save_fl for paravirt
  2018-06-13 21:05 [PATCH v5 0/3] extern inline native_save_fl for paravirt Nick Desaulniers
                   ` (2 preceding siblings ...)
  2018-06-13 21:05 ` [PATCH v5 3/3] x86: paravirt: make native_save_fl extern inline Nick Desaulniers
@ 2018-06-14 13:20 ` Sedat Dilek
  2018-06-19 15:03 ` Juergen Gross
  4 siblings, 0 replies; 12+ messages in thread
From: Sedat Dilek @ 2018-06-14 13:20 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: akpm, hpa, mingo, tglx, linux-efi, linux-kernel, x86,
	virtualization, astrachan, manojgupta, ghackmann, tstellar,
	keescook, yamada.masahiro, michal.lkml, linux-kbuild, geert,
	will.deacon, mawilcox, Arnd Bergmann, rientjes, acme,
	pombredanne, aryabinin, kstewart, boris.ostrovsky, jan.kiszka,
	rostedt, kirill.shutemov, ard.biesheuvel, akataria,
	brijesh.singh, caoj.fnst, gregkh, jarkko.sakkinen, jgross,
	jpoimboe, mka, thomas.lendacky, tweek, mjg59, joe, andrea.parri

On Wed, Jun 13, 2018 at 11:05 PM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> paravirt depends on a custom calling convention (callee saved), but
> expects this from a static inline function that it then forces to be
> outlined. This is problematic because different compilers or flags can
> then add a stack guard that violates the calling conventions.
>
> Uses extern inline with the out-of-line definition in assembly to
> prevent compilers from adding stack guards to the outlined version.
>
> Other parts of the codebase overwrite KBUILD_CFLAGS, which is *extremely
> problematic* for extern inline, as the sematics are completely the
> opposite depending on what C standard is used.
> http://blahg.josefsipek.net/?p=529
>
> Changes since v4:
>   Take Arnd's and hpa's suggestions properly feature detect gnu_inline
>   attribute to support gcc 4.1.
>
> Changes since v3:
>   Take Joe's suggestion to hoist __inline__ and __inline out of
>   conditional block.
>
> Changes since v2:
>   Take hpa's _ASM_ARG patch into the set in order to simplify cross
>   32b/64b x86 assembly and rebase my final patch to use it.  Apply
>   Sedat's typo fix to commit message and sign off on it. Take Joe's
>   suggestion to simplify __inline__ and __inline. Add Arnd to list of
>   folks who made helpful suggestions.
>
> Changes since v1:
>   Prefer gnu_inline function attribute instead of explicitly setting C
>   standard compiler flag in problematic Makefiles. We should instead
>   carefully evaluate if those Makefiles should be overwriting
>   KBUILD_CFLAGS at all. Dropped the previous first two patches and added
>   a new first patch.
>
> H. Peter Anvin (1):
>   x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h>
>
> Nick Desaulniers (2):
>   compiler-gcc.h: add gnu_inline to all inline declarations
>   x86: paravirt: make native_save_fl extern inline
>
>  arch/x86/include/asm/asm.h      | 59 +++++++++++++++++++++++++++++++++
>  arch/x86/include/asm/irqflags.h |  2 +-
>  arch/x86/kernel/Makefile        |  1 +
>  arch/x86/kernel/irqflags.S      | 26 +++++++++++++++
>  include/linux/compiler-gcc.h    | 29 ++++++++++++----
>  5 files changed, 109 insertions(+), 8 deletions(-)
>  create mode 100644 arch/x86/kernel/irqflags.S
>
> --
> 2.18.0.rc1.244.gcf134e6275-goog
>

Hi Nick,

thanks for v5 of your patchset.

I was able to boot into bare metal.

root@iniza:~# cat /proc/version
Linux version 4.14.49-1-iniza-llvmlinux (sedat.dilek@gmail.com@iniza)
(clang version 7.0.0- (trunk)) #1 SMP 2018-06-14

root@iniza:~# dpkg -l | grep clang-7
ii  clang-7
1:7~svn334344-1~exp1+0~20180609091248.1814~1.gbp8ccab3 amd64        C,
C++ and Objective-C compiler

Feel free to add my Tested-By.

Regards,
- Sedat -

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 2/3] x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h>
  2018-06-13 21:05 ` [PATCH v5 2/3] x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h> Nick Desaulniers
@ 2018-06-14 20:47   ` H. Peter Anvin
  2018-06-14 20:59     ` Nick Desaulniers
  0 siblings, 1 reply; 12+ messages in thread
From: H. Peter Anvin @ 2018-06-14 20:47 UTC (permalink / raw)
  To: Nick Desaulniers, akpm, mingo, tglx
  Cc: linux-efi, linux-kernel, x86, virtualization, astrachan,
	manojgupta, ghackmann, sedat.dilek, tstellar, keescook,
	yamada.masahiro, michal.lkml, linux-kbuild, geert, will.deacon,
	mawilcox, arnd, rientjes, acme, pombredanne, aryabinin, kstewart,
	boris.ostrovsky, jan.kiszka, rostedt, kirill.shutemov,
	ard.biesheuvel, akataria, brijesh.singh, caoj.fnst, gregkh,
	jarkko.sakkinen, jgross, jpoimboe, mka, thomas.lendacky, tweek,
	mjg59, joe, andrea.parri, H. Peter Anvin

On 06/13/18 14:05, Nick Desaulniers wrote:
> From: "H. Peter Anvin" <hpa@linux.intel.com>
> 
> i386 and x86-64 uses different registers for arguments; make them
> available so we don't have to #ifdef in the actual code.
> 
> Native size and specified size (q, l, w, b) versions are provided.
> 
> Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>
> Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

I still object to Suggested-by: here.  Sedat did a correction, which is
a Reviewed-by:, but unless I'm completely out to sea, there was no
suggestion on Sedat's part of this; and I had certainly not seen it when
I wrote the code.

	-hpa


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 2/3] x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h>
  2018-06-14 20:47   ` H. Peter Anvin
@ 2018-06-14 20:59     ` Nick Desaulniers
  2018-06-15  0:17       ` H. Peter Anvin
  2018-06-15  4:04       ` Sedat Dilek
  0 siblings, 2 replies; 12+ messages in thread
From: Nick Desaulniers @ 2018-06-14 20:59 UTC (permalink / raw)
  To: hpa
  Cc: Andrew Morton, mingo, Thomas Gleixner, linux-efi, LKML, x86,
	virtualization, Alistair Strachan, Manoj Gupta, Greg Hackmann,
	sedat.dilek, tstellar, Kees Cook, Masahiro Yamada, Michal Marek,
	Linux Kbuild mailing list, geert, Will Deacon, mawilcox,
	Arnd Bergmann, David Rientjes, acme, Philippe Ombredanne,
	Andrey Ryabinin, Kate Stewart, boris.ostrovsky, J. Kiszka,
	rostedt, kirill.shutemov, Ard Biesheuvel, akataria,
	brijesh.singh, Cao jin, Greg KH, jarkko.sakkinen, jgross,
	Josh Poimboeuf, Matthias Kaehlcke, thomas.lendacky,
	Thiebaud Weksteen, mjg59, joe, andrea.parri, H. Peter Anvin

On Thu, Jun 14, 2018 at 1:48 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> On 06/13/18 14:05, Nick Desaulniers wrote:
> > From: "H. Peter Anvin" <hpa@linux.intel.com>
> >
> > i386 and x86-64 uses different registers for arguments; make them
> > available so we don't have to #ifdef in the actual code.
> >
> > Native size and specified size (q, l, w, b) versions are provided.
> >
> > Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>
> > Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>
> I still object to Suggested-by: here.  Sedat did a correction, which is
> a Reviewed-by:, but unless I'm completely out to sea, there was no
> suggestion on Sedat's part of this; and I had certainly not seen it when
> I wrote the code.

I'm fine with changing it from a Suggested-by to a Reviewed-by.  Can
you do that when you apply the set, or should I send a v6?
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 2/3] x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h>
  2018-06-14 20:59     ` Nick Desaulniers
@ 2018-06-15  0:17       ` H. Peter Anvin
  2018-06-20 16:40         ` Nick Desaulniers
  2018-06-15  4:04       ` Sedat Dilek
  1 sibling, 1 reply; 12+ messages in thread
From: H. Peter Anvin @ 2018-06-15  0:17 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Andrew Morton, mingo, Thomas Gleixner, linux-efi, LKML, x86,
	virtualization, Alistair Strachan, Manoj Gupta, Greg Hackmann,
	sedat.dilek, tstellar, Kees Cook, Masahiro Yamada, Michal Marek,
	Linux Kbuild mailing list, geert, Will Deacon, mawilcox,
	Arnd Bergmann, David Rientjes, acme, Philippe Ombredanne,
	Andrey Ryabinin, Kate Stewart, boris.ostrovsky, J. Kiszka,
	rostedt, kirill.shutemov, Ard Biesheuvel, akataria,
	brijesh.singh, Cao jin, Greg KH, jarkko.sakkinen, jgross,
	Josh Poimboeuf, Matthias Kaehlcke, thomas.lendacky,
	Thiebaud Weksteen, mjg59, joe, andrea.parri, H. Peter Anvin

On 06/14/18 13:59, Nick Desaulniers wrote:
> On Thu, Jun 14, 2018 at 1:48 PM H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On 06/13/18 14:05, Nick Desaulniers wrote:
>>> From: "H. Peter Anvin" <hpa@linux.intel.com>
>>>
>>> i386 and x86-64 uses different registers for arguments; make them
>>> available so we don't have to #ifdef in the actual code.
>>>
>>> Native size and specified size (q, l, w, b) versions are provided.
>>>
>>> Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>
>>> Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
>>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>>
>> I still object to Suggested-by: here.  Sedat did a correction, which is
>> a Reviewed-by:, but unless I'm completely out to sea, there was no
>> suggestion on Sedat's part of this; and I had certainly not seen it when
>> I wrote the code.
> 
> I'm fine with changing it from a Suggested-by to a Reviewed-by.  Can
> you do that when you apply the set, or should I send a v6?
> 

I'm not handling patch mechanics for x86 at the moment.

	-hpa


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 2/3] x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h>
  2018-06-14 20:59     ` Nick Desaulniers
  2018-06-15  0:17       ` H. Peter Anvin
@ 2018-06-15  4:04       ` Sedat Dilek
  1 sibling, 0 replies; 12+ messages in thread
From: Sedat Dilek @ 2018-06-15  4:04 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: hpa, Andrew Morton, mingo, Thomas Gleixner, linux-efi, LKML, x86,
	virtualization, Alistair Strachan, Manoj Gupta, Greg Hackmann,
	tstellar, Kees Cook, Masahiro Yamada, Michal Marek,
	Linux Kbuild mailing list, geert, Will Deacon, mawilcox,
	Arnd Bergmann, David Rientjes, acme, Philippe Ombredanne,
	Andrey Ryabinin, Kate Stewart, boris.ostrovsky, J. Kiszka,
	rostedt, kirill.shutemov, Ard Biesheuvel, akataria,
	brijesh.singh, Cao jin, Greg KH, jarkko.sakkinen, jgross,
	Josh Poimboeuf, Matthias Kaehlcke, thomas.lendacky,
	Thiebaud Weksteen, mjg59, joe, andrea.parri, H. Peter Anvin

On Thu, Jun 14, 2018 at 10:59 PM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> On Thu, Jun 14, 2018 at 1:48 PM H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On 06/13/18 14:05, Nick Desaulniers wrote:
>> > From: "H. Peter Anvin" <hpa@linux.intel.com>
>> >
>> > i386 and x86-64 uses different registers for arguments; make them
>> > available so we don't have to #ifdef in the actual code.
>> >
>> > Native size and specified size (q, l, w, b) versions are provided.
>> >
>> > Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>
>> > Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
>> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>>
>> I still object to Suggested-by: here.  Sedat did a correction, which is
>> a Reviewed-by:, but unless I'm completely out to sea, there was no
>> suggestion on Sedat's part of this; and I had certainly not seen it when
>> I wrote the code.
>
> I'm fine with changing it from a Suggested-by to a Reviewed-by.  Can
> you do that when you apply the set, or should I send a v6?
>

Hi,

this issue with Clang and Paravirt is such an ancient issue.
Back to the days in 2011 before LLVMlinux was born, the project was
called lll-project.
Fact is the issue is today fixed.
Not sure about the credits Suggested-by/Reviewed-by in the
Linux-kernel development process.
I did not count the number of compiler runs here.
A Tested-by is OK for me.
Anyway, the beast is dead, this counts for me.

Regards,
- Sedat -

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/3] extern inline native_save_fl for paravirt
  2018-06-13 21:05 [PATCH v5 0/3] extern inline native_save_fl for paravirt Nick Desaulniers
                   ` (3 preceding siblings ...)
  2018-06-14 13:20 ` [PATCH v5 0/3] extern inline native_save_fl for paravirt Sedat Dilek
@ 2018-06-19 15:03 ` Juergen Gross
  4 siblings, 0 replies; 12+ messages in thread
From: Juergen Gross @ 2018-06-19 15:03 UTC (permalink / raw)
  To: Nick Desaulniers, akpm, hpa, mingo, tglx
  Cc: linux-efi, linux-kernel, x86, virtualization, astrachan,
	manojgupta, ghackmann, sedat.dilek, tstellar, keescook,
	yamada.masahiro, michal.lkml, linux-kbuild, geert, will.deacon,
	mawilcox, arnd, rientjes, acme, pombredanne, aryabinin, kstewart,
	boris.ostrovsky, jan.kiszka, rostedt, kirill.shutemov,
	ard.biesheuvel, akataria, brijesh.singh, caoj.fnst, gregkh,
	jarkko.sakkinen, jpoimboe, mka, thomas.lendacky, tweek, mjg59,
	joe, andrea.parri

On 13/06/18 23:05, Nick Desaulniers wrote:
> paravirt depends on a custom calling convention (callee saved), but
> expects this from a static inline function that it then forces to be
> outlined. This is problematic because different compilers or flags can
> then add a stack guard that violates the calling conventions.
> 
> Uses extern inline with the out-of-line definition in assembly to
> prevent compilers from adding stack guards to the outlined version.
> 
> Other parts of the codebase overwrite KBUILD_CFLAGS, which is *extremely
> problematic* for extern inline, as the sematics are completely the
> opposite depending on what C standard is used.
> http://blahg.josefsipek.net/?p=529
> 
> Changes since v4:
>   Take Arnd's and hpa's suggestions properly feature detect gnu_inline
>   attribute to support gcc 4.1.
> 
> Changes since v3:
>   Take Joe's suggestion to hoist __inline__ and __inline out of
>   conditional block.
> 
> Changes since v2:
>   Take hpa's _ASM_ARG patch into the set in order to simplify cross
>   32b/64b x86 assembly and rebase my final patch to use it.  Apply
>   Sedat's typo fix to commit message and sign off on it. Take Joe's
>   suggestion to simplify __inline__ and __inline. Add Arnd to list of
>   folks who made helpful suggestions.
> 
> Changes since v1:
>   Prefer gnu_inline function attribute instead of explicitly setting C
>   standard compiler flag in problematic Makefiles. We should instead
>   carefully evaluate if those Makefiles should be overwriting
>   KBUILD_CFLAGS at all. Dropped the previous first two patches and added
>   a new first patch.
> 
> H. Peter Anvin (1):
>   x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h>
> 
> Nick Desaulniers (2):
>   compiler-gcc.h: add gnu_inline to all inline declarations
>   x86: paravirt: make native_save_fl extern inline
> 
>  arch/x86/include/asm/asm.h      | 59 +++++++++++++++++++++++++++++++++
>  arch/x86/include/asm/irqflags.h |  2 +-
>  arch/x86/kernel/Makefile        |  1 +
>  arch/x86/kernel/irqflags.S      | 26 +++++++++++++++
>  include/linux/compiler-gcc.h    | 29 ++++++++++++----
>  5 files changed, 109 insertions(+), 8 deletions(-)
>  create mode 100644 arch/x86/kernel/irqflags.S
> 

For the series:

Acked-by: Juergen Gross <jgross@suse.com>


Juergen

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 2/3] x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h>
  2018-06-15  0:17       ` H. Peter Anvin
@ 2018-06-20 16:40         ` Nick Desaulniers
  2018-06-21 12:28           ` Ingo Molnar
  0 siblings, 1 reply; 12+ messages in thread
From: Nick Desaulniers @ 2018-06-20 16:40 UTC (permalink / raw)
  To: hpa, mingo, Thomas Gleixner
  Cc: Andrew Morton, linux-efi, LKML, x86, virtualization,
	Alistair Strachan, Manoj Gupta, Greg Hackmann, sedat.dilek,
	tstellar, Kees Cook, Masahiro Yamada, Michal Marek,
	Linux Kbuild mailing list, geert, Will Deacon, mawilcox,
	Arnd Bergmann, David Rientjes, acme, Philippe Ombredanne,
	Andrey Ryabinin, Kate Stewart, boris.ostrovsky, J. Kiszka,
	rostedt, kirill.shutemov, Ard Biesheuvel, akataria,
	brijesh.singh, Cao jin, Greg KH, jarkko.sakkinen, jgross,
	Josh Poimboeuf, Matthias Kaehlcke, thomas.lendacky,
	Thiebaud Weksteen, mjg59, joe, andrea.parri, H. Peter Anvin

On Thu, Jun 14, 2018 at 5:17 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> On 06/14/18 13:59, Nick Desaulniers wrote:
> > On Thu, Jun 14, 2018 at 1:48 PM H. Peter Anvin <hpa@zytor.com> wrote:
> >>
> >> On 06/13/18 14:05, Nick Desaulniers wrote:
> >>> From: "H. Peter Anvin" <hpa@linux.intel.com>
> >>>
> >>> i386 and x86-64 uses different registers for arguments; make them
> >>> available so we don't have to #ifdef in the actual code.
> >>>
> >>> Native size and specified size (q, l, w, b) versions are provided.
> >>>
> >>> Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>
> >>> Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
> >>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> >>
> >> I still object to Suggested-by: here.  Sedat did a correction, which is
> >> a Reviewed-by:, but unless I'm completely out to sea, there was no
> >> suggestion on Sedat's part of this; and I had certainly not seen it when
> >> I wrote the code.
> >
> > I'm fine with changing it from a Suggested-by to a Reviewed-by.  Can
> > you do that when you apply the set, or should I send a v6?
> >
>
> I'm not handling patch mechanics for x86 at the moment.

Hi Ingo and Thomas,
Have you had a chance to review this patch series for application?

Note that Juergen (the paravirt maintainer) acked the series:
https://bugs.llvm.org/show_bug.cgi?id=37880
The only issue being swapping a Suggested-by for a Reviewed-by tag for
one commit, as above.

--
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 2/3] x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h>
  2018-06-20 16:40         ` Nick Desaulniers
@ 2018-06-21 12:28           ` Ingo Molnar
  0 siblings, 0 replies; 12+ messages in thread
From: Ingo Molnar @ 2018-06-21 12:28 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: hpa, mingo, Thomas Gleixner, Andrew Morton, linux-efi, LKML, x86,
	virtualization, Alistair Strachan, Manoj Gupta, Greg Hackmann,
	sedat.dilek, tstellar, Kees Cook, Masahiro Yamada, Michal Marek,
	Linux Kbuild mailing list, geert, Will Deacon, mawilcox,
	Arnd Bergmann, David Rientjes, acme, Philippe Ombredanne,
	Andrey Ryabinin, Kate Stewart, boris.ostrovsky, J. Kiszka,
	rostedt, kirill.shutemov, Ard Biesheuvel, akataria,
	brijesh.singh, Cao jin, Greg KH, jarkko.sakkinen, jgross,
	Josh Poimboeuf, Matthias Kaehlcke, thomas.lendacky,
	Thiebaud Weksteen, mjg59, joe, andrea.parri, H. Peter Anvin


* Nick Desaulniers <ndesaulniers@google.com> wrote:

> On Thu, Jun 14, 2018 at 5:17 PM H. Peter Anvin <hpa@zytor.com> wrote:
> >
> > On 06/14/18 13:59, Nick Desaulniers wrote:
> > > On Thu, Jun 14, 2018 at 1:48 PM H. Peter Anvin <hpa@zytor.com> wrote:
> > >>
> > >> On 06/13/18 14:05, Nick Desaulniers wrote:
> > >>> From: "H. Peter Anvin" <hpa@linux.intel.com>
> > >>>
> > >>> i386 and x86-64 uses different registers for arguments; make them
> > >>> available so we don't have to #ifdef in the actual code.
> > >>>
> > >>> Native size and specified size (q, l, w, b) versions are provided.
> > >>>
> > >>> Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>
> > >>> Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
> > >>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > >>
> > >> I still object to Suggested-by: here.  Sedat did a correction, which is
> > >> a Reviewed-by:, but unless I'm completely out to sea, there was no
> > >> suggestion on Sedat's part of this; and I had certainly not seen it when
> > >> I wrote the code.
> > >
> > > I'm fine with changing it from a Suggested-by to a Reviewed-by.  Can
> > > you do that when you apply the set, or should I send a v6?
> > >
> >
> > I'm not handling patch mechanics for x86 at the moment.
> 
> Hi Ingo and Thomas,
> Have you had a chance to review this patch series for application?
> 
> Note that Juergen (the paravirt maintainer) acked the series:
> https://bugs.llvm.org/show_bug.cgi?id=37880
> The only issue being swapping a Suggested-by for a Reviewed-by tag for
> one commit, as above.

Could you please update the series with all suggestions and tags and send a new 
series?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-06-21 12:28 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-13 21:05 [PATCH v5 0/3] extern inline native_save_fl for paravirt Nick Desaulniers
2018-06-13 21:05 ` [PATCH v5 1/3] compiler-gcc.h: add gnu_inline to all inline declarations Nick Desaulniers
2018-06-13 21:05 ` [PATCH v5 2/3] x86/asm: add _ASM_ARG* constants for argument registers to <asm/asm.h> Nick Desaulniers
2018-06-14 20:47   ` H. Peter Anvin
2018-06-14 20:59     ` Nick Desaulniers
2018-06-15  0:17       ` H. Peter Anvin
2018-06-20 16:40         ` Nick Desaulniers
2018-06-21 12:28           ` Ingo Molnar
2018-06-15  4:04       ` Sedat Dilek
2018-06-13 21:05 ` [PATCH v5 3/3] x86: paravirt: make native_save_fl extern inline Nick Desaulniers
2018-06-14 13:20 ` [PATCH v5 0/3] extern inline native_save_fl for paravirt Sedat Dilek
2018-06-19 15:03 ` Juergen Gross

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).