Messages in this thread | | | Subject | Re: [NEEDS-REVIEW] [PATCH v15 03/26] x86/fpu/xstate: Introduce CET MSR XSAVES supervisor states | From | "Yu, Yu-cheng" <> | Date | Mon, 30 Nov 2020 10:17:09 -0800 |
| |
On 11/30/2020 10:12 AM, Dave Hansen wrote: > On 11/30/20 10:06 AM, Yu, Yu-cheng wrote: >>>> + if (!boot_cpu_has(X86_FEATURE_SHSTK) && >>>> + !boot_cpu_has(X86_FEATURE_IBT)) >>>> + xfeatures_mask_all &= ~BIT_ULL(i); >>>> + } else { >>>> + if ((xsave_cpuid_features[i] == -1) || >>> >>> Where did the -1 come from? Was that introduced earlier in this series? >>> I don't see any way a xsave_cpuid_features[] can be -1 in the >>> current tree. >> >> Yes, we used to have a hole in xsave_cpuid_features[] and put -1 there. >> Do we want to keep this in case we again have holes in the future? > > So, it's dead code for the moment and it's impossible to tell what -1 > means without looking at git history? That seems, um, suboptimal. > > Shouldn't we have: > > #define XFEATURE_NO_DEP -1 > > ? > > And then this code becomes: > > if ((xsave_cpuid_features[i] == XFEATURE_NO_DEP)) > // skip it... > > We can even put a comment in xsave_cpuid_features[] to tell folks to use > it. >
Yes, I will work on that.
Yu-cheng
| |