| Message ID | 20230914063325.85503-5-weijiang.yang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable CET Virtualization | expand |
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.
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.
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 --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(¤t->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;
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(+)