Re: [PATCH v1] x86/fpu: Remove opmask state from avx512_timestamp check

From: Dave Hansen <dave.hansen@intel.com>
To: Noah Goldstein <goldstein.w.n@gmail.com>, Borislav Petkov <bp@alien8.de>
Cc: tglx@linutronix.de, mingo@redhat.com, X86 ML <x86@kernel.org>,
	hpa@zytor.com, Andy Lutomirski <luto@kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	aubrey <aubrey.li@intel.com>,
	"Chen, Tim C" <tim.c.chen@intel.com>,
	"Van De Ven, Arjan" <arjan.van.de.ven@intel.com>
Subject: Re: [PATCH v1] x86/fpu: Remove opmask state from avx512_timestamp check
Date: Fri, 15 Oct 2021 08:25:01 -0700	[thread overview]
Message-ID: <53c43d2d-0e2e-654b-417d-d3dcbca42fc5@intel.com> (raw)
In-Reply-To: <CAFUsyfLf5a6-L7f2AjLx3j8+qbG7EU9iLPJCTA0+UoOUg3C61A@mail.gmail.com>

On 10/14/21 8:49 AM, Noah Goldstein wrote:
> Irrelevant of the still existing flaws, it makes the output more accurate.
> 
> Is there a cost to the change I am not seeing?

We'd want to make sure that this doesn't break anything.  It probably
won't, but it theoretically could.

For instance, if someone was doing:

	avx512_foo();
	xsave->xstate_bv &= ~XFEATURE_MASK_ZMMS;
	XRSTOR(xsave, -1);

That would leave the opmask in place, but would lead to the ZMM
registers tracked as being in their init state.

This would be *very* unlikely, but it would be great if Aubrey (the
original avx512_timestamp patch author) could make sure that it doesn't
break anything.

Also, there's the side issue of AVX-256 use.  AVX-256 uses the ZMM
registers which are a part of XFEATURE_MASK_AVX512, but does not incur
the same frequency penalties of the full 512-bit-wide instructions.
Since ZMM_Hi256 is the *only* ZMM state which is truly 512-bit-only, we
could argue that it's the only one we should consider.

Noah, thanks for bringing this up.  I'm not opposed to your patch, but
let's just make sure that it doesn't break anything and also that we
shouldn't do a bit more at the same time (ignore Hi16_ZMM for
avx512_timestamp).