Re: [PATCH] arm64/fpsimd: Ensure we don't contend a SMCU from idling CPUs

[Catalin Marinas <catalin.marinas@arm.com>]

(replying to the previous version as it looks like I haven't followed up
on the discussion)

On Thu, Jul 11, 2024 at 12:28:12AM +0100, Mark Brown wrote:
> On Wed, Jul 10, 2024 at 09:10:53PM +0100, Catalin Marinas wrote:
> > On Tue, Jun 18, 2024 at 02:57:42PM +0100, Mark Brown wrote:
> > > +	/*
> > > +	 * Leaving SME enabled may leave this core contending with
> > > +	 * other cores if we have a SMCU, disable whenever we enter
> > > +	 * idle to avoid this.  Only do this if they're actually
> > > +	 * enabled to avoid overhead in cases where we don't enter a
> > > +	 * low enough power state to loose register state.
> > > +	 */
> > > +	if (system_supports_sme() &&
> > > +	    (read_sysreg_s(SYS_SVCR) & (SVCR_SM_MASK | SVCR_ZA_MASK)))
> > > +		fpsimd_save_and_flush_cpu_state();
> > > +}
> 
> > Do we always enter here via the idle thread? If we already had a thread
> > switch we probably don't need to save the SME state again, only flush
> > the state.
> 
> If we've actually switched the thread then TIF_FOREIGN_FPSTATE has been
> set and we'll just check the flag and return for the save portion rather
> than actually writing any register values out so the overhead should be
> minimal.  It feels safer to check in case we get better at doing the
> save lazily.

OK, so likely the state is already saved, all we need to do here is
flush the state and SMSTOP. But why would switching to idle be any
different than switching to a thread that doesn't used SME? It feels
like we are just trying to optimise a special case only. Could we not
instead issue an SMSTOP in the context switch code?

Also this looks hypothetical until we have some hardware to test it on,
see how it would behave with a shared SME unit.

-- 
Catalin