[PATCH 03/20] arm64/fpsimd: signal: Clear PSTATE.SM when restoring FPSIMD frame only

[Will Deacon]

On Wed, May 07, 2025 at 03:01:50PM +0100, Mark Rutland wrote:
> On Wed, May 07, 2025 at 01:46:45PM +0100, Will Deacon wrote:
> > On Tue, May 06, 2025 at 04:25:06PM +0100, Mark Rutland wrote:
> > > diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> > > index 48d3c0129dade..fdce1b856f498 100644
> > > --- a/arch/arm64/kernel/signal.c
> > > +++ b/arch/arm64/kernel/signal.c
> > > @@ -280,6 +280,7 @@ static int restore_fpsimd_context(struct user_ctxs *user)
> > >  	__get_user_error(fpsimd.fpcr, &(user->fpsimd->fpcr), err);
> > >  
> > >  	clear_thread_flag(TIF_SVE);
> > > +	current->thread.svcr &= ~SVCR_SM_MASK;
> > >  	current->thread.fp_type = FP_STATE_FPSIMD;
> > 
> > Hmm, I think we're preemptible here so do we need some compiler barriers
> > to make sure that the context-switching code doesn't see these fields in
> > an inconsistent state?
> 
> We avoid that problem by ensuring that the task's FPSIMD/SVE/SME state isn't
> live on any CPU for the duration of the signal code, such that it is safe to
> manipulate the saved state in preemptible context. Details below.
> 
> That was handled in one of the earlier patches in the arm64 for-next/sme-fixes
> branch, queded as commit:
> 
>   929fa99b1215966f ("arm64/fpsimd: signal: Always save+flush state early")
> 
> As of that commit, in setup_rt_frame() we call
> fpsimd_save_and_flush_current_state(), which does:
> 
>   get_cpu_fpsimd_context();
>   fpsimd_save_user_state();
>   fpsimd_flush_task_state(current);
>   put_cpu_fpsimd_context();
> 
> That ensures (in a premption-safe way) that the tasks state has been saved
> to memory, TIF_FOREIGN_FPSTATE is set, and that
> task->thread.fpsimd_cpu==NR_CPUS. Thus on context switch:
> 
> * When switching away from this task, fpsimd_thread_switch() sees
>   TIF_FOREIGN_FPSTATE is set, and doesn't manipulate the task's saved state.
> 
> * When switching to this task, fpsimd_thread_switch() doesn't find any existing
>   state bound on the CPU, and sets TIF_FOREIGN_FPSTATE, without manipulating
>   the task's saved state.
> 
> The task's state will only be relaoded onto the CPU when returning to
> userspace, via fpsimd_restore_current_state().
> 
> Note that we changed fpsimd_update_current_state() to modify the saved state
> without reloading it onto the CPU.

Thanks, that makes sense!

Will