| Message ID | 1482437735-4722-5-git-send-email-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 22, 2016 at 12:15:33PM -0800, Sean Christopherson wrote: > Call sgx_free_encl_page directly from sgx_ewb to make it more clear > why sgx_free_encl_page is called with(out) SGX_FREE_SKIP_EREMOVE. > This removes the need to return success/failure from sgx_ewb, and > eliminates the associated tracking in sgx_write_pages. > > Do not invalidate the enclave if it is already dead. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > drivers/platform/x86/intel_sgx_page_cache.c | 65 +++++++++++++---------------- > 1 file changed, 29 insertions(+), 36 deletions(-) > > diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c > index f4833a0..45964b4 100644 > --- a/drivers/platform/x86/intel_sgx_page_cache.c > +++ b/drivers/platform/x86/intel_sgx_page_cache.c > @@ -232,6 +232,15 @@ static void sgx_etrack(struct sgx_epc_page *epc_page) > sgx_put_epc_page(epc); > } > > +static void sgx_free_encl_page(struct sgx_encl_page *encl_page, > + struct sgx_encl *encl, > + unsigned int flags) > +{ > + sgx_free_page(encl_page->epc_page, encl, flags); > + encl_page->epc_page = NULL; > + encl_page->flags &= ~SGX_ENCL_PAGE_RESERVED; > +} > + > static int __sgx_ewb(struct sgx_encl *encl, > struct sgx_encl_page *encl_page) > { > @@ -267,36 +276,29 @@ static int __sgx_ewb(struct sgx_encl *encl, > return ret; > } > > -static bool sgx_ewb(struct sgx_encl *encl, > - struct sgx_encl_page *entry) > +static void sgx_ewb(struct sgx_encl *encl, > + struct sgx_encl_page *encl_page) > { > - int ret = __sgx_ewb(encl, entry); > + int ret = __sgx_ewb(encl, encl_page); > > if (ret == SGX_NOT_TRACKED) { > /* slow path, IPI needed */ > smp_call_function(sgx_ipi_cb, NULL, 1); > - ret = __sgx_ewb(encl, entry); > + ret = __sgx_ewb(encl, encl_page); > } > > - if (ret) { > - /* make enclave inaccessible */ > - sgx_invalidate(encl); > - smp_call_function(sgx_ipi_cb, NULL, 1); > - if (ret > 0) > - sgx_err(encl, "EWB returned %d, enclave killed\n", ret); > - return false; > + if (ret == SGX_SUCCESS) { > + sgx_free_encl_page(encl_page, encl, SGX_FREE_SKIP_EREMOVE); return true; /Jarkko > + } else { > + sgx_free_encl_page(encl_page, encl, 0); > + if (!(encl->flags & SGX_ENCL_DEAD)) { > + /* make enclave inaccessible */ > + sgx_invalidate(encl); > + smp_call_function(sgx_ipi_cb, NULL, 1); > + if (ret > 0) > + sgx_err(encl, "EWB returned %d, enclave killed\n", ret); > + } > } > - > - return true; > -} > - > -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; > - entry->flags &= ~SGX_ENCL_PAGE_RESERVED; > } > > static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > @@ -304,7 +306,6 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > struct sgx_encl_page *entry; > struct sgx_encl_page *tmp; > struct vm_area_struct *evma; > - unsigned int free_flags; > > if (list_empty(src)) > return; > @@ -335,25 +336,17 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > load_list); > list_del(&entry->load_list); > > - free_flags = 0; > - > evma = sgx_find_vma(encl, entry->addr); > if (evma) { > - if (sgx_ewb(encl, entry)) > - free_flags = SGX_FREE_SKIP_EREMOVE; > + sgx_ewb(encl, entry); > encl->secs_child_cnt--; > + } else { > + sgx_free_encl_page(entry, encl, 0); > } > - > - sgx_free_encl_page(entry, encl, free_flags); > } > > - if (!encl->secs_child_cnt && (encl->flags & SGX_ENCL_INITIALIZED)) { > - free_flags = 0; > - if (sgx_ewb(encl, &encl->secs_page)) > - free_flags = SGX_FREE_SKIP_EREMOVE; > - > - sgx_free_encl_page(&encl->secs_page, encl, free_flags); > - } > + if (!encl->secs_child_cnt && (encl->flags & SGX_ENCL_INITIALIZED)) > + sgx_ewb(encl, &encl->secs_page); > > mutex_unlock(&encl->lock); > } > -- > 2.7.4 > > _______________________________________________ > intel-sgx-kernel-dev mailing list > intel-sgx-kernel-dev@lists.01.org > https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev
On Thu, 2016-12-29 at 14:58 +0200, Jarkko Sakkinen wrote: > On Thu, Dec 22, 2016 at 12:15:33PM -0800, Sean Christopherson wrote: > > > > Call sgx_free_encl_page directly from sgx_ewb to make it more clear > > why sgx_free_encl_page is called with(out) SGX_FREE_SKIP_EREMOVE. > > This removes the need to return success/failure from sgx_ewb, and > > eliminates the associated tracking in sgx_write_pages. > > > > Do not invalidate the enclave if it is already dead. > > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > --- > > drivers/platform/x86/intel_sgx_page_cache.c | 65 +++++++++++++---------------- > > 1 file changed, 29 insertions(+), 36 deletions(-) > > > > diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c > > index f4833a0..45964b4 100644 > > --- a/drivers/platform/x86/intel_sgx_page_cache.c > > +++ b/drivers/platform/x86/intel_sgx_page_cache.c > > @@ -232,6 +232,15 @@ static void sgx_etrack(struct sgx_epc_page *epc_page) > > sgx_put_epc_page(epc); > > } > > > > +static void sgx_free_encl_page(struct sgx_encl_page *encl_page, > > + struct sgx_encl *encl, > > + unsigned int flags) > > +{ > > + sgx_free_page(encl_page->epc_page, encl, flags); > > + encl_page->epc_page = NULL; > > + encl_page->flags &= ~SGX_ENCL_PAGE_RESERVED; > > +} > > + > > static int __sgx_ewb(struct sgx_encl *encl, > > struct sgx_encl_page *encl_page) > > { > > @@ -267,36 +276,29 @@ static int __sgx_ewb(struct sgx_encl *encl, > > return ret; > > } > > > > -static bool sgx_ewb(struct sgx_encl *encl, > > - struct sgx_encl_page *entry) > > +static void sgx_ewb(struct sgx_encl *encl, > > + struct sgx_encl_page *encl_page) > > { > > - int ret = __sgx_ewb(encl, entry); > > + int ret = __sgx_ewb(encl, encl_page); > > > > if (ret == SGX_NOT_TRACKED) { > > /* slow path, IPI needed */ > > smp_call_function(sgx_ipi_cb, NULL, 1); > > - ret = __sgx_ewb(encl, entry); > > + ret = __sgx_ewb(encl, encl_page); > > } > > > > - if (ret) { > > - /* make enclave inaccessible */ > > - sgx_invalidate(encl); > > - smp_call_function(sgx_ipi_cb, NULL, 1); > > - if (ret > 0) > > - sgx_err(encl, "EWB returned %d, enclave killed\n", ret); > > - return false; > > + if (ret == SGX_SUCCESS) { > > + sgx_free_encl_page(encl_page, encl, SGX_FREE_SKIP_EREMOVE); > return true; > > /Jarkko sgx_ewb no longer returns a bool.
On Tue, Jan 03, 2017 at 08:42:19AM -0800, Sean Christopherson wrote: > On Thu, 2016-12-29 at 14:58 +0200, Jarkko Sakkinen wrote: > > On Thu, Dec 22, 2016 at 12:15:33PM -0800, Sean Christopherson wrote: > > > > > > Call sgx_free_encl_page directly from sgx_ewb to make it more clear > > > why sgx_free_encl_page is called with(out) SGX_FREE_SKIP_EREMOVE. > > > This removes the need to return success/failure from sgx_ewb, and > > > eliminates the associated tracking in sgx_write_pages. > > > > > > Do not invalidate the enclave if it is already dead. > > > > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > > --- > > > drivers/platform/x86/intel_sgx_page_cache.c | 65 +++++++++++++---------------- > > > 1 file changed, 29 insertions(+), 36 deletions(-) > > > > > > diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c > > > index f4833a0..45964b4 100644 > > > --- a/drivers/platform/x86/intel_sgx_page_cache.c > > > +++ b/drivers/platform/x86/intel_sgx_page_cache.c > > > @@ -232,6 +232,15 @@ static void sgx_etrack(struct sgx_epc_page *epc_page) > > > sgx_put_epc_page(epc); > > > } > > > > > > +static void sgx_free_encl_page(struct sgx_encl_page *encl_page, > > > + struct sgx_encl *encl, > > > + unsigned int flags) > > > +{ > > > + sgx_free_page(encl_page->epc_page, encl, flags); > > > + encl_page->epc_page = NULL; > > > + encl_page->flags &= ~SGX_ENCL_PAGE_RESERVED; > > > +} > > > + > > > static int __sgx_ewb(struct sgx_encl *encl, > > > struct sgx_encl_page *encl_page) > > > { > > > @@ -267,36 +276,29 @@ static int __sgx_ewb(struct sgx_encl *encl, > > > return ret; > > > } > > > > > > -static bool sgx_ewb(struct sgx_encl *encl, > > > - struct sgx_encl_page *entry) > > > +static void sgx_ewb(struct sgx_encl *encl, > > > + struct sgx_encl_page *encl_page) > > > { > > > - int ret = __sgx_ewb(encl, entry); > > > + int ret = __sgx_ewb(encl, encl_page); > > > > > > if (ret == SGX_NOT_TRACKED) { > > > /* slow path, IPI needed */ > > > smp_call_function(sgx_ipi_cb, NULL, 1); > > > - ret = __sgx_ewb(encl, entry); > > > + ret = __sgx_ewb(encl, encl_page); > > > } > > > > > > - if (ret) { > > > - /* make enclave inaccessible */ > > > - sgx_invalidate(encl); > > > - smp_call_function(sgx_ipi_cb, NULL, 1); > > > - if (ret > 0) > > > - sgx_err(encl, "EWB returned %d, enclave killed\n", ret); > > > - return false; > > > + if (ret == SGX_SUCCESS) { > > > + sgx_free_encl_page(encl_page, encl, SGX_FREE_SKIP_EREMOVE); > > return true; > > > > /Jarkko > > sgx_ewb no longer returns a bool. I overlooked. Then just "return;". /Jarkko
diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c index f4833a0..45964b4 100644 --- a/drivers/platform/x86/intel_sgx_page_cache.c +++ b/drivers/platform/x86/intel_sgx_page_cache.c @@ -232,6 +232,15 @@ static void sgx_etrack(struct sgx_epc_page *epc_page) sgx_put_epc_page(epc); } +static void sgx_free_encl_page(struct sgx_encl_page *encl_page, + struct sgx_encl *encl, + unsigned int flags) +{ + sgx_free_page(encl_page->epc_page, encl, flags); + encl_page->epc_page = NULL; + encl_page->flags &= ~SGX_ENCL_PAGE_RESERVED; +} + static int __sgx_ewb(struct sgx_encl *encl, struct sgx_encl_page *encl_page) { @@ -267,36 +276,29 @@ static int __sgx_ewb(struct sgx_encl *encl, return ret; } -static bool sgx_ewb(struct sgx_encl *encl, - struct sgx_encl_page *entry) +static void sgx_ewb(struct sgx_encl *encl, + struct sgx_encl_page *encl_page) { - int ret = __sgx_ewb(encl, entry); + int ret = __sgx_ewb(encl, encl_page); if (ret == SGX_NOT_TRACKED) { /* slow path, IPI needed */ smp_call_function(sgx_ipi_cb, NULL, 1); - ret = __sgx_ewb(encl, entry); + ret = __sgx_ewb(encl, encl_page); } - if (ret) { - /* make enclave inaccessible */ - sgx_invalidate(encl); - smp_call_function(sgx_ipi_cb, NULL, 1); - if (ret > 0) - sgx_err(encl, "EWB returned %d, enclave killed\n", ret); - return false; + if (ret == SGX_SUCCESS) { + sgx_free_encl_page(encl_page, encl, SGX_FREE_SKIP_EREMOVE); + } else { + sgx_free_encl_page(encl_page, encl, 0); + if (!(encl->flags & SGX_ENCL_DEAD)) { + /* make enclave inaccessible */ + sgx_invalidate(encl); + smp_call_function(sgx_ipi_cb, NULL, 1); + if (ret > 0) + sgx_err(encl, "EWB returned %d, enclave killed\n", ret); + } } - - return true; -} - -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; - entry->flags &= ~SGX_ENCL_PAGE_RESERVED; } static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) @@ -304,7 +306,6 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) struct sgx_encl_page *entry; struct sgx_encl_page *tmp; struct vm_area_struct *evma; - unsigned int free_flags; if (list_empty(src)) return; @@ -335,25 +336,17 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) load_list); list_del(&entry->load_list); - free_flags = 0; - evma = sgx_find_vma(encl, entry->addr); if (evma) { - if (sgx_ewb(encl, entry)) - free_flags = SGX_FREE_SKIP_EREMOVE; + sgx_ewb(encl, entry); encl->secs_child_cnt--; + } else { + sgx_free_encl_page(entry, encl, 0); } - - sgx_free_encl_page(entry, encl, free_flags); } - if (!encl->secs_child_cnt && (encl->flags & SGX_ENCL_INITIALIZED)) { - free_flags = 0; - if (sgx_ewb(encl, &encl->secs_page)) - free_flags = SGX_FREE_SKIP_EREMOVE; - - sgx_free_encl_page(&encl->secs_page, encl, free_flags); - } + if (!encl->secs_child_cnt && (encl->flags & SGX_ENCL_INITIALIZED)) + sgx_ewb(encl, &encl->secs_page); mutex_unlock(&encl->lock); }
Call sgx_free_encl_page directly from sgx_ewb to make it more clear why sgx_free_encl_page is called with(out) SGX_FREE_SKIP_EREMOVE. This removes the need to return success/failure from sgx_ewb, and eliminates the associated tracking in sgx_write_pages. Do not invalidate the enclave if it is already dead. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- drivers/platform/x86/intel_sgx_page_cache.c | 65 +++++++++++++---------------- 1 file changed, 29 insertions(+), 36 deletions(-)