lkml.org 
[lkml]   [2019]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v5 05/13] x86/fsgsbase/64: Enable FSGSBASE instructions in the helper functions
    On Fri, Feb 1, 2019 at 12:54 PM Chang S. Bae <chang.seok.bae@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.bae@intel.com>
    > Cc: Any Lutomirski <luto@kernel.org>
    > Cc: H. Peter Anvin <hpa@zytor.com>
    > Cc: Thomas Gleixner <tglx@linutronix.de>
    > Cc: Ingo Molnar <mingo@kernel.org>
    > Cc: Andrew Cooper <andrew.cooper3@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
    >

    \
     
     \ /
      Last update: 2019-02-02 03:58    [W:4.111 / U:1.396 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site