linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@intel.com>
To: Jarkko Sakkinen <jarkko@kernel.org>, linux-sgx@vger.kernel.org
Cc: Dave Hansen <dave.hansen@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86/sgx: Add SGX_PAGE_REPEAT flag for SGX_IOC_ENCLAVE_ADD_PAGES
Date: Thu, 10 Jun 2021 07:11:53 -0700	[thread overview]
Message-ID: <a5d4b6e4-7caa-06b3-a73b-b3869a92dbb5@intel.com> (raw)
In-Reply-To: <20210610090134.xetwllckm4dugg5c@kernel.org>

On 6/10/21 2:01 AM, Jarkko Sakkinen wrote:
> On Thu, Jun 10, 2021 at 10:21:17AM +0300, Jarkko Sakkinen wrote:
>> For uninitialized data, there's a need to add the same page multiple times,
>> e.g. a zero page, instead of traversing the source memory forward. With the
>> current API, this requires to call SGX_IOC_ENCLAVE_ADD_PAGES multiple
>> times, once per page, which is not very efficient.
>>
>> Add a new SGX_PAGE_REPEAT flag to resolve the issue. When this flag is set
>> to the 'flags' field of struct sgx_enclave_pages, the ioctl will apply the
>> page at 'src' multiple times, instead of moving forward in the address
>> space.
>>
>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> After sending this, I started to think that maybe it would actually better
> to just add SGX_PAGE_ZERO flag, i.e. add zero pages and ignore src. That's
> the main use case right now, and saves the user space from extra trouble of
> having to do such page by hand.
> 
> That neither does prevent adding SGX_PAGE_REPEAT later on. I just see no
> point of that generic functionality right now. It only makes simple use
> case more complex.

Is the main argument behind this new ABI that it increases efficiency?

Let's say I want to add 1MB of 0'd pages to an enclave.  Won't this do it?

	zeros = mmap(NULL, size, PROT_READ, MAP_ANONYMOUS|MAP_PRIVATE,
		     -1, 0);
	ioctl(SGX_IOC_ENCLAVE_ADD_PAGES, zeros, size);

Sure, you'll pay the cost of faulting in the zero page size/PAGE_SIZE
times.  But, that's pretty minuscule.  This zeros buffer can also be
reused without faulting again.  It can be as big or small as you want.
Heck, it could even be 2MB in size and use the transparent huge page.

I agree that there's definitely some optimization work to do.  But, I'm
a bit hesitant to turn to new ABI to do it.

  reply	other threads:[~2021-06-10 14:12 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-10  7:21 [PATCH] x86/sgx: Add SGX_PAGE_REPEAT flag for SGX_IOC_ENCLAVE_ADD_PAGES Jarkko Sakkinen
2021-06-10  9:01 ` Jarkko Sakkinen
2021-06-10 14:11   ` Dave Hansen [this message]
2021-06-14 20:48     ` 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=a5d4b6e4-7caa-06b3-a73b-b3869a92dbb5@intel.com \
    --to=dave.hansen@intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jarkko@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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).