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@xxxxxxxxxxxxx> 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@xxxxxxxxx>
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxx>
> > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > Cc: Borislav Petkov <bp@xxxxxxxxx>
> > Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> > Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> > Cc: "Peter Zijlstra (Intel)" <peterz@xxxxxxxxxxxxx>
> > ---
> >  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
> >   
>