From: "Huang, Kai" <email@example.com>
To: "Christopherson,, Sean" <firstname.lastname@example.org>
Cc: "email@example.com" <firstname.lastname@example.org>,
"Chatre, Reinette" <email@example.com>,
"Dhanraj, Vijay" <firstname.lastname@example.org>,
Subject: Re: [RFC PATCH v4 2/4] x86/sgx: Implement support for MADV_WILLNEED
Date: Wed, 28 Jun 2023 09:37:07 +0000 [thread overview]
Message-ID: <email@example.com> (raw)
On Tue, 2023-06-27 at 07:50 -0700, Sean Christopherson wrote:
> On Tue, Jun 27, 2023, Kai Huang wrote:
> > > > >
> > > > > You can't build an ABI on assumptions. E.g. even if userspace *intends* to behave,
> > > > > it wouldn't take much to compute the wrong offset (math is hard).
> > > >
> > > > But I don't think we have an well established ABI now? Nothing is documented.
> > >
> > > Heh, just because the ABI isn't formally documented doesn't mean it doesn't exist.
> > > In fact, the most problematic ABIs are the ones that aren't documented.
> > >
> > > >
> > Sure. But just want to make sure, what is the current SGX driver ABI (around
> > mmap()) from your perspective?
> To be clear, it's not my perspective, it's simply what the kernel actually does.
Sure. I was just trying to hear your thoughts :)
> > Is it "SGX driver _requires_ pgoff to be 0 for non-ELRANGE-reserve mmap()", or
> > "SGX driver _ignores_ pgoff"?
> Unless there are checks hiding somewhere, it's the latter. You might be able to
> get away with changing the kernel to require pgoff to be '0', i.e. if literally
> all users pass in '0', but proving that all users pass '0' is extremely difficult.
> And I don't see any value in requiring pgoff to be '0' for "legacy" users.
I certainly hate to enforce kernel to "require" 0 pgoff from userspace. I want
to get rid of it.
I believe we both agree "SGX driver _ignores_ pgoff" is the current ABI.
> I think you're fixated too much on precisely defining the concept of ABI.
> question that you really want to ask is "could change XYZ break userspace?"
But since "encl->has_mismatched_offsets" is sort of new ABI, I think we need to
be careful otherwise in the future we may hit this kinda nasty issue again.
Here's my thoughts:
(Again, let's forget about mmap(/dev/sgx_enclave) to reserve ELRANGE for now.)
1) Current ABI is SGX driver _ignores_ pgoff for mmap(/dev/sgx_enclave)s (but
2) Therefore, "passing the correct pgoff" in new userspace app doesn't break the
current ABI. If the new app chooses to pass the correct pgoff, it will work.
3) With additional support of sgx_fadvice(WILLNEED) within the driver, the new
app can use madvice(WILLNEED) if it passes the correct pgoff when mmap(). If
wrong pgoff is passed, then madvice(WILLNEED) will work unexpectedly and could
result in enclave being killed. It's userspace app's responsibility to make
sure the correctness, not the driver's.
4) Old SGX apps which don't use file-based ABIs and still pass 0 pgoff should
continue to work. No break of old apps either.
5) We encourage new apps to always pass the correct pgoff instead of passing 0.
6) If needed, we can modify sgx_mmap() to relax the needing to use MAP_FIXED,
but return the enclave's address "based on pgoff from userspace". This
effectively provides additional mmap() ABI for userspace while not breaking the
existing MAP_FIXED ABI.
In this way, we don't need additional manipulation/fixing up of VMA's pgoff in
the driver. There's no change to existing ABI either.
> Because SGX has users in the wild that don't set pgoff correctly. Changing the
> kernel to require an accurate pgoff would break those users.
Only require an accurate pgoff if userspace wants to use file-based ABIs for the
relevant VMAs. The old apps which pass 0 pgoff should just continue to work.
next prev parent reply other threads:[~2023-06-28 9:52 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
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 [this message]
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
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:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* 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).