On 11/24/2023 5:40 PM, Peter Zijlstra wrote:
On Fri, Nov 24, 2023 at 12:53:06AM -0500, Yang Weijiang wrote:
Remove XFEATURE_CET_USER entry from dependency array as the entry doesn't
reflect true dependency between CET features and the user xstate bit.
Enable the bit in fpu_kernel_cfg.max_features when either SHSTK or IBT is
available.
Both user mode shadow stack and indirect branch tracking features depend
on XFEATURE_CET_USER bit in XSS to automatically save/restore user mode
xstate registers, i.e., IA32_U_CET and IA32_PL3_SSP whenever necessary.
Note, the issue, i.e., CPUID only enumerates IBT but no SHSTK is resulted
from CET KVM series which synthesizes guest CPUIDs based on userspace
settings,in real world the case is rare. In other words, the exitings
dependency check is correct when only user mode SHSTK is available.
Signed-off-by: Yang Weijiang <weijiang.yang@xxxxxxxxx>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
Tested-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
---
arch/x86/kernel/fpu/xstate.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 73f6bc00d178..6e50a4251e2b 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -73,7 +73,6 @@ static unsigned short xsave_cpuid_features[] __initdata = {
[XFEATURE_PT_UNIMPLEMENTED_SO_FAR] = X86_FEATURE_INTEL_PT,
[XFEATURE_PKRU] = X86_FEATURE_OSPKE,
[XFEATURE_PASID] = X86_FEATURE_ENQCMD,
- [XFEATURE_CET_USER] = X86_FEATURE_SHSTK,
[XFEATURE_XTILE_CFG] = X86_FEATURE_AMX_TILE,
[XFEATURE_XTILE_DATA] = X86_FEATURE_AMX_TILE,
};
@@ -798,6 +797,14 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
fpu_kernel_cfg.max_features &= ~BIT_ULL(i);
}
+ /*
+ * CET user mode xstate bit has been cleared by above sanity check.
+ * Now pick it up if either SHSTK or IBT is available. Either feature
+ * depends on the xstate bit to save/restore user mode states.
+ */
+ if (boot_cpu_has(X86_FEATURE_SHSTK) || boot_cpu_has(X86_FEATURE_IBT))
+ fpu_kernel_cfg.max_features |= BIT_ULL(XFEATURE_CET_USER);
So booting a host with "ibt=off" will clear the FEATURE_IBT, this was
fine before this patch-set, but possibly not with.
That kernel argument really only wants to tell the kernel not to use IBT
itself, but not inhibit IBT from being used by guests.
Thanks for pointing this out!
If ibt=off actually causes XFEATURE_CET_USER unset in fpu_kernel_cfg.max_features, in KVM part (patch 24), we already have below check to disable SHSTK/IBT in this cases, so looks like it won't bring issues.
+if ((kvm_caps.supported_xss & (XFEATURE_MASK_CET_USER |
+XFEATURE_MASK_CET_KERNEL)) !=
+(XFEATURE_MASK_CET_USER | XFEATURE_MASK_CET_KERNEL)) {
+kvm_cpu_cap_clear(X86_FEATURE_SHSTK);
+kvm_cpu_cap_clear(X86_FEATURE_IBT);
+kvm_caps.supported_xss &= ~XFEATURE_CET_USER;
+kvm_caps.supported_xss &= ~XFEATURE_CET_KERNEL;
+}
+
