diff mbox series

[v6,04/25] x86/fpu/xstate: Introduce kernel dynamic xfeature set

Message ID 20230914063325.85503-5-weijiang.yang@intel.com
State New
Headers
Series Enable CET Virtualization |

Commit Message

Yang, Weijiang Sept. 14, 2023, 6:33 a.m. UTC 
  Define a new kernel xfeature set including the features can be dynamically
enabled, i.e., the relevant feature is enabled on demand. The xfeature set
is currently used by KVM to configure __guest__ fpstate, i.e., calculating
the xfeature and fpstate storage size etc. The xfeature set is initialized
once and used whenever it's referenced to avoid repeat calculation.

Currently it's used when 1) guest fpstate __state_size is calculated while
guest permits are configured 2) guest vCPU is created and its fpstate is
initialized.

Suggested-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kernel/fpu/xstate.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Edgecombe, Rick P Sept. 15, 2023, 12:24 a.m. UTC | #1 
On Thu, 2023-09-14 at 02:33 -0400, Yang Weijiang wrote:
> +static void __init init_kernel_dynamic_xfeatures(void)
> +{
> +       unsigned short cid;
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(xsave_kernel_dynamic_xfeatures);
> i++) {
> +               cid = xsave_kernel_dynamic_xfeatures[i];
> +
> +               if (cid && boot_cpu_has(cid))
> +                       fpu_kernel_dynamic_xfeatures |= BIT_ULL(i);
> +       }
> +}
> +

I think this can be part of the max_features calculation that uses
xsave_cpuid_features when you use use a fixed mask like Dave suggested
in the other patch.
Yang, Weijiang Sept. 15, 2023, 6:42 a.m. UTC | #2 
On 9/15/2023 8:24 AM, Edgecombe, Rick P wrote:
> On Thu, 2023-09-14 at 02:33 -0400, Yang Weijiang wrote:
>> +static void __init init_kernel_dynamic_xfeatures(void)
>> +{
>> +       unsigned short cid;
>> +       int i;
>> +
>> +       for (i = 0; i < ARRAY_SIZE(xsave_kernel_dynamic_xfeatures);
>> i++) {
>> +               cid = xsave_kernel_dynamic_xfeatures[i];
>> +
>> +               if (cid && boot_cpu_has(cid))
>> +                       fpu_kernel_dynamic_xfeatures |= BIT_ULL(i);
>> +       }
>> +}
>> +
> I think this can be part of the max_features calculation that uses
> xsave_cpuid_features when you use use a fixed mask like Dave suggested
> in the other patch.

Yes, the max_features has already included CET supervisor state bit. After  use
fixed mask, this function is not needed.
Maxim Levitsky Oct. 31, 2023, 5:44 p.m. UTC | #3 
On Fri, 2023-09-15 at 14:42 +0800, Yang, Weijiang wrote:
> On 9/15/2023 8:24 AM, Edgecombe, Rick P wrote:
> > On Thu, 2023-09-14 at 02:33 -0400, Yang Weijiang wrote:
> > > +static void __init init_kernel_dynamic_xfeatures(void)
> > > +{
> > > +       unsigned short cid;
> > > +       int i;
> > > +
> > > +       for (i = 0; i < ARRAY_SIZE(xsave_kernel_dynamic_xfeatures);
> > > i++) {
> > > +               cid = xsave_kernel_dynamic_xfeatures[i];
> > > +
> > > +               if (cid && boot_cpu_has(cid))
> > > +                       fpu_kernel_dynamic_xfeatures |= BIT_ULL(i);
> > > +       }
> > > +}
> > > +
> > I think this can be part of the max_features calculation that uses
> > xsave_cpuid_features when you use use a fixed mask like Dave suggested
> > in the other patch.
> 
> Yes, the max_features has already included CET supervisor state bit. After  use
> fixed mask, this function is not needed.
> 
> 
My 0.2 cents are also on having XFEATURE_MASK_KERNEL_DYNAMIC macro instead.

Best regards,
	Maxim Levitsky
diff mbox series

Patch

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index c3ed86732d33..eaec05bc1b3c 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -84,6 +84,8 @@  static unsigned int xstate_sizes[XFEATURE_MAX] __ro_after_init =
 	{ [ 0 ... XFEATURE_MAX - 1] = -1};
 static unsigned int xstate_flags[XFEATURE_MAX] __ro_after_init;
 
+u64 fpu_kernel_dynamic_xfeatures __ro_after_init;
+
 #define XSTATE_FLAG_SUPERVISOR	BIT(0)
 #define XSTATE_FLAG_ALIGNED64	BIT(1)
 
@@ -740,6 +742,23 @@  static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
 	fpstate_reset(&current->thread.fpu);
 }
 
+static unsigned short xsave_kernel_dynamic_xfeatures[] = {
+	[XFEATURE_CET_KERNEL]	= X86_FEATURE_SHSTK,
+};
+
+static void __init init_kernel_dynamic_xfeatures(void)
+{
+	unsigned short cid;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(xsave_kernel_dynamic_xfeatures); i++) {
+		cid = xsave_kernel_dynamic_xfeatures[i];
+
+		if (cid && boot_cpu_has(cid))
+			fpu_kernel_dynamic_xfeatures |= BIT_ULL(i);
+	}
+}
+
 /*
  * Enable and initialize the xsave feature.
  * Called once per system bootup.
@@ -809,6 +828,8 @@  void __init fpu__init_system_xstate(unsigned int legacy_size)
 	if (boot_cpu_has(X86_FEATURE_SHSTK) || boot_cpu_has(X86_FEATURE_IBT))
 		fpu_kernel_cfg.max_features |= BIT_ULL(XFEATURE_CET_USER);
 
+	init_kernel_dynamic_xfeatures();
+
 	if (!cpu_feature_enabled(X86_FEATURE_XFD))
 		fpu_kernel_cfg.max_features &= ~XFEATURE_MASK_USER_DYNAMIC;