| Message ID | 20191022224922.28144-3-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/sgx: More cleanup for v23 | expand |
On Tue, Oct 22, 2019 at 03:49:21PM -0700, Sean Christopherson wrote: > Don't add an EPC page to the free page list of EREMOVE fails, as doing > so will cause any future attempt to use the EPC page to fail, and likely > WARN as well. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > arch/x86/kernel/cpu/sgx/main.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c > index d45bf6fca0c8..8e7557d3ff03 100644 > --- a/arch/x86/kernel/cpu/sgx/main.c > +++ b/arch/x86/kernel/cpu/sgx/main.c > @@ -138,7 +138,8 @@ int sgx_free_page(struct sgx_epc_page *page) > spin_unlock(&sgx_active_page_list_lock); > > ret = __eremove(sgx_epc_addr(page)); > - WARN_ONCE(ret, "EREMOVE returned %d (0x%x)", ret, ret); > + if (WARN_ONCE(ret, "EREMOVE returned %d (0x%x)", ret, ret)) > + return -EIO; How did you end up choosing -EIO? Don't immediately have any better suggestion but neither sure if that is the best choice. That is why I'm asking. /Jarkko
On Wed, Oct 23, 2019 at 03:43:40PM +0300, Jarkko Sakkinen wrote: > On Tue, Oct 22, 2019 at 03:49:21PM -0700, Sean Christopherson wrote: > > Don't add an EPC page to the free page list of EREMOVE fails, as doing > > so will cause any future attempt to use the EPC page to fail, and likely > > WARN as well. > > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > --- > > arch/x86/kernel/cpu/sgx/main.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c > > index d45bf6fca0c8..8e7557d3ff03 100644 > > --- a/arch/x86/kernel/cpu/sgx/main.c > > +++ b/arch/x86/kernel/cpu/sgx/main.c > > @@ -138,7 +138,8 @@ int sgx_free_page(struct sgx_epc_page *page) > > spin_unlock(&sgx_active_page_list_lock); > > > > ret = __eremove(sgx_epc_addr(page)); > > - WARN_ONCE(ret, "EREMOVE returned %d (0x%x)", ret, ret); > > + if (WARN_ONCE(ret, "EREMOVE returned %d (0x%x)", ret, ret)) > > + return -EIO; > > How did you end up choosing -EIO? Don't immediately have any better > suggestion but neither sure if that is the best choice. That is why I'm > asking. sgx_edbgrd() and sgx_edbgwr() return -EIO on failure, and they're the only other case I can think of where an ENCLS instruction failure is reported to userspace *and* may or may not be due to a fault. I honestly didn't spend much time thinking about it as the code is dropped in the next path.
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index d45bf6fca0c8..8e7557d3ff03 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -138,7 +138,8 @@ int sgx_free_page(struct sgx_epc_page *page) spin_unlock(&sgx_active_page_list_lock); ret = __eremove(sgx_epc_addr(page)); - WARN_ONCE(ret, "EREMOVE returned %d (0x%x)", ret, ret); + if (WARN_ONCE(ret, "EREMOVE returned %d (0x%x)", ret, ret)) + return -EIO; spin_lock(§ion->lock); list_add_tail(&page->list, §ion->page_list);
Don't add an EPC page to the free page list of EREMOVE fails, as doing so will cause any future attempt to use the EPC page to fail, and likely WARN as well. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/kernel/cpu/sgx/main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)