On Fri, 15 Jan 2016 17:25:26 +1100 Michael Neuling <mi...@neuling.org> wrote:
> On Fri, 2016-01-15 at 16:04 +1100, Cyril Bur wrote: > > This patch adds the ability to be able to save the VSX registers to > > the > > thread struct without giving up (disabling the facility) next time > > the > > process returns to userspace. > > > > This patch builds on a previous optimisation for the FPU and VEC > > registers > > in the thread copy path to avoid a possibly pointless reload of VSX > > state. > > > > Signed-off-by: Cyril Bur <cyril...@gmail.com> > > --- > > arch/powerpc/include/asm/switch_to.h | 1 - > > arch/powerpc/kernel/ppc_ksyms.c | 4 ---- > > arch/powerpc/kernel/process.c | 23 ++++++++++++++++++----- > > arch/powerpc/kernel/vector.S | 17 ----------------- > > 4 files changed, 18 insertions(+), 27 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/switch_to.h > > b/arch/powerpc/include/asm/switch_to.h > > index 29dda9d..4dfcd3e 100644 > > --- a/arch/powerpc/include/asm/switch_to.h > > +++ b/arch/powerpc/include/asm/switch_to.h > > @@ -52,7 +52,6 @@ static inline void disable_kernel_altivec(void) > > extern void enable_kernel_vsx(void); > > extern void flush_vsx_to_thread(struct task_struct *); > > extern void giveup_vsx(struct task_struct *); > > -extern void __giveup_vsx(struct task_struct *); > > static inline void disable_kernel_vsx(void) > > { > > msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX); > > diff --git a/arch/powerpc/kernel/ppc_ksyms.c > > b/arch/powerpc/kernel/ppc_ksyms.c > > index 41e1607..ef7024da 100644 > > --- a/arch/powerpc/kernel/ppc_ksyms.c > > +++ b/arch/powerpc/kernel/ppc_ksyms.c > > @@ -28,10 +28,6 @@ EXPORT_SYMBOL(load_vr_state); > > EXPORT_SYMBOL(store_vr_state); > > #endif > > > > -#ifdef CONFIG_VSX > > -EXPORT_SYMBOL_GPL(__giveup_vsx); > > -#endif > > - > > #ifdef CONFIG_EPAPR_PARAVIRT > > EXPORT_SYMBOL(epapr_hypercall_start); > > #endif > > diff --git a/arch/powerpc/kernel/process.c > > b/arch/powerpc/kernel/process.c > > index 5566c32..3d907b8 100644 > > --- a/arch/powerpc/kernel/process.c > > +++ b/arch/powerpc/kernel/process.c > > @@ -252,20 +252,33 @@ EXPORT_SYMBOL_GPL(flush_altivec_to_thread); > > #endif /* CONFIG_ALTIVEC */ > > > > #ifdef CONFIG_VSX > > -void giveup_vsx(struct task_struct *tsk) > > +void __giveup_vsx(struct task_struct *tsk) > > { > > - check_if_tm_restore_required(tsk); > > - > > - msr_check_and_set(MSR_FP|MSR_VEC|MSR_VSX); > > if (tsk->thread.regs->msr & MSR_FP) > > __giveup_fpu(tsk); > > if (tsk->thread.regs->msr & MSR_VEC) > > __giveup_altivec(tsk); > > + tsk->thread.regs->msr &= ~MSR_VSX; > > +} > > + > > +void giveup_vsx(struct task_struct *tsk) > > +{ > > + check_if_tm_restore_required(tsk); > > + > > + msr_check_and_set(MSR_FP|MSR_VEC|MSR_VSX); > > __giveup_vsx(tsk); > > msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX); > > } > > EXPORT_SYMBOL(giveup_vsx); > > > > +void save_vsx(struct task_struct *tsk) > > +{ > > + if (tsk->thread.regs->msr & MSR_FP) > > + save_fpu(tsk); > > + if (tsk->thread.regs->msr & MSR_VEC) > > + save_altivec(tsk); > > +} > > + > > void enable_kernel_vsx(void) > > { > > WARN_ON(preemptible()); > > @@ -465,7 +478,7 @@ void save_all(struct task_struct *tsk) > > #endif > > #ifdef CONFIG_VSX > > if (usermsr & MSR_VSX) > > - __giveup_vsx(tsk); > > + save_vsx(tsk); > > This seems suboptimal. save_vsx() will call save_fpu() and > save_altivec() again, which you just called earlier in save_all(). > Ah yes, will fix > save_vsx() is only used here, so could be static. > Thanks. > Also, put the #ifdef junk as part of the function so that the caller > doesn't have to deal with it. > Can do absolutely, however this means that in save_all I can't check if the function needs to be called or not. For example, without CONFIG_VSX, MSR_VSX won't exist which means we might end up calling save_vsx THEN checking MSR_VSX and returning early. I'm happy to defer to you and mpe on what's nicer, I would side with avoiding the function call at the cost of ugly #ifdefs but I can always see the merits of clean code. Thanks for the review, Cyril > Mikey > > > #endif > > #ifdef CONFIG_SPE > > if (usermsr & MSR_SPE) > > diff --git a/arch/powerpc/kernel/vector.S > > b/arch/powerpc/kernel/vector.S > > index 51b0c17..1c2e7a3 100644 > > --- a/arch/powerpc/kernel/vector.S > > +++ b/arch/powerpc/kernel/vector.S > > @@ -151,23 +151,6 @@ _GLOBAL(load_up_vsx) > > std r12,_MSR(r1) > > b fast_exception_return > > > > -/* > > - * __giveup_vsx(tsk) > > - * Disable VSX for the task given as the argument. > > - * Does NOT save vsx registers. > > - */ > > -_GLOBAL(__giveup_vsx) > > - addi r3,r3,THREAD /* want THREAD of > > task */ > > - ld r5,PT_REGS(r3) > > - cmpdi 0,r5,0 > > - beq 1f > > - ld r4,_MSR-STACK_FRAME_OVERHEAD(r5) > > - lis r3,MSR_VSX@h > > - andc r4,r4,r3 /* disable VSX for > > previous task */ > > - std r4,_MSR-STACK_FRAME_OVERHEAD(r5) > > -1: > > - blr > > - > > #endif /* CONFIG_VSX */ > > > > _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev