[PATCH 04/20] arm64/fpsimd: signal: Mandate SVE payload for streaming-mode state

[Will Deacon]

On Wed, May 07, 2025 at 03:21:46PM +0100, Mark Rutland wrote:
> On Wed, May 07, 2025 at 01:59:16PM +0100, Will Deacon wrote:
> > On Tue, May 06, 2025 at 04:25:07PM +0100, Mark Rutland wrote:
> > > @@ -416,7 +418,16 @@ static int restore_sve_fpsimd_context(struct user_ctxs *user)
> > >  	if (user_vl != vl)
> > >  		return -EINVAL;
> > >  
> > > -	if (user->sve_size == sizeof(*user->sve)) {
> > > +	/*
> > > +	 * Non-streaming SVE state may be preserved without an SVE payload, in
> > > +	 * which case the SVE context only has a header with VL==0, and all
> > > +	 * state can be restored from the FPSIMD context.
> > > +	 *
> > > +	 * Streaming SVE state is always preserved with an SVE payload. For
> > > +	 * consistency and robustness, reject restoring streaming SVE state
> > > +	 * without an SVE payload.
> > > +	 */
> > > +	if (!sm && user->sve_size == sizeof(*user->sve)) {
> > >  		clear_thread_flag(TIF_SVE);
> > >  		current->thread.svcr &= ~SVCR_SM_MASK;
> > >  		current->thread.fp_type = FP_STATE_FPSIMD;
> > 
> > This (along with the 'fpsimd_only:' block) is now practically identical
> > to restore_fpsimd_context(). I think that's a sign that we're doing the
> > right thing, but can we move that into a shared helper function?
> 
> Largely yes, but there's a subtlety.
> 
> We can call restore_fpsimd_context() directly for the block above, since
> that's equivalent as of the prior patch.
> 
> The "fpsimd_only" label is misleading, and is used in two distinct
> cases:
> 
> (1) When the SVE context has no register state, that's used to resrtore
>     an FPSIMD-only context, and is (now) equivalent to
>     restore_fpsimd_context().
> 
> (2) When the SVE context has register state, we restore the SVE state,
>     then fall through to the "fpsimd_only" block to merge the the FPSIMD
>     registers into the SVE registers, preserving the upper bits.
> 
>     The merging is hidden opaquely in fpsimd_update_current_state().
> 
> So we can call restore_fpsimd_context() directly for the !SVE case, and
> we can factor out reading the user_fpsimd_state, but for case 2 we MUST
> NOT mess with TIF_SVE, SVCR, or fp_type, and MUST merge the register
> contents.

Right, all I'm suggesting here is to remove the 'fpsimd_only:' label and
call into a restore_fpsimd_context()-like helper function in the
FPSIMD-only case. We can leave the SVE case as-is (and potentially
de-duplicate the copying later on if we want to).

Will