From: Yang Weijiang <weijiang.yang@intel.com>
Define a new XFEATURE_MASK_SUPERVISOR_GUEST mask to specify the features
that are enabled by default in guest FPUs but not in host FPUs.
Add CET_KERNEL as the first guest-only feature to save host FPUs from
allocating XSAVE buffer space for all threads on CET-capable parts.
Co-developed-by: Chao Gao <chao.gao@intel.com>
Signed-off-by: Chao Gao <chao.gao@intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
Dropped Dave's Suggested-by as the patch has been changed significantly
---
arch/x86/include/asm/fpu/types.h | 9 +++++----
arch/x86/include/asm/fpu/xstate.h | 3 +++
arch/x86/kernel/fpu/xstate.c | 5 ++++-
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index 80647c060b32..079f3241e25b 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -568,8 +568,9 @@ struct fpu_state_config {
* @default_size:
*
* The default size of the register state buffer. Includes all
- * supported features except independent managed features and
- * features which have to be requested by user space before usage.
+ * supported features except independent managed features,
+ * guest-only features and features which have to be requested by
+ * user space before usage.
*/
unsigned int default_size;
@@ -595,8 +596,8 @@ struct fpu_state_config {
* @default_features:
*
* The default supported features bitmap. Does not include
- * independent managed features and features which have to
- * be requested by user space before usage.
+ * independent managed features, guest-only features and features
+ * which have to be requested by user space before usage.
*/
u64 default_features;
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 8990cf381bef..69db17476061 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -45,6 +45,9 @@
/* Features which are dynamically enabled for a process on request */
#define XFEATURE_MASK_USER_DYNAMIC XFEATURE_MASK_XTILE_DATA
+/* Supervisor features which are enabled only in guest FPUs */
+#define XFEATURE_MASK_SUPERVISOR_GUEST 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 1dd6ddba8723..b19960215074 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -55,7 +55,7 @@ static const char *xfeature_names[] =
"Protection Keys User registers",
"PASID state",
"Control-flow User registers",
- "Control-flow Kernel registers",
+ "Control-flow Kernel registers (KVM only)",
"unknown xstate feature",
"unknown xstate feature",
"unknown xstate feature",
@@ -813,6 +813,9 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
fpu_kernel_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
fpu_kernel_cfg.guest_default_features = fpu_kernel_cfg.default_features;
+ /* Clean out guest-only features from default */
+ fpu_kernel_cfg.default_features &= ~XFEATURE_MASK_SUPERVISOR_GUEST;
+
fpu_user_cfg.default_features = fpu_user_cfg.max_features;
fpu_user_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
fpu_user_cfg.guest_default_features = fpu_user_cfg.default_features;
--
2.46.1On 3/18/2025 8:31 AM, Chao Gao wrote: > > Dropped Dave's Suggested-by as the patch has been changed significantly I think you should provide a clear argument outlining the considerable naming options and their trade-offs. I noticed you referenced Thomas’s feedback in the cover letter (it would be clearer to elaborate here rather than using just the above one-liner): > Rename XFEATURE_MASK_KERNEL_DYNAMIC to XFEATURE_MASK_SUPERVISOR_GUEST > as tglx noted "this dynamic naming is really bad": > > https://lore.kernel.org/all/87sg1owmth.ffs@nanos.tec.linutronix.de/ While Thomas objected to the "dynamic" naming, have you fully considered why he found it problematic? Likewise, have you re-evaluated Dave’s original suggestion and his intent? Rather than just quoting feedback, you should summarize the key concerns, analyze the pros and cons of different naming approaches, and clearly justify your final choice. Thanks, Chang
On Tue, Apr 01, 2025 at 10:16:24AM -0700, Chang S. Bae wrote: >On 3/18/2025 8:31 AM, Chao Gao wrote: >> >> Dropped Dave's Suggested-by as the patch has been changed significantly > >I think you should provide a clear argument outlining the considerable naming >options and their trade-offs. > >I noticed you referenced Thomas’s feedback in the cover letter (it would be >clearer to elaborate here rather than using just the above one-liner): > >> Rename XFEATURE_MASK_KERNEL_DYNAMIC to XFEATURE_MASK_SUPERVISOR_GUEST >> as tglx noted "this dynamic naming is really bad": >> >> https://lore.kernel.org/all/87sg1owmth.ffs@nanos.tec.linutronix.de/ > >While Thomas objected to the "dynamic" naming, have you fully considered why >he found it problematic? Likewise, have you re-evaluated Dave’s original >suggestion and his intent? Rather than just quoting feedback, you should >summarize the key concerns, analyze the pros and cons of different naming >approaches, and clearly justify your final choice. Hi Chang, The 'dynamic' naming was initially slightly preferred over 'guest-only'. But later I discovered new evidence suggesting we should be cautious with the 'dynamic' naming, leading me to choose 'guest-only'. 'dynamic' is abstract, while 'guest-only' more clearly conveys the intended purpose. Using "dynamic" might cause confusion, as it could be associated with dynamic user features. As you noted in the last version, it is quite confusing because it doesn't involve permissions and reallocations like dynamic user features do. I'm not entirely sure why Thomas found "dynamic" problematic. His comment, made 4 years ago, was about independent features. But I am sure that we shouldn't reinstate a name that was considered "really bad" without strong justification. And Dave clearly mentioned he wouldn't oppose the "guest-only" name [1]: But I also don't feel strongly about it and I've said my peace. I won't NAK it one way or the other. Therefore, to be cautious, I chose "guest-only," assuming it is acceptable to Dave and can prevent Thomas and others from questioning the reinstatement of dynamic supervisor features. I can add my thoughts below the --- separator line if the above answers your questions. [1]: https://lore.kernel.org/all/893ac578-baaf-4f4f-96ee-e012dfc073a8@intel.com/#t
© 2016 - 2025 Red Hat, Inc.