Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756324AbbHZLcG (ORCPT ); Wed, 26 Aug 2015 07:32:06 -0400 Received: from mail-io0-f170.google.com ([209.85.223.170]:32830 "EHLO mail-io0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753504AbbHZLcE (ORCPT ); Wed, 26 Aug 2015 07:32:04 -0400 MIME-Version: 1.0 In-Reply-To: <20150826111205.GD30466@arm.com> References: <1440556841-17219-1-git-send-email-chunyan.zhang@spreadtrum.com> <20150826111205.GD30466@arm.com> Date: Wed, 26 Aug 2015 13:32:03 +0200 Message-ID: Subject: Re: [PATCH] arm64: fix bug for reloading FPSIMD state after execve on cpu 0. From: Ard Biesheuvel To: Will Deacon Cc: Chunyan Zhang , Catalin Marinas , "linux-arm-kernel@lists.infradead.org" , "jianhua.ljh@gmail.com" , "orson.zhai@spreadtrum.com" , "xiongshan.an@spreadtrum.com" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2688 Lines: 75 On 26 August 2015 at 13:12, Will Deacon wrote: > Hello, > > [adding Ard] > > On Wed, Aug 26, 2015 at 03:40:41AM +0100, Chunyan Zhang wrote: >> From: Janet Liu >> >> If process A is running on CPU 0 and do execve syscall and after sched_exec, >> dest_cpu is 0, fpsimd_state.cpu is 0. If at the time Process A get scheduled >> out and after some kernel threads running on CPU 0, process A is back in CPU 0, >> A's fpsimd_state.cpu is current cpu id "0", and per_cpu(fpsimd_last_state) >> points A's fpsimd_state, TIF_FOREIGN_FPSTATE will be clear, kernel will not >> reload the context during it return to userspace. so set the cpu's >> fpsimd_last_state to NULL to avoid this. > > AFAICT, this is only a problem if one of the kernel threads uses the fpsimd > registers, right? However, kernel_neon_begin_partial clobbers > fpsimd_last_state, so I'm struggling to see the problem. > I think the problem is real, but it would be better to set the fpsimd_state::cpu field to an invalid value like we do in fpsimd_flush_task_state() diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index 44d6f7545505..c56956a16d3f 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -158,6 +158,7 @@ void fpsimd_thread_switch(struct task_struct *next) void fpsimd_flush_thread(void) { memset(¤t->thread.fpsimd_state, 0, sizeof(struct fpsimd_state)); + fpsimd_flush_task_state(current); set_thread_flag(TIF_FOREIGN_FPSTATE); } (note the memset erroneously initializes that field to CPU 0) This more accurately reflects the state of the process after forking, i.e., that its FPSIMD state has never been loaded into any CPU. -- Ard. > Are you seeing an issue in practice? > > Will > >> Signed-off-by: Janet Liu >> Signed-off-by: Chunyan Zhang >> --- >> arch/arm64/kernel/fpsimd.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c >> index 44d6f75..ec58d94 100644 >> --- a/arch/arm64/kernel/fpsimd.c >> +++ b/arch/arm64/kernel/fpsimd.c >> @@ -159,6 +159,7 @@ void fpsimd_flush_thread(void) >> { >> memset(¤t->thread.fpsimd_state, 0, sizeof(struct fpsimd_state)); >> set_thread_flag(TIF_FOREIGN_FPSTATE); >> + this_cpu_write(fpsimd_last_state, NULL); >> } >> >> /* >> -- >> 1.7.9.5 >> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/