[PATCH -tip v4 1/4] x86/fsgsbase: Remove unnecessary "memory" clobbers from FS/GS base accessors

[PATCH -tip v4 1/4] x86/fsgsbase: Remove unnecessary "memory" clobbers from FS/GS base accessors

[Uros Bizjak]

The rdfsbase() and rdgsbase() helpers currently include a "memory"
clobber in their inline assembly definitions. However, the RDFSBASE
and RDGSBASE instructions only read the FS/GS base MSRs into a
general-purpose register and do not access memory. The "memory" clobber,
which acts as a compiler barrier and may inhibit optimization,
is therefore unnecessary.

The "memory" clobber was historically used as a scheduling constraint
to prevent the compiler from moving the instructions before preceding
segment register loads. This is not required because both the segment
register loads and the RDFSBASE/RDGSBASE accessors are implemented
with `asm volatile`, which already prevents reordering between them.

No functional change intended.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Acked-by: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
v2: Note that the "memory" clobber was historically
    used as a scheduling constraint.
---
 arch/x86/include/asm/fsgsbase.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/fsgsbase.h b/arch/x86/include/asm/fsgsbase.h
index ab2547f97c2c..70ff4ef457b1 100644
--- a/arch/x86/include/asm/fsgsbase.h
+++ b/arch/x86/include/asm/fsgsbase.h
@@ -25,7 +25,7 @@ static __always_inline unsigned long rdfsbase(void)
 {
 	unsigned long fsbase;
 
-	asm volatile("rdfsbase %0" : "=r" (fsbase) :: "memory");
+	asm volatile("rdfsbase %0" : "=r" (fsbase));
 
 	return fsbase;
 }
@@ -34,7 +34,7 @@ static __always_inline unsigned long rdgsbase(void)
 {
 	unsigned long gsbase;
 
-	asm volatile("rdgsbase %0" : "=r" (gsbase) :: "memory");
+	asm volatile("rdgsbase %0" : "=r" (gsbase));
 
 	return gsbase;
 }
-- 
2.53.0

[PATCH -tip v4 2/4] x86/segment: Remove unnecessary memory clobber from savesegment()

[Uros Bizjak]

The savesegment() macro uses inline assembly to copy a segment register
into a general-purpose register:

  movl %seg, reg

This instruction does not access memory, yet the inline asm currently
declares a "memory" clobber, which unnecessarily acts as a compiler
barrier and may inhibit optimization.

Remove the "memory" clobber and mark the asm as `asm volatile` instead.
Segment register loads in the kernel are implemented using `asm volatile`,
so the compiler will not schedule segment register reads before those
loads. Using `asm volatile` preserves the intended ordering with other
segment register operations without imposing an unnecessary global memory
barrier.

No functional change intended.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Acked-by: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/x86/include/asm/segment.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
index 9f5be2bbd291..3fe3a310844c 100644
--- a/arch/x86/include/asm/segment.h
+++ b/arch/x86/include/asm/segment.h
@@ -348,7 +348,7 @@ static inline void __loadsegment_fs(unsigned short value)
  * Save a segment register away:
  */
 #define savesegment(seg, value)				\
-	asm("movl %%" #seg ",%k0" : "=r" (value) : : "memory")
+	asm volatile("movl %%" #seg ",%k0" : "=r" (value))
 
 #endif /* !__ASSEMBLER__ */
 #endif /* __KERNEL__ */
-- 
2.53.0

[PATCH -tip v4 3/4] x86/segment: Use ASM_INPUT_RM in __loadsegment_fs()

[Uros Bizjak]

Use the ASM_INPUT_RM macro in __loadsegment_fs() to work around clang
problems with "rm" asm constraint. clang seems to always chose the
memory input, while it is almost always the worst choice.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Acked-by: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Acked-by: Nathan Chancellor <nathan@kernel.org>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Nick Desaulniers <nick.desaulniers+lkml@gmail.com>
Cc: Bill Wendling <morbo@google.com>
Cc: Justin Stitt <justinstitt@google.com>
---
 arch/x86/include/asm/segment.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
index 3fe3a310844c..0f4283dcd0c4 100644
--- a/arch/x86/include/asm/segment.h
+++ b/arch/x86/include/asm/segment.h
@@ -335,7 +335,7 @@ static inline void __loadsegment_fs(unsigned short value)
 
 		     _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_CLEAR_FS)
 
-		     : : "rm" (value) : "memory");
+		     : : ASM_INPUT_RM (value) : "memory");
 }
 
 /* __loadsegment_gs is intentionally undefined.  Use load_gs_index instead. */
-- 
2.53.0

[PATCH -tip v4 4/4] x86/segment: Implement loadsegment()/savesegment() macros with static inline helpers

[Uros Bizjak]

Convert the __loadsegment_simple() and savesegment() macro
implementations into static inline helper functions generated
via small helper macros.

Historically loadsegment() and savesegment() relied on macros that
embedded inline assembly. This approach obscures types, complicates
debugging, and makes the call sites harder for the compiler and static
analysis tools to reason about.

This change is purely mechanical and does not alter the generated code,
but improves readability, type safety, and compiler visibility of the
helpers.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
---
v3: New patch in series.
v4: Use u16 instead of unsigned short.
---
 arch/x86/include/asm/segment.h | 57 +++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
index 0f4283dcd0c4..dbd90fede5e7 100644
--- a/arch/x86/include/asm/segment.h
+++ b/arch/x86/include/asm/segment.h
@@ -302,19 +302,17 @@ extern const char xen_early_idt_handler_array[NUM_EXCEPTION_VECTORS][XEN_EARLY_I
  * failure to fully clear the cached descriptor is only observable for
  * FS and GS.
  */
-#define __loadsegment_simple(seg, value)				\
-do {									\
-	unsigned short __val = (value);					\
-									\
-	asm volatile("						\n"	\
-		     "1:	movl %k0,%%" #seg "		\n"	\
+#define LOAD_SEGMENT(seg)						\
+static inline void __loadsegment_##seg(u16 value)			\
+{									\
+	asm volatile("1:	movl %k0,%%" #seg "\n"			\
 		     _ASM_EXTABLE_TYPE_REG(1b, 1b, EX_TYPE_ZERO_REG, %k0)\
-		     : "+r" (__val) : : "memory");			\
-} while (0)
+		     : "+r" (value) : : "memory");			\
+}
 
-#define __loadsegment_ss(value) __loadsegment_simple(ss, (value))
-#define __loadsegment_ds(value) __loadsegment_simple(ds, (value))
-#define __loadsegment_es(value) __loadsegment_simple(es, (value))
+LOAD_SEGMENT(ss)
+LOAD_SEGMENT(ds)
+LOAD_SEGMENT(es)
 
 #ifdef CONFIG_X86_32
 
@@ -322,19 +320,16 @@ do {									\
  * On 32-bit systems, the hidden parts of FS and GS are unobservable if
  * the selector is NULL, so there's no funny business here.
  */
-#define __loadsegment_fs(value) __loadsegment_simple(fs, (value))
-#define __loadsegment_gs(value) __loadsegment_simple(gs, (value))
+LOAD_SEGMENT(fs)
+LOAD_SEGMENT(gs)
 
 #else
 
-static inline void __loadsegment_fs(unsigned short value)
+static inline void __loadsegment_fs(u16 value)
 {
-	asm volatile("						\n"
-		     "1:	movw %0, %%fs			\n"
-		     "2:					\n"
-
+	asm volatile("1:	movw %0, %%fs\n"
+		     "2:\n"
 		     _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_CLEAR_FS)
-
 		     : : ASM_INPUT_RM (value) : "memory");
 }
 
@@ -342,13 +337,31 @@ static inline void __loadsegment_fs(unsigned short value)
 
 #endif
 
-#define loadsegment(seg, value) __loadsegment_ ## seg (value)
+#undef LOAD_SEGMENT
+
+#define loadsegment(seg, val) __loadsegment_##seg(val)
 
 /*
  * Save a segment register away:
  */
-#define savesegment(seg, value)				\
-	asm volatile("movl %%" #seg ",%k0" : "=r" (value))
+#define SAVE_SEGMENT(seg)				\
+static inline unsigned long __savesegment_##seg(void)	\
+{							\
+	unsigned long v;				\
+	asm volatile("movl %%" #seg ",%k0" : "=r" (v));	\
+	return v;					\
+}
+
+SAVE_SEGMENT(cs)
+SAVE_SEGMENT(ss)
+SAVE_SEGMENT(ds)
+SAVE_SEGMENT(es)
+SAVE_SEGMENT(fs)
+SAVE_SEGMENT(gs)
+
+#undef SAVE_SEGMENT
+
+#define savesegment(seg, var) ((var) = __savesegment_##seg())
 
 #endif /* !__ASSEMBLER__ */
 #endif /* __KERNEL__ */
-- 
2.53.0