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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Ztnvosf0JHsvCf7-@arm.com>
Date: Thu, 5 Sep 2024 18:51:30 +0100
From: Catalin Marinas <catalin.marinas@....com>
To: Mark Brown <broonie@...nel.org>
Cc: Will Deacon <will@...nel.org>, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH] arm64/fpsimd: Ensure we don't contend a SMCU from idling
 CPUs

(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