On Tue, Sep 24, 2019 at 05:28:48PM +0200, Borislav Petkov wrote:
> On Tue, Sep 03, 2019 at 05:26:32PM +0300, Jarkko Sakkinen wrote:
> > diff --git a/arch/x86/include/asm/cpufeatures.h 
> > b/arch/x86/include/asm/cpufeatures.h
> > index 998c2cc08363..c5582e766121 100644
> > --- a/arch/x86/include/asm/cpufeatures.h
> > +++ b/arch/x86/include/asm/cpufeatures.h
> > @@ -222,12 +222,22 @@
> >  #define X86_FEATURE_L1TF_PTEINV            ( 7*32+29) /* "" L1TF 
> > workaround PTE inversion */
> >  #define X86_FEATURE_IBRS_ENHANCED  ( 7*32+30) /* Enhanced IBRS */
> >  
> > -/* Virtualization flags: Linux defined, word 8 */
> > -#define X86_FEATURE_TPR_SHADOW             ( 8*32+ 0) /* Intel TPR Shadow 
> > */
> > -#define X86_FEATURE_VNMI           ( 8*32+ 1) /* Intel Virtual NMI */
> > -#define X86_FEATURE_FLEXPRIORITY   ( 8*32+ 2) /* Intel FlexPriority */
> > -#define X86_FEATURE_EPT                    ( 8*32+ 3) /* Intel Extended 
> > Page Table */
> > -#define X86_FEATURE_VPID           ( 8*32+ 4) /* Intel Virtual Processor 
> > ID */
> > +/*
> > + * Scattered Intel features: Linux defined, word 8.
> > + *
> > + * Note that the bit location of the SGX features is meaningful as KVM 
> > expects
> > + * the Linux defined bit to match the Intel defined bit, e.g. 
> > X86_FEATURE_SGX1
> > + * must remain at bit 0, SGX2 at bit 1, etc...
> 
> Eww, no.
> 
> > + */
> > +#define X86_FEATURE_SGX1           ( 8*32+ 0) /* SGX1 leaf functions */
> > +#define X86_FEATURE_SGX2           ( 8*32+ 1) /* SGX2 leaf functions */
> > +/* Bits [0:7] are reserved for SGX */
> 
> That leaf has "Bits 31 - 07: Reserved." So what happens if they start
> adding more bits there? We shoosh the other defines even further into
> the word?
> 
> Talk to your hw guys, if the plan is to leave those bits for other
> feature flags, then let's allocate a new capability word for F12_EAX.

We tried that, you shot it down[*], hence these shenanigans.  With respect
to more SGX feature flags, the original changelog even stated "with more
expected in the not-too-distant future".

I'm not arguing that this isn't ugly, just want to make it clear that
we're not wantonly throwing junk into the kernel.  I'm all for a dedicated
SGX word, it makes our lives easier.

[*] https://lkml.kernel.org/r/[email protected]

Reply via email to