Re: [PATCH v3 4/9] powerpc: Explicitly disable math features when copying thread

[Cyril Bur <cyrilbur@gmail.com>]

On Mon, 25 Jan 2016 11:04:23 +1100
Balbir Singh <bsingharora@gmail.com> wrote:

> On Thu, 21 Jan 2016 11:55:44 +1100
> Cyril Bur <cyrilbur@gmail.com> wrote:
> 
> > Currently when threads get scheduled off they always giveup the FPU,
> > Altivec (VMX) and Vector (VSX) units if they were using them. When they are
> > scheduled back on a fault is then taken to enable each facility and load
> > registers. As a result explicitly disabling FPU/VMX/VSX has not been
> > necessary.
> > 
> > Future changes and optimisations remove this mandatory giveup and fault
> > which could cause calls such as clone() and fork() to copy threads and run
> > them later with FPU/VMX/VSX enabled but no registers loaded.
> > 
> > This patch starts the process of having MSR_{FP,VEC,VSX} mean that a
> > threads registers are hot while not having MSR_{FP,VEC,VSX} means that the
> > registers must be loaded. This allows for a smarter return to userspace.
> > 
> > Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
> > ---
> >  arch/powerpc/kernel/process.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> > index dccc87e..e0c3d2d 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -1307,6 +1307,7 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
> >  
> >  		f = ret_from_fork;
> >  	}
> > +	childregs->msr &= ~(MSR_FP|MSR_VEC|MSR_VSX);  
> 

Hi Balbir,

Perhaps I'm missing something, are you saying

> Ideally you want to use __msr_check_and_clear()
> 

instead of childregs->msr &= ~(MSR_FP|MSR_VEC|MSR_VSX); ? I don't see how that
can work...

__msr_check_and_clear() operates on the currently active MSR, that is, the msr
for the current kernel context. childregs->msr is the value that will be used
for that userspace context when the kernel returns. Here we must ensure that
that children are created with the bit disabled.

> Basically we start with these bits off and then take an exception on use?
> 

Currently yes, this is what I'm trying to change. This patch hasn't been
necessary until now as any thread which saves its FPU/VMX/VSX data ALSO
disables those bits in regs->msr and so theres no way a clone() or fork() can
create a child with MSR_FP or MSR_VEC or MSR_VSX set. I add a meaning to
'having a regs->msr FP,VEC,VSX bit set' to mean that 'the regs are hot' in a
subsequent patch which means this assumption no longer holds so now we must
explicitly disable (so as to signal that the FPU/VMX/VSX regs are not hot) for
children thread.

Sounds like I still haven't got that commit message quite right yet.

> >  	sp -= STACK_FRAME_OVERHEAD;
> >  
> >  	/*  
>