On Fri, Feb 1, 2019 at 12:54 PM Chang S. Bae <chang.seok....@intel.com> wrote: > > The helper functions will switch on faster accesses to FSBASE and GSBASE > when the FSGSBASE feature is enabled. > > Accessing user GSBASE needs a couple of SWAPGS operations. It is avoidable > if the user GSBASE is saved at kernel entry, being updated as changes, and > restored back at kernel exit. However, it seems to spend more cycles for > savings and restorations. Little or no benefit was measured from > experiments. > > Also, introduce __{rd,wr}gsbase_inactive() as helpers to access user GSBASE > with SWAPGS. Note, for Xen PV, paravirt hooks can be added, since it may > allow a very efficient but different implementation. > > [ Use NOKPROBE_SYMBOL instead of __kprobes ]
^^^ This line looks like it shold be deleted. > > Signed-off-by: Chang S. Bae <chang.seok....@intel.com> > Cc: Any Lutomirski <l...@kernel.org> > Cc: H. Peter Anvin <h...@zytor.com> > Cc: Thomas Gleixner <t...@linutronix.de> > Cc: Ingo Molnar <mi...@kernel.org> > Cc: Andrew Cooper <andrew.coop...@citrix.com> > --- > arch/x86/include/asm/fsgsbase.h | 27 +++++++------- > arch/x86/kernel/process_64.c | 62 +++++++++++++++++++++++++++++++-- > 2 files changed, 72 insertions(+), 17 deletions(-) > > diff --git a/arch/x86/include/asm/fsgsbase.h b/arch/x86/include/asm/fsgsbase.h > index fdd1177499b4..aefd53767a5d 100644 > --- a/arch/x86/include/asm/fsgsbase.h > +++ b/arch/x86/include/asm/fsgsbase.h > @@ -49,35 +49,32 @@ static __always_inline void wrgsbase(unsigned long gsbase) > asm volatile("wrgsbase %0" :: "r" (gsbase) : "memory"); > } > > +#include <asm/cpufeature.h> > + > /* Helper functions for reading/writing FS/GS base */ > > static inline unsigned long x86_fsbase_read_cpu(void) > { > unsigned long fsbase; > > - rdmsrl(MSR_FS_BASE, fsbase); > + if (static_cpu_has(X86_FEATURE_FSGSBASE)) > + fsbase = rdfsbase(); > + else > + rdmsrl(MSR_FS_BASE, fsbase); > > return fsbase; > } > > -static inline unsigned long x86_gsbase_read_cpu_inactive(void) > -{ > - unsigned long gsbase; > - > - rdmsrl(MSR_KERNEL_GS_BASE, gsbase); > - > - return gsbase; > -} > - > static inline void x86_fsbase_write_cpu(unsigned long fsbase) > { > - wrmsrl(MSR_FS_BASE, fsbase); > + if (static_cpu_has(X86_FEATURE_FSGSBASE)) > + wrfsbase(fsbase); > + else > + wrmsrl(MSR_FS_BASE, fsbase); > } > > -static inline void x86_gsbase_write_cpu_inactive(unsigned long gsbase) > -{ > - wrmsrl(MSR_KERNEL_GS_BASE, gsbase); > -} > +extern unsigned long x86_gsbase_read_cpu_inactive(void); > +extern void x86_gsbase_write_cpu_inactive(unsigned long gsbase); > > #endif /* CONFIG_X86_64 */ > > diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c > index 6a62f4af9fcf..ebc55ed31fe7 100644 > --- a/arch/x86/kernel/process_64.c > +++ b/arch/x86/kernel/process_64.c > @@ -160,6 +160,42 @@ enum which_selector { > GS > }; > > +/* > + * Interrupts are disabled here. Out of line to be protected > + * from kprobes. It is not used on Xen paravirt. When paravirt > + * support is needed, it needs to be renamed with native_ prefix. > + */ > +static noinline unsigned long __rdgsbase_inactive(void) > +{ > + unsigned long gsbase, flags; > + > + local_irq_save(flags); > + native_swapgs(); > + gsbase = rdgsbase(); > + native_swapgs(); > + local_irq_restore(flags); > + > + return gsbase; > +} > +NOKPROBE_SYMBOL(__rdgsbase_inactive); > + > +/* > + * Interrupts are disabled here. Out of line to be protected > + * from kprobes. It is not used on Xen paravirt. When paravirt > + * support is needed, it needs to be renamed with native_ prefix. > + */ > +static noinline void __wrgsbase_inactive(unsigned long gsbase) > +{ > + unsigned long flags; > + > + local_irq_save(flags); > + native_swapgs(); > + wrgsbase(gsbase); > + native_swapgs(); > + local_irq_restore(flags); > +} > +NOKPROBE_SYMBOL(__wrgsbase_inactive); > + > /* > * Saves the FS or GS base for an outgoing thread if FSGSBASE extensions are > * not available. The goal is to be reasonably fast on non-FSGSBASE systems. > @@ -338,13 +374,34 @@ static unsigned long x86_fsgsbase_read_task(struct > task_struct *task, > return base; > } > > +unsigned long x86_gsbase_read_cpu_inactive(void) > +{ > + unsigned long gsbase; > + > + if (static_cpu_has(X86_FEATURE_FSGSBASE)) > + gsbase = __rdgsbase_inactive(); > + else > + rdmsrl(MSR_KERNEL_GS_BASE, gsbase); > + > + return gsbase; > +} > + > +void x86_gsbase_write_cpu_inactive(unsigned long gsbase) > +{ > + if (static_cpu_has(X86_FEATURE_FSGSBASE)) > + __wrgsbase_inactive(gsbase); > + else > + wrmsrl(MSR_KERNEL_GS_BASE, gsbase); > +} > + > unsigned long x86_fsbase_read_task(struct task_struct *task) > { > unsigned long fsbase; > > if (task == current) > fsbase = x86_fsbase_read_cpu(); > - else if (task->thread.fsindex == 0) > + else if (static_cpu_has(X86_FEATURE_FSGSBASE) || > + (task->thread.fsindex == 0)) > fsbase = task->thread.fsbase; > else > fsbase = x86_fsgsbase_read_task(task, task->thread.fsindex); > @@ -358,7 +415,8 @@ unsigned long x86_gsbase_read_task(struct task_struct > *task) > > if (task == current) > gsbase = x86_gsbase_read_cpu_inactive(); > - else if (task->thread.gsindex == 0) > + else if (static_cpu_has(X86_FEATURE_FSGSBASE) || > + (task->thread.gsindex == 0)) > gsbase = task->thread.gsbase; > else > gsbase = x86_fsgsbase_read_task(task, task->thread.gsindex); These last two hunks changes do not belong in this patch. Presumably they belong in patch 6. --Andy > -- > 2.19.1 >