[PATCH v2 3/6] x86/fpu/xstate: Introduce XFEATURE_MASK_KERNEL_DYNAMIC xfeature set

Chao Gao posted 6 patches 1 year, 2 months ago
There is a newer version of this series
[PATCH v2 3/6] x86/fpu/xstate: Introduce XFEATURE_MASK_KERNEL_DYNAMIC xfeature set
Posted by Chao Gao 1 year, 2 months ago
From: Yang Weijiang <weijiang.yang@intel.com>

Define a new XFEATURE_MASK_KERNEL_DYNAMIC mask to specify the features
that can be optionally enabled by kernel components. This is similar to
XFEATURE_MASK_USER_DYNAMIC in that it contains optional xfeatures that
can allows the FPU buffer to be dynamically sized. The difference is that
the KERNEL variant contains supervisor features and will be enabled by
kernel components that need them, and not directly by the user. Currently
it's used by KVM to configure guest dedicated fpstate for calculating
the xfeature and fpstate storage size etc.

The kernel dynamic xfeatures now only contain XFEATURE_CET_KERNEL, which
is supported by host as they're enabled in kernel XSS MSR setting but
relevant CPU feature, i.e., supervisor shadow stack, is not enabled in
host kernel therefore it can be omitted for normal fpstate by default.

Remove the kernel dynamic feature from fpu_kernel_cfg.default_features
so that the bits in xstate_bv and xcomp_bv are cleared and xsaves/xrstors
can be optimized by HW for normal fpstate.

Suggested-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
 arch/x86/include/asm/fpu/xstate.h | 5 ++++-
 arch/x86/kernel/fpu/xstate.c      | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 3b4a038d3c57..a212d3851429 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -46,9 +46,12 @@
 #define XFEATURE_MASK_USER_RESTORE	\
 	(XFEATURE_MASK_USER_SUPPORTED & ~XFEATURE_MASK_PKRU)
 
-/* Features which are dynamically enabled for a process on request */
+/* Features which are dynamically enabled per userspace request */
 #define XFEATURE_MASK_USER_DYNAMIC	XFEATURE_MASK_XTILE_DATA
 
+/* Features which are dynamically enabled per kernel side request */
+#define XFEATURE_MASK_KERNEL_DYNAMIC	XFEATURE_MASK_CET_KERNEL
+
 /* All currently supported supervisor features */
 #define XFEATURE_MASK_SUPERVISOR_SUPPORTED (XFEATURE_MASK_PASID | \
 					    XFEATURE_MASK_CET_USER | \
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 2cf6ec536c0d..c38e477e3e45 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -824,6 +824,7 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
 	/* Clean out dynamic features from default */
 	fpu_kernel_cfg.default_features = fpu_kernel_cfg.max_features;
 	fpu_kernel_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
+	fpu_kernel_cfg.default_features &= ~XFEATURE_MASK_KERNEL_DYNAMIC;
 
 	fpu_user_cfg.default_features = fpu_user_cfg.max_features;
 	fpu_user_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
-- 
2.46.1
Re: [PATCH v2 3/6] x86/fpu/xstate: Introduce XFEATURE_MASK_KERNEL_DYNAMIC xfeature set
Posted by Dave Hansen 11 months, 1 week ago
Subjects should ideally be written without large identifiers:

	Subject: x86/fpu/xstate: Introduce dynamic kernel features

On 11/26/24 02:17, Chao Gao wrote:
> Remove the kernel dynamic feature from fpu_kernel_cfg.default_features
> so that the bits in xstate_bv and xcomp_bv are cleared and xsaves/xrstors
> can be optimized by HW for normal fpstate.

I'm really confused why this changelog hunk is here.

Right now, all kernel XSAVE buffers have the same supervisor xfeatures.
This introduces the idea that they can have different xfeature sets.

The _purpose_ of this is to save space when only some FPUs need a given
feature. This saves space in 'struct fpu'. It probably doesn't actually
make XSAVE/XRSTOR any faster because the init optimization is very
likely to be in place for these features.

I'm not sure what point the changelog was trying to make about xstate_bv
and xcomp_bv.  xstate_bv[12] would have been 0 if the feature was in its
init state.
Re: [PATCH v2 3/6] x86/fpu/xstate: Introduce XFEATURE_MASK_KERNEL_DYNAMIC xfeature set
Posted by Chao Gao 11 months, 1 week ago
On Tue, Mar 04, 2025 at 02:37:00PM -0800, Dave Hansen wrote:
>Subjects should ideally be written without large identifiers:
>
>	Subject: x86/fpu/xstate: Introduce dynamic kernel features

will do.

>
>On 11/26/24 02:17, Chao Gao wrote:
>> Remove the kernel dynamic feature from fpu_kernel_cfg.default_features
>> so that the bits in xstate_bv and xcomp_bv are cleared and xsaves/xrstors
>> can be optimized by HW for normal fpstate.
>
>I'm really confused why this changelog hunk is here.
>
>Right now, all kernel XSAVE buffers have the same supervisor xfeatures.
>This introduces the idea that they can have different xfeature sets.
>
>The _purpose_ of this is to save space when only some FPUs need a given
>feature. This saves space in 'struct fpu'. It probably doesn't actually
>make XSAVE/XRSTOR any faster because the init optimization is very
>likely to be in place for these features.

Thanks for the detailed explanation. You're absolutely right—this change
aims to reduce the size of struct fpu. I see now that implying a perf
improvement for XSAVE/XRSTOR was misleading, as the underlying optimization
is likely handled by the init optimization. I'll revise the changelog to
clarify this and incorporate your wording to avoid confusion.

>
>I'm not sure what point the changelog was trying to make about xstate_bv
>and xcomp_bv.  xstate_bv[12] would have been 0 if the feature was in its
>init state.