Currently, guest and host FPUs share the same default features. However,
the CET supervisor xstate is the first feature that needs to be enabled
exclusively for guest FPUs. Enabling it for host FPUs leads to a waste of
24 bytes in the XSAVE buffer.

To support "guest-only" features, introduce two new members,
guest_default_features and guest_default_size, in fpu_kernel_cfg to clearly
differentiate the default features for host and guest FPUs.

An alternative approach is adding a guest_only_xfeatures member to
fpu_kernel_cfg and adding two helper functions to calculate the guest
default xfeatures and size. However, calculating these defaults at runtime
would introduce unnecessary overhead.

Note that, for now, the default features for guest and host FPUs remain the
same. This will change in a follow-up patch once guest permissions, default
xfeatures, and fpstate size are all converted to use the guest defaults.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/include/asm/fpu/types.h | 20 ++++++++++++++++++++
 arch/x86/kernel/fpu/xstate.c     | 16 +++++++++++-----
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index d555f89db42f..80647c060b32 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -573,6 +573,16 @@ struct fpu_state_config {
 	 */
 	unsigned int		default_size;
 
+	/*
+	 * @guest_default_size:
+	 *
+	 * The default size of the register state buffer in guest FPUs.
+	 * Includes all supported features except independent managed
+	 * features and features which have to be requested by user space
+	 * before usage.
+	 */
+	unsigned int		guest_default_size;
+
 	/*
 	 * @max_features:
 	 *
@@ -589,6 +599,16 @@ struct fpu_state_config {
 	 * be requested by user space before usage.
 	 */
 	u64 default_features;
+
+	/*
+	 * @guest_default_features:
+	 *
+	 * The default supported features bitmap in guest FPUs. Does not
+	 * include independent managed features and features which have
+	 * to be requested by user space before usage.
+	 */
+	u64 guest_default_features;
+
 	/*
 	 * @legacy_features:
 	 *
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 14c3a8285f50..1dd6ddba8723 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -673,7 +673,7 @@ static unsigned int __init get_xsave_size_user(void)
 static int __init init_xstate_size(void)
 {
 	/* Recompute the context size for enabled features: */
-	unsigned int user_size, kernel_size, kernel_default_size;
+	unsigned int user_size, kernel_size;
 	bool compacted = cpu_feature_enabled(X86_FEATURE_XCOMPACTED);
 
 	/* Uncompacted user space size */
@@ -692,18 +692,20 @@ static int __init init_xstate_size(void)
 	else
 		kernel_size = user_size;
 
-	kernel_default_size =
-		xstate_calculate_size(fpu_kernel_cfg.default_features, compacted);
-
 	if (!paranoid_xstate_size_valid(kernel_size))
 		return -EINVAL;
 
 	fpu_kernel_cfg.max_size = kernel_size;
 	fpu_user_cfg.max_size = user_size;
 
-	fpu_kernel_cfg.default_size = kernel_default_size;
+	fpu_kernel_cfg.default_size =
+		xstate_calculate_size(fpu_kernel_cfg.default_features, compacted);
+	fpu_kernel_cfg.guest_default_size =
+		xstate_calculate_size(fpu_kernel_cfg.guest_default_features, compacted);
 	fpu_user_cfg.default_size =
 		xstate_calculate_size(fpu_user_cfg.default_features, false);
+	fpu_user_cfg.guest_default_size =
+		xstate_calculate_size(fpu_user_cfg.guest_default_features, false);
 
 	return 0;
 }
@@ -721,8 +723,10 @@ static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
 	/* Restore the legacy size.*/
 	fpu_kernel_cfg.max_size = legacy_size;
 	fpu_kernel_cfg.default_size = legacy_size;
+	fpu_kernel_cfg.guest_default_size = legacy_size;
 	fpu_user_cfg.max_size = legacy_size;
 	fpu_user_cfg.default_size = legacy_size;
+	fpu_user_cfg.guest_default_size = legacy_size;
 
 	/*
 	 * Prevent enabling the static branch which enables writes to the
@@ -807,9 +811,11 @@ 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.guest_default_features = fpu_kernel_cfg.default_features;
 
 	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;
 
 	/* Store it for paranoia check at the end */
 	xfeatures = fpu_kernel_cfg.max_features;
-- 
2.46.1

On 3/18/2025 8:31 AM, Chao Gao wrote:
> 
> @@ -807,9 +811,11 @@ 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.guest_default_features = fpu_kernel_cfg.default_features;
>   
>   	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;

And you'll add up this on patch7:

   + /* Clean out guest-only features from default */
   + fpu_kernel_cfg.default_features &= ~XFEATURE_MASK_SUPERVISOR_GUEST;


I'm not sure this overall hunk is entirely clear.


Taking a step back, we currently define three types of xfeature sets:

   1. 'default_features'     in a task-inlined buffer
   2. 'max_features'         in an extended buffer
   3. 'independent_features' in a separate buffer (only for LBR)

The VCPU fpstate has so far followed (1) and (2). Now, since we're 
introducing divergence at (1), you've named it guest_default_features:

   'default_features' < 'guest_default_features' < 'max_features'

I don’t see a strong reason against introducing this new field, as 
'guest' already implies the VCPU state. However, rather than directly 
modifying or extending struct fpu_state_config — which may not align 
well with VCPU FPU properties — a dedicated struct could provide a 
cleaner and more structured alternative:

   struct vcpu_fpu_config {
     unsigned int size;
     unsigned int user_size;
     u64 features;
     u64 user_features;
   } guest_default_cfg;

This struct would make VCPU-specific state handling clearer:

   (1) Guest permission setup:

        /* Set the guest default permission */
        fpu->guest_perm.__state_perm      = guest_default_cfg.features;
        fpu->guest_perm.__state_size      = guest_default_cfg.size;
        fpu->guest_perm.__user_state_size = guest_default_cfg.user_size;

   (2) VCPU allocation time:

        fpstate->size           = guest_default_cfg.size;
        fpstate->user_size      = guest_default_cfg.user_size;
        fpstate->xfeatures      = guest_default_cfg.features;
        fpstate->user_xfeatures = guest_default_cfg.user_features;

These assignments considerably make the code more readable.

With that, going back to the default settings, perhaps refactoring it 
could be an option to improve clarity in distinguishing guest vs. host 
settings.

See the attached diff file. I thought this restructuring could make the 
logic more explicit and highlight the differences between guest and host 
settings.

Thanks,
Changdiff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 40621ee4d65b..d2f7ce45d4de 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -702,6 +702,11 @@ static int __init init_xstate_size(void)
 	fpu_user_cfg.default_size =
 		xstate_calculate_size(fpu_user_cfg.default_features, false);
 
+	guest_default_cfg.size =
+		xstate_calculate_size(guest_default_cfg.features, compacted);
+	guest_default_cfg.user_size =
+		xstate_calculate_size(guest_default_cfg.user_features, false);
+
 	return 0;
 }
 
@@ -730,6 +735,30 @@ static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
 	fpstate_reset(&current->thread.fpu);
 }
 
+static void __init init_default_features(u64 kernel_max_features, u64 user_max_features)
+{
+	u64 kfeatures = kernel_max_features;
+	u64 ufeatures = user_max_features;
+
+	/*
+	 * Default feature sets should not include dynamic and guest-only
+	 * xfeatures at all:
+	 */
+	kfeatures &= ~(XFEATURE_MASK_USER_DYNAMIC | XFEATURE_MASK_GUEST_SUPERVISOR);
+	ufeatures &= ~XFEATURE_MASK_USER_DYNAMIC;
+
+	fpu_kernel_cfg.default_features = kfeatures;
+	fpu_user_cfg.default_features   = ufeatures;
+
+	/*
+	 * Ensure VCPU FPU container only reserves a space for
+	 * guest-exclusive xfeatures. This distinction can save kernel
+	 * memory by maintaining a necessary amount of XSAVE buffer.
+	 */
+	guest_default_cfg.features      = kfeatures | xfeatures_mask_guest_supervisor();
+	guest_default_cfg.user_features = ufeatures;
+}
+
 /*
  * Enable and initialize the xsave feature.
  * Called once per system bootup.
@@ -801,12 +830,8 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
 	fpu_user_cfg.max_features = fpu_kernel_cfg.max_features;
 	fpu_user_cfg.max_features &= XFEATURE_MASK_USER_SUPPORTED;
 
-	/* 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_user_cfg.default_features = fpu_user_cfg.max_features;
-	fpu_user_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
+	/* Now, given maximum feature set, determin default values: */
+	init_default_features(fpu_kernel_cfg.max_features, fpu_user_cfg.max_features);
 
 	/* Store it for paranoia check at the end */
 	xfeatures = fpu_kernel_cfg.max_features;

On Tue, Apr 01, 2025 at 10:18:07AM -0700, Chang S. Bae wrote:
>On 3/18/2025 8:31 AM, Chao Gao wrote:
>> 
>> @@ -807,9 +811,11 @@ 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.guest_default_features = fpu_kernel_cfg.default_features;
>>   	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;
>
>And you'll add up this on patch7:
>
>  + /* Clean out guest-only features from default */
>  + fpu_kernel_cfg.default_features &= ~XFEATURE_MASK_SUPERVISOR_GUEST;
>
>
>I'm not sure this overall hunk is entirely clear.

I agree that this hunk is unclear, and your version is much better.

>
>
>Taking a step back, we currently define three types of xfeature sets:
>
>  1. 'default_features'     in a task-inlined buffer
>  2. 'max_features'         in an extended buffer
>  3. 'independent_features' in a separate buffer (only for LBR)
>
>The VCPU fpstate has so far followed (1) and (2). Now, since we're
>introducing divergence at (1), you've named it guest_default_features:
>
>  'default_features' < 'guest_default_features' < 'max_features'
>
>I don’t see a strong reason against introducing this new field, as 'guest'
>already implies the VCPU state. However, rather than directly modifying or
>extending struct fpu_state_config — which may not align well with VCPU FPU
>properties — a dedicated struct could provide a cleaner and more structured
>alternative:
>
>  struct vcpu_fpu_config {
>    unsigned int size;
>    unsigned int user_size;
>    u64 features;
>    u64 user_features;
>  } guest_default_cfg;

Your suggestion looks good to me, and I can definitely incorporate the change
in the next version. Thanks a lot, Chang.