Get rid of BUG()
cover Get rid of BUG()
The use of BUG() is not favored for any new kernel code. This patch set implements more reasonable error recovery. In all cases the recovery is not ideal but it is an improvement to the current situation.
In addition, sgx_free_page() has been simplified by removing the parameter for skipping EREMOVE. EREMOVE can be successfully executed for an unused EPC page.
Jarkko Sakkinen (3):
intel_sgx: do not use BUG() in sgx_free_page()
intel_sgx: remove flags parameter from sgx_free_page()
intel_sgx: fix error paths for EBLOCK and ETRACK
drivers/platform/x86/intel_sgx.h | 4 +- drivers/platform/x86/intel_sgx_ioctl.c | 12 ++-- drivers/platform/x86/intel_sgx_page_cache.c | 89 +++++++++++++++-------------- drivers/platform/x86/intel_sgx_util.c | 6 +- drivers/platform/x86/intel_sgx_vma.c | 6 +- 5 files changed, 55 insertions(+), 62 deletions(-)
Signature · 1 line
2.9.3
PATCH 1/3 intel_sgx: do not use BUG() in sgx_free_page()
EREMOVE fails on non-EPC page or when a SECS page with children is to be removed. These do not happen if the driver is working correctly. Log the error but do not crash the driver.
--- drivers/platform/x86/intel_sgx_page_cache.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/platform/x86/intel_sgx_page_cache.c +2 -4
@@ -551,10 +551,8 @@ void sgx_free_page(struct sgx_epc_page *entry, ret =__eremove (epc);sgx_put_epc_page (epc); -if (ret) { -pr_err ("EREMOVE returned %d\n" , ret); -BUG (); - } +if (ret) +sgx_err (encl,"EREMOVE returned %d\n" , ret); }spin_lock (&sgx_free_list_lock);
Signature · 1 line
2.9.3
Re: [PATCH 1/3] intel_sgx: do not use BUG() in sgx_free_page()
Quoted 14 lines · Jarkko Sakkinen
EREMOVE fails on non-EPC page or when a SECS page with children is to be removed. These do not happen if the driver is working correctly. Log the error but do not crash the driver.
Signed-off-by Jarkko Sakkinen <[email protected]>--- drivers/platform/x86/intel_sgx_page_cache.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)drivers/platform/x86/intel_sgx_page_cache.c +0
-0
drivers/platform/x86/intel_sgx_page_cache.c void sgx_free_page(struct sgx_epc_page *entry, · line 555
Quoted 9 lines
drivers/platform/x86/intel_sgx_page_cache.c +2
-4@@ -551,10 +551,8 @@ void sgx_free_page(struct sgx_epc_page *entry, ret =__eremove (epc);sgx_put_epc_page (epc); -if (ret) { -pr_err ("EREMOVE returned %d\n" , ret); -BUG (); - } +if (ret) +sgx_err (encl,"EREMOVE returned %d\n" , ret);
Do you have something like critical level? For me seems reasonable to increase the level of message if BUG() was somehow related to actual situation.
Quoted 3 lines
drivers/platform/x86/intel_sgx_page_cache.c +0
-0}spin_lock (&sgx_free_list_lock);
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH 1/3] intel_sgx: do not use BUG() in sgx_free_page()
Quoted 19 lines · Andy Shevchenko
Quoted 17 lines · Jarkko Sakkinen
EREMOVE fails on non-EPC page or when a SECS page with children is to be removed. These do not happen if the driver is working correctly. Log the error but do not crash the driver.
Signed-off-by Jarkko Sakkinen <[email protected]>--- drivers/platform/x86/intel_sgx_page_cache.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)drivers/platform/x86/intel_sgx_page_cache.c +0
-0@@ -551,10 +551,8 @@ void sgx_free_page(struct sgx_epc_page *entry, ret =__eremove (epc);sgx_put_epc_page (epc);
Quoted 10 lines
drivers/platform/x86/intel_sgx_page_cache.c void sgx_free_page(struct sgx_epc_page *entry, · line 555
Quoted 6 lines
drivers/platform/x86/intel_sgx_page_cache.c +2
-4-if (ret) { -pr_err ("EREMOVE returned %d\n" , ret); -BUG (); - } +if (ret) +sgx_err (encl,"EREMOVE returned %d\n" , ret);Do you have something like critical level? For me seems reasonable to increase the level of message if
BUG()was somehow related to actual situation.
Hmm... I think that would make sense. This could only happen when the driver implementation is working incorrectly.
/Jarkko
Quoted 8 lines · Jarkko Sakkinen
The use of
BUG()is not favored for any new kernel code. This patch set implements more reasonable error recovery. In all cases the recovery is not ideal but it is an improvement to the current situation.In addition,
sgx_free_page()has been simplified by removing the parameter for skipping EREMOVE. EREMOVE can be successfully executed for an unused EPC page.Jarkko Sakkinen (3):
By some reason I only see first patch.
What's going on?
Quoted 14 lines
intel_sgx: do not useBUG()insgx_free_page()
intel_sgx: remove flags parameter fromsgx_free_page()
intel_sgx: fix error paths for EBLOCK and ETRACKdrivers/platform/x86/intel_sgx.h | 4 +- drivers/platform/x86/intel_sgx_ioctl.c | 12 ++-- drivers/platform/x86/intel_sgx_page_cache.c | 89 +++++++++++++++-------------- drivers/platform/x86/intel_sgx_util.c | 6 +- drivers/platform/x86/intel_sgx_vma.c | 6 +- 5 files changed, 55 insertions(+), 62 deletions(-)--
2.9.3
Signature · 2 lines
With Best Regards, Andy Shevchenko