From patchwork Mon Nov 28 22:32:47 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sean Christopherson X-Patchwork-Id: 9450643 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0B7F36071C for ; Mon, 28 Nov 2016 22:32:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F1B3F2808F for ; Mon, 28 Nov 2016 22:32:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E6408280CF; Mon, 28 Nov 2016 22:32:56 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 4ED2E2808F for ; Mon, 28 Nov 2016 22:32:56 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 2822881D36 for ; Mon, 28 Nov 2016 14:32:56 -0800 (PST) X-Original-To: intel-sgx-kernel-dev@lists.01.org Delivered-To: intel-sgx-kernel-dev@lists.01.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4DDA381D36 for ; Mon, 28 Nov 2016 14:32:54 -0800 (PST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga102.jf.intel.com with ESMTP; 28 Nov 2016 14:32:53 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,565,1473145200"; d="scan'208";a="906485632" Received: from sjchrist-ts.jf.intel.com ([10.54.74.20]) by orsmga003.jf.intel.com with ESMTP; 28 Nov 2016 14:32:53 -0800 From: Sean Christopherson To: intel-sgx-kernel-dev@lists.01.org Date: Mon, 28 Nov 2016 14:32:47 -0800 Message-Id: <1480372367-12679-1-git-send-email-sean.j.christopherson@intel.com> X-Mailer: git-send-email 2.7.4 Subject: [intel-sgx-kernel-dev] [PATCH v2] intel_sgx: Add lock to make EPC eviction atomic X-BeenThere: intel-sgx-kernel-dev@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Project: Intel® Software Guard Extensions for Linux*: https://01.org/intel-software-guard-extensions" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-sgx-kernel-dev-bounces@lists.01.org Sender: "intel-sgx-kernel-dev" X-Virus-Scanned: ClamAV using ClamSMTP 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: T1: lock_mutex(E1) T1: ETRACK(E1) T1: unlock_mutex(E1) T2: lock_mutex(E1) T2: ETRACK(E1) fails Signed-off-by: Sean Christopherson --- 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); }