diff mbox series

[RFC,v6,13/25] x86/sgx: Add helpers to expose ECREATE and EINIT to KVM

Message ID 14908104a7ff5724b6fb4e4c7df6e675adafe5a7.1614338774.git.kai.huang@intel.com (mailing list archive)
State New, archived
Headers show
Series KVM SGX virtualization support | expand

Commit Message

Huang, Kai Feb. 26, 2021, 12:15 p.m. UTC 
From: Sean Christopherson <sean.j.christopherson@intel.com>

The host kernel must intercept ECREATE to be able to impose policies on
guests.  When it does this, the host kernel runs ECREATE against the
userspace mapping of the virtualized EPC.

Provide wrappers around __ecreate() and __einit() to hide the ugliness
of overloading the ENCLS return value to encode multiple error formats
in a single int.  KVM will trap-and-execute ECREATE and EINIT as part
of SGX virtualization, and on an exception, KVM needs the trapnr so that
it can inject the correct fault into the guest.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
---
v5->v6:

 - Code change due to sgx_arch.h merged to sgx.h.
 - Added a new empty line before return in __sgx_virt_einit() per Jarkko.

v4->v5:

 - No code change.

v3->v4:

 - Added one new line before last return in sgx_virt_einit(), per Jarkko.

v2->v3:

 - Added kdoc for sgx_virt_ecreate() and sgx_virt_einit(), per Jarkko.
 - Changed to use CONFIG_X86_SGX_KVM.

---
 arch/x86/include/asm/sgx.h     |  7 +++
 arch/x86/kernel/cpu/sgx/virt.c | 95 ++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)

Comments

Dave Hansen March 5, 2021, 5:51 p.m. UTC | #1 
On 2/26/21 4:15 AM, Kai Huang wrote:
> +int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs,
> +		     int *trapnr)
> +{
> +	int ret;
> +
> +	/*
> +	 * @secs is userspace address, and it's not guaranteed @secs points at
> +	 * an actual EPC page. 

There are four cases that I can think of:
1. @secs points to an EPC page.  Good, return 0 and go on with life.
2. @secs points to a non-EPC page.  It will fault and permanently error
   out
3. @secs points to a Present=0 PTE.  It will fault, but we need to call
   the fault handler to get a page in here.
4. @secs points to a kernel address

#1 and #2 are handled and described.

#4 is probably impossible because the address comes out of some
gpa_to_hva() KVM code.  But, it still _looks_ wonky here.  I wouldn't
hate an access_ok() check on it.

	/*
	 * @secs is an untrusted, userspace-provided address.  It comes
         * from KVM and is assumed to point somewhere in userspace.
 	 * This can fault and call SGX or other fault handlers.
	 */

You can also spend a moment to describe the kinds of faults that are
handled and what is fatal.


> +	 * to physical EPC page by resolving PFN but using __uaccess_xx() is
> +	 * simpler.
> +	 */

I'd leave the justification for the changelog.

> +	__uaccess_begin();
> +	ret = __ecreate(pageinfo, (void *)secs);
> +	__uaccess_end();
> +
> +	if (encls_faulted(ret)) {
> +		*trapnr = ENCLS_TRAPNR(ret);
> +		return -EFAULT;
> +	}
Huang, Kai March 8, 2021, 9:30 a.m. UTC | #2 
On Fri, 5 Mar 2021 09:51:56 -0800 Dave Hansen wrote:
> On 2/26/21 4:15 AM, Kai Huang wrote:
> > +int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs,
> > +		     int *trapnr)
> > +{
> > +	int ret;
> > +
> > +	/*
> > +	 * @secs is userspace address, and it's not guaranteed @secs points at
> > +	 * an actual EPC page. 
> 
> There are four cases that I can think of:
> 1. @secs points to an EPC page.  Good, return 0 and go on with life.
> 2. @secs points to a non-EPC page.  It will fault and permanently error
>    out
> 3. @secs points to a Present=0 PTE.  It will fault, but we need to call
>    the fault handler to get a page in here.
> 4. @secs points to a kernel address
> 
> #1 and #2 are handled and described.
> 
> #4 is probably impossible because the address comes out of some
> gpa_to_hva() KVM code.  But, it still _looks_ wonky here.  I wouldn't
> hate an access_ok() check on it.
> 
> 	/*
> 	 * @secs is an untrusted, userspace-provided address.  It comes
>          * from KVM and is assumed to point somewhere in userspace.
>  	 * This can fault and call SGX or other fault handlers.
> 	 */
> 
> You can also spend a moment to describe the kinds of faults that are
> handled and what is fatal.

Thanks Dave for the comments. I'll refine accordingly.

> 
> 
> > +	 * to physical EPC page by resolving PFN but using __uaccess_xx() is
> > +	 * simpler.
> > +	 */
> 
> I'd leave the justification for the changelog.

Will do.

> 
> > +	__uaccess_begin();
> > +	ret = __ecreate(pageinfo, (void *)secs);
> > +	__uaccess_end();
> > +
> > +	if (encls_faulted(ret)) {
> > +		*trapnr = ENCLS_TRAPNR(ret);
> > +		return -EFAULT;
> > +	}
>
diff mbox series

Patch

diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
index 0db1e47a90c5..d2e1f9a6dd4d 100644
--- a/arch/x86/include/asm/sgx.h
+++ b/arch/x86/include/asm/sgx.h
@@ -365,4 +365,11 @@  struct sgx_sigstruct {
  * line!
  */
 
+#ifdef CONFIG_X86_SGX_KVM
+int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs,
+		     int *trapnr);
+int sgx_virt_einit(void __user *sigstruct, void __user *token,
+		   void __user *secs, u64 *lepubkeyhash, int *trapnr);
+#endif
+
 #endif /* _ASM_X86_SGX_H */
diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c
index d206d81280cf..2ffa4ecb92c7 100644
--- a/arch/x86/kernel/cpu/sgx/virt.c
+++ b/arch/x86/kernel/cpu/sgx/virt.c
@@ -258,3 +258,98 @@  int __init sgx_vepc_init(void)
 
 	return misc_register(&sgx_vepc_dev);
 }
+
+/**
+ * sgx_virt_ecreate() - Run ECREATE on behalf of guest
+ * @pageinfo:	Pointer to PAGEINFO structure
+ * @secs:	Userspace pointer to SECS page
+ * @trapnr:	trap number injected to guest in case of ECREATE error
+ *
+ * Run ECREATE on behalf of guest after KVM traps ECREATE for the purpose
+ * of enforcing policies of guest's enclaves, and return the trap number
+ * which should be injected to guest in case of any ECREATE error.
+ *
+ * Return:
+ * - 0: 	ECREATE was successful.
+ * - -EFAULT:	ECREATE returned error.
+ */
+int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs,
+		     int *trapnr)
+{
+	int ret;
+
+	/*
+	 * @secs is userspace address, and it's not guaranteed @secs points at
+	 * an actual EPC page. It's also possible to generate a kernel mapping
+	 * to physical EPC page by resolving PFN but using __uaccess_xx() is
+	 * simpler.
+	 */
+	__uaccess_begin();
+	ret = __ecreate(pageinfo, (void *)secs);
+	__uaccess_end();
+
+	if (encls_faulted(ret)) {
+		*trapnr = ENCLS_TRAPNR(ret);
+		return -EFAULT;
+	}
+
+	/* ECREATE doesn't return an error code, it faults or succeeds. */
+	WARN_ON_ONCE(ret);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(sgx_virt_ecreate);
+
+static int __sgx_virt_einit(void __user *sigstruct, void __user *token,
+			    void __user *secs)
+{
+	int ret;
+
+	__uaccess_begin();
+	ret =  __einit((void *)sigstruct, (void *)token, (void *)secs);
+	__uaccess_end();
+
+	return ret;
+}
+
+/**
+ * sgx_virt_einit() - Run EINIT on behalf of guest
+ * @sigstruct:		Userspace pointer to SIGSTRUCT structure
+ * @token:		Userspace pointer to EINITTOKEN structure
+ * @secs:		Userspace pointer to SECS page
+ * @lepubkeyhash:	Pointer to guest's *virtual* SGX_LEPUBKEYHASH MSR
+ * 			values
+ * @trapnr:		trap number injected to guest in case of EINIT error
+ *
+ * Run EINIT on behalf of guest after KVM traps EINIT. If SGX_LC is available
+ * in host, SGX driver may rewrite the hardware values at wish, therefore KVM
+ * needs to update hardware values to guest's virtual MSR values in order to
+ * ensure EINIT is executed with expected hardware values.
+ *
+ * Return:
+ * - 0: 	EINIT was successful.
+ * - -EFAULT:	EINIT returned error.
+ */
+int sgx_virt_einit(void __user *sigstruct, void __user *token,
+		   void __user *secs, u64 *lepubkeyhash, int *trapnr)
+{
+	int ret;
+
+	if (!boot_cpu_has(X86_FEATURE_SGX_LC)) {
+		ret = __sgx_virt_einit(sigstruct, token, secs);
+	} else {
+		preempt_disable();
+
+		sgx_update_lepubkeyhash(lepubkeyhash);
+
+		ret = __sgx_virt_einit(sigstruct, token, secs);
+		preempt_enable();
+	}
+
+	if (encls_faulted(ret)) {
+		*trapnr = ENCLS_TRAPNR(ret);
+		return -EFAULT;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(sgx_virt_einit);