Re: [PATCH v5 04/27] x86/fpu/xstate: Add XSAVES system states for shadow stack
From: Yu-cheng Yu <hidden>
Date: 2018-11-08 20:45:20
Also in:
linux-api, linux-doc, linux-mm, lkml
On Thu, 2018-11-08 at 19:40 +0100, Borislav Petkov wrote:
On Thu, Oct 11, 2018 at 08:15:00AM -0700, Yu-cheng Yu wrote:quoted
[...] +/* + * State component 11 is Control flow Enforcement user statesWhy the Camel-cased naming? "Control" then "flow" then capitalized again "Enforcement". Fix all occurrences pls, especially the user-visible strings.
I will change it to "Control-flow Enforcement" everywhere.
quoted
+ */ +struct cet_user_state { + u64 u_cet; /* user control flow settings */ + u64 user_ssp; /* user shadow stack pointer */Prefix both with "usr_" instead.
Ok.
[...] Just write "privilege level" everywhere - not "ring". Btw, do you see how the type and the name of all those other fields in that file are tabulated? Except yours...
I will fix it. [...]
quoted
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 605ec6decf3e..ad36ea28bfd1 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c@@ -35,6 +35,9 @@ static const char *xfeature_names[] = "Processor Trace (unused)" , "Protection Keys User registers", "unknown xstate feature" , + "Control flow User registers" , + "Control flow Kernel registers" , + "unknown xstate feature" ,So there are two "unknown xstate feature" array elems now...quoted
static short xsave_cpuid_features[] __initdata = {@@ -48,6 +51,9 @@ static short xsave_cpuid_features[] __initdata = { X86_FEATURE_AVX512F, X86_FEATURE_INTEL_PT, X86_FEATURE_PKU, + 0, /* Unused */What's that for?
In fpu_init_system_xstate(), we test and clear features that are not enabled. There we depend on the order of these elements. This is the tenth "unknown xstate feature". Yu-cheng