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

From: Borislav Petkov
Date: Tue Sep 24 2019 - 11:28:52 EST


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.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette