On Wed, Jan 17, 2018 at 10:10 AM, Mathieu Desnoyers
[off-list ref] wrote:

----- On Jan 17, 2018, at 12:53 PM, Andy Lutomirski luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org wrote:

quoted

On Wed, Jan 17, 2018 at 8:54 AM, Mathieu Desnoyers
[off-list ref] wrote:

quoted

Ensure that a core serializing instruction is issued before returning to
user-mode. x86 implements return to user-space through sysexit, sysrel,
and sysretq, which are not core serializing.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
Reviewed-by: Thomas Gleixner <redacted>
CC: Peter Zijlstra <redacted>
CC: Andy Lutomirski <redacted>
CC: Paul E. McKenney <redacted>
CC: Boqun Feng <redacted>
CC: Andrew Hunter <redacted>
CC: Maged Michael <redacted>
CC: Avi Kivity <avi-VrcmuVmyx1hWk0Htik3J/w@public.gmane.org>
CC: Benjamin Herrenschmidt <redacted>
CC: Paul Mackerras <redacted>
CC: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
CC: Dave Watson <redacted>
CC: Ingo Molnar <redacted>
CC: "H. Peter Anvin" <redacted>
CC: Andrea Parri <redacted>
CC: Russell King <redacted>
CC: Greg Hackmann <redacted>
CC: Will Deacon <redacted>
CC: David Sehr <redacted>
CC: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
CC: x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
CC: linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
Changes since v1:
- Fix prototype of sync_core_before_usermode in generic code (missing
  return type).
- Add linux/processor.h include to sched/core.c.
- Add ARCH_HAS_SYNC_CORE_BEFORE_USERMODE to init/Kconfig.
- Fix linux/processor.h ifdef to target
  CONFIG_ARCH_HAS_SYNC_CORE_BEFORE_USERMODE rather than
  ARCH_HAS_SYNC_CORE_BEFORE_USERMODE.
- Move empty static inline in processor.h to generic patch.
---
 arch/x86/Kconfig                 |  1 +
 arch/x86/include/asm/processor.h | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 20da391b5f32..0b44c8dd0e95 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig

@@ -61,6 +61,7 @@ config X86
        select ARCH_HAS_SG_CHAIN
        select ARCH_HAS_STRICT_KERNEL_RWX
        select ARCH_HAS_STRICT_MODULE_RWX
+       select ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
        select ARCH_HAS_UBSAN_SANITIZE_ALL
        select ARCH_HAS_ZONE_DEVICE             if X86_64
        select ARCH_HAVE_NMI_SAFE_CMPXCHG

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index d3a67fba200a..3257d34dbb40 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h

@@ -722,6 +722,16 @@ static inline void sync_core(void)
 #endif
 }

+/*
+ * Ensure that a core serializing instruction is issued before returning
+ * to user-mode. x86 implements return to user-space through sysexit,
+ * sysrel, and sysretq, which are not core serializing.
+ */
+static inline void sync_core_before_usermode(void)
+{

/* With PTI, we unconditionally serialize before running user code. */
if (static_cpu_has(X86_FEATURE_PTI))
 return;

One issue I'm facing with this change is header dependency:
sync_core_before_usermode() is currently implemented in
arch/x86/include/asm/processor.h, but arch/x86/include/asm/cpufeature.h
is needed for static_cpu_has, and it happens to include
asm/processor.h.

I'm facing a similar issue for adding a (in_irq() || in_nmi()) check.

Should we move sync_core_before_usermode() to a different header, and if
so, any suggestion ?

tlbflush.h, maybe?

Thanks,

Mathieu

quoted

quoted

+       sync_core();
+}

--Andy

--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com