[PATCH 1/2] x86/sgx: cleanup: Remove unused struct

[PATCH 1/2] x86/sgx: cleanup: Remove unused struct

[Jarkko Sakkinen]

Remove struct sgx_einittoken as it is no longer required. Only size
of the microarchitectural structure is needed by the kernel.

Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 arch/x86/kernel/cpu/sgx/arch.h  | 54 +--------------------------------
 arch/x86/kernel/cpu/sgx/encls.h |  5 ++-
 arch/x86/kernel/cpu/sgx/ioctl.c | 14 ++++-----
 3 files changed, 9 insertions(+), 64 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/arch.h b/arch/x86/kernel/cpu/sgx/arch.h
index 98836ab3eeaf..ddae55e9d4d8 100644
--- a/arch/x86/kernel/cpu/sgx/arch.h
+++ b/arch/x86/kernel/cpu/sgx/arch.h
@@ -338,58 +338,6 @@ struct sgx_sigstruct {
 	u8  q2[SGX_MODULUS_SIZE];
 } __packed;
 
-#define SGX_EINITTOKEN_RESERVED1_SIZE 11
-#define SGX_EINITTOKEN_RESERVED2_SIZE 32
-#define SGX_EINITTOKEN_RESERVED3_SIZE 32
-#define SGX_EINITTOKEN_RESERVED4_SIZE 24
-
-/**
- * struct sgx_einittoken - a token permitting to launch an enclave
- * @valid:			one if valid and zero if invalid
- * @attributes:			attributes for enclave
- * @xfrm:			XSave-Feature Request Mask (subset of XCR0)
- * @mrenclave:			SHA256-hash of the enclave contents
- * @mrsigner:			SHA256-hash of the public key used to sign the
- *				SIGSTRUCT
- * @le_cpusvn:			a value that reflects the SGX implementation
- *				running in in the CPU
- * @le_isvprodid:		a user-defined value that is used in key
- *				derivation
- * @le_isvsvn:			a user-defined value that is used in key
- *				derivation
- * @le_keyed_miscselect:	LE's miscselect masked with the token keys
- *				miscselect
- * @le_keyed_attributes:	LE's attributes masked with the token keys
- *				attributes
- * @le_keyed_xfrm:		LE's XFRM masked with the token keys xfrm
- * @salt:			random salt for wear-out protection
- * @mac:			CMAC over the preceding fields
- *
- * An enclave with EINITTOKENKEY attribute can access a key with the same name
- * by using ENCLS(EGETKEY) and use this to sign cryptographic tokens that can
- * be passed to ENCLS(EINIT) to permit the launch of other enclaves. This is
- * the only viable way to launch enclaves if IA32_SGXLEPUBKEYHASHn MSRs are
- * locked assuming that there is a Launch Enclave (LE) available that can be
- * used for generating these tokens.
- */
-struct sgx_einittoken {
-	u32 valid;
-	u32 reserved1[SGX_EINITTOKEN_RESERVED1_SIZE];
-	u64 attributes;
-	u64 xfrm;
-	u8  mrenclave[32];
-	u8  reserved2[SGX_EINITTOKEN_RESERVED2_SIZE];
-	u8  mrsigner[32];
-	u8  reserved3[SGX_EINITTOKEN_RESERVED3_SIZE];
-	u8  le_cpusvn[16];
-	u16 le_isvprodid;
-	u16 le_isvsvn;
-	u8  reserved4[SGX_EINITTOKEN_RESERVED4_SIZE];
-	u32 le_keyed_miscselect;
-	u64 le_keyed_attributes;
-	u64 le_keyed_xfrm;
-	u8  salt[32];
-	u8  mac[16];
-} __packed __aligned(512);
+#define SGX_LAUNCH_TOKEN_SIZE 304
 
 #endif /* _ASM_X86_SGX_ARCH_H */
diff --git a/arch/x86/kernel/cpu/sgx/encls.h b/arch/x86/kernel/cpu/sgx/encls.h
index 30132edddcd6..b4c27d8bd2ac 100644
--- a/arch/x86/kernel/cpu/sgx/encls.h
+++ b/arch/x86/kernel/cpu/sgx/encls.h
@@ -186,10 +186,9 @@ static inline int __eadd(struct sgx_pageinfo *pginfo, void *addr)
 	return __encls_2(EADD, pginfo, addr);
 }
 
-static inline int __einit(void *sigstruct, struct sgx_einittoken *einittoken,
-			  void *secs)
+static inline int __einit(void *sigstruct, void *token, void *secs)
 {
-	return __encls_ret_3(EINIT, sigstruct, secs, einittoken);
+	return __encls_ret_3(EINIT, sigstruct, secs, token);
 }
 
 static inline int __eremove(void *addr)
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 2880ed4745b6..eaae533578b9 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -595,8 +595,7 @@ static void sgx_update_lepubkeyhash_msrs(u64 *lepubkeyhash, bool enforce)
 	}
 }
 
-static int sgx_einit(struct sgx_sigstruct *sigstruct,
-		     struct sgx_einittoken *token,
+static int sgx_einit(struct sgx_sigstruct *sigstruct, void *token,
 		     struct sgx_epc_page *secs, u64 *lepubkeyhash)
 {
 	int ret;
@@ -616,7 +615,7 @@ static int sgx_einit(struct sgx_sigstruct *sigstruct,
 }
 
 static int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
-			 struct sgx_einittoken *token)
+			 void *token)
 {
 	u64 mrsigner[4];
 	int ret;
@@ -694,10 +693,10 @@ static int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
  */
 static long sgx_ioc_enclave_init(struct sgx_encl *encl, void __user *arg)
 {
-	struct sgx_einittoken *einittoken;
 	struct sgx_sigstruct *sigstruct;
 	struct sgx_enclave_init einit;
 	struct page *initp_page;
+	void *token;
 	int ret;
 
 	if (!(atomic_read(&encl->flags) & SGX_ENCL_CREATED))
@@ -711,9 +710,8 @@ static long sgx_ioc_enclave_init(struct sgx_encl *encl, void __user *arg)
 		return -ENOMEM;
 
 	sigstruct = kmap(initp_page);
-	einittoken = (struct sgx_einittoken *)
-		((unsigned long)sigstruct + PAGE_SIZE / 2);
-	memset(einittoken, 0, sizeof(*einittoken));
+	token = (void *)((unsigned long)sigstruct + PAGE_SIZE / 2);
+	memset(token, 0, SGX_LAUNCH_TOKEN_SIZE);
 
 	if (copy_from_user(sigstruct, (void __user *)einit.sigstruct,
 			   sizeof(*sigstruct))) {
@@ -721,7 +719,7 @@ static long sgx_ioc_enclave_init(struct sgx_encl *encl, void __user *arg)
 		goto out;
 	}
 
-	ret = sgx_encl_init(encl, sigstruct, einittoken);
+	ret = sgx_encl_init(encl, sigstruct, token);
 
 out:
 	kunmap(initp_page);
-- 
2.25.0

[PATCH 2/2] x86/sgx: Remove non-LC bit from sgx_einit()

[Jarkko Sakkinen]

Remove the non-LC flow. While doing this, start using __encls_ret_3()
instead __einit() to reduce the insane amount of wrapping we have in
this flow.

Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 arch/x86/kernel/cpu/sgx/encls.h |  5 -----
 arch/x86/kernel/cpu/sgx/ioctl.c | 12 +++++++-----
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/encls.h b/arch/x86/kernel/cpu/sgx/encls.h
index b4c27d8bd2ac..fb201e384bc7 100644
--- a/arch/x86/kernel/cpu/sgx/encls.h
+++ b/arch/x86/kernel/cpu/sgx/encls.h
@@ -186,11 +186,6 @@ static inline int __eadd(struct sgx_pageinfo *pginfo, void *addr)
 	return __encls_2(EADD, pginfo, addr);
 }
 
-static inline int __einit(void *sigstruct, void *token, void *secs)
-{
-	return __encls_ret_3(EINIT, sigstruct, secs, token);
-}
-
 static inline int __eremove(void *addr)
 {
 	return __encls_ret_1(EREMOVE, addr);
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index eaae533578b9..6e9e6a7380b1 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -600,17 +600,19 @@ static int sgx_einit(struct sgx_sigstruct *sigstruct, void *token,
 {
 	int ret;
 
-	if (!boot_cpu_has(X86_FEATURE_SGX_LC))
-		return __einit(sigstruct, token, sgx_epc_addr(secs));
-
 	preempt_disable();
+
 	sgx_update_lepubkeyhash_msrs(lepubkeyhash, false);
-	ret = __einit(sigstruct, token, sgx_epc_addr(secs));
+	ret = __encls_ret_3(EINIT, sigstruct, sgx_epc_addr(secs), token);
+
 	if (ret == SGX_INVALID_EINITTOKEN) {
 		sgx_update_lepubkeyhash_msrs(lepubkeyhash, true);
-		ret = __einit(sigstruct, token, sgx_epc_addr(secs));
+		ret = __encls_ret_3(EINIT, sigstruct, sgx_epc_addr(secs),
+				    token);
 	}
+
 	preempt_enable();
+
 	return ret;
 }
 
-- 
2.25.0

Re: [PATCH 2/2] x86/sgx: Remove non-LC bit from sgx_einit()

[Sean Christopherson]

On Tue, Mar 03, 2020 at 10:53:59PM +0200, Jarkko Sakkinen wrote:
> Remove the non-LC flow. While doing this, start using __encls_ret_3()
> instead __einit() to reduce the insane amount of wrapping we have in
> this flow.

I strongly prefer keeping __einit(), I find it much more readable than
__encls_ret_3(EINIT, ...).  If you remove __einit() then I don't see the
justification for keeping all the other wrappers, and for me, removing
all of those would be a big step backwards.

> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/sgx/encls.h |  5 -----
>  arch/x86/kernel/cpu/sgx/ioctl.c | 12 +++++++-----
>  2 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/sgx/encls.h b/arch/x86/kernel/cpu/sgx/encls.h
> index b4c27d8bd2ac..fb201e384bc7 100644
> --- a/arch/x86/kernel/cpu/sgx/encls.h
> +++ b/arch/x86/kernel/cpu/sgx/encls.h
> @@ -186,11 +186,6 @@ static inline int __eadd(struct sgx_pageinfo *pginfo, void *addr)
>  	return __encls_2(EADD, pginfo, addr);
>  }
>  
> -static inline int __einit(void *sigstruct, void *token, void *secs)
> -{
> -	return __encls_ret_3(EINIT, sigstruct, secs, token);
> -}
> -
>  static inline int __eremove(void *addr)
>  {
>  	return __encls_ret_1(EREMOVE, addr);
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
> index eaae533578b9..6e9e6a7380b1 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -600,17 +600,19 @@ static int sgx_einit(struct sgx_sigstruct *sigstruct, void *token,
>  {
>  	int ret;
>  
> -	if (!boot_cpu_has(X86_FEATURE_SGX_LC))
> -		return __einit(sigstruct, token, sgx_epc_addr(secs));
> -
>  	preempt_disable();
> +
>  	sgx_update_lepubkeyhash_msrs(lepubkeyhash, false);
> -	ret = __einit(sigstruct, token, sgx_epc_addr(secs));
> +	ret = __encls_ret_3(EINIT, sigstruct, sgx_epc_addr(secs), token);
> +
>  	if (ret == SGX_INVALID_EINITTOKEN) {
>  		sgx_update_lepubkeyhash_msrs(lepubkeyhash, true);
> -		ret = __einit(sigstruct, token, sgx_epc_addr(secs));
> +		ret = __encls_ret_3(EINIT, sigstruct, sgx_epc_addr(secs),
> +				    token);
>  	}
> +
>  	preempt_enable();
> +
>  	return ret;
>  }
>  
> -- 
> 2.25.0
> 

Re: [PATCH 2/2] x86/sgx: Remove non-LC bit from sgx_einit()

[Jarkko Sakkinen]

On Tue, Mar 03, 2020 at 02:33:46PM -0800, Sean Christopherson wrote:
> On Tue, Mar 03, 2020 at 10:53:59PM +0200, Jarkko Sakkinen wrote:
> > Remove the non-LC flow. While doing this, start using __encls_ret_3()
> > instead __einit() to reduce the insane amount of wrapping we have in
> > this flow.
> 
> I strongly prefer keeping __einit(), I find it much more readable than
> __encls_ret_3(EINIT, ...).  If you remove __einit() then I don't see the
> justification for keeping all the other wrappers, and for me, removing
> all of those would be a big step backwards.

Rather than arguing about this I'll merge only the part that removes
the LC flow.

/Jarkko