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, add a new structure to hold the
default features and sizes for guest FPUs to clearly differentiate them
from those for host FPUs.
Note that,
1) 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.
2) only supervisor features will diverge between guest FPUs and host
FPUs, while user features will remain the same [1][2]. So, the new
vcpu_fpu_config struct does not include default user features and size
for the UABI buffer.
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.
Suggested-by: Chang S. Bae <chang.seok.bae@intel.com>
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Link: https://lore.kernel.org/kvm/aAwdQ759Y6V7SGhv@google.com/ [1]
Link: https://lore.kernel.org/kvm/9ca17e1169805f35168eb722734fbf3579187886.camel@intel.com/ [2]
---
v7: add Rick's Reviewed-by
v6:
Drop vcpu_fpu_config.user_* (Rick)
Reset guest default size when XSAVE is unavaiable or disabled (Chang)
v5:
Add a new vcpu_fpu_config instead of adding new members to
fpu_state_config (Chang)
Extract a helper to set default values (Chang)
---
arch/x86/include/asm/fpu/types.h | 26 ++++++++++++++++++++++++++
arch/x86/kernel/fpu/core.c | 1 +
arch/x86/kernel/fpu/init.c | 1 +
arch/x86/kernel/fpu/xstate.c | 27 +++++++++++++++++++++------
4 files changed, 49 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index 1c94121acd3d..abd193a1a52e 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -551,6 +551,31 @@ struct fpu_guest {
struct fpstate *fpstate;
};
+/*
+ * FPU state configuration data for fpu_guest.
+ * Initialized at boot time. Read only after init.
+ */
+struct vcpu_fpu_config {
+ /*
+ * @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 size;
+
+ /*
+ * @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 features;
+};
+
/*
* FPU state configuration data. Initialized at boot time. Read only after init.
*/
@@ -606,5 +631,6 @@ struct fpu_state_config {
/* FPU state configuration information */
extern struct fpu_state_config fpu_kernel_cfg, fpu_user_cfg;
+extern struct vcpu_fpu_config guest_default_cfg;
#endif /* _ASM_X86_FPU_TYPES_H */
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 1cda5b78540b..2cd5e1910ff8 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -36,6 +36,7 @@ DEFINE_PER_CPU(u64, xfd_state);
/* The FPU state configuration data for kernel and user space */
struct fpu_state_config fpu_kernel_cfg __ro_after_init;
struct fpu_state_config fpu_user_cfg __ro_after_init;
+struct vcpu_fpu_config guest_default_cfg __ro_after_init;
/*
* Represents the initial FPU state. It's mostly (but not completely) zeroes,
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index 6bb3e35c40e2..e19660cdc70c 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -202,6 +202,7 @@ static void __init fpu__init_system_xstate_size_legacy(void)
fpu_kernel_cfg.default_size = size;
fpu_user_cfg.max_size = size;
fpu_user_cfg.default_size = size;
+ guest_default_cfg.size = size;
}
/*
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 1c8410b68108..f32047e12500 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -742,6 +742,9 @@ 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);
+
return 0;
}
@@ -762,6 +765,7 @@ static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
fpu_kernel_cfg.default_size = legacy_size;
fpu_user_cfg.max_size = legacy_size;
fpu_user_cfg.default_size = legacy_size;
+ guest_default_cfg.size = legacy_size;
/*
* Prevent enabling the static branch which enables writes to the
@@ -772,6 +776,21 @@ static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
fpstate_reset(x86_task_fpu(current));
}
+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 xfeatures. */
+ kfeatures &= ~XFEATURE_MASK_USER_DYNAMIC;
+ ufeatures &= ~XFEATURE_MASK_USER_DYNAMIC;
+
+ fpu_kernel_cfg.default_features = kfeatures;
+ fpu_user_cfg.default_features = ufeatures;
+
+ guest_default_cfg.features = kfeatures;
+}
+
/*
* Enable and initialize the xsave feature.
* Called once per system bootup.
@@ -854,12 +873,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, determine 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;
--
2.47.1On Mon, May 12, 2025, Chao Gao wrote:
> @@ -772,6 +776,21 @@ static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
> fpstate_reset(x86_task_fpu(current));
> }
>
> +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 xfeatures. */
> + kfeatures &= ~XFEATURE_MASK_USER_DYNAMIC;
> + ufeatures &= ~XFEATURE_MASK_USER_DYNAMIC;
> +
> + fpu_kernel_cfg.default_features = kfeatures;
> + fpu_user_cfg.default_features = ufeatures;
> +
> + guest_default_cfg.features = kfeatures;
> +}
> +
> /*
> * Enable and initialize the xsave feature.
> * Called once per system bootup.
> @@ -854,12 +873,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, determine default values */
> + init_default_features(fpu_kernel_cfg.max_features, fpu_user_cfg.max_features);
Passing in max_features is rather odd. I assume the intent is to capture the
dependencies, but that falls apart by the end of series as the guest features
are initialized as:
guest_default_cfg.features = kfeatures | xfeatures_mask_guest_supervisor();
where xfeatures_mask_guest_supervisor() sneakily consumes fpu_kernel_cfg.max_features,
the very field this patch deliberately avoids consuming directly.
static inline u64 xfeatures_mask_guest_supervisor(void)
{
return fpu_kernel_cfg.max_features & XFEATURE_MASK_GUEST_SUPERVISOR;
}
Rather than providing a helper to initialize the defaults, what if we provide
helpers to provide the default *masks*? Then the dependencies on max_features
are super clear.
E.g. spread over multiple patches (completely untested)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index be1cdfa9b00d..e52c7517df5f 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -780,27 +780,22 @@ static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
fpstate_reset(x86_task_fpu(current));
}
-static void __init init_default_features(u64 kernel_max_features, u64 user_max_features)
+static u64 __init host_default_mask(void)
{
- u64 kfeatures = kernel_max_features;
- u64 ufeatures = user_max_features;
-
/*
- * Default feature sets should not include dynamic and guest-only
- * xfeatures at all.
+ * Exclude dynamic features (require userspace opt-in) and features
+ * that are supported only for KVM guests.
*/
- 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;
+ return ~((u64)XFEATURE_MASK_USER_DYNAMIC | XFEATURE_MASK_GUEST_SUPERVISOR);
+}
+static u64 __init guest_default_mask(void)
+{
/*
- * Ensure VCPU FPU container only reserves a space for guest-only
- * xfeatures. This distinction can save kernel memory by
- * maintaining a necessary amount of XSAVE buffer.
+ * Exclude dynamic features, which require userspace opt-in even for
+ * KVM guests.
*/
- guest_default_cfg.features = kfeatures | xfeatures_mask_guest_supervisor();
+ return ~(u64)XFEATURE_MASK_USER_DYNAMIC;
}
/*
@@ -886,7 +881,9 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
fpu_user_cfg.max_features &= XFEATURE_MASK_USER_SUPPORTED;
/* Now, given maximum feature set, determine default values */
- init_default_features(fpu_kernel_cfg.max_features, fpu_user_cfg.max_features);
+ fpu_kernel_cfg.default_features = fpu_kernel_cfg.max_features & host_default_mask();
+ fpu_user_cfg.default_features = fpu_user_cfg.max_features & host_default_mask();
+ guest_default_cfg.features = fpu_kernel_cfg.max_features & guest_default_mask();
/* Store it for paranoia check at the end */
xfeatures = fpu_kernel_cfg.max_features;
diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
index 9e496391b5f0..52ce19289989 100644
--- a/arch/x86/kernel/fpu/xstate.h
+++ b/arch/x86/kernel/fpu/xstate.h
@@ -62,11 +62,6 @@ static inline u64 xfeatures_mask_supervisor(void)
return fpu_kernel_cfg.max_features & XFEATURE_MASK_SUPERVISOR_SUPPORTED;
}
-static inline u64 xfeatures_mask_guest_supervisor(void)
-{
- return fpu_kernel_cfg.max_features & XFEATURE_MASK_GUEST_SUPERVISOR;
-}
-
static inline u64 xfeatures_mask_independent(void)
{
if (!cpu_feature_enabled(X86_FEATURE_ARCH_LBR))
On Wed, May 21, 2025 at 09:49:48AM -0700, Sean Christopherson wrote:
>On Mon, May 12, 2025, Chao Gao wrote:
>> @@ -772,6 +776,21 @@ static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
>> fpstate_reset(x86_task_fpu(current));
>> }
>>
>> +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 xfeatures. */
>> + kfeatures &= ~XFEATURE_MASK_USER_DYNAMIC;
>> + ufeatures &= ~XFEATURE_MASK_USER_DYNAMIC;
>> +
>> + fpu_kernel_cfg.default_features = kfeatures;
>> + fpu_user_cfg.default_features = ufeatures;
>> +
>> + guest_default_cfg.features = kfeatures;
>> +}
>> +
>> /*
>> * Enable and initialize the xsave feature.
>> * Called once per system bootup.
>> @@ -854,12 +873,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, determine default values */
>> + init_default_features(fpu_kernel_cfg.max_features, fpu_user_cfg.max_features);
>
>Passing in max_features is rather odd. I assume the intent is to capture the
>dependencies, but that falls apart by the end of series as the guest features
>are initialized as:
>
> guest_default_cfg.features = kfeatures | xfeatures_mask_guest_supervisor();
>
>where xfeatures_mask_guest_supervisor() sneakily consumes fpu_kernel_cfg.max_features,
>the very field this patch deliberately avoids consuming directly.
>
> static inline u64 xfeatures_mask_guest_supervisor(void)
> {
> return fpu_kernel_cfg.max_features & XFEATURE_MASK_GUEST_SUPERVISOR;
> }
>
Indeed, it is odd. And your suggestion looks good to me. Thanks.
I will fix this and the comment issue you pointed out and post a new version.
© 2016 - 2025 Red Hat, Inc.