Re: [RFC PATCH v4 2/4] x86/sgx: Implement support for MADV_WILLNEED

["Haitao Huang" <haitao.huang@linux.intel.com>]

Hi Kai, Jarkko and Dave

On Thu, 09 Mar 2023 05:31:29 -0600, Huang, Kai <kai.huang@intel.com> wrote:
>
> So I am still a little bit confused about where does "SGX driver uses
> MAP_ANONYMOUS semantics for fd-based mmap()" come from.
>
> Anyway, we certainly don't want to break userspace.  However, IIUC, even  
> from
> now on we change the driver to depend on userspace to pass the correct  
> pgoff in
> mmap(), this won't break userspace, because old userspace which doesn't  
> use
> fadvice() and pgoff actually doesn't matter.  For new userspace which  
> uses
> fadvice(), it needs to pass the correct pgoff.
>
> I am not saying we should do this, but it doesn't seem we can break  
> userspace?
>

Sorry for delayed update but I thought about this more and likely to  
propose a new EAUG ioctl for this and for enabling SGX-CET shadow stack  
pages. But regardless, I'd like to wrap up this discussion to just clarify  
this anonymous semantics design in documentation so people won't get  
confused in future.

I think we all agree to keep this semantics so no user space would need  
specify 'offset' for mmap with enclave fd. And here is my proposed  
documentation changes.

--- a/Documentation/x86/sgx.rst
+++ b/Documentation/x86/sgx.rst
@@ -100,6 +100,23 @@ pages and establish enclave page permissions.
                 sgx_ioc_enclave_init
                 sgx_ioc_enclave_provision

+Enclave memory mapping
+----------------------
+
+A file descriptor created from opening **/dev/sgx_enclave** represents an
+enclave object. The mmap() syscall with enclave file descriptors does not
+support non-zero value for the 'offset' parameter.
+
+Rational:
+
+Enclave mapping is very similar to anonymous mapping in that it maps  
physical
+EPC pages to virtual addresses and the physical pages need not be  
contiguous. And
+the content of each enclave page must be loaded at an expected offset  
relative
+to SECS.BASEADDR as is reflected in measurements in its SIGSTRUCT.  
Otherwise
+EINIT would fail to verify the measurements and initialize the enclave.  
This is
+unlike regular file mapping in that no content offset can be defined that  
is
+independent from the virtual address it is loaded to.
+
  Enclave runtime management
  --------------------------

Let me know your thoughts and I can submit the patch if you think this is  
the direction to go.

Thanks
Haitao