Re: [PATCH v14 03/26] x86/fpu/xstate: Introduce CET MSR XSAVES supervisor states

[Yu, Yu-cheng]

On 10/12/2020 12:58 PM, Cyrill Gorcunov wrote:
> On Mon, Oct 12, 2020 at 08:38:27AM -0700, Yu-cheng Yu wrote:
> ...
> >   /*
> >    * x86-64 Task Priority Register, CR8
> > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> > index 038e19c0019e..705fd9b94e31 100644
> > --- a/arch/x86/kernel/fpu/xstate.c
> > +++ b/arch/x86/kernel/fpu/xstate.c
> > @@ -38,6 +38,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"	,
> >   };
> >   
> >   static short xsave_cpuid_features[] __initdata = {
> > @@ -51,6 +54,9 @@ static short xsave_cpuid_features[] __initdata = {
> >   	X86_FEATURE_AVX512F,
> >   	X86_FEATURE_INTEL_PT,
> >   	X86_FEATURE_PKU,
> > +	-1,		   /* Unused */
> > +	X86_FEATURE_SHSTK, /* XFEATURE_CET_USER */
> > +	X86_FEATURE_SHSTK, /* XFEATURE_CET_KERNEL */
> >   };
>
> Why do you need "-1" here in the array? The only 1:1 mapping is between
> the names itselves and values, not indices of arrays so i don't understand
> why we need this unused value. Sorry if it is a dumb questions and
> been discussed already.

The indices are used indirectly in fpu__init_system_xstate() to set bits 
in xfeatures_mask_all, i.e.

xfeatures_mask_all &= ~BIT_ULL(i).

So they need to match the xstate feature bits.

Yu-cheng