| Message ID | 20200303205359.108336-2-jarkko.sakkinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] x86/sgx: cleanup: Remove unused struct | expand |
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 >
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
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; }
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(-)