linux-kernel - Re: [PATCH v1] x86/fpu: Remove opmask state from avx512

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]

Date:   Fri, 15 Oct 2021 08:25:01 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Noah Goldstein <goldstein.w.n@...il.com>,
        Borislav Petkov <bp@...en8.de>
Cc:     tglx@...utronix.de, mingo@...hat.com, X86 ML <x86@...nel.org>,
        hpa@...or.com, Andy Lutomirski <luto@...nel.org>,
        open list <linux-kernel@...r.kernel.org>,
        aubrey <aubrey.li@...el.com>,
        "Chen, Tim C" <tim.c.chen@...el.com>,
        "Van De Ven, Arjan" <arjan.van.de.ven@...el.com>
Subject: Re: [PATCH v1] x86/fpu: Remove opmask state from avx512_timestamp
 check

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).