Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Andy Lutomirski <luto@amacapital.net>
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH v4 14/22] x86/fpu/xstate: Expand the xstate buffer on the first use of dynamic user state
Date:   Wed, 24 Mar 2021 14:26:38 -0700
Message-Id: <DEF855E6-185F-4E23-91F0-A4544F62EE50@amacapital.net>
References: <CAJvTdKm_uP-RqoprWWatg725WZOm3T-BHZU8S08dpWe2=1apGw@mail.gmail.com>
Cc:     "Liu, Jing2" <jing2.liu@linux.intel.com>,
        Andy Lutomirski <luto@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Borislav Petkov <bp@suse.de>, Ingo Molnar <mingo@kernel.org>,
        X86 ML <x86@kernel.org>, Len Brown <len.brown@intel.com>,
        Dave Hansen <dave.hansen@intel.com>,
        "Liu, Jing2" <jing2.liu@intel.com>,
        "Ravi V. Shankar" <ravi.v.shankar@intel.com>,
        LKML <linux-kernel@vger.kernel.org>,
        "Bae, Chang Seok" <chang.seok.bae@intel.com>
In-Reply-To: <CAJvTdKm_uP-RqoprWWatg725WZOm3T-BHZU8S08dpWe2=1apGw@mail.gmail.com>
To:     Len Brown <lenb@kernel.org>
Precedence: bulk


> On Mar 24, 2021, at 2:09 PM, Len Brown <lenb@kernel.org> wrote:
>=20
> =EF=BB=BFOn Tue, Mar 23, 2021 at 11:15 PM Liu, Jing2 <jing2.liu@linux.inte=
l.com> wrote:
>=20
>>> IMO, the problem with AVX512 state
>>> is that we guaranteed it will be zero for XINUSE=3D0.
>>> That means we have to write 0's on saves.
>=20
>> why "we have to write 0's on saves" when XINUSE=3D0.
>>=20
>> Since due to SDM, if XINUSE=3D0, the XSAVES will *not* save the data and
>> xstate_bv bit is 0; if use XSAVE, it need save the state but
>> xstate_bv bit is also 0.
>>>  It would be better
>>> to be able to skip the write -- even if we can't save the space
>>> we can save the data transfer.  (This is what we did for AMX).
>> With XFD feature that XFD=3D1, XSAVE command still has to save INIT state=

>> to the area. So it seems with XINUSE=3D0 and XFD=3D1, the XSAVE(S) comman=
ds
>> do the same that both can help save the data transfer.
>=20
> Hi Jing, Good observation!
>=20
> There are 3 cases.
>=20
> 1. Task context switch save into the context switch buffer.
> Here we use XSAVES, and as you point out, XSAVES includes
> the compaction optimization feature tracked by XINUSE.
> So when AMX is enabled, but clean, XSAVES doesn't write zeros.
> Further, it omits the buffer space for AMX in the destination altogether!
> However, since XINUSE=3D1 is possible, we have to *allocate* a buffer
> large enough to handle the dirty data for when XSAVES can not
> employ that optimization.
>=20
> 2. Entry into user signal handler saves into the user space sigframe.
> Here we use XSAVE, and so the hardware will write zeros for XINUSE=3D0,
> and for AVX512, we save neither time or space.
>=20
> My understanding that for application compatibility, we can *not* compact
> the destination buffer that user-space sees.  This is because existing cod=
e
> may have adopted fixed size offsets.  (which is unfortunate).
>=20
> And so, for AVX512, we both reserve the space, and we write zeros
> for clean AVX512 state.
>=20
> For AMX, we must still reserve the space, but we are not going to write ze=
ros
> for clean state.  We so this in software by checking XINUSE=3D0, and clear=
ing
> the xstate_bf for the XSAVE.  As a result, for XINUSE=3D0, we can skip
> writing the zeros, even though we can't compress the space.

Why?

>=20
> 3. user space always uses fully uncompacted XSAVE buffers.
>=20

There is no reason we have to do this for new states. Arguably we shouldn=E2=
=80=99t for AMX to avoid yet another altstack explosion.=