diff mbox series

[v3,05/10] x86/fpu/xstate: Introduce guest FPU configuration

Message ID 20250307164123.1613414-6-chao.gao@intel.com (mailing list archive)
State New
Headers show
Series Introduce CET supervisor state support | expand

Commit Message

Chao Gao March 7, 2025, 4:41 p.m. UTC 
From: Yang Weijiang <weijiang.yang@intel.com>

The guest fpstate is currently initialized using fpu_kernel_cfg. This lacks
flexibility because every feature added to the kernel FPU config is
automatically available for use in the guest FPU. And to make any feature
available for the guest FPU, the feature should be added to the kernel FPU
config.

Introduce fpu_guest_cfg to separate the guest FPU config from the kernel
FPU config. This enhances code readability by allowing the guest FPU to be
initialized with its own config and also improves extensibility by allowing
the guest FPU config and kernel FPU config to evolve independently.

Note fpu_guest_cfg and fpu_kernel_cfg remain the same for now.

Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
 arch/x86/include/asm/fpu/types.h |  2 +-
 arch/x86/kernel/fpu/core.c       |  3 ++-
 arch/x86/kernel/fpu/xstate.c     | 10 ++++++++++
 3 files changed, 13 insertions(+), 2 deletions(-)

Comments

Dave Hansen March 7, 2025, 6:06 p.m. UTC | #1 
On 3/7/25 08:41, Chao Gao wrote:
> +	fpu_guest_cfg.max_features = fpu_kernel_cfg.max_features;
> +	fpu_guest_cfg.default_features = fpu_guest_cfg.max_features;
> +	fpu_guest_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;

Is just copying and pasting the 'fpu_kernel_cfg' initialization really
the best way to explain what's going on?

Let's say you just did:

	fpu_guest_cfg = fpu_kernel_cfg;

?

That would explicitly tell the reader that they're equal.
Chao Gao March 8, 2025, 3 a.m. UTC | #2 
On Fri, Mar 07, 2025 at 10:06:56AM -0800, Dave Hansen wrote:
>On 3/7/25 08:41, Chao Gao wrote:
>> +	fpu_guest_cfg.max_features = fpu_kernel_cfg.max_features;
>> +	fpu_guest_cfg.default_features = fpu_guest_cfg.max_features;
>> +	fpu_guest_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
>
>Is just copying and pasting the 'fpu_kernel_cfg' initialization really
>the best way to explain what's going on?
>
>Let's say you just did:
>
>	fpu_guest_cfg = fpu_kernel_cfg;
>
>?
>
>That would explicitly tell the reader that they're equal.
>

Yes. This is much better.

>
diff mbox series

Patch

diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index 9f9ed406b179..d9515d7f65e4 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -596,6 +596,6 @@  struct fpu_state_config {
 };
 
 /* FPU state configuration information */
-extern struct fpu_state_config fpu_kernel_cfg, fpu_user_cfg;
+extern struct fpu_state_config fpu_kernel_cfg, fpu_user_cfg, fpu_guest_cfg;
 
 #endif /* _ASM_X86_FPU_TYPES_H */
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index adc34914634e..b0c1ef40d105 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -33,9 +33,10 @@  DEFINE_STATIC_KEY_FALSE(__fpu_state_size_dynamic);
 DEFINE_PER_CPU(u64, xfd_state);
 #endif
 
-/* The FPU state configuration data for kernel and user space */
+/* The FPU state configuration data for kernel, user space and guest */
 struct fpu_state_config	fpu_kernel_cfg __ro_after_init;
 struct fpu_state_config fpu_user_cfg __ro_after_init;
+struct fpu_state_config fpu_guest_cfg __ro_after_init;
 
 /*
  * Represents the initial FPU state. It's mostly (but not completely) zeroes,
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 7caafdb7f6b8..58325b3b8914 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -683,6 +683,7 @@  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 guest_default_size;
 	bool compacted = cpu_feature_enabled(X86_FEATURE_XCOMPACTED);
 
 	/* Uncompacted user space size */
@@ -704,13 +705,18 @@  static int __init init_xstate_size(void)
 	kernel_default_size =
 		xstate_calculate_size(fpu_kernel_cfg.default_features, compacted);
 
+	guest_default_size =
+		xstate_calculate_size(fpu_guest_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_guest_cfg.max_size = kernel_size;
 
 	fpu_kernel_cfg.default_size = kernel_default_size;
+	fpu_guest_cfg.default_size = guest_default_size;
 	fpu_user_cfg.default_size =
 		xstate_calculate_size(fpu_user_cfg.default_features, false);
 
@@ -820,6 +826,10 @@  void __init fpu__init_system_xstate(unsigned int legacy_size)
 	fpu_user_cfg.default_features = fpu_user_cfg.max_features;
 	fpu_user_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
 
+	fpu_guest_cfg.max_features = fpu_kernel_cfg.max_features;
+	fpu_guest_cfg.default_features = fpu_guest_cfg.max_features;
+	fpu_guest_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
+
 	/* Store it for paranoia check at the end */
 	xfeatures = fpu_kernel_cfg.max_features;