[PATCH v2 2/7] arm64/fpsimd: Track the saved FPSIMD state type separately to TIF_SVE

Mark Brown broonie at kernel.org
Mon Jul 11 04:39:51 PDT 2022


On Mon, Jul 11, 2022 at 10:40:50AM +0100, Marc Zyngier wrote:
> Mark Brown <broonie at kernel.org> wrote:

> > +	enum fp_state *type;

> For consistency: s/type/fp_type/ ?

Sure if nobody else wants a different bikeshed.  It really needs a
longer name like fp_state_t or something but that had it's own problems
with non-idiomaticness.

> >  	if (test_and_clear_tsk_thread_flag(task, TIF_SVE) ||
> > -	    thread_sm_enabled(&task->thread))
> > +	    thread_sm_enabled(&task->thread)) {
> >  		sve_to_fpsimd(task);
> > +		task->thread.fp_type = FP_STATE_FPSIMD;

> Can you move this assignment into the sve_to_fpsimd() helper?

There are cases where we want a FPSIMD version of the state for reading
but don't want to affect the actual state of the process (eg, if someone
reads the FPSIMD registers via ptrace) so we don't want to change the
active register state just because we converted it.  Adding another API
that does the convert and update didn't feel like it was helping since
you then have to remember which API does what and we already have lots
of similarly named functions for slightly different contexts.

> >  	} else {
> >  		fpsimd_to_sve(current);
> > +		current->thread.fp_type = FP_STATE_SVE;

> Same thing here.

There's not the same issue with reading FPSIMD state via the SVE APIs
but for consistency it seems best to always leave these updates in the
callers.

> > --- a/arch/arm64/kernel/ptrace.c
> > +++ b/arch/arm64/kernel/ptrace.c
> > @@ -892,8 +892,7 @@ static int sve_set_common(struct task_struct *target,
> >  		ret = __fpr_set(target, regset, pos, count, kbuf, ubuf,
> >  				SVE_PT_FPSIMD_OFFSET);
> >  		clear_tsk_thread_flag(target, TIF_SVE);
> > -		if (type == ARM64_VEC_SME)
> > -			fpsimd_force_sync_to_sve(target);

> I don't get this particular change. Can you please clarify?

That should probably be shifted to a later patch in the series, I think
I just rebased it to the wrong place.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20220711/8bc8ec9c/attachment.sig>


More information about the linux-arm-kernel mailing list