linux-sgx.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Huang, Kai" <kai.huang@intel.com>
To: "linux-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>,
	"Chatre, Reinette" <reinette.chatre@intel.com>,
	"jarkko@kernel.org" <jarkko@kernel.org>,
	"Dhanraj, Vijay" <vijay.dhanraj@intel.com>,
	"haitao.huang@linux.intel.com" <haitao.huang@linux.intel.com>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>
Subject: Re: [RFC PATCH v4 2/4] x86/sgx: Implement support for MADV_WILLNEED
Date: Thu, 16 Feb 2023 07:53:47 +0000	[thread overview]
Message-ID: <68d19c7bb8e28e85d503da53124fc57b916dcaf9.camel@intel.com> (raw)
In-Reply-To: <op.10ey84qywjvjmi@hhuan26-mobl.amr.corp.intel.com>

On Wed, 2023-02-15 at 09:42 -0600, Haitao Huang wrote:
> On Wed, 15 Feb 2023 02:51:23 -0600, Huang, Kai <kai.huang@intel.com> wrote:
> 
> > > > > > 
> > > > 
> > > > Since sgx_mmap() can happen before enclave is created, calculating the
> > > > vm_pgoff
> > > > from enclave_base is conceptually wrong.  Even if you really want to  
> > > do
> > > > it, it
> > > > should be:
> > > > 
> > > > 	if (enclave_has_initialized())
> > > > 		vma->vm_pgoff = ...;
> > > 
> > > I got your point now. I can add a condition to test the SGX_ENCL_CREATED
> > > bit. However, we still have a hole if we must handle the sequence
> > > mmap(..., enclave_fd) being called before ECREATE ioctl. We can't leave
> > > vm_pgoff not set for those cases.
> > > 
> > > Since no one does that so far, can we explicitly return an error from
> > > sgx_mmap when that happens?
> > > Other suggestions?
> > 
> > As I replied to patch 4/4, I believe userspace should pass the correct  
> > pgoff in
> > mmap().  It's wrong to always pass 0 or any random value.
> > 
> > If userspace follow the mmap() rule, you won't need to manually set  
> > vm_pgoff
> > here (which is hacky IMHO).  Everything works fine.
> > 
> 
> SGX driver was following MAP_ANONYMOUS semantics. If we change that, it'd  
> break current usage/ABI.

Is there any doc/reference mentioning this? 

> 
> I still think returning error for cases mmap(..., enclave_fd) if enclave  
> is not created would be less intrusive change.

But you need the first mmap() to work before IOCTL(ecreate) to get enclave's
base address, correct?

> 
> Haitao


  reply	other threads:[~2023-02-16  7:57 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-28  4:55 [RFC PATCH v4 0/4] x86/sgx: implement support for MADV_WILLNEED Haitao Huang
2023-01-28  4:55 ` [RFC PATCH v4 1/4] x86/sgx: Export sgx_encl_eaug_page Haitao Huang
2023-01-28  4:55   ` [RFC PATCH v4 2/4] x86/sgx: Implement support for MADV_WILLNEED Haitao Huang
2023-01-28  4:55     ` [RFC PATCH v4 3/4] selftests/sgx: add len field for EACCEPT op Haitao Huang
2023-01-28  4:55       ` [RFC PATCH v4 4/4] selftests/sgx: Add test for madvise(..., WILLNEED) Haitao Huang
2023-02-07 23:30         ` Jarkko Sakkinen
2023-02-15  2:38         ` Huang, Kai
2023-02-15  4:42           ` Haitao Huang
2023-02-15  8:46             ` Huang, Kai
2023-02-17 22:29               ` jarkko
2023-02-07 23:29       ` [RFC PATCH v4 3/4] selftests/sgx: add len field for EACCEPT op Jarkko Sakkinen
2023-02-07 23:28     ` [RFC PATCH v4 2/4] x86/sgx: Implement support for MADV_WILLNEED Jarkko Sakkinen
2023-02-14  9:47     ` Huang, Kai
2023-02-14 19:18       ` Haitao Huang
2023-02-14 20:54         ` Huang, Kai
2023-02-14 21:42           ` Haitao Huang
2023-02-14 22:36             ` Huang, Kai
2023-02-15  3:59               ` Haitao Huang
2023-02-15  8:51                 ` Huang, Kai
2023-02-15 15:42                   ` Haitao Huang
2023-02-16  7:53                     ` Huang, Kai [this message]
2023-02-16 17:12                       ` Haitao Huang
2023-02-17 22:32                     ` jarkko
2023-02-17 23:03                       ` Haitao Huang
2023-02-21 22:10                         ` jarkko
2023-02-22  1:37                           ` Haitao Huang
2023-03-07 23:32                             ` Huang, Kai
2023-03-09  0:50                               ` Haitao Huang
2023-03-09 11:31                                 ` Huang, Kai
2023-03-14 14:54                                   ` Haitao Huang
2023-03-19 13:26                                     ` jarkko
2023-03-20  9:36                                       ` Huang, Kai
2023-03-20 14:04                                         ` jarkko
2023-05-27  0:32                                   ` Haitao Huang
2023-06-06  4:11                                     ` Huang, Kai
2023-06-07 16:59                                       ` Haitao Huang
2023-06-16  3:49                                       ` Huang, Kai
2023-06-16 22:05                                         ` Sean Christopherson
2023-06-19 11:17                                           ` Huang, Kai
2023-06-22 22:01                                             ` Sean Christopherson
2023-06-22 23:21                                               ` Huang, Kai
2023-06-26 22:28                                                 ` Sean Christopherson
2023-06-27 11:43                                                   ` Huang, Kai
2023-06-27 14:50                                                     ` Sean Christopherson
2023-06-28  9:37                                                       ` Huang, Kai
2023-06-28 14:57                                                         ` Sean Christopherson
2023-06-29  3:10                                                           ` Huang, Kai
2023-06-29 14:23                                                             ` Sean Christopherson
2023-06-29 23:29                                                               ` Huang, Kai
2023-06-30  0:14                                                                 ` Sean Christopherson
2023-06-30  0:56                                                                   ` Huang, Kai
2023-06-30  1:54                                                                 ` Jarkko Sakkinen
2023-06-30  1:57                                                                   ` Jarkko Sakkinen
2023-06-30  4:26                                                                     ` Huang, Kai
2023-06-30  9:35                                                                       ` Jarkko Sakkinen
2023-03-12  1:25                               ` jarkko
2023-03-12 22:25                                 ` Huang, Kai
2023-02-17 22:07           ` jarkko
2023-02-17 21:50       ` jarkko
2023-02-07 23:26   ` [RFC PATCH v4 1/4] x86/sgx: Export sgx_encl_eaug_page Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=68d19c7bb8e28e85d503da53124fc57b916dcaf9.camel@intel.com \
    --to=kai.huang@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=haitao.huang@linux.intel.com \
    --cc=jarkko@kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=reinette.chatre@intel.com \
    --cc=vijay.dhanraj@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).