diff mbox series

[v17,03/23] x86/cpufeatures: Add SGX sub-features (as Linux-defined bits)

Message ID 20181116010412.23967-4-jarkko.sakkinen@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [v17,01/23] x86/sgx: Update MAINTAINERS | expand

Commit Message

Jarkko Sakkinen Nov. 16, 2018, 1:01 a.m. UTC 
From: Sean Christopherson <sean.j.christopherson@intel.com>

CPUID_12_EAX is an Intel-defined feature bits leaf dedicated for SGX
that enumerates the SGX instruction sets that are supported by the
CPU, e.g. SGX1, SGX2, etc...  Because Linux currently only cares about
two bits (SGX1 and SGX2) and there are currently only four documented
bits in total, relocate the bits to Linux-defined word 8 to conserve
space.

But, keep the bit positions identical between the Intel-defined value
and the Linux-defined value, e.g. keep SGX1 at bit 0.  This allows KVM
to use its existing code for probing guest CPUID bits using Linux's
X86_FEATURE_* definitions.  To do so, shift around some existing bits
to effectively reserve bits 0-7 of word 8 for SGX sub-features.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 arch/x86/include/asm/cpufeatures.h       | 21 +++++++++++++++------
 arch/x86/kernel/cpu/scattered.c          |  2 ++
 tools/arch/x86/include/asm/cpufeatures.h | 21 +++++++++++++++------
 3 files changed, 32 insertions(+), 12 deletions(-)

Comments

Borislav Petkov Nov. 16, 2018, 2:37 p.m. UTC | #1 
On Fri, Nov 16, 2018 at 03:01:10AM +0200, Jarkko Sakkinen wrote:
> From: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> CPUID_12_EAX is an Intel-defined feature bits leaf dedicated for SGX
> that enumerates the SGX instruction sets that are supported by the
> CPU, e.g. SGX1, SGX2, etc...  Because Linux currently only cares about
> two bits (SGX1 and SGX2) and there are currently only four documented
> bits in total, relocate the bits to Linux-defined word 8 to conserve
> space.
> 
> But, keep the bit positions identical between the Intel-defined value
> and the Linux-defined value, e.g. keep SGX1 at bit 0.  This allows KVM
> to use its existing code for probing guest CPUID bits using Linux's
> X86_FEATURE_* definitions.  To do so, shift around some existing bits
> to effectively reserve bits 0-7 of word 8 for SGX sub-features.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  arch/x86/include/asm/cpufeatures.h       | 21 +++++++++++++++------
>  arch/x86/kernel/cpu/scattered.c          |  2 ++
>  tools/arch/x86/include/asm/cpufeatures.h | 21 +++++++++++++++------
>  3 files changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index da7fed4939a3..afdf5f2e13b5 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -222,12 +222,21 @@
>  #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...
> + */
> +#define X86_FEATURE_SGX1		( 8*32+ 0) /* SGX1 leaf functions */
> +#define X86_FEATURE_SGX2		( 8*32+ 1) /* SGX2 leaf functions */
> +

Yeah, add here	^^^^

/* Bits [0:7] are reserved for SGX */

or so, so that people don't use those. Once CPUID(12) gets more bits
added to it, I don't see anything wrong with allocating a separate leaf
for that.

BUT(!), the question then is whether kvm would still be ok with that?
I'm thinking yes, as it will simply use the new definitions, or?
Sean Christopherson Nov. 16, 2018, 3:38 p.m. UTC | #2 
On Fri, Nov 16, 2018 at 03:37:15PM +0100, Borislav Petkov wrote:
> On Fri, Nov 16, 2018 at 03:01:10AM +0200, Jarkko Sakkinen wrote:
> > From: Sean Christopherson <sean.j.christopherson@intel.com>
> > 
> > CPUID_12_EAX is an Intel-defined feature bits leaf dedicated for SGX
> > that enumerates the SGX instruction sets that are supported by the
> > CPU, e.g. SGX1, SGX2, etc...  Because Linux currently only cares about
> > two bits (SGX1 and SGX2) and there are currently only four documented
> > bits in total, relocate the bits to Linux-defined word 8 to conserve
> > space.
> > 
> > But, keep the bit positions identical between the Intel-defined value
> > and the Linux-defined value, e.g. keep SGX1 at bit 0.  This allows KVM
> > to use its existing code for probing guest CPUID bits using Linux's
> > X86_FEATURE_* definitions.  To do so, shift around some existing bits
> > to effectively reserve bits 0-7 of word 8 for SGX sub-features.
> > 
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >  arch/x86/include/asm/cpufeatures.h       | 21 +++++++++++++++------
> >  arch/x86/kernel/cpu/scattered.c          |  2 ++
> >  tools/arch/x86/include/asm/cpufeatures.h | 21 +++++++++++++++------
> >  3 files changed, 32 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> > index da7fed4939a3..afdf5f2e13b5 100644
> > --- a/arch/x86/include/asm/cpufeatures.h
> > +++ b/arch/x86/include/asm/cpufeatures.h
> > @@ -222,12 +222,21 @@
> >  #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...
> > + */
> > +#define X86_FEATURE_SGX1		( 8*32+ 0) /* SGX1 leaf functions */
> > +#define X86_FEATURE_SGX2		( 8*32+ 1) /* SGX2 leaf functions */
> > +
> 
> Yeah, add here	^^^^
> 
> /* Bits [0:7] are reserved for SGX */
> 
> or so, so that people don't use those. Once CPUID(12) gets more bits
> added to it, I don't see anything wrong with allocating a separate leaf
> for that.
> 
> BUT(!), the question then is whether kvm would still be ok with that?
> I'm thinking yes, as it will simply use the new definitions, or?

Yep, wouldn't be a problem for KVM.
Dave Hansen Nov. 16, 2018, 11:31 p.m. UTC | #3 
On 11/15/18 5:01 PM, Jarkko Sakkinen wrote:
> +#define X86_FEATURE_SGX1		( 8*32+ 0) /* SGX1 leaf functions */
> +#define X86_FEATURE_SGX2		( 8*32+ 1) /* SGX2 leaf functions */

Is there a reason these are not (all) tied to CONFIG_INTEL_SGX via:

arch/x86/include/asm/disabled-features.h

?
Jarkko Sakkinen Nov. 18, 2018, 8:36 a.m. UTC | #4 
On Fri, Nov 16, 2018 at 03:31:28PM -0800, Dave Hansen wrote:
> On 11/15/18 5:01 PM, Jarkko Sakkinen wrote:
> > +#define X86_FEATURE_SGX1		( 8*32+ 0) /* SGX1 leaf functions */
> > +#define X86_FEATURE_SGX2		( 8*32+ 1) /* SGX2 leaf functions */
> 
> Is there a reason these are not (all) tied to CONFIG_INTEL_SGX via:
> 
> arch/x86/include/asm/disabled-features.h
> 
> ?

No any particular reason.

/Jarkko
diff mbox series

Patch

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index da7fed4939a3..afdf5f2e13b5 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -222,12 +222,21 @@ 
 #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...
+ */
+#define X86_FEATURE_SGX1		( 8*32+ 0) /* SGX1 leaf functions */
+#define X86_FEATURE_SGX2		( 8*32+ 1) /* SGX2 leaf functions */
+
+#define X86_FEATURE_TPR_SHADOW		( 8*32+ 8) /* Intel TPR Shadow */
+#define X86_FEATURE_VNMI		( 8*32+ 9) /* Intel Virtual NMI */
+#define X86_FEATURE_FLEXPRIORITY	( 8*32+10) /* Intel FlexPriority */
+#define X86_FEATURE_EPT			( 8*32+11) /* Intel Extended Page Table */
+#define X86_FEATURE_VPID		( 8*32+12) /* Intel Virtual Processor ID */
 
 #define X86_FEATURE_VMMCALL		( 8*32+15) /* Prefer VMMCALL to VMCALL */
 #define X86_FEATURE_XENPV		( 8*32+16) /* "" Xen paravirtual guest */
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 772c219b6889..f7f0970b8f89 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -26,6 +26,8 @@  static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_CDP_L3,		CPUID_ECX,  2, 0x00000010, 1 },
 	{ X86_FEATURE_CDP_L2,		CPUID_ECX,  2, 0x00000010, 2 },
 	{ X86_FEATURE_MBA,		CPUID_EBX,  3, 0x00000010, 0 },
+	{ X86_FEATURE_SGX1,             CPUID_EAX,  0, 0x00000012, 0 },
+	{ X86_FEATURE_SGX2,             CPUID_EAX,  1, 0x00000012, 0 },
 	{ X86_FEATURE_HW_PSTATE,	CPUID_EDX,  7, 0x80000007, 0 },
 	{ X86_FEATURE_CPB,		CPUID_EDX,  9, 0x80000007, 0 },
 	{ X86_FEATURE_PROC_FEEDBACK,    CPUID_EDX, 11, 0x80000007, 0 },
diff --git a/tools/arch/x86/include/asm/cpufeatures.h b/tools/arch/x86/include/asm/cpufeatures.h
index 89a048c2faec..9cc7628b9845 100644
--- a/tools/arch/x86/include/asm/cpufeatures.h
+++ b/tools/arch/x86/include/asm/cpufeatures.h
@@ -222,12 +222,21 @@ 
 #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 numbers of the SGX features are 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...
+ */
+#define X86_FEATURE_SGX1		( 8*32+ 0) /* SGX1 leaf functions */
+#define X86_FEATURE_SGX2		( 8*32+ 1) /* SGX2 leaf functions */
+
+#define X86_FEATURE_TPR_SHADOW		( 8*32+ 8) /* Intel TPR Shadow */
+#define X86_FEATURE_VNMI		( 8*32+ 9) /* Intel Virtual NMI */
+#define X86_FEATURE_FLEXPRIORITY	( 8*32+10) /* Intel FlexPriority */
+#define X86_FEATURE_EPT			( 8*32+11) /* Intel Extended Page Table */
+#define X86_FEATURE_VPID		( 8*32+12) /* Intel Virtual Processor ID */
 
 #define X86_FEATURE_VMMCALL		( 8*32+15) /* Prefer VMMCALL to VMCALL */
 #define X86_FEATURE_XENPV		( 8*32+16) /* "" Xen paravirtual guest */