Re: [PATCH v2 8/8] x86/fpu/xstate: Restore supervisor xstates for __fpu__restore_sig()

From: Borislav Petkov <bp@alien8.de>
To: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: linux-kernel@vger.kernel.org, x86@kernel.org,
	"H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Tony Luck <tony.luck@intel.com>,
	Andy Lutomirski <luto@kernel.org>,
	Rik van Riel <riel@surriel.com>,
	"Ravi V. Shankar" <ravi.v.shankar@intel.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH v2 8/8] x86/fpu/xstate: Restore supervisor xstates for __fpu__restore_sig()
Date: Fri, 28 Feb 2020 18:22:02 +0100	[thread overview]
Message-ID: <20200228172202.GD25261@zn.tnic> (raw)
In-Reply-To: <6f91699c91f9ea0f527e80ed3ea2999444a8d2d1.camel@intel.com>

On Fri, Feb 28, 2020 at 08:20:27AM -0800, Yu-cheng Yu wrote:
> When XSAVES writes to an xsave buffer, xsave->header.xcomp_bv is set to
> include only saved components, effectively changing the buffer's format.

So you want to *save* the supervisor states and xcomp_bv will be set to
supervisor states only and since we don't care about the user states
there - they will be loaded later - we're good.

Or do you have to set xcomp_bv later in order to save the user
components too and also rearrange the buffer to undo the format change
above?

We have using_compacted_format() and we do conversion from compacted to
standard buffers - I'm looking at copy_xstate_to_kernel() et al - so it
shouldn't be impossible. So to repeat Sebastian's question which you
ignored:

"How large is this supervisor state at most? I guess saving the AVX512
state just to get the 2 bytes of the supervisor state at the right spot
is not really optimal."

In any case, this performance penalty better be paid only by those
who are actually using some supervisor states. I haven't looked at
the CET patchset but I'm assuming you're setting the CET bit in
xfeatures_mask_all only when the feature is being actually used?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette