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

[PATCH -tip 1/3] 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. As such, the "memory" clobber is unnecessary.

No functional change intended.

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>
---
 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 2/3] 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`,
and the compiler should 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>
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>
---
 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 3/3] 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>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Nathan Chancellor <nathan@kernel.org>
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

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

[Peter Zijlstra]

On Tue, Mar 10, 2026 at 09:21:22AM +0100, Uros Bizjak wrote:
> 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. As such, the "memory" clobber is unnecessary.

The point isn't that this accesses memory, but that prior or later
accesses would end up at different memory locations (as would happen
when setting the per-cpu segment.

Anyway, aside from that nit, yes these 3 patches look good to me.

> 
> No functional change intended.
> 
> 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>
> ---
>  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
> 

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

[David Laight]

On Tue, 10 Mar 2026 11:27:42 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Mar 10, 2026 at 09:21:22AM +0100, Uros Bizjak wrote:
> > 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. As such, the "memory" clobber is unnecessary.  
> 
> The point isn't that this accesses memory, but that prior or later
> accesses would end up at different memory locations (as would happen
> when setting the per-cpu segment.

Don't they need a "memory" clobber to stop them being moved the other
side of a request to set the relevant register?
In effect the [fg]sbase register has to be treated like a memory location.

Given that a memory clobber often makes little (or no) difference to
the generated code it is probably better to be safe and leave the clobber
in place.
If the memory clobber does make a significant difference (unlikely here)
it can often be mitigated by changing the source code to not rely on CSE.

	David 

> 
> Anyway, aside from that nit, yes these 3 patches look good to me.
> 
> > 
> > No functional change intended.
> > 
> > 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>
> > ---
> >  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
> >   
> 

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

[Peter Zijlstra]

On Tue, Mar 10, 2026 at 02:11:04PM +0000, David Laight wrote:
> On Tue, 10 Mar 2026 11:27:42 +0100
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Tue, Mar 10, 2026 at 09:21:22AM +0100, Uros Bizjak wrote:
> > > 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. As such, the "memory" clobber is unnecessary.  
> > 
> > The point isn't that this accesses memory, but that prior or later
> > accesses would end up at different memory locations (as would happen
> > when setting the per-cpu segment.
> 
> Don't they need a "memory" clobber to stop them being moved the other
> side of a request to set the relevant register?
> In effect the [fg]sbase register has to be treated like a memory location.

Its a read, it doesn't matter. And no, IIRC the compilers are not
allowed to re-order asm volatile, so as long as both the load and store
of the segments register are asm volatile, those cannot get reordered.

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

[Uros Bizjak]

On Tue, Mar 10, 2026 at 3:11 PM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Tue, 10 Mar 2026 11:27:42 +0100
> Peter Zijlstra <peterz@infradead.org> wrote:
>
> > On Tue, Mar 10, 2026 at 09:21:22AM +0100, Uros Bizjak wrote:
> > > 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. As such, the "memory" clobber is unnecessary.
> >
> > The point isn't that this accesses memory, but that prior or later
> > accesses would end up at different memory locations (as would happen
> > when setting the per-cpu segment.
>
> Don't they need a "memory" clobber to stop them being moved the other
> side of a request to set the relevant register?

Only if the register setting instruction also uses "memory" clobber,
otherwise "asm" can be scheduled around "asm volatile".

> In effect the [fg]sbase register has to be treated like a memory location.

Yes, but only segment register setting instructions should have
"memory" clobber. They do in fact affect memory, when e.g. a follow up
insn uses memory in __seg_gs named address space. Although it is a
"big hammer" approach, "memory" clobber prevents the compiler from
scheduling these instructions before register setting insn (there is
no separate notation of memory in non-generic address spaces).

When reading the register, "asm volatile" prevents the compiler from
scheduling register reading insn before register setting instruction
(so, both should be marked "asm volatile"). There is no memory
involved, so "asm volatile" is the instrument that tells the compiler
what it can do with the insn.

> Given that a memory clobber often makes little (or no) difference to
> the generated code it is probably better to be safe and leave the clobber
> in place.

asm with the "memory" clobber is the biggest hammer approach here. It
blocks scheduling of *all* memory related instructions and should
really be used as the last resort. Merely reading the segment register
doesn't justify its usage.

> If the memory clobber does make a significant difference (unlikely here)
> it can often be mitigated by changing the source code to not rely on CSE.

There is a trade-off when using "asm volatile". It prevents the
compiler from scheduling insn with other "asm volatile" insns, but it
also prevents the compiler from moving insns out of loops or omitting
them on the assumption that the result from a previous call is still
valid. But this can easily be solved in the source code.

Uros.

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

[Uros Bizjak]

On 3/10/26 11:27, Peter Zijlstra wrote:

On Tue, Mar 10, 2026 at 09:21:22AM +0100, Uros Bizjak wrote:

>> 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. As such, the "memory" clobber is unnecessary.
>
> The point isn't that this accesses memory, but that prior or later
> accesses would end up at different memory locations (as would happen
> when setting the per-cpu segment.

Please note that the above comment is talking about rd[fs,gs]base
instructions that only *read* the segment register into GPR. They do
not affect memory in any way and can be scheduled anywhere, but they
should not be scheduled over segment register *setting* instructions.

> Anyway, aside from that nit, yes these 3 patches look good to me.

Can this be considered as Acked-by: from your side?

Thanks,
Uros.

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

[Peter Zijlstra]

On Tue, Mar 10, 2026 at 04:48:34PM +0100, Uros Bizjak wrote:
> On 3/10/26 11:27, Peter Zijlstra wrote:
> 
> On Tue, Mar 10, 2026 at 09:21:22AM +0100, Uros Bizjak wrote:
> 
> >> 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. As such, the "memory" clobber is unnecessary.
> >
> > The point isn't that this accesses memory, but that prior or later
> > accesses would end up at different memory locations (as would happen
> > when setting the per-cpu segment.
> 
> Please note that the above comment is talking about rd[fs,gs]base
> instructions that only *read* the segment register into GPR. They do
> not affect memory in any way and can be scheduled anywhere, but they
> should not be scheduled over segment register *setting* instructions.

Yes, we are in violent agreement :-) I just took issue with the way your
changelog expressed things. The memory clobber isn't about the asm
accessing memory or not, it is about adding a scheduling constraint.

> > Anyway, aside from that nit, yes these 3 patches look good to me.
> 
> Can this be considered as Acked-by: from your side?

Yep.

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

[Uros Bizjak]

On Tue, Mar 10, 2026 at 7:11 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Tue, Mar 10, 2026 at 04:48:34PM +0100, Uros Bizjak wrote:
> > On 3/10/26 11:27, Peter Zijlstra wrote:
> >
> > On Tue, Mar 10, 2026 at 09:21:22AM +0100, Uros Bizjak wrote:
> >
> > >> 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. As such, the "memory" clobber is unnecessary.
> > >
> > > The point isn't that this accesses memory, but that prior or later
> > > accesses would end up at different memory locations (as would happen
> > > when setting the per-cpu segment.
> >
> > Please note that the above comment is talking about rd[fs,gs]base
> > instructions that only *read* the segment register into GPR. They do
> > not affect memory in any way and can be scheduled anywhere, but they
> > should not be scheduled over segment register *setting* instructions.
>
> Yes, we are in violent agreement :-) I just took issue with the way your
> changelog expressed things. The memory clobber isn't about the asm
> accessing memory or not, it is about adding a scheduling constraint.

I see. I will add this detail to a v2 ChangeLog with a remark that
"asm volatile" alone is enough to prevent unwanted scheduling
(something like the explanation in patch 2/3 which explains in more
detail why "asm volatile" should be used instead of memory clobber).

> > > Anyway, aside from that nit, yes these 3 patches look good to me.
> >
> > Can this be considered as Acked-by: from your side?
>
> Yep.

Thanks,
Uros.