[PATCH] Check mm_users!=0 when pinning enclave's MM
by Sean Christopherson
Abort sgx_pin_mm if mm_users==0 after acquiring mmap_sem to avoid
racing with do_exit during allocation and evictions flows. Remove
the same check from sgx_process_add_page_req as it is now redundant.
Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com>
---
drivers/platform/x86/intel_sgx_ioctl.c | 6 ------
drivers/platform/x86/intel_sgx_util.c | 11 ++++++++++-
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c
index 6c8962f..43aaaad 100644
--- a/drivers/platform/x86/intel_sgx_ioctl.c
+++ b/drivers/platform/x86/intel_sgx_ioctl.c
@@ -262,12 +262,6 @@ static bool sgx_process_add_page_req(struct sgx_add_page_req *req)
if (IS_ERR(backing))
goto out;
- /* Do not race with do_exit() */
- if (!atomic_read(&encl->mm->mm_users)) {
- sgx_put_backing(backing, 0);
- goto out;
- }
-
ret = vm_insert_pfn(vma, encl_page->addr, PFN_DOWN(epc_page->pa));
if (ret)
goto out;
diff --git a/drivers/platform/x86/intel_sgx_util.c b/drivers/platform/x86/intel_sgx_util.c
index d1c4c71..d076d77 100644
--- a/drivers/platform/x86/intel_sgx_util.c
+++ b/drivers/platform/x86/intel_sgx_util.c
@@ -149,7 +149,16 @@ bool sgx_pin_mm(struct sgx_encl *encl)
down_read(&encl->mm->mmap_sem);
- if (!encl->vma_cnt) {
+ /* Check both vma_cnt and mm_users after acquiring mmap_sem
+ * to avoid racing with the owning process exiting. mm_users
+ * needs to be checked as do_exit->exit_mmap tears down VMAs
+ * and PTEs without holding any MM locks (once mm_users==0).
+ * mm_count only guarantees the MM's kernel objects will not
+ * be freed, it doesn't protect the VMAs or PTEs. Allowing
+ * EPC page eviction to race with the PTEs being dismantled
+ * can result in PTEs being left in use when the MM is freed.
+ */
+ if (!encl->vma_cnt || !atomic_read(&encl->mm->mm_users)) {
sgx_unpin_mm(encl);
return false;
}
--
2.7.4
5 years, 8 months
[PATCH v2 0/7] Fixes and performance improvements
by Jarkko Sakkinen
Jarkko Sakkinen (4):
intel_sgx: fix deadlock in sgx_ioc_enclave_create()
intel_sgx: freeze VMAs after EPC pages have been added
intel_sgx: fix null pointer deref in sgx_invalidate()
intel_sgx: invalidate enclave when the user threads cease to exist
Sean Christopherson (3):
intel_sgx: add LRU algorithm to page swapping
intel_sgx: return correct errno for EINIT ioctl
intel_sgx: lock the enclave for the entire EPC eviction flow
drivers/platform/x86/Kconfig | 1 +
drivers/platform/x86/intel_sgx.h | 9 ++++--
drivers/platform/x86/intel_sgx_ioctl.c | 36 +++++++++++++++++++---
drivers/platform/x86/intel_sgx_page_cache.c | 46 ++++++++++++++++++++++-------
drivers/platform/x86/intel_sgx_util.c | 11 +++++--
drivers/platform/x86/intel_sgx_vma.c | 33 ++++++++-------------
6 files changed, 95 insertions(+), 41 deletions(-)
--
2.9.3
5 years, 10 months
[PATCH v2] intel_sgx: Add lock to make EPC eviction atomic
by Sean Christopherson
Add an eviction specific, per-enclave lock so that the lock can be held
for the entire EPC eviction sequence without having to hold the overall
per-enclave lock for an extended duration. Holding an enclave's overall
lock prevents any other driver operations on the enclave, e.g. faulting
a page into the enclave, and evicting 16 EPC pages takes upwards of half
a millisecond.
Holding a lock for the entire EBLOCK->ETRACK->EWB sequence is necessary
to make the sequence atomic with respect to other evictions on the same
enclave. The SGX architecture does not allow multiple ETRACK sequences
to be in flight at the same time if the enclave being tracked has one or
more active threads in the enclave. A second ETRACK will fail due to
PREV_TRK_INCMPL if it is attempted before EWB is executed on all blocked
pages (in the presence of active enclave threads).
Currently the driver releases the enclave's lock after ETRACK and then
reacquires the lock prior to EWB. Releasing the lock from the thread
that performed ETRACK, T1, allows a different thread, T2, to acquire the
lock for its own ETRACK. T2's ETRACK will fail due to the architectural
restrictions if a third thread, T3, is executing inside the enclave
whose pages are being swapped.
T3: <active in enclave E1>
T1: lock_mutex(E1)
T1: ETRACK(E1)
T1: unlock_mutex(E1)
T2: lock_mutex(E1)
T2: ETRACK(E1) fails
Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com>
---
drivers/platform/x86/intel_sgx.h | 1 +
drivers/platform/x86/intel_sgx_ioctl.c | 1 +
drivers/platform/x86/intel_sgx_page_cache.c | 69 +++++++++++++++++------------
3 files changed, 43 insertions(+), 28 deletions(-)
diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h
index 464763d..d837789 100644
--- a/drivers/platform/x86/intel_sgx.h
+++ b/drivers/platform/x86/intel_sgx.h
@@ -137,6 +137,7 @@ struct sgx_encl {
unsigned int secs_child_cnt;
unsigned int vma_cnt;
struct mutex lock;
+ struct mutex eviction_lock;
struct task_struct *owner;
struct mm_struct *mm;
struct file *backing;
diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c
index d54c410..72fe9aa 100644
--- a/drivers/platform/x86/intel_sgx_ioctl.c
+++ b/drivers/platform/x86/intel_sgx_ioctl.c
@@ -522,6 +522,7 @@ static long sgx_ioc_enclave_create(struct file *filep, unsigned int cmd,
INIT_LIST_HEAD(&encl->load_list);
INIT_LIST_HEAD(&encl->encl_list);
mutex_init(&encl->lock);
+ mutex_init(&encl->eviction_lock);
INIT_WORK(&encl->add_page_work, sgx_add_page_worker);
encl->owner = current->group_leader;
diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c
index 8b1cc82..ce60b76 100644
--- a/drivers/platform/x86/intel_sgx_page_cache.c
+++ b/drivers/platform/x86/intel_sgx_page_cache.c
@@ -224,9 +224,9 @@ static int sgx_ewb(struct sgx_encl *encl,
return ret;
}
-void sgx_free_encl_page(struct sgx_encl_page *entry,
- struct sgx_encl *encl,
- unsigned int flags)
+static void sgx_free_encl_page(struct sgx_encl_page *entry,
+ struct sgx_encl *encl,
+ unsigned int flags)
{
sgx_free_page(entry->epc_page, encl, flags);
entry->epc_page = NULL;
@@ -238,7 +238,7 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src)
struct sgx_encl_page *entry;
struct sgx_encl_page *tmp;
struct page *pages[SGX_NR_SWAP_CLUSTER_MAX + 1];
- struct vm_area_struct *evma;
+ struct vm_area_struct *vma;
int cnt = 0;
int i = 0;
int ret;
@@ -261,13 +261,15 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src)
return;
}
- /* EBLOCK */
+ mutex_lock(&encl->eviction_lock);
+ /* EBLOCK */
list_for_each_entry_safe(entry, tmp, src, load_list) {
- mutex_lock(&encl->lock);
- evma = sgx_find_vma(encl, entry->addr);
- if (!evma) {
+ vma = sgx_find_vma(encl, entry->addr);
+ if (!vma) {
list_del(&entry->load_list);
+
+ mutex_lock(&encl->lock);
sgx_free_encl_page(entry, encl, 0);
mutex_unlock(&encl->lock);
continue;
@@ -276,36 +278,32 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src)
pages[cnt] = sgx_get_backing(encl, entry);
if (IS_ERR(pages[cnt])) {
list_del(&entry->load_list);
+
+ mutex_lock(&encl->lock);
list_add_tail(&entry->load_list, &encl->load_list);
entry->flags &= ~SGX_ENCL_PAGE_RESERVED;
mutex_unlock(&encl->lock);
continue;
}
- zap_vma_ptes(evma, entry->addr, PAGE_SIZE);
+ zap_vma_ptes(vma, entry->addr, PAGE_SIZE);
sgx_eblock(entry->epc_page);
cnt++;
- mutex_unlock(&encl->lock);
}
- /* ETRACK */
+ if (!cnt) {
+ mutex_unlock(&encl->eviction_lock);
+ goto out;
+ }
- mutex_lock(&encl->lock);
+ /* ETRACK */
sgx_etrack(encl->secs_page.epc_page);
- mutex_unlock(&encl->lock);
/* EWB */
-
- mutex_lock(&encl->lock);
i = 0;
-
- while (!list_empty(src)) {
- entry = list_first_entry(src, struct sgx_encl_page,
- load_list);
- list_del(&entry->load_list);
-
- evma = sgx_find_vma(encl, entry->addr);
- if (evma) {
+ list_for_each_entry(entry, src, load_list) {
+ vma = sgx_find_vma(encl, entry->addr);
+ if (vma) {
ret = sgx_ewb(encl, entry, pages[i]);
BUG_ON(ret != 0 && ret != SGX_NOT_TRACKED);
/* Only kick out threads with an IPI if needed. */
@@ -313,15 +311,30 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src)
smp_call_function(sgx_ipi_cb, NULL, 1);
BUG_ON(sgx_ewb(encl, entry, pages[i]));
}
- encl->secs_child_cnt--;
}
+ sgx_put_backing(pages[i++], vma);
+ }
+
+ mutex_unlock(&encl->eviction_lock);
+
+ mutex_lock(&encl->lock);
+
+ /* Bookeeping */
+ while (!list_empty(src)) {
+ entry = list_first_entry(src, struct sgx_encl_page,
+ load_list);
+ list_del(&entry->load_list);
sgx_free_encl_page(entry, encl,
- evma ? SGX_FREE_SKIP_EREMOVE : 0);
- sgx_put_backing(pages[i++], evma);
+ vma ? SGX_FREE_SKIP_EREMOVE : 0);
}
- /* Allow SECS page eviction only when the encl is initialized. */
+ encl->secs_child_cnt -= cnt;
+
+ /* Allow SECS page eviction only when the encl is initialized.
+ * The eviction lock does not need to be held to evict the SECS,
+ * conflict is impossible as there are no other pages to evict.
+ */
if (!encl->secs_child_cnt &&
(encl->flags & SGX_ENCL_INITIALIZED)) {
pages[cnt] = sgx_get_backing(encl, &encl->secs_page);
@@ -336,9 +349,9 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src)
sgx_put_backing(pages[cnt], true);
}
}
-
mutex_unlock(&encl->lock);
+out:
sgx_unpin_mm(encl);
}
--
2.7.4
5 years, 10 months
[PATCH v3 0/7] Fixes and performance improvements
by Jarkko Sakkinen
Jarkko Sakkinen (4):
intel_sgx: fix deadlock in sgx_ioc_enclave_create()
intel_sgx: freeze VMAs after EPC pages have been added
intel_sgx: fix null pointer deref in sgx_invalidate()
intel_sgx: invalidate enclave when the user threads cease to exist
Sean Christopherson (3):
intel_sgx: add LRU algorithm to page swapping
intel_sgx: return correct errno for EINIT ioctl
intel_sgx: lock the enclave for the entire EPC eviction flow
drivers/platform/x86/Kconfig | 1 +
drivers/platform/x86/intel_sgx.h | 9 ++++--
drivers/platform/x86/intel_sgx_ioctl.c | 37 ++++++++++++++++++++---
drivers/platform/x86/intel_sgx_page_cache.c | 46 ++++++++++++++++++++++-------
drivers/platform/x86/intel_sgx_util.c | 12 ++++++--
drivers/platform/x86/intel_sgx_vma.c | 33 ++++++++-------------
6 files changed, 97 insertions(+), 41 deletions(-)
--
2.9.3
5 years, 10 months
Re: [intel-sgx-kernel-dev] [PATCH] Lock the enclave for the entire EPC eviction flow
by Ayoun, Serge
The failure of etrack is a problem but the solution below is not ideal: taking the enclave
Lock for the period of time including ewb is problematic and may aggrabate lock contention
and have serious performance problems.
Also we should not have the same sequence (T1, T2....) on the same enclave since isolate_cluster pushes the enclave
At the end of the enclave load list - unless this is the only enclave in which case it would be better
Not to start the second sequence of eviction.
> The SGX architecture does not allow multiple ETRACK sequences to be
> in flight at the same time if the enclave being tracked has one or
> more active threads in the enclave. A second ETRACK will fail due
> to PREV_TRK_INCMPL if it is attempted before EWB is executed on all
> EBLOCKed pages (in the presence of active enclave threads).
>
> Currently the driver releases the enclave's lock after ETRACK and
> then reacquires the lock prior to EWB. Releasing the lock from the
> thread that performed ETRACK, T1, allows a different thread, T2, to
> acquire the lock for its own ETRACK. T2's ETRACK will fail due to
> the architectural restrictions if a third thread, T3, is executing
> inside the enclave whose pages are being swapped.
>
> T3: <active in enclave E1>
> T1: lock_mutex(E1)
> T1: ETRACK(E1)
> T1: unlock_mutex(E1)
> T2: lock_mutex(E1)
> T2: ETRACK(E1) fails
>
> ETRACK failing is not fatal in and of itself, but the driver is not
> designed to handle an ETRACK failure (BUG_ON non-zero return code).
> Rather than modifying the driver to handle the failing path, e.g by
> kicking T3 out of the enclave with an IPI, simply lock the enclave
> for the entire ETRACK->EWB sequence to prevent multiple concurrent
> ETRACKs on a single enclave. Holding the lock for ETRACK->EWB does
> not negatively impact performance in any way.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson at intel.com>
> ---
> drivers/platform/x86/intel_sgx_page_cache.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.
> index 8b1cc82..52f51da 100644
> --- a/drivers/platform/x86/intel_sgx_page_cache.c
> +++ b/drivers/platform/x86/intel_sgx_page_cache.c
> @@ -261,15 +261,14 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src)
> return;
> }
>
> - /* EBLOCK */
> + mutex_lock(&encl->lock);
>
> + /* EBLOCK */
> list_for_each_entry_safe(entry, tmp, src, load_list) {
> - mutex_lock(&encl->lock);
> evma = sgx_find_vma(encl, entry->addr);
> if (!evma) {
> list_del(&entry->load_list);
> sgx_free_encl_page(entry, encl, 0);
> - mutex_unlock(&encl->lock);
> continue;
> }
>
> @@ -278,27 +277,22 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src)
> list_del(&entry->load_list);
> list_add_tail(&entry->load_list, &encl->load_list);
> entry->flags &= ~SGX_ENCL_PAGE_RESERVED;
> - mutex_unlock(&encl->lock);
> continue;
> }
>
> zap_vma_ptes(evma, entry->addr, PAGE_SIZE);
> sgx_eblock(entry->epc_page);
> cnt++;
> - mutex_unlock(&encl->lock);
> }
>
> - /* ETRACK */
> + if (!cnt)
> + goto out;
>
> - mutex_lock(&encl->lock);
> + /* ETRACK */
> sgx_etrack(encl->secs_page.epc_page);
> - mutex_unlock(&encl->lock);
>
> /* EWB */
> -
> - mutex_lock(&encl->lock);
> i = 0;
> -
> while (!list_empty(src)) {
> entry = list_first_entry(src, struct sgx_encl_page,
> load_list);
> @@ -337,6 +331,7 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src)
> }
> }
>
> +out:
> mutex_unlock(&encl->lock);
>
> sgx_unpin_mm(encl);
> --
> 2.7.4
---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
5 years, 10 months
[PATCH] Lock the enclave for the entire EPC eviction flow
by Sean Christopherson
The SGX architecture does not allow multiple ETRACK sequences to be
in flight at the same time if the enclave being tracked has one or
more active threads in the enclave. A second ETRACK will fail due
to PREV_TRK_INCMPL if it is attempted before EWB is executed on all
EBLOCKed pages (in the presence of active enclave threads).
Currently the driver releases the enclave's lock after ETRACK and
then reacquires the lock prior to EWB. Releasing the lock from the
thread that performed ETRACK, T1, allows a different thread, T2, to
acquire the lock for its own ETRACK. T2's ETRACK will fail due to
the architectural restrictions if a third thread, T3, is executing
inside the enclave whose pages are being swapped.
T3: <active in enclave E1>
T1: lock_mutex(E1)
T1: ETRACK(E1)
T1: unlock_mutex(E1)
T2: lock_mutex(E1)
T2: ETRACK(E1) fails
ETRACK failing is not fatal in and of itself, but the driver is not
designed to handle an ETRACK failure (BUG_ON non-zero return code).
Rather than modifying the driver to handle the failing path, e.g by
kicking T3 out of the enclave with an IPI, simply lock the enclave
for the entire ETRACK->EWB sequence to prevent multiple concurrent
ETRACKs on a single enclave. Holding the lock for ETRACK->EWB does
not negatively impact performance in any way.
Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com>
---
drivers/platform/x86/intel_sgx_page_cache.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c
index 8b1cc82..52f51da 100644
--- a/drivers/platform/x86/intel_sgx_page_cache.c
+++ b/drivers/platform/x86/intel_sgx_page_cache.c
@@ -261,15 +261,14 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src)
return;
}
- /* EBLOCK */
+ mutex_lock(&encl->lock);
+ /* EBLOCK */
list_for_each_entry_safe(entry, tmp, src, load_list) {
- mutex_lock(&encl->lock);
evma = sgx_find_vma(encl, entry->addr);
if (!evma) {
list_del(&entry->load_list);
sgx_free_encl_page(entry, encl, 0);
- mutex_unlock(&encl->lock);
continue;
}
@@ -278,27 +277,22 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src)
list_del(&entry->load_list);
list_add_tail(&entry->load_list, &encl->load_list);
entry->flags &= ~SGX_ENCL_PAGE_RESERVED;
- mutex_unlock(&encl->lock);
continue;
}
zap_vma_ptes(evma, entry->addr, PAGE_SIZE);
sgx_eblock(entry->epc_page);
cnt++;
- mutex_unlock(&encl->lock);
}
- /* ETRACK */
+ if (!cnt)
+ goto out;
- mutex_lock(&encl->lock);
+ /* ETRACK */
sgx_etrack(encl->secs_page.epc_page);
- mutex_unlock(&encl->lock);
/* EWB */
-
- mutex_lock(&encl->lock);
i = 0;
-
while (!list_empty(src)) {
entry = list_first_entry(src, struct sgx_encl_page,
load_list);
@@ -337,6 +331,7 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src)
}
}
+out:
mutex_unlock(&encl->lock);
sgx_unpin_mm(encl);
--
2.7.4
5 years, 10 months
LRU patch
by Jarkko Sakkinen
Hey
I don't what has happened but I do not have the email in my inbox
for the email that had your LRU patch so I'll just have to write
this disjoint email message.
I tried to think why I dropped that part from the upstream kernel
and the reason is quite simple. You can DoS the EPC by adding
pages but not using them.
I'm sure that this can be sorted out. I didn't do it for whatever
reasons that I can't recall at the time. Could you do these things.
1. Check that what I say holds. If it doesn't, explain why. I ripped
this part off without any deep analysis.
2. If it does hold, rework on the patch to get around this issue. Try
to find something that doesn't turn things around too much.
This optimization is obvious so I can take it as long as it doesn't
break anything. However, we should eventually have a common benchmark
for checking the claims.
/Jarkko
5 years, 10 months
[PATCH 0/7] small scoped fixes
by Jarkko Sakkinen
Hardened the enclave in a way that VMAs cannot be changed after the
enclave contains pages. This could be easened with SGX2 but for now
it is needed for stability. This patch set also includes bug fixes
for race conditions and performance optimizations.
Jarkko Sakkinen (4):
intel_sgx: fix deadlock in sgx_ioc_enclave_create()
intel_sgx: freeze VMAs after EPC pages have been added
intel_sgx: fix null pointer deref in sgx_invalidate()
intel_sgx: invalidate enclave when the user threads cease to exist
Sean Christopherson (3):
intel_sgx: add LRU algorithm to page swapping
intel_sgx: return correct errno for EINIT ioctl
intel_sgx: lock the enclave for the entire EPC eviction flow
drivers/platform/x86/Kconfig | 1 +
drivers/platform/x86/intel_sgx.h | 6 ++-
drivers/platform/x86/intel_sgx_ioctl.c | 40 ++++++++++++++++--
drivers/platform/x86/intel_sgx_page_cache.c | 65 ++++++++++++++++++++---------
drivers/platform/x86/intel_sgx_util.c | 10 +++--
drivers/platform/x86/intel_sgx_vma.c | 8 +++-
6 files changed, 101 insertions(+), 29 deletions(-)
--
2.9.3
5 years, 10 months
[PATCH] intel_sgx: return correct errno for EINIT ioctl
by Sean Christopherson
Return EINVAL for any EINIT failure due to an invalid configuration,
e.g. attribute, measurement, signature, etc... Return EBUSY if and
only if EINIT failed due to an unmasked event. Return EFAULT in all
other cases.
Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com>
---
drivers/platform/x86/intel_sgx_ioctl.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c
index f24e19b..9d09447 100644
--- a/drivers/platform/x86/intel_sgx_ioctl.c
+++ b/drivers/platform/x86/intel_sgx_ioctl.c
@@ -817,7 +817,22 @@ static int __sgx_encl_init(struct sgx_encl *encl, char *sigstruct,
out:
if (ret) {
sgx_dbg(encl, "EINIT returned %d\n", ret);
- ret = -EBUSY;
+ switch (ret) {
+ case SGX_UNMASKED_EVENT:
+ ret = -EBUSY;
+ break;
+ case SGX_INVALID_SIG_STRUCT:
+ case SGX_INVALID_ATTRIBUTE:
+ case SGX_INVALID_MEASUREMENT:
+ case SGX_INVALID_SIGNATURE:
+ case SGX_INVALID_LICENSE:
+ case SGX_INVALID_CPUSVN:
+ ret = -EINVAL;
+ break;
+ default:
+ ret = -EFAULT;
+ break;
+ }
} else {
encl->flags |= SGX_ENCL_INITIALIZED;
--
2.7.4
5 years, 10 months
[PATCH] intel_sgx: add LRU algorithm to page swapping
by Sean Christopherson
Test and clear the associated `accessed` bit of an EPC page when
isolating pages during EPC page swap. Move accessed pages to the
end of the load list instead of the eviction list, i.e. isolate
only those pages that have not been accessed since the last time
the swapping flow was run.
This basic LRU algorithm yields a 15-20% improvement in throughput
when the system is under heavy EPC pressure.
Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com>
---
drivers/platform/x86/intel_sgx.h | 2 +-
drivers/platform/x86/intel_sgx_ioctl.c | 2 +-
drivers/platform/x86/intel_sgx_page_cache.c | 40 ++++++++++++++++++++++++++++-
drivers/platform/x86/intel_sgx_vma.c | 2 +-
4 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h
index 464763d..9b6924e 100644
--- a/drivers/platform/x86/intel_sgx.h
+++ b/drivers/platform/x86/intel_sgx.h
@@ -190,7 +190,7 @@ long sgx_compat_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
#endif
/* Utility functions */
-
+void sgx_activate_epc_page(struct sgx_encl_page *page, struct sgx_encl *encl);
void *sgx_get_epc_page(struct sgx_epc_page *entry);
void sgx_put_epc_page(void *epc_page_vaddr);
struct page *sgx_get_backing(struct sgx_encl *encl,
diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c
index f24e19b..cc018bc 100644
--- a/drivers/platform/x86/intel_sgx_ioctl.c
+++ b/drivers/platform/x86/intel_sgx_ioctl.c
@@ -287,7 +287,7 @@ static bool sgx_process_add_page_req(struct sgx_add_page_req *req)
}
encl_page->epc_page = epc_page;
- list_add_tail(&encl_page->load_list, &encl->load_list);
+ sgx_activate_epc_page(encl_page, encl);
mutex_unlock(&encl->lock);
sgx_unpin_mm(encl);
diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c
index 8b1cc82..325bb7a 100644
--- a/drivers/platform/x86/intel_sgx_page_cache.c
+++ b/drivers/platform/x86/intel_sgx_page_cache.c
@@ -77,6 +77,43 @@ static unsigned int sgx_nr_high_pages;
struct task_struct *ksgxswapd_tsk;
static DECLARE_WAIT_QUEUE_HEAD(ksgxswapd_waitq);
+
+static int sgx_test_and_clear_young_cb(pte_t *ptep, pgtable_t token,
+ unsigned long addr, void *data)
+{
+ int ret = pte_young(*ptep);
+ if (ret) {
+ pte_t pte = pte_mkold(*ptep);
+ set_pte_at((struct mm_struct *) data, addr, ptep, pte);
+ }
+ return ret;
+}
+
+/**
+ * sgx_test_and_clear_young() - Test and reset the accessed bit
+ * @page: enclave EPC page to be tested for recent access
+ * @encl: enclave which owns @page
+ *
+ * Checks the Access (A) bit from the PTE corresponding to the
+ * enclave page and clears it. Returns 1 if the page has been
+ * recently accessed and 0 if not.
+ */
+static int sgx_test_and_clear_young(struct sgx_encl_page *page, struct sgx_encl *encl)
+{
+ struct vm_area_struct *vma = sgx_find_vma(encl, page->addr);
+ if (!vma)
+ return 0;
+
+ return apply_to_page_range(vma->vm_mm, page->addr, PAGE_SIZE,
+ sgx_test_and_clear_young_cb, vma->vm_mm);
+}
+
+void sgx_activate_epc_page(struct sgx_encl_page *page, struct sgx_encl *encl)
+{
+ sgx_test_and_clear_young(page, encl);
+ list_add_tail(&page->load_list, &encl->load_list);
+}
+
static struct sgx_tgid_ctx *sgx_isolate_tgid_ctx(unsigned long nr_to_scan)
{
struct sgx_tgid_ctx *ctx = NULL;
@@ -166,7 +203,8 @@ static void sgx_isolate_pages(struct sgx_encl *encl,
struct sgx_encl_page,
load_list);
- if (!(entry->flags & SGX_ENCL_PAGE_RESERVED)) {
+ if (!sgx_test_and_clear_young(entry, encl) &&
+ !(entry->flags & SGX_ENCL_PAGE_RESERVED)) {
entry->flags |= SGX_ENCL_PAGE_RESERVED;
list_move_tail(&entry->load_list, dst);
} else {
diff --git a/drivers/platform/x86/intel_sgx_vma.c b/drivers/platform/x86/intel_sgx_vma.c
index 0649978..5d1c6da 100644
--- a/drivers/platform/x86/intel_sgx_vma.c
+++ b/drivers/platform/x86/intel_sgx_vma.c
@@ -265,7 +265,7 @@ static struct sgx_encl_page *sgx_vma_do_fault(struct vm_area_struct *vma,
/* Do not free */
epc_page = NULL;
- list_add_tail(&entry->load_list, &encl->load_list);
+ sgx_activate_epc_page(entry, encl);
out:
mutex_unlock(&encl->lock);
if (encl->flags & SGX_ENCL_SUSPEND)
--
2.7.4
5 years, 10 months