Re: [PATCH v33 02/21] x86/cpufeatures: x86/msr: Add Intel SGX Launch Control hardware bits
On Wed, Jun 24, 2020 at 07:34:40AM -0700, Sean Christopherson wrote:
> On Wed, Jun 24, 2020 at 03:04:34PM +0200, Borislav Petkov wrote:
> > On Thu, Jun 18, 2020 at 01:08:24AM +0300, Jarkko Sakkinen wrote:
> > > From: Sean Christopherson
> > >
> > > Add X86_FEATURE_SGX_LC, which informs whether or not the CPU supports SGX
> > > Launch Control.
> > >
> > > Add MSR_IA32_SGXLEPUBKEYHASH{0, 1, 2, 3}, which when combined contain a
> > > SHA256 hash of a 3072-bit RSA public key. SGX backed software packages, so
> > > called enclaves, are always signed. All enclaves signed with the public
> > > key
> > > are unconditionally allowed to initialize. [1]
> > >
> > > Add FEATURE_CONTROL_SGX_LE_WR bit of the feature control MSR, which
> > > informs
> >
> > LE_WR or LC_ENABLED?
>
> It should be FEAT_CTL_SGX_LC_ENABLED, i.e. the actual code is correct. We
> updated the #define to use the SDM name to be consistent with the other bits
> and neglected to update the changelog.
>
> Thanks!
>
> > With that addressed:
> >
> > Reviewed-by: Borislav Petkov
I'll update the commit according to this information.
> >
> > --
> > Regards/Gruss,
> > Boris.
> >
> > https://people.kernel.org/tglx/notes-about-netiquette
Thank you.
/Jarkko
Re: [PATCH v33 02/21] x86/cpufeatures: x86/msr: Add Intel SGX Launch Control hardware bits
On Wed, Jun 24, 2020 at 03:04:34PM +0200, Borislav Petkov wrote:
> On Thu, Jun 18, 2020 at 01:08:24AM +0300, Jarkko Sakkinen wrote:
> > From: Sean Christopherson
> >
> > Add X86_FEATURE_SGX_LC, which informs whether or not the CPU supports SGX
> > Launch Control.
> >
> > Add MSR_IA32_SGXLEPUBKEYHASH{0, 1, 2, 3}, which when combined contain a
> > SHA256 hash of a 3072-bit RSA public key. SGX backed software packages, so
> > called enclaves, are always signed. All enclaves signed with the public key
> > are unconditionally allowed to initialize. [1]
> >
> > Add FEATURE_CONTROL_SGX_LE_WR bit of the feature control MSR, which informs
>
> LE_WR or LC_ENABLED?
It should be FEAT_CTL_SGX_LC_ENABLED, i.e. the actual code is correct. We
updated the #define to use the SDM name to be consistent with the other bits
and neglected to update the changelog.
Thanks!
> With that addressed:
>
> Reviewed-by: Borislav Petkov
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v33 02/21] x86/cpufeatures: x86/msr: Add Intel SGX Launch Control hardware bits
On Thu, Jun 18, 2020 at 01:08:24AM +0300, Jarkko Sakkinen wrote:
> From: Sean Christopherson
>
> Add X86_FEATURE_SGX_LC, which informs whether or not the CPU supports SGX
> Launch Control.
>
> Add MSR_IA32_SGXLEPUBKEYHASH{0, 1, 2, 3}, which when combined contain a
> SHA256 hash of a 3072-bit RSA public key. SGX backed software packages, so
> called enclaves, are always signed. All enclaves signed with the public key
> are unconditionally allowed to initialize. [1]
>
> Add FEATURE_CONTROL_SGX_LE_WR bit of the feature control MSR, which informs
LE_WR or LC_ENABLED?
With that addressed:
Reviewed-by: Borislav Petkov
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
[PATCH v33 02/21] x86/cpufeatures: x86/msr: Add Intel SGX Launch Control hardware bits
From: Sean Christopherson
Add X86_FEATURE_SGX_LC, which informs whether or not the CPU supports SGX
Launch Control.
Add MSR_IA32_SGXLEPUBKEYHASH{0, 1, 2, 3}, which when combined contain a
SHA256 hash of a 3072-bit RSA public key. SGX backed software packages, so
called enclaves, are always signed. All enclaves signed with the public key
are unconditionally allowed to initialize. [1]
Add FEATURE_CONTROL_SGX_LE_WR bit of the feature control MSR, which informs
whether the aformentioned MSRs are writable or not. If the bit is off, the
public key MSRs are read-only for the OS.
If the MSRs are read-only, the platform must provide a launch enclave (LE).
LE can create cryptographic tokens for other enclaves that they can pass
together with their signature to the ENCLS(EINIT) opcode, which is used
to initialize enclaves.
Linux is unlikely to support the locked configuration because it takes away
the control of the launch decisions from the kernel.
[1] Intel SDM: 38.1.4 Intel SGX Launch Control Configuration
Acked-by: Jethro Beekman
Signed-off-by: Sean Christopherson
Co-developed-by: Jarkko Sakkinen
Signed-off-by: Jarkko Sakkinen
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/msr-index.h | 7 +++
2 files changed, 8 insertions(+)
diff --git a/arch/x86/include/asm/cpufeatures.h
b/arch/x86/include/asm/cpufeatures.h
index 545ac3e0e269..0a4541e4f076 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -352,6 +352,7 @@
#define X86_FEATURE_CLDEMOTE (16*32+25) /* CLDEMOTE instruction */
#define X86_FEATURE_MOVDIRI(16*32+27) /* MOVDIRI instruction */
#define X86_FEATURE_MOVDIR64B (16*32+28) /* MOVDIR64B instruction */
+#define X86_FEATURE_SGX_LC (16*32+30) /* Software Guard Extensions
Launch Control */
/* AMD-defined CPU features, CPUID level 0x8007 (EBX), word 17 */
#define X86_FEATURE_OVERFLOW_RECOV (17*32+ 0) /* MCA overflow recovery
support */
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 18e08da19f16..3d7c89a8533f 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -582,6 +582,7 @@
#define FEAT_CTL_LOCKEDBIT(0)
#define FEAT_CTL_VMX_ENABLED_INSIDE_SMXBIT(1)
#define FEAT_CTL_VMX_ENABLED_OUTSIDE_SMX BIT(2)
+#define FEAT_CTL_SGX_LC_ENABLEDBIT(17)
#define FEAT_CTL_SGX_ENABLED BIT(18)
#define FEAT_CTL_LMCE_ENABLED BIT(20)
@@ -602,6 +603,12 @@
#define MSR_IA32_UCODE_WRITE 0x0079
#define MSR_IA32_UCODE_REV 0x008b
+/* Intel SGX Launch Enclave Public Key Hash MSRs */
+#define MSR_IA32_SGXLEPUBKEYHASH0 0x008C
+#define MSR_IA32_SGXLEPUBKEYHASH1 0x008D
+#define MSR_IA32_SGXLEPUBKEYHASH2 0x008E
+#define MSR_IA32_SGXLEPUBKEYHASH3 0x008F
+
#define MSR_IA32_SMM_MONITOR_CTL 0x009b
#define MSR_IA32_SMBASE0x009e
--
2.25.1
