All of lore.kernel.org
 help / color / mirror / Atom feed
From: Len Brown <lenb@kernel.org>
To: Andy Lutomirski <luto@kernel.org>
Cc: 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>
Subject: Re: [PATCH v4 14/22] x86/fpu/xstate: Expand the xstate buffer on the first use of dynamic user state
Date: Tue, 23 Mar 2021 17:01:58 -0400	[thread overview]
Message-ID: <CAJvTdKmz7aePcpi4i+d3vnqLuNAJEuJCjpGDv5WTYcSUfuxoDg@mail.gmail.com> (raw)
In-Reply-To: <CALCETrVaCmG4jzLCSuy7WYP2K7r-MVZntfugWa8HiVxQ7LpF_A@mail.gmail.com>

> I have an obnoxious question: do we really want to use the XFD mechanism?

Obnoxious questions are often the most valuable! :-)

> Right now, glibc, and hence most user space code, blindly uses
> whatever random CPU features are present for no particularly good
> reason, which means that all these features get stuck in the XINUSE=1
> state, even if there is no code whatsoever in the process that
> benefits.  AVX512 is bad enough as we're seeing right now.  AMX will
> be much worse if this happens.
>
> We *could* instead use XCR0 and require an actual syscall to enable
> it.  We could even then play games like requiring whomever enables the
> feature to allocate memory for the state save area for signals, and
> signal delivery could save the state and disable the feature, this
> preventing the signal frame from blowing up to 8 or 12 or who knows
> how many kB.

This approach would have some challenges.

Enumeration today is two parts.
1. CPUID tells you if the feature exists in the HW
2. xgetbv/XCR0 tells you if the OS supports that feature

Since #2 would be missing, you are right, there would need to be
a new API enabling the user to request the OS to enable support
for that task.

If that new API is not invoked before the user touches the feature,
they die with a #UD.

And so there would need to be some assurance that the API is successfully
called before any library might use the feature.  Is there a practical way
to guarantee that, given that the feature may be used (or not) only by
a dynamically
linked library?

If a library spawns threads and queries the size of XSAVE before the API
is called, it may be confused when that size changes after the API is called.

So a simple question, "who calls the API, and when?" isn't so simple.

Finally, note that XCR0 updates cause a VMEXIT,
while XFD updates do not.

So context switching XCR0 is possible, but is problematic.

The other combination is XFD + API rather than XCR0 + API.
With XFD, the context switching is faster, and the faulting (#NM and
the new MSR with #NM cause) is viable.
We have the bit set in XCR0, so no state size advantage.
Still have issues with API logistics.
So we didn't see that the API adds any value, only pain,
over transparent 1st use enabling with XFD and no API.

cheers,
Len Brown, Intel Open Source Technology Center

ps. I agree that un-necessary XINUSE=1 is possible.
Notwithstanding the issues initially deploying AVX512, I am skeptical
that it is common today.  IMO, the problem with AVX512 state
is that we guaranteed it will be zero for XINUSE=0.
That means we have to write 0's on saves.  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).

pps. your idea of requiring the user to allocate their own signal stack
is interesting.   It isn't really about allocating the stack, though --
the stack of the task that uses the feature is generally fine already.
The opportunity is to allow tasks that do *not* use the new feature to
get away with minimal data transfer and stack size.  As we don't
have the 0's guarantee for AMX, we bought the important part
of that back.

  reply	other threads:[~2021-03-23 21:03 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-21 18:56 [PATCH v4 00/22] x86: Support Intel Advanced Matrix Extensions Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 01/22] x86/fpu/xstate: Modify the initialization helper to handle both static and dynamic buffers Chang S. Bae
2021-03-10 13:40   ` Borislav Petkov
2021-02-21 18:56 ` [PATCH v4 02/22] x86/fpu/xstate: Modify state copy helpers " Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 03/22] x86/fpu/xstate: Modify address finders " Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 04/22] x86/fpu/xstate: Modify the context restore helper " Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 05/22] x86/fpu/xstate: Add a new variable to indicate dynamic user states Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 06/22] x86/fpu/xstate: Add new variables to indicate dynamic xstate buffer size Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 07/22] x86/fpu/xstate: Calculate and remember dynamic xstate buffer sizes Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 08/22] x86/fpu/xstate: Convert the struct fpu 'state' field to a pointer Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 09/22] x86/fpu/xstate: Introduce helpers to manage the xstate buffer dynamically Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 10/22] x86/fpu/xstate: Define the scope of the initial xstate data Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 11/22] x86/fpu/xstate: Update the xstate save function to support dynamic states Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 12/22] x86/fpu/xstate: Update the xstate buffer address finder " Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 13/22] x86/fpu/xstate: Update the xstate context copy function " Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 14/22] x86/fpu/xstate: Expand the xstate buffer on the first use of dynamic user state Chang S. Bae
2021-03-20 22:13   ` Thomas Gleixner
2021-03-20 22:21     ` Andy Lutomirski
2021-03-23 21:01       ` Len Brown [this message]
2021-03-24  3:14         ` Liu, Jing2
2021-03-24 21:09           ` Len Brown
2021-03-24 21:26             ` Andy Lutomirski
2021-03-24 21:30               ` Dave Hansen
2021-03-24 21:42                 ` Andy Lutomirski
2021-03-24 21:58                   ` Dave Hansen
2021-03-24 22:12                     ` Andy Lutomirski
2021-03-25  5:12             ` Liu, Jing2
2021-03-25  6:59               ` Bae, Chang Seok
2021-03-25  7:26                 ` Liu, Jing2
2021-03-23 21:52     ` Bae, Chang Seok
2021-03-24 14:24       ` Dave Hansen
2021-03-29 13:14     ` Len Brown
2021-03-29 13:33       ` Thomas Gleixner
2021-03-29 15:43         ` Len Brown
2021-03-29 16:06           ` Len Brown
2021-03-29 17:43             ` Andy Lutomirski
2021-03-29 18:57               ` Len Brown
2021-03-29 18:49           ` Thomas Gleixner
2021-03-29 22:16             ` Len Brown
2021-03-30  8:28               ` Thomas Gleixner
2021-03-30 16:38                 ` Len Brown
2021-03-26 16:34   ` Jann Horn
2021-03-29 18:14     ` Bae, Chang Seok
2021-02-21 18:56 ` [PATCH v4 15/22] x86/fpu/xstate: Support ptracer-induced xstate buffer expansion Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 16/22] x86/fpu/xstate: Extend the table to map state components with features Chang S. Bae
2021-03-20 21:25   ` Thomas Gleixner
2021-03-23 21:52     ` Bae, Chang Seok
2021-02-21 18:56 ` [PATCH v4 17/22] x86/cpufeatures/amx: Enumerate Advanced Matrix Extension (AMX) feature bits Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 18/22] x86/fpu/amx: Define AMX state components and have it used for boot-time checks Chang S. Bae
2021-03-20 21:31   ` Thomas Gleixner
2021-03-23 21:52     ` Bae, Chang Seok
2021-02-21 18:56 ` [PATCH v4 19/22] x86/fpu/amx: Enable the AMX feature in 64-bit mode Chang S. Bae
2021-03-20 21:26   ` Thomas Gleixner
2021-03-23 21:51     ` Bae, Chang Seok
2021-02-21 18:56 ` [PATCH v4 20/22] selftest/x86/amx: Include test cases for the AMX state management Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 21/22] x86/fpu/xstate: Support dynamic user state in the signal handling path Chang S. Bae
2021-02-21 18:56 ` [PATCH v4 22/22] x86/fpu/xstate: Introduce boot-parameters to control state component support Chang S. Bae
2021-02-21 19:30   ` Randy Dunlap
2021-02-21 20:10     ` Bae, Chang Seok
2021-02-21 20:37       ` Randy Dunlap
2021-03-20 20:56   ` Thomas Gleixner
2021-03-25 22:59     ` Len Brown
2021-03-25 23:10       ` Dave Hansen
2021-03-26 15:27         ` Len Brown
2021-03-26 19:22           ` Thomas Gleixner
2021-03-26  1:41       ` Andy Lutomirski
2021-03-26 15:33         ` Len Brown
2021-03-26 15:48           ` Andy Lutomirski
2021-03-26 17:53             ` Len Brown
2021-03-26 18:12               ` Andy Lutomirski
2021-03-27  4:53                 ` Len Brown
2021-03-27 22:20                   ` Thomas Gleixner
2021-03-29 13:31                     ` Len Brown
2021-03-29 14:10                       ` Thomas Gleixner
2021-03-26 18:17               ` Borislav Petkov
2021-03-27  4:41                 ` Len Brown
2021-03-26  1:50       ` Thomas Gleixner
2021-03-26 15:36         ` Len Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJvTdKmz7aePcpi4i+d3vnqLuNAJEuJCjpGDv5WTYcSUfuxoDg@mail.gmail.com \
    --to=lenb@kernel.org \
    --cc=bp@suse.de \
    --cc=chang.seok.bae@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=jing2.liu@intel.com \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=ravi.v.shankar@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.