diff mbox series

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

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

Commit Message

Huang, Kai Feb. 13, 2021, 1:29 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>
---
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     | 16 ++++++
 arch/x86/kernel/cpu/sgx/virt.c | 94 ++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+)
 create mode 100644 arch/x86/include/asm/sgx.h

Comments

Jarkko Sakkinen Feb. 16, 2021, 3:08 a.m. UTC | #1
On Sun, Feb 14, 2021 at 02:29:15AM +1300, Kai Huang wrote:
> 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>
> ---
> 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     | 16 ++++++
>  arch/x86/kernel/cpu/sgx/virt.c | 94 ++++++++++++++++++++++++++++++++++
>  2 files changed, 110 insertions(+)
>  create mode 100644 arch/x86/include/asm/sgx.h
> 
> diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> new file mode 100644
> index 000000000000..8a3ea3e1efbe
> --- /dev/null
> +++ b/arch/x86/include/asm/sgx.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_SGX_H
> +#define _ASM_X86_SGX_H
> +
> +#include <linux/types.h>
> +
> +#ifdef CONFIG_X86_SGX_KVM
> +struct sgx_pageinfo;
> +
> +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);

s/virt/vepc/g

> +#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 47542140f8c1..016bad7cff8d 100644
> --- a/arch/x86/kernel/cpu/sgx/virt.c
> +++ b/arch/x86/kernel/cpu/sgx/virt.c

Rename as vepc.c.

> @@ -257,3 +257,97 @@ 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);

Empty line.

> +	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();

Ditto.

> +	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);
> -- 
> 2.29.2
> 
> 

/Jarkko
Jarkko Sakkinen Feb. 16, 2021, 3:09 a.m. UTC | #2
On Tue, Feb 16, 2021 at 05:08:20AM +0200, Jarkko Sakkinen wrote:
> On Sun, Feb 14, 2021 at 02:29:15AM +1300, Kai Huang wrote:
> > 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>
> > ---
> > 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     | 16 ++++++
> >  arch/x86/kernel/cpu/sgx/virt.c | 94 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 110 insertions(+)
> >  create mode 100644 arch/x86/include/asm/sgx.h
> > 
> > diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> > new file mode 100644
> > index 000000000000..8a3ea3e1efbe
> > --- /dev/null
> > +++ b/arch/x86/include/asm/sgx.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _ASM_X86_SGX_H
> > +#define _ASM_X86_SGX_H
> > +
> > +#include <linux/types.h>
> > +
> > +#ifdef CONFIG_X86_SGX_KVM
> > +struct sgx_pageinfo;
> > +
> > +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);
> 
> s/virt/vepc/g
> 
> > +#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 47542140f8c1..016bad7cff8d 100644
> > --- a/arch/x86/kernel/cpu/sgx/virt.c
> > +++ b/arch/x86/kernel/cpu/sgx/virt.c
> 
> Rename as vepc.c.
> 
> > @@ -257,3 +257,97 @@ 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);
> 
> Empty line.
> 
> > +	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();
> 
> Ditto.
> 
> > +	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);

Remove exports.

> > -- 
> > 2.29.2
> > 
> > 
> 
> /Jarkko
Huang, Kai Feb. 16, 2021, 4:55 a.m. UTC | #3
> 
> On Tue, Feb 16, 2021 at 05:08:20AM +0200, Jarkko Sakkinen wrote:
> > On Sun, Feb 14, 2021 at 02:29:15AM +1300, Kai Huang wrote:
> > > 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>
> > > ---
> > > 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     | 16 ++++++
> > >  arch/x86/kernel/cpu/sgx/virt.c | 94
> > > ++++++++++++++++++++++++++++++++++
> > >  2 files changed, 110 insertions(+)
> > >  create mode 100644 arch/x86/include/asm/sgx.h
> > >
> > > diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> > > new file mode 100644 index 000000000000..8a3ea3e1efbe
> > > --- /dev/null
> > > +++ b/arch/x86/include/asm/sgx.h
> > > @@ -0,0 +1,16 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */ #ifndef _ASM_X86_SGX_H
> > > +#define _ASM_X86_SGX_H
> > > +
> > > +#include <linux/types.h>
> > > +
> > > +#ifdef CONFIG_X86_SGX_KVM
> > > +struct sgx_pageinfo;
> > > +
> > > +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);
> >
> > s/virt/vepc/g

No. The two are related to enclave construction (from guest), not EPC. 

For instance, what does ECREATE mean for virtual EPC? ECREATE is meaningful for enclave.

> >
> > > +#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 47542140f8c1..016bad7cff8d
> > > 100644
> > > --- a/arch/x86/kernel/cpu/sgx/virt.c
> > > +++ b/arch/x86/kernel/cpu/sgx/virt.c
> >
> > Rename as vepc.c.

No. This file contains more than virtual EPC implementation, but also other staff like sgx_virt_ecreate()/sgx_virt_einit(), which are used by KVM to run ECREATE/EINIT on behalf of guest.

> >
> > > @@ -257,3 +257,97 @@ 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);
> >
> > Empty line.
> >
> > > +	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();
> >
> > Ditto.
> >
> > > +	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);
> 
> Remove exports.

Why? KVM needs to use them in later patches.
Jarkko Sakkinen Feb. 16, 2021, 8:33 a.m. UTC | #4
On Tue, Feb 16, 2021 at 04:55:49AM +0000, Huang, Kai wrote:
> > 
> > On Tue, Feb 16, 2021 at 05:08:20AM +0200, Jarkko Sakkinen wrote:
> > > On Sun, Feb 14, 2021 at 02:29:15AM +1300, Kai Huang wrote:
> > > > 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>
> > > > ---
> > > > 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     | 16 ++++++
> > > >  arch/x86/kernel/cpu/sgx/virt.c | 94
> > > > ++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 110 insertions(+)
> > > >  create mode 100644 arch/x86/include/asm/sgx.h
> > > >
> > > > diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> > > > new file mode 100644 index 000000000000..8a3ea3e1efbe
> > > > --- /dev/null
> > > > +++ b/arch/x86/include/asm/sgx.h
> > > > @@ -0,0 +1,16 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */ #ifndef _ASM_X86_SGX_H
> > > > +#define _ASM_X86_SGX_H
> > > > +
> > > > +#include <linux/types.h>
> > > > +
> > > > +#ifdef CONFIG_X86_SGX_KVM
> > > > +struct sgx_pageinfo;
> > > > +
> > > > +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);
> > >
> > > s/virt/vepc/g
> 
> No. The two are related to enclave construction (from guest), not EPC. 
> 
> For instance, what does ECREATE mean for virtual EPC? ECREATE is meaningful for enclave.
> 
> > >
> > > > +#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 47542140f8c1..016bad7cff8d
> > > > 100644
> > > > --- a/arch/x86/kernel/cpu/sgx/virt.c
> > > > +++ b/arch/x86/kernel/cpu/sgx/virt.c
> > >
> > > Rename as vepc.c.
> 
> No. This file contains more than virtual EPC implementation, but also other staff like sgx_virt_ecreate()/sgx_virt_einit(), which are used by KVM to run ECREATE/EINIT on behalf of guest.
> 
> > >
> > > > @@ -257,3 +257,97 @@ 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);
> > >
> > > Empty line.
> > >
> > > > +	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();
> > >
> > > Ditto.
> > >
> > > > +	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);
> > 
> > Remove exports.
> 
> Why? KVM needs to use them in later patches.

Because they are only required for LKM's.

/Jarkko
Jarkko Sakkinen Feb. 16, 2021, 8:35 a.m. UTC | #5
On Tue, Feb 16, 2021 at 10:33:17AM +0200, Jarkko Sakkinen wrote:
> On Tue, Feb 16, 2021 at 04:55:49AM +0000, Huang, Kai wrote:
> > > 
> > > On Tue, Feb 16, 2021 at 05:08:20AM +0200, Jarkko Sakkinen wrote:
> > > > On Sun, Feb 14, 2021 at 02:29:15AM +1300, Kai Huang wrote:
> > > > > 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>
> > > > > ---
> > > > > 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     | 16 ++++++
> > > > >  arch/x86/kernel/cpu/sgx/virt.c | 94
> > > > > ++++++++++++++++++++++++++++++++++
> > > > >  2 files changed, 110 insertions(+)
> > > > >  create mode 100644 arch/x86/include/asm/sgx.h
> > > > >
> > > > > diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> > > > > new file mode 100644 index 000000000000..8a3ea3e1efbe
> > > > > --- /dev/null
> > > > > +++ b/arch/x86/include/asm/sgx.h
> > > > > @@ -0,0 +1,16 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0 */ #ifndef _ASM_X86_SGX_H
> > > > > +#define _ASM_X86_SGX_H
> > > > > +
> > > > > +#include <linux/types.h>
> > > > > +
> > > > > +#ifdef CONFIG_X86_SGX_KVM
> > > > > +struct sgx_pageinfo;
> > > > > +
> > > > > +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);
> > > >
> > > > s/virt/vepc/g
> > 
> > No. The two are related to enclave construction (from guest), not EPC. 
> > 
> > For instance, what does ECREATE mean for virtual EPC? ECREATE is meaningful for enclave.
> > 
> > > >
> > > > > +#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 47542140f8c1..016bad7cff8d
> > > > > 100644
> > > > > --- a/arch/x86/kernel/cpu/sgx/virt.c
> > > > > +++ b/arch/x86/kernel/cpu/sgx/virt.c
> > > >
> > > > Rename as vepc.c.
> > 
> > No. This file contains more than virtual EPC implementation, but also other staff like sgx_virt_ecreate()/sgx_virt_einit(), which are used by KVM to run ECREATE/EINIT on behalf of guest.
> > 
> > > >
> > > > > @@ -257,3 +257,97 @@ 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);
> > > >
> > > > Empty line.
> > > >
> > > > > +	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();
> > > >
> > > > Ditto.
> > > >
> > > > > +	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);
> > > 
> > > Remove exports.
> > 
> > Why? KVM needs to use them in later patches.
> 
> Because they are only required for LKM's.

I mean when LKM needs to call kernel functions.

Right, KVM can be compiled as LKM.

/Jarkko
Huang, Kai Feb. 16, 2021, 9:33 a.m. UTC | #6
> > > > > > +EXPORT_SYMBOL_GPL(sgx_virt_einit);
> > > >
> > > > Remove exports.
> > >
> > > Why? KVM needs to use them in later patches.
> >
> > Because they are only required for LKM's.
> 
> I mean when LKM needs to call kernel functions.
> 
> Right, KVM can be compiled as LKM.

Just to confirm, you are OK with exposing them as symbol, since KVM (as a module) needs to use them, right?
diff mbox series

Patch

diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
new file mode 100644
index 000000000000..8a3ea3e1efbe
--- /dev/null
+++ b/arch/x86/include/asm/sgx.h
@@ -0,0 +1,16 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_SGX_H
+#define _ASM_X86_SGX_H
+
+#include <linux/types.h>
+
+#ifdef CONFIG_X86_SGX_KVM
+struct sgx_pageinfo;
+
+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 47542140f8c1..016bad7cff8d 100644
--- a/arch/x86/kernel/cpu/sgx/virt.c
+++ b/arch/x86/kernel/cpu/sgx/virt.c
@@ -257,3 +257,97 @@  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);