On Tue, 15 Nov 2022 16:03:45 -0600, Jarkko Sakkinen <jarkko@xxxxxxxxxx>
wrote:
+ if (!encl)
+ return -EINVAL;
Why !encl check is needed?
It was intended as sanity check. But I think encl should not be null at
this point so will remove in next version.
+ if (!cpu_feature_enabled(X86_FEATURE_SGX2))
+ return -EINVAL;
This should be done first before doing anything else in the function.
Will do
+
+ if (offset + len < offset)
+ return -EINVAL;
+ if (encl->base + offset < encl->base)
+ return -EINVAL;
+ start = offset + encl->base;
+ end = start + len;
These could be just as well assigned in the declarations, which would
definitely be also less convoluted.
Will do
...
diff --git a/arch/x86/kernel/cpu/sgx/encl.c
b/arch/x86/kernel/cpu/sgx/encl.c
index 1abc5e7f2660..c57e60d5a0aa 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -305,11 +305,11 @@ struct sgx_encl_page *sgx_encl_load_page(struct
sgx_encl *encl,
* on a SGX2 system then the EPC can be added dynamically via the SGX2
* ENCLS[EAUG] instruction.
*
- * Returns: Appropriate vm_fault_t: VM_FAULT_NOPAGE when PTE was
installed
- * successfully, VM_FAULT_SIGBUS or VM_FAULT_OOM as error otherwise.
+ * Returns: 0 when PTE was installed successfully, -EBUSY for waiting
on
+ * reclaimer to free EPC, -ENOMEM for out of RAM, -EFAULT as error
otherwise.
*/
-vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma,
- struct sgx_encl *encl, unsigned long addr)
+int sgx_encl_eaug_page(struct vm_area_struct *vma,
+ struct sgx_encl *encl, unsigned long addr)
{
vm_fault_t vmret = VM_FAULT_SIGBUS;
struct sgx_pageinfo pginfo = {0};
@@ -318,10 +318,10 @@ vm_fault_t sgx_encl_eaug_page(struct
vm_area_struct *vma,
struct sgx_va_page *va_page;
unsigned long phys_addr;
u64 secinfo_flags;
- int ret;
+ int ret = -EFAULT;
Why?
Original code uses ret only to temporarily store return values from the
called functions.
Now I also use it as return of this function and assign -EFAULT as default
in all cases that ret is not assigned explicitly below, e.g., -EBUSY cases.
Basically -EFAULT cases are the same as VM_FAULT_SIGBUS cases in original
code.
I'll clarify in comments above for the return values.
Thanks
Haitao
