linux-kernel - Re: [PATCH v22 01/24] x86/cpufeatures: x86/msr: Add Intel SGX hardware bits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 24 Sep 2019 09:11:50 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        linux-kernel@...r.kernel.org, x86@...nel.org,
        linux-sgx@...r.kernel.org, akpm@...ux-foundation.org,
        dave.hansen@...el.com, nhorman@...hat.com, npmccallum@...hat.com,
        serge.ayoun@...el.com, shay.katz-zamir@...el.com,
        haitao.huang@...el.com, andriy.shevchenko@...ux.intel.com,
        tglx@...utronix.de, kai.svahn@...el.com, josh@...htriplett.org,
        luto@...nel.org, kai.huang@...el.com, rientjes@...gle.com,
        cedric.xing@...el.com, Kai Huang <kai.huang@...ux.intel.com>
Subject: Re: [PATCH v22 01/24] x86/cpufeatures: x86/msr: Add Intel SGX
 hardware bits

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/20180828102140.GA31102@nazgul.tnic