Re: [PATCH v3 4/9] powerpc: Explicitly disable math features when copying thread
From: Balbir Singh <bsingharora@gmail.com>
Date: 2016-01-27 12:02:03
On Wed, Jan 27, 2016 at 10:50 AM, Cyril Bur [off-list ref] wrote:
On Mon, 25 Jan 2016 11:04:23 +1100 Balbir Singh [off-list ref] wrote:quoted
On Thu, 21 Jan 2016 11:55:44 +1100 Cyril Bur [off-list ref] wrote:quoted
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 <redacted> --- 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 sayingquoted
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.
Yes, my bad! I thought the routine took generic bits, hoping to reuse the CONFIG_VSX bits. I don't think it helps much, what you have is correct.
quoted
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.
I think the older series had more data to help understand the patch. It would help to move some of them to the current series Balbir